Re: [PATCH v3] HID: multitouch: add support for Goodix GXTP7863 touchpad

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `MT_CLS_DEFAULT` instead of a Win8 PTP class disables critical Windows 8 Precision Touchpad (PTP) quirks, causing severe regressions in touchpad functionality (such as broken clickpad button tracking and stuck finger releases).
--

--- Patch [1]: [PATCH v3] HID: multitouch: add support for Goodix GXTP7863 touchpad ---
commit 04179c956954e748355b634f8668ad96cef6871c
Author: Ruzal Daminov <[email protected]>

HID: multitouch: add support for Goodix GXTP7863 touchpad

This commit adds the Goodix GXTP7863 touchpad controller to the mt_devices[]
table so it is claimed by hid-multitouch instead of hid-generic. It uses
MT_CLS_DEFAULT to prevent the vendor telemetry collection (0xFF01) from
being mapped to an input node.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c

[ ... ]

> @@ -2460,6 +2460,11 @@ static const struct hid_device_id mt_devices[] = {
>  		MT_BT_DEVICE(USB_VENDOR_ID_FRUCTEL,
>  			USB_DEVICE_ID_GAMETEL_MT_MODE) },
>  
> +	/* Goodix GXTP7863 Touchpad */
> +	{ .driver_data = MT_CLS_DEFAULT,
> +	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,
> +		     I2C_DEVICE_ID_GOODIX_01E0) },
> +

[Severity: High]
Does using MT_CLS_DEFAULT here disable necessary Windows 8 Precision
Touchpad quirks?

While it successfully prevents export_all_inputs from mapping the telemetry
node, MT_CLS_DEFAULT lacks quirks like MT_QUIRK_WIN8_PTP_BUTTONS and
MT_QUIRK_STICKY_FINGERS.

Without MT_QUIRK_WIN8_PTP_BUTTONS, will mt_sync_frame() fail to emit
BTN_LEFT events and break physical left-clicks?

Without MT_QUIRK_STICKY_FINGERS, will mt_touch_report() fail to arm the
release_timer, potentially leaving touches stuck if the firmware drops a
release report?

The file-level documentation in mt_devices mentions that there is no
point in adding a device with MT_CLS_DEFAULT. Is there a better way to
ignore the vendor telemetry collection without losing the PTP quirks?

>  	/* Goodix GT7868Q devices */
>  	{ .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU,
>  	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1