[v5,2/2] Input: msg2638 - Add support for msg2138 key events

  Some devices with msg2138 have back/menu/home keys.
Add support for them.

Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
---
 drivers/input/touchscreen/msg2638.c | 57 +++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)
  

Hi Vincent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on next-20221116]
[cannot apply to dtor-input/for-linus robh/for-next krzk-dt/for-next linus/master v6.1-rc5]
[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/Vincent-Knecht/Input-msg2638-Add-support-for-msg2138-and-key-events/20221117-021842
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20221116181715.2118436-3-vincent.knecht%40mailoo.org
patch subject: [PATCH v5 2/2] Input: msg2638 - Add support for msg2138 key events
config: m68k-allyesconfig
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/290da623ffac857edd71f373a1c152e0fd71f593
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Vincent-Knecht/Input-msg2638-Add-support-for-msg2138-and-key-events/20221117-021842
        git checkout 290da623ffac857edd71f373a1c152e0fd71f593
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/input/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/acpi.h:15,
                    from include/linux/i2c.h:13,
                    from drivers/input/touchscreen/msg2638.c:16:
   drivers/input/touchscreen/msg2638.c: In function 'msg2638_ts_probe':
>> drivers/input/touchscreen/msg2638.c:407:31: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'unsigned int' [-Wformat=]
     407 |                 dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
         |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
     146 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/input/touchscreen/msg2638.c:407:17: note: in expansion of macro 'dev_warn'
     407 |                 dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
         |                 ^~~~~~~~
   drivers/input/touchscreen/msg2638.c:407:69: note: format string is defined here
     407 |                 dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
         |                                                                   ~~^
         |                                                                     |
         |                                                                     long int
         |                                                                   %d


vim +407 drivers/input/touchscreen/msg2638.c

   355	
   356	static int msg2638_ts_probe(struct i2c_client *client)
   357	{
   358		const struct msg_chip_data *chip_data;
   359		struct device *dev = &client->dev;
   360		struct msg2638_ts_data *msg2638;
   361		int error;
   362	
   363		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
   364			dev_err(dev, "Failed to assert adapter's support for plain I2C.\n");
   365			return -ENXIO;
   366		}
   367	
   368		msg2638 = devm_kzalloc(dev, sizeof(*msg2638), GFP_KERNEL);
   369		if (!msg2638)
   370			return -ENOMEM;
   371	
   372		msg2638->client = client;
   373		i2c_set_clientdata(client, msg2638);
   374	
   375		chip_data = device_get_match_data(&client->dev);
   376		if (!chip_data || !chip_data->max_fingers) {
   377			dev_err(dev, "Invalid or missing chip data\n");
   378			return -EINVAL;
   379		}
   380	
   381		msg2638->max_fingers = chip_data->max_fingers;
   382	
   383		msg2638->supplies[0].supply = "vdd";
   384		msg2638->supplies[1].supply = "vddio";
   385		error = devm_regulator_bulk_get(dev, ARRAY_SIZE(msg2638->supplies),
   386						msg2638->supplies);
   387		if (error) {
   388			dev_err(dev, "Failed to get regulators: %d\n", error);
   389			return error;
   390		}
   391	
   392		msg2638->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
   393		if (IS_ERR(msg2638->reset_gpiod)) {
   394			error = PTR_ERR(msg2638->reset_gpiod);
   395			dev_err(dev, "Failed to request reset GPIO: %d\n", error);
   396			return error;
   397		}
   398	
   399		msg2638->num_keycodes = fwnode_property_count_u32(dev->fwnode, "linux,keycodes");
   400		if (msg2638->num_keycodes == -EINVAL) {
   401			msg2638->num_keycodes = 0;
   402		} else if (msg2638->num_keycodes < 0) {
   403			dev_err(dev, "Unable to parse linux,keycodes property: %d\n",
   404				msg2638->num_keycodes);
   405			return msg2638->num_keycodes;
   406		} else if (msg2638->num_keycodes > ARRAY_SIZE(msg2638->keycodes)) {
 > 407			dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
   408				 msg2638->num_keycodes, ARRAY_SIZE(msg2638->keycodes));
   409			msg2638->num_keycodes = ARRAY_SIZE(msg2638->keycodes);
   410		}
   411	
   412		error = fwnode_property_read_u32_array(dev->fwnode, "linux,keycodes",
   413						       msg2638->keycodes, msg2638->num_keycodes);
   414		if (error) {
   415			dev_err(dev, "Unable to read linux,keycodes values: %d\n", error);
   416			return error;
   417		}
   418	
   419		error = devm_request_threaded_irq(dev, client->irq,
   420						  NULL, chip_data->irq_handler,
   421						  IRQF_ONESHOT | IRQF_NO_AUTOEN,
   422						  client->name, msg2638);
   423		if (error) {
   424			dev_err(dev, "Failed to request IRQ: %d\n", error);
   425			return error;
   426		}
   427	
   428		error = msg2638_init_input_dev(msg2638);
   429		if (error) {
   430			dev_err(dev, "Failed to initialize input device: %d\n", error);
   431			return error;
   432		}
   433	
   434		return 0;
   435	}
   436
  

