Re: [PATCH v5 2/2] input: zinitix: Add touchkey support

[Nikita Travkin <nikita@trvn.ru>]

Dmitry Torokhov писал(а) 20.07.2024 05:29:
> On Wed, Jul 17, 2024 at 06:55:34PM +0500, Nikita Travkin wrote:
>> Zinitix touch controllers can use some of the sense lines for virtual
>> keys (like those found on many phones). Add support for those keys.
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> 
> Applied, thank you. However:
> 
>> -->
>> +	if (le16_to_cpu(touch_event.status) & BIT_ICON_EVENT) {
>> +		error = zinitix_read_data(bt541->client, ZINITIX_ICON_STATUS_REG,
>> +					  &icon_events, sizeof(icon_events));
>> +		if (error) {
>> +			dev_err(&client->dev, "Failed to read icon events\n");
>> +			goto out;
>> +		}
> 
> I wonder, would it make sense (and be more efficient) to issue a single
> read of size sizeof(struct touch_event) + sizeof(icon_events) and the
> parse the data based on touch_event.status?

Maybe, but I would be really hesitant to such a change: Original driver
also makes a dedicated read for the "icon" data and per my understanding,
those "register reads" may also not be really "register" based but rather
kind of "command" based, where controller will start streaming the data
based on the request for the specific "register". In this case i'd prefer
to not accidentally confuse the touch firmware by over-reading the data,
if its somehow firmware-version-defined.

Thanks for giving it a look and picking this up!
Nikita

> 
> Thanks.