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] [day] [month] [year] [list]

Message-ID: <e480fbc14bae5e2e24f2fec3d7be308c@disroot.org>
Date: Mon, 30 Jun 2025 18:44:34 +0000
From: Kaustabh Chakraborty <kauschluss@...root.org>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
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

On 2025-06-29 05:12, Dmitry Torokhov wrote:
> 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.

Yeah, you're correct. Though I'm not sure this is the correct way to do
what I'm trying to do. What I want is a driver which will drive both
touchscreen and pure-touchkey devices.

I feel like a separate driver is how it should be done, although the
protocol is same and it can be done here...

I had tried to get some input (hence the RFC) about this but I couldn't.
Either my wording wasn't clear or I haven't been able to communicate it
properly.

Sorry for bringing this out of the blue in a thread.

> 
> 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.

Agree, will do so.

> 
>>  	}
>> 
>>  	/* 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