[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMCVhVMPuoaEzTthmaOBR+YbT3AN2YPKRv3XAw_r4owUvSRCKQ@mail.gmail.com>
Date: Wed, 12 Nov 2025 17:49:53 -0600
From: Jonathan Denose <jdenose@...gle.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] HID: multitouch: Toggle touch surface on Elan
touchpad on lid event
Hi Dmitry,
Thanks for your review.
On Tue, Nov 11, 2025 at 4:37 PM Dmitry Torokhov
<dmitry.torokhov@...il.com> wrote:
>
> Hi Jonathan,
>
> On Tue, Nov 11, 2025 at 09:34:07PM +0000, Jonathan Denose wrote:
> > Many touchpad modules have a pin which is expected to be connected to the
> > lid angle sensor in laptops. The pin sends a signal to the touchpad module
> > about the lid state and each touchpad vendor handles this notification in
> > their firmware.
> >
> > The Elan touchpad with VID 323b does not always have this aforementioned
> > pin, which then causes interference between the lid and the touchpad when
> > the lid is closed. This interference causes a few seconds delay before the
> > touchpad works again, or it causes it to be come completely unresponsive.
> > To circumvent this hardware issue in software, implement a device quirk
> > which will allow the hid-multitouch driver to register a notifier_block
> > to listen for lid switch events. When the lid switch closes, the
> > touchpad surface will be turned off and when the lid switch opens, the
> > touchpad surgace will be turned on. This triggers recalibration which
> > resolves interference issues when the lid is closed.
> >
> > Signed-off-by: Jonathan Denose <jdenose@...gle.com>
> > ---
> > drivers/hid/hid-multitouch.c | 32 +++++++++++++++++++++++++++++++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index 2879e65cf303b1456311ac06115adda5a78a2600..9a89913c193bc110a0a821a901aebd97892c66bd 100644
> > --- a/drivers/hid/hid-multitouch.c
> > +++ b/drivers/hid/hid-multitouch.c
> > @@ -35,6 +35,7 @@
> > #include <linux/device.h>
> > #include <linux/hid.h>
> > #include <linux/module.h>
> > +#include <linux/notifier.h>
> > #include <linux/slab.h>
> > #include <linux/input/mt.h>
> > #include <linux/jiffies.h>
> > @@ -76,6 +77,7 @@ MODULE_LICENSE("GPL");
> > #define MT_QUIRK_DISABLE_WAKEUP BIT(21)
> > #define MT_QUIRK_ORIENTATION_INVERT BIT(22)
> > #define MT_QUIRK_APPLE_TOUCHBAR BIT(23)
> > +#define MT_QUIRK_REGISTER_LID_NOTIFIER BIT(24)
> >
> > #define MT_INPUTMODE_TOUCHSCREEN 0x02
> > #define MT_INPUTMODE_TOUCHPAD 0x03
> > @@ -183,6 +185,8 @@ struct mt_device {
> > struct list_head reports;
> > };
> >
> > +static struct hid_device *lid_notify_hdev;
>
> This should really be per-device.
Just to be sure I'm following correctly, the initialization of
lid_notify_hdev should be per-device?
> > +
> > static void mt_post_parse_default_settings(struct mt_device *td,
> > struct mt_application *app);
> > static void mt_post_parse(struct mt_device *td, struct mt_application *app);
> > @@ -227,6 +231,7 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app);
> > #define MT_CLS_SMART_TECH 0x0113
> > #define MT_CLS_APPLE_TOUCHBAR 0x0114
> > #define MT_CLS_SIS 0x0457
> > +#define MT_CLS_REGISTER_LID_NOTIFIER 0x0115
> >
> > #define MT_DEFAULT_MAXCONTACT 10
> > #define MT_MAX_MAXCONTACT 250
> > @@ -327,7 +332,9 @@ static const struct mt_class mt_classes[] = {
> > MT_QUIRK_CONTACT_CNT_ACCURATE |
> > MT_QUIRK_WIN8_PTP_BUTTONS,
> > .export_all_inputs = true },
> > -
> > + { .name = MT_CLS_REGISTER_LID_NOTIFIER,
> > + .quirks = MT_QUIRK_REGISTER_LID_NOTIFIER,
> > + .export_all_inputs = true },
> > /*
> > * vendor specific classes
> > */
> > @@ -1840,6 +1847,20 @@ static void mt_expired_timeout(struct timer_list *t)
> > clear_bit_unlock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags);
> > }
> >
> > +static int mt_input_notifier(struct notifier_block *nb, unsigned long action, void *dev)
> > +{
> > + if (action)
> > + mt_set_modes(lid_notify_hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_NONE);
> > + else if (!action)
> > + mt_set_modes(lid_notify_hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL);
> > +
> > + return 0;
> > +}
> > +
> > +static struct notifier_block mt_lid_notifier_block = {
> > + .notifier_call = mt_input_notifier
> > +};
> > +
> > static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > {
> > int ret, i;
> > @@ -1920,6 +1941,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > if (hdev->vendor == USB_VENDOR_ID_SIS_TOUCH)
> > hdev->quirks |= HID_QUIRK_NOGET;
> >
> > + if (mtclass->quirks & MT_CLS_REGISTER_LID_NOTIFIER) {
> > + lid_notify_hdev = hdev;
> > + register_lid_notifier(&mt_lid_notifier_block);
> > + }
> > +
> > ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > if (ret)
> > return ret;
> > @@ -2150,6 +2176,10 @@ static const struct hid_device_id mt_devices[] = {
> > HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> > USB_VENDOR_ID_ELAN, 0x32ae) },
> >
> > + { .driver_data = MT_CLS_REGISTER_LID_NOTIFIER,
> > + HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> > + USB_VENDOR_ID_ELAN, 0x323b) },
>
> The need to have special handling of LID events is a quirk of board
> design, not quire of a controller. So I think it needs to be triggered
> by DMI quirk.
> > +
> > /* Elitegroup panel */
> > { .driver_data = MT_CLS_SERIAL,
> > MT_USB_DEVICE(USB_VENDOR_ID_ELITEGROUP,
> >
>
> Thanks.
>
> --
> Dmitry
--
Jonathan
Powered by blists - more mailing lists