mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Tissoires <bentiss@kernel.org>
To: Ruzal <daminovruzal7@gmail.com>
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 18:18:32 +0200	[thread overview]
Message-ID: <aqwUYWPz_nORoFr6@beelink> (raw)
In-Reply-To: <51055e02-5bf1-4b1e-b430-382d00d3d004@gmail.com>

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

  reply	other threads:[~2026-09-18 16:18 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 [this message]
2026-09-18 18:01         ` 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=aqwUYWPz_nORoFr6@beelink \
    --to=bentiss@kernel.org \
    --cc=daminovruzal7@gmail.com \
    --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®