* Re: [PATCH v2 1/3] HID: asus: fix missing hid_is_usb() check
From: sashiko-bot @ 2026-07-03 15:58 UTC (permalink / raw)
To: Jann Horn; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260703-hid-usbcheck-v2-1-c5ed7bc94772@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Dangling pointer in `drvdata->kbd_backlight` leading to Use-After-Free when WMI listener registration fails.
--
commit 0bcf68d0cd670d6cc4fac38b7be14d0855140731
Author: Jann Horn <jannh@google.com>
HID: asus: fix missing hid_is_usb() check
This commit fixes a kernel splat caused by uhid devices that identify as
being on BUS_USB without actually having a USB parent. It adds a missing
hid_is_usb() check before calling to_usb_interface().
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..befa990b3210e 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -753,7 +753,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> return ret;
> }
>
> - if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> + if ((drvdata->quirks & QUIRK_ROG_ALLY_XPAD) && hid_is_usb(hdev)) {
> intf = to_usb_interface(hdev->dev.parent);
> udev = interface_to_usbdev(intf);
> validate_mcu_fw_version(hdev,
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
use-after-free later in this same function.
When asus_hid_register_listener() fails, the code frees the backlight
structure but leaves the pointer dangling:
drivers/hid/hid-asus.c:asus_kbd_register_leds() {
...
ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
if (ret < 0) {
/* No need to have this still around */
devm_kfree(&hdev->dev, drvdata->kbd_backlight);
}
return ret;
}
Since asus_probe() logs a warning but continues successfully when this
fails, could this cause a use-after-free during device removal or
suspend?
It looks like asus_remove() checks if the pointer is non-null before
dereferencing it:
drivers/hid/hid-asus.c:asus_remove() {
...
if (drvdata->kbd_backlight) {
asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
...
}
Should drvdata->kbd_backlight be set to NULL after being freed to prevent
this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-usbcheck-v2-0-c5ed7bc94772@google.com?part=1
^ permalink raw reply
* [PATCH v2 3/3] HID: rapoo: fix missing hid_is_usb() check
From: Jann Horn @ 2026-07-03 15:45 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
In-Reply-To: <20260703-hid-usbcheck-v2-0-c5ed7bc94772@google.com>
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent.
Fix the use of to_usb_interface() without a hid_is_usb() check.
Add a dependency on USB_HID for hid_is_usb(), as other HID drivers do; the
alternative would be to provide a simple stub implementation on !USB_HID
builds.
I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.
Fixes: b3b1c68fb726 ("HID: rapoo: Add support for side buttons on RAPOO 0x2015 mouse")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/Kconfig | 1 +
drivers/hid/hid-rapoo.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index f9bcaeb66385..48934c4f3c45 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,6 +1048,7 @@ config HID_PXRC
config HID_RAPOO
tristate "Rapoo non-fully HID-compliant devices"
+ depends on USB_HID
help
Support for Rapoo devices that are not fully compliant with the
HID standard.
diff --git a/drivers/hid/hid-rapoo.c b/drivers/hid/hid-rapoo.c
index 4c81f3086de4..5c9c396fabf7 100644
--- a/drivers/hid/hid-rapoo.c
+++ b/drivers/hid/hid-rapoo.c
@@ -36,7 +36,7 @@ static int rapoo_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- if (hdev->bus == BUS_USB) {
+ if (hid_is_usb(hdev)) {
struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
if (intf->cur_altsetting->desc.bInterfaceNumber != 1)
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v2 2/3] HID: huawei: fix missing hid_is_usb() check
From: Jann Horn @ 2026-07-03 15:45 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
In-Reply-To: <20260703-hid-usbcheck-v2-0-c5ed7bc94772@google.com>
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent.
Fix the use of to_usb_interface() without a hid_is_usb() check.
I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.
Fixes: e93faaca84b7 ("HID: huawei: fix CD30 keyboard report descriptor issue")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/hid-huawei.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-huawei.c b/drivers/hid/hid-huawei.c
index 6a616bf21b38..ee3fc6f68475 100644
--- a/drivers/hid/hid-huawei.c
+++ b/drivers/hid/hid-huawei.c
@@ -44,11 +44,12 @@ static const __u8 huawei_cd30_kbd_rdesc_fixed[] = {
static const __u8 *huawei_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
- struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_interface *intf = hid_is_usb(hdev) ?
+ to_usb_interface(hdev->dev.parent) : NULL;
switch (hdev->product) {
case USB_DEVICE_ID_HUAWEI_CD30KBD:
- if (intf->cur_altsetting->desc.bInterfaceNumber == 1) {
+ if (!intf || intf->cur_altsetting->desc.bInterfaceNumber == 1) {
if (*rsize != sizeof(huawei_cd30_kbd_rdesc_fixed) ||
memcmp(huawei_cd30_kbd_rdesc_fixed, rdesc,
sizeof(huawei_cd30_kbd_rdesc_fixed)) != 0) {
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v2 1/3] HID: asus: fix missing hid_is_usb() check
From: Jann Horn @ 2026-07-03 15:45 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
In-Reply-To: <20260703-hid-usbcheck-v2-0-c5ed7bc94772@google.com>
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent.
Fix the use of to_usb_interface() without a hid_is_usb() check.
I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.
Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/hid-asus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 3f5e96900b67..befa990b3210 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -753,7 +753,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
return ret;
}
- if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ if ((drvdata->quirks & QUIRK_ROG_ALLY_XPAD) && hid_is_usb(hdev)) {
intf = to_usb_interface(hdev->dev.parent);
udev = interface_to_usbdev(intf);
validate_mcu_fw_version(hdev,
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v2 0/3] hid: fix missing hid_is_usb() checks in three drivers
From: Jann Horn @ 2026-07-03 15:45 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
This fixes missing hid_is_usb() checks before to_usb_interface() in
three HID drivers.
I've split it into three patches so that they can have separate "Fixes"
tags, hopefully they are easier to stable-backport this way.
Signed-off-by: Jann Horn <jannh@google.com>
---
Changes in v2:
- patch 3/3: fix typo in "Fixes" line
- patch 3/3: add USB_HID dependency (alternative would be to implement a
stub for hid_is_usb())
- Link to v1: https://patch.msgid.link/20260703-hid-usbcheck-v1-0-e80259ff625d@google.com
---
Jann Horn (3):
HID: asus: fix missing hid_is_usb() check
HID: huawei: fix missing hid_is_usb() check
HID: rapoo: fix missing hid_is_usb() check
drivers/hid/Kconfig | 1 +
drivers/hid/hid-asus.c | 2 +-
drivers/hid/hid-huawei.c | 5 +++--
drivers/hid/hid-rapoo.c | 2 +-
4 files changed, 6 insertions(+), 4 deletions(-)
---
base-commit: 51512e22efe813d8223de27f6fd02a8a48ea2323
change-id: 20260703-hid-usbcheck-9163e6cf6015
Best regards,
--
Jann Horn <jannh@google.com>
^ permalink raw reply
* Re: [PATCH] HID: asus: fix UAF of ->kbd_backlight on !CONFIG_ASUS_WMI
From: sashiko-bot @ 2026-07-03 15:42 UTC (permalink / raw)
To: Jann Horn; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260703-hid-asus-uaf-v1-1-1a2af2534eb0@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Race condition in asus_remove() due to incorrect teardown order for fn_lock_sync_work, leading to Use-After-Free.
--
commit 10088228c9fb50f752e52778b8014fc75f17200d
Author: Jann Horn <jannh@google.com>
HID: asus: fix UAF of ->kbd_backlight on !CONFIG_ASUS_WMI
This commit fixes a Use-After-Free bug where drvdata->kbd_backlight was not
set to NULL after being freed in asus_kbd_register_leds(). By explicitly
clearing the pointer when asus_hid_register_listener() fails, it prevents
asus_remove() from following a freed pointer during teardown.
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..c2a5edebbd7a7 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -777,6 +777,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (ret < 0) {
> /* No need to have this still around */
> devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> + drvdata->kbd_backlight = NULL;
> }
>
> return ret;
[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition in
asus_remove() that might lead to a Use-After-Free.
When a device is removed, asus_remove() calls cancel_work_sync() for
fn_lock_sync_work before stopping the hardware:
drivers/hid/hid-asus.c:asus_remove() {
...
if (drvdata->quirks & QUIRK_HID_FN_LOCK)
cancel_work_sync(&drvdata->fn_lock_sync_work);
hid_hw_stop(hdev);
}
If a KEY_FN_ESC event arrives in the window between these two calls, won't
the HID core invoke asus_event() and blindly requeue the work?
drivers/hid/hid-asus.c:asus_event() {
...
case KEY_FN_ESC:
if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
drvdata->fn_lock = !drvdata->fn_lock;
schedule_work(&drvdata->fn_lock_sync_work);
}
break;
...
}
Since the driver data is freed after device removal, could this executed
work access freed memory if the teardown order isn't reversed so that
hid_hw_stop() runs before cancel_work_sync()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-asus-uaf-v1-1-1a2af2534eb0@google.com?part=1
^ permalink raw reply
* Re: [PATCH 1/3] HID: asus: fix missing hid_is_usb() check
From: sashiko-bot @ 2026-07-03 15:32 UTC (permalink / raw)
To: Jann Horn; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260703-hid-usbcheck-v1-1-e80259ff625d@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing pointer invalidation after `devm_kfree` causes a deterministic Use-After-Free on the failure path.
--
commit d3bce769a13470fb7e8717746a98294d5eb006c9
Author: Jann Horn <jannh@google.com>
HID: asus: fix missing hid_is_usb() check
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent. Fix the use of to_usb_interface() without a
hid_is_usb() check.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..befa990b3210e 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -753,7 +753,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> return ret;
> }
>
> - if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> + if ((drvdata->quirks & QUIRK_ROG_ALLY_XPAD) && hid_is_usb(hdev)) {
> intf = to_usb_interface(hdev->dev.parent);
> udev = interface_to_usbdev(intf);
> validate_mcu_fw_version(hdev,
[Severity: High]
This is a pre-existing issue, but does the failure path in this function leave
a dangling pointer that can cause a use-after-free?
If asus_hid_register_listener() fails later in asus_kbd_register_leds():
ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
if (ret < 0) {
/* No need to have this still around */
devm_kfree(&hdev->dev, drvdata->kbd_backlight);
}
The memory allocated for drvdata->kbd_backlight is freed, but the pointer is
not set to NULL.
Then asus_probe() catches the failure, emits a warning, but improperly allows
probe to succeed:
if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
(asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
(asus_kbd_register_leds(hdev)))
hid_warn(hdev, "Failed to initialize backlight.\n");
If this occurs, could a subsequent call to asus_remove() or asus_resume()
dereference the freed memory?
if (drvdata->kbd_backlight) {
asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
drvdata->kbd_backlight->removed = true;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-usbcheck-v1-0-e80259ff625d@google.com?part=1
^ permalink raw reply
* Re: [PATCH 3/3] HID: rapoo: fix missing hid_is_usb() check
From: sashiko-bot @ 2026-07-03 15:29 UTC (permalink / raw)
To: Jann Horn; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260703-hid-usbcheck-v1-3-e80259ff625d@google.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing Kconfig dependency `depends on USB_HID` for `CONFIG_HID_RAPOO` causes a linker error.
- [Low] Malformed `Fixes:` tags in the commit message.
--
commit ac5c5d7fc93db76e1ff4b587da9d47600a05ff83
Author: Jann Horn <jannh@google.com>
HID: rapoo: fix missing hid_is_usb() check
This commit fixes a potential splat by verifying a HID device is actually on
a USB bus before casting its parent to a usb_interface.
> I have verified that it is currently possible to trigger a kernel splat due
> to this bug in an ASAN build, and that this commit fixes the issue.
>
> Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")Fixes: b3b1c68fb726 ("HID: rapoo: Add support for side buttons on RAPOO 0x2015 mouse")
[Severity: Low]
This isn't a bug, but there are two Fixes tags concatenated on the same line.
Could this break automated patch parsing tools and stable backporting scripts
that expect each tag to be on a separate line?
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
>
> diff --git a/drivers/hid/hid-rapoo.c b/drivers/hid/hid-rapoo.c
> index 4c81f3086de43..5c9c396fabf77 100644
> --- a/drivers/hid/hid-rapoo.c
> +++ b/drivers/hid/hid-rapoo.c
[ ... ]
> @@ -36,7 +36,7 @@ static int rapoo_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
> }
>
> - if (hdev->bus == BUS_USB) {
> + if (hid_is_usb(hdev)) {
[Severity: Low]
Does the introduction of hid_is_usb() in rapoo_probe() require an update to
the CONFIG_HID_RAPOO entry in drivers/hid/Kconfig?
Because hid_is_usb() is defined and exported exclusively by the usbhid module,
if a user configures the kernel with CONFIG_HID_RAPOO=y and CONFIG_USB_HID=n
(or m), will this cause a build failure during the linking stage due to an
undefined reference?
Should CONFIG_HID_RAPOO now explicitly depend on USB_HID?
> struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>
> if (intf->cur_altsetting->desc.bInterfaceNumber != 1)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-hid-usbcheck-v1-0-e80259ff625d@google.com?part=3
^ permalink raw reply
* [PATCH] HID: asus: fix UAF of ->kbd_backlight on !CONFIG_ASUS_WMI
From: Jann Horn @ 2026-07-03 15:27 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Carlo Caione, linux-input, linux-kernel, stable, Jann Horn
On kernels without !CONFIG_ASUS_WMI, asus_hid_register_listener() will
fail. asus_kbd_register_leds() reacts to this by freeing
drvdata->kbd_backlight, but doesn't NULL out the pointer, causing UAF when
asus_remove() follows this pointer.
I have tested that this bug causes an ASAN splat, and that this change
fixes the issue.
Cc: stable@vger.kernel.org
Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards")
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/hid-asus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 3f5e96900b67..c2a5edebbd7a 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -777,6 +777,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (ret < 0) {
/* No need to have this still around */
devm_kfree(&hdev->dev, drvdata->kbd_backlight);
+ drvdata->kbd_backlight = NULL;
}
return ret;
---
base-commit: 51512e22efe813d8223de27f6fd02a8a48ea2323
change-id: 20260703-hid-asus-uaf-0b8e1325fc00
Best regards,
--
Jann Horn <jannh@google.com>
^ permalink raw reply related
* [PATCH 3/3] HID: rapoo: fix missing hid_is_usb() check
From: Jann Horn @ 2026-07-03 15:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
In-Reply-To: <20260703-hid-usbcheck-v1-0-e80259ff625d@google.com>
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent.
Fix the use of to_usb_interface() without a hid_is_usb() check.
I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.
Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")Fixes: b3b1c68fb726 ("HID: rapoo: Add support for side buttons on RAPOO 0x2015 mouse")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/hid-rapoo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-rapoo.c b/drivers/hid/hid-rapoo.c
index 4c81f3086de4..5c9c396fabf7 100644
--- a/drivers/hid/hid-rapoo.c
+++ b/drivers/hid/hid-rapoo.c
@@ -36,7 +36,7 @@ static int rapoo_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- if (hdev->bus == BUS_USB) {
+ if (hid_is_usb(hdev)) {
struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
if (intf->cur_altsetting->desc.bInterfaceNumber != 1)
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 2/3] HID: huawei: fix missing hid_is_usb() check
From: Jann Horn @ 2026-07-03 15:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
In-Reply-To: <20260703-hid-usbcheck-v1-0-e80259ff625d@google.com>
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent.
Fix the use of to_usb_interface() without a hid_is_usb() check.
I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.
Fixes: e93faaca84b7 ("HID: huawei: fix CD30 keyboard report descriptor issue")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/hid-huawei.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-huawei.c b/drivers/hid/hid-huawei.c
index 6a616bf21b38..ee3fc6f68475 100644
--- a/drivers/hid/hid-huawei.c
+++ b/drivers/hid/hid-huawei.c
@@ -44,11 +44,12 @@ static const __u8 huawei_cd30_kbd_rdesc_fixed[] = {
static const __u8 *huawei_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
- struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_interface *intf = hid_is_usb(hdev) ?
+ to_usb_interface(hdev->dev.parent) : NULL;
switch (hdev->product) {
case USB_DEVICE_ID_HUAWEI_CD30KBD:
- if (intf->cur_altsetting->desc.bInterfaceNumber == 1) {
+ if (!intf || intf->cur_altsetting->desc.bInterfaceNumber == 1) {
if (*rsize != sizeof(huawei_cd30_kbd_rdesc_fixed) ||
memcmp(huawei_cd30_kbd_rdesc_fixed, rdesc,
sizeof(huawei_cd30_kbd_rdesc_fixed)) != 0) {
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 1/3] HID: asus: fix missing hid_is_usb() check
From: Jann Horn @ 2026-07-03 15:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
In-Reply-To: <20260703-hid-usbcheck-v1-0-e80259ff625d@google.com>
to_usb_interface() can only be used on a hid_device whose parent is really
USB; uhid can create devices that identify as being on BUS_USB, but don't
actually have a USB parent.
Fix the use of to_usb_interface() without a hid_is_usb() check.
I have verified that it is currently possible to trigger a kernel splat due
to this bug in an ASAN build, and that this commit fixes the issue.
Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/hid/hid-asus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 3f5e96900b67..befa990b3210 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -753,7 +753,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
return ret;
}
- if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ if ((drvdata->quirks & QUIRK_ROG_ALLY_XPAD) && hid_is_usb(hdev)) {
intf = to_usb_interface(hdev->dev.parent);
udev = interface_to_usbdev(intf);
validate_mcu_fw_version(hdev,
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 0/3] hid: fix missing hid_is_usb() checks in three drivers
From: Jann Horn @ 2026-07-03 15:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Ilpo Järvinen, Mario Limonciello, Luke D. Jones, Miao Li,
linux-input, linux-kernel, Jann Horn, stable
This fixes missing hid_is_usb() checks before to_usb_interface() in
three HID drivers.
I've split it into three patches so that they can have separate "Fixes"
tags, hopefully they are easier to stable-backport this way.
Signed-off-by: Jann Horn <jannh@google.com>
---
Jann Horn (3):
HID: asus: fix missing hid_is_usb() check
HID: huawei: fix missing hid_is_usb() check
HID: rapoo: fix missing hid_is_usb() check
drivers/hid/hid-asus.c | 2 +-
drivers/hid/hid-huawei.c | 5 +++--
drivers/hid/hid-rapoo.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
---
base-commit: 51512e22efe813d8223de27f6fd02a8a48ea2323
change-id: 20260703-hid-usbcheck-9163e6cf6015
Best regards,
--
Jann Horn <jannh@google.com>
^ permalink raw reply
* [PATCH] Add a quirk for a Cirque I2C device.
From: Vadim Klishko @ 2026-07-03 14:47 UTC (permalink / raw)
To: jikos; +Cc: linux-input, linux-kernel, Vadim Klishko
Cirque touchpads with PID D0C1 generate an error when probed
by the I2C HID driver, resulting in no hidraw device created.
Signed-off-by: Vadim Klishko <vadim@cirque.com>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/i2c-hid/i2c-hid-core.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1059922baaac..496589277bd5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -334,6 +334,7 @@
#define I2C_VENDOR_ID_CIRQUE 0x0488
#define I2C_PRODUCT_ID_CIRQUE_1063 0x1063
+#define I2C_PRODUCT_ID_CIRQUE_D0C1 0xD0C1
#define USB_VENDOR_ID_CJTOUCH 0x24b8
#define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0020 0x0020
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..63004bcbfa71 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -136,6 +136,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_BAD_INPUT_SIZE },
{ I2C_VENDOR_ID_CIRQUE, I2C_PRODUCT_ID_CIRQUE_1063,
I2C_HID_QUIRK_NO_SLEEP_ON_SUSPEND },
+ { I2C_VENDOR_ID_CIRQUE, I2C_PRODUCT_ID_CIRQUE_D0C1,
+ I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
/*
* Without additional power on command, at least some QTEC devices send garbage
*/
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 0/5] Add OneXPlayer Configuration HID Driver
From: Günther Noack @ 2026-07-03 14:25 UTC (permalink / raw)
To: Derek J. Clark
Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A . Griffais,
Lee Jones, Lambert Fan, Zhouwang Huang, linux-input, linux-doc,
linux-kernel
In-Reply-To: <20260419042624.625746-1-derekjohn.clark@gmail.com>
Hello Derek!
On Sat, Apr 18, 2026 at 09:26:19PM -0700, Derek J. Clark wrote:
> Adds an HID driver for OneXPlayer HID configuration devices. There are
> currently 2 generations of OneXPlayer HID protocol. The first (OneXPlayer
> F1 series) only provides an RGB control interface over HID. The Second
> (X1 mini series, G1 series, AOKZOE A1X) also includes a hardware level
> button mapping interface, vibration intensity settings, and the ability
> to switch output between xinput and a debug mode that can be used to debug
> the button mapping. Some devices (G1 Series, APEX) use a hybrid of Gen1
> RGB control and Gen 2 controller settings. To ensure there is no conflicts
> when the driver is loaded, we skip creating the RGB interface for Gen 2
> devices if there is a DMI match.
>
> I'll also add a note that Gen 1 devices also have an interface for
> setting the key map and debug mode, but that is done entirely over a
> serial TTY device so it is not able to be added to this driver. There
> are also some "Gen 0" devices (OneXPlayer 2 Series) also use it, but
> the TTY interface also handles the RGB control so no support is
> provided by this driver for those interfaces.
>
> Signed-off-by: Derel J. Clark <derekjohn.clark@gmail.com>
Sorry I am late to this review, but here are two issues I discovered
when looking at the code:
(1) The functions oxp_hid_raw_event_gen_1() and
oxp_hid_raw_event_gen_2() are both forgetting to do bounds checks
against the "size" argument.
For real devices, which send a real report descriptor, these buffers
will be large enough, but a device that sends a faked report
descriptor can provoke an out-of-bounds-read here by underspecifying
the size for these reports.
(2) oxp_hid_probe() and other functions are populating drvdata, and
drvdata is a static variable. If you plug in two of these devices
at the same time, they will step on each other's toes, and this
leads to all kinds of memory corruption problems when they do.
I believe the right way to go about this is to allocate a separate
piece of memory for each device that you are plugging in. Other
device drivers do this uing devm_kzalloc().
Disclaimer:
I found these through code inspection and curiosity but have not tried
to reproduce the crashes.
Per Linux's official threat model[1], these are not considered security
vulnerabilities. An attacker who impersonates a USB device and gains
illegitimate access to the USB port might be able to provoke these bugs
though, and I wouldn't be surprised if (2) also just leads to system
crashes when using two of these devices at the same time.
—Günther
[1] https://docs.kernel.org/process/threat-model.html
^ permalink raw reply
* Re: [PATCH 1/2] HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support
From: Bastien Nocera @ 2026-07-03 12:53 UTC (permalink / raw)
To: Elliot Douglas; +Cc: linux-input, lains, jikos, bentiss, linux-kernel
In-Reply-To: <CAGt6S1pqCNPeV-Hy6w-8eynKhKwzz5X4fefkJfM0gYGgQzGAwA@mail.gmail.com>
On Wed, 2026-07-01 at 15:32 -0700, Elliot Douglas wrote:
> Just wanted to poke on this thread again, Benjamin or Bastien, what
> is needed to
> push this forward or should I send the v2 at this point?
You're definitely better off sending a new version when there's no
feedback incoming, it acts as a gentle reminder and avoids another back
and forth if there's no further comments (apart from the ones in the
original review).
>
> Thanks,
> Elliot
>
> On Wed, Jun 17, 2026 at 6:16 PM Elliot Douglas
> <edouglas7358@gmail.com> wrote:
> >
> > Thanks, that makes sense.
> >
> > For Solaar, this is not continuously forced. The kernel only
> > programs
> > temporary diversion when the device connects. Solaar can still
> > issue HID++
> > commands through hidraw, so if Solaar changes reporting for the
> > same controls
> > afterwards, the last writer wins.
> >
> > If Solaar takes over those controls for custom actions, the kernel
> > would stop
> > receiving the diverted button notifications for normal evdev
> > reporting until
> > the kernel diverts the controls again, for example after reconnect.
> > While the
> > controls remain diverted, hidraw clients should still receive the
> > raw HID++
> > reports.
> >
> > I have addressed the inline comments locally for v2:
> > - replaced the profile/count wrapper with NULL-terminated mapping
> > arrays
> > - cached the selected mapping pointer in struct hidpp_device
> >
> > I'll wait for Benjamin's input to send Patch v2.
> >
> >
> > On Wed, Jun 17, 2026 at 3:28 AM Bastien Nocera <hadess@hadess.net>
> > wrote:
> > >
> > > On Sat, 2026-06-13 at 10:51 -0700, Elliot Douglas wrote:
> > > > Some Logitech HID++ 2.0 mice can report diverted reprogrammable
> > > > controls through HID++ feature 0x1b04, SpecialKeysMseButtons /
> > > > REPROG_CONTROLS_V4, instead of the normal HID mouse report.
> > > >
> > > > Add a quirk-gated event path for those controls. The handler
> > > > temporarily
> > > > diverts verified per-product controls, parses
> > > > divertedButtonsEvent as the
> > > > current pressed-control list, and reports the corresponding
> > > > evdev key state
> > > > for every mapped control.
> > > >
> > > > Keep the control mappings in per-product profiles so adding
> > > > support for
> > > > another mouse does not change the evdev capabilities advertised
> > > > by
> > > > already-supported devices.
> > >
> > > How does this forced setting work/clash with the programmable
> > > buttons
> > > in Solaar?
> > >
> > > I've added some inline comments below.
> > >
> > > >
> > > > Documentation for feature 0x1b04 describes divertedButtonsEvent
> > > > as a list
> > > > of currently pressed diverted buttons, which is the event
> > > > format handled
> > > > here.
> > > >
> > > > Link:
> > > > https://lekensteyn.nl/files/logitech/x1b04_specialkeysmsebuttons.html
> > > >
> > > > Signed-off-by: Elliot Douglas <edouglas7358@gmail.com>
> > > > ---
> > > > drivers/hid/hid-logitech-hidpp.c | 215
> > > > +++++++++++++++++++++++++++++++
> > > > 1 file changed, 215 insertions(+)
> > > >
> > > > diff --git a/drivers/hid/hid-logitech-hidpp.c
> > > > b/drivers/hid/hid-logitech-hidpp.c
> > > > index 70ba1a5e40d8..24c9cfaa4f37 100644
> > > > --- a/drivers/hid/hid-logitech-hidpp.c
> > > > +++ b/drivers/hid/hid-logitech-hidpp.c
> > > > @@ -76,6 +76,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
> > > > #define HIDPP_QUIRK_HI_RES_SCROLL_1P0 BIT(28)
> > > > #define HIDPP_QUIRK_WIRELESS_STATUS BIT(29)
> > > > #define HIDPP_QUIRK_RESET_HI_RES_SCROLL BIT(30)
> > > > +#define HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS BIT(31)
> > > >
> > > > /* These are just aliases for now */
> > > > #define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
> > > > @@ -205,6 +206,7 @@ struct hidpp_device {
> > > > struct hidpp_scroll_counter vertical_wheel_counter;
> > > >
> > > > u8 wireless_feature_index;
> > > > + u8 reprog_controls_feature_index;
> > > >
> > > > int hires_wheel_multiplier;
> > > > u8 hires_wheel_feature_index;
> > > > @@ -3601,6 +3603,209 @@ static int
> > > > hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device
> > > > *hidpp,
> > > > return 1;
> > > > }
> > > >
> > > > +/* -----------------------------------------------------------
> > > > --------------- */
> > > > +/* HID++2.0 reprogrammable
> > > > controls */
> > > > +/* -----------------------------------------------------------
> > > > --------------- */
> > > > +
> > > > +#define HIDPP_PAGE_REPROG_CONTROLS_V4
> > > > 0x1b04
> > > > +
> > > > +#define HIDPP_REPROG_CONTROLS_GET_COUNT
> > > > 0x00
> > > > +#define HIDPP_REPROG_CONTROLS_GET_CID_INFO 0x10
> > > > +#define HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING 0x30
> > > > +
> > > > +#define HIDPP_REPROG_CONTROLS_FLAG_MOUSE BIT(0)
> > > > +#define HIDPP_REPROG_CONTROLS_FLAG_DIVERT BIT(5)
> > > > +
> > > > +#define HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED BIT(0)
> > > > +#define HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT
> > > > BIT(1)
> > > > +
> > > > +#define HIDPP_REPROG_CONTROLS_EVENT_DIVERTED 0x00
> > > > +
> > > > +struct hidpp_reprog_control_mapping {
> > > > + u16 control;
> > > > + u16 code;
> > > > +};
> > > > +
> > > > +struct hidpp_reprog_controls_profile {
> > > > + const struct hidpp_reprog_control_mapping *mappings;
> > >
> > > probably needs a __counted_by(), or maybe as it's static, it
> > > might be
> > > better to not require an intermediate struct, and return a NULL-
> > > terminated array instead.
> > >
> > > > + unsigned int mapping_count;
> > > > +};
> > > > +
> > > > +static const struct hidpp_reprog_controls_profile *
> > > > +hidpp20_reprog_controls_get_profile(struct hidpp_device
> > > > *hidpp)
> > > > +{
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static int hidpp20_reprog_controls_get_count(struct
> > > > hidpp_device *hidpp)
> > > > +{
> > > > + struct hidpp_report response;
> > > > + u8 feature_index = hidpp->reprog_controls_feature_index;
> > > > + u8 cmd = HIDPP_REPROG_CONTROLS_GET_COUNT;
> > > > + int ret;
> > > > +
> > > > + ret = hidpp_send_fap_command_sync(hidpp, feature_index,
> > > > cmd, NULL, 0,
> > > > + &response);
> > > > + if (ret > 0)
> > > > + return -EPROTO;
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + return response.fap.params[0];
> > > > +}
> > > > +
> > > > +static int hidpp20_reprog_controls_get_cid_info(struct
> > > > hidpp_device *hidpp,
> > > > + u8 index, u16
> > > > *control,
> > > > + u8 *flags)
> > > > +{
> > > > + struct hidpp_report response;
> > > > + u8 feature_index = hidpp->reprog_controls_feature_index;
> > > > + u8 cmd = HIDPP_REPROG_CONTROLS_GET_CID_INFO;
> > > > + int ret;
> > > > +
> > > > + ret = hidpp_send_fap_command_sync(hidpp, feature_index,
> > > > cmd, &index,
> > > > + sizeof(index),
> > > > &response);
> > > > + if (ret > 0)
> > > > + return -EPROTO;
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + *control = get_unaligned_be16(&response.fap.params[0]);
> > > > + *flags = response.fap.params[4];
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static bool hidpp20_reprog_controls_find_control(struct
> > > > hidpp_device *hidpp,
> > > > + u16 control)
> > > > +{
> > > > + int count, ret;
> > > > + u16 cid;
> > > > + u8 flags;
> > > > + int i;
> > > > +
> > > > + count = hidpp20_reprog_controls_get_count(hidpp);
> > > > + if (count < 0)
> > > > + return false;
> > > > +
> > > > + for (i = 0; i < count; i++) {
> > > > + ret = hidpp20_reprog_controls_get_cid_info(hidpp,
> > > > i, &cid,
> > > > +
> > > > &flags);
> > > > + if (ret)
> > > > + return false;
> > > > +
> > > > + if (cid == control)
> > > > + return (flags &
> > > > HIDPP_REPROG_CONTROLS_FLAG_MOUSE) &&
> > > > + (flags &
> > > > HIDPP_REPROG_CONTROLS_FLAG_DIVERT);
> > > > + }
> > > > +
> > > > + return false;
> > > > +}
> > > > +
> > > > +static int
> > > > hidpp20_reprog_controls_set_control_reporting(struct
> > > > hidpp_device *hidpp,
> > > > + u16
> > > > control, u8 flags)
> > > > +{
> > > > + struct hidpp_report response;
> > > > + u8 params[5];
> > > > +
> > > > + put_unaligned_be16(control, ¶ms[0]);
> > > > + params[2] = flags;
> > > > + put_unaligned_be16(control, ¶ms[3]);
> > > > +
> > > > + return hidpp_send_fap_command_sync(hidpp,
> > > > + hidpp-
> > > > >reprog_controls_feature_index,
> > > > +
> > > > HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING,
> > > > + params,
> > > > sizeof(params), &response);
> > > > +}
> > > > +
> > > > +static void hidpp20_reprog_controls_connect(struct
> > > > hidpp_device *hidpp)
> > > > +{
> > > > + const struct hidpp_reprog_controls_profile *profile;
> > > > + u8 flags = HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED |
> > > > + HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT;
> > > > + unsigned int i;
> > > > +
> > > > + if (!(hidpp->quirks &
> > > > HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS))
> > > > + return;
> > > > +
> > > > + profile = hidpp20_reprog_controls_get_profile(hidpp);
> > >
> > > Could the profile be cached in the hidpp_device struct?
> > >
> > > > + if (!profile)
> > > > + return;
> > > > +
> > > > + if (hidpp_root_get_feature(hidpp,
> > > > HIDPP_PAGE_REPROG_CONTROLS_V4,
> > > > + &hidpp-
> > > > >reprog_controls_feature_index))
> > > > + return;
> > > > +
> > > > + for (i = 0; i < profile->mapping_count; i++) {
> > > > + u16 control = profile->mappings[i].control;
> > > > +
> > > > + if (!hidpp20_reprog_controls_find_control(hidpp,
> > > > control))
> > > > + continue;
> > > > +
> > > > +
> > > > hidpp20_reprog_controls_set_control_reporting(hidpp, control,
> > > > flags);
> > > > + }
> > > > +}
> > > > +
> > > > +static int hidpp20_reprog_controls_raw_event(struct
> > > > hidpp_device *hidpp,
> > > > + u8 *data, int size)
> > > > +{
> > > > + const struct hidpp_reprog_controls_profile *profile;
> > > > + const struct hidpp_reprog_control_mapping *mapping;
> > > > + struct hidpp_report *report = (struct hidpp_report
> > > > *)data;
> > > > + u16 controls[4];
> > > > + bool pressed;
> > > > + unsigned int i, j;
> > > > +
> > > > + if (!(hidpp->quirks &
> > > > HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS) ||
> > > > + !hidpp->input ||
> > > > + hidpp->reprog_controls_feature_index == 0xff)
> > > > + return 0;
> > > > +
> > > > + profile = hidpp20_reprog_controls_get_profile(hidpp);
> > > > + if (!profile)
> > > > + return 0;
> > > > +
> > > > + if (size < HIDPP_REPORT_LONG_LENGTH ||
> > > > + report->fap.feature_index != hidpp-
> > > > >reprog_controls_feature_index ||
> > > > + report->fap.funcindex_clientid !=
> > > > HIDPP_REPROG_CONTROLS_EVENT_DIVERTED)
> > > > + return 0;
> > > > +
> > > > + for (i = 0; i < ARRAY_SIZE(controls); i++)
> > > > + controls[i] = get_unaligned_be16(&report-
> > > > >fap.params[i * 2]);
> > > > +
> > > > + for (i = 0; i < profile->mapping_count; i++) {
> > > > + mapping = &profile->mappings[i];
> > > > + pressed = false;
> > > > +
> > > > + for (j = 0; j < ARRAY_SIZE(controls); j++) {
> > > > + if (controls[j] == mapping->control) {
> > > > + pressed = true;
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + input_report_key(hidpp->input, mapping->code,
> > > > pressed);
> > > > + }
> > > > +
> > > > + input_sync(hidpp->input);
> > > > +
> > > > + return 1;
> > > > +}
> > > > +
> > > > +static void hidpp20_reprog_controls_populate_input(struct
> > > > hidpp_device *hidpp,
> > > > + struct
> > > > input_dev *input_dev)
> > > > +{
> > > > + const struct hidpp_reprog_controls_profile *profile;
> > > > + unsigned int i;
> > > > +
> > > > + profile = hidpp20_reprog_controls_get_profile(hidpp);
> > > > + if (!profile)
> > > > + return;
> > > > +
> > > > + for (i = 0; i < profile->mapping_count; i++)
> > > > + input_set_capability(input_dev, EV_KEY, profile-
> > > > >mappings[i].code);
> > > > +}
> > > > +
> > > > static void hidpp10_extra_mouse_buttons_populate_input(
> > > > struct hidpp_device *hidpp, struct
> > > > input_dev *input_dev)
> > > > {
> > > > @@ -3859,6 +4064,9 @@ static void hidpp_populate_input(struct
> > > > hidpp_device *hidpp,
> > > >
> > > > if (hidpp->quirks & HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS)
> > > > hidpp10_extra_mouse_buttons_populate_input(hidpp,
> > > > input);
> > > > +
> > > > + if (hidpp->quirks &
> > > > HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS)
> > > > + hidpp20_reprog_controls_populate_input(hidpp,
> > > > input);
> > > > }
> > > >
> > > > static int hidpp_input_configured(struct hid_device *hdev,
> > > > @@ -3971,6 +4179,10 @@ static int hidpp_raw_hidpp_event(struct
> > > > hidpp_device *hidpp, u8 *data,
> > > > return ret;
> > > > }
> > > >
> > > > + ret = hidpp20_reprog_controls_raw_event(hidpp, data,
> > > > size);
> > > > + if (ret != 0)
> > > > + return ret;
> > > > +
> > > > if (hidpp->quirks &
> > > > HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
> > > > ret = hidpp10_consumer_keys_raw_event(hidpp,
> > > > data, size);
> > > > if (ret != 0)
> > > > @@ -4264,6 +4476,8 @@ static void hidpp_connect_event(struct
> > > > work_struct *work)
> > > > return;
> > > > }
> > > >
> > > > + hidpp20_reprog_controls_connect(hidpp);
> > > > +
> > > > if (hidpp->quirks &
> > > > HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
> > > > ret = hidpp10_consumer_keys_connect(hidpp);
> > > > if (ret)
> > > > @@ -4436,6 +4650,7 @@ static int hidpp_probe(struct hid_device
> > > > *hdev, const struct hid_device_id *id)
> > > > hidpp->hid_dev = hdev;
> > > > hidpp->name = hdev->name;
> > > > hidpp->quirks = id->driver_data;
> > > > + hidpp->reprog_controls_feature_index = 0xff;
> > > > hid_set_drvdata(hdev, hidpp);
> > > >
> > > > ret = hid_parse(hdev);
^ permalink raw reply
* Re: [PATCH v2 2/6] iio: hid-sensors: align function parenthesis for readability
From: Andy Shevchenko @ 2026-07-03 12:52 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Sanjay Chitroda via B4 Relay, sanjayembeddedse, David Lechner,
Nuno Sá, Andy Shevchenko, Jiri Kosina, Srinivas Pandruvada,
linux-iio, linux-kernel, linux-input
In-Reply-To: <20260702182015.303db93c@jic23-huawei>
On Thu, Jul 02, 2026 at 06:20:15PM +0100, Jonathan Cameron wrote:
> On Thu, 02 Jul 2026 21:47:59 +0530
> Sanjay Chitroda via B4 Relay <devnull+sanjayembeddedse.gmail.com@kernel.org> wrote:
>
> > Adjust alignment of parentheses across HID sensor IIO drivers to
> > improve readability and maintain consistency with kernel coding style.
> >
> > While updating the formatting, group related arguments consistently in
> > multi-line function signatures where appropriate.
> >
> > No functional change intended.
>
> Whilst I appreciate this code isn't quite in line with standards
> and usually like that stuff to be fixed up, in this particular case
> this is a massive amount of churn. That churn will make backporting
> fixes etc messier, so I'd like input on whether others consider this
> one worthwhile. Jiri, Srinivas, Andy etc. What do you think?
I am fine as long as Srinivas is. I understand pros and cons of this, but from
time to time we have patches à la this one that messes up with backporting but
were accepted as a good part of some bigger series.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 5/7] platform/chrome: of_hw_prober: Add delay for hana trackpads
From: sashiko-bot @ 2026-07-03 12:10 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: devicetree, linux-input, conor+dt, dmitry.torokhov, robh
In-Reply-To: <20260703115601.1323491-6-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The result of regulator_is_enabled() is assigned to or bitwise-ANDed with a boolean variable, causing negative error codes to incorrectly evaluate to true and skip required hardware initialization delays.
--
commit 1aa1abec8abc658e89bd1583fb36c5b23bda7cf8
Author: Chen-Yu Tsai <wenst@chromium.org>
platform/chrome: of_hw_prober: Add delay for hana trackpads
Up until now, the MT8173 elm/hana device tree has set the dedicated
regulator supplying the trackpad as always-on, simply because the Elan
driver was missing proper delays. As a result the delay for the
Synaptics trackpad was also omitted, as it was not strictly required
under such a model and delayed the availability of the trackpad to the
user.
> The Elan driver recently gained proper delays after power-up, with
> adaptive skipping of the delay if the regulator was originally
> on. The I2C HID driver and I2C OF component prober library gained
> similar adaptive delay skipping. The device tree will be fixed to have
> the regulator not be always on, and proper post-power-on delay time
> added to the I2C HID device.
[Severity: High]
Does the adaptive skipping logic mentioned here safely handle negative error
codes from regulator_is_enabled()?
Looking at the implementations of this adaptive skipping,
regulator_is_enabled() can return a negative error code (e.g., -EIO, -EBUSY)
on failure, but this result is being directly assigned to or bitwise-ANDed
with a boolean.
For example, in drivers/hid/i2c-hid/i2c-hid-of.c:i2c_hid_of_power_up():
for (unsigned int i = 0; i < ARRAY_SIZE(ihid_of->supplies); i++)
supply_was_enabled &= regulator_is_enabled(ihid_of->supplies[i].consumer);
Since supply_was_enabled is promoted to int for the bitwise AND, does this
outcome depend on the parity of the error code? It looks like odd errors
(like -EIO) will yield true (skipping the delay), while even errors (like
-EBUSY) will yield false.
Similarly, in drivers/i2c/i2c-core-of-prober.c:
i2c_of_probe_simple_enable_regulator():
supply_was_on = regulator_is_enabled(ctx->supply);
And in drivers/input/mouse/elan_i2c_core.c:elan_probe() and elan_resume():
supply_was_enabled = regulator_is_enabled(data->vcc);
Since any non-zero integer evaluates to true in C, assigning a negative error
code directly to a bool means all errors will incorrectly signal that the
regulator was already on.
Could this cause a regression where a transient I2C or hardware error when
querying the regulator state makes the driver incorrectly skip required
hardware initialization delays and fail the device probe?
> Also add the post-power-on delay to the ChromeOS OF component prober,
> so that if the regulator is off at the time of probing, the prober knows
> to wait for the hardware to initialize.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=5
^ permalink raw reply
* Re: [PATCH v2 4/7] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
From: sashiko-bot @ 2026-07-03 12:08 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: dmitry.torokhov, devicetree, linux-input, conor+dt, robh
In-Reply-To: <20260703115601.1323491-5-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The prober unconditionally sleeps for 100ms during the cleanup path even when no matching I2C component is found, unnecessarily delaying system boot.
--
commit 772a8f5b00b1078ef6740a0746df36699760f2f2
Author: Chen-Yu Tsai <wenst@chromium.org>
i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
This patch defers regulator disabling in the I2C OF component prober by adding
a 100ms delay. The delay allows asynchronously probing drivers to catch and
increase the regulator enable count, avoiding hardware re-initialization.
> diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
> index f274e260353cd..17495125467ed 100644
> --- a/drivers/i2c/i2c-core-of-prober.c
> +++ b/drivers/i2c/i2c-core-of-prober.c
> @@ -243,11 +243,23 @@ static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_o
> return 0;
> }
>
> -static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c_of_probe_simple_ctx *ctx)
> +static void i2c_of_probe_simple_disable_regulator(struct device *dev,
> + struct i2c_of_probe_simple_ctx *ctx,
> + bool defer_disable)
> {
> if (!ctx->supply)
> return;
>
> + /*
> + * Wait a bit of time for async drivers to probe and increase the
> + * regulator enable count. This allows the drivers to check and
> + * skip waiting for re-initialization.
> + */
> + if (defer_disable) {
> + dev_dbg(dev, "Deferring regulator disable\n");
> + msleep(100);
> + }
> +
[ ... ]
> @@ -409,7 +421,7 @@ void i2c_of_probe_simple_cleanup(struct device *dev, void *data)
> i2c_of_probe_simple_disable_gpio(dev, ctx);
> i2c_of_probe_simple_put_gpiod(ctx);
>
> - i2c_of_probe_simple_disable_regulator(dev, ctx);
> + i2c_of_probe_simple_disable_regulator(dev, ctx, true);
[Severity: Medium]
Does this unconditionally sleep for 100ms even when no matching I2C component
is found?
If i2c_of_probe_component() iterates over potential components but finds none,
it calls ops->cleanup(dev, ctx), which invokes i2c_of_probe_simple_cleanup().
Because defer_disable is hardcoded to true here,
i2c_of_probe_simple_disable_regulator() will execute the 100ms sleep.
Could this result in an unnecessary 100ms boot delay on systems missing the
probed component, since no async driver will ever probe in this failure path?
> i2c_of_probe_simple_put_supply(ctx);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=4
^ permalink raw reply
* Re: [PATCH v2 2/7] HID: i2c-hid-of: skip post-power-on delay if already powered on
From: sashiko-bot @ 2026-07-03 12:07 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: devicetree, robh, conor+dt, linux-input, dmitry.torokhov
In-Reply-To: <20260703115601.1323491-3-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bitwise AND assignment (`&=`) between a `bool` and the `int` return value of `regulator_is_enabled()` causes pseudo-random evaluation based on the least significant bit (LSB) of the integer, potentially skipping mandatory delays.
- [High] Skipping the post-power-on delay based solely on the current state of `regulator_is_enabled()` introduces a race condition for devices sharing a power rail.
--
commit 14324865d2e3b5ccaf90d6ff025e907befb17313
Author: Chen-Yu Tsai <wenst@chromium.org>
HID: i2c-hid-of: skip post-power-on delay if already powered on
This patch checks if the regulator supplies for an i2c-hid device are
already enabled during probe. If all supplies are already on, it skips
the post-power-on delay to save boot time.
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
> --- a/drivers/hid/i2c-hid/i2c-hid-of.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of.c
> @@ -46,8 +47,12 @@ static int i2c_hid_of_power_up(struct i2chid_ops *ops)
> {
> struct i2c_hid_of *ihid_of = container_of(ops, struct i2c_hid_of, ops);
> struct device *dev = &ihid_of->client->dev;
> + bool supply_was_enabled = true;
> int ret;
>
> + for (unsigned int i = 0; i < ARRAY_SIZE(ihid_of->supplies); i++)
> + supply_was_enabled &= regulator_is_enabled(ihid_of->supplies[i].consumer);
[Severity: High]
Could this bitwise operation cause a regression by inadvertently evaluating
the least significant bit of the return value?
The function regulator_is_enabled() returns an int (positive state, 0, or
a negative error). In C, performing a bitwise AND (&=) between a bool
(promoted to integer 1) and an int isolates the least significant bit.
For example, if regulator_is_enabled() returns a negative error code like
-ENODEV (-19), the least significant bit is 1, causing supply_was_enabled
to evaluate to true despite the error. If it returns an even positive
state like 2, the least significant bit is 0, making it false.
[ ... ]
> if (ret) {
> dev_warn(dev, "Failed to enable supplies: %d\n", ret);
> return ret;
> }
>
> - if (ihid_of->post_power_delay_ms)
> + dev_dbg(dev, "supply was %s.\n", str_on_off(supply_was_enabled));
> + if (!supply_was_enabled && ihid_of->post_power_delay_ms)
> msleep(ihid_of->post_power_delay_ms);
[Severity: High]
Is it safe to skip this delay based solely on whether the regulator is
currently enabled?
If two devices (e.g. a touchscreen and a trackpad) share the same
non-always-on regulator and probe asynchronously:
Device 1 starts probing, calls regulator_bulk_enable(), and begins
waiting its post-power delay.
Device 2 concurrently probes in i2c_hid_of_power_up(), sees
regulator_is_enabled() is true, and skips its post_power_delay_ms here.
Device 2 would then immediately issue I2C transactions to a newly powered
device that might not have finished initializing, potentially resulting
in NACKs and a probe regression.
Does the regulator framework provide a way to ensure the rail has been on
for the required duration, rather than just currently enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=2
^ permalink raw reply
* Re: [PATCH v2 1/7] Input: elan_i2c - Wait for initialization after enabling regulator supply
From: sashiko-bot @ 2026-07-03 12:06 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: conor+dt, devicetree, linux-input, robh, dmitry.torokhov
In-Reply-To: <20260703115601.1323491-2-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The return value of `regulator_is_enabled()` is implicitly cast to `bool`, causing negative error codes to evaluate as true and incorrectly bypassing the required power-on delay.
- [High] Skipping the power-on delay for shared regulators is susceptible to unsafe power domain assumptions and TOCTOU races, leading to bypassed hardware initialization.
--
commit 62ef124436c7ca613cd0f8d922094f42cdd2a0b8
Author: Chen-Yu Tsai <wenst@chromium.org>
Input: elan_i2c - Wait for initialization after enabling regulator supply
This commit adds proper delays after enabling the regulator supply for Elan
trackpad controllers, which require time for hardware and firmware to
initialize. It attempts to avoid boot time impact by skipping the delay if
the regulator was already enabled.
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index fee1796da3d08..110c662cef96f 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
[ ... ]
> @@ -1187,6 +1189,7 @@ static int elan_probe(struct i2c_client *client)
> struct device *dev = &client->dev;
> struct elan_tp_data *data;
> unsigned long irqflags;
> + bool supply_was_enabled;
> int error;
>
> if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_I2C) &&
> @@ -1218,6 +1221,8 @@ static int elan_probe(struct i2c_client *client)
> if (IS_ERR(data->vcc))
> return dev_err_probe(dev, PTR_ERR(data->vcc), "Failed to get 'vcc' regulator\n");
>
> + supply_was_enabled = regulator_is_enabled(data->vcc);
[Severity: High]
In elan_probe(), regulator_is_enabled() returns an int, which can include
negative error codes like -EPROBE_DEFER or -EREMOTEIO. Will assigning this to
the bool supply_was_enabled cause negative error codes to evaluate as true?
If this happens, it looks like it would incorrectly bypass the power-on delay
and lead to a permanent -ENXIO probe failure later rather than allowing a
proper deferral.
> +
> error = regulator_enable(data->vcc);
> if (error) {
> dev_err(dev, "Failed to enable regulator: %d\n", error);
> @@ -1231,6 +1236,9 @@ static int elan_probe(struct i2c_client *client)
> return error;
> }
>
> + if (!supply_was_enabled)
> + msleep(ETP_POWER_ON_DELAY);
[Severity: High]
Is it safe to assume that a regulator that is already enabled has been powered
on for a sufficient amount of time?
If the trackpad shares a regulator with another device (like a touchscreen)
that just finished its own probe milliseconds earlier, the regulator would
already be enabled, but the trackpad hardware wouldn't be fully initialized.
Could this cause elan_probe() to skip the required hardware initialization
delay and fail the subsequent i2c_smbus_read_byte() check?
[ ... ]
> @@ -1374,11 +1382,16 @@ static int elan_resume(struct device *dev)
> int error;
>
> if (!device_may_wakeup(dev)) {
> + bool supply_was_enabled = regulator_is_enabled(data->vcc);
[Severity: High]
In elan_resume(), could the same boolean cast issue happen if
regulator_is_enabled() encounters a transient failure during system resume,
causing the resume delay to be skipped?
> +
> error = regulator_enable(data->vcc);
> if (error) {
> dev_err(dev, "error %d enabling regulator\n", error);
> goto err;
> }
> +
> + if (!supply_was_enabled)
> + msleep(ETP_POWER_ON_DELAY);
[Severity: High]
Could the same shared-regulator timing race occur during the system resume
path, skipping the initialization delay if another device on the same shared
power domain resumed just before the trackpad?
> }
>
> error = elan_set_power(data, true);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=1
^ permalink raw reply
* Re: [PATCH v2 3/7] i2c: of-prober: skip post-power-on delay if already powered on
From: sashiko-bot @ 2026-07-03 12:05 UTC (permalink / raw)
To: Chen-Yu Tsai; +Cc: conor+dt, dmitry.torokhov, robh, linux-input, devicetree
In-Reply-To: <20260703115601.1323491-4-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The integer return value of `regulator_is_enabled()` (which can be a negative error code) is implicitly cast to or bitwise-ANDed with `bool` variables, leading to incorrect evaluations that inadvertently skip required hardware power-on delays.
- [Medium] Shared regulator logic flaw and TOCTOU race allow skipping required power-on delays.
Pre-existing issues:
- [High] Optional regulator supplies missing from the device tree incorrectly cause the prober to fail, contradicting the intended optional behavior.
--
commit 7072efff03a9502c14f9b2eb5a3863213e3992ef
Author: Chen-Yu Tsai <wenst@chromium.org>
i2c: of-prober: skip post-power-on delay if already powered on
This commit modifies the I2C OF prober to check if regulator supplies are
already enabled before applying a post-power-on delay. If the supply is
already active, it skips the delay to avoid unnecessary boot time penalties.
> diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
> index 6a82b03809d4f..f274e260353cd 100644
> --- a/drivers/i2c/i2c-core-of-prober.c
> +++ b/drivers/i2c/i2c-core-of-prober.c
[ ... ]
> @@ -219,19 +220,25 @@ static void i2c_of_probe_simple_put_supply(struct i2c_of_probe_simple_ctx *ctx)
>
> static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_of_probe_simple_ctx *ctx)
> {
> + bool supply_was_on;
> int ret;
>
> if (!ctx->supply)
> return 0;
>
> - dev_dbg(dev, "Enabling regulator supply \"%s\"\n", ctx->opts->supply_name);
> + supply_was_on = regulator_is_enabled(ctx->supply);
[Severity: High]
Could this introduce a regression by skipping required hardware power-on
delays if regulator_is_enabled() fails?
The integer return value of regulator_is_enabled() can be a negative error
code, but it is implicitly cast to the bool variable supply_was_on without
error checking. Any non-zero integer, including negative values, becomes true.
If regulator_is_enabled() returns an error, the code will consider the supply
as already on, causing the device to skip the mandatory post-power-on delay
and fail to probe because it has not finished initializing.
I noticed similar patterns in other drivers:
- drivers/hid/i2c-hid/i2c-hid-of.c:i2c_hid_of_power_up()
- drivers/input/mouse/elan_i2c_core.c:elan_probe()
- drivers/input/mouse/elan_i2c_core.c:elan_resume()
[Severity: Medium]
Is there a potential regression here regarding a shared regulator TOCTOU race
condition?
If two devices share a regulator and probe concurrently, Driver A might enable
the regulator. When Driver B checks regulator_is_enabled(), it returns true.
Driver B will then skip its required power-on delay and attempt to communicate
with the device.
If Driver A enabled the regulator less than the required delay time ago,
Driver B will access its device before the hardware has initialized, leading
to probe failure or I2C communication errors.
> +
> + dev_dbg(dev, "Enabling regulator supply \"%s\" (was %s)\n", ctx->opts->supply_name,
> + str_on_off(supply_was_on));
>
> ret = regulator_enable(ctx->supply);
> if (ret)
> return ret;
>
> - if (ctx->opts->post_power_on_delay_ms)
> + if (!supply_was_on && ctx->opts->post_power_on_delay_ms) {
> + dev_dbg(dev, "Waiting after enabling regulator\n");
> msleep(ctx->opts->post_power_on_delay_ms);
> + }
>
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but I noticed optional regulator supplies missing
from the device tree incorrectly cause the prober to fail, contradicting the
intended optional behavior.
drivers/i2c/i2c-core-of-prober.c:i2c_of_probe_simple_get_supply() {
...
supply = of_regulator_get_optional(dev, node, supply_name);
if (IS_ERR(supply)) {
return dev_err_probe(dev, PTR_ERR(supply),
"Failed to get regulator supply \"%s\" from %pOF\n",
supply_name, node);
}
...
}
of_regulator_get_optional() returns -ENODEV when the supply is not in the
device tree. The function checks IS_ERR(supply) and propagates the error,
failing the probe unconditionally on hardware platforms where the device tree
omits the optional regulator.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=3
^ permalink raw reply
* [PATCH v2 7/7] arm64: dts: mediatek: mt8192-asurada-spherion: Add Synaptics trackpad's supply
From: Chen-Yu Tsai @ 2026-07-03 11:56 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
Cc: Chen-Yu Tsai, linux-mediatek, devicetree, linux-arm-kernel,
chrome-platform, linux-input, linux-i2c, linux-kernel,
stable+noautosel
In-Reply-To: <20260703115601.1323491-1-wenst@chromium.org>
The Synaptics trackpad, like the Elan trackpad option, is fed from the
system 3.3V power rail. Add it to the trackpad device node.
Also add the correct post-power-on delay, even though in practice it is
not required. The Synaptics trackpad requires 100ms after power-on (or
deasserting the reset, whichever comes later) to fully initialize. The
power is always on and the reset pin is not routed out, so the
implementation could try skipping the delay.
Cc: <stable+noautosel@kernel.org> # Without driver changes only lengthens probe time
Fixes: 925ebc0cd55c ("arm64: dts: mt8192-asurada-spherion: Add Synaptics trackpad support")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
I think this shouldn't be backported, as backporting it without the
driver enhancements just delays the trackpad probing with no real
gains.
---
arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts b/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts
index 163960f58db5..147a8e9a3a71 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada-spherion-r0.dts
@@ -90,6 +90,8 @@ trackpad@2c {
hid-descr-addr = <0x20>;
interrupts-extended = <&pio 15 IRQ_TYPE_LEVEL_LOW>;
wakeup-source;
+ vdd-supply = <&pp3300_u>;
+ post-power-on-delay-ms = <100>;
status = "fail-needs-probe";
};
};
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v2 6/7] arm64: dts: mediatek: mt8173-elm-hana: Unmark trackpad supply as always-on
From: Chen-Yu Tsai @ 2026-07-03 11:55 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
Cc: Chen-Yu Tsai, linux-mediatek, devicetree, linux-arm-kernel,
chrome-platform, linux-input, linux-i2c, linux-kernel
In-Reply-To: <20260703115601.1323491-1-wenst@chromium.org>
Up until now, the MT8173 elm/hana device tree has set the dedicated
regulator supplying the trackpad as always-on, simply because the Elan
driver was missing proper delays. As a result the delay for the
Synaptics trackpad was also omitted, as it was not strictly required
under such a model and delayed the availability of the trackpad to the
user.
The Elan driver recently gained proper delays after power up, with
opportunistic skipping of the delay when the regulator was originally
on. The I2C HID driver gained similar opportunistic delay skipping.
So has the I2C OF component prober library.
Now fix the device tree to have the regulator not be always on, and
let the I2C HID device have the correct post-power-on delay time.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi | 8 +-------
arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi | 1 -
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
index 1004eb8ea52c..b9e311fcd9a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
@@ -62,13 +62,7 @@ trackpad2: trackpad@2c {
pinctrl-0 = <&trackpad_irq>;
reg = <0x2c>;
hid-descr-addr = <0x0020>;
- /*
- * The trackpad needs a post-power-on delay of 100ms,
- * but at time of writing, the power supply for it on
- * this board is always on. The delay is therefore not
- * added to avoid impacting the readiness of the
- * trackpad.
- */
+ post-power-on-delay-ms = <100>;
vdd-supply = <&mt6397_vgp6_reg>;
wakeup-source;
status = "fail-needs-probe";
diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
index a0573bc359fb..6b9f47f515c7 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
@@ -1093,7 +1093,6 @@ mt6397_vgp6_reg: ldo_vgp6 {
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
regulator-enable-ramp-delay = <218>;
- regulator-always-on;
};
mt6397_vibr_reg: ldo_vibr {
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v2 5/7] platform/chrome: of_hw_prober: Add delay for hana trackpads
From: Chen-Yu Tsai @ 2026-07-03 11:55 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti
Cc: Chen-Yu Tsai, linux-mediatek, devicetree, linux-arm-kernel,
chrome-platform, linux-input, linux-i2c, linux-kernel
In-Reply-To: <20260703115601.1323491-1-wenst@chromium.org>
Up until now, the MT8173 elm/hana device tree has set the dedicated
regulator supplying the trackpad as always-on, simply because the Elan
driver was missing proper delays. As a result the delay for the
Synaptics trackpad was also omitted, as it was not strictly required
under such a model and delayed the availability of the trackpad to the
user.
The Elan driver recently gained proper delays after power-up, with
adaptive skipping of the delay if the regulator was originally
on. The I2C HID driver and I2C OF component prober library gained
similar adaptive delay skipping. The device tree will be fixed to have
the regulator not be always on, and proper post-power-on delay time
added to the I2C HID device.
Also add the post-power-on delay to the ChromeOS OF component prober,
so that if the regulator is off at the time of probing, the prober knows
to wait for the hardware to initialize.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/platform/chrome/chromeos_of_hw_prober.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/chromeos_of_hw_prober.c b/drivers/platform/chrome/chromeos_of_hw_prober.c
index 8562a0e89dc6..54d8941617e2 100644
--- a/drivers/platform/chrome/chromeos_of_hw_prober.c
+++ b/drivers/platform/chrome/chromeos_of_hw_prober.c
@@ -70,10 +70,8 @@ static const struct chromeos_i2c_probe_data chromeos_i2c_probe_hana_trackpad = {
/*
* ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init.
* Synaptics trackpad needs 100 ms.
- * However, the regulator is set to "always-on", presumably to
- * avoid this delay. The ELAN driver is also missing delays.
*/
- .post_power_on_delay_ms = 0,
+ .post_power_on_delay_ms = 110,
},
};
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox