linux-kernel - Re: [PATCH RFC v2 3/5] Input: melfas-mip4 - add support for touchkey input events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ugwy3adqmxodsyhohpdv337lvbxpdzhgtojpbtrykkuyf2eivl@sl36qsvcju6v>
Date: Sat, 28 Jun 2025 22:12:12 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Kaustabh Chakraborty <kauschluss@...root.org>
Cc: Sangwon Jee <jeesw@...fas.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Henrik Rydberg <rydberg@...math.org>, linux-input@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v2 3/5] Input: melfas-mip4 - add support for touchkey
 input events

Hi Kaustabh,

On Fri, Jun 13, 2025 at 01:11:35AM +0530, Kaustabh Chakraborty wrote:
> The MIP4 protocol are supposed to support touchscreens, touchkeys, and
> combo-devices. The driver handles touchscreen events, but touchkey
> events are unimplemented.

I am confused, because I clearly see the driver parsing and forwarding
key events. It appears that this patch adds the ability to set the
keymap via device tree instead of relying on userspace to load it.

Please adjust the patch description.

> 
> Implement them. If compiled with devicetree support, the driver attempts
> to retrieve keycodes from a property named "linux,keycodes".
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss@...root.org>
> ---
>  drivers/input/touchscreen/melfas_mip4.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/melfas_mip4.c b/drivers/input/touchscreen/melfas_mip4.c
> index a6946e3d8376d7e9b4c26f4194409e0ba78bb075..061ac353bc7a2e28f17581411af81f35c89733a1 100644
> --- a/drivers/input/touchscreen/melfas_mip4.c
> +++ b/drivers/input/touchscreen/melfas_mip4.c
> @@ -169,7 +169,7 @@ struct mip4_ts {
>  	unsigned int event_format;
>  
>  	unsigned int key_num;
> -	unsigned short key_code[MIP4_MAX_KEYS];
> +	unsigned int key_code[MIP4_MAX_KEYS];
>  
>  	bool wake_irq_enabled;
>  
> @@ -337,8 +337,13 @@ static int mip4_query_device(struct mip4_ts *ts)
>  			ts->ppm_x, ts->ppm_y);
>  
>  		/* Key ts */
> -		if (ts->node_key > 0)
> +		if (ts->node_key > MIP4_MAX_KEYS) {
> +			dev_warn(&ts->client->dev,
> +				"Too many keys (%u) found\n", ts->node_key);
> +			ts->key_num = MIP4_MAX_KEYS;
> +		} else {
>  			ts->key_num = ts->node_key;
> +		}

I believe this is a bugfix. Please extract it into a separate patch.

>  	}
>  
>  	/* Protocol */
> @@ -1080,6 +1085,7 @@ static int mip4_flash_fw(struct mip4_ts *ts,
>  			 const u8 *fw_data, u32 fw_size, u32 fw_offset)
>  {
>  	struct i2c_client *client = ts->client;
> +	unsigned int i;
>  	int offset;
>  	u16 buf_addr;
>  	int error, error2;
> @@ -1149,6 +1155,11 @@ static int mip4_flash_fw(struct mip4_ts *ts,
>  	input_abs_set_res(ts->input, ABS_X, ts->ppm_x);
>  	input_abs_set_res(ts->input, ABS_Y, ts->ppm_y);
>  
> +	for (i = 0; i < ts->key_num; i++) {
> +		if (ts->key_code[i])
> +			input_set_capability(ts->input, EV_KEY, ts->key_code[i]);
> +	}
> +
>  	return error ? error : 0;
>  }
>  
> @@ -1425,6 +1436,7 @@ static int mip4_probe(struct i2c_client *client)
>  {
>  	struct mip4_ts *ts;
>  	struct input_dev *input;
> +	unsigned int i;
>  	int error;
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> @@ -1471,6 +1483,17 @@ static int mip4_probe(struct i2c_client *client)
>  
>  	input_set_drvdata(input, ts);
>  
> +#ifdef CONFIG_OF
> +	error = of_property_read_u32_array(client->dev.of_node, "linux,keycodes",
> +					   ts->key_code, ts->key_num);
> +	if (error && ts->key_num) {
> +		dev_warn(&client->dev,
> +			 "Failed to get codes for %u supported keys", ts->key_num);
> +		/* Disable touchkey support */
> +		ts->key_num = 0;
> +	}

Please use generic device properties (device_property_read_u32_array())
and drop the dependency on OF.

> +#endif
> +
>  	input->keycode = ts->key_code;
>  	input->keycodesize = sizeof(*ts->key_code);
>  	input->keycodemax = ts->key_num;
> @@ -1491,6 +1514,11 @@ static int mip4_probe(struct i2c_client *client)
>  	if (error)
>  		return error;
>  
> +	for (i = 0; i < ts->key_num; i++) {
> +		if (ts->key_code[i])
> +			input_set_capability(input, EV_KEY, ts->key_code[i]);
> +	}
> +
>  	i2c_set_clientdata(client, ts);
>  
>  	error = devm_request_threaded_irq(&client->dev, client->irq,
> 

Thanks.

-- 
Dmitry