* Re: [PATCH v3] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: 谢致邦 (XIE Zhibang) @ 2026-06-09 10:10 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, bentiss
Cc: Yeking, dianders, jikos, linux-kernel, sashiko-bot,
sashiko-reviews, superm1
In-Reply-To: <4243c594-6535-4111-86cb-69784b0a34b7@kernel.org>
Hi Hans,
Thanks for the review.
The current header-based approach lets i2c-hid-acpi-prp0001.c work
independently by just including a header for the _DSM helper. Switching
to EXPORT_SYMBOL_GPL would force prp0001 to depend on i2c-hid-acpi —
a Kconfig "depends on I2C_HID_ACPI", plus module-level dependency at
load time — all for sharing a single function. I find that hard to
accept.
I understand that adding a new header for one prototype is unusual,
though it contains only a single static inline function and introduces
no runtime dependency between the two drivers.
Looking back at the full discussion: Benjamin made it clear from the
start that he doesn't want i2c-hid-of.c handling ACPI _DSM fallback.
His line is that ACPI devices and OF devices should each go through
their own drivers without cross-contamination. If you still prefer
exporting the function from i2c-hid-acpi.c, then prp0001 would have to
drag i2c-hid-acpi along with it — an independent leaf driver turned
into something that can't stand alone.
If you still prefer the export approach, then I'd rather take a simpler
approach: add an of_match directly to
i2c-hid-acpi.c, and return -ENODEV silently at the top of probe if
hid-descr-addr is present. Same logic as the prp0001 driver, but one
fewer file, no module dependency, no extra Kconfig entry, and no OF
driver touching ACPI devices — which should also satisfy Benjamin's
concern.
Regards,
Zhibang
^ permalink raw reply
* Re: [PATCH v3] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: 谢致邦 (XIE Zhibang) @ 2026-06-09 10:21 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, bentiss
Cc: Yeking, dianders, jikos, linux-kernel, sashiko-bot,
sashiko-reviews, superm1
In-Reply-To: <tencent_C2BEF7C0584BFE713DC7B41FFD7007B6D30A@qq.com>
Hi Hans,
Thanks for the review.
The current header-based approach lets i2c-hid-acpi-prp0001.c work
independently by just including a header for the _DSM helper. Switching
to EXPORT_SYMBOL_GPL would force prp0001 to depend on i2c-hid-acpi —
a Kconfig "depends on I2C_HID_ACPI", plus module-level dependency at
load time — all for sharing a single function. I find that hard to
accept.
I understand that adding a new header for one function is unusual,
though it contains only a single static inline and introduces no runtime
dependency between the two drivers.
Looking back at the full discussion: Benjamin made it clear from the
start that he doesn't want i2c-hid-of.c handling ACPI _DSM fallback.
His line is that ACPI devices and OF devices should each go through
their own drivers without cross-contamination. If you still prefer
exporting the function from i2c-hid-acpi.c, then prp0001 would have to
drag i2c-hid-acpi along with it — an independent leaf driver turned
into something that can't stand alone.
If you still prefer the export approach, then I'd rather take a simpler
approach: add an of_match directly to i2c-hid-acpi.c, and return
-ENODEV silently at the top of probe if hid-descr-addr is present. Same
logic as the prp0001 driver, but one fewer file, no module dependency,
no extra Kconfig entry, and no OF driver touching ACPI devices — which
should also satisfy Benjamin's concern.
[corrected: s/prototype/function/ — the header contains a definition,
not just a declaration]
Regards,
Zhibang
^ permalink raw reply
* [PATCH v4] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: 谢致邦 (XIE Zhibang) @ 2026-06-09 10:54 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, bentiss, yeking
Cc: dianders, jikos, linux-kernel, sashiko-bot, sashiko-reviews,
superm1, 谢致邦 (XIE Zhibang)
In-Reply-To: <tencent_45D6247F04CF37ED6A3F858178B4303D6808@qq.com>
Move the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c into i2c-hid-acpi.h as a static inline so both the ACPI
and the new PRP0001 driver can use it. While refactoring, move the
blacklist check and the _DSM call to the top of probe() to avoid a
pointless alloc when the device is blacklisted or does not implement the
_DSM.
Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
are declared with _HID "PRP0001" and _DSD compatible "hid-over-i2c" but
lack "hid-descr-addr" from the _DSD and provide the HID descriptor
address only through an ACPI _DSM. The OF driver fails to probe them
because it requires hid-descr-addr. Add a new driver that handles these
devices by calling the shared _DSM helper.
Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
v2: Name the unused parameter and document why
acpi_device_fix_up_power() is skipped.
v3: Add a dev_warn() asking users to contact vendors for firmware
updates, and use existing locals in devm_kzalloc() and
acpi_device_fix_up_power().
v4: Double the power-up delay from 250ms to 500ms.
drivers/hid/i2c-hid/Kconfig | 18 ++++
drivers/hid/i2c-hid/Makefile | 1 +
drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c | 98 ++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-acpi.c | 52 ++++--------
drivers/hid/i2c-hid/i2c-hid-acpi.h | 32 +++++++
5 files changed, 163 insertions(+), 38 deletions(-)
create mode 100644 drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
create mode 100644 drivers/hid/i2c-hid/i2c-hid-acpi.h
diff --git a/drivers/hid/i2c-hid/Kconfig b/drivers/hid/i2c-hid/Kconfig
index e8d51f410cc1..7db8b2abff78 100644
--- a/drivers/hid/i2c-hid/Kconfig
+++ b/drivers/hid/i2c-hid/Kconfig
@@ -22,6 +22,24 @@ config I2C_HID_ACPI
will be called i2c-hid-acpi. It will also build/depend on the
module i2c-hid.
+config I2C_HID_ACPI_PRP0001
+ tristate "Driver for PRP0001 devices missing hid-descr-addr"
+ depends on ACPI
+ depends on DRM || !DRM
+ select I2C_HID_CORE
+ help
+ Say Y here if you want support for I2C HID touchpads that
+ are declared with _HID "PRP0001" and _DSD compatible
+ "hid-over-i2c" but lack the "hid-descr-addr" property from
+ the _DSD. The HID descriptor address is instead provided
+ through an ACPI _DSM. Known affected devices include the
+ Lenovo KaiTian N60d and Inspur CP300L3.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the
+ module will be called i2c-hid-acpi-prp0001.
+
config I2C_HID_OF
tristate "HID over I2C transport layer Open Firmware driver"
# No "depends on OF" because this can also be used for manually
diff --git a/drivers/hid/i2c-hid/Makefile b/drivers/hid/i2c-hid/Makefile
index 55bd5e0f35af..da420c1a8ec6 100644
--- a/drivers/hid/i2c-hid/Makefile
+++ b/drivers/hid/i2c-hid/Makefile
@@ -9,6 +9,7 @@ i2c-hid-objs = i2c-hid-core.o
i2c-hid-$(CONFIG_DMI) += i2c-hid-dmi-quirks.o
obj-$(CONFIG_I2C_HID_ACPI) += i2c-hid-acpi.o
+obj-$(CONFIG_I2C_HID_ACPI_PRP0001) += i2c-hid-acpi-prp0001.o
obj-$(CONFIG_I2C_HID_OF) += i2c-hid-of.o
obj-$(CONFIG_I2C_HID_OF_ELAN) += i2c-hid-of-elan.o
obj-$(CONFIG_I2C_HID_OF_GOODIX) += i2c-hid-of-goodix.o
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
new file mode 100644
index 000000000000..a65830279f63
--- /dev/null
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HID over I2C driver for PRP0001 devices missing hid-descr-addr
+ *
+ * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3, use
+ * _HID "PRP0001" with _DSD compatible "hid-over-i2c" but lack "hid-descr-addr"
+ * from the _DSD. The HID descriptor address is provided only through an ACPI
+ * _DSM. The TPD0 node in the DSDT shows _DSM Function 1 returning 0x20.
+ *
+ * Copyright (C) 2026 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include "i2c-hid.h"
+#include "i2c-hid-acpi.h"
+
+static int i2c_hid_acpi_prp0001_power_up(struct i2chid_ops *ops)
+{
+ /* give the device time to power up */
+ msleep(500);
+ return 0;
+}
+
+static struct i2chid_ops i2c_hid_acpi_prp0001_ops = {
+ .power_up = i2c_hid_acpi_prp0001_power_up,
+ /*
+ * No .restore_sequence needed: the _DSM on these devices returns a
+ * constant (0x20) with no side effects, unlike some PNP0C50 _DSM
+ * implementations that switch the hardware between PS/2 and I2C modes.
+ */
+};
+
+static int i2c_hid_acpi_prp0001_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct acpi_device *adev;
+ u16 hid_descriptor_address;
+ int ret;
+
+ /* If hid-descr-addr is present, let i2c-hid-of handle it */
+ if (device_property_present(dev, "hid-descr-addr"))
+ return -ENODEV;
+
+ adev = ACPI_COMPANION(dev);
+ if (!adev)
+ return -ENODEV;
+
+ ret = i2c_hid_acpi_get_descriptor(adev);
+ if (ret < 0)
+ return ret;
+ dev_warn(dev,
+ "hid-descr-addr device property NOT found, using ACPI _DSM fallback. Contact vendor for firmware update!\n");
+ hid_descriptor_address = ret;
+
+ /*
+ * No acpi_device_fix_up_power() needed: TPD0 has no _PS0, _PS3, _PSC
+ * or _PRx methods and follows I2C bus power.
+ */
+ return i2c_hid_core_probe(client, &i2c_hid_acpi_prp0001_ops,
+ hid_descriptor_address, 0);
+}
+
+static const struct of_device_id i2c_hid_acpi_prp0001_of_match[] = {
+ { .compatible = "hid-over-i2c" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, i2c_hid_acpi_prp0001_of_match);
+
+static const struct i2c_device_id i2c_hid_acpi_prp0001_id[] = {
+ { .name = "hid-over-i2c" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, i2c_hid_acpi_prp0001_id);
+
+static struct i2c_driver i2c_hid_acpi_prp0001_driver = {
+ .driver = {
+ .name = "i2c_hid_acpi_prp0001",
+ .pm = &i2c_hid_core_pm,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .of_match_table = of_match_ptr(i2c_hid_acpi_prp0001_of_match),
+ },
+
+ .probe = i2c_hid_acpi_prp0001_probe,
+ .remove = i2c_hid_core_remove,
+ .shutdown = i2c_hid_core_shutdown,
+ .id_table = i2c_hid_acpi_prp0001_id,
+};
+
+module_i2c_driver(i2c_hid_acpi_prp0001_driver);
+
+MODULE_DESCRIPTION("HID over I2C driver for PRP0001 devices missing hid-descr-addr");
+MODULE_AUTHOR("谢致邦 (XIE Zhibang) <Yeking@Red54.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..13f977d6aab6 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,9 +25,9 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pm.h>
-#include <linux/uuid.h>
#include "i2c-hid.h"
+#include "i2c-hid-acpi.h"
struct i2c_hid_acpi {
struct i2chid_ops ops;
@@ -48,39 +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;
-
- 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) {
- 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_acpi_get_descriptor(ihid_acpi->adev);
}
static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -93,24 +65,28 @@ 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;
- ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
+ if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+ return -ENODEV;
+
+ ret = i2c_hid_acpi_get_descriptor(adev);
+ if (ret < 0)
+ return ret;
+ hid_descriptor_address = ret;
+
+ ihid_acpi = devm_kzalloc(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;
- ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
- if (ret < 0)
- return ret;
- hid_descriptor_address = ret;
-
- acpi_device_fix_up_power(ihid_acpi->adev);
+ acpi_device_fix_up_power(adev);
return i2c_hid_core_probe(client, &ihid_acpi->ops,
hid_descriptor_address, 0);
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.h b/drivers/hid/i2c-hid/i2c-hid-acpi.h
new file mode 100644
index 000000000000..8cebbeebcc23
--- /dev/null
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _I2C_HID_ACPI_H
+#define _I2C_HID_ACPI_H
+
+#include <linux/acpi.h>
+#include <linux/uuid.h>
+
+static inline int i2c_hid_acpi_get_descriptor(struct acpi_device *adev)
+{
+ /* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+ static const guid_t i2c_hid_guid =
+ GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+ 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+ acpi_handle handle = acpi_device_handle(adev);
+ union acpi_object *obj;
+ u16 addr;
+
+ 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;
+ }
+
+ addr = obj->integer.value;
+ ACPI_FREE(obj);
+ return addr;
+}
+
+#endif
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] arm64: dts: qcom: Add Microsoft Surface Pro 12in
From: Harrison Vanderbyl @ 2026-06-09 11:04 UTC (permalink / raw)
To: konrad.dybcio
Cc: andersson, bentiss, conor+dt, devicetree, dmitry.baryshkov,
harrison.vanderbyl, jikos, konradybcio, krzk+dt, linux-arm-msm,
linux-input, linux-kernel, robh
In-Reply-To: <4ced3207-a132-4ec5-a0eb-79f75f5cb4bb@oss.qualcomm.com>
On 6/9/26 11:30 AM, Konrad Dybcio wrote:
> Because we'd put the subnodes for devices under the 'status' line (DTC
> enforces that), please move the comment below as well
Will do,
> Or since it's @28, perhaps it's just nxp,nxp-nci-i2c and it could work
> for you ootb?
Unfortunately for me, the device/antenna is not present on the consumer version,
I was able to confirm this during a speaker replacement.
> Please align the <s (or is it my mail client playing tricks on me?)
The alignment changes depending on what text editor I copy paste this section into.
I have them aligned the same as the other device trees are aligned though.
^ permalink raw reply
* Re: [PATCH v4] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: sashiko-bot @ 2026-06-09 11:06 UTC (permalink / raw)
Cc: dmitry.torokhov, linux-input
In-Reply-To: <tencent_6169D37BDAAD7FFEEC9E41DA197FE6028309@qq.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `of_match_ptr()` on the `of_match_table` prevents native ACPI PRP0001 matching when `CONFIG_OF` is disabled.
--
commit fdffe866c804a579c14dbd7b1c8529b20466017b
Author: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
This commit moves the _DSM call that retrieves the HID descriptor address into
a shared header. It also introduces the i2c-hid-acpi-prp0001 driver to handle
devices with _HID "PRP0001" and _DSD compatible "hid-over-i2c" that are missing
the "hid-descr-addr" property.
> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
[ ... ]
> +static const struct of_device_id i2c_hid_acpi_prp0001_of_match[] = {
> + { .compatible = "hid-over-i2c" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, i2c_hid_acpi_prp0001_of_match);
> +
[ ... ]
> +static struct i2c_driver i2c_hid_acpi_prp0001_driver = {
> + .driver = {
> + .name = "i2c_hid_acpi_prp0001",
> + .pm = &i2c_hid_core_pm,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + .of_match_table = of_match_ptr(i2c_hid_acpi_prp0001_of_match),
[Severity: High]
Will this prevent the driver from probing on ACPI systems that do not enable
CONFIG_OF?
The driver relies on ACPI PRP0001 matching to match the _DSD compatible
property against the of_match_table. However, if CONFIG_OF is disabled
(which is standard for x86 ACPI platforms where this hardware exists),
of_match_ptr() evaluates to NULL.
When of_match_table is NULL, the ACPI driver core will silently fail to
match the device, leaving the touchpad non-functional. Should we drop the
of_match_ptr() wrapper here to ensure the table is always available for
PRP0001 matching?
> + },
> +
> + .probe = i2c_hid_acpi_prp0001_probe,
> + .remove = i2c_hid_core_remove,
> + .shutdown = i2c_hid_core_shutdown,
> + .id_table = i2c_hid_acpi_prp0001_id,
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_6169D37BDAAD7FFEEC9E41DA197FE6028309@qq.com?part=1
^ permalink raw reply
* Re: [PATCH v3 2/2] arm64: dts: qcom: Add Microsoft Surface Pro 12in
From: Konrad Dybcio @ 2026-06-09 12:06 UTC (permalink / raw)
To: Harrison Vanderbyl
Cc: andersson, bentiss, conor+dt, devicetree, dmitry.baryshkov, jikos,
konradybcio, krzk+dt, linux-arm-msm, linux-input, linux-kernel,
robh
In-Reply-To: <20260609110429.11427-1-harrison.vanderbyl@gmail.com>
On 6/9/26 1:04 PM, Harrison Vanderbyl wrote:
> On 6/9/26 11:30 AM, Konrad Dybcio wrote:
>> Because we'd put the subnodes for devices under the 'status' line (DTC
>> enforces that), please move the comment below as well
>
> Will do,
>
>> Or since it's @28, perhaps it's just nxp,nxp-nci-i2c and it could work
>> for you ootb?
>
> Unfortunately for me, the device/antenna is not present on the consumer version,
> I was able to confirm this during a speaker replacement.
So perhaps "commercial devices only" refers to ""Surface Pro for Business
12-inch"" [1] which is presumably an actual separate SKU?
I initially interpreted "commercial devices" as "not prototypes"
[1] https://cdn-dynmedia-1.microsoft.com/is/content/microsoftcorp/microsoft/mlsd/documents/presentations/en-us/surface/MSFT-echo-surface-pro-12-inch-snapdragon-techspecs-factsheet.pdf
>
>> Please align the <s (or is it my mail client playing tricks on me?)
>
> The alignment changes depending on what text editor I copy paste this section into.
This shouldn't be the case, so long as you set the kernel-standard tab
width to 8.
Konrad
^ permalink raw reply
* [PATCH v3 1/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Lee Jones @ 2026-06-09 12:13 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
inside the interrupt handler context. If the Touch sibling device
is disconnected, wacom_remove_shared_data() clears 'touch_input'
outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
race condition where a preempted reader in interrupt context
dereferences the freed pointer, leading to a Use-After-Free.
Resolve this by introducing RCU protection for the touch_input
pointer:
- Annotate 'touch_input' in wacom_shared struct with __rcu
- Wrap all lockless readers in wacom_wac.c with rcu_read_lock() and
rcu_dereference() using a unified wacom_report_touch_mute()
helper
- Update writers in wacom_sys.c using rcu_assign_pointer()
- Call synchronize_rcu() in wacom_remove_shared_data() to ensure
all active RCU readers have finished before the input device is
freed
Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
concurrent probe assignments, and verify that 'shared->touch == hdev'
before setting touch_input to prevent concurrent sibling probe state
desynchronization.
Finally, advertise the SW_MUTE_DEVICE capability on Touch input
devices prior to registration in wacom_setup_touch_input_capabilities()
to prevent invalid post-registration capability modifications.
Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
drivers/hid/wacom_sys.c | 41 ++++++++++++++----------
drivers/hid/wacom_wac.c | 70 ++++++++++++++++++++++-------------------
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..7ba589826548 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
- if (wacom_wac->shared->touch == wacom->hdev)
- wacom_wac->shared->touch = NULL;
- else if (wacom_wac->shared->pen == wacom->hdev)
- wacom_wac->shared->pen = NULL;
+ scoped_guard(mutex, &wacom_udev_list_lock) {
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->touch = NULL;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+ } else if (wacom_wac->shared->pen == wacom->hdev) {
+ wacom_wac->shared->pen = NULL;
+ }
+ }
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list);
}
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+ data->shared.touch = hdev;
+ else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+ data->shared.pen = hdev;
+
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
if (retval)
return retval;
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- wacom_wac->shared->touch = hdev;
- else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- wacom_wac->shared->pen = hdev;
-
return retval;
}
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+ mutex_lock(&wacom_udev_list_lock);
+
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
}
if (wacom_wac->has_mute_touch_switch) {
@@ -2361,12 +2373,7 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
wacom_wac->shared->is_touch_on = true;
}
- if (wacom_wac->shared->has_mute_touch_switch &&
- wacom_wac->shared->touch_input) {
- set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
- input_set_capability(wacom_wac->shared->touch_input, EV_SW,
- SW_MUTE_DEVICE);
- }
+ mutex_unlock(&wacom_udev_list_lock);
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..495960227b8d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -510,6 +510,22 @@ static void wacom_intuos_schedule_prox_event(struct wacom_wac *wacom_wac)
}
}
+static void wacom_report_touch_mute(struct wacom_wac *wacom_wac, bool mute)
+{
+ struct input_dev *touch_input;
+
+ if (!wacom_wac->shared)
+ return;
+
+ rcu_read_lock();
+ touch_input = rcu_dereference(wacom_wac->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input, SW_MUTE_DEVICE, mute);
+ input_sync(touch_input);
+ }
+ rcu_read_unlock();
+}
+
static int wacom_intuos_pad(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
@@ -650,12 +666,8 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
input_report_key(input, KEY_CONTROLPANEL, menu);
input_report_key(input, KEY_INFO, info);
- if (wacom->shared && wacom->shared->touch_input) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE,
- !wacom->shared->is_touch_on);
- input_sync(wacom->shared->touch_input);
- }
+ if (wacom->shared)
+ wacom_report_touch_mute(wacom, !wacom->shared->is_touch_on);
input_report_abs(input, ABS_RX, strip1);
input_report_abs(input, ABS_RY, strip2);
@@ -2153,7 +2165,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
*/
if ((equivalent_usage == WACOM_HID_WD_MUTE_DEVICE) ||
(equivalent_usage == WACOM_HID_WD_TOUCHONOFF)) {
- if (wacom_wac->shared->touch_input) {
+ if (wacom_wac->shared) {
bool *is_touch_on = &wacom_wac->shared->is_touch_on;
if (equivalent_usage == WACOM_HID_WD_MUTE_DEVICE && value)
@@ -2161,9 +2173,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
else if (equivalent_usage == WACOM_HID_WD_TOUCHONOFF)
*is_touch_on = value;
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, !(*is_touch_on));
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, !(*is_touch_on));
}
return;
}
@@ -3381,11 +3391,8 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
if ((wacom->shared->type == INTUOSHT ||
wacom->shared->type == INTUOSHT2) &&
- wacom->shared->touch_input &&
wacom->shared->touch_max) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE, data[5] & 0x40);
- input_sync(wacom->shared->touch_input);
+ wacom_report_touch_mute(wacom, data[5] & 0x40);
}
pid = get_unaligned_be16(&data[6]);
@@ -3420,11 +3427,8 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
if ((features->type == INTUOSHT ||
features->type == INTUOSHT2) &&
- wacom_wac->shared->touch_input &&
features->touch_max) {
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, data[8] & 0x40);
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, data[8] & 0x40);
}
if (data[9] & 0x02) { /* wireless module is attached */
@@ -3951,11 +3955,22 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+ struct hid_device *hdev = wacom->hdev;
struct wacom_features *features = &wacom_wac->features;
if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
return -ENODEV;
+ if (features->type != TABLETPC &&
+ features->type != TABLETPC2FG &&
+ features->type != MTSCREEN &&
+ features->type != MTTPC &&
+ features->type != MTTPC_B) {
+ input_dev->evbit[0] |= BIT_MASK(EV_SW);
+ __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+ }
+
if (features->device_type & WACOM_DEVICETYPE_DIRECT)
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
else
@@ -3992,22 +4007,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
switch (features->type) {
case INTUOSP2_BT:
case INTUOSP2S_BT:
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
-
- if (wacom_wac->shared->touch->product == 0x361) {
+ if (hdev->product == 0x361) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 12440, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
0, 8640, 4, 0);
- }
- else if (wacom_wac->shared->touch->product == 0x360) {
+ } else if (hdev->product == 0x360) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 8960, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
0, 5920, 4, 0);
- }
- else if (wacom_wac->shared->touch->product == 0x393) {
+ } else if (hdev->product == 0x393) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 6400, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -4037,10 +4047,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
fallthrough;
case WACOM_27QHDT:
- if (wacom_wac->shared->touch->product == 0x32C ||
- wacom_wac->shared->touch->product == 0xF6) {
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+ if (hdev->product == 0x32C ||
+ hdev->product == 0xF6) {
wacom_wac->has_mute_touch_switch = true;
wacom_wac->is_soft_touch_switch = true;
}
@@ -4059,8 +4067,6 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
case INTUOSHT:
case INTUOSHT2:
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
fallthrough;
case BAMBOO_PT:
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
/* for wireless device to access USB interfaces */
unsigned touch_max;
int type;
- struct input_dev *touch_input;
+ struct input_dev __rcu *touch_input;
struct hid_device *pen;
struct hid_device *touch;
bool has_mute_touch_switch;
--
2.54.0.1099.g489fc7bff1-goog
^ permalink raw reply related
* [PATCH v3 2/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: Lee Jones @ 2026-06-09 12:13 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-1-lee@kernel.org>
wacom_bamboo_pad_pen_event() accesses wacom->shared->pen locklessly
relative to wacom_remove_shared_data() which nullifies it. This
can lead to a Use-After-Free if the sibling device is removed while
events are being processed.
Resolve this by introducing RCU protection for pen and touch
pointers:
- Annotate 'pen' and 'touch' in wacom_shared struct with __rcu.
- Wrap lockless readers in wacom_bamboo_pad_pen_event() with
rcu_read_lock() and rcu_dereference().
- Update writers in wacom_sys.c using rcu_assign_pointer().
- Use rcu_dereference_protected for comparisons under
wacom_udev_list_lock.
- Also use rcu_access_pointer in wacom_mode_change_work() to avoid
warnings (while lockless access there remains a pre-existing issue).
Fixes: 8c97a765467c ("HID: wacom: add full support of the Wacom Bamboo PAD")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
drivers/hid/wacom_sys.c | 36 +++++++++++++++++++++++++-----------
drivers/hid/wacom_wac.c | 14 +++++++-------
drivers/hid/wacom_wac.h | 4 ++--
3 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 7ba589826548..9b352027aa98 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -878,11 +878,18 @@ static void wacom_remove_shared_data(void *res)
shared);
scoped_guard(mutex, &wacom_udev_list_lock) {
- if (wacom_wac->shared->touch == wacom->hdev) {
- wacom_wac->shared->touch = NULL;
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+ struct hid_device *pen =
+ rcu_dereference_protected(wacom_wac->shared->pen,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (wacom_wac->shared->pen == wacom->hdev) {
- wacom_wac->shared->pen = NULL;
+ } else if (pen == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
@@ -916,9 +923,9 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- data->shared.touch = hdev;
+ rcu_assign_pointer(data->shared.touch, hdev);
else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- data->shared.pen = hdev;
+ rcu_assign_pointer(data->shared.pen, hdev);
mutex_unlock(&wacom_udev_list_lock);
@@ -2356,7 +2363,11 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
mutex_lock(&wacom_udev_list_lock);
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- if (wacom_wac->shared->touch == wacom->hdev) {
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
wacom_wac->shared->type = wacom_wac->features.type;
rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
}
@@ -2797,16 +2808,19 @@ static void wacom_mode_change_work(struct work_struct *work)
bool is_direct = wacom->wacom_wac.is_direct_mode;
int error = 0;
- if (shared->pen) {
- wacom1 = hid_get_drvdata(shared->pen);
+ struct hid_device *pen = rcu_access_pointer(shared->pen);
+ struct hid_device *touch = rcu_access_pointer(shared->touch);
+
+ if (pen) {
+ wacom1 = hid_get_drvdata(pen);
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
wacom1->wacom_wac.has_mode_change = true;
wacom1->wacom_wac.is_direct_mode = is_direct;
}
- if (shared->touch) {
- wacom2 = hid_get_drvdata(shared->touch);
+ if (touch) {
+ wacom2 = hid_get_drvdata(touch);
wacom_release_resources(wacom2);
hid_hw_stop(wacom2->hdev);
wacom2->wacom_wac.has_mode_change = true;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 495960227b8d..32d6f1dfb001 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3296,6 +3296,7 @@ static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
unsigned char *data)
{
+ struct hid_device *pen;
unsigned char prefix;
/*
@@ -3308,13 +3309,12 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
prefix = data[0];
data[0] = WACOM_REPORT_BPAD_PEN;
- /*
- * actually reroute the event.
- * No need to check if wacom->shared->pen is valid, hid_input_report()
- * will check for us.
- */
- hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
- WACOM_PKGLEN_PENABLED, 1);
+ rcu_read_lock();
+ pen = rcu_dereference(wacom->shared->pen);
+ if (pen)
+ hid_input_report(pen, HID_INPUT_REPORT, data,
+ WACOM_PKGLEN_PENABLED, 1);
+ rcu_read_unlock();
data[0] = prefix;
}
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index a8bbba4a6f37..170d6adbe02a 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -286,8 +286,8 @@ struct wacom_shared {
unsigned touch_max;
int type;
struct input_dev __rcu *touch_input;
- struct hid_device *pen;
- struct hid_device *touch;
+ struct hid_device __rcu *pen;
+ struct hid_device __rcu *touch;
bool has_mute_touch_switch;
bool is_touch_on;
};
--
2.54.0.1099.g489fc7bff1-goog
^ permalink raw reply related
* [PATCH v3 3/4] HID: wacom: Redesign shared sibling data lifecycle
From: Lee Jones @ 2026-06-09 12:13 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-1-lee@kernel.org>
The Wacom driver coordinates state between sibling interfaces of
the same physical device using a shared structure 'wacom_shared'
inside 'wacom_hdev_data'. The driver kept a volatile representative
pointer 'data->dev' pointing to a sibling 'hid_device' for physical
path comparisons during sibling matching.
This pointer management is fragile. When the representative device
is disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when
subsequent sibling probes dereference the dangling 'data->dev'
pointer.
Resolve this issue by redesigning the sibling data lifecycle:
- Eliminate the volatile 'data->dev' representative pointer
completely
- Redesign 'wacom_hdev_data' to store stable static copies of the
required attributes upon first allocation: 'phys' path string,
'vendor', 'product' IDs and the sibling's 'device_type'
- Use these static attributes for stable sibling matching in
wacom_are_sibling() and wacom_get_hdev_data()
This ensures sibling matching remains safe and stable even if
individual siblings are dynamically added or removed.
To secure the lifecycle against concurrent probe/disconnect races:
- Switch kref_put() to kref_put_mutex() in
wacom_remove_shared_data() to serialize refcount drops with list
traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is
already held
Also, do not accumulate the 'device_type' capability flag during
subsequent sibling probes. Keeping only the first probed sibling's
device_type exactly preserves the original sibling matching behavior
without introducing side effects.
Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
drivers/hid/wacom_sys.c | 70 +++++++++++++++++++++++++++--------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 9b352027aa98..1b019e3331b4 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,47 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
struct wacom_hdev_data {
struct list_head list;
struct kref kref;
- struct hid_device *dev;
+ char phys[64];
+ __u32 vendor;
+ __u32 product;
+ __u32 device_type;
struct wacom_shared shared;
};
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+ const char *phys_b, char separator)
+{
+ const char *p1 = strrchr(hdev_a->phys, separator);
+ const char *p2 = strrchr(phys_b, separator);
+ int n1, n2;
+
+ if (!p1 || !p2)
+ return false;
+
+ n1 = p1 - hdev_a->phys;
+ n2 = p2 - phys_b;
+
+ if (n1 != n2 || n1 <= 0 || n2 <= 0)
+ return false;
+
+ return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
static LIST_HEAD(wacom_udev_list);
static DEFINE_MUTEX(wacom_udev_list_lock);
static bool wacom_are_sibling(struct hid_device *hdev,
- struct hid_device *sibling)
+ struct wacom_hdev_data *data)
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- struct wacom *sibling_wacom = hid_get_drvdata(sibling);
- struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
__u32 oPid = features->oPid ? features->oPid : hdev->product;
/* The defined oVid/oPid must match that of the sibling */
- if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+ if (features->oVid != HID_ANY_ID && data->vendor != oVid)
return false;
- if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+ if (features->oPid != HID_ANY_ID && data->product != oPid)
return false;
/*
@@ -787,11 +807,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* device path, while those with different VID/PID must share
* the same physical parent device path.
*/
- if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
- if (!hid_compare_device_paths(hdev, sibling, '/'))
+ if (hdev->vendor == data->vendor && hdev->product == data->product) {
+ if (!wacom_compare_device_paths(hdev, data->phys, '/'))
return false;
} else {
- if (!hid_compare_device_paths(hdev, sibling, '.'))
+ if (!wacom_compare_device_paths(hdev, data->phys, '.'))
return false;
}
@@ -804,7 +824,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ !(data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/*
@@ -812,17 +832,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ (data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/* Pen devices may only be siblings of touch devices */
if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+ !(data->device_type & WACOM_DEVICETYPE_TOUCH))
return false;
/* Touch devices may only be siblings of pen devices */
if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+ !(data->device_type & WACOM_DEVICETYPE_PEN))
return false;
/*
@@ -838,7 +858,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Try to find an already-probed interface from the same device */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (hid_compare_device_paths(hdev, data->dev, '/')) {
+ if (wacom_compare_device_paths(hdev, data->phys, '/')) {
kref_get(&data->kref);
return data;
}
@@ -846,7 +866,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Fallback to finding devices that appear to be "siblings" */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (wacom_are_sibling(hdev, data->dev)) {
+ if (wacom_are_sibling(hdev, data)) {
kref_get(&data->kref);
return data;
}
@@ -860,18 +880,15 @@ static void wacom_release_shared_data(struct kref *kref)
struct wacom_hdev_data *data =
container_of(kref, struct wacom_hdev_data, kref);
- mutex_lock(&wacom_udev_list_lock);
list_del(&data->list);
- mutex_unlock(&wacom_udev_list_lock);
-
kfree(data);
}
static void wacom_remove_shared_data(void *res)
{
- struct wacom *wacom = res;
+ struct wacom *res_wacom = res;
struct wacom_hdev_data *data;
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
if (wacom_wac->shared) {
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
@@ -885,17 +902,19 @@ static void wacom_remove_shared_data(void *res)
rcu_dereference_protected(wacom_wac->shared->pen,
lockdep_is_held(&wacom_udev_list_lock));
- if (touch == wacom->hdev) {
+ if (touch == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (pen == wacom->hdev) {
+ } else if (pen == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
synchronize_rcu();
- kref_put(&data->kref, wacom_release_shared_data);
+ if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+ mutex_unlock(&wacom_udev_list_lock);
+
wacom_wac->shared = NULL;
}
}
@@ -918,7 +937,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strscpy(data->phys, hdev->phys, sizeof(data->phys));
+ data->vendor = hdev->vendor;
+ data->product = hdev->product;
+ data->device_type = wacom_wac->features.device_type;
list_add_tail(&data->list, &wacom_udev_list);
}
--
2.54.0.1099.g489fc7bff1-goog
^ permalink raw reply related
* [PATCH v3 4/4] HID: wacom: Fix teardown order in wacom_mode_change_work
From: Lee Jones @ 2026-06-09 12:13 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-1-lee@kernel.org>
wacom_mode_change_work() called wacom_release_resources() before
hid_hw_stop(). wacom_release_resources() triggers devres cleanup
which calls wacom_remove_shared_data() to nullify wacom_wac->shared.
Since hid_hw_stop() was not called yet, hardware IRQs could still fire
and dereference the nullified wacom_wac->shared pointer, causing a crash.
Fix this by correcting the teardown order to call hid_hw_stop()
first, matching the order used in wacom_remove().
Fixes: 4082da80f46a ("HID: wacom: generic: add mode change touch key")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
drivers/hid/wacom_sys.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 1b019e3331b4..378dddbf7ec5 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2835,16 +2835,16 @@ static void wacom_mode_change_work(struct work_struct *work)
if (pen) {
wacom1 = hid_get_drvdata(pen);
- wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
+ wacom_release_resources(wacom1);
wacom1->wacom_wac.has_mode_change = true;
wacom1->wacom_wac.is_direct_mode = is_direct;
}
if (touch) {
wacom2 = hid_get_drvdata(touch);
- wacom_release_resources(wacom2);
hid_hw_stop(wacom2->hdev);
+ wacom_release_resources(wacom2);
wacom2->wacom_wac.has_mode_change = true;
wacom2->wacom_wac.is_direct_mode = is_direct;
}
--
2.54.0.1099.g489fc7bff1-goog
^ permalink raw reply related
* Re: [PATCH v3 2/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: sashiko-bot @ 2026-06-09 12:33 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260609121353.3743782-2-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but `wacom_mode_change_work()` misuses `rcu_access_pointer()` to fetch RCU-protected pointers without holding `rcu_read_lock()`, leading to a Use-After-Free.
--
commit 4eb3f98aa9f594548c94852bcf57eb40ae946cd8
Author: Lee Jones <lee@kernel.org>
Subject: HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
This commit resolves a Use-After-Free vulnerability in
wacom_bamboo_pad_pen_event() by introducing RCU protection for the pen and
touch pointers within the wacom_shared structure. It annotates the pointers
with __rcu, wraps lockless read access with rcu_read_lock() and
rcu_dereference(), and updates writers using the appropriate RCU assignment
primitives. It also updates pointer access in wacom_mode_change_work() to use
rcu_access_pointer() to avoid warnings.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 7ba5898265481..9b352027aa980 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2797,16 +2808,19 @@ static void wacom_mode_change_work(struct work_struct *work)
> bool is_direct = wacom->wacom_wac.is_direct_mode;
> int error = 0;
>
> - if (shared->pen) {
> - wacom1 = hid_get_drvdata(shared->pen);
> + struct hid_device *pen = rcu_access_pointer(shared->pen);
> + struct hid_device *touch = rcu_access_pointer(shared->touch);
> +
> + if (pen) {
> + wacom1 = hid_get_drvdata(pen);
> wacom_release_resources(wacom1);
> hid_hw_stop(wacom1->hdev);
[Severity: High]
This isn't a bug introduced by this patch, but does wacom_mode_change_work()
misuse rcu_access_pointer() here by fetching RCU-protected pointers without
holding rcu_read_lock(), which still leaves an exploitable Use-After-Free?
If the sibling device is unbound concurrently, its wacom_remove() triggers
wacom_remove_shared_data(), which nullifies the shared pointer and calls
synchronize_rcu(). Because the worker does not hold rcu_read_lock(),
synchronize_rcu() will not wait for it.
Doesn't this allow the sibling device to be completely freed while the worker
is still actively dereferencing its memory via hid_get_drvdata(),
wacom_release_resources(), and hid_hw_stop()?
Could using rcu_access_pointer() (which is explicitly intended for pointers
that will not be dereferenced) be papering over the underlying race condition
just to silence sparse/lockdep warnings?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609121353.3743782-1-lee@kernel.org?part=2
^ permalink raw reply
* Re: [PATCH v3 1/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: sashiko-bot @ 2026-06-09 12:35 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260609121353.3743782-1-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Lockless TOCTOU Use-After-Free of sibling devices in wacom_mode_change_work
--
commit bb8e808fb26bf4a0ca1a10604a506fb1d60c8173
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
This commit resolves a time-of-check to time-of-use race condition in
wacom_intuos_pad() that could lead to a use-after-free. It introduces
RCU protection for the touch_input pointer, wrapping lockless readers
with rcu_read_lock() and writers with rcu_assign_pointer().
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf1164..7ba5898265481 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> - wacom_wac->shared->touch = NULL;
> - else if (wacom_wac->shared->pen == wacom->hdev)
> - wacom_wac->shared->pen = NULL;
> + scoped_guard(mutex, &wacom_udev_list_lock) {
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->touch = NULL;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> + } else if (wacom_wac->shared->pen == wacom->hdev) {
> + wacom_wac->shared->pen = NULL;
> + }
> + }
> +
> + synchronize_rcu();
[Severity: High]
This is a pre-existing issue, but is it possible for a sibling device to
still be used after this synchronize_rcu() call?
Looking at wacom_mode_change_work(), it reads shared->pen and shared->touch
locklessly:
drivers/hid/wacom_sys.c:wacom_mode_change_work() {
...
if (shared->pen) {
wacom1 = hid_get_drvdata(shared->pen);
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
...
}
If a context switch occurs right after shared->pen is evaluated as true,
could a sibling device be physically disconnected, triggering
wacom_remove_shared_data() to nullify shared->pen and free the object?
Because wacom_mode_change_work() does not hold an rcu_read_lock(), this
synchronize_rcu() wouldn't block it. When the workqueue resumes, could it
pass the now-freed pointer to hid_get_drvdata() and cause a use-after-free?
>
> kref_put(&data->kref, wacom_release_shared_data);
> wacom_wac->shared = NULL;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609121353.3743782-1-lee@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH v3 4/4] HID: wacom: Fix teardown order in wacom_mode_change_work
From: sashiko-bot @ 2026-06-09 12:48 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260609121353.3743782-4-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing synchronization between sibling device removal and `wacom_mode_change_work` leads to Use-After-Free and double `hid_hw_stop()`.
- [Critical] Missing synchronization allows multiple `wacom_mode_change_work` instances to execute concurrently on both siblings, leading to double teardown and state corruption.
- [Critical] Error paths in `wacom_parse_and_register()` after `hid_hw_start()` fail to call `hid_hw_stop()`, leaving hardware IRQs active after releasing resources.
- [High] If `wacom_parse_and_register()` fails for the first sibling device, the second device is permanently left in a stopped and uninitialized state.
- [High] Pending background workqueues and timers are not cancelled before releasing device resources in `wacom_mode_change_work`, violating subsystem teardown guidelines and causing UAF.
- [High] `wacom_remove()` cancels `mode_change_work` after stopping the hardware, creating a race that leaves the device started and permanently leaks HID interfaces.
--
commit 0812538ee6f90ea0ab3dbb1250c3e8460816a993
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix teardown order in wacom_mode_change_work
wacom_mode_change_work() called wacom_release_resources() before
hid_hw_stop(), which could lead to a crash if hardware IRQs fire and
dereference the nullified shared pointer. This commit fixes the teardown
order to call hid_hw_stop() first.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 1b019e3331b45..378dddbf7ec5b 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2835,16 +2835,16 @@ static void wacom_mode_change_work(struct work_struct *work)
>
> if (pen) {
[Severity: Critical]
This is a pre-existing issue, but is it possible for the shared device
pointers to be dereferenced after they are freed?
In wacom_mode_change_work(), the sibling device pointers are fetched:
struct hid_device *pen = rcu_access_pointer(shared->pen);
...
These pointers are accessed without holding an rcu_read_lock or acquiring
a device reference. If a sibling device is concurrently unbound,
wacom_remove() clears the shared pointer and calls synchronize_rcu(), but
it would not wait for this worker. Could this result in a use-after-free
when hid_hw_stop() is called on the stale pointer?
[Severity: Critical]
This isn't a bug introduced by this patch, but does this function have
sufficient synchronization if mode change events arrive concurrently on
both pen and touch interfaces?
Since the code unconditionally operates on both sibling devices without
locking:
wacom1 = hid_get_drvdata(pen);
hid_hw_stop(wacom1->hdev);
wacom_release_resources(wacom1);
If both devices independently schedule their own mode_change_work, could
both workers run simultaneously and race to call wacom_release_resources()
on the exact same pointers, potentially causing a double-free in the
devres core?
> wacom1 = hid_get_drvdata(pen);
> - wacom_release_resources(wacom1);
> hid_hw_stop(wacom1->hdev);
> + wacom_release_resources(wacom1);
[Severity: High]
This is a pre-existing issue, but are we certain all background tasks
are cancelled before releasing resources here?
Unlike wacom_remove(), wacom_mode_change_work() does not cancel pending
tasks like init_work before calling wacom_release_resources(). Since
wacom_release_resources() destroys devres allocations (such as led groups),
could a delayed wacom_init_work() execute concurrently and access these
freed structures, resulting in a use-after-free?
> wacom1->wacom_wac.has_mode_change = true;
> wacom1->wacom_wac.is_direct_mode = is_direct;
> }
>
> if (touch) {
> wacom2 = hid_get_drvdata(touch);
> - wacom_release_resources(wacom2);
> hid_hw_stop(wacom2->hdev);
> + wacom_release_resources(wacom2);
> wacom2->wacom_wac.has_mode_change = true;
> wacom2->wacom_wac.is_direct_mode = is_direct;
> }
[Severity: High]
This isn't a bug introduced by this patch, but what happens to the second
device if re-registration fails for the first one?
Looking at the rest of wacom_mode_change_work():
if (wacom1) {
error = wacom_parse_and_register(wacom1, false);
if (error)
return;
}
if (wacom2) {
...
}
If wacom_parse_and_register() fails for wacom1, the early return bypasses
the initialization for wacom2. Does this permanently leave the touch
interface hardware stopped and its resources released without recovery?
[Severity: Critical]
This is a pre-existing issue, but is there a missing hardware teardown
in the error handling path of wacom_parse_and_register()?
When an error occurs after hid_hw_start() succeeds:
drivers/hid/wacom_sys.c:wacom_parse_and_register() {
...
fail_quirks:
hid_hw_stop(hdev);
fail:
wacom_release_resources(wacom);
return error;
}
The goto fail jumps past hid_hw_stop(hdev). Does this leave the hid
hardware running while its resources are freed, allowing subsequent
hardware irqs to trigger wacom_raw_event() and cause a use-after-free?
[Severity: High]
This isn't a bug introduced by this patch, but does wacom_remove()
cancel the mode_change_work too late?
In wacom_remove(), the cancellation happens after stopping the hardware:
drivers/hid/wacom_sys.c:wacom_remove() {
hid_hw_stop(hdev);
cancel_delayed_work_sync(&wacom->init_work);
...
cancel_work_sync(&wacom->mode_change_work);
}
If mode_change_work is pending and runs after hid_hw_stop(), it will
call wacom_parse_and_register(), which invokes hid_hw_start(hdev).
Once cancel_work_sync() returns, wacom_remove() completes without
re-stopping the hardware. Could this permanently leak the active hid
interfaces?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609121353.3743782-1-lee@kernel.org?part=4
^ permalink raw reply
* Re: [PATCH v3] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: Hans de Goede @ 2026-06-09 13:07 UTC (permalink / raw)
To: 谢致邦 (XIE Zhibang), linux-input,
dmitry.torokhov, bentiss
Cc: dianders, jikos, linux-kernel, sashiko-bot, sashiko-reviews,
superm1
In-Reply-To: <tencent_C2BEF7C0584BFE713DC7B41FFD7007B6D30A@qq.com>
Hi,
On 9-Jun-26 12:10, 谢致邦 (XIE Zhibang) wrote:
> Hi Hans,
>
> Thanks for the review.
>
> The current header-based approach lets i2c-hid-acpi-prp0001.c work
> independently by just including a header for the _DSM helper. Switching
> to EXPORT_SYMBOL_GPL would force prp0001 to depend on i2c-hid-acpi —
> a Kconfig "depends on I2C_HID_ACPI", plus module-level dependency at
> load time — all for sharing a single function. I find that hard to
> accept.
>
> I understand that adding a new header for one prototype is unusual,
> though it contains only a single static inline function and introduces
> no runtime dependency between the two drivers.
>
> Looking back at the full discussion: Benjamin made it clear from the
> start that he doesn't want i2c-hid-of.c handling ACPI _DSM fallback.
> His line is that ACPI devices and OF devices should each go through
> their own drivers without cross-contamination. If you still prefer
> exporting the function from i2c-hid-acpi.c, then prp0001 would have to
> drag i2c-hid-acpi along with it — an independent leaf driver turned
> into something that can't stand alone.
Well it is a special case/version of the ACPI driver so depending
on it seems fine to me.
Anyways if you prefer the inline function in header solution that is
fine with me.
Lets see what bentiss has to say about this.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 3/8] dt-bindings: input: microchip,cap11xx: Cleanup and refine LED constraints
From: Jun Yan @ 2026-06-09 13:56 UTC (permalink / raw)
To: conor
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101, krzk+dt,
linux-input, linux-kernel, robh
In-Reply-To: <20260608-mooned-ambiguous-02493b8d383e@spud>
> On Sat, Jun 06, 2026 at 11:03:56PM +0800, Jun Yan wrote:
> > Add detailed datasheet links for all supported CAP11xx devices.
> >=20
> > Unify LED node pattern to support all chip variants in preparation
> > for CAP1114 support.
> >=20
> > Remove redundant LED properties inherited from common.yaml
> >=20
> > Apply per-chip LED channel limits:
> > - CAP1126: max 2 channels (0-1)
> > - CAP1188: max 8 channels (0-7)
> > - CAP1106, CAP12xx: no LED support
> >=20
> > Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> > ---
> > .../bindings/input/microchip,cap11xx.yaml | 51 ++++++++++++++-----
> > 1 file changed, 37 insertions(+), 14 deletions(-)
> >=20
> > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > index 7ade03f1b32b..99d00f572a2d 100644
> > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > @@ -10,6 +10,15 @@ description: |
> > The Microchip CAP1xxx Family of RightTouchTM multiple-channel capaciti=
> ve
> > touch controllers and LED drivers. The device communication via I2C on=
> ly.
> > =20
> > + For more product information please see the links below:
> > + CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.=
> pdf
> > + CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.=
> pdf
> > + CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.=
> pdf
> > + CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.=
> pdf
> > + CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.=
> pdf
> > + CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.=
> pdf
> > + CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.=
> pdf
> > +
> > maintainers:
> > - Rob Herring <robh@kernel.org>
> > =20
> > @@ -124,25 +133,21 @@ properties:
> > The number of entries must correspond to the number of channels.
> > =20
> > patternProperties:
> > - "^led@[0-7]$":
> > + "^led@[0-9a-f]$":
> > type: object
> > description: CAP11xx LEDs
> > $ref: /schemas/leds/common.yaml#
> > =20
> > properties:
> > reg:
> > - enum: [0, 1, 2, 3, 4, 5, 6, 7]
> > -
> > - label: true
> > -
> > - linux,default-trigger: true
> > -
> > - default-state: true
> > + description: LED channel number
> > + minimum: 0
> > + maximum: 7
> > =20
> > required:
> > - reg
> > =20
> > - additionalProperties: false
> > + unevaluatedProperties: false
>
> > Remove redundant LED properties inherited from common.yaml
>
> But the pattern here suggests they aren't redundant, this is how you say
> "these properties are the only common ones allowed". You need to explain
> why other ones are valid to change this.
>
I checked other LED dt-bindings and related driver code.
This change is not suitable and will be dropped in V2.
> pw-bot: changes-requested
> (a commit message change is required at least)
>
> > =20
> > allOf:
> > - $ref: input.yaml
> > @@ -158,7 +163,20 @@ allOf:
> > - microchip,cap1298
> > then:
> > patternProperties:
> > - "^led@[0-7]$": false
> > + "^led@": false
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - microchip,cap1126
> > + then:
> > + patternProperties:
> > + "^led@":
> > + properties:
> > + reg:
> > + maximum: 1
>
> I think these changes to permitted properties should be a standalone
> patch.
Agreed. I will split these changes into a standalone patch for V2.
> Thanks,
> Conor.
>
> > =20
> > - if:
> > properties:
> > @@ -183,6 +201,9 @@ additionalProperties: false
> > =20
> > examples:
> > - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/leds/common.h>
> > +
> > i2c {
> > #address-cells =3D <1>;
> > #size-cells =3D <0>;
> > @@ -208,19 +229,21 @@ examples:
> > #size-cells =3D <0>;
> > =20
> > led@0 {
> > - label =3D "cap11xx:green:usr0";
> > reg =3D <0>;
> > + function =3D LED_FUNCTION_INDICATOR;
> > + color =3D <LED_COLOR_ID_GREEN>;
> > };
> > =20
> > led@1 {
> > - label =3D "cap11xx:green:usr1";
> > reg =3D <1>;
> > + function =3D LED_FUNCTION_INDICATOR;
> > + color =3D <LED_COLOR_ID_GREEN>;
> > };
> > =20
> > led@2 {
> > - label =3D "cap11xx:green:alive";
> > reg =3D <2>;
> > - linux,default-trigger =3D "heartbeat";
> > + function =3D LED_FUNCTION_INDICATOR;
> > + color =3D <LED_COLOR_ID_GREEN>;
> > };
> > };
> > };
> > --=20
> > 2.54.0
> >=20
^ permalink raw reply
* Re: [PATCH 7/8] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: Jun Yan @ 2026-06-09 14:08 UTC (permalink / raw)
To: conor
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101, krzk+dt,
linux-input, linux-kernel, robh
In-Reply-To: <20260608-broadly-reunion-09fc8eca5e87@spud>
> On Sat, Jun 06, 2026 at 11:04:00PM +0800, Jun Yan wrote:
> > CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
> > and hardware reset support.
> >=20
> > Add the compatible string for CAP1114, include its datasheet URL,
> > and update the maximum count of LED channels and linux,keycodes entries.
> >=20
> > Add description for microchip,input-threshold: CAP1114 only provides eight
> > threshold entries, which does not match its total channel count.
> >=20
> > Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> > ---
> > .../bindings/input/microchip,cap11xx.yaml | 21 +++++++++++++++++--
> > 1 file changed, 19 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > index e307628350c2..08233386a433 100644
> > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > @@ -12,6 +12,7 @@ description: |
> > =20
> > For more product information please see the links below:
> > CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.=
> pdf
> > + CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.=
> pdf
> > CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.=
> pdf
> > CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.=
> pdf
> > CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.=
> pdf
> > @@ -26,6 +27,7 @@ properties:
> > compatible:
> > enum:
> > - microchip,cap1106
> > + - microchip,cap1114
> > - microchip,cap1126
> > - microchip,cap1188
> > - microchip,cap1203
> > @@ -62,7 +64,7 @@ properties:
> > =20
> > linux,keycodes:
> > minItems: 3
> > - maxItems: 8
> > + maxItems: 14
> > description: |
> > Specifies an array of numeric keycode values to
> > be used for the channels. If this property is
> > @@ -122,6 +124,8 @@ properties:
> > is required for a touch to be registered, making the touch sensor =
> less
> > sensitive.
> > The number of entries must correspond to the number of channels.
> > + CAP1114 is an exception where channels 8~14 reuse the eighth entry=
> 's
> > + threshold, so counts differ.
> > =20
> > microchip,calib-sensitivity:
> > $ref: /schemas/types.yaml#/definitions/uint32-array
> > @@ -149,7 +153,7 @@ patternProperties:
> > reg:
> > description: LED channel number
> > minimum: 0
> > - maximum: 7
> > + maximum: 10
> > =20
> > required:
> > - reg
> > @@ -199,6 +203,19 @@ allOf:
> > reg:
> > maximum: 1
> > =20
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - microchip,cap1188
>
> I don't understand this restriction, shouldn't this be
> if: properties: compatible: not: contains: microchip,cap1114
> so that the constraints before your change are retained?
>
Previously, the LED reg property had a default maximum of 7 for CAP1188.
With the addition of CAP1114, the default maximum is now 11.
An if-then constraint is added to limit the LED count for CAP1188.
> Also, you don't add a new constraint for linux,keycodes, so now it is
> permitted to have > 8 entries for existing devices.
>
OK. I will add the relevant constraints in V2.
> pw-bot: changes-requested
>
> Thanks,
> Conor.
>
> > + then:
> > + patternProperties:
> > + "^led@":
> > + properties:
> > + reg:
> > + maximum: 7
> > +
> > - if:
> > properties:
> > compatible:
> > --=20
> > 2.54.0
> >=20
^ permalink raw reply
* Re: [PATCH 7/8] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: Conor Dooley @ 2026-06-09 14:31 UTC (permalink / raw)
To: Jun Yan
Cc: conor+dt, devicetree, dmitry.torokhov, krzk+dt, linux-input,
linux-kernel, robh
In-Reply-To: <20260609140827.850117-1-jerrysteve1101@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3766 bytes --]
On Tue, Jun 09, 2026 at 10:08:27PM +0800, Jun Yan wrote:
> > On Sat, Jun 06, 2026 at 11:04:00PM +0800, Jun Yan wrote:
> > > CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
> > > and hardware reset support.
> > >=20
> > > Add the compatible string for CAP1114, include its datasheet URL,
> > > and update the maximum count of LED channels and linux,keycodes entries.
> > >=20
> > > Add description for microchip,input-threshold: CAP1114 only provides eight
> > > threshold entries, which does not match its total channel count.
> > >=20
> > > Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> > > ---
> > > .../bindings/input/microchip,cap11xx.yaml | 21 +++++++++++++++++--
> > > 1 file changed, 19 insertions(+), 2 deletions(-)
> > >=20
> > > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> > ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > > index e307628350c2..08233386a433 100644
> > > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > > @@ -12,6 +12,7 @@ description: |
> > > =20
> > > For more product information please see the links below:
> > > CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.=
> > pdf
> > > + CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.=
> > pdf
> > > CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.=
> > pdf
> > > CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.=
> > pdf
> > > CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.=
> > pdf
> > > @@ -26,6 +27,7 @@ properties:
> > > compatible:
> > > enum:
> > > - microchip,cap1106
> > > + - microchip,cap1114
> > > - microchip,cap1126
> > > - microchip,cap1188
> > > - microchip,cap1203
> > > @@ -62,7 +64,7 @@ properties:
> > > =20
> > > linux,keycodes:
> > > minItems: 3
> > > - maxItems: 8
> > > + maxItems: 14
> > > description: |
> > > Specifies an array of numeric keycode values to
> > > be used for the channels. If this property is
> > > @@ -122,6 +124,8 @@ properties:
> > > is required for a touch to be registered, making the touch sensor =
> > less
> > > sensitive.
> > > The number of entries must correspond to the number of channels.
> > > + CAP1114 is an exception where channels 8~14 reuse the eighth entry=
> > 's
> > > + threshold, so counts differ.
> > > =20
> > > microchip,calib-sensitivity:
> > > $ref: /schemas/types.yaml#/definitions/uint32-array
> > > @@ -149,7 +153,7 @@ patternProperties:
> > > reg:
> > > description: LED channel number
> > > minimum: 0
> > > - maximum: 7
> > > + maximum: 10
> > > =20
> > > required:
> > > - reg
> > > @@ -199,6 +203,19 @@ allOf:
> > > reg:
> > > maximum: 1
> > > =20
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + enum:
> > > + - microchip,cap1188
> >
> > I don't understand this restriction, shouldn't this be
> > if: properties: compatible: not: contains: microchip,cap1114
> > so that the constraints before your change are retained?
> >
>
> Previously, the LED reg property had a default maximum of 7 for CAP1188.
> With the addition of CAP1114, the default maximum is now 11.
> An if-then constraint is added to limit the LED count for CAP1188.
Ah, is this because cap1188 is the only existing device in the binding
that actually has LEDs?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v1] Input: Drop unused assignments from pnp_device_id arrays
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-09 14:53 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, Werner Sembach, Christoffer Sandberg, feng, gongqi,
linux-input, linux-kernel
Explicitly assigning .driver_data in drivers that don't use this member
is silly and a bit irritating. Drop these. Also simplify the list
terminator entry to be just empty to match what most other device_id
tables do.
There is no changed semantic, not even a change in the compiled result.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
drivers/input/gameport/ns558.c | 46 +++++++++++-----------
drivers/input/serio/i8042-acpipnpio.h | 56 +++++++++++++--------------
2 files changed, 51 insertions(+), 51 deletions(-)
diff --git a/drivers/input/gameport/ns558.c b/drivers/input/gameport/ns558.c
index fdece6ec1df3..f70a96c4f1fd 100644
--- a/drivers/input/gameport/ns558.c
+++ b/drivers/input/gameport/ns558.c
@@ -148,29 +148,29 @@ static int ns558_isa_probe(int io)
#ifdef CONFIG_PNP
static const struct pnp_device_id pnp_devids[] = {
- { .id = "@P@0001", .driver_data = 0 }, /* ALS 100 */
- { .id = "@P@0020", .driver_data = 0 }, /* ALS 200 */
- { .id = "@P@1001", .driver_data = 0 }, /* ALS 100+ */
- { .id = "@P@2001", .driver_data = 0 }, /* ALS 120 */
- { .id = "ASB16fd", .driver_data = 0 }, /* AdLib NSC16 */
- { .id = "AZT3001", .driver_data = 0 }, /* AZT1008 */
- { .id = "CDC0001", .driver_data = 0 }, /* Opl3-SAx */
- { .id = "CSC0001", .driver_data = 0 }, /* CS4232 */
- { .id = "CSC000f", .driver_data = 0 }, /* CS4236 */
- { .id = "CSC0101", .driver_data = 0 }, /* CS4327 */
- { .id = "CTL7001", .driver_data = 0 }, /* SB16 */
- { .id = "CTL7002", .driver_data = 0 }, /* AWE64 */
- { .id = "CTL7005", .driver_data = 0 }, /* Vibra16 */
- { .id = "ENS2020", .driver_data = 0 }, /* SoundscapeVIVO */
- { .id = "ESS0001", .driver_data = 0 }, /* ES1869 */
- { .id = "ESS0005", .driver_data = 0 }, /* ES1878 */
- { .id = "ESS6880", .driver_data = 0 }, /* ES688 */
- { .id = "IBM0012", .driver_data = 0 }, /* CS4232 */
- { .id = "OPT0001", .driver_data = 0 }, /* OPTi Audio16 */
- { .id = "YMH0006", .driver_data = 0 }, /* Opl3-SA */
- { .id = "YMH0022", .driver_data = 0 }, /* Opl3-SAx */
- { .id = "PNPb02f", .driver_data = 0 }, /* Generic */
- { .id = "", },
+ { .id = "@P@0001" }, /* ALS 100 */
+ { .id = "@P@0020" }, /* ALS 200 */
+ { .id = "@P@1001" }, /* ALS 100+ */
+ { .id = "@P@2001" }, /* ALS 120 */
+ { .id = "ASB16fd" }, /* AdLib NSC16 */
+ { .id = "AZT3001" }, /* AZT1008 */
+ { .id = "CDC0001" }, /* Opl3-SAx */
+ { .id = "CSC0001" }, /* CS4232 */
+ { .id = "CSC000f" }, /* CS4236 */
+ { .id = "CSC0101" }, /* CS4327 */
+ { .id = "CTL7001" }, /* SB16 */
+ { .id = "CTL7002" }, /* AWE64 */
+ { .id = "CTL7005" }, /* Vibra16 */
+ { .id = "ENS2020" }, /* SoundscapeVIVO */
+ { .id = "ESS0001" }, /* ES1869 */
+ { .id = "ESS0005" }, /* ES1878 */
+ { .id = "ESS6880" }, /* ES688 */
+ { .id = "IBM0012" }, /* CS4232 */
+ { .id = "OPT0001" }, /* OPTi Audio16 */
+ { .id = "YMH0006" }, /* Opl3-SA */
+ { .id = "YMH0022" }, /* Opl3-SAx */
+ { .id = "PNPb02f" }, /* Generic */
+ { }
};
MODULE_DEVICE_TABLE(pnp, pnp_devids);
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 8ebdf4fb9030..412f82d7a303 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1539,22 +1539,22 @@ static int i8042_pnp_aux_probe(struct pnp_dev *dev, const struct pnp_device_id *
}
static const struct pnp_device_id pnp_kbd_devids[] = {
- { .id = "PNP0300", .driver_data = 0 },
- { .id = "PNP0301", .driver_data = 0 },
- { .id = "PNP0302", .driver_data = 0 },
- { .id = "PNP0303", .driver_data = 0 },
- { .id = "PNP0304", .driver_data = 0 },
- { .id = "PNP0305", .driver_data = 0 },
- { .id = "PNP0306", .driver_data = 0 },
- { .id = "PNP0309", .driver_data = 0 },
- { .id = "PNP030a", .driver_data = 0 },
- { .id = "PNP030b", .driver_data = 0 },
- { .id = "PNP0320", .driver_data = 0 },
- { .id = "PNP0343", .driver_data = 0 },
- { .id = "PNP0344", .driver_data = 0 },
- { .id = "PNP0345", .driver_data = 0 },
- { .id = "CPQA0D7", .driver_data = 0 },
- { .id = "", },
+ { .id = "PNP0300" },
+ { .id = "PNP0301" },
+ { .id = "PNP0302" },
+ { .id = "PNP0303" },
+ { .id = "PNP0304" },
+ { .id = "PNP0305" },
+ { .id = "PNP0306" },
+ { .id = "PNP0309" },
+ { .id = "PNP030a" },
+ { .id = "PNP030b" },
+ { .id = "PNP0320" },
+ { .id = "PNP0343" },
+ { .id = "PNP0344" },
+ { .id = "PNP0345" },
+ { .id = "CPQA0D7" },
+ { }
};
MODULE_DEVICE_TABLE(pnp, pnp_kbd_devids);
@@ -1569,18 +1569,18 @@ static struct pnp_driver i8042_pnp_kbd_driver = {
};
static const struct pnp_device_id pnp_aux_devids[] = {
- { .id = "AUI0200", .driver_data = 0 },
- { .id = "FJC6000", .driver_data = 0 },
- { .id = "FJC6001", .driver_data = 0 },
- { .id = "PNP0f03", .driver_data = 0 },
- { .id = "PNP0f0b", .driver_data = 0 },
- { .id = "PNP0f0e", .driver_data = 0 },
- { .id = "PNP0f12", .driver_data = 0 },
- { .id = "PNP0f13", .driver_data = 0 },
- { .id = "PNP0f19", .driver_data = 0 },
- { .id = "PNP0f1c", .driver_data = 0 },
- { .id = "SYN0801", .driver_data = 0 },
- { .id = "", },
+ { .id = "AUI0200" },
+ { .id = "FJC6000" },
+ { .id = "FJC6001" },
+ { .id = "PNP0f03" },
+ { .id = "PNP0f0b" },
+ { .id = "PNP0f0e" },
+ { .id = "PNP0f12" },
+ { .id = "PNP0f13" },
+ { .id = "PNP0f19" },
+ { .id = "PNP0f1c" },
+ { .id = "SYN0801" },
+ { },
};
MODULE_DEVICE_TABLE(pnp, pnp_aux_devids);
base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72
--
2.47.3
^ permalink raw reply related
* [PATCH] HID: pidff: Use correct effect type in effect update
From: Oleg Makarenko @ 2026-06-09 16:00 UTC (permalink / raw)
To: jikos, bentiss
Cc: Oleg Makarenko, stable, Oliver Roundtree, Ryno Kotzé,
Tomasz Pakuła, Anssi Hannula, Dmitry Torokhov, linux-input,
linux-usb, linux-kernel
When updating an existing effect, the effect type from the last created
effect was sent to the device instead of the updated one.
This caused incorrect reports when a game creates multiple different
effects and updates only one that is not the last created.
Fixes FFB in multiple games that create multiple simultaneous effects
(Forza Horizon 5/6).
Fixes: 224ee88fe395 ("Input: add force feedback driver for PID devices")
Cc: <stable@vger.kernel.org>
Tested-by: Oliver Roundtree <oroundtree1@gmail.com>
Co-developed-by: Ryno Kotzé <lemon.xah@gmail.com>
Signed-off-by: Ryno Kotzé <lemon.xah@gmail.com>
Signed-off-by: Oleg Makarenko <oleg@makarenk.ooo>
---
drivers/hid/usbhid/hid-pidff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index c45f182d0448..5f4395f7c645 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -522,7 +522,7 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
pidff->set_effect[PID_EFFECT_BLOCK_INDEX].value[0] =
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
pidff->set_effect_type->value[0] =
- pidff->create_new_effect_type->value[0];
+ pidff_get_effect_type_id(pidff, effect);
pidff_set_duration(&pidff->set_effect[PID_DURATION],
effect->replay.length);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] HID: pidff: Use correct effect type in effect update
From: sashiko-bot @ 2026-06-09 16:21 UTC (permalink / raw)
To: Oleg Makarenko; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260609160031.493353-1-oleg@makarenk.ooo>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Pre-existing out-of-bounds read in `pidff_find_effects` leading to potential kernel panic (local DoS via crafted USB device).
--
commit 07c3b678f1bae4a1c16dfff6971345dc4b180f89
Author: Oleg Makarenko <oleg@makarenk.ooo>
HID: pidff: Use correct effect type in effect update
This commit fixes an issue where the effect type from the last created effect
was incorrectly used when updating existing effects. It ensures the correct
effect type ID is sent to the device, fixing force feedback in games that
manage multiple simultaneous effects.
> diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
> index c45f182d04480..5f4395f7c6458 100644
> --- a/drivers/hid/usbhid/hid-pidff.c
> +++ b/drivers/hid/usbhid/hid-pidff.c
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but looking at pidff_find_effects()
in this same file:
drivers/hid/usbhid/hid-pidff.c:pidff_find_effects() {
...
for (i = 0; i < ARRAY_SIZE(pidff_effect_types); i++) {
int pidff_type = pidff->type_id[i];
if (pidff->set_effect_type->usage[pidff_type].hid !=
pidff->create_new_effect_type->usage[pidff_type].hid) {
hid_err(pidff->hid,
"effect type number %d is invalid\n", i);
return -1;
}
}
...
}
Could this code cause an out-of-bounds read if a malicious USB device is
plugged in?
The type_id array is populated with 1-based logical indices, but pidff_type is
used directly as a 0-based index to access the usage arrays.
If the device descriptor defines many usages in create_new_effect_type but
very few in set_effect_type, would pidff_type exceed
set_effect_type->maxusage and read past the end of the array?
Does this risk a kernel panic if the read crosses page boundaries during
device probe?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609160031.493353-1-oleg@makarenk.ooo?part=1
^ permalink raw reply
* [PATCHv2] input: ipaq-micro-keys: simplify allocation
From: Rosen Penev @ 2026-06-09 21:35 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, open list
Embed the keycode array in the struct to have a single allocation.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
v2: use regular array instead of flexible array member.
drivers/input/keyboard/ipaq-micro-keys.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
index ca7ec054b1ce..695ef3c2081a 100644
--- a/drivers/input/keyboard/ipaq-micro-keys.c
+++ b/drivers/input/keyboard/ipaq-micro-keys.c
@@ -20,12 +20,6 @@
#include <linux/platform_device.h>
#include <linux/mfd/ipaq-micro.h>
-struct ipaq_micro_keys {
- struct ipaq_micro *micro;
- struct input_dev *input;
- u16 *codes;
-};
-
static const u16 micro_keycodes[] = {
KEY_RECORD, /* 1: Record button */
KEY_CALENDAR, /* 2: Calendar */
@@ -38,6 +32,12 @@ static const u16 micro_keycodes[] = {
KEY_DOWN, /* 9: Down */
};
+struct ipaq_micro_keys {
+ struct ipaq_micro *micro;
+ struct input_dev *input;
+ u16 codes[ARRAY_SIZE(micro_keycodes)];
+};
+
static void micro_key_receive(void *data, int len, unsigned char *msg)
{
struct ipaq_micro_keys *keys = data;
@@ -102,10 +102,7 @@ static int micro_key_probe(struct platform_device *pdev)
keys->input->keycodesize = sizeof(micro_keycodes[0]);
keys->input->keycodemax = ARRAY_SIZE(micro_keycodes);
- keys->codes = devm_kmemdup_array(&pdev->dev, micro_keycodes, keys->input->keycodemax,
- keys->input->keycodesize, GFP_KERNEL);
- if (!keys->codes)
- return -ENOMEM;
+ memcpy(keys->codes, micro_keycodes, sizeof(keys->codes));
keys->input->keycode = keys->codes;
--
2.54.0
^ permalink raw reply related
* Re: [PATCHv2] input: ipaq-micro-keys: simplify allocation
From: sashiko-bot @ 2026-06-09 21:44 UTC (permalink / raw)
To: Rosen Penev; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260609213532.25181-1-rosenp@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Potential deadlock in `micro_key_start()` and `micro_key_stop()` due to taking a spinlock without disabling interrupts, which is also taken in a hardirq handler.
--
commit 59a420f5a26f394b8f84d4e056f388e517d6f808
Author: Rosen Penev <rosenp@gmail.com>
input: ipaq-micro-keys: simplify allocation
This commit simplifies the keycode array allocation in the ipaq-micro-keys
driver. By embedding the array directly into the device structure, it removes
the need for a separate devm_kmemdup_array() call during probe.
> diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
> index ca7ec054b1cef..695ef3c2081a4 100644
> --- a/drivers/input/keyboard/ipaq-micro-keys.c
> +++ b/drivers/input/keyboard/ipaq-micro-keys.c
[ ... ]
> @@ -102,10 +102,7 @@ static int micro_key_probe(struct platform_device *pdev)
>
> keys->input->keycodesize = sizeof(micro_keycodes[0]);
> keys->input->keycodemax = ARRAY_SIZE(micro_keycodes);
> - keys->codes = devm_kmemdup_array(&pdev->dev, micro_keycodes, keys->input->keycodemax,
> - keys->input->keycodesize, GFP_KERNEL);
> - if (!keys->codes)
> - return -ENOMEM;
> + memcpy(keys->codes, micro_keycodes, sizeof(keys->codes));
>
> keys->input->keycode = keys->codes;
[Severity: High]
This isn't a bug introduced by this patch, but could the lock usage in
micro_key_start() and micro_key_stop() cause a deadlock if an interrupt
fires?
Looking at drivers/input/keyboard/ipaq-micro-keys.c, the micro->lock is
acquired in process context without disabling interrupts:
micro_key_start() {
guard(spinlock)(&keys->micro->lock);
...
}
If a serial interrupt fires on the same CPU while the process context holds
the lock, the interrupt handler micro_serial_isr() runs, processes the
character, and calls micro_rx_msg().
micro_rx_msg() in drivers/mfd/ipaq-micro.c then attempts to acquire the same
lock in hardirq context:
spin_lock(µ->lock);
Does this sequence lead to a lock inversion deadlock? Should the process
context functions use guard(spinlock_irqsave) or guard(spinlock_irq)
instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609213532.25181-1-rosenp@gmail.com?part=1
^ permalink raw reply
* Re: (subset) [PATCH v2 00/10] gpiolib: fence off legacy interfaces
From: Kevin Hilman @ 2026-06-09 22:25 UTC (permalink / raw)
To: linux-gpio, Arnd Bergmann
Cc: linux-kernel, Arnd Bergmann, Christian Lamparter, Johannes Berg,
Aaro Koskinen, Andreas Kemnade, Roger Quadros, Tony Lindgren,
Thomas Bogendoerfer, John Paul Adrian Glaubitz, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, Lee Jones,
Pavel Machek, Matti Vaittinen, Florian Fainelli, Jonas Gorski,
Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-wireless, linux-omap,
linux-arm-kernel, linux-mips, linux-sh, linux-input, linux-leds,
netdev
In-Reply-To: <20260520183815.2510387-1-arnd@kernel.org>
On Wed, 20 May 2026 20:38:05 +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> This is an update of all the patches that are still required before
> we can actually turn off CONFIG_GPIOLIB_LEGACY for most platforms
> in the final patch of this series.
>
> I originally posted this as a series in
> https://lore.kernel.org/all/20250808151822.536879-1-arnd@kernel.org/
>
> [...]
Applied, thanks!
[09/10] ARM: dts: omap2: add stlc4560 spi-wireless node
commit: c5a0ac76b364bbd1d4fb7e440edabcd2a369343c
Best regards,
--
Kevin Hilman (TI) <khilman@baylibre.com>
^ permalink raw reply
* Re: [PATCH 1/2] HID: lenovo: Add support for ThinkPad X13 Folio keyboard
From: Vishnu Sankar @ 2026-06-09 23:05 UTC (permalink / raw)
To: jikos, bentiss, derekjohn.clark, mpearson-lenovo
Cc: vsankar, linux-input, linux-kernel
In-Reply-To: <20260522050632.8079-1-vishnuocv@gmail.com>
Hi Jiri / Benjamin,
Gently pinging on this series.
Is there anything blocking acceptance or any changes needed from my side?
The series has been tested on physical hardware.
Thank you!
Vishnu
On Fri, May 22, 2026 at 2:07 PM Vishnu Sankar <vishnuocv@gmail.com> wrote:
>
> From: Vishnu Sankar <vishnuocv@gmail.com>
>
> Add USB ID support for the ThinkPad X13 detachable keyboard.
> The Keyboard uses the same HID raw event protocol as the ThinkPad
> X12 Gen 2. The functionality stays the same with X12 Gen 2 Keyboards.
>
> Also declare KEY_PERFORMANCE capability in lenovo_input_configured()
> for X13 detachable, allowing userspace to discover the key via evdev
> capability bits.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-lenovo.c | 21 +++++++++++++++++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 4657d96fb083..064be2333a00 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -874,6 +874,7 @@
> #define USB_DEVICE_ID_LENOVO_X1_TAB3 0x60b5
> #define USB_DEVICE_ID_LENOVO_X12_TAB 0x60fe
> #define USB_DEVICE_ID_LENOVO_X12_TAB2 0x61ae
> +#define USB_DEVICE_ID_LENOVO_X13_TAB 0x62af
> #define USB_DEVICE_ID_LENOVO_YOGABOOK9I 0x6161
> #define USB_DEVICE_ID_LENOVO_OPTICAL_USB_MOUSE_600E 0x600e
> #define USB_DEVICE_ID_LENOVO_PIXART_USB_MOUSE_608D 0x608d
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index a6b73e03c16b..35b00619f045 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -504,6 +504,7 @@ static int lenovo_input_mapping(struct hid_device *hdev,
> usage, bit, max);
> case USB_DEVICE_ID_LENOVO_X12_TAB:
> case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB2:
> case USB_DEVICE_ID_LENOVO_X1_TAB3:
> @@ -620,6 +621,7 @@ static ssize_t attr_fn_lock_store(struct device *dev,
> break;
> case USB_DEVICE_ID_LENOVO_X12_TAB:
> case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> case USB_DEVICE_ID_LENOVO_X1_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB2:
> @@ -791,9 +793,10 @@ static int lenovo_raw_event(struct hid_device *hdev,
> * Lenovo TP X12 Tab KBD's Fn+XX is HID raw data defined. Report ID is 0x03
> * e.g.: Raw data received for MIC mute is 0x00020003.
> */
> - if (unlikely((hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB
> - || hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2)
> - && size >= 3 && report->id == 0x03))
> + if (unlikely((hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB ||
> + hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2 ||
> + hdev->product == USB_DEVICE_ID_LENOVO_X13_TAB) &&
> + size >= 3 && report->id == 0x03))
> return lenovo_raw_event_TP_X12_tab(hdev, le32_to_cpu(*(__le32 *)data));
>
> return 0;
> @@ -877,6 +880,7 @@ static int lenovo_event(struct hid_device *hdev, struct hid_field *field,
> return lenovo_event_cptkbd(hdev, field, usage, value);
> case USB_DEVICE_ID_LENOVO_X12_TAB:
> case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> case USB_DEVICE_ID_LENOVO_X1_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB2:
> @@ -1161,6 +1165,7 @@ static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
> break;
> case USB_DEVICE_ID_LENOVO_X12_TAB:
> case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> case USB_DEVICE_ID_LENOVO_X1_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB2:
> @@ -1355,7 +1360,8 @@ static int lenovo_probe_tp10ubkbd(struct hid_device *hdev)
> */
>
> data->fn_lock = !(hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB ||
> - hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2);
> + hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2 ||
> + hdev->product == USB_DEVICE_ID_LENOVO_X13_TAB);
>
> lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED, data->fn_lock);
>
> @@ -1402,6 +1408,7 @@ static int lenovo_probe(struct hid_device *hdev,
> break;
> case USB_DEVICE_ID_LENOVO_X12_TAB:
> case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> case USB_DEVICE_ID_LENOVO_X1_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB2:
> @@ -1490,6 +1497,7 @@ static void lenovo_remove(struct hid_device *hdev)
> break;
> case USB_DEVICE_ID_LENOVO_X12_TAB:
> case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> case USB_DEVICE_ID_LENOVO_X1_TAB:
> case USB_DEVICE_ID_LENOVO_X1_TAB2:
> @@ -1517,6 +1525,9 @@ static int lenovo_input_configured(struct hid_device *hdev,
> hi->input->propbit);
> }
> break;
> + case USB_DEVICE_ID_LENOVO_X13_TAB:
> + input_set_capability(hi->input, EV_KEY, KEY_PERFORMANCE);
> + break;
> }
>
> return 0;
> @@ -1551,6 +1562,8 @@ static const struct hid_device_id lenovo_devices[] = {
> USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X12_TAB) },
> { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X12_TAB2) },
> + { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> + USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X13_TAB) },
> { HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC,
> USB_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD) },
> { }
> --
> 2.54.0
>
--
Regards,
Vishnu Sankar
^ permalink raw reply
* [PATCH] HID: add quirk for IBM SK-8845 keyboard
From: TheDragonWhoAte @ 2026-06-10 4:09 UTC (permalink / raw)
To: linux-input@vger.kernel.org
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 792 bytes --]
HID: add quirk for IBM SK-8845 keyboard
The TrackPoint on the IBM SK-8845 (Synaptics 06cb:0009) sends corrupted
input events when bound to the synaptics_usb driver. Moving the stick
generates random BTN_LEFT/BTN_RIGHT/BTN_MIDDLE events instead of
REL_X/REL_Y movement.
Force the device to bind to hid-generic instead by setting
HID_QUIRK_IGNORE_SPECIAL_DRIVER.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221624
Note: This quirk disables two-finger scrolling on the touchpad. That's a trade-off, but without it the TrackPoint is completely unusable. The real fix would be to make synaptics_usb handle this device properly, so both the touchpad gestures and the TrackPoint work. That's beyond this patch.
Signed-off-by: TheDragonWhoAte <TheDragonWhoAte@protonmail.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-HID-add-quirk-for-IBM-SK-8845-keyboard.patch --]
[-- Type: text/x-patch; name=0001-HID-add-quirk-for-IBM-SK-8845-keyboard.patch, Size: 2037 bytes --]
From 0bc38d7fac65dc07f48e074130329924adf5744b Mon Sep 17 00:00:00 2001
From: thedragonwhoate <thedragonwhoate@protonmail.com>
Date: Wed, 10 Jun 2026 06:38:03 +0300
Subject: [PATCH] HID: add quirk for IBM SK-8845 keyboard
To: linux-input@vger.kernel.org
The TrackPoint on the IBM SK-8845 (Synaptics 06cb:0009) sends corrupted
input events when bound to the synaptics_usb driver. Moving the stick
generates random BTN_LEFT/BTN_RIGHT/BTN_MIDDLE events instead of
REL_X/REL_Y movement.
Force the device to bind to hid-generic instead by setting
HID_QUIRK_IGNORE_SPECIAL_DRIVER.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221624
Note: This quirk disables two-finger scrolling on the touchpad. That's a trade-off, but without it the TrackPoint is completely unusable. The real fix would be to make synaptics_usb handle this device properly, so both the touchpad gestures and the TrackPoint work. That's beyond this patch.
Signed-off-by: thedragonwhoate <thedragonwhoate@protonmail.com>
---
drivers/hid/hid-quirks.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 57d8efdd9..c8394b896 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -203,6 +203,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_TP_V103), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DELL_K12A), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DELL_K15A), HID_QUIRK_NO_INIT_REPORTS },
+ { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_COMP_TP), HID_QUIRK_IGNORE_SPECIAL_DRIVER },
{ HID_USB_DEVICE(USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD), HID_QUIRK_BADPAD },
{ HID_USB_DEVICE(USB_VENDOR_ID_TOUCHPACK, USB_DEVICE_ID_TOUCHPACK_RTS), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN_8882), HID_QUIRK_NOGET },
--
2.54.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