* Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: Hans de Goede @ 2026-06-03 13:12 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: 谢致邦 (XIE Zhibang), linux-input,
dmitry.torokhov, dianders, jikos, linux-kernel, superm1,
Pin-yen Lin, Xu Rao, Kwok Kin Ming, Dan Carpenter
In-Reply-To: <aiAW9sSLVqmT6l87@beelink>
Hi,
On 3-Jun-26 1:59 PM, Benjamin Tissoires wrote:
> On Jun 03 2026, Hans de Goede wrote:
>> Hi Benjamin,
>>
>> On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
>>> On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
>>>> Move the _DSM call that gets the HID descriptor address from
>>>> i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
>>>> i2c-hid-acpi.c and i2c-hid-of.c can use it.
>>>>
>>>> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>>>> ---
>>>> drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
>>>> drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
>>>
>>> I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
>>> it in i2c-hid-core.c.
>>
>> It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.
>
> My concern is not so much about these 35 lines of code, but the fact
> that i2c-hid-acpi.c is now down to only 3 functions:
> - i2c_hid_acpi_probe()
> - i2c_hid_acpi_shutdown_tail()
> - i2c_hid_acpi_restore_sequence()
>
> So then, what's the point of keeping it as a separate driver? (more
> rethorical question than anything).
>
>>
>>> There's something I can't really understand in the problem:
>>> - we are talking about non arm architecture, which should not have OF in
>>> them
>>> - the current compatible hid-over-i2c loads the OF version?
>>> - how can you make the device working without the couple of ACPI
>>> functions that are in the i2c-hid-acpi.c driver for powering up and
>>> down the device?
>>>
>>> If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
>>> problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
>>> this particular platform/device? Kind of what we have with
>>> i2c-hid-of-elan.c
>>
>> As part of the work to use ACPI on ARM systems, it is possible now a days
>> to inject DT snippets into ACPI tables.
>>
>> The problem on these Loongson laptops is that they have created this
>> very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
>> node for the touchscreen, which means it contains an embedded DT
>> snippet and in that snipped out "compatible=hid-over-i2c" but NOT
>> also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
>> the implemented the ACPI _DSM on the same ACPI device/fwnode.
>>
>> The ACPI core sees HID=PRP0001 so it creates an of-compatible
>> modalias / match and not an ACPI one, so the i2c-hid-of driver will
>> bind.
>
> But if this fix is for just one platform, why not keeping the design
> clean and have a dmi_match instead in a separate driver, like the
> i2c-hid-of-elan does for Elan touchpads/touchscreens?
> i2c-hid-acpi-loongson.c for instance.
Ok, so let me see if I've got this right:
1. We create a i2c-hid-acpi-loongson.c with a DMI
MODULE_DEVICE_TABLE() for auto-module-loading
2. This will have a struct of_device_id table with
the "hid-over-i2c" compatible. But *no* / without a
MODULE_DEVICE_TABLE() for this so that it will not
autoload on regular DT platforms with regular
"hid-over-i2c" compatible touchscreens.
3. We will rely on i2c-hid-of.c error-ing out
due to the missing "hid-descr-addr" and we are ok
with the dev_err() this logs.
4. The new i2c-hid-acpi-loongson.c will in essence
be a copy of i2c-hid-acpi.c without the acpi_match
and with the DMI id + of match as described above.
Yes I think that should work, do you want to just copy
i2c_hid_acpi_get_descriptor() or do you want
i2c-hid-acpi to export this and have i2c-hid-acpi-longsoon
depend on i2c-hid-acpi for this ?
>> Note these "fixed" (as in we cannot fix them) ACPI tables use
>> the generic i2c-over-hid OF/DT compatible _not_ something
>> more specific so we can also not do a more specific driver like
>> i2c-hid-of-elan.c.
>
> Do they use anything else than the compatible DT entry? regulators and
> others? If not, then the device is pure ACPI, and should not be handled
> by i2c-hid-of.c at all, no?
No they don't use anything other then the compatible. This is
really just a very bad / messy decision by whomever created
the ACPI tables for these 2 laptops. But they are out there, so
we somehow have to deal with them.
<snip>
> so far :)
True, see above.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2] HID: wacom: fix slab-out-of-bounds write in wacom_wac_queue_insert
From: Jiri Kosina @ 2026-06-03 12:24 UTC (permalink / raw)
To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov, benjamin.tissoires, stable
In-Reply-To: <20260528175945.2987781-1-jinmo44.yang@gmail.com>
On Fri, 29 May 2026, Jinmo Yang wrote:
> wacom_wac_queue_insert() calls kfifo_skip() in a loop when the kfifo
> doesn't have enough space for the incoming report. If the kfifo is
> empty, kfifo_skip() reads stale data left in the kmalloc'd buffer
> via __kfifo_peek_n() and interprets it as a record length, advancing
> fifo->out by that garbage value. This corrupts the internal kfifo
> state, causing kfifo_unused() to return a value much larger than the
> actual buffer size, which bypasses __kfifo_in_r()'s guard:
>
> if (len + recsize > kfifo_unused(fifo))
> return 0;
>
> kfifo_copy_in() then performs an out-of-bounds memcpy, writing up to
> 3842 bytes past the 256-byte buffer.
>
> Add a !kfifo_is_empty() condition to the while loop so kfifo_skip()
> is never called on an empty fifo, and check the return value of
> kfifo_in() to reject reports that are too large for the fifo.
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: wacom: stop hardware after post-start probe failures
From: Jiri Kosina @ 2026-06-03 12:22 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Myeonghun Pak, Ping Cheng, Jason Gerecke, Benjamin Tissoires,
linux-input, linux-kernel, stable, Ijae Kim
In-Reply-To: <ahdE9G8I7kd_OoGW@google.com>
On Wed, 27 May 2026, Dmitry Torokhov wrote:
> > wacom_parse_and_register() starts HID hardware before registering inputs
> > and initializing pad LEDs/remotes. Those later steps can fail, but their
> > error paths currently release Wacom resources without stopping the HID
> > hardware.
> >
> > Route post-hid_hw_start() failures through hid_hw_stop() before
> > releasing driver resources.
> >
> > This issue was identified during our ongoing static-analysis research while
> > reviewing kernel code.
> >
> > Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> > ---
> > drivers/hid/wacom_sys.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index 0d1c6d90fe..c824d9c224 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> >
> > error = wacom_register_inputs(wacom);
> > if (error)
> > - goto fail;
> > + goto fail_hw_stop;
> >
> > if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
> > error = wacom_initialize_leds(wacom);
> > if (error)
> > - goto fail;
> > + goto fail_hw_stop;
> >
> > error = wacom_initialize_remotes(wacom);
> > if (error)
> > - goto fail;
> > + goto fail_hw_stop;
> > }
> >
> > if (!wireless) {
> > @@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > return 0;
> >
> > fail_quirks:
> > +fail_hw_stop:
> > hid_hw_stop(hdev);
> > fail:
> > wacom_release_resources(wacom);
>
> I'd get rid of 'fail_quirks' and use 'fail_hw_stop' everywhere,
Agreed. Myeonghun, will you send v2 please?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-lenovo-go: cancel cfg_setup work in hid_go_cfg_remove()
From: Jiri Kosina @ 2026-06-03 12:19 UTC (permalink / raw)
To: Manish Khadka
Cc: linux-input, Derek J . Clark, Mark Pearson, Benjamin Tissoires,
linux-kernel
In-Reply-To: <20260515153607.76175-1-maskmemanish@gmail.com>
On Fri, 15 May 2026, Manish Khadka wrote:
> hid_go_cfg_probe() initialises drvdata.go_cfg_setup and schedules it
> to run 2 ms later:
>
> INIT_DELAYED_WORK(&drvdata.go_cfg_setup, &cfg_setup);
> schedule_delayed_work(&drvdata.go_cfg_setup, msecs_to_jiffies(2));
>
> cfg_setup() dereferences drvdata.hdev to issue MCU command requests.
> hid_go_cfg_remove() tears down sysfs and stops the HID device, ending
> with hid_set_drvdata(hdev, NULL), but never drains the delayed work.
> If the device is unbound within the 2 ms scheduling delay (a probe
> failure rolling back via remove, or a fast rmmod after probe), the
> work fires after hid_set_drvdata(NULL) has cleared the back pointer,
> leaving cfg_setup() with a NULL or stale drvdata.hdev.
>
> Mirror the sibling driver hid-lenovo-go-s.c, whose hid_gos_cfg_remove()
> already calls cancel_delayed_work_sync() on its analogous work, and
> drain go_cfg_setup at the top of hid_go_cfg_remove(). The cancel
> must come before guard(mutex)(&drvdata.cfg_mutex) because cfg_setup()
> acquires that mutex; reversing the order would deadlock.
>
> Fixes: d69ccfcbc955 ("HID: hid-lenovo-go: Add Lenovo Legion Go Series HID Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 2/2] HID: nintendo: Use %pM format specifier for MAC addresses
From: Benjamin Tissoires @ 2026-06-03 12:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jiri Kosina, Daniel J. Ogorchock, Petr Mladek, Tamir Duberstein,
linux-doc, linux-kernel, linux-input, Steven Rostedt,
Rasmus Villemoes, Sergey Senozhatsky, Jonathan Corbet, Shuah Khan,
Andrew Morton
In-Reply-To: <20260603104351.152085-3-andriy.shevchenko@linux.intel.com>
On Jun 03 2026, Andy Shevchenko wrote:
> Convert to %pM instead of using custom code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Not sure where the first patch should land, so in case someone prefers
having the full series through their tree:
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Cheers,
Benjamin
> ---
> drivers/hid/hid-nintendo.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 29008c2cc530..05c50f2530ef 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2431,14 +2431,8 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> for (i = 4, j = 0; j < 6; i++, j++)
> ctlr->mac_addr[j] = report->subcmd_reply.data[i];
>
> - ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL,
> - "%02X:%02X:%02X:%02X:%02X:%02X",
> - ctlr->mac_addr[0],
> - ctlr->mac_addr[1],
> - ctlr->mac_addr[2],
> - ctlr->mac_addr[3],
> - ctlr->mac_addr[4],
> - ctlr->mac_addr[5]);
> + ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL, "%pMU",
> + ctlr->mac_addr);
> if (!ctlr->mac_addr_str)
> return -ENOMEM;
> hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
> --
> 2.50.1
>
>
^ permalink raw reply
* Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: Benjamin Tissoires @ 2026-06-03 11:59 UTC (permalink / raw)
To: Hans de Goede
Cc: 谢致邦 (XIE Zhibang), linux-input,
dmitry.torokhov, dianders, jikos, linux-kernel, superm1,
Pin-yen Lin, Xu Rao, Kwok Kin Ming, Dan Carpenter
In-Reply-To: <7ff8529d-6b20-4088-aa10-77b304da49b4@kernel.org>
On Jun 03 2026, Hans de Goede wrote:
> Hi Benjamin,
>
> On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
> > On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
> >> Move the _DSM call that gets the HID descriptor address from
> >> i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
> >> i2c-hid-acpi.c and i2c-hid-of.c can use it.
> >>
> >> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> >> ---
> >> drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
> >> drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
> >
> > I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
> > it in i2c-hid-core.c.
>
> It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.
My concern is not so much about these 35 lines of code, but the fact
that i2c-hid-acpi.c is now down to only 3 functions:
- i2c_hid_acpi_probe()
- i2c_hid_acpi_shutdown_tail()
- i2c_hid_acpi_restore_sequence()
So then, what's the point of keeping it as a separate driver? (more
rethorical question than anything).
>
> > There's something I can't really understand in the problem:
> > - we are talking about non arm architecture, which should not have OF in
> > them
> > - the current compatible hid-over-i2c loads the OF version?
> > - how can you make the device working without the couple of ACPI
> > functions that are in the i2c-hid-acpi.c driver for powering up and
> > down the device?
> >
> > If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
> > problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
> > this particular platform/device? Kind of what we have with
> > i2c-hid-of-elan.c
>
> As part of the work to use ACPI on ARM systems, it is possible now a days
> to inject DT snippets into ACPI tables.
>
> The problem on these Loongson laptops is that they have created this
> very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
> node for the touchscreen, which means it contains an embedded DT
> snippet and in that snipped out "compatible=hid-over-i2c" but NOT
> also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
> the implemented the ACPI _DSM on the same ACPI device/fwnode.
>
> The ACPI core sees HID=PRP0001 so it creates an of-compatible
> modalias / match and not an ACPI one, so the i2c-hid-of driver will
> bind.
But if this fix is for just one platform, why not keeping the design
clean and have a dmi_match instead in a separate driver, like the
i2c-hid-of-elan does for Elan touchpads/touchscreens?
i2c-hid-acpi-loongson.c for instance.
>
> Note these "fixed" (as in we cannot fix them) ACPI tables use
> the generic i2c-over-hid OF/DT compatible _not_ something
> more specific so we can also not do a more specific driver like
> i2c-hid-of-elan.c.
Do they use anything else than the compatible DT entry? regulators and
others? If not, then the device is pure ACPI, and should not be handled
by i2c-hid-of.c at all, no?
> If we could fix the ACPI tables we would just
> change the ACPI HID to the standard PNPxxxx ACPI HID for i2c-hid
> devices.
>
> The first version of this patch-set added an of match table
> to i2c-hid-acpi making it also load and probe i2c-hid devices
> described in devicetree on devicetree only systems which IMHO
> is very messy.
Yeah, this one is slightly less messy than the previous versions. It is
just sad to mess up with a clean separation and make the i2c-hid-acpi
driver just a placeholder for a probe function.
>
> So this new series is the least ugly way to deal with this.
so far :)
Cheers,
Benjamin
^ permalink raw reply
* [PATCH v2 1/2] vsprintf: Add upper case flavour to %p[mM]
From: Andy Shevchenko @ 2026-06-03 10:34 UTC (permalink / raw)
To: Andy Shevchenko, Jiri Kosina, Daniel J. Ogorchock, Petr Mladek,
Tamir Duberstein, linux-doc, linux-kernel, linux-input
Cc: Steven Rostedt, Rasmus Villemoes, Sergey Senozhatsky,
Jonathan Corbet, Shuah Khan, Benjamin Tissoires, Andrew Morton
In-Reply-To: <20260603104351.152085-1-andriy.shevchenko@linux.intel.com>
Some of the (ABI aware) code needs an upper case when printing MAC
addresses. Introduce an extension for that into the existing %p[mM].
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/core-api/printk-formats.rst | 3 +++
lib/tests/printf_kunit.c | 2 ++
lib/vsprintf.c | 22 ++++++++++++++++------
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index c0b1b6089307..57e887ff24bc 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -322,6 +322,7 @@ MAC/FDDI addresses
%pMF 00-01-02-03-04-05
%pm 000102030405
%pmR 050403020100
+ %p[mM][FR][U]
For printing 6-byte MAC/FDDI addresses in hex notation. The ``M`` and ``m``
specifiers result in a printed address with (M) or without (m) byte
@@ -335,6 +336,8 @@ For Bluetooth addresses the ``R`` specifier shall be used after the ``M``
specifier to use reversed byte order suitable for visual interpretation
of Bluetooth addresses which are in the little endian order.
+When ``U`` is passed, the result is printed in the upper case.
+
Passed by reference.
IPv4 addresses
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index 58e639b01e83..eccf041ebd56 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -435,8 +435,10 @@ mac(struct kunit *kunittest)
test("2d:48:d6:fc:7a:05", "%pM", addr);
test("05:7a:fc:d6:48:2d", "%pMR", addr);
+ test("05:7A:FC:D6:48:2D", "%pMRU", addr);
test("2d-48-d6-fc-7a-05", "%pMF", addr);
test("2d48d6fc7a05", "%pm", addr);
+ test("2D48D6FC7A05", "%pmU", addr);
test("057afcd6482d", "%pmR", addr);
}
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1389b50266bf..c9b6ec0f0de6 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1303,31 +1303,39 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
char mac_addr[sizeof("xx:xx:xx:xx:xx:xx")];
char *p = mac_addr;
int i;
- char separator;
+ char separator = ':';
bool reversed = false;
+ bool uc = false;
if (check_pointer(&buf, end, addr, spec))
return buf;
switch (fmt[1]) {
case 'F':
+ uc = fmt[2] == 'U';
separator = '-';
break;
case 'R':
+ uc = fmt[2] == 'U';
reversed = true;
- fallthrough;
+ break;
+
+ case 'U':
+ uc = true;
+ break;
default:
- separator = ':';
break;
}
for (i = 0; i < 6; i++) {
- if (reversed)
- p = hex_byte_pack(p, addr[5 - i]);
+ u8 byte = reversed ? addr[5 - i] : addr[i];
+
+ if (uc)
+ p = hex_byte_pack_upper(p, byte);
else
- p = hex_byte_pack(p, addr[i]);
+ p = hex_byte_pack(p, byte);
if (fmt[0] == 'M' && i != 5)
*p++ = separator;
@@ -2410,6 +2418,7 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
* - 'MF' For a 6-byte MAC FDDI address, it prints the address
* with a dash-separated hex notation
* - '[mM]R' For a 6-byte MAC address, Reverse order (Bluetooth)
+ * - '[mM][FR][U]' One of the above in the upper case
* - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
* IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
* IPv6 uses colon separated network-order 16 bit hex with leading 0's
@@ -2544,6 +2553,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'm': /* Contiguous: 000102030405 */
/* [mM]F (FDDI) */
/* [mM]R (Reverse order; Bluetooth) */
+ /* [mM][FR][U] (One of the above in the upper case) */
return mac_address_string(buf, end, ptr, spec, fmt);
case 'I': /* Formatted IP supported
* 4: 1.2.3.4
--
2.50.1
^ permalink raw reply related
* [PATCH v2 0/2] vsprintf: add upper case to %p[mM] et alia
From: Andy Shevchenko @ 2026-06-03 10:34 UTC (permalink / raw)
To: Andy Shevchenko, Jiri Kosina, Daniel J. Ogorchock, Petr Mladek,
Tamir Duberstein, linux-doc, linux-kernel, linux-input
Cc: Steven Rostedt, Rasmus Villemoes, Sergey Senozhatsky,
Jonathan Corbet, Shuah Khan, Benjamin Tissoires, Andrew Morton
The first patch induced by Sashiko rightfully rises a concern on
potential ABI breakage. To avoid that and allow the user (patch 2)
to be converted to use unified output introduce %p[mM][...]U for
printing in upper case. Tests are included and passed.
Changelog v2:
- added first patch (Sashiko)
Andy Shevchenko (2):
vsprintf: Add upper case flavour to %p[mM]
HID: nintendo: Use %pM format specifier for MAC addresses
Documentation/core-api/printk-formats.rst | 3 +++
drivers/hid/hid-nintendo.c | 10 ++--------
lib/tests/printf_kunit.c | 2 ++
lib/vsprintf.c | 22 ++++++++++++++++------
4 files changed, 23 insertions(+), 14 deletions(-)
--
2.50.1
^ permalink raw reply
* [PATCH v2 2/2] HID: nintendo: Use %pM format specifier for MAC addresses
From: Andy Shevchenko @ 2026-06-03 10:34 UTC (permalink / raw)
To: Andy Shevchenko, Jiri Kosina, Daniel J. Ogorchock, Petr Mladek,
Tamir Duberstein, linux-doc, linux-kernel, linux-input
Cc: Steven Rostedt, Rasmus Villemoes, Sergey Senozhatsky,
Jonathan Corbet, Shuah Khan, Benjamin Tissoires, Andrew Morton
In-Reply-To: <20260603104351.152085-1-andriy.shevchenko@linux.intel.com>
Convert to %pM instead of using custom code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-nintendo.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 29008c2cc530..05c50f2530ef 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2431,14 +2431,8 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
for (i = 4, j = 0; j < 6; i++, j++)
ctlr->mac_addr[j] = report->subcmd_reply.data[i];
- ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL,
- "%02X:%02X:%02X:%02X:%02X:%02X",
- ctlr->mac_addr[0],
- ctlr->mac_addr[1],
- ctlr->mac_addr[2],
- ctlr->mac_addr[3],
- ctlr->mac_addr[4],
- ctlr->mac_addr[5]);
+ ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL, "%pMU",
+ ctlr->mac_addr);
if (!ctlr->mac_addr_str)
return -ENOMEM;
hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: Hans de Goede @ 2026-06-03 10:25 UTC (permalink / raw)
To: Benjamin Tissoires, 谢致邦 (XIE Zhibang)
Cc: linux-input, dmitry.torokhov, dianders, jikos, linux-kernel,
superm1, Pin-yen Lin, Xu Rao, Kwok Kin Ming, Dan Carpenter
In-Reply-To: <ah_2JXN7AAos4kjM@beelink>
Hi Benjamin,
On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
> On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
>> Move the _DSM call that gets the HID descriptor address from
>> i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
>> i2c-hid-acpi.c and i2c-hid-of.c can use it.
>>
>> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>> ---
>> drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
>> drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
>
> I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
> it in i2c-hid-core.c.
It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.
> There's something I can't really understand in the problem:
> - we are talking about non arm architecture, which should not have OF in
> them
> - the current compatible hid-over-i2c loads the OF version?
> - how can you make the device working without the couple of ACPI
> functions that are in the i2c-hid-acpi.c driver for powering up and
> down the device?
>
> If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
> problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
> this particular platform/device? Kind of what we have with
> i2c-hid-of-elan.c
As part of the work to use ACPI on ARM systems, it is possible now a days
to inject DT snippets into ACPI tables.
The problem on these Loongson laptops is that they have created this
very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
node for the touchscreen, which means it contains an embedded DT
snippet and in that snipped out "compatible=hid-over-i2c" but NOT
also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
the implemented the ACPI _DSM on the same ACPI device/fwnode.
The ACPI core sees HID=PRP0001 so it creates an of-compatible
modalias / match and not an ACPI one, so the i2c-hid-of driver will
bind.
Note these "fixed" (as in we cannot fix them) ACPI tables use
the generic i2c-over-hid OF/DT compatible _not_ something
more specific so we can also not do a more specific driver like
i2c-hid-of-elan.c. If we could fix the ACPI tables we would just
change the ACPI HID to the standard PNPxxxx ACPI HID for i2c-hid
devices.
The first version of this patch-set added an of match table
to i2c-hid-acpi making it also load and probe i2c-hid devices
described in devicetree on devicetree only systems which IMHO
is very messy.
So this new series is the least ugly way to deal with this.
Regards,
Hans
>
> Cheers,
> Benjamin
>
>> drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++++
>> 3 files changed, 50 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
>> index f65fb6396b69..234789a07047 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
>> @@ -25,12 +25,12 @@
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/pm.h>
>> -#include <linux/uuid.h>
>>
>> #include "i2c-hid.h"
>>
>> struct i2c_hid_acpi {
>> struct i2chid_ops ops;
>> + struct i2c_client *client;
>> struct acpi_device *adev;
>> };
>>
>> @@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
>> { }
>> };
>>
>> -/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
>> -static guid_t i2c_hid_guid =
>> - GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
>> - 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
>> -
>> -static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
>> -{
>> - struct acpi_device *adev = ihid_acpi->adev;
>> - acpi_handle handle = acpi_device_handle(adev);
>> - union acpi_object *obj;
>> - u16 hid_descriptor_address;
>> -
>> - obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
>> - ACPI_TYPE_INTEGER);
>> - if (!obj) {
>> - acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
>> - return -ENODEV;
>> - }
>> -
>> - hid_descriptor_address = obj->integer.value;
>> - ACPI_FREE(obj);
>> -
>> - return hid_descriptor_address;
>> -}
>> -
>> static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
>> {
>> struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
>>
>> - i2c_hid_acpi_get_descriptor(ihid_acpi);
>> + i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
>> }
>>
>> static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
>> @@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
>> if (!ihid_acpi)
>> return -ENOMEM;
>>
>> + ihid_acpi->client = client;
>> ihid_acpi->adev = adev;
>> ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
>> ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
>>
>> - ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
>> + ret = i2c_hid_core_acpi_get_descriptor(dev);
>> if (ret < 0)
>> return ret;
>> hid_descriptor_address = ret;
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index 3adb16366e93..1e1a8df5686d 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
>> };
>> EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
>>
>> +#ifdef CONFIG_ACPI
>> +#include <linux/acpi.h>
>> +
>> +/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
>> +static guid_t i2c_hid_guid =
>> + GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
>> + 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
>> +
>> +int i2c_hid_core_acpi_get_descriptor(struct device *dev)
>> +{
>> + struct acpi_device *adev = ACPI_COMPANION(dev);
>> + acpi_handle handle;
>> + union acpi_object *obj;
>> + u16 hid_descriptor_address;
>> +
>> + if (!adev)
>> + return -ENODEV;
>> +
>> + handle = acpi_device_handle(adev);
>> + obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
>> + ACPI_TYPE_INTEGER);
>> + if (!obj) {
>> + acpi_handle_err(handle,
>> + "Error _DSM call to get HID descriptor address failed\n");
>> + return -ENODEV;
>> + }
>> +
>> + hid_descriptor_address = obj->integer.value;
>> + ACPI_FREE(obj);
>> +
>> + return hid_descriptor_address;
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
>> +#endif
>> +
>> MODULE_DESCRIPTION("HID over I2C core driver");
>> MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
>> index 1724a435c783..bc8661c65b1a 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.h
>> +++ b/drivers/hid/i2c-hid/i2c-hid.h
>> @@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
>>
>> extern const struct dev_pm_ops i2c_hid_core_pm;
>>
>> +#ifdef CONFIG_ACPI
>> +struct device;
>> +int i2c_hid_core_acpi_get_descriptor(struct device *dev);
>> +#else
>> +struct device;
>> +static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
>> +{
>> + return -ENODEV;
>> +}
>> +#endif
>> +
>> #endif
>> --
>> 2.54.0
>>
>>
^ permalink raw reply
* [PATCH v3 3/3] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
From: 谢致邦 (XIE Zhibang) @ 2026-06-03 10:22 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, sashiko-bot
Cc: sashiko-reviews, 谢致邦 (XIE Zhibang),
Jiri Kosina, Benjamin Tissoires, Mario Limonciello (AMD),
Douglas Anderson, Pin-yen Lin, Xu Rao, Kwok Kin Ming,
Dan Carpenter, Uwe Kleine-König (The Capable Hub),
linux-kernel
In-Reply-To: <20260603102239.26491-1-Yeking@Red54.com>
Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method. Call the common
i2c_hid_core_acpi_get_descriptor() helper as a fallback, and set safe
post-power-on and post-reset-deassert delays.
Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
v3: Add restore_sequence
drivers/hid/i2c-hid/i2c-hid-of.c | 38 ++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 59393d71ddb9..325c64dfe81e 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -74,6 +74,13 @@ static void i2c_hid_of_power_down(struct i2chid_ops *ops)
ihid_of->supplies);
}
+static void i2c_hid_of_restore_sequence(struct i2chid_ops *ops)
+{
+ struct i2c_hid_of *ihid_of = container_of(ops, struct i2c_hid_of, ops);
+
+ i2c_hid_core_acpi_get_descriptor(&ihid_of->client->dev);
+}
+
static int i2c_hid_of_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -92,6 +99,37 @@ static int i2c_hid_of_probe(struct i2c_client *client)
ihid_of->ops.power_down = i2c_hid_of_power_down;
ret = device_property_read_u32(dev, "hid-descr-addr", &val);
+ if (ret) {
+ /*
+ * Some devices, for example the Lenovo KaiTian N60d and Inspur
+ * CP300L3, declare their I2C HID touchpad with _HID "PRP0001"
+ * and _DSD compatible "hid-over-i2c" but lack the
+ * "hid-descr-addr" property. Fall back to _DSM to obtain the
+ * HID descriptor address.
+ */
+ int dsm_ret = i2c_hid_core_acpi_get_descriptor(dev);
+
+ if (dsm_ret >= 0) {
+ dev_warn(dev,
+ "hid-descr-addr NOT found, using _DSM fallback. Contact vendor for firmware update!\n");
+ val = dsm_ret;
+
+ ihid_of->ops.restore_sequence = i2c_hid_of_restore_sequence;
+ /*
+ * Firmware providing the descriptor address only
+ * through _DSM may also lack "post-power-on-delay-ms"
+ * or "post-reset-deassert-delay-ms", leaving the
+ * driver without enough delay before the first HID
+ * descriptor read. Set safe defaults to avoid reading
+ * the descriptor before the device has finished its
+ * internal power-on reset.
+ */
+ ihid_of->post_power_delay_ms = 250;
+ ihid_of->post_reset_delay_ms = 250;
+
+ ret = 0;
+ }
+ }
if (ret) {
dev_err(dev, "HID register address not provided\n");
return -ENODEV;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: 谢致邦 (XIE Zhibang) @ 2026-06-03 10:22 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, sashiko-bot
Cc: sashiko-reviews, 谢致邦 (XIE Zhibang),
Jiri Kosina, Benjamin Tissoires, Mario Limonciello (AMD),
Douglas Anderson, Pin-yen Lin, Xu Rao, Kwok Kin Ming,
Dan Carpenter, Uwe Kleine-König (The Capable Hub),
linux-kernel
In-Reply-To: <20260603102239.26491-1-Yeking@Red54.com>
Move the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++++
3 files changed, 50 insertions(+), 28 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index f65fb6396b69..234789a07047 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,12 +25,12 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pm.h>
-#include <linux/uuid.h>
#include "i2c-hid.h"
struct i2c_hid_acpi {
struct i2chid_ops ops;
+ struct i2c_client *client;
struct acpi_device *adev;
};
@@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
{ }
};
-/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
-static guid_t i2c_hid_guid =
- GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
- 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
-
-static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
-{
- struct acpi_device *adev = ihid_acpi->adev;
- acpi_handle handle = acpi_device_handle(adev);
- union acpi_object *obj;
- u16 hid_descriptor_address;
-
- obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
- ACPI_TYPE_INTEGER);
- if (!obj) {
- acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
- return -ENODEV;
- }
-
- hid_descriptor_address = obj->integer.value;
- ACPI_FREE(obj);
-
- return hid_descriptor_address;
-}
-
static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
{
struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
- i2c_hid_acpi_get_descriptor(ihid_acpi);
+ i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
}
static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
if (!ihid_acpi)
return -ENOMEM;
+ ihid_acpi->client = client;
ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
- ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
+ ret = i2c_hid_core_acpi_get_descriptor(dev);
if (ret < 0)
return ret;
hid_descriptor_address = ret;
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..1e1a8df5686d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
};
EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+static guid_t i2c_hid_guid =
+ GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+ 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ acpi_handle handle;
+ union acpi_object *obj;
+ u16 hid_descriptor_address;
+
+ if (!adev)
+ return -ENODEV;
+
+ handle = acpi_device_handle(adev);
+ obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID descriptor address failed\n");
+ return -ENODEV;
+ }
+
+ hid_descriptor_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ return hid_descriptor_address;
+}
+EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
+#endif
+
MODULE_DESCRIPTION("HID over I2C core driver");
MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
index 1724a435c783..bc8661c65b1a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
extern const struct dev_pm_ops i2c_hid_core_pm;
+#ifdef CONFIG_ACPI
+struct device;
+int i2c_hid_core_acpi_get_descriptor(struct device *dev);
+#else
+struct device;
+static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+ return -ENODEV;
+}
+#endif
+
#endif
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/3] HID: i2c-hid-acpi: Move blacklist check to probe() before devm_kzalloc()
From: 谢致邦 (XIE Zhibang) @ 2026-06-03 10:22 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, sashiko-bot
Cc: sashiko-reviews, 谢致邦 (XIE Zhibang),
Jiri Kosina, Benjamin Tissoires, Mario Limonciello (AMD),
Douglas Anderson, Pin-yen Lin, Xu Rao, Kwok Kin Ming,
Dan Carpenter, Uwe Kleine-König (The Capable Hub),
linux-kernel
In-Reply-To: <20260603102239.26491-1-Yeking@Red54.com>
Move the check so the function can return early without wasting an
allocation. This is a pure refactoring, no functional change.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
v2: Merge declaration into assignment.
drivers/hid/i2c-hid/i2c-hid-acpi.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..f65fb6396b69 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -60,9 +60,6 @@ static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
union acpi_object *obj;
u16 hid_descriptor_address;
- if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
- return -ENODEV;
-
obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
ACPI_TYPE_INTEGER);
if (!obj) {
@@ -93,15 +90,19 @@ static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
static int i2c_hid_acpi_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct i2c_hid_acpi *ihid_acpi;
u16 hid_descriptor_address;
int ret;
+ if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+ return -ENODEV;
+
ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
if (!ihid_acpi)
return -ENOMEM;
- ihid_acpi->adev = ACPI_COMPANION(dev);
+ ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/3] HID: i2c-hid: Fix some PRP0001 touchpads probe after OF/ACPI split
From: 谢致邦 (XIE Zhibang) @ 2026-06-03 10:22 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, sashiko-bot
Cc: sashiko-reviews, 谢致邦 (XIE Zhibang),
Jiri Kosina, Benjamin Tissoires, Mario Limonciello (AMD),
Douglas Anderson, Pin-yen Lin, Xu Rao, Kwok Kin Ming,
Dan Carpenter, Uwe Kleine-König (The Capable Hub),
linux-kernel
In-Reply-To: <20260601184747.843DB1F00893@smtp.kernel.org>
Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method and thus fail to probe.
Patch 1 moves the blacklist check so the function can return early
without wasting an allocation.
Patch 2 moves the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.
Patch 3 calls the common helper as a fallback when "hid-descr-addr" is
missing, and sets safe post-power-on and post-reset-deassert delays.
谢致邦 (XIE Zhibang) (3):
HID: i2c-hid-acpi: Move blacklist check to probe() before
devm_kzalloc()
HID: i2c-hid: Move common ACPI _DSM helper into core
HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
drivers/hid/i2c-hid/i2c-hid-acpi.c | 41 +++++++-----------------------
drivers/hid/i2c-hid/i2c-hid-core.c | 35 +++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-of.c | 38 +++++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++
4 files changed, 93 insertions(+), 32 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: Benjamin Tissoires @ 2026-06-03 9:43 UTC (permalink / raw)
To: 谢致邦 (XIE Zhibang)
Cc: linux-input, hansg, dmitry.torokhov, dianders, jikos,
linux-kernel, superm1, Pin-yen Lin, Xu Rao, Kwok Kin Ming,
Dan Carpenter
In-Reply-To: <tencent_71AA07C8BAF64FE0F9B0FDFC072BB08B3605@qq.com>
On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
> Move the _DSM call that gets the HID descriptor address from
> i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
> i2c-hid-acpi.c and i2c-hid-of.c can use it.
>
> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> ---
> drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
> drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
it in i2c-hid-core.c.
There's something I can't really understand in the problem:
- we are talking about non arm architecture, which should not have OF in
them
- the current compatible hid-over-i2c loads the OF version?
- how can you make the device working without the couple of ACPI
functions that are in the i2c-hid-acpi.c driver for powering up and
down the device?
If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
this particular platform/device? Kind of what we have with
i2c-hid-of-elan.c
Cheers,
Benjamin
> drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++++
> 3 files changed, 50 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> index f65fb6396b69..234789a07047 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> @@ -25,12 +25,12 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pm.h>
> -#include <linux/uuid.h>
>
> #include "i2c-hid.h"
>
> struct i2c_hid_acpi {
> struct i2chid_ops ops;
> + struct i2c_client *client;
> struct acpi_device *adev;
> };
>
> @@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
> { }
> };
>
> -/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
> -static guid_t i2c_hid_guid =
> - GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
> - 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
> -
> -static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
> -{
> - struct acpi_device *adev = ihid_acpi->adev;
> - acpi_handle handle = acpi_device_handle(adev);
> - union acpi_object *obj;
> - u16 hid_descriptor_address;
> -
> - obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
> - ACPI_TYPE_INTEGER);
> - if (!obj) {
> - acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
> - return -ENODEV;
> - }
> -
> - hid_descriptor_address = obj->integer.value;
> - ACPI_FREE(obj);
> -
> - return hid_descriptor_address;
> -}
> -
> static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
> {
> struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
>
> - i2c_hid_acpi_get_descriptor(ihid_acpi);
> + i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
> }
>
> static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
> @@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
> if (!ihid_acpi)
> return -ENOMEM;
>
> + ihid_acpi->client = client;
> ihid_acpi->adev = adev;
> ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
> ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
>
> - ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
> + ret = i2c_hid_core_acpi_get_descriptor(dev);
> if (ret < 0)
> return ret;
> hid_descriptor_address = ret;
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 3adb16366e93..1e1a8df5686d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
> };
> EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
>
> +#ifdef CONFIG_ACPI
> +#include <linux/acpi.h>
> +
> +/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
> +static guid_t i2c_hid_guid =
> + GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
> + 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
> +
> +int i2c_hid_core_acpi_get_descriptor(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> + acpi_handle handle;
> + union acpi_object *obj;
> + u16 hid_descriptor_address;
> +
> + if (!adev)
> + return -ENODEV;
> +
> + handle = acpi_device_handle(adev);
> + obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
> + ACPI_TYPE_INTEGER);
> + if (!obj) {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID descriptor address failed\n");
> + return -ENODEV;
> + }
> +
> + hid_descriptor_address = obj->integer.value;
> + ACPI_FREE(obj);
> +
> + return hid_descriptor_address;
> +}
> +EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
> +#endif
> +
> MODULE_DESCRIPTION("HID over I2C core driver");
> MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
> index 1724a435c783..bc8661c65b1a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.h
> +++ b/drivers/hid/i2c-hid/i2c-hid.h
> @@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
>
> extern const struct dev_pm_ops i2c_hid_core_pm;
>
> +#ifdef CONFIG_ACPI
> +struct device;
> +int i2c_hid_core_acpi_get_descriptor(struct device *dev);
> +#else
> +struct device;
> +static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> #endif
> --
> 2.54.0
>
>
^ permalink raw reply
* Re: [PATCH v1 1/1] HID: nintendo: Use %pM format specifier for MAC addresses
From: Andy Shevchenko @ 2026-06-03 9:05 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260603083946.923AD1F00893@smtp.kernel.org>
On Wed, Jun 03, 2026 at 08:39:46AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] ABI Breakage: The patch changes the MAC address exposed via sysfs from uppercase to lowercase, breaking userspace tools that rely on exact string matching.
> --
>
> commit 6c8c4e53eb547a4381d6f8dabe9201540c4f0168
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> HID: nintendo: Use %pM format specifier for MAC addresses
>
> This commit refactors the MAC address string generation in the nintendo HID
> driver. It replaces a custom sprintf format string with the standard %pM
> format specifier.
>
> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> > index 29008c2cc5304..77b14dfdae867 100644
> > --- a/drivers/hid/hid-nintendo.c
> > +++ b/drivers/hid/hid-nintendo.c
> > @@ -2431,14 +2431,8 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> > for (i = 4, j = 0; j < 6; i++, j++)
> > ctlr->mac_addr[j] = report->subcmd_reply.data[i];
> >
> > - ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL,
> > - "%02X:%02X:%02X:%02X:%02X:%02X",
> > - ctlr->mac_addr[0],
> > - ctlr->mac_addr[1],
> > - ctlr->mac_addr[2],
> > - ctlr->mac_addr[3],
> > - ctlr->mac_addr[4],
> > - ctlr->mac_addr[5]);
> > + ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL, "%pM",
> > + ctlr->mac_addr);
>
> [Severity: Critical]
> Does this change the sysfs output from uppercase to lowercase?
I think yes, it does.
> The original format string %02X explicitly generated an uppercase MAC
> address string, while %pM delegates to hex_byte_pack() and generates a
> lowercase string.
>
> Since ctlr->mac_addr_str is exposed to userspace via the uniq attribute,
> could this cause an ABI regression?
This is a good question. This is MAC address string and conceptually the
user space should be prepared to parse it as a mac address and not an arbitrary
string. OTOH, the field is named something like uniq, which means the
driver just supplies MAC as an arbitrary (unique) string.
> Userspace utilities and gaming libraries (like SDL and the joycond daemon)
> often map USB controllers to their Bluetooth pairings by performing
> case-sensitive matching against the uppercase Bluetooth MAC addresses
> exposed by BlueZ.
Hmm... Interesting. Okay, perhaps we need to have %pMU first.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 1/1] HID: nintendo: Use %pM format specifier for MAC addresses
From: sashiko-bot @ 2026-06-03 8:39 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260603082808.119958-1-andriy.shevchenko@linux.intel.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] ABI Breakage: The patch changes the MAC address exposed via sysfs from uppercase to lowercase, breaking userspace tools that rely on exact string matching.
--
commit 6c8c4e53eb547a4381d6f8dabe9201540c4f0168
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
HID: nintendo: Use %pM format specifier for MAC addresses
This commit refactors the MAC address string generation in the nintendo HID
driver. It replaces a custom sprintf format string with the standard %pM
format specifier.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 29008c2cc5304..77b14dfdae867 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2431,14 +2431,8 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> for (i = 4, j = 0; j < 6; i++, j++)
> ctlr->mac_addr[j] = report->subcmd_reply.data[i];
>
> - ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL,
> - "%02X:%02X:%02X:%02X:%02X:%02X",
> - ctlr->mac_addr[0],
> - ctlr->mac_addr[1],
> - ctlr->mac_addr[2],
> - ctlr->mac_addr[3],
> - ctlr->mac_addr[4],
> - ctlr->mac_addr[5]);
> + ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL, "%pM",
> + ctlr->mac_addr);
[Severity: Critical]
Does this change the sysfs output from uppercase to lowercase?
The original format string %02X explicitly generated an uppercase MAC
address string, while %pM delegates to hex_byte_pack() and generates a
lowercase string.
Since ctlr->mac_addr_str is exposed to userspace via the uniq attribute,
could this cause an ABI regression?
Userspace utilities and gaming libraries (like SDL and the joycond daemon)
often map USB controllers to their Bluetooth pairings by performing
case-sensitive matching against the uppercase Bluetooth MAC addresses
exposed by BlueZ.
> if (!ctlr->mac_addr_str)
> return -ENOMEM;
> hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603082808.119958-1-andriy.shevchenko@linux.intel.com?part=1
^ permalink raw reply
* [PATCH v1 1/1] HID: nintendo: Use %pM format specifier for MAC addresses
From: Andy Shevchenko @ 2026-06-03 8:28 UTC (permalink / raw)
To: Jiri Kosina, Daniel J. Ogorchock, linux-input, linux-kernel
Cc: Benjamin Tissoires, Andy Shevchenko
Convert to %pM instead of using custom code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-nintendo.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 29008c2cc530..77b14dfdae86 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2431,14 +2431,8 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
for (i = 4, j = 0; j < 6; i++, j++)
ctlr->mac_addr[j] = report->subcmd_reply.data[i];
- ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL,
- "%02X:%02X:%02X:%02X:%02X:%02X",
- ctlr->mac_addr[0],
- ctlr->mac_addr[1],
- ctlr->mac_addr[2],
- ctlr->mac_addr[3],
- ctlr->mac_addr[4],
- ctlr->mac_addr[5]);
+ ctlr->mac_addr_str = devm_kasprintf(&ctlr->hdev->dev, GFP_KERNEL, "%pM",
+ ctlr->mac_addr);
if (!ctlr->mac_addr_str)
return -ENOMEM;
hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v4 2/2] Input: axp20x-pek - add support for AXP313A variant
From: sashiko-bot @ 2026-06-03 8:11 UTC (permalink / raw)
To: Steven Feng; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <tencent_58979D4AAAB2237B7CFCA5445D81B687E207@qq.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Suspend and resume callbacks unconditionally disable/enable IRQs even when the input device is not registered, potentially disabling the system timer (IRQ 0).
- [Low] Manual assignment of the input device's parent is redundant when using devm_input_allocate_device().
--
commit 54f63f8b19bf8de6d0676d801f797fd4b2f755ae
Author: Steven Feng <steven@joint-cloud.com>
Input: axp20x-pek - add support for AXP313A variant
Add power button support for the AXP313A PMIC. Creates a dedicated
axp313a_info configuration with NULL time tables to support the
device since it lacks the PEK_KEY register.
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index c1a4e5ea1e001..964b39817af9f 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
[ ... ]
> @@ -414,6 +421,10 @@ static const struct platform_device_id axp_pek_id_match[] = {
> .name = "axp221-pek",
> .driver_data = (kernel_ulong_t)&axp221_info,
> },
> + {
> + .name = "axp313a-pek",
> + .driver_data = (kernel_ulong_t)&axp313a_info,
> + },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(platform, axp_pek_id_match);
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the driver
I noticed an issue in the suspend and resume callbacks:
drivers/input/misc/axp20x-pek.c:axp20x_pek_suspend() {
...
disable_irq(axp20x_pek->irq_dbf);
disable_irq(axp20x_pek->irq_dbr);
...
}
If axp20x_pek_should_register_input() returns false, the input device
is not registered and the IRQ numbers remain initialized to 0.
During suspend and resume, these callbacks unconditionally disable and
enable the IRQs. Since device_may_wakeup() evaluates to false in this
state, the callbacks execute disable_irq(0) and enable_irq(0).
On systems where IRQ 0 is the system timer, manipulating its state
during power transitions can lead to system hangs or kernel panics.
Should these callbacks verify that the input device is actually registered
before interacting with the IRQs?
[Severity: Low]
This is also a pre-existing issue, but I noticed a redundant assignment
in axp20x_pek_probe_input_device():
drivers/input/misc/axp20x-pek.c:axp20x_pek_probe_input_device() {
...
idev->dev.parent = &pdev->dev;
...
}
Since devm_input_allocate_device() automatically sets the input device's
parent, is it necessary to manually assign it here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603074727.613319-1-steven@joint-cloud.com?part=2
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: axp20x-pek - add is_visible callback for sysfs attributes
From: sashiko-bot @ 2026-06-03 8:02 UTC (permalink / raw)
To: Steven Feng; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <tencent_ACA5ECDD5E0E1F239149E0F961FE45A65205@qq.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver unconditionally operates on uninitialized IRQs (0) during suspend and resume when the input device is not registered, which can crash or hang the system.
--
commit edceaed72d31aa98549e1afb728e99699b609d10
Author: Steven Feng <steven@joint-cloud.com>
Input: axp20x-pek - add is_visible callback for sysfs attributes
This commit adds an is_visible callback to control the visibility of sysfs
attributes based on hardware capabilities. It hides the attributes for
devices without a startup_time configuration.
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 24f9e9d893de3..c1a4e5ea1e001 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -193,7 +193,28 @@ static struct attribute *axp20x_attrs[] = {
> &dev_attr_shutdown.attr,
> NULL,
> };
> -ATTRIBUTE_GROUPS(axp20x);
> +
> +static umode_t axp20x_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> +
> + if (!axp20x_pek->info->startup_time)
> + return 0;
> +
> + return attr->mode;
> +}
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
suspend and resume are handled when the input device is not registered.
If axp20x_pek_should_register_input() returns false (for example, on Cherry
Trail platforms with certain ACPI devices), axp20x_pek_probe_input_device()
is skipped. Since axp20x_pek is allocated with devm_kzalloc(), the irq_dbf
and irq_dbr fields remain 0.
During system suspend and resume, if device_may_wakeup(dev) is false, the
callbacks in axp20x_pek_suspend() and axp20x_pek_resume() unconditionally
execute:
disable_irq(axp20x_pek->irq_dbf);
disable_irq(axp20x_pek->irq_dbr);
and
enable_irq(axp20x_pek->irq_dbf);
enable_irq(axp20x_pek->irq_dbr);
Could this cause the driver to unconditionally call disable_irq(0) and
enable_irq(0)? On x86 architectures, tampering with IRQ 0 (typically the
legacy system timer) could cause severe system instability or kernel hangs
during the suspend and resume cycle.
> +
> +static const struct attribute_group axp20x_group = {
> + .attrs = axp20x_attrs,
> + .is_visible = axp20x_attr_is_visible,
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603074727.613319-1-steven@joint-cloud.com?part=1
^ permalink raw reply
* [PATCH v4 2/2] Input: axp20x-pek - add support for AXP313A variant
From: Steven Feng @ 2026-06-03 7:47 UTC (permalink / raw)
To: wens; +Cc: dmitry.torokhov, linux-input, linux-kernel, Steven Feng
In-Reply-To: <20260603074727.613319-1-steven@joint-cloud.com>
Add power button support for the AXP313A PMIC. The AXP313A does not
have the PEK_KEY register (0x36) used to configure startup and shutdown
timings, unlike AXP20X and AXP221.
Create a dedicated axp313a_info configuration with NULL time tables.
The sysfs attributes for startup/shutdown timing are automatically
hidden by the is_visible callback added in the previous patch.
Signed-off-by: Steven Feng <steven@joint-cloud.com>
---
drivers/input/misc/axp20x-pek.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index c1a4e5ea1e00..964b39817af9 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -85,6 +85,13 @@ static const struct axp20x_info axp221_info = {
.shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
};
+static const struct axp20x_info axp313a_info = {
+ .startup_time = NULL,
+ .startup_mask = 0,
+ .shutdown_time = NULL,
+ .shutdown_mask = 0,
+};
+
static ssize_t axp20x_show_attr(struct device *dev,
const struct axp20x_time *time,
unsigned int mask, char *buf)
@@ -414,6 +421,10 @@ static const struct platform_device_id axp_pek_id_match[] = {
.name = "axp221-pek",
.driver_data = (kernel_ulong_t)&axp221_info,
},
+ {
+ .name = "axp313a-pek",
+ .driver_data = (kernel_ulong_t)&axp313a_info,
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(platform, axp_pek_id_match);
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/2] Input: axp20x-pek - add is_visible callback for sysfs attributes
From: Steven Feng @ 2026-06-03 7:47 UTC (permalink / raw)
To: wens; +Cc: dmitry.torokhov, linux-input, linux-kernel, Steven Feng
In-Reply-To: <20260603074727.613319-1-steven@joint-cloud.com>
Add an is_visible callback to control the visibility of sysfs attributes
based on hardware capabilities. This is needed to support PMICs that lack
the PEK_KEY register for configuring startup and shutdown timings.
Devices without startup_time configuration will have these attributes
hidden from sysfs, providing a cleaner interface than returning errors
when the attributes are accessed.
This prepares the driver for adding support for the AXP313A variant.
Signed-off-by: Steven Feng <steven@joint-cloud.com>
---
drivers/input/misc/axp20x-pek.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 24f9e9d893de..c1a4e5ea1e00 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -193,7 +193,28 @@ static struct attribute *axp20x_attrs[] = {
&dev_attr_shutdown.attr,
NULL,
};
-ATTRIBUTE_GROUPS(axp20x);
+
+static umode_t axp20x_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+ if (!axp20x_pek->info->startup_time)
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group axp20x_group = {
+ .attrs = axp20x_attrs,
+ .is_visible = axp20x_attr_is_visible,
+};
+
+static const struct attribute_group *axp20x_groups[] = {
+ &axp20x_group,
+ NULL,
+};
static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
{
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/2] Input: axp20x-pek - add AXP313A support
From: Steven Feng @ 2026-06-03 7:47 UTC (permalink / raw)
To: wens; +Cc: dmitry.torokhov, linux-input, linux-kernel, Steven Feng
In-Reply-To: <CAGb2v64PLZn+VD9LdxOQxHC0FUJ=5fMrpwEyJEJHkZnUZkHRRw@mail.gmail.com>
This series adds power button support for the AXP313A PMIC.
The AXP313A lacks the PEK_KEY register used for configuring startup
and shutdown timings. To handle this, the first patch adds an
is_visible callback to hide unsupported sysfs attributes, and the
second patch adds the AXP313A device itself.
Changes in v4:
- Split into two patches
- First patch adds is_visible infrastructure
- Second patch adds AXP313A device support
- Fixed subject line to show version number
Changes in v3:
- Use is_visible to hide attributes instead of returning -EOPNOTSUPP
- Corrected author name from "steven" to "Steven Feng"
Changes in v2:
- Fixed regmap out-of-bounds access issue reported by Sashiko AI
- Created dedicated axp313a_info with NULL time tables
- AXP313A lacks PEK_KEY (0x36) per datasheet verification
v3: https://lore.kernel.org/all/tencent_81FCC0443B81A3078C17F44AB4B22DFB2E07@qq.com/
v2: https://lore.kernel.org/all/tencent_48A497E0CA81323CFB6C7CB84428019A8707@qq.com/
v1: https://lore.kernel.org/all/tencent_5F1FF80489E702360F352F889570656BF608@qq.com/
Steven Feng (2):
Input: axp20x-pek - add is_visible callback for sysfs attributes
Input: axp20x-pek - add support for AXP313A variant
drivers/input/misc/axp20x-pek.c | 34 ++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] Input: axp20x-pek - add support for AXP313A variant
From: Chen-Yu Tsai @ 2026-06-03 7:23 UTC (permalink / raw)
To: Steven Feng; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <tencent_81FCC0443B81A3078C17F44AB4B22DFB2E07@qq.com>
On Wed, Jun 3, 2026 at 11:00 AM Steven Feng <steven@joint-cloud.com> wrote:
>
> Add power button support for the AXP313A PMIC. The AXP313A does not
> have the PEK_KEY register (0x36) used to configure startup and shutdown
> timings, unlike AXP20X and AXP221. Create a dedicated axp313a_info
> configuration with NULL time tables to prevent regmap out-of-bounds
> access.
>
> For AXP313A, the sysfs attributes (startup/shutdown) are hidden using
> the is_visible callback, as they are not supported by the hardware.
>
> Signed-off-by: Steven Feng <steven@joint-cloud.com>
> ---
> Changes in v3:
> - Use is_visible to hide attributes instead of returning -EOPNOTSUPP
Adding this should be a separate patch. In the commit message you can
say it is needed for a future device added in the next patch.
> - Corrected author name from "steven" to "Steven Feng"
Please label the version in the subject as well. The subject should
read "[PATCH v3]" for v3. `git format-patch -vN ...` will do the right
thing for you.
ChenYu
> Changes in v2:
> - Fixed regmap out-of-bounds access issue reported by Sashiko AI
> - Created dedicated axp313a_info with NULL time tables
> - Added NULL pointer checks to prevent crashes
> - AXP313A lacks PEK_KEY (0x36) per datasheet verification
> - Power button still functional using hardware default timings
>
> v2: https://lore.kernel.org/all/tencent_48A497E0CA81323CFB6C7CB84428019A8707@qq.com/
> v1: https://lore.kernel.org/all/tencent_5F1FF80489E702360F352F889570656BF608@qq.com/
> ---
> drivers/input/misc/axp20x-pek.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index d4e2fc9a937f..964b39817af9 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -85,6 +85,13 @@ static const struct axp20x_info axp221_info = {
> .shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
> };
>
> +static const struct axp20x_info axp313a_info = {
> + .startup_time = NULL,
> + .startup_mask = 0,
> + .shutdown_time = NULL,
> + .shutdown_mask = 0,
> +};
> +
> static ssize_t axp20x_show_attr(struct device *dev,
> const struct axp20x_time *time,
> unsigned int mask, char *buf)
> @@ -193,7 +200,28 @@ static struct attribute *axp20x_attrs[] = {
> &dev_attr_shutdown.attr,
> NULL,
> };
> -ATTRIBUTE_GROUPS(axp20x);
> +
> +static umode_t axp20x_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> +
> + if (!axp20x_pek->info->startup_time)
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static const struct attribute_group axp20x_group = {
> + .attrs = axp20x_attrs,
> + .is_visible = axp20x_attr_is_visible,
> +};
> +
> +static const struct attribute_group *axp20x_groups[] = {
> + &axp20x_group,
> + NULL,
> +};
>
> static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
> {
> @@ -395,7 +423,7 @@ static const struct platform_device_id axp_pek_id_match[] = {
> },
> {
> .name = "axp313a-pek",
> - .driver_data = (kernel_ulong_t)&axp20x_info,
> + .driver_data = (kernel_ulong_t)&axp313a_info,
> },
> { /* sentinel */ }
> };
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: Xu, Even @ 2026-06-03 7:20 UTC (permalink / raw)
To: d3z, Sun, Xinpeng, jikos@kernel.org, bentiss@kernel.org
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
abhishektamboli9@gmail.com, sakari.ailus@linux.intel.com
In-Reply-To: <20260602151317.27768-1-d3z.the.dev@gmail.com>
Hi, Danny,
Thank you very much!
Very appreciate your testing!
I noticed one thing in your description " there's no fresh 'Wait RESET_RESPONSE timeout' line this cycle ".
Generally, during reset_tic() execution, THC will send/receive data to/from TIC for RESET response, device descriptor.
If there is no any error log after you only add reset_tic() in resume() callback, that means THC isn't power gated, it still keep previous setting and can receive interrupt and response interrupt correctly.
Suddenly, I recognized, reset_tic() will set device state to RESETTING:
qsdev->state = QUICKSPI_RESETING;
So after reset_tic() call, driver must manually set device state to ENABLE:
qsdev->state = QUICKSPI_ENABLED;
If no this setting, THC hardware works normally, but driver will ignore all input data, the phenomenon is that the touch screen still has no response.
That's my bad, I forgot this.
So would you mind try again, to add the missed line, like this:
--- i/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ w/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -822,10 +822,17 @@ static int quickspi_resume(struct device *device)
if (ret)
return ret;
+ /* TIC may lose power, needs go through reset flow */
+ ret = reset_tic(qsdev);
+ if (ret)
+ return ret;
+
ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
if (ret)
return ret;
+ qsdev->state = QUICKSPI_ENABLED;
+
if (!device_may_wakeup(qsdev->dev))
return quickspi_set_power(qsdev, HIDSPI_ON);
Thanks!
Best Regards,
Even Xu
> -----Original Message-----
> From: d3z <d3z.the.dev@gmail.com>
> Sent: Tuesday, June 2, 2026 11:13 PM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>;
> jikos@kernel.org; bentiss@kernel.org
> Cc: Danny D . <d3z.the.dev@gmail.com>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; abhishektamboli9@gmail.com;
> sakari.ailus@linux.intel.com
> Subject: Re: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
>
> From: Danny D. <d3z.the.dev@gmail.com>
>
> Hi Even,
>
> Good way to split it - "touch IC off" vs "THC power-gated" - so I ran the
> reset_tic()-only test you asked for.
>
> I added just reset_tic() to a pristine quickspi_resume(), with none of the THC
> reconfig (no thc_spi_*_config, no thc_ltr_*). On the Surface Pro 10, after an
> s2idle suspend/resume, the touchscreen does NOT come back - so
> reset_tic() alone isn't enough here.
>
> The interesting bit is why. The one new line on resume is:
>
> intel_quickspi 0000:00:10.0: THC interrupt already unquiesce
>
> That's reset_tic() -> thc_interrupt_quiesce(dev, false) finding
> DEVINT_QUIESCE_EN already clear - even though quickspi_suspend() set it in
> suspend. thc_regmap is REGCACHE_NONE, so that's the real register, not a
> cached value. So the THC port-control state we wrote in suspend is gone after
> resume.
>
> That's your second case: the THC itself loses its register context across s2idle,
> without ever hitting PM_SUSPEND_MEM. reset_tic()'s SPI exchange then runs
> against a THC whose I/O address and read/write config are wiped, so the reset
> never completes. Reprogramming those first (like quickspi_restore(), and like v2
> on the no-wake path) is what brings touch and pen back.
>
> So on the SP10 both are needed - reset the touch IC AND reconfigure the THC -
> the full reconfigure looks necessary here.
>
> One note on the log: there's no fresh "Wait RESET_RESPONSE timeout" line this
> cycle, but that path is dev_err_once() and the box had ~12h uptime, so an earlier
> cycle likely already consumed it.
>
> Thanks,
> Danny
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox