| Message ID | 20250610154609.1382818-1-jurenatomas@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Input: tca6416-keypad - Add OF support for driver instantiation | expand |
On Tue, 10 Jun 2025 17:46:10 +0200, Tomáš Juřena wrote: > From: Tomas Jurena <jurenatomas@gmail.com> > > Adds support for instantiating the tca6416-keypad driver via > Device Tree. If no platform data is present, the driver can now be > probed based on OF bindings. > > A corresponding Device Tree binding document is added at: > Documentation/devicetree/bindings/input/tca6416-keypad.yaml > > This allows the driver to be used in systems that rely solely on the > Device Tree for hardware description, such as embedded ARM platforms. > > Tested on Toradex Ixora 1.3A board and Apalis imx8 SOM. > > Signed-off-by: Tomas Jurena <jurenatomas@gmail.com> > --- > .../bindings/input/tca6416-keypad.yaml | 87 ++++++++++++++++++ > drivers/input/keyboard/tca6416-keypad.c | 88 +++++++++++++++++-- > 2 files changed, 170 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/input/tca6416-keypad.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/input/tca6416-keypad.yaml:9:13: [error] empty value in block mapping (empty-values) dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/tca6416-keypad.yaml: pinmask: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/tca6416-keypad.yaml: maintainers: None is not of type 'array' from schema $id: http://devicetree.org/meta-schemas/base.yaml# Error: Documentation/devicetree/bindings/input/tca6416-keypad.example.dts:35.40-41 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/input/tca6416-keypad.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1519: dt_binding_check] Error 2 make: *** [Makefile:248: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250610154609.1382818-1-jurenatomas@gmail.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hi Tomáš, kernel test robot noticed the following build warnings: [auto build test WARNING on dtor-input/next] [also build test WARNING on dtor-input/for-linus linus/master v6.16-rc1 next-20250611] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Tom-Ju-ena/Input-tca6416-keypad-Add-OF-support-for-driver-instantiation/20250611-094643 base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next patch link: https://lore.kernel.org/r/20250610154609.1382818-1-jurenatomas%40gmail.com patch subject: [PATCH] Input: tca6416-keypad - Add OF support for driver instantiation config: arm-randconfig-001-20250611 (https://download.01.org/0day-ci/archive/20250611/202506112236.gn3kTosZ-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 11.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250611/202506112236.gn3kTosZ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202506112236.gn3kTosZ-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/input/keyboard/tca6416-keypad.c: In function 'tca6416_parse_properties': >> drivers/input/keyboard/tca6416-keypad.c:194:12: warning: unused variable 'pin' [-Wunused-variable] 194 | u8 pin; | ^~~ vim +/pin +194 drivers/input/keyboard/tca6416-keypad.c 185 186 static struct tca6416_keys_platform_data * 187 tca6416_parse_properties(struct device *dev, uint8_t io_size) 188 { 189 static const char keymap_property[] = "linux,gpio-keymap"; 190 struct tca6416_keys_platform_data *pdata; 191 u32 keymap[TCA6416_MAX_IO_SIZE]; 192 struct tca6416_button *buttons; 193 int ret, i; > 194 u8 pin; 195 196 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); 197 if (!pdata) 198 return NULL; 199 200 ret = device_property_count_u32(dev, keymap_property); 201 if (ret <= 0) 202 return NULL; 203 204 pdata->nbuttons = ret; 205 if (pdata->nbuttons > io_size) 206 pdata->nbuttons = io_size; 207 208 ret = device_property_read_u32_array(dev, keymap_property, keymap, 209 pdata->nbuttons); 210 if (ret) 211 return NULL; 212 213 buttons = devm_kcalloc(dev, pdata->nbuttons, sizeof(*buttons), 214 GFP_KERNEL); 215 if (!buttons) 216 return NULL; 217 218 for (i = 0; i < pdata->nbuttons; i++) { 219 buttons[i].code = FIELD_GET(CFG_CODE, keymap[i]); 220 buttons[i].type = FIELD_GET(CFG_TYPE, keymap[i]); 221 buttons[i].active_low = FIELD_GET(CFG_ACTIVE_LOW, keymap[i]); 222 /* enable all inputs by default */ 223 pdata->pinmask |= BIT(i); 224 } 225 226 pdata->buttons = buttons; 227 228 pdata->rep = device_property_read_bool(dev, "autorepeat"); 229 /* we can ignore the result as by default all inputs are enabled */ 230 device_property_read_u16(dev, "pinmask", &pdata->pinmask); 231 pdata->use_polling = device_property_read_bool(dev, "polling"); 232 233 return pdata; 234 } 235
On Tue, Jun 10, 2025 at 01:15:42PM -0500, Rob Herring wrote: > On Tue, Jun 10, 2025 at 05:46:10PM +0200, Tomáš Juřena wrote: > > From: Tomas Jurena <jurenatomas@gmail.com> > > > > Adds support for instantiating the tca6416-keypad driver via > > Device Tree. If no platform data is present, the driver can now be > > probed based on OF bindings. > > > > A corresponding Device Tree binding document is added at: > > Documentation/devicetree/bindings/input/tca6416-keypad.yaml > > > > This allows the driver to be used in systems that rely solely on the > > Device Tree for hardware description, such as embedded ARM platforms. > > > > Tested on Toradex Ixora 1.3A board and Apalis imx8 SOM. > > We already have a GPIO driver for this chip. Would the > gpio-keys driver work here instead? Seems to work for > arch/arm/boot/dts/ti/omap/am3517-evm-ui.dtsi. > I was not aware of this. Anyway, I tested this today, and it seems to work just fine with my hardware. There is no need for this change from my side.
On Wed, Jun 11, 2025 at 06:08:07PM +0200, Tomas Jurena wrote: > On Tue, Jun 10, 2025 at 01:15:42PM -0500, Rob Herring wrote: > > On Tue, Jun 10, 2025 at 05:46:10PM +0200, Tomáš Juřena wrote: > > > From: Tomas Jurena <jurenatomas@gmail.com> > > > > > > Adds support for instantiating the tca6416-keypad driver via > > > Device Tree. If no platform data is present, the driver can now be > > > probed based on OF bindings. > > > > > > A corresponding Device Tree binding document is added at: > > > Documentation/devicetree/bindings/input/tca6416-keypad.yaml > > > > > > This allows the driver to be used in systems that rely solely on the > > > Device Tree for hardware description, such as embedded ARM platforms. > > > > > > Tested on Toradex Ixora 1.3A board and Apalis imx8 SOM. > > > > We already have a GPIO driver for this chip. Would the > > gpio-keys driver work here instead? Seems to work for > > arch/arm/boot/dts/ti/omap/am3517-evm-ui.dtsi. > > > I was not aware of this. Anyway, I tested this today, and it seems to work just fine with my hardware. There is no need for this change from my side. I wonder if we should simply remove tca6416-keypad driver. I guess it was created before we has a gpio driver for this block. Thanks.
diff --git a/Documentation/devicetree/bindings/input/tca6416-keypad.yaml b/Documentation/devicetree/bindings/input/tca6416-keypad.yaml new file mode 100644 index 000000000000..f050403c4dbe --- /dev/null +++ b/Documentation/devicetree/bindings/input/tca6416-keypad.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/tca6416-keypad.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI TCA6416 keypad + +maintainers: + +description: | + Texas Instruments TCA6416 IO expander as a keypad input device. + +allOf: + - $ref: input.yaml# + +properties: + compatible: + enum: + - ti,tca6416_keys + - ti,tca6408_keys + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + linux,gpio-keymap: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: | + Array of gpio keys provided by the driver instance. Each entry is a + bitfield holding configuration of the input key. The bitfield looks like + this: + +------------------------------------------------------------+ + | Bits | 31:18 | 17 | 16:14 | 13:10 | 9:0 | + | Function | reserved | active_low | type | reserved | code | + +------------------------------------------------------------+ + code - Linux key code + type - EV_KEY or EV_SW + active_low - Key is active in low state + + linux,keycodes: + minItems: 1 + maxItems: 16 + + autorepeat: + type: boolean + description: | + Enables the Linux input system's autorepeat feature on the input device. + + polling: + type: boolean + description: | + Forces driver to use polling mode instead of IRQ. + + pinmask: + description: | + Allows to disable certain keys. By default are all inputs enabled. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + keypad@21 { + compatible = "ti,tca6416_keys"; + reg = <0x21>; + interrupt-parent = <&gpio>; + interrupts = <26 IRQ_TYPE_EDGE_FALLING>; + linux,gpio-keymap = < + 0x24290, // active low, EV_KEY, 0, KEY_MACRO1 + 0x24291, // active low, EV_KEY, 1, KEY_MACRO2 + 0x24292, // active low, EV_KEY, 2, KEY_MACRO3 + >; + }; + }; + +... diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c index fbc674d7b9f0..8910498cf266 100644 --- a/drivers/input/keyboard/tca6416-keypad.c +++ b/drivers/input/keyboard/tca6416-keypad.c @@ -17,6 +17,7 @@ #include <linux/i2c.h> #include <linux/input.h> #include <linux/tca6416_keypad.h> +#include <linux/bitfield.h> #define TCA6416_INPUT 0 #define TCA6416_OUTPUT 1 @@ -24,6 +25,7 @@ #define TCA6416_DIRECTION 3 #define TCA6416_POLL_INTERVAL 100 /* msec */ +#define TCA6416_MAX_IO_SIZE 16 /* maximum number of inputs */ static const struct i2c_device_id tca6416_id[] = { { "tca6416-keys", 16, }, @@ -173,9 +175,67 @@ static int tca6416_setup_registers(struct tca6416_keypad_chip *chip) return 0; } +/* Configuration bitmap + * | 31:18 | 17 | 16:14 | 13:10 | 9:0 | + * | reserved | active_low | type | reserved | code | + */ +#define CFG_CODE GENMASK(9, 0) +#define CFG_TYPE GENMASK(16, 14) +#define CFG_ACTIVE_LOW BIT(17) + +static struct tca6416_keys_platform_data * +tca6416_parse_properties(struct device *dev, uint8_t io_size) +{ + static const char keymap_property[] = "linux,gpio-keymap"; + struct tca6416_keys_platform_data *pdata; + u32 keymap[TCA6416_MAX_IO_SIZE]; + struct tca6416_button *buttons; + int ret, i; + u8 pin; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + ret = device_property_count_u32(dev, keymap_property); + if (ret <= 0) + return NULL; + + pdata->nbuttons = ret; + if (pdata->nbuttons > io_size) + pdata->nbuttons = io_size; + + ret = device_property_read_u32_array(dev, keymap_property, keymap, + pdata->nbuttons); + if (ret) + return NULL; + + buttons = devm_kcalloc(dev, pdata->nbuttons, sizeof(*buttons), + GFP_KERNEL); + if (!buttons) + return NULL; + + for (i = 0; i < pdata->nbuttons; i++) { + buttons[i].code = FIELD_GET(CFG_CODE, keymap[i]); + buttons[i].type = FIELD_GET(CFG_TYPE, keymap[i]); + buttons[i].active_low = FIELD_GET(CFG_ACTIVE_LOW, keymap[i]); + /* enable all inputs by default */ + pdata->pinmask |= BIT(i); + } + + pdata->buttons = buttons; + + pdata->rep = device_property_read_bool(dev, "autorepeat"); + /* we can ignore the result as by default all inputs are enabled */ + device_property_read_u16(dev, "pinmask", &pdata->pinmask); + pdata->use_polling = device_property_read_bool(dev, "polling"); + + return pdata; +} + static int tca6416_keypad_probe(struct i2c_client *client) { - const struct i2c_device_id *id = i2c_client_get_device_id(client); + uint8_t io_size = (uintptr_t)i2c_get_match_data(client); struct tca6416_keys_platform_data *pdata; struct tca6416_keypad_chip *chip; struct input_dev *input; @@ -190,9 +250,13 @@ static int tca6416_keypad_probe(struct i2c_client *client) } pdata = dev_get_platdata(&client->dev); - if (!pdata) { - dev_dbg(&client->dev, "no platform data\n"); - return -EINVAL; + if (!pdata && dev_fwnode(&client->dev)) { + pdata = tca6416_parse_properties(&client->dev, io_size); + if (!pdata) { + dev_err(&client->dev, + "Failed to parse device configuration from properties\n"); + return -EINVAL; + } } chip = devm_kzalloc(&client->dev, @@ -207,7 +271,7 @@ static int tca6416_keypad_probe(struct i2c_client *client) chip->client = client; chip->input = input; - chip->io_size = id->driver_data; + chip->io_size = io_size; chip->pinmask = pdata->pinmask; chip->use_polling = pdata->use_polling; @@ -279,9 +343,23 @@ static int tca6416_keypad_probe(struct i2c_client *client) return 0; } +static const struct of_device_id tca6416_of_match[] = { + { + .compatible = "ti,tca6416_keys", + .data = (void *)16, + }, + { + .compatible = "ti,tca6408_keys", + .data = (void *)8, + }, + {} +}; +MODULE_DEVICE_TABLE(of, tca6416_of_match); + static struct i2c_driver tca6416_keypad_driver = { .driver = { .name = "tca6416-keypad", + .of_match_table = tca6416_of_match, }, .probe = tca6416_keypad_probe, .id_table = tca6416_id,