On Wed, Nov 16, 2022 at 07:17:12PM +0100, Vincent Knecht wrote:
> Some devices with msg2138 have back/menu/home keys.
> Add support for them.
> 
> Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> ---
>  drivers/input/touchscreen/msg2638.c | 57 +++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/msg2638.c b/drivers/input/touchscreen/msg2638.c
> index 95b18563326a..a0f5e1ecd612 100644
> --- a/drivers/input/touchscreen/msg2638.c
> +++ b/drivers/input/touchscreen/msg2638.c
> @@ -21,6 +21,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/property.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> @@ -29,6 +30,8 @@
>  #define MSG2138_MAX_FINGERS		2
>  #define MSG2638_MAX_FINGERS		5
>  
> +#define MAX_BUTTONS			4
> +
>  #define CHIP_ON_DELAY_MS		15
>  #define FIRMWARE_ON_DELAY_MS		50
>  #define RESET_DELAY_MIN_US		10000
> @@ -72,6 +75,8 @@ struct msg2638_ts_data {
>  	struct regulator_bulk_data supplies[2];
>  	struct gpio_desc *reset_gpiod;
>  	int max_fingers;
> +	u32 keycodes[MAX_BUTTONS];
> +	int num_keycodes;
>  };
>  
>  static u8 msg2638_checksum(u8 *data, u32 length)
> @@ -85,6 +90,18 @@ static u8 msg2638_checksum(u8 *data, u32 length)
>  	return (u8)((-sum) & 0xFF);
>  }
>  
> +static void msg2138_report_keys(struct msg2638_ts_data *msg2638, u8 keys)
> +{
> +	int i;
> +
> +	/* keys can be 0x00 or 0xff when all keys have been released */
> +	if (keys == 0xff)
> +		keys = 0;
> +
> +	for (i = 0; i < msg2638->num_keycodes; ++i)
> +		input_report_key(msg2638->input_dev, msg2638->keycodes[i], keys & BIT(i));
> +}
> +
>  static irqreturn_t msg2138_ts_irq_handler(int irq, void *msg2638_handler)
>  {
>  	struct msg2638_ts_data *msg2638 = msg2638_handler;
> @@ -121,9 +138,12 @@ static irqreturn_t msg2138_ts_irq_handler(int irq, void *msg2638_handler)
>  	p0 = &touch_event.pkt[0];
>  	p1 = &touch_event.pkt[1];
>  
> -	/* Ignore non-pressed finger data */
> -	if (p0->xy_hi == 0xFF && p0->x_low == 0xFF && p0->y_low == 0xFF)
> +	/* Ignore non-pressed finger data, but check for key code */
> +	if (p0->xy_hi == 0xFF && p0->x_low == 0xFF && p0->y_low == 0xFF) {
> +		if (p1->xy_hi == 0xFF && p1->y_low == 0xFF)
> +			msg2138_report_keys(msg2638, p1->x_low);
>  		goto report;
> +	}
>  
>  	x = ((p0->xy_hi & 0xF0) << 4) | p0->x_low;
>  	y = ((p0->xy_hi & 0x0F) << 8) | p0->y_low;
> @@ -283,6 +303,7 @@ static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638)
>  	struct device *dev = &msg2638->client->dev;
>  	struct input_dev *input_dev;
>  	int error;
> +	int i;
>  
>  	input_dev = devm_input_allocate_device(dev);
>  	if (!input_dev) {
> @@ -299,6 +320,14 @@ static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638)
>  	input_dev->open = msg2638_input_open;
>  	input_dev->close = msg2638_input_close;
>  
> +	if (msg2638->num_keycodes) {
> +		input_dev->keycode = msg2638->keycodes;
> +		input_dev->keycodemax = msg2638->num_keycodes;
> +		input_dev->keycodesize = sizeof(msg2638->keycodes[0]);
> +		for (i = 0; i < msg2638->num_keycodes; i++)
> +			input_set_capability(input_dev, EV_KEY, msg2638->keycodes[i]);
> +	}
> +
>  	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
>  	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
>  
> @@ -367,9 +396,23 @@ static int msg2638_ts_probe(struct i2c_client *client)
>  		return error;
>  	}
>  
> -	error = msg2638_init_input_dev(msg2638);
> +	msg2638->num_keycodes = fwnode_property_count_u32(dev->fwnode, "linux,keycodes");

Please use device_property_count_u32().

> +	if (msg2638->num_keycodes == -EINVAL) {
> +		msg2638->num_keycodes = 0;
> +	} else if (msg2638->num_keycodes < 0) {
> +		dev_err(dev, "Unable to parse linux,keycodes property: %d\n",
> +			msg2638->num_keycodes);
> +		return msg2638->num_keycodes;
> +	} else if (msg2638->num_keycodes > ARRAY_SIZE(msg2638->keycodes)) {
> +		dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
> +			 msg2638->num_keycodes, ARRAY_SIZE(msg2638->keycodes));

I think you want "%zd" in place of "%ld".

> +		msg2638->num_keycodes = ARRAY_SIZE(msg2638->keycodes);
> +	}
> +
> +	error = fwnode_property_read_u32_array(dev->fwnode, "linux,keycodes",
> +					       msg2638->keycodes, msg2638->num_keycodes);

Please use device_property_read_u32_array().

>  	if (error) {
> -		dev_err(dev, "Failed to initialize input device: %d\n", error);
> +		dev_err(dev, "Unable to read linux,keycodes values: %d\n", error);
>  		return error;
>  	}
>  
> @@ -382,6 +425,12 @@ static int msg2638_ts_probe(struct i2c_client *client)
>  		return error;
>  	}
>  
> +	error = msg2638_init_input_dev(msg2638);
> +	if (error) {
> +		dev_err(dev, "Failed to initialize input device: %d\n", error);
> +		return error;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.38.1
> 
> 
> 

Thanks.