Re: [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver fails to restore the device to multitouch mode after a system suspend/resume cycle.
- [High] In USB mode, if more than two fingers are active, the driver assigns identical coordinates to all fingers after the first one.
--

commit d5ae02b47586e24db0bca9c6a7b552b9fc7ef6e4
Author: Damien Dejean <damiendejean@google.com>

HID: elan: Add support for ELAN SB974D touchpad

This commit adds support for the ELAN SB974D touchpad by registering its
device ID in the supported devices list so the driver can parse its
vendor-specific formats.

> diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
> index 76d93fc48f6a2..0190ad567ce4d 100644
> --- a/drivers/hid/hid-elan.c
> +++ b/drivers/hid/hid-elan.c
> @@ -513,6 +513,7 @@ static const struct hid_device_id elan_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_HP_X2_10_COVER),
>  	  .driver_data = ELAN_HAS_LED },
>  	{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_TOSHIBA_CLICK_L9W) },
> +	{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_SB974D) },

[Severity: High]
This is a pre-existing issue, but does the driver fail to restore the device
to multitouch mode after a system suspend/resume cycle?

The ELAN touchpads require a vendor-specific initialization command
(elan_start_multitouch) to disable mouse emulation and enable multitouch
mode. This sequence is currently only executed during the initial elan_probe.

The hid_driver struct for elan_driver does not provide resume or reset_resume
callbacks. When the system suspends and the device loses power (both in USB
and I2C modes), it reverts to its default mouse emulation mode.

Upon resume, the core HID drivers call hid_driver_reset_resume(), which acts
as a no-op since the callback is missing and returns success without rebinding.

Does this leave the touchpad permanently in a degraded state after a
suspend/resume cycle?

[Severity: High]
This is a pre-existing issue, but in USB mode, if more than two fingers are
active, does the driver assign identical coordinates to all fingers after the
first one?

Looking at elan_usb_report_input(), when parsing the ELAN_MT_SECOND_FINGER
packet, the driver iterates over ELAN_MAX_FINGERS to dispatch slot updates
based on the active touch bits in prev_report[2].

For the first active bit found, it correctly uses coordinates from
prev_report + 3. For all subsequent active bits, the else branch executes:

    elan_report_mt_slot(drvdata, data + 1, i);

Because data + 1 is passed directly each time without incrementing any offset,
are the exact same coordinates and pressure values assigned to all fingers
after the first?

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(hid, elan_devices);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-28-sashal@kernel.org?part=1