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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch fails to eliminate the phantom input node and KEY_BRIGHTNESSUP autorepeat events because the selected multitouch class (MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU) explicitly exports all unknown inputs.
--

--- Patch [1]: [PATCH v2] HID: multitouch: add support for Goodix GXTP7863 touchpad ---
commit 27cf212c4e16014a4475e61f74a677cd87497bef
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 in hid-multitouch. It uses MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU with the
intent of claiming the device to eliminate an erroneous phantom input node
that causes endless KEY_BRIGHTNESSUP autorepeat events.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index edb37b4c867e4..ca0b40e6b8a83 100644
> --- 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_WIN_8_FORCE_MULTI_INPUT_NSMU,
> +	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,
> +		     I2C_DEVICE_ID_GOODIX_01E0) },

[Severity: High]
Will this actually prevent the phantom KEY_BRIGHTNESSUP events?

Because MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU sets export_all_inputs = true,
the filter in drivers/hid/hid-multitouch.c:mt_input_mapping() is bypassed
for the vendor collection:

    if (!td->mtclass.export_all_inputs &&
        field->application != HID_DG_TOUCHSCREEN &&
    ...
        return -1;

This causes mt_input_mapping() to return 0, delegating the mapping to
hid-input.c which still maps the telemetry usage to KEY_BRIGHTNESSUP.

Then, when the telemetry report arrives, mt_event() returns 0 for this
non-MT collection:

    rdata = mt_find_report_data(td, field->report);
    if (rdata && rdata->is_mt_collection)
        return mt_touch_event(hid, field, usage, value);

    return 0;

This allows hidinput_hid_event() to process it and emit the phantom
keypresses, which seems to leave the system in the same state it was
under hid-generic.

Would a different class that doesn't force export_all_inputs be more
appropriate here to successfully ignore the vendor telemetry?

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