* Re: [PATCH 7/8] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: Jun Yan @ 2026-06-08 13:27 UTC (permalink / raw)
To: sashiko-bot
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101,
linux-input, robh, sashiko-reviews
In-Reply-To: <20260606151658.60E2C1F00893@smtp.kernel.org>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(=
> s) to consider:
> - [Medium] The new `microchip,cap1114` compatible string was omitted from t=
> he `if-then` conditional block that restricts `microchip,signal-guard` and =
> `microchip,calib-sensitivity` for prior CAP11xx generations.
> --
>
> commit 5c58a6baee363da7434a9782efa913c250015130
> Author: Jun Yan <jerrysteve1101@gmail.com>
>
> dt-bindings: input: microchip,cap11xx: Add CAP1114 support
>
> This commit adds device tree binding support for the CAP1114 capacitive
> touch sensor. It updates the compatible string list, maximum LED channel
> counts, and keycode entries.
>
> > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > index e307628350c24..08233386a4336 100644
> > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> [ ... ]
> > @@ -26,6 +27,7 @@ properties:
> > compatible:
> > enum:
> > - microchip,cap1106
> > + - microchip,cap1114
> > - microchip,cap1126
> > - microchip,cap1188
> > - microchip,cap1203
>
> [Severity: Medium]
> Since microchip,cap1114 is being added here, could this validation
> regression be avoided by also appending it to the if-then conditional
> block further down the schema that restricts microchip,signal-guard and
> microchip,calib-sensitivity?
>
Good catch. CAP1114 doesn't support these features, so I'll add it to the if-then block in v2.
>
> Documentation/devicetree/bindings/input/microchip,cap11xx.yaml:
> - if:
> properties:
> compatible:
> contains:
> enum:
> - microchip,cap1106
> - microchip,cap1126
> - microchip,cap1188
> - microchip,cap1203
> - microchip,cap1206
> then:
> properties:
> microchip,signal-guard: false
> microchip,calib-sensitivity: false
>
> Since the maxItems for these properties remains at 8 (and CAP1114 has 14
> channels), it appears CAP1114 does not support them either. Omitting the
> new compatible string from this block allows device trees with these
> unsupported properties to pass schema validation.
>
> --=20
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260606150458.2506=
> 06-1-jerrysteve1101@gmail.com?part=3D7
^ permalink raw reply
* Re: [PATCH 5/8] Input: cap11xx - add reset gpio support
From: Jun Yan @ 2026-06-08 13:20 UTC (permalink / raw)
To: sashiko-bot
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101,
linux-input, robh, sashiko-reviews
In-Reply-To: <20260606151346.8C5861F00893@smtp.kernel.org>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(=
> s) to consider:
> - [Low] Inefficient delay using usleep_range() for a large duration (400-50=
> 0ms).
> - [Low] Missing explicit inclusion of <linux/delay.h>.
> --
>
> commit 83de47341df3454fdb8216dc0a95948c3f158a40
> Author: Jun Yan <jerrysteve1101@gmail.com>
>
> Input: cap11xx - add reset gpio support
>
> This commit adds hardware reset support for CAP11xx devices by toggling a
> dedicated reset GPIO during the i2c probe phase. It incorporates specific
> timing requirements for the reset filter and active reset duration.
>
> > diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/ca=
> p11xx.c
> > index 686174722204c..7015aa7654ce7 100644
> > --- a/drivers/input/keyboard/cap11xx.c
> > +++ b/drivers/input/keyboard/cap11xx.c
>
> [ ... ]
>
> > @@ -452,6 +457,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_=
> client)
> > if (IS_ERR(priv->regmap))
> > return PTR_ERR(priv->regmap);
> > =20
> > + priv->reset_gpio =3D devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LO=
> W);
> > + if (IS_ERR(priv->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
> > + "Failed to get 'reset' GPIO\n");
> > + if (priv->reset_gpio) {
> > + gpiod_set_value_cansleep(priv->reset_gpio, 1);
> > + usleep_range(CAP11XX_T_RST_FILT_MIN, CAP11XX_T_RST_FILT_MIN * 2);
> > + gpiod_set_value_cansleep(priv->reset_gpio, 0);
> > + usleep_range(CAP11XX_T_RST_ON_MIN, CAP11XX_T_RST_ON_MAX);
>
> [Severity: Low]
> Does this regression introduce unnecessary overhead by using usleep_range()
> for a 400-500ms delay in cap11xx_i2c_probe()?
>
> According to kernel timer guidelines, usleep_range() is designed for short
> delays (10us to 20ms) and sets up high-resolution timers. For delays longer
> than 20ms, using msleep() or fsleep() avoids unnecessary overhead and
> expensive CPU wakeups.
>
Agreed. Will replace usleep_range() with msleep() in V2.
>
> [Severity: Low]
> Could this regression in header dependencies be avoided by explicitly
> including <linux/delay.h> since we are now calling usleep_range() here?
>
> While the code might compile due to implicit inclusion via other headers,
> relying on implicit includes can sometimes lead to build failures on other
> architectures or if intermediate headers are refactored in the future.
>
OK. Will add #include <linux/delay.h> in v2,
> > + }
> > +
> > error =3D regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
> > if (error)
> > return dev_err_probe(dev, error, "Failed to read product ID\n");
>
> --=20
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260606150458.2506=
> 06-1-jerrysteve1101@gmail.com?part=3D5
^ 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-08 11:51 UTC (permalink / raw)
To: 谢致邦 (XIE Zhibang), linux-input,
dmitry.torokhov, bentiss
Cc: dianders, jikos, linux-kernel, sashiko-bot, sashiko-reviews,
superm1
In-Reply-To: <52c113ef-e81e-476a-a62c-3cd09e7fdcbf@kernel.org>
Hi,
On 8-Jun-26 1:48 PM, Hans de Goede wrote:
> Hi,
>
> On 6-Jun-26 6:06 PM, 谢致邦 (XIE Zhibang) wrote:
>> 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>
>
> Thank you for the new version.
>
>> ---
>> 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().
>>
>> 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..b54370a938ab
>> --- /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(250);
>> + 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;
>> -}
>> -
>
> Instead of moving this to i2c-hid-acpi.h, please make it non static an
> export it using EXPORT_SYMBOL_GPL() and only put a prototype in i2c-hid-acpi.h .
I just noticed i2c-hid-acpi.h is a whole new file. No need to add that just for
the prototype (I think / IMHO).
So for the next version just export the function from i2c-hid-acpi.c and
add a prototype declaration directly in i2c-hid-acpi-prp0001.c
Regards,
Hans
>> 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
>
>
>
>
^ 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-08 11:48 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_E0B44D3DB1D67A7F20CF82ED6D3F0881FE08@qq.com>
Hi,
On 6-Jun-26 6:06 PM, 谢致邦 (XIE Zhibang) wrote:
> 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>
Thank you for the new version.
> ---
> 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().
>
> 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..b54370a938ab
> --- /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(250);
> + 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;
> -}
> -
Instead of moving this to i2c-hid-acpi.h, please make it non static an
export it using EXPORT_SYMBOL_GPL() and only put a prototype in i2c-hid-acpi.h .
The change to make it take an acpi_device pointer instead of i2c_hid_acpi *ihid_acpi
should be kept, but without moving the function to the .h file.
Regards,
Hans
> 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
^ permalink raw reply
* Re: [linux-5.10.y 2/3] HID: pass the buffer size to hid_report_raw_event
From: sashiko-bot @ 2026-06-08 10:30 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260608100236.2781796-2-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch introduces `bufsize` to `hid_report_raw_event` to allow safe zero-padding of short reports. However, it passes `size` (the received data length) instead of the actual buffer capacity in key call sites. Most notably, in `wacom_wac_queue_flush()`, the buffer has a fixed capacity (`u8 buf[WACOM_PKGLEN_MAX]`), but `size` is incorrectly passed as `bufsize`. In `hid_input_report()`, `bufsize` is hardcoded to `size`. This guarantees that `bsize == csize`, meaning any short report (`csize < rsize`) will unconditionally trigger the `if (bsize < rsize)` error check and be dropped with `-EINVAL`. The intended zero-padding mitigation (`memset`) is therefore completely unreachable for these paths.
- [Low] Format specifier mismatch in error logs: `bsize` is of type `size_t` but is printed using `%ld` instead of `%zu`. Additionally, unsigned 32-bit variables (`report->id`, `csize`, `rsize`) are printed using the signed `%d` specifier instead of `%u`.
--
commit acf01e17656800b6ab94002473e3b03a8259b323
Author: Benjamin Tissoires <bentiss@kernel.org>
HID: pass the buffer size to hid_report_raw_event
This commit updates hid_report_raw_event to accept both the buffer size and
the data size. This allows the function to make a better decision about
whether to reject a buffer to prevent an overflow, or to safely pad it with
zeroes.
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index aa9ae6ccb28a8..c73f4ac16fdf2 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1784,16 +1784,24 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
> int max_buffer_size = HID_MAX_BUFFER_SIZE;
> unsigned int a;
> u32 rsize, csize = size;
> + size_t bsize = bufsize;
> u8 *cdata = data;
> int ret = 0;
>
> report = hid_get_report(report_enum, data);
> if (!report)
> - goto out;
> + return 0;
> +
> + if (unlikely(bsize < csize)) {
> + hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
> + report->id, csize, bsize);
[Severity: Low]
This isn't a bug, but could the format specifiers cause compiler warnings on
32-bit architectures? bsize is a size_t and csize and report->id are unsigned,
so should they use %zu and %u instead of %ld and %d?
> + return -EINVAL;
> + }
>
> if (report_enum->numbered) {
> cdata++;
> csize--;
> + bsize--;
> }
>
> rsize = hid_compute_report_size(report);
> @@ -1806,9 +1814,15 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
> else if (rsize > max_buffer_size)
> rsize = max_buffer_size;
>
> + if (bsize < rsize) {
> + hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
> + report->id, rsize, bsize);
[Severity: Low]
Similarly here, should this use %u for report->id and rsize, and %zu for
bsize instead of %d and %ld?
> + return -EINVAL;
> + }
[Severity: High]
If callers pass the incoming data length as both the size and bufsize
arguments, does this mean bsize will equal csize?
If bsize == csize, and we receive a short report where csize < rsize,
wouldn't the bsize < rsize check just above this evaluate to true and return
-EINVAL? Does this make the memset padding logic unreachable for those paths?
> +
> if (csize < rsize) {
> dbg_hid("report %d is too short, (%d < %d)\n", report->id,
> - csize, rsize);
> + csize, rsize);
> memset(cdata + csize, 0, rsize - csize);
> }
>
> @@ -1851,6 +1865,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int i
> struct hid_report_enum *report_enum;
> struct hid_driver *hdrv;
> struct hid_report *report;
> + size_t bufsize = size;
> int ret = 0;
>
> if (!hid)
> @@ -1889,7 +1904,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int i
> goto unlock;
> }
>
> - ret = hid_report_raw_event(hid, type, data, size, interrupt);
> + ret = hid_report_raw_event(hid, type, data, bufsize, size, interrupt);
[Severity: High]
Does setting bufsize directly to size here defeat the padding mitigation?
This hardcodes bufsize to the received payload length, which propagates the
drop behavior to all transport drivers relying on hid_input_report().
[ ... ]
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index abbfb53bb7dc9..09b513812fff1 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -79,7 +79,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
> int err;
>
> size = kfifo_out(fifo, buf, sizeof(buf));
> - err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
> + err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
[Severity: High]
Could we pass sizeof(buf) here instead of size?
Since buf has a fixed capacity of WACOM_PKGLEN_MAX, passing size for
bufsize prevents the core from safely padding short reports and will
instead cause them to be dropped.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100236.2781796-2-lee@kernel.org?part=1
^ permalink raw reply
* [linux-5.10.y 3/3] HID: core: Fix size_t specifier in hid_report_raw_event()
From: Lee Jones @ 2026-06-08 10:02 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Viresh Kumar, Johan Hovold,
Alex Elder, Greg Kroah-Hartman, linux-input, linux-kernel,
greybus-dev, linux-staging
Cc: stable, Nathan Chancellor, Miguel Ojeda, Linus Torvalds,
Sasha Levin
In-Reply-To: <20260608100236.2781796-1-lee@kernel.org>
From: Nathan Chancellor <nathan@kernel.org>
[ Upstream commit 4d3a2a466b8d68d852a1f3bbf11204b718428dc4 ]
When building for 32-bit platforms, for which 'size_t' is
'unsigned int', there are warnings around using the incorrect format
specifier to print bsize in hid_report_raw_event():
drivers/hid/hid-core.c:2054:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
2053 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
| ~~~
| %zu
2054 | report->id, csize, bsize);
| ^~~~~
drivers/hid/hid-core.c:2076:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
2075 | hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
| ~~~
| %zu
2076 | report->id, rsize, bsize);
| ^~~~~
Use the proper 'size_t' format specifier, '%zu', to clear up the
warnings.
Cc: stable@vger.kernel.org
Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/20260516020430.110135-1-ojeda@kernel.org/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit 3ab135238832446399614e7a4bb796d620717806)
Signed-off-by: Lee Jones <lee@kernel.org>
(cherry picked from commit 0f77a993b5426cca1b046c9ab4b2f8355a4d45dc)
Signed-off-by: Lee Jones <lee@kernel.org>
(cherry picked from commit 70333a8f866aad8cbd6956e2ec4ace159fa4243b)
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c73f4ac16fdf..918c66d5bc93 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1793,7 +1793,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data,
return 0;
if (unlikely(bsize < csize)) {
- hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
report->id, csize, bsize);
return -EINVAL;
}
@@ -1815,7 +1815,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data,
rsize = max_buffer_size;
if (bsize < rsize) {
- hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
report->id, rsize, bsize);
return -EINVAL;
}
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [linux-5.10.y 2/3] HID: pass the buffer size to hid_report_raw_event
From: Lee Jones @ 2026-06-08 10:02 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Viresh Kumar, Johan Hovold,
Alex Elder, Greg Kroah-Hartman, linux-input, linux-kernel,
greybus-dev, linux-staging
Cc: stable, Benjamin Tissoires, Jiri Kosina, Sasha Levin
In-Reply-To: <20260608100236.2781796-1-lee@kernel.org>
From: Benjamin Tissoires <bentiss@kernel.org>
[ Upstream commit 2c85c61d1332e1e16f020d76951baf167dcb6f7a ]
commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
bogus memset()") enforced the provided data to be at least the size of
the declared buffer in the report descriptor to prevent a buffer
overflow. However, we can try to be smarter by providing both the buffer
size and the data size, meaning that hid_report_raw_event() can make
better decision whether we should plaining reject the buffer (buffer
overflow attempt) or if we can safely memset it to 0 and pass it to the
rest of the stack.
Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Acked-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Stable-dep-of: 206342541fc8 ("HID: core: introduce hid_safe_input_report()")
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit 509c2605065004fc4cd86ee50a9350d402785307)
[Lee: Backported to linux-6.12.y and beyond]
Signed-off-by: Lee Jones <lee@kernel.org>
(cherry picked from commit f9393998660f146970047bda31526aeb96190f28)
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/hid-core.c | 29 ++++++++++++++++++++++-------
drivers/hid/hid-gfrm.c | 4 ++--
drivers/hid/hid-logitech-hidpp.c | 2 +-
drivers/hid/hid-multitouch.c | 2 +-
drivers/hid/hid-primax.c | 2 +-
drivers/hid/hid-vivaldi.c | 2 +-
drivers/hid/wacom_sys.c | 6 +++---
drivers/staging/greybus/hid.c | 2 +-
include/linux/hid.h | 4 ++--
9 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index aa9ae6ccb28a..c73f4ac16fdf 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1775,8 +1775,8 @@ int __hid_request(struct hid_device *hid, struct hid_report *report,
}
EXPORT_SYMBOL_GPL(__hid_request);
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
- int interrupt)
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data,
+ size_t bufsize, u32 size, int interrupt)
{
struct hid_report_enum *report_enum = hid->report_enum + type;
struct hid_report *report;
@@ -1784,16 +1784,24 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
int max_buffer_size = HID_MAX_BUFFER_SIZE;
unsigned int a;
u32 rsize, csize = size;
+ size_t bsize = bufsize;
u8 *cdata = data;
int ret = 0;
report = hid_get_report(report_enum, data);
if (!report)
- goto out;
+ return 0;
+
+ if (unlikely(bsize < csize)) {
+ hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+ report->id, csize, bsize);
+ return -EINVAL;
+ }
if (report_enum->numbered) {
cdata++;
csize--;
+ bsize--;
}
rsize = hid_compute_report_size(report);
@@ -1806,9 +1814,15 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
else if (rsize > max_buffer_size)
rsize = max_buffer_size;
+ if (bsize < rsize) {
+ hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+ report->id, rsize, bsize);
+ return -EINVAL;
+ }
+
if (csize < rsize) {
dbg_hid("report %d is too short, (%d < %d)\n", report->id,
- csize, rsize);
+ csize, rsize);
memset(cdata + csize, 0, rsize - csize);
}
@@ -1817,7 +1831,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
if (hid->claimed & HID_CLAIMED_HIDRAW) {
ret = hidraw_report_event(hid, data, size);
if (ret)
- goto out;
+ return ret;
}
if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
@@ -1830,7 +1844,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
if (hid->claimed & HID_CLAIMED_INPUT)
hidinput_report_event(hid, report);
-out:
+
return ret;
}
EXPORT_SYMBOL_GPL(hid_report_raw_event);
@@ -1851,6 +1865,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int i
struct hid_report_enum *report_enum;
struct hid_driver *hdrv;
struct hid_report *report;
+ size_t bufsize = size;
int ret = 0;
if (!hid)
@@ -1889,7 +1904,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int i
goto unlock;
}
- ret = hid_report_raw_event(hid, type, data, size, interrupt);
+ ret = hid_report_raw_event(hid, type, data, bufsize, size, interrupt);
unlock:
up(&hid->driver_input_lock);
diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
index 699186ff2349..d2a56bf92b41 100644
--- a/drivers/hid/hid-gfrm.c
+++ b/drivers/hid/hid-gfrm.c
@@ -66,7 +66,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
switch (data[1]) {
case GFRM100_SEARCH_KEY_DOWN:
ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_dn,
- sizeof(search_key_dn), 1);
+ sizeof(search_key_dn), sizeof(search_key_dn), 1);
break;
case GFRM100_SEARCH_KEY_AUDIO_DATA:
@@ -74,7 +74,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
case GFRM100_SEARCH_KEY_UP:
ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_up,
- sizeof(search_key_up), 1);
+ sizeof(search_key_up), sizeof(search_key_up), 1);
break;
default:
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 98562a0ed0c3..d31f2737b13d 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3176,7 +3176,7 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp,
memcpy(&consumer_report[1], &data[3], 4);
/* We are called from atomic context */
hid_report_raw_event(hidpp->hid_dev, HID_INPUT_REPORT,
- consumer_report, 5, 1);
+ consumer_report, sizeof(consumer_report), 5, 1);
return 1;
}
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 948bd59ab5d2..c3bcc23d7c7c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -449,7 +449,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
}
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
- size, 0);
+ size, size, 0);
if (ret)
dev_warn(&hdev->dev, "failed to report feature\n");
}
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c
index 1e6413d07cae..16e2a811eda9 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -44,7 +44,7 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
data[0] |= (1 << (data[idx] - 0xE0));
data[idx] = 0;
}
- hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, 0);
+ hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, size, 0);
return 1;
default: /* unknown report */
diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c
index d57ec1767037..fdfea1355ee7 100644
--- a/drivers/hid/hid-vivaldi.c
+++ b/drivers/hid/hid-vivaldi.c
@@ -126,7 +126,7 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
}
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
- report_len, 0);
+ report_len, report_len, 0);
if (ret) {
dev_warn(&hdev->dev, "failed to report feature %d\n",
field->report->id);
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index abbfb53bb7dc..09b513812fff 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -79,7 +79,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
int err;
size = kfifo_out(fifo, buf, sizeof(buf));
- err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
+ err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
if (err) {
hid_warn(hdev, "%s: unable to flush event due to error %d\n",
__func__, err);
@@ -324,7 +324,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
data, n, WAC_CMD_RETRIES);
if (ret == n && features->type == HID_GENERIC) {
ret = hid_report_raw_event(hdev,
- HID_FEATURE_REPORT, data, n, 0);
+ HID_FEATURE_REPORT, data, n, n, 0);
} else if (ret == 2 && features->type != HID_GENERIC) {
features->touch_max = data[1];
} else {
@@ -385,7 +385,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
data, n, WAC_CMD_RETRIES);
if (ret == n) {
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT,
- data, n, 0);
+ data, n, n, 0);
} else {
hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
__func__);
diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c
index ed706f39e87a..d68f60da0dd1 100644
--- a/drivers/staging/greybus/hid.c
+++ b/drivers/staging/greybus/hid.c
@@ -201,7 +201,7 @@ static void gb_hid_init_report(struct gb_hid *ghid, struct hid_report *report)
* we just need to setup the input fields, so using
* hid_report_raw_event is safe.
*/
- hid_report_raw_event(ghid->hid, report->type, ghid->inbuf, size, 1);
+ hid_report_raw_event(ghid->hid, report->type, ghid->inbuf, ghid->bufsize, size, 1);
}
static void gb_hid_init_reports(struct gb_hid *ghid)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index ab56fffb74a2..aaae2fecd4ae 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1175,8 +1175,8 @@ static inline u32 hid_report_len(struct hid_report *report)
return DIV_ROUND_UP(report->size, 8) + (report->id > 0);
}
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
- int interrupt);
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data,
+ size_t bufsize, u32 size, int interrupt);
/* HID quirks API */
unsigned long hid_lookup_quirk(const struct hid_device *hdev);
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [linux-5.10.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc
From: Lee Jones @ 2026-06-08 10:02 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Viresh Kumar, Johan Hovold,
Alex Elder, Greg Kroah-Hartman, linux-input, linux-kernel,
greybus-dev, linux-staging
Cc: stable, Vicki Pfau, Jiri Kosina
From: Vicki Pfau <vi@endrift.com>
hid_warn_ratelimited() is needed. Add the others as part of the block.
Signed-off-by: Vicki Pfau <vi@endrift.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
(cherry picked from commit 1d64624243af8329b4b219d8c39e28ea448f9929)
Signed-off-by: Lee Jones <lee@kernel.org>
(cherry picked from commit 3dc96d0b81eae69bf71e129e3f331c982c5c70fd)
Signed-off-by: Lee Jones <lee@kernel.org>
(cherry picked from commit 023f03a90d4fcc809ffbfc70e6927ce9889b2578)
Signed-off-by: Lee Jones <lee@kernel.org>
---
include/linux/hid.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 03627c96d814..ab56fffb74a2 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1217,4 +1217,15 @@ do { \
#define hid_dbg_once(hid, fmt, ...) \
dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_err_ratelimited(hid, fmt, ...) \
+ dev_err_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_ratelimited(hid, fmt, ...) \
+ dev_notice_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_ratelimited(hid, fmt, ...) \
+ dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_ratelimited(hid, fmt, ...) \
+ dev_info_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_ratelimited(hid, fmt, ...) \
+ dev_dbg_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+
#endif
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related
* Re: [PATCH v3 2/2] arm64: dts: qcom: glymur-crd: Add FocalTech ft3d81 touchscreen support
From: Konrad Dybcio @ 2026-06-08 9:58 UTC (permalink / raw)
To: Pradyot Kumar Nayak, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Peng, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, Abel Vesa
Cc: linux-arm-msm, linux-input, devicetree, linux-kernel
In-Reply-To: <20260603-arm64-dts-glymur-crd-add-reset-gpio-to-v3-2-3453ef577bcf@oss.qualcomm.com>
On 6/3/26 5:11 AM, Pradyot Kumar Nayak wrote:
> The touchscreen module on Glymur/Mahua CRDs is different from
> the one used on Hamoa CRDs and requires the reset-gpios to be wired to
> the device. Without this in place the reset line will remain
> permanently asserted during resume leaving the device offline and causing
> all I2C transactions to fail with -ENXIO.
>
> i2c_hid_of 3-0038: failed to change power setting.
> i2c_hid_of 3-0038: PM: dpm_run_callback():
> i2c_hid_core_pm_resume [i2c_hid] returns -6
> i2c_hid_of 3-0038: PM: failed to resume async: error -6
>
> The touchscreen on Glymur/Mahua-CRD is a focaltech ft3d81,
> which is hardware-compatible with the ft8112.
> we have added the required change in DT.
>
> Fixes: e6bf559f7eb9 ("arm64: dts: qcom: glymur-crd: Enable keyboard, trackpad and touchscreen")
> Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH] HID: uhid: convert to hid_safe_input_report()
From: Lee Jones @ 2026-06-08 8:59 UTC (permalink / raw)
To: Carlos Llamas
Cc: David Rheinsberg, Jiri Kosina, Benjamin Tissoires, kernel-team,
linux-kernel, stable, open list:UHID USERSPACE HID IO DRIVER
In-Reply-To: <20260606181552.3095967-1-cmllamas@google.com>
On Sat, 06 Jun 2026, Carlos Llamas wrote:
> Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()"), added a check in hid_report_raw_event() to reject
> reports if the received data size is smaller than expected. This was
> intended to prevent OOB errors by no longer allowing zeroing-out of
> shorter reports due to the lack of buffer size information.
>
> However, this leads to regressions in hid_report_raw_event(), where
> shorter than expected reports are rejected, even though their buffers
> are sufficiently large to be zero-padded.
>
> To solve this issue, Benjamin introduced a safer alternative in commit
> 206342541fc8 ("HID: core: introduce hid_safe_input_report()"), which
> forwards the buffer size and allows hid_report_raw_event() to safely
> zero-pad the data.
>
> Convert uhid to use hid_safe_input_report() and pass UHID_DATA_MAX as
> the buffer size. This prevents the reported regressions [1], allowing
> hid core to zero-pad the shorter reports safely as expected.
>
> Cc: stable@vger.kernel.org
> Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
> Closes: https://lore.kernel.org/all/ahsh0UtTX6e0ZeHa@google.com/ [1]
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Lee Jones <lee@kernel.org>
> ---
> drivers/hid/uhid.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 524b53a3c87b..37b60c3aaf66 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -595,8 +595,8 @@ static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
> if (!READ_ONCE(uhid->running))
> return -EINVAL;
>
> - hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
> - min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
> + hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data, UHID_DATA_MAX,
> + min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
>
> return 0;
> }
> @@ -606,8 +606,8 @@ static int uhid_dev_input2(struct uhid_device *uhid, struct uhid_event *ev)
> if (!READ_ONCE(uhid->running))
> return -EINVAL;
>
> - hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data,
> - min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
> + hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data, UHID_DATA_MAX,
> + min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
>
> return 0;
> }
> --
> 2.54.0.1032.g2f8565e1d1-goog
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH] ARM: footbridge: convert to sparse IRQs
From: Linus Walleij @ 2026-06-08 8:49 UTC (permalink / raw)
To: Ethan Nelson-Moore
Cc: linux-arm-kernel, linux-input, linux-serial, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, Dmitry Torokhov, Jiri Slaby,
Russell King (Oracle), Kees Cook, Nathan Chancellor,
Sebastian Andrzej Siewior, Steven Rostedt, Thomas Weissschuh,
Peter Zijlstra
In-Reply-To: <20260510052107.555825-1-enelsonmoore@gmail.com>
On Sun, May 10, 2026 at 7:21 AM Ethan Nelson-Moore
<enelsonmoore@gmail.com> wrote:
> To improve future maintainability, change the interrupt handling for
> mach-footbridge to use sparse IRQs.
>
> Since the number of possible interrupts is already fixed and relatively
> small, just make it use all legacy interrupts preallocated using the
> .nr_irqs field in the machine descriptor, rather than actually
> allocating domains on the fly.
>
> Many files had to be adjusted to include <mach/irqs.h>
> explicitly because it is no longer implicitly included with sparse
> IRQs.
>
> Description adapted from commit c78a41fc04f0 ("ARM: s3c24xx: convert
> to sparse-irq").
>
> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
Have you tested this on real hardware?
The commit doesn't really say.
I might have to dig out my NetWinder to test this.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Joshua Crofts @ 2026-06-08 6:41 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-3-dd4a6820b674@gmail.com>
On Sat, 6 Jun 2026 at 14:19, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Prefer 'u32' instead of bare 'unsigned' variable to improve code
> clarity and consistency with kernel style.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
> drivers/iio/light/hid-sensor-als.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index d72e260b8266..ae2fda8dc500 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -241,7 +241,7 @@ static const struct iio_info als_info = {
>
> /* Callback handler to send event after all samples are received and captured */
> static int als_proc_event(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> void *priv)
Code-wise fine, however looking at this function, usage_id is never
actually used,
though not sure whether removing it would break the ABI... up to you
to check it.
--
Kind regards
CJD
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Joshua Crofts @ 2026-06-08 6:37 UTC (permalink / raw)
To: Maxwell Doose
Cc: Sanjay Chitroda, Jiri Kosina, Jonathan Cameron,
Srinivas Pandruvada, David Lechner, Nuno Sá, Andy Shevchenko,
linux-input, linux-iio, linux-kernel
In-Reply-To: <CAKqfh0Efa1+VShSW76tqaV8XtAZToRfsCy0cTi5sXOUm_nwLiw@mail.gmail.com>
On Sun, 7 Jun 2026 at 22:54, Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Sat, Jun 6, 2026 at 7:19 AM Sanjay Chitroda
> <sanjayembeddedse@gmail.com> wrote:
> >
> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
> > Prefer 'u32' instead of bare 'unsigned' variable to improve code
> > clarity and consistency with kernel style.
> >
> > Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> > ---
> > drivers/iio/light/hid-sensor-als.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
>
> Do we *need* u32 though? Usually these types are for places where we
> require specific bit-widths for a reason (e.g., reading a hardware
> register) but I'm not sure that's the case here. I could be totally
> wrong, in that case please correct me but otherwise I unfortunately
> don't see much value in this. That's just my personal opinion though,
> Jonathan may think otherwise.
Aside from the array of usage ids that are u32 defined here:
https://elixir.bootlin.com/linux/v7.1-rc6/source/drivers/iio/light/hid-sensor-als.c#L46
there are additional structs used in the HID drivers that take u32 (I
had a similar
patch moving `unsigned` to `unsigned int`, but it was recommended to use the
types that the structs use, hence u32).
--
Kind regards
CJD
^ permalink raw reply
* [dtor-input:master] BUILD SUCCESS d4579af29e67ca8722db0a1194227f8015c8981d
From: kernel test robot @ 2026-06-08 5:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git master
branch HEAD: d4579af29e67ca8722db0a1194227f8015c8981d Input: ims-pcu - fix potential infinite loop in CDC union descriptor parsing
elapsed time: 1528m
configs tested: 192
configs skipped: 4
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-16.1.0
alpha allyesconfig gcc-16.1.0
alpha defconfig gcc-16.1.0
arc allmodconfig gcc-16.1.0
arc allnoconfig gcc-16.1.0
arc allyesconfig gcc-16.1.0
arc defconfig gcc-16.1.0
arc randconfig-001 gcc-8.5.0
arc randconfig-001-20260608 gcc-8.5.0
arc randconfig-002 gcc-8.5.0
arc randconfig-002-20260608 gcc-12.5.0
arm allnoconfig clang-23
arm allyesconfig gcc-16.1.0
arm defconfig clang-23
arm randconfig-001 gcc-16.1.0
arm randconfig-001-20260608 gcc-8.5.0
arm randconfig-002 gcc-15.2.0
arm randconfig-002-20260608 gcc-8.5.0
arm randconfig-003 clang-23
arm randconfig-003-20260608 clang-23
arm randconfig-004 gcc-13.4.0
arm randconfig-004-20260608 clang-23
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-16.1.0
arm64 defconfig gcc-16.1.0
arm64 randconfig-001 gcc-14.3.0
arm64 randconfig-001-20260608 gcc-8.5.0
arm64 randconfig-002 gcc-8.5.0
arm64 randconfig-002-20260608 gcc-12.5.0
arm64 randconfig-003 clang-20
arm64 randconfig-003-20260608 clang-23
arm64 randconfig-004 clang-23
arm64 randconfig-004-20260608 gcc-12.5.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
csky defconfig gcc-16.1.0
csky randconfig-001 gcc-13.4.0
csky randconfig-001-20260608 gcc-12.5.0
csky randconfig-002 gcc-14.3.0
csky randconfig-002-20260608 gcc-16.1.0
hexagon allmodconfig clang-23
hexagon allnoconfig clang-23
hexagon defconfig clang-23
hexagon randconfig-001 clang-23
hexagon randconfig-001-20260608 clang-23
hexagon randconfig-002 clang-23
hexagon randconfig-002-20260608 clang-17
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allyesconfig gcc-14
i386 buildonly-randconfig-001-20260608 gcc-14
i386 buildonly-randconfig-002-20260608 clang-22
i386 buildonly-randconfig-003-20260608 gcc-14
i386 buildonly-randconfig-004-20260608 clang-22
i386 buildonly-randconfig-005-20260608 gcc-14
i386 buildonly-randconfig-006-20260608 clang-22
i386 defconfig clang-22
i386 randconfig-012-20260608 gcc-14
i386 randconfig-014 clang-22
i386 randconfig-015 gcc-14
i386 randconfig-016 gcc-14
i386 randconfig-016-20260608 gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-20
loongarch defconfig clang-23
loongarch randconfig-001 clang-23
loongarch randconfig-001-20260608 clang-23
loongarch randconfig-002 gcc-16.1.0
loongarch randconfig-002-20260608 clang-18
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig gcc-16.1.0
m68k defconfig gcc-16.1.0
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
microblaze defconfig gcc-16.1.0
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig gcc-11.5.0
nios2 defconfig gcc-11.5.0
nios2 randconfig-001 gcc-11.5.0
nios2 randconfig-001-20260608 gcc-11.5.0
nios2 randconfig-002 gcc-8.5.0
nios2 randconfig-002-20260608 gcc-11.5.0
openrisc allmodconfig gcc-16.1.0
openrisc allnoconfig gcc-16.1.0
openrisc defconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig gcc-16.1.0
parisc allyesconfig gcc-16.1.0
parisc defconfig gcc-16.1.0
parisc randconfig-001 gcc-8.5.0
parisc randconfig-001-20260608 gcc-8.5.0
parisc randconfig-002 gcc-12.5.0
parisc randconfig-002-20260608 gcc-8.5.0
parisc64 defconfig gcc-16.1.0
powerpc allmodconfig gcc-16.1.0
powerpc allnoconfig gcc-16.1.0
powerpc randconfig-001 gcc-10.5.0
powerpc randconfig-001-20260608 clang-17
powerpc randconfig-002 gcc-8.5.0
powerpc randconfig-002-20260608 clang-16
powerpc64 randconfig-001 clang-16
powerpc64 randconfig-001-20260608 gcc-14.3.0
powerpc64 randconfig-002 clang-23
powerpc64 randconfig-002-20260608 gcc-8.5.0
riscv allmodconfig clang-23
riscv allnoconfig gcc-16.1.0
riscv allyesconfig clang-23
riscv defconfig clang-23
riscv randconfig-001-20260608 clang-23
s390 allmodconfig clang-23
s390 allnoconfig clang-23
s390 allyesconfig gcc-16.1.0
s390 defconfig clang-18
s390 randconfig-001-20260608 gcc-15.2.0
s390 randconfig-002-20260608 clang-20
sh allmodconfig gcc-16.1.0
sh allnoconfig gcc-16.1.0
sh allyesconfig gcc-16.1.0
sh defconfig gcc-16.1.0
sh kfr2r09_defconfig gcc-16.1.0
sh randconfig-001-20260608 gcc-12.5.0
sh randconfig-002-20260608 gcc-10.5.0
sparc allnoconfig gcc-16.1.0
sparc defconfig gcc-16.1.0
sparc randconfig-001 gcc-16.1.0
sparc randconfig-001-20260608 gcc-13.4.0
sparc randconfig-002 gcc-11.5.0
sparc randconfig-002-20260608 gcc-15.2.0
sparc64 allmodconfig clang-20
sparc64 defconfig clang-23
sparc64 randconfig-001 gcc-12.5.0
sparc64 randconfig-001-20260608 clang-20
sparc64 randconfig-002 clang-20
sparc64 randconfig-002-20260608 gcc-15.2.0
um allmodconfig clang-23
um allnoconfig clang-16
um allyesconfig gcc-14
um defconfig clang-23
um i386_defconfig gcc-14
um randconfig-001 clang-23
um randconfig-001-20260608 gcc-14
um randconfig-002 clang-23
um randconfig-002-20260608 clang-23
um x86_64_defconfig clang-23
x86_64 allmodconfig clang-22
x86_64 allnoconfig clang-22
x86_64 allyesconfig clang-22
x86_64 buildonly-randconfig-001-20260608 gcc-14
x86_64 buildonly-randconfig-002-20260608 clang-22
x86_64 buildonly-randconfig-003-20260608 gcc-13
x86_64 buildonly-randconfig-004-20260608 gcc-14
x86_64 buildonly-randconfig-005-20260608 clang-22
x86_64 buildonly-randconfig-006-20260608 gcc-14
x86_64 defconfig gcc-14
x86_64 randconfig-001 gcc-14
x86_64 randconfig-001-20260608 clang-22
x86_64 randconfig-002 gcc-14
x86_64 randconfig-002-20260608 gcc-14
x86_64 randconfig-003 clang-22
x86_64 randconfig-003-20260608 gcc-14
x86_64 randconfig-004 clang-22
x86_64 randconfig-005 gcc-14
x86_64 randconfig-005-20260608 clang-22
x86_64 randconfig-006-20260608 gcc-14
x86_64 randconfig-011 gcc-14
x86_64 randconfig-011-20260608 gcc-14
x86_64 randconfig-012 gcc-14
x86_64 randconfig-012-20260608 clang-22
x86_64 randconfig-013 clang-22
x86_64 randconfig-013-20260608 clang-22
x86_64 randconfig-014 gcc-14
x86_64 randconfig-014-20260608 gcc-14
x86_64 randconfig-015 gcc-14
x86_64 randconfig-015-20260608 gcc-14
x86_64 randconfig-016 clang-22
x86_64 randconfig-016-20260608 gcc-14
x86_64 randconfig-071-20260608 gcc-14
x86_64 randconfig-072-20260608 gcc-14
x86_64 randconfig-073-20260608 clang-22
x86_64 randconfig-074-20260608 clang-22
x86_64 randconfig-075-20260608 clang-22
x86_64 randconfig-076-20260608 clang-22
x86_64 rhel-9.4-rust clang-22
xtensa allnoconfig gcc-16.1.0
xtensa randconfig-001 gcc-8.5.0
xtensa randconfig-001-20260608 gcc-9.5.0
xtensa randconfig-002 gcc-13.4.0
xtensa randconfig-002-20260608 gcc-11.5.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] input: ipaq-micro-keys: simplify allocation
From: Rosen Penev @ 2026-06-08 4:50 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, open list
Use a flexible array member to have a single allocation.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/input/keyboard/ipaq-micro-keys.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
index ca7ec054b1ce..0fc5b5dcdef5 100644
--- a/drivers/input/keyboard/ipaq-micro-keys.c
+++ b/drivers/input/keyboard/ipaq-micro-keys.c
@@ -23,7 +23,7 @@
struct ipaq_micro_keys {
struct ipaq_micro *micro;
struct input_dev *input;
- u16 *codes;
+ u16 codes[];
};
static const u16 micro_keycodes[] = {
@@ -90,10 +90,11 @@ static int micro_key_probe(struct platform_device *pdev)
int error;
int i;
- keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
+ keys = devm_kzalloc(&pdev->dev, struct_size(keys, codes, ARRAY_SIZE(micro_keycodes)), GFP_KERNEL);
if (!keys)
return -ENOMEM;
+ memcpy(keys->codes, micro_keycodes, ARRAY_SIZE(micro_keycodes) * sizeof(*keys->codes));
keys->micro = dev_get_drvdata(pdev->dev.parent);
keys->input = devm_input_allocate_device(&pdev->dev);
@@ -102,10 +103,6 @@ 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;
keys->input->keycode = keys->codes;
--
2.54.0
^ permalink raw reply related
* RE: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: Xu, Even @ 2026-06-08 2:48 UTC (permalink / raw)
To: d3z, jikos@kernel.org, bentiss@kernel.org
Cc: Sun, Xinpeng, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, sakari.ailus@linux.intel.com,
abhishektamboli9@gmail.com
In-Reply-To: <20260601211828.112626-1-d3z.the.dev@gmail.com>
> -----Original Message-----
> From: d3z <d3z.the.dev@gmail.com>
> Sent: Tuesday, June 2, 2026 5:18 AM
> To: Xu, Even <even.xu@intel.com>; jikos@kernel.org; bentiss@kernel.org
> Cc: Sun, Xinpeng <xinpeng.sun@intel.com>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; sakari.ailus@linux.intel.com;
> abhishektamboli9@gmail.com; Danny D . <d3z.the.dev@gmail.com>
> Subject: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
>
> From: Danny D. <d3z.the.dev@gmail.com>
>
> On the Surface Pro 10 (Meteor Lake) the touchscreen stops working after a
> suspend/resume cycle and only recovers after a reboot. The driver logs
> "GET_DEVICE_INFO: recv failed: -11" on resume.
>
> This platform suspends through s2idle: /sys/power/mem_sleep exposes "[s2idle]"
> as the only state, there is no "deep"/S3 entry at all. The touch IC nonetheless
> loses power across that s2idle suspend, the same way it does across hibernation.
> quickspi_resume() only re-selects the THC port, restores interrupts and DMA and
> sends a HIDSPI_ON command, assuming the touch IC kept its power and state.
> When it has actually lost power the HIDSPI_ON command is never acknowledged
> and the descriptor read fails, leaving the touchscreen dead until the module is
> reloaded.
>
> quickspi_restore() already handles this for hibernation by reconfiguring the THC
> SPI/LTR settings and running reset_tic() to re-enumerate the device. Make
> quickspi_resume() do the same when the device is not a wake source. A wake-
> enabled device keeps its power and state across suspend, so it stays on the light
> restore path: resetting it would discard a pending wake touch event and break
> wake-on-touch.
>
> The non-wake path mirrors the existing quickspi_restore() sequence, including
> enabling interrupts before reset_tic(), so it introduces no new ordering relative to
> code already in the driver.
>
> This change has been validated on a Surface Pro 10 running the linux-surface
> kernel across multiple s2idle suspend/resume cycles; it has not been tested on a
> mainline build.
>
> Closes: https://github.com/linux-surface/linux-surface/issues/1799
> Signed-off-by: Danny D. <d3z.the.dev@gmail.com>
> ---
> v1 -> v2:
> - Only run the full reset when the device is not a wake source
> (device_may_wakeup()), so wake-on-touch is no longer disturbed.
> - Reword the changelog around s2idle: the SP10 has no "deep"/S3 state, the
> touch IC loses power across s2idle.
>
> .../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 60 +++++++++++++++++--
> 1 file changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -784,20 +784,72 @@ static int quickspi_resume(struct device
> if (ret)
> return ret;
>
> + /*
> + * A wake-enabled device keeps its power and state across suspend, so
> + * only restore the THC context. Resetting it here would discard a
> + * pending wake touch event and break wake-on-touch.
> + */
> + if (device_may_wakeup(qsdev->dev)) {
> + thc_interrupt_config(qsdev->thc_hw);
> +
> + thc_interrupt_enable(qsdev->thc_hw, true);
> +
> + ret = thc_dma_configure(qsdev->thc_hw);
> + if (ret)
> + return ret;
> +
> + return thc_interrupt_quiesce(qsdev->thc_hw, false);
> + }
> +
> + /*
> + * Otherwise the touch IC may have lost power across suspend. On
> + * platforms that suspend through s2idle (for example the Surface Pro
> + * 10, whose firmware exposes s2idle as the only mem_sleep state) the
> + * IC loses power the same way it does across hibernation. A plain
> + * HIDSPI_ON is then not acknowledged and the descriptor read fails, so
> + * re-enumerate the device through the full reset flow already used by
> + * quickspi_restore().
> + */
> + thc_spi_input_output_address_config(qsdev->thc_hw,
> + qsdev->input_report_hdr_addr,
> + qsdev->input_report_bdy_addr,
> + qsdev->output_report_addr);
> +
> + ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
> + qsdev->spi_read_io_mode,
> + qsdev->spi_read_opcode,
> + qsdev->spi_packet_size);
> + if (ret)
> + return ret;
> +
> + ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
> + qsdev->spi_write_io_mode,
> + qsdev->spi_write_opcode,
> + qsdev->spi_packet_size,
> + qsdev->performance_limit);
> + if (ret)
> + return ret;
> +
> thc_interrupt_config(qsdev->thc_hw);
>
> thc_interrupt_enable(qsdev->thc_hw, true);
>
> - ret = thc_dma_configure(qsdev->thc_hw);
> + /* The touch IC may have lost power, reset it to recover */
> + ret = reset_tic(qsdev);
> if (ret)
> return ret;
>
> - ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
> + ret = thc_dma_configure(qsdev->thc_hw);
> if (ret)
> return ret;
>
> - if (!device_may_wakeup(qsdev->dev))
> - return quickspi_set_power(qsdev, HIDSPI_ON);
> + thc_ltr_config(qsdev->thc_hw,
> + qsdev->active_ltr_val,
> + qsdev->low_power_ltr_val);
> +
> + thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
> +
> + qsdev->state = QUICKSPI_ENABLED;
>
> return 0;
> }
Thanks for your patch!
Reviewed-by: Even Xu <even.xu@intel.com>
> --
> 2.43.0
^ permalink raw reply
* RE: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: Xu, Even @ 2026-06-08 2:46 UTC (permalink / raw)
To: d3z, Sun, Xinpeng, jikos@kernel.org, bentiss@kernel.org
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
abhishektamboli9@gmail.com, sakari.ailus@linux.intel.com
In-Reply-To: <20260604202647.16777-1-d3z.the.dev@gmail.com>
Hi, Danny,
Thanks for your patient to have some many tries.
According to above tests, it seems Surface Pro 10 does not only power off the TIC but also the THC for suspend.
Then your patch is need for this case.
I will add my review flag for your patch, thanks for your attribution!
Best Regards,
Even Xu
> -----Original Message-----
> From: d3z <d3z.the.dev@gmail.com>
> Sent: Friday, June 5, 2026 4:27 AM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>;
> jikos@kernel.org; bentiss@kernel.org
> Cc: Danny D . <d3z.the.dev@gmail.com>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; abhishektamboli9@gmail.com;
> sakari.ailus@linux.intel.com
> Subject: Re: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
>
> From: Danny D. <d3z.the.dev@gmail.com>
>
> Hi Even,
>
> Good catch on the missing state restore. I rebuilt with your version and re-ran the
> test.
>
> This is exactly your proposal: reset_tic() after thc_dma_configure(), then qsdev-
> >state = QUICKSPI_ENABLED, with no thc_spi_*_config and no thc_ltr_*. I built it
> as a module and swapped it into a clean linux-surface 6.19.8 on the Surface Pro
> 10.
>
> It still doesn't survive s2idle - touch is fine at probe, but after one
> suspend/resume it's dead again. This time resume() fails outright instead of
> coming back quietly broken:
>
> intel_quickspi 0000:00:10.0: Wait DEVICE_DESCRIPTOR timeout, ret:0
> intel_quickspi 0000:00:10.0: PM: dpm_run_callback(): pci_pm_resume returns
> -110
> intel_quickspi 0000:00:10.0: PM: failed to resume async: error -110
>
> So reset_tic() can't complete here. It clears the reset handshake, then times out
> in quickspi_get_device_descriptor() and the whole resume bails with -
> ETIMEDOUT. The device isn't a wake source on this box - "Can't find wake GPIO
> resource" - so device_may_wakeup() is false and we're on the reset path, as
> expected.
>
> And the timeout is the THC, not the touch IC: across s2idle its SPI input/output
> address and read/write config are gone, so reset_tic()'s descriptor exchange has
> nothing valid to run against. Reprogram those first - the
> thc_spi_input_output_address_config + read_config + write_config block, same
> as quickspi_restore(), same as v2 on the no-wake path - and reset_tic() completes
> and touch comes back. That's exactly why
> v2 reconfigures the THC instead of only resetting the IC.
>
> One more platform note: the SP10 never reaches S3. Firmware only offers s2idle -
> mem_sleep is just "[s2idle]", with no "deep" entry to select - yet the THC still
> loses its context across plain s2idle. So a reset gated on S3 would never fire here,
> which is why it has to run on the ordinary resume path.
>
> Thanks,
> Danny
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Maxwell Doose @ 2026-06-07 20:50 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-3-dd4a6820b674@gmail.com>
On Sat, Jun 6, 2026 at 7:19 AM Sanjay Chitroda
<sanjayembeddedse@gmail.com> wrote:
>
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Prefer 'u32' instead of bare 'unsigned' variable to improve code
> clarity and consistency with kernel style.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
> drivers/iio/light/hid-sensor-als.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Do we *need* u32 though? Usually these types are for places where we
require specific bit-widths for a reason (e.g., reading a hardware
register) but I'm not sure that's the case here. I could be totally
wrong, in that case please correct me but otherwise I unfortunately
don't see much value in this. That's just my personal opinion though,
Jonathan may think otherwise.
--
best regards,
max
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS fb402386af4cdce108ff991a796386de55439735
From: kernel test robot @ 2026-06-07 19:52 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: fb402386af4cdce108ff991a796386de55439735 Input: atkbd - skip deactivate for HONOR BCC-N's internal keyboard
elapsed time: 1254m
configs tested: 54
configs skipped: 1
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-16.1.0
alpha allyesconfig gcc-16.1.0
arc allmodconfig gcc-16.1.0
arc allnoconfig gcc-16.1.0
arc allyesconfig gcc-16.1.0
arm allnoconfig clang-23
arm allyesconfig gcc-16.1.0
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-16.1.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
hexagon allmodconfig clang-23
hexagon allnoconfig clang-23
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allyesconfig gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-20
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig gcc-16.1.0
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig gcc-11.5.0
openrisc allmodconfig gcc-16.1.0
openrisc allnoconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig gcc-16.1.0
parisc allyesconfig gcc-16.1.0
powerpc allmodconfig gcc-16.1.0
powerpc allnoconfig gcc-16.1.0
riscv allmodconfig clang-23
riscv allnoconfig gcc-16.1.0
riscv allyesconfig clang-23
s390 allmodconfig clang-23
s390 allnoconfig clang-23
s390 allyesconfig gcc-16.1.0
sh allmodconfig gcc-16.1.0
sh allnoconfig gcc-16.1.0
sh allyesconfig gcc-16.1.0
sparc allnoconfig gcc-16.1.0
sparc64 allmodconfig clang-20
um allmodconfig clang-23
um allnoconfig clang-16
um allyesconfig gcc-14
x86_64 allmodconfig clang-22
x86_64 allnoconfig clang-22
x86_64 allyesconfig clang-22
x86_64 rhel-9.4-rust clang-22
xtensa allnoconfig gcc-16.1.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] HID: usbhid: skip interrupt IN polling for devices with no input reports
From: Antheas Kapenekakis @ 2026-06-07 17:11 UTC (permalink / raw)
To: Yaseen
Cc: Denis Benato, Jiri Kosina, Benjamin Tissoires, Ilpo Järvinen,
Kerim Kabirov, GameBurrow, linux-usb, linux-input, linux-kernel
In-Reply-To: <CAGwozwHHF5URNDxus5_WgPNiPc7VD_Gc4NJhV3eDDGdX6e53OA@mail.gmail.com>
On Sun, 7 Jun 2026 at 19:03, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Sun, 7 Jun 2026 at 18:51, Yaseen <yaseen@ghoul.dev> wrote:
> >
> > On 06/06/2026 18:13, Antheas Kapenekakis wrote:
> > > On Sat, 6 Jun 2026 at 14:42, Denis Benato <benato.denis96@gmail.com> wrote:
> > >>
> > >>
> > >> On 6/5/26 14:02, Antheas Kapenekakis wrote:
> > >>> On Fri, 5 Jun 2026 at 13:40, Ahmed Yaseen <yaseen@ghoul.dev> wrote:
> > >>>> usbhid starts polling a device's interrupt IN endpoint on open
> > >>>> (usbhid_open() -> hid_start_in()). If the report descriptor declares no
> > >>>> input reports there is nothing to read there, so the poll is useless,
> > >>>> and on some composite devices it is also harmful.
> > >>> If it did have input reports, would starting the polling still cause
> > >>> issues? Because if it would, the issue is in the polling itself.
> > >> So far we haven't found an asus device that has more than one interface
> > >> that supports reading data out of if.
> > >>> Given the creativity of manufacturers when implementing hid protocols,
> > >>> I find it certain that they do use the in endpoint even without input
> > >>> reports. E.g., for feature reports. This could cause regressions.
> >
> > The ASUS ROG N-Key Device does have feature reports. They are used for
> > RGB control on the keyboard. I have confirmed this with a test by not
> > registering the hidraw node for this interface at all and noted that RGB
> > stops working after. So hiding or ejecting this interface is not an
> > option. Therefore, after this patch, I myself, together with Kerim and
> > GameBurrow have paid attention explicitly to ensure there are no
> > regressions to the LED controls, while fixing the keyboard issue.
> >
> > Also worth noting that feature reports travel over EP0 via
> > usbhid_{get,set}_raw_report() in both directions. The interrupt IN
> > endpoint is only ever used to receive input reports: hid_irq_in() passes
> > everything it gets to hid_safe_input_report(HID_INPUT_REPORT, ...).
> > There is no code in usbhid that reads feature reports from the interrupt
> > IN endpoint at all, so skipping that poll cannot break any feature
> > reports on any device. This is also mentioned in my patch description:
> >
> > "Feature reports and hidraw output keep working over the control and
> > OUT endpoints, so the interface is otherwise unaffected."
> >
> > Regarding a manufacturer using in endpoint without an input report, even
> > today, the HID core would drop that data before it reaches hidraw:
> > __hid_input_report() bails when hid_get_report() finds no matching
> > report. That bail is also before the driver's ->raw_event() callback, so
> > no driver or hidraw reader can currently be relying on such traffic.
>
> Interesting, so it should noop. Muting the in endpoint would not
> affect feature reports that get sent over the in endpoint? I do not
> think this patch will cause regressions for Asus devices. I'm more
> concerned with other ones. E.g., the Legion Go S has a malformed
> report, and I do not recall which endpoints it uses. Then, the Win 5
> also does a mix. Those are two devices I'd be concerned with, but
> there are a myriad of other hid devices this could affect.
>
> I'd rather if possible the fix goes towards fixing the underlying
> issue that blocks processing inputs from other devices. This way, even
> for devices with an actual input report that is infrequent, this issue
> stops being present, even if the blocking wouldn't have been
> perceptible.
>
> I can reproduce on my Z13 in the following days.
>
> Best,
> Antheas
>
> > >> While I mostly agree with this it is also true that the general direction
> > >> for the kernel (especially lately) has been to not do out-of-spec things
> > >> at least by default.
> > >>
> > >> If things really regress it's expected to do so only an very few specific
> > >> devices with a buggy firmware, and we can think of something different
> > >> for those (hopefully very few ones).
> > >>
> > >> Perhaps someone concerned with security might be interested in what
> > >> we have because it doesn't look very normal.
> > >>
> > >> Note that below I have written a few ideas that maybe are worth
> > >
> > > The degradation would be silent.
> > >
> > >> looking into.
> > >>>> The ASUS ROG N-Key keyboards expose a second, input-less interface used
> > >>>> only for RGB control via feature reports. Opening its hidraw node (any
> > >>>> hidraw reader does, including SDL/Steam Input or a plain cat) starts the
> > >>> cating a hidraw causing issues would be expected, so let's focus on the former.
> > >
> > > Try to add spaces before and after your responses
> > >
> > >> Simply opening an hidraw should not trigger a delayed disconnect of that device,
> > >> I don't know why you would expect this to happen nor why you would
> > >> consider it acceptable. It's a bug.
> > >>
> > >> Focusing on userspace software exposing the bug is not a realistic option
> > >> because over the time we found a good chunk of software doing that:
> > >> - logitech control software (forgot the name)
> > >> - open razer software
> > >> - sdl
> > >> - asusctl (obviously it opens the device albeit in the future I will change this)
> > >>
> > >> and likely more given the fact not all software was identified.
> > >>> Asusctl has a bug where if you add the quirk that separates the event
> > >>> nodes per hid, this bug is reproduced as well. I chucked it to
> > >>> complicated threading getting out of control. It is the reason we
> > >>> skipped that patch that was in my series.
> > >> I found and solved the bug already. Regardless the issue remains:
> > >> Even with no asusctl at all, if a user has one logitech mouse
> > >> (and its control software) and a razer keyboard (and its control software)
> > >> the asus N-Key device will start an endless disconnect-reconnect loop.
> > >>
> > >> Any combination of two or more of those tools will trigger the issue
> > >> on some devices (weirdly enough not every model is affected):
> > >>
> > >> this is not good.
> > >>> Now, you say SDL/Steam do a spurious read as well, can you identify
> > >>> the codepath so we can look into it? What devices are affected? The
> > >>> early return fixes a warning on the Z13, but it also feeds through the
> > >>> universal lamp interface on the new Xbox Allies. Is this a bug on
> > >>> those devices or keyboards? If yes, it could be caused by userspace
> > >>> hanging on that node
> >
> > Affected devices include the ROG STRIX 2025 lineup: Scar 16/18
> > (G635L/G835L) and G16/G18 (G615L/G815L). My patch has been tested on
> > both Scar 18 and G18. Additionally a user with a Scar 18 2024 model
> > (G634JZR) has reported the issue as well; they were unable to
> > participate in testing but reproduce the issue with the same cat command
> > (reproduction command provided below). It is likely the G16/G18 of 2024
> > will also be affected. Models prior to 2024 appear unaffected so far.
> >
> > A user with an Xbox Ally X has tested this for me as well as of writing
> > this email. So we are able to confirm that this device is unaffected and
> > no regressions are noticed on that device from my patch, including the
> > lamp/RGB controls.
> >
> > I do not have access to a Z13 at the moment. If you have one, it would
> > be very helpful for me if you could test for any regressions on that
> > device and if the device is affected by the bug, and whether or not this
> > patch fixes the issue.
> >
> > I would also like to take this opportunity to mention that the 3 testers
> > and I are all daily driving a kernel with this patch applied, and over
> > the last few days, have noticed no issues with any devices.
> >
> > >> Sure, and I agree with you that fixing all userspace tools is desirable
> > >> but it's also unfeasible to fix them all, if we managed to do that
> > >> there will be years before everyone receives a fixed version of every
> > >> affected software and even then a core issue would remain:
> > >> linux tries to poll something it can't have anything out from.
> > >>
> > >> I am much more oriented on the fact that kernel shouldn't
> > >> be doing weird things (at least not by default) so this has to
> > >> somehow be stopped regardless of how well userspace behaves.
> > >
> > > The kernel is not doing weird things and I also did not ask you to fix
> > > all userspace software. I asked for a reproduction scenario, as it is
> > > not covered in the patch description. Relooking at the patch today, I
> > > also do not understand what it does fully.
> >
> > The reproduction scenario is in the patch description:
> >
> > "Opening its hidraw node (any hidraw reader does, including SDL/Steam
> > Input or a plain cat) starts the pointless IN poll and keypress reports
> > on the keyboard interface get dropped for as long as the node stays
> > open: a lost key-down drops a letter, a lost key-up leaves the key stuck."
> >
> > i.e. run "sudo timeout 15 cat /dev/hidrawX" against the N-Key RGB
> > interface, then type on the internal keyboard.
> >
> > >
> > > It skips enabling input interrupts (but not only that) for devices
> > > that have no input reports. So the kernel behavior will depend on the
> > > feature descriptor moving forward.
> >
> > What the patch does is the last paragraph of the description:
> >
> > "Skip the poll in usbhid_open() when the device has no input reports."
> >
> > Interrupt IN endpoint on a device with 0 input reports isn't doing
> > anything anyway. The other things the early return skips only matter
> > when input is possible.
> >
> > >
> > > And that fixes a hang on the affected devices because enabling
> > > interrupts on an endpoint without periodic input reports blocks a
> > > parallel endpoint that does have input reports?
> > >
> > > I would like this fix to target the actual cause that causes the block
> > > but it is not clear to me what that is or what is affected.
> >
> > As per my investigations with usbmon, I can see that the keyboard
> > interface's input reports never reach the URB layer while the RGB
> > interface is being polled. From the patch description:
> >
> > "usbmon shows the dropped reports never reach the URB layer"
> >
> > So the blocking likely happens inside the device's firmware, and not in
> > the kernel, so the kernel cannot fix that part. What the kernel can do
> > is to stop arming the IN URB on an endpoint that as per its own
> > descriptor, can never produce data.
> >
> > >
> > > Antheas
> > >
> > >> If you have better ideas on how to fix the kernel we would
> > >> like to hear those as well.
> > >>
> > >> Best regards,
> > >> Denis
> > >>> Antheas
> > >>>
> > >>>> pointless IN poll and keypress reports on the keyboard interface get
> > >>>> dropped for as long as the node stays open: a lost key-down drops a
> > >>>> letter, a lost key-up leaves the key stuck. usbmon shows the dropped
> > >>>> reports never reach the URB layer.
> > >>>>
> > >>>> The useless poll itself is long-standing; commit 4ac74ea68f64 ("HID:
> > >>>> asus: early return for ROG devices") is what exposes it on these
> > >>>> devices by keeping the input-less interface alive instead of ejecting
> > >>>> it, so its hidraw node can be opened and the poll started.
> > >>>>
> > >>>> Skip the poll in usbhid_open() when the device has no input reports.
> > >>>> Feature reports and hidraw output keep working over the control and OUT
> > >>>> endpoints, so the interface is otherwise unaffected.
> > >> I will write my review here to avoid forking the discussion:
> > >>
> > >> I agree with the general idea but perhaps we can avoid
> > >> some hid devices to ever get HID_QUIRK_ALWAYS_POLL
> > >> and that might be enough to skip the problematic code?
> > >>
> > >> Maybe there is value in doing this with a quirk flag in hid-asus.c
> > >> affecting the least amount of devices?
> > >>
> > >> Or maybe just prevent devices with no data possibly coming out
> > >> to ever get HID_QUIRK_ALWAYS_POLL?
> >
> > Thank you for the review!
> >
> > I would like to also highlight one thing here; the HID_QUIRK_ALWAYS_POLL
> > is not given to this specific device. It was already in the if
> > condition, for the devices that do use it; my change only ORs a second
> > independent condition into it. So keeping devices away from that quirk
> > would not change anything here.
> >
> > Adding a quirk flag for this specific device is something I too have
> > considered and will be happy to change it like so if Jiri or Benjamin
> > feel it is more appropriate. My reasoning for taking the current route
> > is that it would prevent any hidden issues that might arise similarly,
> > and fix the whole class of this issue rather than for one vendor when
> > the likelihood of a regression is very low from skipping interrupt IN
> > polling if a device doesn't have input reports in the first place.
> >
I missed this part of the response. Here I'd add that
HID_QUIRK_ALWAYS_POLL is part of six if statements so your patch is
not equivalent to HID_QUIRK_ALWAYS_POLL. If it should be, perhaps an
alternative for fixing just asus devices would be to OR that quirk
when hid-asus initializes for all devices. I am not sure if it is a
firmware issue, if it is a kernel issue that can be mitigated without
quirks or skipping enabling the in endpoint, I'd prefer that. Failing
that, a quirk would perhaps limit the affected devices
Antheas
> > Best Regards,
> > Yaseen
> >
> > >>
> > >> For how to best do this we will need to hear what Jiri and
> > >> Benjamin have to say but if they think the proposed solution
> > >> is the correct solution:
> > >>
> > >> Reviewed-by: Denis Benato <denis.benato@linux.dev>
> > >>>> Fixes: 4ac74ea68f64 ("HID: asus: early return for ROG devices")
> > >>>> Tested-by: Kerim Kabirov <the.privat33r+linux@pm.me>
> > >>>> Tested-by: GameBurrow <gameburrow@pm.me>
> > >>>> Signed-off-by: Ahmed Yaseen <yaseen@ghoul.dev>
> > >>>> ---
> > >>>> drivers/hid/usbhid/hid-core.c | 3 ++-
> > >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > >>>> index 96b0181cf819..90a8b34d9305 100644
> > >>>> --- a/drivers/hid/usbhid/hid-core.c
> > >>>> +++ b/drivers/hid/usbhid/hid-core.c
> > >>>> @@ -688,7 +688,8 @@ static int usbhid_open(struct hid_device *hid)
> > >>>>
> > >>>> set_bit(HID_OPENED, &usbhid->iofl);
> > >>>>
> > >>>> - if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
> > >>>> + if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) ||
> > >>>> + list_empty(&hid->report_enum[HID_INPUT_REPORT].report_list)) {
> > >>>> res = 0;
> > >>>> goto Done;
> > >>>> }
> > >>>> --
> > >>>> 2.54.0
> > >>>>
> > >>>>
> > >>>>
> >
> >
> >
^ permalink raw reply
* Re: [PATCH] HID: usbhid: skip interrupt IN polling for devices with no input reports
From: Antheas Kapenekakis @ 2026-06-07 17:03 UTC (permalink / raw)
To: Yaseen
Cc: Denis Benato, Jiri Kosina, Benjamin Tissoires, Ilpo Järvinen,
Kerim Kabirov, GameBurrow, linux-usb, linux-input, linux-kernel
In-Reply-To: <20a9d77f-c60a-44ba-ac39-15107fc81256@ghoul.dev>
On Sun, 7 Jun 2026 at 18:51, Yaseen <yaseen@ghoul.dev> wrote:
>
> On 06/06/2026 18:13, Antheas Kapenekakis wrote:
> > On Sat, 6 Jun 2026 at 14:42, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >>
> >> On 6/5/26 14:02, Antheas Kapenekakis wrote:
> >>> On Fri, 5 Jun 2026 at 13:40, Ahmed Yaseen <yaseen@ghoul.dev> wrote:
> >>>> usbhid starts polling a device's interrupt IN endpoint on open
> >>>> (usbhid_open() -> hid_start_in()). If the report descriptor declares no
> >>>> input reports there is nothing to read there, so the poll is useless,
> >>>> and on some composite devices it is also harmful.
> >>> If it did have input reports, would starting the polling still cause
> >>> issues? Because if it would, the issue is in the polling itself.
> >> So far we haven't found an asus device that has more than one interface
> >> that supports reading data out of if.
> >>> Given the creativity of manufacturers when implementing hid protocols,
> >>> I find it certain that they do use the in endpoint even without input
> >>> reports. E.g., for feature reports. This could cause regressions.
>
> The ASUS ROG N-Key Device does have feature reports. They are used for
> RGB control on the keyboard. I have confirmed this with a test by not
> registering the hidraw node for this interface at all and noted that RGB
> stops working after. So hiding or ejecting this interface is not an
> option. Therefore, after this patch, I myself, together with Kerim and
> GameBurrow have paid attention explicitly to ensure there are no
> regressions to the LED controls, while fixing the keyboard issue.
>
> Also worth noting that feature reports travel over EP0 via
> usbhid_{get,set}_raw_report() in both directions. The interrupt IN
> endpoint is only ever used to receive input reports: hid_irq_in() passes
> everything it gets to hid_safe_input_report(HID_INPUT_REPORT, ...).
> There is no code in usbhid that reads feature reports from the interrupt
> IN endpoint at all, so skipping that poll cannot break any feature
> reports on any device. This is also mentioned in my patch description:
>
> "Feature reports and hidraw output keep working over the control and
> OUT endpoints, so the interface is otherwise unaffected."
>
> Regarding a manufacturer using in endpoint without an input report, even
> today, the HID core would drop that data before it reaches hidraw:
> __hid_input_report() bails when hid_get_report() finds no matching
> report. That bail is also before the driver's ->raw_event() callback, so
> no driver or hidraw reader can currently be relying on such traffic.
Interesting, so it should noop. Muting the in endpoint would not
affect feature reports that get sent over the in endpoint? I do not
think this patch will cause regressions for Asus devices. I'm more
concerned with other ones. E.g., the Legion Go S has a malformed
report, and I do not recall which endpoints it uses. Then, the Win 5
also does a mix. Those are two devices I'd be concerned with, but
there are a myriad of other hid devices this could affect.
I'd rather if possible the fix goes towards fixing the underlying
issue that blocks processing inputs from other devices. This way, even
for devices with an actual input report that is infrequent, this issue
stops being present, even if the blocking wouldn't have been
perceptible.
I can reproduce on my Z13 in the following days.
Best,
Antheas
> >> While I mostly agree with this it is also true that the general direction
> >> for the kernel (especially lately) has been to not do out-of-spec things
> >> at least by default.
> >>
> >> If things really regress it's expected to do so only an very few specific
> >> devices with a buggy firmware, and we can think of something different
> >> for those (hopefully very few ones).
> >>
> >> Perhaps someone concerned with security might be interested in what
> >> we have because it doesn't look very normal.
> >>
> >> Note that below I have written a few ideas that maybe are worth
> >
> > The degradation would be silent.
> >
> >> looking into.
> >>>> The ASUS ROG N-Key keyboards expose a second, input-less interface used
> >>>> only for RGB control via feature reports. Opening its hidraw node (any
> >>>> hidraw reader does, including SDL/Steam Input or a plain cat) starts the
> >>> cating a hidraw causing issues would be expected, so let's focus on the former.
> >
> > Try to add spaces before and after your responses
> >
> >> Simply opening an hidraw should not trigger a delayed disconnect of that device,
> >> I don't know why you would expect this to happen nor why you would
> >> consider it acceptable. It's a bug.
> >>
> >> Focusing on userspace software exposing the bug is not a realistic option
> >> because over the time we found a good chunk of software doing that:
> >> - logitech control software (forgot the name)
> >> - open razer software
> >> - sdl
> >> - asusctl (obviously it opens the device albeit in the future I will change this)
> >>
> >> and likely more given the fact not all software was identified.
> >>> Asusctl has a bug where if you add the quirk that separates the event
> >>> nodes per hid, this bug is reproduced as well. I chucked it to
> >>> complicated threading getting out of control. It is the reason we
> >>> skipped that patch that was in my series.
> >> I found and solved the bug already. Regardless the issue remains:
> >> Even with no asusctl at all, if a user has one logitech mouse
> >> (and its control software) and a razer keyboard (and its control software)
> >> the asus N-Key device will start an endless disconnect-reconnect loop.
> >>
> >> Any combination of two or more of those tools will trigger the issue
> >> on some devices (weirdly enough not every model is affected):
> >>
> >> this is not good.
> >>> Now, you say SDL/Steam do a spurious read as well, can you identify
> >>> the codepath so we can look into it? What devices are affected? The
> >>> early return fixes a warning on the Z13, but it also feeds through the
> >>> universal lamp interface on the new Xbox Allies. Is this a bug on
> >>> those devices or keyboards? If yes, it could be caused by userspace
> >>> hanging on that node
>
> Affected devices include the ROG STRIX 2025 lineup: Scar 16/18
> (G635L/G835L) and G16/G18 (G615L/G815L). My patch has been tested on
> both Scar 18 and G18. Additionally a user with a Scar 18 2024 model
> (G634JZR) has reported the issue as well; they were unable to
> participate in testing but reproduce the issue with the same cat command
> (reproduction command provided below). It is likely the G16/G18 of 2024
> will also be affected. Models prior to 2024 appear unaffected so far.
>
> A user with an Xbox Ally X has tested this for me as well as of writing
> this email. So we are able to confirm that this device is unaffected and
> no regressions are noticed on that device from my patch, including the
> lamp/RGB controls.
>
> I do not have access to a Z13 at the moment. If you have one, it would
> be very helpful for me if you could test for any regressions on that
> device and if the device is affected by the bug, and whether or not this
> patch fixes the issue.
>
> I would also like to take this opportunity to mention that the 3 testers
> and I are all daily driving a kernel with this patch applied, and over
> the last few days, have noticed no issues with any devices.
>
> >> Sure, and I agree with you that fixing all userspace tools is desirable
> >> but it's also unfeasible to fix them all, if we managed to do that
> >> there will be years before everyone receives a fixed version of every
> >> affected software and even then a core issue would remain:
> >> linux tries to poll something it can't have anything out from.
> >>
> >> I am much more oriented on the fact that kernel shouldn't
> >> be doing weird things (at least not by default) so this has to
> >> somehow be stopped regardless of how well userspace behaves.
> >
> > The kernel is not doing weird things and I also did not ask you to fix
> > all userspace software. I asked for a reproduction scenario, as it is
> > not covered in the patch description. Relooking at the patch today, I
> > also do not understand what it does fully.
>
> The reproduction scenario is in the patch description:
>
> "Opening its hidraw node (any hidraw reader does, including SDL/Steam
> Input or a plain cat) starts the pointless IN poll and keypress reports
> on the keyboard interface get dropped for as long as the node stays
> open: a lost key-down drops a letter, a lost key-up leaves the key stuck."
>
> i.e. run "sudo timeout 15 cat /dev/hidrawX" against the N-Key RGB
> interface, then type on the internal keyboard.
>
> >
> > It skips enabling input interrupts (but not only that) for devices
> > that have no input reports. So the kernel behavior will depend on the
> > feature descriptor moving forward.
>
> What the patch does is the last paragraph of the description:
>
> "Skip the poll in usbhid_open() when the device has no input reports."
>
> Interrupt IN endpoint on a device with 0 input reports isn't doing
> anything anyway. The other things the early return skips only matter
> when input is possible.
>
> >
> > And that fixes a hang on the affected devices because enabling
> > interrupts on an endpoint without periodic input reports blocks a
> > parallel endpoint that does have input reports?
> >
> > I would like this fix to target the actual cause that causes the block
> > but it is not clear to me what that is or what is affected.
>
> As per my investigations with usbmon, I can see that the keyboard
> interface's input reports never reach the URB layer while the RGB
> interface is being polled. From the patch description:
>
> "usbmon shows the dropped reports never reach the URB layer"
>
> So the blocking likely happens inside the device's firmware, and not in
> the kernel, so the kernel cannot fix that part. What the kernel can do
> is to stop arming the IN URB on an endpoint that as per its own
> descriptor, can never produce data.
>
> >
> > Antheas
> >
> >> If you have better ideas on how to fix the kernel we would
> >> like to hear those as well.
> >>
> >> Best regards,
> >> Denis
> >>> Antheas
> >>>
> >>>> pointless IN poll and keypress reports on the keyboard interface get
> >>>> dropped for as long as the node stays open: a lost key-down drops a
> >>>> letter, a lost key-up leaves the key stuck. usbmon shows the dropped
> >>>> reports never reach the URB layer.
> >>>>
> >>>> The useless poll itself is long-standing; commit 4ac74ea68f64 ("HID:
> >>>> asus: early return for ROG devices") is what exposes it on these
> >>>> devices by keeping the input-less interface alive instead of ejecting
> >>>> it, so its hidraw node can be opened and the poll started.
> >>>>
> >>>> Skip the poll in usbhid_open() when the device has no input reports.
> >>>> Feature reports and hidraw output keep working over the control and OUT
> >>>> endpoints, so the interface is otherwise unaffected.
> >> I will write my review here to avoid forking the discussion:
> >>
> >> I agree with the general idea but perhaps we can avoid
> >> some hid devices to ever get HID_QUIRK_ALWAYS_POLL
> >> and that might be enough to skip the problematic code?
> >>
> >> Maybe there is value in doing this with a quirk flag in hid-asus.c
> >> affecting the least amount of devices?
> >>
> >> Or maybe just prevent devices with no data possibly coming out
> >> to ever get HID_QUIRK_ALWAYS_POLL?
>
> Thank you for the review!
>
> I would like to also highlight one thing here; the HID_QUIRK_ALWAYS_POLL
> is not given to this specific device. It was already in the if
> condition, for the devices that do use it; my change only ORs a second
> independent condition into it. So keeping devices away from that quirk
> would not change anything here.
>
> Adding a quirk flag for this specific device is something I too have
> considered and will be happy to change it like so if Jiri or Benjamin
> feel it is more appropriate. My reasoning for taking the current route
> is that it would prevent any hidden issues that might arise similarly,
> and fix the whole class of this issue rather than for one vendor when
> the likelihood of a regression is very low from skipping interrupt IN
> polling if a device doesn't have input reports in the first place.
>
> Best Regards,
> Yaseen
>
> >>
> >> For how to best do this we will need to hear what Jiri and
> >> Benjamin have to say but if they think the proposed solution
> >> is the correct solution:
> >>
> >> Reviewed-by: Denis Benato <denis.benato@linux.dev>
> >>>> Fixes: 4ac74ea68f64 ("HID: asus: early return for ROG devices")
> >>>> Tested-by: Kerim Kabirov <the.privat33r+linux@pm.me>
> >>>> Tested-by: GameBurrow <gameburrow@pm.me>
> >>>> Signed-off-by: Ahmed Yaseen <yaseen@ghoul.dev>
> >>>> ---
> >>>> drivers/hid/usbhid/hid-core.c | 3 ++-
> >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> >>>> index 96b0181cf819..90a8b34d9305 100644
> >>>> --- a/drivers/hid/usbhid/hid-core.c
> >>>> +++ b/drivers/hid/usbhid/hid-core.c
> >>>> @@ -688,7 +688,8 @@ static int usbhid_open(struct hid_device *hid)
> >>>>
> >>>> set_bit(HID_OPENED, &usbhid->iofl);
> >>>>
> >>>> - if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
> >>>> + if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) ||
> >>>> + list_empty(&hid->report_enum[HID_INPUT_REPORT].report_list)) {
> >>>> res = 0;
> >>>> goto Done;
> >>>> }
> >>>> --
> >>>> 2.54.0
> >>>>
> >>>>
> >>>>
>
>
>
^ permalink raw reply
* Re: [PATCH] HID: usbhid: skip interrupt IN polling for devices with no input reports
From: Yaseen @ 2026-06-07 16:51 UTC (permalink / raw)
To: Antheas Kapenekakis, Denis Benato
Cc: Jiri Kosina, Benjamin Tissoires, Ilpo Järvinen,
Kerim Kabirov, GameBurrow, linux-usb, linux-input, linux-kernel
In-Reply-To: <CAGwozwFMm2o2G-fOixvu+QVXGsDC+E81+1Nsk4kQ7xbpnvhVPg@mail.gmail.com>
On 06/06/2026 18:13, Antheas Kapenekakis wrote:
> On Sat, 6 Jun 2026 at 14:42, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>>
>> On 6/5/26 14:02, Antheas Kapenekakis wrote:
>>> On Fri, 5 Jun 2026 at 13:40, Ahmed Yaseen <yaseen@ghoul.dev> wrote:
>>>> usbhid starts polling a device's interrupt IN endpoint on open
>>>> (usbhid_open() -> hid_start_in()). If the report descriptor declares no
>>>> input reports there is nothing to read there, so the poll is useless,
>>>> and on some composite devices it is also harmful.
>>> If it did have input reports, would starting the polling still cause
>>> issues? Because if it would, the issue is in the polling itself.
>> So far we haven't found an asus device that has more than one interface
>> that supports reading data out of if.
>>> Given the creativity of manufacturers when implementing hid protocols,
>>> I find it certain that they do use the in endpoint even without input
>>> reports. E.g., for feature reports. This could cause regressions.
The ASUS ROG N-Key Device does have feature reports. They are used for
RGB control on the keyboard. I have confirmed this with a test by not
registering the hidraw node for this interface at all and noted that RGB
stops working after. So hiding or ejecting this interface is not an
option. Therefore, after this patch, I myself, together with Kerim and
GameBurrow have paid attention explicitly to ensure there are no
regressions to the LED controls, while fixing the keyboard issue.
Also worth noting that feature reports travel over EP0 via
usbhid_{get,set}_raw_report() in both directions. The interrupt IN
endpoint is only ever used to receive input reports: hid_irq_in() passes
everything it gets to hid_safe_input_report(HID_INPUT_REPORT, ...).
There is no code in usbhid that reads feature reports from the interrupt
IN endpoint at all, so skipping that poll cannot break any feature
reports on any device. This is also mentioned in my patch description:
"Feature reports and hidraw output keep working over the control and
OUT endpoints, so the interface is otherwise unaffected."
Regarding a manufacturer using in endpoint without an input report, even
today, the HID core would drop that data before it reaches hidraw:
__hid_input_report() bails when hid_get_report() finds no matching
report. That bail is also before the driver's ->raw_event() callback, so
no driver or hidraw reader can currently be relying on such traffic.
>> While I mostly agree with this it is also true that the general direction
>> for the kernel (especially lately) has been to not do out-of-spec things
>> at least by default.
>>
>> If things really regress it's expected to do so only an very few specific
>> devices with a buggy firmware, and we can think of something different
>> for those (hopefully very few ones).
>>
>> Perhaps someone concerned with security might be interested in what
>> we have because it doesn't look very normal.
>>
>> Note that below I have written a few ideas that maybe are worth
>
> The degradation would be silent.
>
>> looking into.
>>>> The ASUS ROG N-Key keyboards expose a second, input-less interface used
>>>> only for RGB control via feature reports. Opening its hidraw node (any
>>>> hidraw reader does, including SDL/Steam Input or a plain cat) starts the
>>> cating a hidraw causing issues would be expected, so let's focus on the former.
>
> Try to add spaces before and after your responses
>
>> Simply opening an hidraw should not trigger a delayed disconnect of that device,
>> I don't know why you would expect this to happen nor why you would
>> consider it acceptable. It's a bug.
>>
>> Focusing on userspace software exposing the bug is not a realistic option
>> because over the time we found a good chunk of software doing that:
>> - logitech control software (forgot the name)
>> - open razer software
>> - sdl
>> - asusctl (obviously it opens the device albeit in the future I will change this)
>>
>> and likely more given the fact not all software was identified.
>>> Asusctl has a bug where if you add the quirk that separates the event
>>> nodes per hid, this bug is reproduced as well. I chucked it to
>>> complicated threading getting out of control. It is the reason we
>>> skipped that patch that was in my series.
>> I found and solved the bug already. Regardless the issue remains:
>> Even with no asusctl at all, if a user has one logitech mouse
>> (and its control software) and a razer keyboard (and its control software)
>> the asus N-Key device will start an endless disconnect-reconnect loop.
>>
>> Any combination of two or more of those tools will trigger the issue
>> on some devices (weirdly enough not every model is affected):
>>
>> this is not good.
>>> Now, you say SDL/Steam do a spurious read as well, can you identify
>>> the codepath so we can look into it? What devices are affected? The
>>> early return fixes a warning on the Z13, but it also feeds through the
>>> universal lamp interface on the new Xbox Allies. Is this a bug on
>>> those devices or keyboards? If yes, it could be caused by userspace
>>> hanging on that node
Affected devices include the ROG STRIX 2025 lineup: Scar 16/18
(G635L/G835L) and G16/G18 (G615L/G815L). My patch has been tested on
both Scar 18 and G18. Additionally a user with a Scar 18 2024 model
(G634JZR) has reported the issue as well; they were unable to
participate in testing but reproduce the issue with the same cat command
(reproduction command provided below). It is likely the G16/G18 of 2024
will also be affected. Models prior to 2024 appear unaffected so far.
A user with an Xbox Ally X has tested this for me as well as of writing
this email. So we are able to confirm that this device is unaffected and
no regressions are noticed on that device from my patch, including the
lamp/RGB controls.
I do not have access to a Z13 at the moment. If you have one, it would
be very helpful for me if you could test for any regressions on that
device and if the device is affected by the bug, and whether or not this
patch fixes the issue.
I would also like to take this opportunity to mention that the 3 testers
and I are all daily driving a kernel with this patch applied, and over
the last few days, have noticed no issues with any devices.
>> Sure, and I agree with you that fixing all userspace tools is desirable
>> but it's also unfeasible to fix them all, if we managed to do that
>> there will be years before everyone receives a fixed version of every
>> affected software and even then a core issue would remain:
>> linux tries to poll something it can't have anything out from.
>>
>> I am much more oriented on the fact that kernel shouldn't
>> be doing weird things (at least not by default) so this has to
>> somehow be stopped regardless of how well userspace behaves.
>
> The kernel is not doing weird things and I also did not ask you to fix
> all userspace software. I asked for a reproduction scenario, as it is
> not covered in the patch description. Relooking at the patch today, I
> also do not understand what it does fully.
The reproduction scenario is in the patch description:
"Opening its hidraw node (any hidraw reader does, including SDL/Steam
Input or a plain cat) starts the pointless IN poll and keypress reports
on the keyboard interface get dropped for as long as the node stays
open: a lost key-down drops a letter, a lost key-up leaves the key stuck."
i.e. run "sudo timeout 15 cat /dev/hidrawX" against the N-Key RGB
interface, then type on the internal keyboard.
>
> It skips enabling input interrupts (but not only that) for devices
> that have no input reports. So the kernel behavior will depend on the
> feature descriptor moving forward.
What the patch does is the last paragraph of the description:
"Skip the poll in usbhid_open() when the device has no input reports."
Interrupt IN endpoint on a device with 0 input reports isn't doing
anything anyway. The other things the early return skips only matter
when input is possible.
>
> And that fixes a hang on the affected devices because enabling
> interrupts on an endpoint without periodic input reports blocks a
> parallel endpoint that does have input reports?
>
> I would like this fix to target the actual cause that causes the block
> but it is not clear to me what that is or what is affected.
As per my investigations with usbmon, I can see that the keyboard
interface's input reports never reach the URB layer while the RGB
interface is being polled. From the patch description:
"usbmon shows the dropped reports never reach the URB layer"
So the blocking likely happens inside the device's firmware, and not in
the kernel, so the kernel cannot fix that part. What the kernel can do
is to stop arming the IN URB on an endpoint that as per its own
descriptor, can never produce data.
>
> Antheas
>
>> If you have better ideas on how to fix the kernel we would
>> like to hear those as well.
>>
>> Best regards,
>> Denis
>>> Antheas
>>>
>>>> pointless IN poll and keypress reports on the keyboard interface get
>>>> dropped for as long as the node stays open: a lost key-down drops a
>>>> letter, a lost key-up leaves the key stuck. usbmon shows the dropped
>>>> reports never reach the URB layer.
>>>>
>>>> The useless poll itself is long-standing; commit 4ac74ea68f64 ("HID:
>>>> asus: early return for ROG devices") is what exposes it on these
>>>> devices by keeping the input-less interface alive instead of ejecting
>>>> it, so its hidraw node can be opened and the poll started.
>>>>
>>>> Skip the poll in usbhid_open() when the device has no input reports.
>>>> Feature reports and hidraw output keep working over the control and OUT
>>>> endpoints, so the interface is otherwise unaffected.
>> I will write my review here to avoid forking the discussion:
>>
>> I agree with the general idea but perhaps we can avoid
>> some hid devices to ever get HID_QUIRK_ALWAYS_POLL
>> and that might be enough to skip the problematic code?
>>
>> Maybe there is value in doing this with a quirk flag in hid-asus.c
>> affecting the least amount of devices?
>>
>> Or maybe just prevent devices with no data possibly coming out
>> to ever get HID_QUIRK_ALWAYS_POLL?
Thank you for the review!
I would like to also highlight one thing here; the HID_QUIRK_ALWAYS_POLL
is not given to this specific device. It was already in the if
condition, for the devices that do use it; my change only ORs a second
independent condition into it. So keeping devices away from that quirk
would not change anything here.
Adding a quirk flag for this specific device is something I too have
considered and will be happy to change it like so if Jiri or Benjamin
feel it is more appropriate. My reasoning for taking the current route
is that it would prevent any hidden issues that might arise similarly,
and fix the whole class of this issue rather than for one vendor when
the likelihood of a regression is very low from skipping interrupt IN
polling if a device doesn't have input reports in the first place.
Best Regards,
Yaseen
>>
>> For how to best do this we will need to hear what Jiri and
>> Benjamin have to say but if they think the proposed solution
>> is the correct solution:
>>
>> Reviewed-by: Denis Benato <denis.benato@linux.dev>
>>>> Fixes: 4ac74ea68f64 ("HID: asus: early return for ROG devices")
>>>> Tested-by: Kerim Kabirov <the.privat33r+linux@pm.me>
>>>> Tested-by: GameBurrow <gameburrow@pm.me>
>>>> Signed-off-by: Ahmed Yaseen <yaseen@ghoul.dev>
>>>> ---
>>>> drivers/hid/usbhid/hid-core.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>>>> index 96b0181cf819..90a8b34d9305 100644
>>>> --- a/drivers/hid/usbhid/hid-core.c
>>>> +++ b/drivers/hid/usbhid/hid-core.c
>>>> @@ -688,7 +688,8 @@ static int usbhid_open(struct hid_device *hid)
>>>>
>>>> set_bit(HID_OPENED, &usbhid->iofl);
>>>>
>>>> - if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
>>>> + if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) ||
>>>> + list_empty(&hid->report_enum[HID_INPUT_REPORT].report_list)) {
>>>> res = 0;
>>>> goto Done;
>>>> }
>>>> --
>>>> 2.54.0
>>>>
>>>>
>>>>
^ permalink raw reply
* Re: [PATCH] Input: appletouch - fix offset caused by smoothing
From: sashiko-bot @ 2026-06-07 16:17 UTC (permalink / raw)
To: Icenowy Zheng; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260607160311.2192061-1-zhengxingda@iscas.ac.cn>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Removing the 4-sensor offset causes valid edge touches to evaluate to 0, which the caller treats as dropped events.
Pre-existing issues:
- [High] Race condition between `atp_close()` and `atp_reinit()` causes use-after-free.
- [High] A 32-bit signed integer overflow in absolute coordinate calculation causes negative cursor jumps.
--
commit 39d19e6e224959faeb0b8190e5d07e558b5470fe
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Input: appletouch - fix offset caused by smoothing
This commit subtracts 4 from the index when accumulating smoothed values
to compensate for the 4-sensor margin introduced by smoothing. This aims
to keep the calculated coordinates within the maximum reported range.
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index eebeb57515e1c..174762d59f87b 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -415,7 +415,7 @@ static int atp_calculate_abs(struct atp *dev, int offset, int nb_sensors,
> * by scale. Mostly noise.
> */
> if ((dev->smooth[i] >> ATP_SCALE) > 0) {
> - pcum += dev->smooth[i] * i;
> + pcum += dev->smooth[i] * (i - 4);
[Severity: High]
Does this change cause valid touches on the first sensor to be dropped?
If a touch is centered exactly on the first sensor (index 4 in the smooth
array), the smoothed values will be perfectly symmetric around index 4. The
new (i - 4) multiplier will cause the left and right sides to cancel out
exactly, making pcum equal 0.
The callers use 0 as a sentinel for no touch:
drivers/input/mouse/appletouch.c:atp_complete_geyser_3_4() {
...
if (x && y && fingers == dev->fingers_old) {
...
}
Will this cause valid edge touches to be mistakenly treated as finger
lift-offs and create dead zones on the trackpad edges?
> psum += dev->smooth[i];
> }
> }
>
> if (psum > 0) {
> *z = psum >> ATP_SCALE; /* Scale down pressure output. */
> return pcum * fact / psum;
[Severity: High]
This is a pre-existing issue, but does this expression overflow a 32-bit
signed integer?
In atp_calculate_abs(), the accumulator pcum can reach large values for
multi-finger touches at the far edges of the trackpad. If pcum reaches
around 100,000,000, multiplying it by fact (which is 64 for xfact) yields
over 6 billion.
Does this silently wrap to a massive negative number, causing the cursor to
violently jerk to the top or left edge when reported to userspace?
[ ... ]
[Severity: High]
This is a pre-existing issue, but can a race condition between atp_close()
and atp_reinit() cause a use-after-free?
In atp_close(), usb_kill_urb() is called before cancel_work_sync():
drivers/input/mouse/appletouch.c:atp_close() {
...
usb_kill_urb(dev->urb);
cancel_work_sync(&dev->work);
...
}
If the worker atp_reinit() is executing concurrently, can it submit the URB
after usb_kill_urb() has finished? The cancel_work_sync() simply waits for
the worker to finish and returns, leaving the URB actively running after
the device is closed.
Furthermore, atp_disconnect() does not call cancel_work_sync() if the
device was already closed:
drivers/input/mouse/appletouch.c:atp_disconnect() {
...
usb_kill_urb(dev->urb);
input_unregister_device(dev->input);
usb_free_coherent(...);
usb_free_urb(dev->urb);
kfree(dev);
}
Since a running URB can schedule the work again, could atp_disconnect()
free the dev structure while the worker or URB completion handler is still
running?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260607160311.2192061-1-zhengxingda@iscas.ac.cn?part=1
^ permalink raw reply
* [PATCH] Input: appletouch - fix offset caused by smoothing
From: Icenowy Zheng @ 2026-06-07 16:03 UTC (permalink / raw)
To: Dmitry Torokhov, Kees Cook, Johan Hovold, Clinton Sprain,
Henrik Rydberg
Cc: linux-input, linux-kernel, Icenowy Zheng, stable
The smoothing code introduces 4 sensors of margin on each side of the
input, which is not compensated and leads to a offset of 4 * factor added
to the calculated coordinate values.
However, the maximum value reported as ABS axis parameters are
calculated with the sensor count multiplied by the factor, which leads
to the coordinate values going beyond the maximum value and get ignored
by libinput.
Fix this by subtracting the index by 4 when accumlating the smoothed
values. This makes the reported coordinates in-range again, and libinput
stops to drop touch events in the right and down side of the trackpad.
Fixes: 739204bc9577 ("Input: appletouch - implement sensor data smoothing")
Cc: stable@vger.kernel.org
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
drivers/input/mouse/appletouch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index eebeb57515e1c..174762d59f87b 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -415,7 +415,7 @@ static int atp_calculate_abs(struct atp *dev, int offset, int nb_sensors,
* by scale. Mostly noise.
*/
if ((dev->smooth[i] >> ATP_SCALE) > 0) {
- pcum += dev->smooth[i] * i;
+ pcum += dev->smooth[i] * (i - 4);
psum += dev->smooth[i];
}
}
--
2.52.0
^ permalink raw reply related
* Re: [git pull] Input updates for v7.1-rc6
From: pr-tracker-bot @ 2026-06-07 15:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <aiT7-939zBREw3FG@google.com>
The pull request you sent on Sat, 6 Jun 2026 22:05:16 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v7.1-rc6
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/33d8d8ec31b591952ded59d596c2f3db45004937
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox