* 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
* Re: [PATCH] Input: xilinx_ps2 - remove driver
From: Michal Simek @ 2026-06-03 6:20 UTC (permalink / raw)
To: Rosen Penev, linux-input
Cc: Dmitry Torokhov, open list, moderated list:ARM/ZYNQ ARCHITECTURE
In-Reply-To: <20260603054217.442016-1-rosenp@gmail.com>
On 6/3/26 07:42, Rosen Penev wrote:
> Remove the Xilinx XPS PS/2 controller driver. This driver supports an
> old Xilinx EDK IP core that is no longer in active use. The hardware
> is not available on modern platforms, and the driver has no users here.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/input/serio/Kconfig | 10 -
> drivers/input/serio/Makefile | 1 -
> drivers/input/serio/xilinx_ps2.c | 363 -------------------------------
> 3 files changed, 374 deletions(-)
> delete mode 100644 drivers/input/serio/xilinx_ps2.c
Acked-by: Michal Simek <michal.simek@amd.com>
Thanks,
Michal
^ permalink raw reply
* [PATCH] Input: xilinx_ps2 - remove driver
From: Rosen Penev @ 2026-06-03 5:42 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, Michal Simek, open list,
moderated list:ARM/ZYNQ ARCHITECTURE
Remove the Xilinx XPS PS/2 controller driver. This driver supports an
old Xilinx EDK IP core that is no longer in active use. The hardware
is not available on modern platforms, and the driver has no users here.
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/input/serio/Kconfig | 10 -
drivers/input/serio/Makefile | 1 -
drivers/input/serio/xilinx_ps2.c | 363 -------------------------------
3 files changed, 374 deletions(-)
delete mode 100644 drivers/input/serio/xilinx_ps2.c
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 5f15a6462056..bacab1f58400 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -188,16 +188,6 @@ config SERIO_RAW
To compile this driver as a module, choose M here: the
module will be called serio_raw.
-config SERIO_XILINX_XPS_PS2
- tristate "Xilinx XPS PS/2 Controller Support"
- depends on PPC || MICROBLAZE
- help
- This driver supports XPS PS/2 IP from the Xilinx EDK on
- PowerPC platform.
-
- To compile this driver as a module, choose M here: the
- module will be called xilinx_ps2.
-
config SERIO_ALTERA_PS2
tristate "Altera UP PS/2 controller"
depends on HAS_IOMEM
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 8ab98f4aa28d..7f5137dc02e2 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -23,7 +23,6 @@ obj-$(CONFIG_SERIO_SGI_IOC3) += ioc3kbd.o
obj-$(CONFIG_SERIO_LIBPS2) += libps2.o
obj-$(CONFIG_SERIO_RAW) += serio_raw.o
obj-$(CONFIG_SERIO_AMS_DELTA) += ams_delta_serio.o
-obj-$(CONFIG_SERIO_XILINX_XPS_PS2) += xilinx_ps2.o
obj-$(CONFIG_SERIO_ALTERA_PS2) += altera_ps2.o
obj-$(CONFIG_SERIO_ARC_PS2) += arc_ps2.o
obj-$(CONFIG_SERIO_APBPS2) += apbps2.o
diff --git a/drivers/input/serio/xilinx_ps2.c b/drivers/input/serio/xilinx_ps2.c
deleted file mode 100644
index 411d55ca1a66..000000000000
--- a/drivers/input/serio/xilinx_ps2.c
+++ /dev/null
@@ -1,363 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Xilinx XPS PS/2 device driver
- *
- * (c) 2005 MontaVista Software, Inc.
- * (c) 2008 Xilinx, Inc.
- */
-
-
-#include <linux/module.h>
-#include <linux/serio.h>
-#include <linux/interrupt.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/list.h>
-#include <linux/io.h>
-#include <linux/mod_devicetable.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/platform_device.h>
-
-#define DRIVER_NAME "xilinx_ps2"
-
-/* Register offsets for the xps2 device */
-#define XPS2_SRST_OFFSET 0x00000000 /* Software Reset register */
-#define XPS2_STATUS_OFFSET 0x00000004 /* Status register */
-#define XPS2_RX_DATA_OFFSET 0x00000008 /* Receive Data register */
-#define XPS2_TX_DATA_OFFSET 0x0000000C /* Transmit Data register */
-#define XPS2_GIER_OFFSET 0x0000002C /* Global Interrupt Enable reg */
-#define XPS2_IPISR_OFFSET 0x00000030 /* Interrupt Status register */
-#define XPS2_IPIER_OFFSET 0x00000038 /* Interrupt Enable register */
-
-/* Reset Register Bit Definitions */
-#define XPS2_SRST_RESET 0x0000000A /* Software Reset */
-
-/* Status Register Bit Positions */
-#define XPS2_STATUS_RX_FULL 0x00000001 /* Receive Full */
-#define XPS2_STATUS_TX_FULL 0x00000002 /* Transmit Full */
-
-/*
- * Bit definitions for ISR/IER registers. Both the registers have the same bit
- * definitions and are only defined once.
- */
-#define XPS2_IPIXR_WDT_TOUT 0x00000001 /* Watchdog Timeout Interrupt */
-#define XPS2_IPIXR_TX_NOACK 0x00000002 /* Transmit No ACK Interrupt */
-#define XPS2_IPIXR_TX_ACK 0x00000004 /* Transmit ACK (Data) Interrupt */
-#define XPS2_IPIXR_RX_OVF 0x00000008 /* Receive Overflow Interrupt */
-#define XPS2_IPIXR_RX_ERR 0x00000010 /* Receive Error Interrupt */
-#define XPS2_IPIXR_RX_FULL 0x00000020 /* Receive Data Interrupt */
-
-/* Mask for all the Transmit Interrupts */
-#define XPS2_IPIXR_TX_ALL (XPS2_IPIXR_TX_NOACK | XPS2_IPIXR_TX_ACK)
-
-/* Mask for all the Receive Interrupts */
-#define XPS2_IPIXR_RX_ALL (XPS2_IPIXR_RX_OVF | XPS2_IPIXR_RX_ERR | \
- XPS2_IPIXR_RX_FULL)
-
-/* Mask for all the Interrupts */
-#define XPS2_IPIXR_ALL (XPS2_IPIXR_TX_ALL | XPS2_IPIXR_RX_ALL | \
- XPS2_IPIXR_WDT_TOUT)
-
-/* Global Interrupt Enable mask */
-#define XPS2_GIER_GIE_MASK 0x80000000
-
-struct xps2data {
- int irq;
- spinlock_t lock;
- void __iomem *base_address; /* virt. address of control registers */
- unsigned int flags;
- struct serio *serio; /* serio */
- struct device *dev;
-};
-
-/************************************/
-/* XPS PS/2 data transmission calls */
-/************************************/
-
-/**
- * xps2_recv() - attempts to receive a byte from the PS/2 port.
- * @drvdata: pointer to ps2 device private data structure
- * @byte: address where the read data will be copied
- *
- * If there is any data available in the PS/2 receiver, this functions reads
- * the data, otherwise it returns error.
- */
-static int xps2_recv(struct xps2data *drvdata, u8 *byte)
-{
- u32 sr;
- int status = -1;
-
- /* If there is data available in the PS/2 receiver, read it */
- sr = in_be32(drvdata->base_address + XPS2_STATUS_OFFSET);
- if (sr & XPS2_STATUS_RX_FULL) {
- *byte = in_be32(drvdata->base_address + XPS2_RX_DATA_OFFSET);
- status = 0;
- }
-
- return status;
-}
-
-/*********************/
-/* Interrupt handler */
-/*********************/
-static irqreturn_t xps2_interrupt(int irq, void *dev_id)
-{
- struct xps2data *drvdata = dev_id;
- u32 intr_sr;
- u8 c;
- int status;
-
- /* Get the PS/2 interrupts and clear them */
- intr_sr = in_be32(drvdata->base_address + XPS2_IPISR_OFFSET);
- out_be32(drvdata->base_address + XPS2_IPISR_OFFSET, intr_sr);
-
- /* Check which interrupt is active */
- if (intr_sr & XPS2_IPIXR_RX_OVF)
- dev_warn(drvdata->dev, "receive overrun error\n");
-
- if (intr_sr & XPS2_IPIXR_RX_ERR)
- drvdata->flags |= SERIO_PARITY;
-
- if (intr_sr & (XPS2_IPIXR_TX_NOACK | XPS2_IPIXR_WDT_TOUT))
- drvdata->flags |= SERIO_TIMEOUT;
-
- if (intr_sr & XPS2_IPIXR_RX_FULL) {
- status = xps2_recv(drvdata, &c);
-
- /* Error, if a byte is not received */
- if (status) {
- dev_err(drvdata->dev,
- "wrong rcvd byte count (%d)\n", status);
- } else {
- serio_interrupt(drvdata->serio, c, drvdata->flags);
- drvdata->flags = 0;
- }
- }
-
- return IRQ_HANDLED;
-}
-
-/*******************/
-/* serio callbacks */
-/*******************/
-
-/**
- * sxps2_write() - sends a byte out through the PS/2 port.
- * @pserio: pointer to the serio structure of the PS/2 port
- * @c: data that needs to be written to the PS/2 port
- *
- * This function checks if the PS/2 transmitter is empty and sends a byte.
- * Otherwise it returns error. Transmission fails only when nothing is connected
- * to the PS/2 port. Thats why, we do not try to resend the data in case of a
- * failure.
- */
-static int sxps2_write(struct serio *pserio, unsigned char c)
-{
- struct xps2data *drvdata = pserio->port_data;
- u32 sr;
-
- guard(spinlock_irqsave)(&drvdata->lock);
-
- /* If the PS/2 transmitter is empty send a byte of data */
- sr = in_be32(drvdata->base_address + XPS2_STATUS_OFFSET);
- if (sr & XPS2_STATUS_TX_FULL)
- return -EAGAIN;
-
- out_be32(drvdata->base_address + XPS2_TX_DATA_OFFSET, c);
- return 0;
-}
-
-/**
- * sxps2_open() - called when a port is opened by the higher layer.
- * @pserio: pointer to the serio structure of the PS/2 device
- *
- * This function requests irq and enables interrupts for the PS/2 device.
- */
-static int sxps2_open(struct serio *pserio)
-{
- struct xps2data *drvdata = pserio->port_data;
- int error;
- u8 c;
-
- error = request_irq(drvdata->irq, &xps2_interrupt, 0,
- DRIVER_NAME, drvdata);
- if (error) {
- dev_err(drvdata->dev,
- "Couldn't allocate interrupt %d\n", drvdata->irq);
- return error;
- }
-
- /* start reception by enabling the interrupts */
- out_be32(drvdata->base_address + XPS2_GIER_OFFSET, XPS2_GIER_GIE_MASK);
- out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, XPS2_IPIXR_RX_ALL);
- (void)xps2_recv(drvdata, &c);
-
- return 0; /* success */
-}
-
-/**
- * sxps2_close() - frees the interrupt.
- * @pserio: pointer to the serio structure of the PS/2 device
- *
- * This function frees the irq and disables interrupts for the PS/2 device.
- */
-static void sxps2_close(struct serio *pserio)
-{
- struct xps2data *drvdata = pserio->port_data;
-
- /* Disable the PS2 interrupts */
- out_be32(drvdata->base_address + XPS2_GIER_OFFSET, 0x00);
- out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, 0x00);
- free_irq(drvdata->irq, drvdata);
-}
-
-/**
- * xps2_of_probe - probe method for the PS/2 device.
- * @ofdev: pointer to OF device structure
- *
- * This function probes the PS/2 device in the device tree.
- * It initializes the driver data structure and the hardware.
- * It returns 0, if the driver is bound to the PS/2 device, or a negative
- * value if there is an error.
- */
-static int xps2_of_probe(struct platform_device *ofdev)
-{
- struct resource r_mem; /* IO mem resources */
- struct xps2data *drvdata;
- struct serio *serio;
- struct device *dev = &ofdev->dev;
- resource_size_t remap_size, phys_addr;
- unsigned int irq;
- int error;
-
- dev_info(dev, "Device Tree Probing \'%pOFn\'\n", dev->of_node);
-
- /* Get iospace for the device */
- error = of_address_to_resource(dev->of_node, 0, &r_mem);
- if (error) {
- dev_err(dev, "invalid address\n");
- return error;
- }
-
- /* Get IRQ for the device */
- irq = irq_of_parse_and_map(dev->of_node, 0);
- if (!irq) {
- dev_err(dev, "no IRQ found\n");
- return -ENODEV;
- }
-
- drvdata = kzalloc_obj(*drvdata);
- serio = kzalloc_obj(*serio);
- if (!drvdata || !serio) {
- error = -ENOMEM;
- goto failed1;
- }
-
- spin_lock_init(&drvdata->lock);
- drvdata->irq = irq;
- drvdata->serio = serio;
- drvdata->dev = dev;
-
- phys_addr = r_mem.start;
- remap_size = resource_size(&r_mem);
- if (!request_mem_region(phys_addr, remap_size, DRIVER_NAME)) {
- dev_err(dev, "Couldn't lock memory region at 0x%08llX\n",
- (unsigned long long)phys_addr);
- error = -EBUSY;
- goto failed1;
- }
-
- /* Fill in configuration data and add them to the list */
- drvdata->base_address = ioremap(phys_addr, remap_size);
- if (drvdata->base_address == NULL) {
- dev_err(dev, "Couldn't ioremap memory at 0x%08llX\n",
- (unsigned long long)phys_addr);
- error = -EFAULT;
- goto failed2;
- }
-
- /* Disable all the interrupts, just in case */
- out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, 0);
-
- /*
- * Reset the PS2 device and abort any current transaction,
- * to make sure we have the PS2 in a good state.
- */
- out_be32(drvdata->base_address + XPS2_SRST_OFFSET, XPS2_SRST_RESET);
-
- dev_info(dev, "Xilinx PS2 at 0x%08llX mapped to 0x%p, irq=%d\n",
- (unsigned long long)phys_addr, drvdata->base_address,
- drvdata->irq);
-
- serio->id.type = SERIO_8042;
- serio->write = sxps2_write;
- serio->open = sxps2_open;
- serio->close = sxps2_close;
- serio->port_data = drvdata;
- serio->dev.parent = dev;
- snprintf(serio->name, sizeof(serio->name),
- "Xilinx XPS PS/2 at %08llX", (unsigned long long)phys_addr);
- snprintf(serio->phys, sizeof(serio->phys),
- "xilinxps2/serio at %08llX", (unsigned long long)phys_addr);
-
- serio_register_port(serio);
-
- platform_set_drvdata(ofdev, drvdata);
- return 0; /* success */
-
-failed2:
- release_mem_region(phys_addr, remap_size);
-failed1:
- kfree(serio);
- kfree(drvdata);
-
- return error;
-}
-
-/**
- * xps2_of_remove - unbinds the driver from the PS/2 device.
- * @of_dev: pointer to OF device structure
- *
- * This function is called if a device is physically removed from the system or
- * if the driver module is being unloaded. It frees any resources allocated to
- * the device.
- */
-static void xps2_of_remove(struct platform_device *of_dev)
-{
- struct xps2data *drvdata = platform_get_drvdata(of_dev);
- struct resource r_mem; /* IO mem resources */
-
- serio_unregister_port(drvdata->serio);
- iounmap(drvdata->base_address);
-
- /* Get iospace of the device */
- if (of_address_to_resource(of_dev->dev.of_node, 0, &r_mem))
- dev_err(drvdata->dev, "invalid address\n");
- else
- release_mem_region(r_mem.start, resource_size(&r_mem));
-
- kfree(drvdata);
-}
-
-/* Match table for of_platform binding */
-static const struct of_device_id xps2_of_match[] = {
- { .compatible = "xlnx,xps-ps2-1.00.a", },
- { /* end of list */ },
-};
-MODULE_DEVICE_TABLE(of, xps2_of_match);
-
-static struct platform_driver xps2_of_driver = {
- .driver = {
- .name = DRIVER_NAME,
- .of_match_table = xps2_of_match,
- },
- .probe = xps2_of_probe,
- .remove = xps2_of_remove,
-};
-module_platform_driver(xps2_of_driver);
-
-MODULE_AUTHOR("Xilinx, Inc.");
-MODULE_DESCRIPTION("Xilinx XPS PS/2 driver");
-MODULE_LICENSE("GPL");
-
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Dmitry Torokhov @ 2026-06-03 5:10 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: sashiko-reviews, linux-input, Hans de Goede, Jin Yao
In-Reply-To: <dfe153e1-a3d1-4dca-a59b-59899082c4e9@app.fastmail.com>
On Mon, Jun 01, 2026 at 08:32:31PM +0200, Arnd Bergmann wrote:
> On Sat, May 30, 2026, at 02:45, sashiko-bot@kernel.org wrote:
> > [Severity: High]
> > Does this change break legacy devices that legitimately provide both a valid
> > GPIO and a valid IRQ?
> >
> > If a legacy platform provides both a valid GPIO and an IRQ, this condition
> > now evaluates to false.
> >
> > By skipping the GPIO request entirely, the driver falls back to treating the
> > device as a pure IRQ button. This forces the driver to lose hardware
> > debouncing and rely on a simulated release timer instead of checking the
> > physical GPIO line.
>
> There are only three drivers that provide the ->irq field. Two are the
> ones that I'm changing in this patch, the soc_button_array driver
> is the third and none of them set ->irq and ->gpio at the same time.
>
> >> */
> >> struct gpio_keys_button {
> >> unsigned int code;
> >> +#ifdef CONFIG_GPIOLIB_LEGACY
> >> int gpio;
> >> +#endif
> >> int active_low;
> >
> > [Severity: Medium]
> > Will this cause build failures for drivers that assign to this field when
> > CONFIG_GPIOLIB_LEGACY is disabled?
>
> Yes, that is the intention of the patch: nothing should set the
> ->gpio field unless GPIOLIB_LEGACY is set.
>
> > Other modern drivers dynamically create the platform device and pass legacy
> > GPIO numbers by directly assigning to this field:
> >
> > drivers/input/misc/soc_button_array.c:soc_button_device_create() {
> > ...
> > gpio_keys[n_buttons].gpio = gpio;
> > ...
> > }
> >
> > When CONFIG_GPIOLIB_LEGACY is disabled, this results in a compilation failure
> > since struct gpio_keys_button no longer has the gpio member.
>
> I previously included a patch force-enable GPIOLIB_LEGACY in this
> patch series, see
>
> https://lore.kernel.org/all/npijagtgyad33xxlq46b7kwzydhcgt5tkgd5ihsjl6t4czbqyf@umovipwh73i2/
>
> I made a mistake during a rebase, so my older patch was still
> included in my randconfig testing but not in the series I
> sent. All the other changes in it are now redundant, bit
> the soc_button_array portion indeed still remains.
>
> Hans, Dmitry, do you already have plans to deal with the
> soc_button_array driver to move it away from legacy GPIOs?
>
> So far I can see four possible ways we can deal with it,
> but none that I actually like:
>
> a) delay the problem, apply my original oneline change
> to 'select GPIOLIB_LEGACY' and fix it later, so we
> can make GPIOLIB_LEGACY default-disabled in 7.3.
>
> b) add a gpiod member to struct gpio_keys_button and skip
> the intermediate gpio number here. Clean it up later.
>
> c) always pass the gpio as an interrupt, as the driver
> already does for some machines
>
> d) add dynamic device properties that duplicate the
> information from ACPI/DMI, so the driver can
> stop using platform data
>
> e) disconnect gpio_keys_button from gpio-keys.c and
> register the buttons to the input subsystem
> directly from soc_button_device_create().
I tried doing e) and while it is possible I ended up re-implementing
most of gpio_keys.c which is not great. We still need both GPIO and
IRQ-only modes, and handling debouncing, both software and hardware, and
so on. So I would like to see if d) is possible. The problem is that we
need to reconstruct GPIO reference from existing gpio_desc. If Bart will
be OK with something like below then I think the conversion should be
achievable.
gpiolib: Introduce gpiod_get_software_node_reference()
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To support converting drivers (such as soc_button_array) to use static
device properties/software nodes, we need a way to reconstruct a GPIO
reference from an existing gpio_desc.
Introduce gpiod_get_software_node_reference() which populates a
struct software_node_ref_args with the GPIO device's fwnode, the pin
offset, and the appropriate active-low flag.
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/gpio/gpiolib.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/gpio/consumer.h | 12 ++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1e6dce430dca..63cef0fb02b5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -247,6 +247,41 @@ int gpiod_hwgpio(const struct gpio_desc *desc)
}
EXPORT_SYMBOL_GPL(gpiod_hwgpio);
+/**
+ * gpiod_get_software_node_reference() - Reconstruct a software node reference
+ * from a GPIO descriptor
+ * @desc: GPIO descriptor
+ * @args: Pointer to software node reference arguments structure to populate
+ *
+ * This function can be used to construct a software node reference from
+ * an existing GPIO descriptor. This is useful when a driver wants to pass
+ * a software node to a secondary device, but needs to reconstruct a GPIO
+ * reference dynamically.
+ *
+ * Returns:
+ * 0 on success, or a negative error code on failure.
+ */
+int gpiod_get_software_node_reference(const struct gpio_desc *desc,
+ struct software_node_ref_args *args)
+{
+ struct gpio_device *gdev;
+
+ if (IS_ERR_OR_NULL(desc) || !args)
+ return -EINVAL;
+
+ gdev = desc->gdev;
+
+ memset(args, 0, sizeof(*args));
+ args->fwnode = dev_fwnode(&gdev->dev);
+ args->nargs = 2;
+ args->args[0] = gpiod_hwgpio(desc);
+ if (test_bit(GPIOD_FLAG_ACTIVE_LOW, &desc->flags))
+ args->args[1] = GPIO_ACTIVE_LOW;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gpiod_get_software_node_reference);
+
/**
* gpiod_to_chip - Return the GPIO chip to which a GPIO descriptor belongs
* @desc: descriptor to return the chip of
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 3efb5cb1e1d1..c95b715a1d15 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -11,6 +11,7 @@
struct acpi_device;
struct device;
struct fwnode_handle;
+struct software_node_ref_args;
struct gpio_array;
struct gpio_desc;
@@ -177,6 +178,9 @@ int desc_to_gpio(const struct gpio_desc *desc);
int gpiod_hwgpio(const struct gpio_desc *desc);
+int gpiod_get_software_node_reference(const struct gpio_desc *desc,
+ struct software_node_ref_args *args);
+
struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode,
const char *con_id, int index,
enum gpiod_flags flags,
@@ -545,6 +549,14 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
return -EINVAL;
}
+static inline
+int gpiod_get_software_node_reference(const struct gpio_desc *desc,
+ struct software_node_ref_args *args)
+{
+ WARN_ON(desc);
+ return -EINVAL;
+}
+
static inline
struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode,
const char *con_id, int index,
Thanks.
--
Dmitry
^ permalink raw reply related
* [PATCH v3 2/2] arm64: dts: qcom: glymur-crd: Add FocalTech ft3d81 touchscreen support
From: Pradyot Kumar Nayak @ 2026-06-03 3:11 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Peng, Bjorn Andersson, Konrad Dybcio, Dmitry Baryshkov,
Abel Vesa
Cc: linux-arm-msm, linux-input, devicetree, linux-kernel,
Konrad Dybcio, Pradyot Kumar Nayak
In-Reply-To: <20260603-arm64-dts-glymur-crd-add-reset-gpio-to-v3-0-3453ef577bcf@oss.qualcomm.com>
The touchscreen module on Glymur/Mahua CRDs is different from
the one used on Hamoa CRDs and requires the reset-gpios to be wired to
the device. Without this in place the reset line will remain
permanently asserted during resume leaving the device offline and causing
all I2C transactions to fail with -ENXIO.
i2c_hid_of 3-0038: failed to change power setting.
i2c_hid_of 3-0038: PM: dpm_run_callback():
i2c_hid_core_pm_resume [i2c_hid] returns -6
i2c_hid_of 3-0038: PM: failed to resume async: error -6
The touchscreen on Glymur/Mahua-CRD is a focaltech ft3d81,
which is hardware-compatible with the ft8112.
we have added the required change in DT.
Fixes: e6bf559f7eb9 ("arm64: dts: qcom: glymur-crd: Enable keyboard, trackpad and touchscreen")
Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/glymur-crd.dts | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dts b/arch/arm64/boot/dts/qcom/glymur-crd.dts
index c98dfb3941fa..5cda08bb48bc 100644
--- a/arch/arm64/boot/dts/qcom/glymur-crd.dts
+++ b/arch/arm64/boot/dts/qcom/glymur-crd.dts
@@ -154,14 +154,15 @@ &i2c8 {
status = "okay";
touchscreen@38 {
- compatible = "hid-over-i2c";
+ compatible = "focaltech,ft3d81", "focaltech,ft8112";
reg = <0x38>;
- hid-descr-addr = <0x1>;
interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
- vdd-supply = <&vreg_misc_3p3>;
- vddl-supply = <&vreg_l15b_e0_1p8>;
+ vcc33-supply = <&vreg_misc_3p3>;
+ vccio-supply = <&vreg_l15b_e0_1p8>;
+
+ reset-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
pinctrl-0 = <&ts0_default>;
pinctrl-names = "default";
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/2] dt-bindings: input: focaltech,ft8112: Add focaltech,ft3d81 compatible
From: Pradyot Kumar Nayak @ 2026-06-03 3:11 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Peng, Bjorn Andersson, Konrad Dybcio, Dmitry Baryshkov,
Abel Vesa
Cc: linux-arm-msm, linux-input, devicetree, linux-kernel,
Konrad Dybcio, Pradyot Kumar Nayak
In-Reply-To: <20260603-arm64-dts-glymur-crd-add-reset-gpio-to-v3-0-3453ef577bcf@oss.qualcomm.com>
The Focaltech ft3d81 is fully compatible with the ft8112 i.e.
it uses the same I2C-HID protocol and the same power-on/reset sequencing,
DT nodes for boards carrying an ft3d81,can therefore bind to the existing
ft8112 driver without any additional changes.
Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
---
Documentation/devicetree/bindings/input/focaltech,ft8112.yaml | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml b/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
index 197f30b14d45..5ffa1246aba1 100644
--- a/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
+++ b/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
@@ -18,8 +18,13 @@ allOf:
properties:
compatible:
- enum:
- - focaltech,ft8112
+ oneOf:
+ - items:
+ - enum:
+ - focaltech,ft3d81
+ - const: focaltech,ft8112
+ - enum:
+ - focaltech,ft8112
reg:
maxItems: 1
--
2.43.0
^ permalink raw reply related
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