mirror of https://lore.kernel.org/lkml/
 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: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14 17:12 [PATCH 1/2] " Ruzal Daminov
2026-08-19  9:15 ` [PATCH v2] " Ruzal Daminov
2026-08-19  9:39 ` [PATCH v3] " Ruzal Daminov
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]

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®