All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ruzal <daminovruzal7@gmail.com>
To: Benjamin Tissoires <bentiss@kernel.org>
Cc: jikos@kernel.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] HID: multitouch: add support for Goodix GXTP7863 touchpad
Date: Fri, 18 Sep 2026 21:01:52 +0300	[thread overview]
Message-ID: <33a7c239-ede5-4091-ba72-ff429d97013c@gmail.com> (raw)
In-Reply-To: <aqwUYWPz_nORoFr6@beelink>

On 9/18/26 7:18 PM, Benjamin Tissoires wrote:

> On Sep 17 2026, Ruzal wrote:
>> On 9/17/26 10:30 AM, Benjamin Tissoires wrote:
>>> Hi,
>>>
>>> sorry it looks like this one fell through the cracks.
>>>
>>> On Aug 19 2026, Ruzal Daminov wrote:
>>>> The Goodix GXTP7863 touchpad controller (VID: 0x27c6, PID: 0x01e0)
>>>> found on Honor MagicBook laptops (e.g. FMI-76 / X14 Plus)
>>>> was missing from the mt_devices[] table.
>>>>
> [...]
>>>>  
>>>> +	/* Goodix GXTP7863 Touchpad */
>>>> +	{ .driver_data = MT_CLS_WIN_8,
>>>> +	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,
>>> I'm puzzled here: HID_GROUP_ANY? How can this even be working with hid-multitouch?
>>>
>>> Are you sure the touchpad part is not already handled by hid-multitouch, but only
>>> the telemetry/phantom event gets assigned to hid-generic?
>>>
>>> Can you share the report descriptors of all nodes with hid-recorder so I
>>> can understand why we suddenly have to map to non multitouch devices.
>>>
>>> Cheers,
>>> Benjamin
>> Hi Benjamin,
>>
>> Thanks for pointing that out. You were completely right, and I apologize
>> for the inaccurate explanation in my previous commit messages.
> no need to apologize, we can make mistakes, and that's our job as
> maintainers to catch them :)
>
>> I re-checked the device binding: the touchpad is indeed claimed by
>> hid-multitouch out of the box under HID_GROUP_MULTITOUCH_WIN_8 (using
>> MT_CLS_WIN_8). It was never claimed by hid-generic.
> Well, I guess it was the first time you started the laptop, until
> hid-multitouch gets loaded.
>
>> The actual root cause is that MT_CLS_WIN_8 sets `.export_all_inputs = true`.
>> Because of this, hid-multitouch exports the vendor application collection
>> (Usage Page 0xFF01, Usage 0x01, Report ID 8) to an input node.
> Actually, it's even worse than that.
>
> Because some vendors are not using standard HID usages, we do have a
> `HID_UP_HPVENDOR2` definition of (Usage Page 0xFF01). And hid-input.c
> considers that this is only used by HP laptops/keyboards, and then we
> have some fancy mapping of non standard usages.
>
> The real fix would be to actually ensure this mapping is only triggered
> for:
> - keyboards
> - HP platforms
>
> But, there is always a but, `HID_UP_HPVENDOR` was there since the origin
> of time (git conversion IIRC), and `HID_UP_HPVENDOR2` appeared in 2012.
>
> We do have some info on the affected devices (VID/PID) but nothing else
> like the report descriptor. So it's the typical case of "damn, we can't
> fix this without possibly regressing a lot of existing hardware".
>
>> As captured in the hid-recorder trace below, the firmware sends a 1-Hz
>> heartbeat packet on Report ID 8:
>>   E: 000000.000000 30 08 ab 00 00 2a ...
>>   E: 000001.006503 30 08 ab 00 00 2a ...
>>   E: 000002.012748 30 08 ab 00 00 2a ...
>>
>> hid-input interprets these changing payload bytes as key events, resulting
>> in the endless KEY_BRIGHTNESSUP autorepeat loop.
>>
>> So, adding the device entry with HID_GROUP_ANY to mt_devices[] is indeed
>> completely redundant. The only thing needed is to ignore the 0xFF01 vendor
>> collection so that no phantom input node is created for it.
>>
>> Here is the report descriptor and event recording from hid-recorder:
>>
>> # GXTP7863:00 27C6:01E0
>> # 0x05, 0x01,                    // Usage Page (Generic Desktop)        0
>> # 0x09, 0x02,                    // Usage (Mouse)                       2
> [...]
>> # 0xc0,                          // End Collection                      624
>> # 0x06, 0x01, 0xff,              // Usage Page (Vendor Usage Page 0xff01) 625
>> # 0x09, 0x01,                    // Usage (Vendor Usage 0x01)           628
>> # 0xa1, 0x01,                    // Collection (Application)            630
>> # 0x85, 0x08,                    //  Report ID (8)                      632
>> # 0x09, 0x01,                    //  Usage (Vendor Usage 0x01)          634
>> # 0x19, 0x00,                    //  Usage Minimum (0)                  636
>> # 0x29, 0xff,                    //  Usage Maximum (255)                638
>> # 0x15, 0x00,                    //  Logical Minimum (0)                640
>> # 0x25, 0xff,                    //  Logical Maximum (255)              642
>> # 0x95, 0x40,                    //  Report Count (64)                  644
>> # 0x75, 0x08,                    //  Report Size (8)                    646
>> # 0x91, 0x02,                    //  Output (Data,Var,Abs)              648
>> # 0x09, 0x01,                    //  Usage (Vendor Usage 0x01)          650
>> # 0x19, 0x00,                    //  Usage Minimum (0)                  652
> Technically, we could simply convert these 0x01 and 0x00 into 0x05 and
> this would make the mapping ignored by hid-input.c
>
> However, when changing those bytes in the hid-recorder output and
> replaying the device this creates a 10 seconds freeze on my desktop
> because some component is not happy about the empty input node it creates.
>
> I'm trying to investigate what is going on, without much success. I'll
> continue working on it on Monday.
>
>> # 0x29, 0xff,                    //  Usage Maximum (255)                654
>> # 0x15, 0x00,                    //  Logical Minimum (0)                656
>> # 0x25, 0xff,                    //  Logical Maximum (255)              658
>> # 0x95, 0x1d,                    //  Report Count (29)                  660
>> # 0x81, 0x02,                    //  Input (Data,Var,Abs)               662
>> # 0xc0,                          // End Collection                      664
> [...]
>> Would you prefer handling this by simply dropping the 0xff010001 collection
>> in mt_input_mapping() in hid-multitouch, or should this device quirk be
>> implemented via HID-BPF instead?
> So:
> - mt_input_mapping() in hid-multitouch -> probably not. Having such
>   quirk in the hid-multitouch code would be better handled with a proper
>   quirk, not a random check in mt_input_mapping().
> - HID-BPF: so far, if it weren't for that 10s freeze, I would have said
>   yes, go for it. But right now there is something fishy in the code
>   that creates empty input devices that are not properly cleaned up by
>   hid-input and that messes up userspace.
>  
> Ideally we should fix the generic mapping, but that has a strong chance
> of regressing existing HW, which is a PITA.
>
> And to add to the bucket, when replaying your device, I see that fwupd
> is trying to communicate with the device, so we should be sure to not
> break this as well :(
>
> Hopefully I'll have a better understanding next week.
>
> Cheers,
> Benjamin
Hi Benjamin,

I did some testing to investigate the freeze you
experienced.

I tried two different descriptor modifications to see how userspace reacts:

1. Test A (Your approach: changing Usage Page 0xFF01 to 0xFF05):
   Changing the usage page indeed strips the keyboard keys
   (KEY_BRIGHTNESSUP/DOWN disappear), but hid-input still registers the
   third input node ("GXTP7863:00 27C6:01E0 UNKNOWN").

   Checking /proc/bus/input/devices during the replay shows that this node
   is created with an anomalous capability set:
     N: Name="GXTP7863:00 27C6:01E0 UNKNOWN"
     H: Handlers=event15
     B: PROP=0
     B: EV=9 (EV_SYN | EV_ABS)
     B: ABS=10000000000 (ABS_MISC only, zero keys, no X/Y axes)

   This triggered the desktop freeze for ~15 seconds until the timeout
   expired and the system recovered.

2. Test B (Changing the 29-byte payload to Constant/Padding):
   I also tried keeping the original Usage Page 0xFF01 but changing
   `Input (Data,Var,Abs)` (0x81, 0x02) to `Input (Cnst,Var,Abs)` (0x81, 0x03)
   in Report ID 8, hoping hid-input would ignore the fields.

   Running `libinput debug-events` during this test captured the exact
   watchdog trace during the stall:
     event13  DEVICE_ADDED                 GXTP7863:00 27C6:01E0 Mouse
     client bug: timer event6 keyboard: scheduled expiry is in the past (-14747ms), your system is too slow
     client bug: timer event6 hold: scheduled expiry is in the past (-14396ms), your system is too slow
     client bug: timer event6 hold: scheduled expiry is in the past (-14389ms), your system is too slow
     client bug: timer event6 hold: scheduled expiry is in the past (-14369ms), your system is too slow
     event14  DEVICE_ADDED                 GXTP7863:00 27C6:01E0 Touchpad

   libinput explicitly confirms that the compositor's event loop was
   completely blocked for 14,747 ms (~15 seconds) during enumeration.
   Notice that the third node (UNKNOWN) is never even announced as
   DEVICE_ADDED.

Conclusion:
Both descriptor-level approaches confirm what you suspected: modifying the
descriptor still leaves the application collection instantiated as an empty/
broken input node, which trips up libinput and blocks the compositor event
loop for ~15 seconds.

Hope this empirical data helps you track down the hid-input cleanup issue on
Monday!

Cheers,
Ruzal

  reply	other threads:[~2026-09-18 18:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14 17:12 [PATCH 1/2] HID: multitouch: add support for Goodix GXTP7863 touchpad Ruzal Daminov
2026-08-19  9:15 ` [PATCH v2] " Ruzal Daminov
2026-08-19  9:28   ` sashiko-bot
2026-08-19  9:39 ` [PATCH v3] " Ruzal Daminov
2026-08-19  9:50   ` sashiko-bot
2026-08-19 12:33 ` [PATCH v4] " Ruzal Daminov
2026-09-17  7:30   ` Benjamin Tissoires
2026-09-17 12:51     ` Ruzal
2026-09-18 16:18       ` Benjamin Tissoires
2026-09-18 18:01         ` Ruzal [this message]
2026-09-19  9:23           ` Ruzal
2026-09-22 13:24             ` Benjamin Tissoires
2026-09-22 16:39               ` Ruzal
2026-09-23 13:31                 ` Benjamin Tissoires
2026-09-23 15:05                   ` Ruzal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=33a7c239-ede5-4091-ba72-ff429d97013c@gmail.com \
    --to=daminovruzal7@gmail.com \
    --cc=bentiss@kernel.org \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.