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: Tue, 22 Sep 2026 15:24:30 +0200	[thread overview]
Message-ID: <arJ-vvEvd93l_aAp@beelink> (raw)
In-Reply-To: <816a300c-1f63-4d4a-a0f9-8eef5bd7e748@gmail.com>

Hi,

On Sep 19 2026, Ruzal wrote:
> On 9/18/26 9:01 PM, Ruzal wrote:
> > 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.
> 
> Hi Benjamin,
> 
> Quick update on the stall and a tested quirk fix:
> 
> The userspace stall happens because unrecognized vendor pages hit
> `unknown:` in hid-input.c:1420 (`map_abs(ABS_MISC)`). This sets EV_ABS,
> causing hidinput_has_been_populated() to return true and register an
> anomalous node with only ABS_MISC, which trips up libinput.

Actually, after a lot of debugging, it turns out that the session stall
was a red herring: it's hid-replay's behavior which basically only
listen for SET_REPORT events before it is ready, but then ignores
everything else.

And in the case where the UNKNOWN input node didn't have the KEY_*,
systemd-logind would not keep the hid node opened, and would further
re-open it, 3 times, and 3 times we have a timeout of 5 seconds due to
uhid not responding to SET_REPORT.

TL;DR: changing the report descriptor is doable, and there won't be this
session lock with a real device.

Regarding the `map_abs(ABS_MISC)`, you are almost correct. The solution
is simply to not use an unknown vendor usage. the second vendor
collection is 0xff00, which matches HID_UP_MSVENDOR which goes to
ignore.

So making the 0xff01 bogus collection 0xff09 (HID_UP_LOGIVENDOR2) or any
other HID_UP_LOGIVENDOR* also goes to ignore, and the phantom node just
disappears.

And TL;DR of rest, changing the usage page of the report ID 8 from
0xff01 to 0xff09 solves the problem entirely :)

> 
> Instead of touching the descriptor, we can handle this via hid-multitouch:
> - Add MT_QUIRK_IGNORE_VENDOR_TELEMETRY to drop HID_UP_HPVENDOR2 in mt_input_mapping().
> - Add MT_CLS_GOODIX (mirroring MT_CLS_WIN_8 quirks).
> - Bind with HID_GROUP_MULTITOUCH_WIN_8 in mt_devices[] (preserving INPUT_PER_APP).
> 
> This drops the phantom input node while leaving raw reports on /dev/hidraw intact.
> 
> I built the module and tested loading it: /proc/bus/input/devices no longer
> creates the UNKNOWN node, and /dev/hidraw0 remains available.

Thanks for this attempt, however, this is not great because there are
multiple checks for MT_CLS_WIN_8 throughout the code, and so your patch
misses them.

It is not great to have a BPF to work around a kernel bug, but in that
case I think that's the most sensible thing to do. A proper fix is to
restrict the parsing of 0xff01 for non keyboards or for non HP machines,
but as mentioned previously, this is prone to regressions :(

Also, I've just checked the fwupd sources, and I think changing the
vendor collection is fine. Hopefully this won't break any user space
program, but OTOH the touchpad is pretty unusable at the moment.

Cheers,
Benjamin

> 
> Diff:
> 
> ---
>  drivers/hid/hid-ids.h        |  1 +
>  drivers/hid/hid-multitouch.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b3aca5aa9176..d0850a98c9f8 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -567,6 +567,7 @@
>  #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_E100 0xe100
>  
>  #define I2C_VENDOR_ID_GOODIX		0x27c6
> +#define I2C_DEVICE_ID_GOODIX_01E0	0x01e0
>  #define I2C_DEVICE_ID_GOODIX_01E8	0x01e8
>  #define I2C_DEVICE_ID_GOODIX_01E9	0x01e9
>  #define I2C_DEVICE_ID_GOODIX_01F0	0x01f0
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 451c7324e6a0..2e973a3b652b 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -79,6 +79,7 @@ MODULE_LICENSE("GPL");
>  #define MT_QUIRK_APPLE_TOUCHBAR		BIT(23)
>  #define MT_QUIRK_YOGABOOK9I		BIT(24)
>  #define MT_QUIRK_KEEP_LATENCY_ON_CLOSE	BIT(25)
> +#define MT_QUIRK_IGNORE_VENDOR_TELEMETRY	BIT(26)
>  
>  #define MT_INPUTMODE_TOUCHSCREEN	0x02
>  #define MT_INPUTMODE_TOUCHPAD		0x03
> @@ -235,6 +236,7 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app);
>  #define MT_CLS_APPLE_TOUCHBAR			0x0114
>  #define MT_CLS_YOGABOOK9I			0x0115
>  #define MT_CLS_EGALAX_P80H84			0x0116
> +#define MT_CLS_GOODIX				0x0117
>  #define MT_CLS_SIS				0x0457
>  
>  #define MT_DEFAULT_MAXCONTACT	10
> @@ -457,6 +459,15 @@ static const struct mt_class mt_classes[] = {
>  			MT_QUIRK_IGNORE_DUPLICATES |
>  			MT_QUIRK_CONTACT_CNT_ACCURATE,
>  	},
> +	{ .name = MT_CLS_GOODIX,
> +		.quirks = MT_QUIRK_ALWAYS_VALID |
> +			MT_QUIRK_IGNORE_DUPLICATES |
> +			MT_QUIRK_HOVERING |
> +			MT_QUIRK_CONTACT_CNT_ACCURATE |
> +			MT_QUIRK_STICKY_FINGERS |
> +			MT_QUIRK_WIN8_PTP_BUTTONS |
> +			MT_QUIRK_IGNORE_VENDOR_TELEMETRY,
> +		.export_all_inputs = true },
>  	{ }
>  };
>  
> @@ -1524,6 +1535,17 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		return 1;
>  	}
>  
> +	/*
> +	 * Goodix FreeTouch devices emit a periodic heartbeat on vendor
> +	 * page 0xFF01 which hid-input would otherwise map to
> +	 * KEY_MICMUTE / KEY_BRIGHTNESS*, creating a phantom keyboard.
> +	 * Drop it here so no input node is exported; the report remains
> +	 * available through hidraw.
> +	 */
> +	if ((application->quirks & MT_QUIRK_IGNORE_VENDOR_TELEMETRY) &&
> +	    (usage->hid & HID_USAGE_PAGE) == HID_UP_HPVENDOR2)
> +		return -1;
> +
>  	if (rdata->is_mt_collection)
>  		return mt_touch_input_mapping(hdev, hi, field, usage, bit, max,
>  					      application);
> @@ -2457,6 +2479,11 @@ static const struct hid_device_id mt_devices[] = {
>  	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,
>  		     I2C_DEVICE_ID_GOODIX_01E9) },
>  
> +	/* Goodix GXTP7863 device with FreeTouch heartbeat on page 0xFF01 */
> +	{ .driver_data = MT_CLS_GOODIX,
> +	  HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8, I2C_VENDOR_ID_GOODIX,
> +		     I2C_DEVICE_ID_GOODIX_01E0) },
> +
>  	/* GoodTouch panels */
>  	{ .driver_data = MT_CLS_NSMU,
>  		MT_USB_DEVICE(USB_VENDOR_ID_GOODTOUCH,
> 
> If this approach looks good to you, I can formally submit it as [PATCH v5].
> 
> Cheers,
> Ruzal
> 

  reply	other threads:[~2026-09-22 13:24 UTC|newest]

Thread overview: 13+ 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
2026-09-19  9:23           ` Ruzal
2026-09-22 13:24             ` Benjamin Tissoires [this message]
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=arJ-vvEvd93l_aAp@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®