* [PATCH v4 02/11] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
If connecting a hid_device with bus field indicating BUS_SPI print out
"SPI" in the debug print.
Macro sets the bus field to BUS_SPI and uses arguments to set vendor
product fields.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/hid-core.c | 3 +++
include/linux/hid.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5b3a8ca2fcb..813c9c743ccd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2316,6 +2316,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
case BUS_I2C:
bus = "I2C";
break;
+ case BUS_SPI:
+ bus = "SPI";
+ break;
case BUS_SDW:
bus = "SOUNDWIRE";
break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dce862cafbbd..957f322a0ebd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -786,6 +786,8 @@ struct hid_descriptor {
.bus = BUS_BLUETOOTH, .vendor = (ven), .product = (prod)
#define HID_I2C_DEVICE(ven, prod) \
.bus = BUS_I2C, .vendor = (ven), .product = (prod)
+#define HID_SPI_DEVICE(ven, prod) \
+ .bus = BUS_SPI, .vendor = (ven), .product = (prod)
#define HID_REPORT_ID(rep) \
.report_type = (rep)
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 01/11] Documentation: Correction in HID output_report callback description.
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
Originally output_report callback was described as must-be asynchronous,
but that is not the case in some implementations, namely i2c-hid.
Correct the documentation to say that it may be asynchronous.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Documentation/hid/hid-transport.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/hid/hid-transport.rst b/Documentation/hid/hid-transport.rst
index 6f1692da296c..2008cf432af1 100644
--- a/Documentation/hid/hid-transport.rst
+++ b/Documentation/hid/hid-transport.rst
@@ -327,8 +327,8 @@ The available HID callbacks are:
Send raw output report via intr channel. Used by some HID device drivers
which require high throughput for outgoing requests on the intr channel. This
- must not cause SET_REPORT calls! This must be implemented as asynchronous
- output report on the intr channel!
+ must not cause SET_REPORT calls! This call might be asynchronous, so the
+ caller should not expect an immediate response!
::
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 00/11] Add spi-hid transport driver
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov, Angela Czubak
This series picks up the spi-hid driver work originally started by
Microsoft. The patch breakdown has been modified and the implementation
has been refactored to address upstream feedback and testing issues. We
are submitting this as a new series while keeping the original sign-off
chain to reflect the history.
Same as the original series, there is a change to HID documentation, some
HID core changes to support a SPI device, the SPI HID transport driver,
and HID over SPI Device Tree binding. We have added the HID over SPI ACPI
support, power management, panel follower, and quirks for Ilitek touch
controllers.
Original authors: Jarrett Schultz <jaschultz@microsoft.com>,
Dmitry Antipov <dmanti@microsoft.com>
Link: https://lore.kernel.org/r/86b63b7b-afda-d7f4-7bfa-175085d5a8ef@gmail.com
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Changes in v4:
- Extended io_lock scope to protect the shid->hid pointer lifecycle
against races with the IRQ handler
- Cacheline-aligned DMA buffers, enforced 4-byte alignment
- Added error rollback for failed suspend/resume transitions
- Moved IRQ request to probe with IRQF_NO_AUTOEN (disabled by default)
- DT Bindings & OF Driver:
- Required a device-specific compatible string in the schema.
- Added description to explain why opcodes and addresses properties
need to be defined in the schema
- Added `spi-peripheral-props.yaml` reference and switched to
`unevaluatedProperties: false`.
- Added fallback to default timing parameters in OF driver if match
data is missing.
- Link to v3: https://lore.kernel.org/r/20260402-send-upstream-v3-0-6091c458d357@chromium.org
Changes in v3:
- Add io_lock init
- Relocate tracepoints to drivers/hid/spi-hid/ and fix tracepoint macros
- Add tracepoints for sync, error handling, reset, and report processing
- Clean up internal includes and fix Makefile CFLAGS
- Add more details in v2 changelog
- Link to v2: https://lore.kernel.org/r/20260324-send-upstream-v2-0-521ce8afff86@chromium.org
Changes in v2:
- Clean up DT bindings: fix formatting and remove timing and flags properties
- Update DT binding example: use a device-specific compatible and drop
reset_assert
- Simplify ACPI/OF match tables by removing ACPI_PTR/of_match_ptr
- Refactor OF driver to use match data for timing parameters instead
of DT properties
- Switch to fsleep() for delays in ACPI and OF drivers
- Drop patch 12 as it is vendor specific
- Add a lock to fix input/output concurrency race
- Link to v1: https://lore.kernel.org/r/20260303-send-upstream-v1-0-1515ba218f3d@chromium.org
---
Angela Czubak (2):
HID: spi-hid: add transport driver skeleton for HID over SPI bus
HID: spi_hid: add ACPI support for SPI over HID
Jarrett Schultz (3):
Documentation: Correction in HID output_report callback description.
HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
HID: spi_hid: add device tree support for SPI over HID
Jingyuan Liang (6):
HID: spi-hid: add spi-hid driver HID layer
HID: spi-hid: add HID SPI protocol implementation
HID: spi_hid: add spi_hid traces
dt-bindings: input: Document hid-over-spi DT schema
HID: spi-hid: add power management implementation
HID: spi-hid: add panel follower support
.../devicetree/bindings/input/hid-over-spi.yaml | 128 ++
Documentation/hid/hid-transport.rst | 4 +-
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-core.c | 3 +
drivers/hid/spi-hid/Kconfig | 45 +
drivers/hid/spi-hid/Makefile | 12 +
drivers/hid/spi-hid/spi-hid-acpi.c | 254 ++++
drivers/hid/spi-hid/spi-hid-core.c | 1521 ++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.h | 98 ++
drivers/hid/spi-hid/spi-hid-of.c | 247 ++++
drivers/hid/spi-hid/spi-hid-trace.h | 169 +++
drivers/hid/spi-hid/spi-hid.h | 46 +
include/linux/hid.h | 2 +
14 files changed, 2531 insertions(+), 2 deletions(-)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260212-send-upstream-75f6fd9ed92e
Best regards,
--
Jingyuan Liang <jingyliang@chromium.org>
^ permalink raw reply
* Re: [PATCH] Input: cyapa: fix runtime PM refcount leak in cyapa_update_rt_suspend_scanrate()
From: sashiko-bot @ 2026-06-09 0:57 UTC (permalink / raw)
To: Wentao Liang; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260609004617.175476-1-vulab@iscas.ac.cn>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] System suspend and resume callbacks unconditionally dereference `cyapa->input->mutex`, leading to a kernel crash if the device is bound but not operational.
- [High] Explicitly unregistering a `devm`-managed input device during firmware updates leads to devres leaks and use-after-free on driver unbind.
- [High] The driver calls the core helper `input_device_enabled()` without holding `input_dev->mutex`, violating core locking rules.
--
commit fd1e73fff3322eda5030f5978379f809f689c870
Author: Wentao Liang <vulab@iscas.ac.cn>
Input: cyapa: fix runtime PM refcount leak in cyapa_update_rt_suspend_scanrate()
The commit fixes a runtime PM reference leak in
cyapa_update_rt_suspend_scanrate(). It adds a missing
pm_runtime_put_sync_autosuspend() call on the error path when
mutex_lock_interruptible() fails.
Cc: stable@vger.kernel.org
Fixes: 672865080a8f ("Input: cyapa - add runtime power management support")
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index 47000c30e4d8b..c1c23fcf494a9 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -901,8 +901,10 @@ static ssize_t cyapa_update_rt_suspend_scanrate(struct device *dev,
> pm_runtime_get_sync(dev);
>
> error = mutex_lock_interruptible(&cyapa->state_sync_lock);
> - if (error)
> + if (error) {
> + pm_runtime_put_sync_autosuspend(dev);
> return error;
> + }
>
> cyapa->runtime_suspend_sleep_time = min_t(u16, time, 1000);
> cyapa->runtime_suspend_power_mode =
[Severity: High]
This isn't a bug introduced by this patch, but looking at cyapa_suspend() and
cyapa_resume(), they unconditionally acquire the input device mutex:
drivers/input/mouse/cyapa.c:cyapa_suspend() {
...
error = mutex_lock_interruptible(&cyapa->input->mutex);
...
}
drivers/input/mouse/cyapa.c:cyapa_resume() {
...
mutex_lock(&cyapa->input->mutex);
...
}
Could this lead to a kernel crash if the trackpad is not operational during
probe (e.g., stuck in bootloader mode) or during a firmware update?
In this scenario, cyapa_create_input_dev() is skipped and cyapa->input remains
NULL. Since cyapa_probe() still returns 0, the device is successfully bound
and the suspend/resume callbacks will dereference the NULL cyapa->input pointer.
[Severity: High]
This is a pre-existing issue, but does cyapa_update_fw_store() leak devres
memory and cause a use-after-free on driver unbind?
cyapa_create_input_dev() allocates the input device using a devm wrapper:
input = devm_input_allocate_device(dev);
This automatically queues a devres action to unregister the device upon driver
unbind. However, cyapa_update_fw_store() explicitly calls:
input_unregister_device(cyapa->input);
This unregisters and frees the device, but the devres action attached to the
parent device is not cancelled.
When the driver is eventually unbound, will the devres framework attempt to
execute input_unregister_device() on the already-freed cyapa->input pointer?
[Severity: High]
This is a pre-existing issue, but does cyapa_enable_irq_for_cmd() call
input_device_enabled() without holding the input device mutex?
drivers/input/mouse/cyapa.c:cyapa_enable_irq_for_cmd() {
...
if (!input || !input_device_enabled(input)) {
...
}
The input subsystem requires input_device_enabled() to be called while holding
input_dev->mutex. This function is invoked from multiple sysfs attribute
handlers, such as cyapa_calibrate_store(), while only holding the driver's
internal state_sync_lock.
Could this cause a lockdep splat upon sysfs interaction?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609004617.175476-1-vulab@iscas.ac.cn?part=1
^ permalink raw reply
* Re: [linux-5.10.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc
From: Sasha Levin @ 2026-06-09 0:51 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: Sasha Levin, stable, Vicki Pfau, Jiri Kosina
In-Reply-To: <20260608100236.2781796-1-lee@kernel.org>
> [linux-5.10.y 1/3] HID: core: Add printk_ratelimited variants to hid_warn() etc
> [linux-5.10.y 2/3] HID: pass the buffer size to hid_report_raw_event
> [linux-5.10.y 3/3] HID: core: Fix size_t specifier in hid_report_raw_event()
Whole series queued for 5.10, thanks.
--
Thanks,
Sasha
^ permalink raw reply
* [PATCH] Input: cyapa: fix runtime PM refcount leak in cyapa_update_rt_suspend_scanrate()
From: Wentao Liang @ 2026-06-09 0:46 UTC (permalink / raw)
To: dmitry.torokhov
Cc: sakari.ailus, linux-input, linux-kernel, Wentao Liang, stable
pm_runtime_get_sync() at line 901 increments the runtime PM usage
count. If mutex_lock_interruptible() fails at line 903, the function
returns the error immediately at line 905 without calling
pm_runtime_put_sync_autosuspend(), leaking the PM reference and
preventing the device from ever entering runtime suspend.
Add pm_runtime_put_sync_autosuspend(dev) on the error path before
returning.
Cc: stable@vger.kernel.org
Fixes: 672865080a8f ("Input: cyapa - add runtime power management support")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/input/mouse/cyapa.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
index 6e0d956617a1..5edf4fdbb373 100644
--- a/drivers/input/mouse/cyapa.c
+++ b/drivers/input/mouse/cyapa.c
@@ -901,8 +901,10 @@ static ssize_t cyapa_update_rt_suspend_scanrate(struct device *dev,
pm_runtime_get_sync(dev);
error = mutex_lock_interruptible(&cyapa->state_sync_lock);
- if (error)
+ if (error) {
+ pm_runtime_put_sync_autosuspend(dev);
return error;
+ }
cyapa->runtime_suspend_sleep_time = min_t(u16, time, 1000);
cyapa->runtime_suspend_power_mode =
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 0/7] HID: iio: basic clean up for usage_id
From: Maxwell Doose @ 2026-06-08 22:03 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-0-dd4a6820b674@gmail.com>
On Sat, 06 Jun 2026 17:47:41 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> Hi all,
>
> This series updates all HID IIO drivers to use 'u32' instead of
> bare 'unsigned' for the usage_id parameter.
>
> This improves type clarity and ensures consistency with kernel
> coding style, as HID usage IDs are defined as 32-bit values.
>
> No functional changes are introduced.
>
> Testing:
> - Compiled with W=1 for each patch in the series
>
> ---
> Sanjay Chitroda (7):
> iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> iio: light: hid-sensor-als: use u32 instead of unsigned
> iio: light: hid-sensor-prox: use u32 instead of unsigned
> iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
> iio: pressure: hid-sensor-press: use u32 instead of unsigned
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 6 +++---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
> drivers/iio/light/hid-sensor-als.c | 6 +++---
> drivers/iio/light/hid-sensor-prox.c | 4 ++--
> drivers/iio/orientation/hid-sensor-incl-3d.c | 6 +++---
> drivers/iio/orientation/hid-sensor-rotation.c | 6 +++---
> drivers/iio/pressure/hid-sensor-press.c | 6 +++---
> 7 files changed, 20 insertions(+), 20 deletions(-)
> ---
> base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
> change-id: 20260606-6-june-hid-iio-correct-usage-id-57ce92cb102b
>
> Best regards,
> --
> Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
>
This series:
Reviewed-by: Maxwell Doose <m32285159@gmail.com>
--
best regards,
max
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Maxwell Doose @ 2026-06-08 22:01 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Joshua Crofts, Sanjay Chitroda, Jiri Kosina, Srinivas Pandruvada,
David Lechner, Nuno Sá, Andy Shevchenko, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20260608194047.23791938@jic23-huawei>
On Mon, 8 Jun 2026 19:40:47 +0100
Jonathan Cameron <jic23@kernel.org> wrote:
> On Mon, 8 Jun 2026 08:37:38 +0200
> Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> > 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).
> >
> Here the reason for the change is even simpler. These are callbacks assigned to:
>
> struct hid_sensor_hub_callbacks {
> struct platform_device *pdev;
> int (*suspend)(struct hid_sensor_hub_device *hsdev, void *priv);
> int (*resume)(struct hid_sensor_hub_device *hsdev, void *priv);
> int (*capture_sample)(struct hid_sensor_hub_device *hsdev,
> u32 usage_id, size_t raw_len, char *raw_data,
> void *priv);
> int (*send_event)(struct hid_sensor_hub_device *hsdev, u32 usage_id,
> void *priv);
> };
>
> So given those signatures, these should always have been u32.
>
Ah then perhaps I was a bit harsh on Sanjay.
>
> I would like that clearly stated in the patch descriptions. Consistency
> is a bit too vague.
>
Agree though. But given this
Reviewed-by: Maxwell Doose <m32285159@gmail.com>
--
best regards,
max
^ permalink raw reply
* Re: [PATCH] input: ipaq-micro-keys: simplify allocation
From: Rosen Penev @ 2026-06-08 20:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, open list
In-Reply-To: <aibx1y8ANiZoMHyY@google.com>
On Mon, Jun 8, 2026 at 9:49 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Rosen,
>
> On Sun, Jun 07, 2026 at 09:50:41PM -0700, Rosen Penev wrote:
> > Use a flexible array member to have a single allocation.
>
> Why does it have to be flexible? The size is known.
From a quick glance, that would be a compilation error. micro_keycodes
gets declared after ipaq_micro_keys
From looking at a bunch of struct definitions, [ARRAY_SIZE(foo)] seems
to be atypical.
>
> >
> > 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);
>
> Please keep to the limits for the kernel. 100 columns is acceptable when
> leads to better readability, but I prefer 80 when it makes sense.
Seems I forgot to run git clang-format here. Will resend.
>
> checkpatch.pl would have warned you.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: d3z @ 2026-06-08 20:02 UTC (permalink / raw)
To: even.xu, xinpeng.sun, jikos, bentiss
Cc: Danny D ., linux-input, linux-kernel, abhishektamboli9,
sakari.ailus
In-Reply-To: <IA1PR11MB6098E927297E716FA72FDD04F41C2@IA1PR11MB6098.namprd11.prod.outlook.com>
From: Danny D. <d3z.the.dev@gmail.com>
Hi Even,
Thanks for the review, and for working through this together.
Danny
^ permalink raw reply
* Re: [PATCH] HID: hiddev: Use kref to track struct hiddev lifetime
From: Dmitry Torokhov @ 2026-06-08 18:44 UTC (permalink / raw)
To: Heitor Alves de Siqueira
Cc: Jiri Kosina, Benjamin Tissoires, kernel-dev, linux-usb,
linux-input, linux-kernel, syzbot+563191a4939ddbfe73d4
In-Reply-To: <20260608-hiddev_kref-v1-1-cd240c95423f@igalia.com>
Hi Heitor,
On Mon, Jun 08, 2026 at 01:33:03PM -0300, Heitor Alves de Siqueira wrote:
> If a USB HID device is disconnected while userspace still holds the
> hiddev node open, hiddev_disconnect() and hiddev_release() can race on
> the embedded existancelock mutex. Syzbot has triggered this with kfree()
> happening during the mutex slow path.
>
> Fix by introducing a kref in struct hiddev, and moving kfree() into a
> dedicated release callback. This way, struct hiddev will only be freed
> after both hiddev_release() and hiddev_disconnect() are done.
This looks like a common issue with usb_register_dev() that does not
allow tying the lifetime of the created device, lifetime of user of the
created device, and userspace accessing it. Ideally the class device
would be embedded into struct hiddev, and tie its lifetime with lifetime
of the chardev associated with it and userspace accessors using it. tie
its lifetime with lifetime of the chardev associated with it and
userspace accessors using it. See cdev_device_add() and how it is being
used by multiple subsystems and how they handle class devices.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/7] HID: iio: basic clean up for usage_id
From: Jonathan Cameron @ 2026-06-08 18:41 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, 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-0-dd4a6820b674@gmail.com>
On Sat, 06 Jun 2026 17:47:41 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> Hi all,
>
> This series updates all HID IIO drivers to use 'u32' instead of
> bare 'unsigned' for the usage_id parameter.
>
> This improves type clarity and ensures consistency with kernel
> coding style, as HID usage IDs are defined as 32-bit values.
Other than adding some more detail to the consistency part
(make sure to mention the callback signatures), this looks fine to
me.
Jonathan
>
> No functional changes are introduced.
>
> Testing:
> - Compiled with W=1 for each patch in the series
>
> ---
> Sanjay Chitroda (7):
> iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> iio: light: hid-sensor-als: use u32 instead of unsigned
> iio: light: hid-sensor-prox: use u32 instead of unsigned
> iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
> iio: pressure: hid-sensor-press: use u32 instead of unsigned
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 6 +++---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
> drivers/iio/light/hid-sensor-als.c | 6 +++---
> drivers/iio/light/hid-sensor-prox.c | 4 ++--
> drivers/iio/orientation/hid-sensor-incl-3d.c | 6 +++---
> drivers/iio/orientation/hid-sensor-rotation.c | 6 +++---
> drivers/iio/pressure/hid-sensor-press.c | 6 +++---
> 7 files changed, 20 insertions(+), 20 deletions(-)
> ---
> base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
> change-id: 20260606-6-june-hid-iio-correct-usage-id-57ce92cb102b
>
> Best regards,
> --
> Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Jonathan Cameron @ 2026-06-08 18:40 UTC (permalink / raw)
To: Joshua Crofts
Cc: Maxwell Doose, Sanjay Chitroda, Jiri Kosina, Srinivas Pandruvada,
David Lechner, Nuno Sá, Andy Shevchenko, linux-input,
linux-iio, linux-kernel
In-Reply-To: <CALoEA-yiDwjkKxUE3B=qiTdBhbvinfq-UN8h1H-HysXGm0prSg@mail.gmail.com>
On Mon, 8 Jun 2026 08:37:38 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> 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).
>
Here the reason for the change is even simpler. These are callbacks assigned to:
struct hid_sensor_hub_callbacks {
struct platform_device *pdev;
int (*suspend)(struct hid_sensor_hub_device *hsdev, void *priv);
int (*resume)(struct hid_sensor_hub_device *hsdev, void *priv);
int (*capture_sample)(struct hid_sensor_hub_device *hsdev,
u32 usage_id, size_t raw_len, char *raw_data,
void *priv);
int (*send_event)(struct hid_sensor_hub_device *hsdev, u32 usage_id,
void *priv);
};
So given those signatures, these should always have been u32.
I would like that clearly stated in the patch descriptions. Consistency
is a bit too vague.
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Jonathan Cameron @ 2026-06-08 18:34 UTC (permalink / raw)
To: Joshua Crofts
Cc: Sanjay Chitroda, Jiri Kosina, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko, linux-input, linux-iio,
linux-kernel
In-Reply-To: <CALoEA-yasQhXE9o-iYL5h8w0wPMrP4f-iva-YOJ2KZV9UuGe3g@mail.gmail.com>
On Mon, 8 Jun 2026 08:41:07 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> 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.
>
It is a callback so signature needs to match.
^ permalink raw reply
* Re: [PATCH 3/8] dt-bindings: input: microchip,cap11xx: Cleanup and refine LED constraints
From: Conor Dooley @ 2026-06-08 17:44 UTC (permalink / raw)
To: Jun Yan
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260606150458.250606-4-jerrysteve1101@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4602 bytes --]
On Sat, Jun 06, 2026 at 11:03:56PM +0800, Jun Yan wrote:
> Add detailed datasheet links for all supported CAP11xx devices.
>
> Unify LED node pattern to support all chip variants in preparation
> for CAP1114 support.
>
> Remove redundant LED properties inherited from common.yaml
>
> Apply per-chip LED channel limits:
> - CAP1126: max 2 channels (0-1)
> - CAP1188: max 8 channels (0-7)
> - CAP1106, CAP12xx: no LED support
>
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> .../bindings/input/microchip,cap11xx.yaml | 51 ++++++++++++++-----
> 1 file changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> index 7ade03f1b32b..99d00f572a2d 100644
> --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> @@ -10,6 +10,15 @@ description: |
> The Microchip CAP1xxx Family of RightTouchTM multiple-channel capacitive
> touch controllers and LED drivers. The device communication via I2C only.
>
> + For more product information please see the links below:
> + CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
> + CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.pdf
> + CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.pdf
> + CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.pdf
> + CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.pdf
> + CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.pdf
> + CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.pdf
> +
> maintainers:
> - Rob Herring <robh@kernel.org>
>
> @@ -124,25 +133,21 @@ properties:
> The number of entries must correspond to the number of channels.
>
> patternProperties:
> - "^led@[0-7]$":
> + "^led@[0-9a-f]$":
> type: object
> description: CAP11xx LEDs
> $ref: /schemas/leds/common.yaml#
>
> properties:
> reg:
> - enum: [0, 1, 2, 3, 4, 5, 6, 7]
> -
> - label: true
> -
> - linux,default-trigger: true
> -
> - default-state: true
> + description: LED channel number
> + minimum: 0
> + maximum: 7
>
> required:
> - reg
>
> - additionalProperties: false
> + unevaluatedProperties: false
> Remove redundant LED properties inherited from common.yaml
But the pattern here suggests they aren't redundant, this is how you say
"these properties are the only common ones allowed". You need to explain
why other ones are valid to change this.
pw-bot: changes-requested
(a commit message change is required at least)
>
> allOf:
> - $ref: input.yaml
> @@ -158,7 +163,20 @@ allOf:
> - microchip,cap1298
> then:
> patternProperties:
> - "^led@[0-7]$": false
> + "^led@": false
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - microchip,cap1126
> + then:
> + patternProperties:
> + "^led@":
> + properties:
> + reg:
> + maximum: 1
I think these changes to permitted properties should be a standalone
patch.
Thanks,
Conor.
>
> - if:
> properties:
> @@ -183,6 +201,9 @@ additionalProperties: false
>
> examples:
> - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/leds/common.h>
> +
> i2c {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -208,19 +229,21 @@ examples:
> #size-cells = <0>;
>
> led@0 {
> - label = "cap11xx:green:usr0";
> reg = <0>;
> + function = LED_FUNCTION_INDICATOR;
> + color = <LED_COLOR_ID_GREEN>;
> };
>
> led@1 {
> - label = "cap11xx:green:usr1";
> reg = <1>;
> + function = LED_FUNCTION_INDICATOR;
> + color = <LED_COLOR_ID_GREEN>;
> };
>
> led@2 {
> - label = "cap11xx:green:alive";
> reg = <2>;
> - linux,default-trigger = "heartbeat";
> + function = LED_FUNCTION_INDICATOR;
> + color = <LED_COLOR_ID_GREEN>;
> };
> };
> };
> --
> 2.54.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] dt-bindings: input: microchip,cap11xx: Add reset-gpios property
From: Conor Dooley @ 2026-06-08 17:38 UTC (permalink / raw)
To: Jun Yan
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260606150458.250606-5-jerrysteve1101@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 7/8] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: Conor Dooley @ 2026-06-08 17:38 UTC (permalink / raw)
To: Jun Yan
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260606150458.250606-8-jerrysteve1101@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3409 bytes --]
On Sat, Jun 06, 2026 at 11:04:00PM +0800, Jun Yan wrote:
> CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
> and hardware reset support.
>
> Add the compatible string for CAP1114, include its datasheet URL,
> and update the maximum count of LED channels and linux,keycodes entries.
>
> Add description for microchip,input-threshold: CAP1114 only provides eight
> threshold entries, which does not match its total channel count.
>
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> .../bindings/input/microchip,cap11xx.yaml | 21 +++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> index e307628350c2..08233386a433 100644
> --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> @@ -12,6 +12,7 @@ description: |
>
> For more product information please see the links below:
> CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
> + CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.pdf
> CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.pdf
> CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.pdf
> CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.pdf
> @@ -26,6 +27,7 @@ properties:
> compatible:
> enum:
> - microchip,cap1106
> + - microchip,cap1114
> - microchip,cap1126
> - microchip,cap1188
> - microchip,cap1203
> @@ -62,7 +64,7 @@ properties:
>
> linux,keycodes:
> minItems: 3
> - maxItems: 8
> + maxItems: 14
> description: |
> Specifies an array of numeric keycode values to
> be used for the channels. If this property is
> @@ -122,6 +124,8 @@ properties:
> is required for a touch to be registered, making the touch sensor less
> sensitive.
> The number of entries must correspond to the number of channels.
> + CAP1114 is an exception where channels 8~14 reuse the eighth entry's
> + threshold, so counts differ.
>
> microchip,calib-sensitivity:
> $ref: /schemas/types.yaml#/definitions/uint32-array
> @@ -149,7 +153,7 @@ patternProperties:
> reg:
> description: LED channel number
> minimum: 0
> - maximum: 7
> + maximum: 10
>
> required:
> - reg
> @@ -199,6 +203,19 @@ allOf:
> reg:
> maximum: 1
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - microchip,cap1188
I don't understand this restriction, shouldn't this be
if: properties: compatible: not: contains: microchip,cap1114
so that the constraints before your change are retained?
Also, you don't add a new constraint for linux,keycodes, so now it is
permitted to have > 8 entries for existing devices.
pw-bot: changes-requested
Thanks,
Conor.
> + then:
> + patternProperties:
> + "^led@":
> + properties:
> + reg:
> + maximum: 7
> +
> - if:
> properties:
> compatible:
> --
> 2.54.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: footbridge: convert to sparse IRQs
From: Ethan Nelson-Moore @ 2026-06-08 17:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Torokhov, linux-arm-kernel, linux-input, linux-serial,
Russell King, Greg Kroah-Hartman, Jiri Slaby, Russell King,
Linus Walleij, Kees Cook, Nathan Chancellor,
Sebastian Andrzej Siewior, Steven Rostedt, Thomas Weißschuh,
Peter Zijlstra
In-Reply-To: <86765df4-0d2d-435e-85d8-b9bd4f3ea85d@app.fastmail.com>
Hi, Arnd,
On Mon, Jun 8, 2026 at 10:11 AM Arnd Bergmann <arnd@arndb.de> wrote:
> I think this is correct, as footbridge is the only one that selects
> CONFIG_ARCH_MIGHT_HAVE_PC_SERIO and defines I8042_KBD_IRQ on arm.
I came to the same conclusion.
Ethan
^ permalink raw reply
* Re: [PATCH] ARM: footbridge: convert to sparse IRQs
From: Arnd Bergmann @ 2026-06-08 17:11 UTC (permalink / raw)
To: Dmitry Torokhov, Ethan Nelson-Moore
Cc: linux-arm-kernel, linux-input, linux-serial, Russell King,
Greg Kroah-Hartman, Jiri Slaby, Russell King, Linus Walleij,
Kees Cook, Nathan Chancellor, Sebastian Andrzej Siewior,
Steven Rostedt, Thomas Weißschuh, Peter Zijlstra
In-Reply-To: <aibz3auoiq4CFa9l@google.com>
On Mon, Jun 8, 2026, at 18:56, Dmitry Torokhov wrote:
> On Sat, May 09, 2026 at 10:20:49PM -0700, Ethan Nelson-Moore wrote:
>> diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
>> index a8f4b2d70e59..cea72bd888af 100644
>>
>> -#if defined(__arm__)
>> -/* defined in include/asm-arm/arch-xxx/irqs.h */
>> -#include <asm/irq.h>
>> +#ifdef CONFIG_ARCH_FOOTBRIDGE
>> +/* defined in arch/arm/mach-footbridge/include/mach/irqs.h */
>> +#include <mach/irqs.h>
>> #elif defined(CONFIG_PPC)
>> extern int of_i8042_kbd_irq;
>> extern int of_i8042_aux_irq;
>
> This does not look right. Did you mean to drop inclusion of asm/irq.h
> for all other arms?
I think this is correct, as footbridge is the only one that selects
CONFIG_ARCH_MIGHT_HAVE_PC_SERIO and defines I8042_KBD_IRQ on arm.
Arnd
^ permalink raw reply
* Re: [PATCH] ARM: footbridge: convert to sparse IRQs
From: Dmitry Torokhov @ 2026-06-08 16:56 UTC (permalink / raw)
To: Ethan Nelson-Moore
Cc: linux-arm-kernel, linux-input, linux-serial, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, Jiri Slaby,
Russell King (Oracle), Linus Walleij, Kees Cook,
Nathan Chancellor, Sebastian Andrzej Siewior, Steven Rostedt,
Thomas Weissschuh, Peter Zijlstra
In-Reply-To: <20260510052107.555825-1-enelsonmoore@gmail.com>
Hi Ethan,
On Sat, May 09, 2026 at 10:20:49PM -0700, Ethan Nelson-Moore wrote:
> diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
> index a8f4b2d70e59..cea72bd888af 100644
> --- a/drivers/input/serio/i8042-io.h
> +++ b/drivers/input/serio/i8042-io.h
> @@ -15,9 +15,9 @@
> * IRQs.
> */
>
> -#if defined(__arm__)
> -/* defined in include/asm-arm/arch-xxx/irqs.h */
> -#include <asm/irq.h>
> +#ifdef CONFIG_ARCH_FOOTBRIDGE
> +/* defined in arch/arm/mach-footbridge/include/mach/irqs.h */
> +#include <mach/irqs.h>
> #elif defined(CONFIG_PPC)
> extern int of_i8042_kbd_irq;
> extern int of_i8042_aux_irq;
This does not look right. Did you mean to drop inclusion of asm/irq.h
for all other arms?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: hiddev: Use kref to track struct hiddev lifetime
From: sashiko-bot @ 2026-06-08 16:52 UTC (permalink / raw)
To: Heitor Alves de Siqueira; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260608-hiddev_kref-v1-1-cd240c95423f@igalia.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Use-After-Free in hiddev_send_event due to race between hid_disconnect and usbhid_irq_in.
--
commit a4056f7ab6df2200de09a38f314eabfb8dd4a066
Author: Heitor Alves de Siqueira <halves@igalia.com>
HID: hiddev: Use kref to track struct hiddev lifetime
This commit fixes a race between hiddev_disconnect() and hiddev_release()
by introducing a kref in struct hiddev. It moves the deallocation into a
dedicated release callback, ensuring the structure is only freed after both
functions are complete.
Closes: https://syzkaller.appspot.com/bug?extid=563191a4939ddbfe73d4
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 6378801b22c6d..1f2802920beee 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
[ ... ]
> @@ -933,13 +947,11 @@ void hiddev_disconnect(struct hid_device *hid)
>
> mutex_lock(&hiddev->existancelock);
> hiddev->exist = 0;
> -
> if (hiddev->open) {
> hid_hw_close(hiddev->hid);
> wake_up_interruptible(&hiddev->wait);
> - mutex_unlock(&hiddev->existancelock);
> - } else {
> - mutex_unlock(&hiddev->existancelock);
> - kfree(hiddev);
> }
> + mutex_unlock(&hiddev->existancelock);
> +
> + kref_put(&hiddev->kref, hiddev_kref_free);
[Severity: Critical]
This isn't a bug introduced by this patch, but is there a use-after-free
race condition when freeing struct hiddev while URBs are still active?
When a USB HID device is disconnected, usbhid_disconnect() eventually calls
hid_hw_stop(), which tears down the high-level structures before stopping
the low-level driver:
drivers/hid/hid-core.c:hid_hw_stop() {
hid_disconnect(hdev);
hdev->ll_driver->stop(hdev);
}
Because hid_disconnect() calls hiddev_disconnect() (which drops the kref
here), if no userspace process holds the file open, struct hiddev will be
freed immediately.
Since the input URBs have not yet been killed by hdev->ll_driver->stop(),
can an URB complete concurrently on another CPU?
If the URB completes, the completion handler usbhid_irq_in() propagates to
hid_input_report() -> hid_input_field() -> hiddev_send_event(), which then
dereferences the freed hiddev pointer:
drivers/hid/usbhid/hiddev.c:hiddev_send_event() {
struct hiddev *hiddev = hid->hiddev;
struct hiddev_list *list;
unsigned long flags;
spin_lock_irqsave(&hiddev->list_lock, flags);
Would this cause a use-after-free panic in interrupt context when it tries
to acquire list_lock?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-hiddev_kref-v1-1-cd240c95423f@igalia.com?part=1
^ permalink raw reply
* Re: [PATCH] input: ipaq-micro-keys: simplify allocation
From: Dmitry Torokhov @ 2026-06-08 16:49 UTC (permalink / raw)
To: Rosen Penev; +Cc: linux-input, open list
In-Reply-To: <20260608045041.4664-1-rosenp@gmail.com>
Hi Rosen,
On Sun, Jun 07, 2026 at 09:50:41PM -0700, Rosen Penev wrote:
> Use a flexible array member to have a single allocation.
Why does it have to be flexible? The size is known.
>
> 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);
Please keep to the limits for the kernel. 100 columns is acceptable when
leads to better readability, but I prefer 80 when it makes sense.
checkpatch.pl would have warned you.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] ARM: footbridge: convert to sparse IRQs
From: Ethan Nelson-Moore @ 2026-06-08 16:40 UTC (permalink / raw)
To: Linus Walleij
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: <CAD++jLmUW4KjxEjwv7_an_dEMTSOtvG=q2MjhqKvnuGrWaaCsQ@mail.gmail.com>
Hi, Linus,
On Mon, Jun 8, 2026 at 1:50 AM Linus Walleij <linusw@kernel.org> wrote:
> Have you tested this on real hardware?
> The commit doesn't really say.
No, I haven't. I can try to test it in GXemul, but I'm not sure if it
can boot Linux. The developer only lists it as capable of booting
NetBSD.
Ethan
^ permalink raw reply
* [PATCH] HID: hiddev: Use kref to track struct hiddev lifetime
From: Heitor Alves de Siqueira @ 2026-06-08 16:33 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: kernel-dev, linux-usb, linux-input, linux-kernel,
syzbot+563191a4939ddbfe73d4
If a USB HID device is disconnected while userspace still holds the
hiddev node open, hiddev_disconnect() and hiddev_release() can race on
the embedded existancelock mutex. Syzbot has triggered this with kfree()
happening during the mutex slow path.
Fix by introducing a kref in struct hiddev, and moving kfree() into a
dedicated release callback. This way, struct hiddev will only be freed
after both hiddev_release() and hiddev_disconnect() are done.
Fixes: 6cb4b040795c ("HID: hiddev: fix race between hiddev_disconnect and hiddev_release")
Reported-by: syzbot+563191a4939ddbfe73d4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=563191a4939ddbfe73d4
Signed-off-by: Heitor Alves de Siqueira <halves@igalia.com>
---
drivers/hid/usbhid/hiddev.c | 36 ++++++++++++++++++++++++------------
include/linux/hiddev.h | 1 +
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 6378801b22c6..1f2802920bee 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -46,6 +46,13 @@ struct hiddev_list {
struct mutex thread_lock;
};
+static void hiddev_kref_free(struct kref *kref)
+{
+ struct hiddev *hiddev = container_of(kref, struct hiddev, kref);
+
+ kfree(hiddev);
+}
+
/*
* Find a report, given the report's type and ID. The ID can be specified
* indirectly by REPORT_ID_FIRST (which returns the first report of the given
@@ -227,15 +234,11 @@ static int hiddev_release(struct inode * inode, struct file * file)
if (list->hiddev->exist) {
hid_hw_close(list->hiddev->hid);
hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
- } else {
- mutex_unlock(&list->hiddev->existancelock);
- kfree(list->hiddev);
- vfree(list);
- return 0;
}
}
-
mutex_unlock(&list->hiddev->existancelock);
+
+ kref_put(&list->hiddev->kref, hiddev_kref_free);
vfree(list);
return 0;
@@ -298,10 +301,20 @@ static int hiddev_open(struct inode *inode, struct file *file)
hid = usb_get_intfdata(intf);
hiddev = hid->hiddev;
+ /*
+ * kref_get_unless_zero() checks if we've already dropped
+ * the last reference; bail out early in this case
+ */
+ if (!hiddev || !kref_get_unless_zero(&hiddev->kref))
+ return -ENODEV;
+
mutex_lock(&hiddev->existancelock);
res = hiddev->exist ? __hiddev_open(hiddev, file) : -ENODEV;
mutex_unlock(&hiddev->existancelock);
+ if (res < 0)
+ kref_put(&hiddev->kref, hiddev_kref_free);
+
return res;
}
@@ -893,6 +906,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
if (!(hiddev = kzalloc_obj(struct hiddev)))
return -ENOMEM;
+ kref_init(&hiddev->kref);
init_waitqueue_head(&hiddev->wait);
INIT_LIST_HEAD(&hiddev->list);
spin_lock_init(&hiddev->list_lock);
@@ -904,7 +918,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
if (retval) {
hid_err(hid, "Not able to get a minor for this device\n");
hid->hiddev = NULL;
- kfree(hiddev);
+ kref_put(&hiddev->kref, hiddev_kref_free);
return retval;
}
@@ -933,13 +947,11 @@ void hiddev_disconnect(struct hid_device *hid)
mutex_lock(&hiddev->existancelock);
hiddev->exist = 0;
-
if (hiddev->open) {
hid_hw_close(hiddev->hid);
wake_up_interruptible(&hiddev->wait);
- mutex_unlock(&hiddev->existancelock);
- } else {
- mutex_unlock(&hiddev->existancelock);
- kfree(hiddev);
}
+ mutex_unlock(&hiddev->existancelock);
+
+ kref_put(&hiddev->kref, hiddev_kref_free);
}
diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
index 2164c03d2c72..1613c9c37ab7 100644
--- a/include/linux/hiddev.h
+++ b/include/linux/hiddev.h
@@ -24,6 +24,7 @@ struct hiddev {
int minor;
int exist;
int open;
+ struct kref kref;
struct mutex existancelock;
wait_queue_head_t wait;
struct hid_device *hid;
---
base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
change-id: 20260603-hiddev_kref-52f5507d4e42
Best regards,
--
Heitor Alves de Siqueira <halves@igalia.com>
^ permalink raw reply related
* Re: [PATCH 2/9] iio: orientation: hid-sensor-incl-3d: Fix race between callback registration and device exposure
From: Pandruvada, Srinivas @ 2026-06-08 15:34 UTC (permalink / raw)
To: dlechner@baylibre.com, archana.patni@linux.intel.com,
hongyan.song@intel.com, nuno.sa@analog.com, jic23@kernel.org,
jikos@kernel.org, andy@kernel.org, sanjayembeddedse@gmail.com
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-2-27a848c5758f@gmail.com>
On Sat, 2026-06-06 at 17:07 +0530, Sanjay Chitroda wrote:
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> The driver registers the IIO device before completing sensor hub
> callback registration and unregisters callbacks while the IIO device
> is still exposed during teardown.
>
> This creates race windows in both probe and remove paths, which can
> lead to NULL pointer dereferences or use-after-free.
Reordering is fine, but can you show how this use after free is
possible?
Thanks,
Srinivas
>
> Fix this by correct ordering of callback registration and
> IIO device registration in probe and remove paths.
>
> Fixes: 098d3beccfb6 ("iio: hid-sensors: Added Inclinometer 3D")
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
> drivers/iio/orientation/hid-sensor-incl-3d.c | 20 ++++++++++--------
> --
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c
> b/drivers/iio/orientation/hid-sensor-incl-3d.c
> index 4e23a598a3fb..5e3d2bb9b5bf 100644
> --- a/drivers/iio/orientation/hid-sensor-incl-3d.c
> +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
> @@ -356,12 +356,6 @@ static int hid_incl_3d_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> - ret = iio_device_register(indio_dev);
> - if (ret) {
> - dev_err(&pdev->dev, "device register failed\n");
> - goto error_remove_trigger;
> - }
> -
> incl_state->callbacks.send_event = incl_3d_proc_event;
> incl_state->callbacks.capture_sample =
> incl_3d_capture_sample;
> incl_state->callbacks.pdev = pdev;
> @@ -370,13 +364,19 @@ static int hid_incl_3d_probe(struct
> platform_device *pdev)
> &incl_state->callbacks);
> if (ret) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
>
> return 0;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev,
> HID_USAGE_SENSOR_INCLINOMETER_3D);
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &incl_state-
> >common_attributes);
> return ret;
> @@ -389,8 +389,8 @@ static void hid_incl_3d_remove(struct
> platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct incl_3d_state *incl_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev,
> HID_USAGE_SENSOR_INCLINOMETER_3D);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev,
> HID_USAGE_SENSOR_INCLINOMETER_3D);
> hid_sensor_remove_trigger(indio_dev, &incl_state-
> >common_attributes);
> }
>
^ 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