* [PATCH AUTOSEL 6.17-6.1] HID: apple: Add SONiX AK870 PRO to non_apple_keyboards quirk list
From: Sasha Levin @ 2025-11-24 8:06 UTC (permalink / raw)
To: patches, stable
Cc: April Grimoire, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input
In-Reply-To: <20251124080644.3871678-1-sashal@kernel.org>
From: April Grimoire <april@aprilg.moe>
[ Upstream commit 743c81cdc98fd4fef62a89eb70efff994112c2d9 ]
SONiX AK870 PRO keyboard pretends to be an apple keyboard by VID:PID,
rendering function keys not treated properly. Despite being a
SONiX USB DEVICE, it uses a different name, so adding it to the list.
Signed-off-by: April Grimoire <april@aprilg.moe>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Comprehensive Analysis
### 1. Commit Message Analysis
The commit "HID: apple: Add SONiX AK870 PRO to non_apple_keyboards quirk
list" addresses a specific hardware compatibility issue. The SONiX AK870
PRO keyboard incorrectly identifies itself using Apple's Vendor ID (VID)
and Product ID (PID). This causes the `hid-apple` driver to mistakenly
apply Apple-specific function key behavior (where F1-F12 act as
media/special keys by default, requiring the Fn key to access standard
F-key functions).
The commit has no "Cc: stable" tag or "Fixes:" tag, but clearly
describes a hardware quirk fix.
### 2. Deep Code Research & Mechanism
The change modifies `drivers/hid/hid-apple.c` by adding a single entry
to the `non_apple_keyboards` array.
**The Mechanism:**
The `hid-apple` driver contains a function
`apple_is_non_apple_keyboard()` which iterates through the
`non_apple_keyboards` list:
```c
static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
{ "SONiX USB DEVICE" },
{ "SONiX AK870 PRO" }, /* Added by this commit */
/* ... other existing entries like Keychron, GANSS ... */
};
static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
{
/* ... iterates array and checks strncmp(hdev->name, ...) ... */
}
```
When a device matches an entry in this list, the driver sets the
`APPLE_IS_NON_APPLE` quirk. This flag fundamentally alters how the
driver processes input, specifically preventing the swapping of function
keys and Option/Command keys that is typical for genuine Apple hardware.
**Code Context:**
This infrastructure was introduced specifically to handle the growing
number of mechanical keyboards that use Apple's protocols/IDs for
compatibility but have PC physical layouts. The commit adds one string
literal to this existing, stable lookup table.
### 3. Feature vs. Bug Fix Classification
While technically "adding" a device ID string, this falls squarely under
the **Quirks and Workarounds** exception in the stable kernel rules.
- **The Bug:** Users of this specific keyboard currently experience
broken function key behavior (F1-F12 do not work as expected for a PC
layout).
- **The Fix:** A hardware-specific quirk that restores correct
functionality.
### 4. Risk and Impact Assessment
- **Impact:** High for owners of this device (restores usability). Zero
for everyone else.
- **Regression Risk:** **Negligible.** The change is a string addition
to a static array. The logic uses `strncmp` against the device name.
It is physically impossible for this change to affect any device that
does not report its name starting with "SONiX AK870 PRO".
- **Mainline Status:** The commit was merged recently (late October
2025). The pattern is well-established and has been used for other
keyboards (e.g., Keychron, Hailuck) without incident.
### 5. Stable Kernel Rules Compliance
- **Fixes a real bug?** Yes, function key behavior is incorrect without
it.
- **No new features?** Yes, it simply enables correct existing driver
behavior for this hardware.
- **Small and contained?** Yes, one line of code.
- **Valid Exception?** Yes, explicitly allowed under "Hardware-specific
quirks for broken/buggy devices."
### Conclusion
This is a textbook candidate for a stable backport. It fixes a user-
visible hardware regression (incorrect key mapping) using a low-risk,
established quirk mechanism. The lack of a "Cc: stable" tag is likely an
oversight common with trivial hardware ID additions, but it fully
qualifies under stable maintenance rules.
**YES**
drivers/hid/hid-apple.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 61404d7a43ee1..57da4f86a9fa7 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -355,6 +355,7 @@ static const struct apple_key_translation swapped_fn_leftctrl_keys[] = {
static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
{ "SONiX USB DEVICE" },
+ { "SONiX AK870 PRO" },
{ "Keychron" },
{ "AONE" },
{ "GANSS" },
--
2.51.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.17] HID: lenovo: fixup Lenovo Yoga Slim 7x Keyboard rdesc
From: Sasha Levin @ 2025-11-24 8:06 UTC (permalink / raw)
To: patches, stable
Cc: Lauri Tirkkonen, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input
In-Reply-To: <20251124080644.3871678-1-sashal@kernel.org>
From: Lauri Tirkkonen <lauri@hacktheplanet.fi>
[ Upstream commit a45f15808fb753a14c6041fd1e5bef5d552bd2e3 ]
The keyboard of this device has the following in its report description
for Usage (Keyboard) in Collection (Application):
# 0x15, 0x00, // Logical Minimum (0) 52
# 0x25, 0x65, // Logical Maximum (101) 54
# 0x05, 0x07, // Usage Page (Keyboard) 56
# 0x19, 0x00, // Usage Minimum (0) 58
# 0x29, 0xdd, // Usage Maximum (221) 60
# 0x81, 0x00, // Input (Data,Arr,Abs) 62
Since the Usage Min/Max range exceeds the Logical Min/Max range,
keypresses outside the Logical range are not recognized. This includes,
for example, the Japanese language keyboard variant's keys for |, _ and
\.
Fixup the report description to make the Logical range match the Usage
range, fixing the interpretation of keypresses above 101 on this device.
Signed-off-by: Lauri Tirkkonen <lauri@hacktheplanet.fi>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
HID: lenovo: fixup Lenovo Yoga Slim 7x Keyboard rdesc
**1. COMMIT MESSAGE ANALYSIS**
- **Problem:** The commit addresses a firmware bug in the Lenovo Yoga
Slim 7x keyboard HID descriptor. The "Logical Maximum" (101) is
defined incorrectly as smaller than the "Usage Maximum" (221).
- **User Impact:** This causes the OS to ignore keypresses with usage
codes above 101. This specifically breaks functionality for keys like
`|`, `_`, and `\` on Japanese keyboard layouts, rendering the device
partially unusable for those users.
- **Fix:** The patch modifies the report descriptor at runtime to align
the Logical Maximum with the Usage Maximum.
- **Stable Context:** This is a classic "quirk" or "workaround" for
broken hardware, which is a standard exception for stable backports.
**2. DEEP CODE RESEARCH**
- **Mechanism:** The commit implements a fixup that intercepts the
report descriptor before the HID core parses it.
- **Specific Changes:**
1. Adds a new device ID
`I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD` (0x8987).
2. Defines the specific byte sequence of the broken descriptor to
identify it uniquely:
```151:157:drivers/hid/hid-lenovo.c
static const __u8 lenovo_yoga7x_kbd_need_fixup_collection[] = {
0x15, 0x00, // Logical Minimum (0)
0x25, 0x65, // Logical Maximum (101)
0x05, 0x07, // Usage Page (Keyboard)
```
3. Updates the `lenovo_report_fixup` function to apply the patch only
when the device ID matches *and* the descriptor content matches:
```189:192:drivers/hid/hid-lenovo.c
case I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD:
if (*rsize == 176 &&
memcmp(&rdesc[52], lenovo_yoga7x_kbd_need_fixup_collection,
sizeof(lenovo_yoga7x_kbd_need_fixup_collection)) == 0) {
```
- **Safety:** The fix is defensive. It requires an exact match of the
device product ID and the descriptor bytes. It uses established
patterns found elsewhere in the driver (e.g., for the ThinkPad Pro
Dock).
**3. STABLE KERNEL RULES ASSESSMENT**
- **Obviously correct and tested:** The fix uses standard HID subsystem
mechanisms for correcting buggy hardware descriptors.
- **Fixes real bug:** Without this patch, specific keys do not function
on this hardware.
- **Small and contained:** The changes are surgical, affecting only
`drivers/hid/hid-lenovo.c` and `drivers/hid/hid-ids.h`.
- **Exceptions Applied:**
- **NEW DEVICE IDs:** Adds support for the Yoga Slim 7x (allowed).
- **QUIRKS/WORKAROUNDS:** Fixes the broken report descriptor
(allowed).
**4. RISK VS BENEFIT**
- **Benefit:** High for affected users. It makes the keyboard fully
functional.
- **Risk:** Very Low. The change is additive and guarded by a specific
device ID check. It cannot cause regressions for other Lenovo devices
because the code path is unreachable for them.
**5. CONCLUSION**
This commit is a textbook candidate for stable backporting. It supports
new hardware (Device ID exception) and fixes a firmware bug that breaks
functionality (Quirk exception). It is low-risk, contained, and provides
a necessary fix for users of this specific laptop model.
**YES**
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-lenovo.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5721b8414bbdf..4b1946eb4e7fc 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -715,6 +715,7 @@
#define USB_DEVICE_ID_ITE_LENOVO_YOGA2 0x8350
#define I2C_DEVICE_ID_ITE_LENOVO_LEGION_Y720 0x837a
#define USB_DEVICE_ID_ITE_LENOVO_YOGA900 0x8396
+#define I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD 0x8987
#define USB_DEVICE_ID_ITE8595 0x8595
#define USB_DEVICE_ID_ITE_MEDION_E1239T 0xce50
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 654879814f97a..9cc3e029e9f61 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -148,6 +148,14 @@ static const __u8 lenovo_tpIIbtkbd_need_fixup_collection[] = {
0x81, 0x01, /* Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) */
};
+static const __u8 lenovo_yoga7x_kbd_need_fixup_collection[] = {
+ 0x15, 0x00, // Logical Minimum (0)
+ 0x25, 0x65, // Logical Maximum (101)
+ 0x05, 0x07, // Usage Page (Keyboard)
+ 0x19, 0x00, // Usage Minimum (0)
+ 0x29, 0xDD, // Usage Maximum (221)
+};
+
static const __u8 *lenovo_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
@@ -177,6 +185,13 @@ static const __u8 *lenovo_report_fixup(struct hid_device *hdev, __u8 *rdesc,
rdesc[260] = 0x01; /* report count (2) = 0x01 */
}
break;
+ case I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD:
+ if (*rsize == 176 &&
+ memcmp(&rdesc[52], lenovo_yoga7x_kbd_need_fixup_collection,
+ sizeof(lenovo_yoga7x_kbd_need_fixup_collection)) == 0) {
+ rdesc[55] = rdesc[61]; // logical maximum = usage maximum
+ }
+ break;
}
return rdesc;
}
@@ -1538,6 +1553,8 @@ static const struct hid_device_id lenovo_devices[] = {
USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X12_TAB) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X12_TAB2) },
+ { HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD) },
{ }
};
--
2.51.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.17-6.6] HID: hid-input: Extend Elan ignore battery quirk to USB
From: Sasha Levin @ 2025-11-24 8:06 UTC (permalink / raw)
To: patches, stable
Cc: Mario Limonciello (AMD), Hans de Goede, André Barata,
Jiri Kosina, Sasha Levin, jikos, bentiss, linux-input
In-Reply-To: <20251124080644.3871678-1-sashal@kernel.org>
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
[ Upstream commit 534ca75e8e3b713514b3f2da85dab96831cf5b2a ]
USB Elan devices have the same problem as the I2C ones with a fake
battery device showing up.
Reviewed-by: Hans de Goede <hansg@kernel.org>
Reported-by: André Barata <andretiagob@protonmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220722
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Comprehensive Analysis
### 1. Commit Message Analysis
The commit "HID: hid-input: Extend Elan ignore battery quirk to USB"
addresses a specific hardware compatibility issue where Elan USB
touchscreens incorrectly report a battery device to the OS.
- **Problem:** Users see a non-existent or "fake" battery in their
system tray/power settings, which can cause confusion and interfere
with power management logic.
- **Context:** The message notes that "USB Elan devices have the same
problem as the I2C ones," referencing a known issue.
- **References:** It links to a specific bug report (Bugzilla #220722)
and has a `Reviewed-by` tag from the subsystem maintainer (Hans de
Goede).
### 2. Deep Code Research
- **Code Change:** The patch modifies `drivers/hid/hid-input.c` by
adding a single line to the `hid_battery_quirks` table:
```c
{ HID_USB_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID),
HID_BATTERY_QUIRK_IGNORE },
```
- **Historical Context:** This change mirrors commit `bcc31692a1d1e`
(August 2024), which applied the same `HID_ANY_ID` catch-all quirk for
**I2C** Elan devices. That previous commit was successfully backported
to stable trees.
- **Mechanism:** The `hid-input` driver checks connected devices against
the `hid_battery_quirks` table. When a match is found with
`HID_BATTERY_QUIRK_IGNORE`, the function `hidinput_setup_battery()`
returns early, preventing the creation of the bogus power supply
device in `/sys/class/power_supply/`.
- **Precedent:** The file already contains specific quirks for some Elan
USB devices (e.g., ASUS UX550). This commit generalizes the fix to all
Elan USB devices, cleaning up the approach.
### 3. Stable Kernel Rules Assessment
- **Fixes a Real Bug:** Yes. It prevents the kernel from exposing false
hardware information to userspace.
- **Quirks and Workarounds Exception:** This falls strictly under the
"QUIRKS and WORKAROUNDS" exception category allowed in stable kernels
("Hardware-specific quirks for broken/buggy devices").
- **Small and Contained:** The change is surgical—one line of code added
to a static array. It has no logic complexity or dependencies.
- **Regression Risk:** Extremely Low. It simply disables battery
reporting for a specific vendor's input devices. Since the battery
reporting is known to be broken (always 0% or 1%), ignoring it
restores correct behavior.
- **Mainline Status:** Reviewed by maintainers and tested by the
reporter.
### 4. Conclusion
This commit is an ideal candidate for backporting. It is a one-line
hardware quirk that fixes a user-visible annoyance (fake battery
devices). It follows a pattern already established and backported for
I2C devices, ensuring consistency across different bus types for the
same hardware vendor.
**YES**
drivers/hid/hid-input.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 2c743e35c1d33..bc7de9ef45ecd 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -386,10 +386,11 @@ static const struct hid_device_id hid_battery_quirks[] = {
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_CHROMEBOOK_TROGDOR_POMPOM),
HID_BATTERY_QUIRK_AVOID_QUERY },
/*
- * Elan I2C-HID touchscreens seem to all report a non present battery,
- * set HID_BATTERY_QUIRK_IGNORE for all Elan I2C-HID devices.
+ * Elan HID touchscreens seem to all report a non present battery,
+ * set HID_BATTERY_QUIRK_IGNORE for all Elan I2C and USB HID devices.
*/
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_IGNORE },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_IGNORE },
{}
};
--
2.51.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.17-5.15] HID: elecom: Add support for ELECOM M-XT3URBK (018F)
From: Sasha Levin @ 2025-11-24 8:06 UTC (permalink / raw)
To: patches, stable
Cc: Naoki Ueki, Jiri Kosina, Sasha Levin, jikos, bentiss, linux-input
In-Reply-To: <20251124080644.3871678-1-sashal@kernel.org>
From: Naoki Ueki <naoki25519@gmail.com>
[ Upstream commit cdcbb8e8d10f656642380ee13516290437b52b36 ]
The ELECOM M-XT3URBK trackball has an additional device ID (0x018F), which
shares the same report descriptor as the existing device (0x00FB). However,
the driver does not currently recognize this new ID, resulting in only five
buttons being functional.
This patch adds the new device ID so that all six buttons work properly.
Signed-off-by: Naoki Ueki <naoki25519@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
### Comprehensive Analysis
**1. Commit Message Analysis**
The commit "HID: elecom: Add support for ELECOM M-XT3URBK (018F)"
addresses a specific hardware deficiency where a new revision of the
ELECOM M-XT3URBK trackball (ID 0x018F) is not recognized by the driver.
This results in the device falling back to a default mode where only
five buttons function, instead of the intended six. The commit aims to
add the new ID to the existing driver to enable full functionality. The
message clearly describes the problem (missing button support) and the
solution (adding the ID).
**2. Deep Code Research**
- **Code Change Scope:** The patch modifies three files:
`drivers/hid/hid-elecom.c`, `drivers/hid/hid-ids.h`, and
`drivers/hid/hid-quirks.c`.
- **Mechanism:** The patch performs a rename of the existing macro
`USB_DEVICE_ID_ELECOM_M_XT3URBK` to
`USB_DEVICE_ID_ELECOM_M_XT3URBK_00FB` to differentiate the old
revision (0x00FB) from the new one. It then defines the new ID
`USB_DEVICE_ID_ELECOM_M_XT3URBK_018F` (0x018F).
- **Logic Update:** In `elecom_report_fixup`, the switch statement is
updated to apply the existing `mouse_button_fixup` logic to *both* the
old and new device IDs. This ensures the 6th button is correctly
mapped for the new hardware.
- **Safety Verification:** A codebase search confirms that
`USB_DEVICE_ID_ELECOM_M_XT3URBK` was only used in the three files
modified by the patch. Therefore, the rename is safe and does not
break any external dependencies or out-of-tree modules that rely on
the standard kernel headers.
- **Context:** This uses the existing `hid-elecom` infrastructure. It
does not introduce new logic, only new data (IDs).
**3. Stable Kernel Rules Compliance**
- **Rule Alignment:** The commit complies with the stable kernel rules
requiring fixes to be "obviously correct" and for "real bugs".
- **Exception Category:** This falls squarely under the **"NEW DEVICE
IDs"** exception detailed in `Documentation/process/stable-kernel-
rules.rst`:
> "Adding PCI IDs, USB IDs, ACPI IDs, etc. to existing drivers...
These are trivial one-line additions that enable hardware support...
Rule: The driver must already exist in stable; only the ID is new"
- **Driver Existence:** The `hid-elecom` driver already exists in stable
trees.
**4. Risk Assessment**
- **Regression Risk:** **Very Low**. The changes are declarative. The
existing hardware path remains effectively unchanged (just a renamed
macro constant with the same value). The new path reuses existing,
tested fixup logic.
- **User Impact:** Users with the new revision of this trackball will
regain full functionality (the 6th button). Users with the old
revision are unaffected.
**5. Conclusion**
This commit is a textbook candidate for stable backporting. It fixes a
functional regression for users with newer hardware by adding a device
ID to an existing driver, which is explicitly permitted and encouraged
in stable kernels.
**YES**
drivers/hid/hid-elecom.c | 6 ++++--
drivers/hid/hid-ids.h | 3 ++-
drivers/hid/hid-quirks.c | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-elecom.c b/drivers/hid/hid-elecom.c
index 69771fd350060..981d1b6e96589 100644
--- a/drivers/hid/hid-elecom.c
+++ b/drivers/hid/hid-elecom.c
@@ -75,7 +75,8 @@ static const __u8 *elecom_report_fixup(struct hid_device *hdev, __u8 *rdesc,
*/
mouse_button_fixup(hdev, rdesc, *rsize, 20, 28, 22, 14, 8);
break;
- case USB_DEVICE_ID_ELECOM_M_XT3URBK:
+ case USB_DEVICE_ID_ELECOM_M_XT3URBK_00FB:
+ case USB_DEVICE_ID_ELECOM_M_XT3URBK_018F:
case USB_DEVICE_ID_ELECOM_M_XT3DRBK:
case USB_DEVICE_ID_ELECOM_M_XT4DRBK:
/*
@@ -119,7 +120,8 @@ static const __u8 *elecom_report_fixup(struct hid_device *hdev, __u8 *rdesc,
static const struct hid_device_id elecom_devices[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XGL20DLBK) },
- { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK_00FB) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK_018F) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3DRBK) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT4DRBK) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_DT1URBK) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4b1946eb4e7fc..fb96ded1b4428 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -446,7 +446,8 @@
#define USB_VENDOR_ID_ELECOM 0x056e
#define USB_DEVICE_ID_ELECOM_BM084 0x0061
#define USB_DEVICE_ID_ELECOM_M_XGL20DLBK 0x00e6
-#define USB_DEVICE_ID_ELECOM_M_XT3URBK 0x00fb
+#define USB_DEVICE_ID_ELECOM_M_XT3URBK_00FB 0x00fb
+#define USB_DEVICE_ID_ELECOM_M_XT3URBK_018F 0x018f
#define USB_DEVICE_ID_ELECOM_M_XT3DRBK 0x00fc
#define USB_DEVICE_ID_ELECOM_M_XT4DRBK 0x00fd
#define USB_DEVICE_ID_ELECOM_M_DT1URBK 0x00fe
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index ffd034566e2e1..0bbb7425b935a 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -408,7 +408,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
#if IS_ENABLED(CONFIG_HID_ELECOM)
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XGL20DLBK) },
- { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK_00FB) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK_018F) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3DRBK) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT4DRBK) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_DT1URBK) },
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v3] HID: memory leak in dualshock4_get_calibration_data
From: Jiri Slaby @ 2025-11-24 6:32 UTC (permalink / raw)
To: Eslam Khafagy, roderick.colenbrander, jikos, bentiss, max
Cc: linux-input, linux-kernel, syzbot+4f5f81e1456a1f645bf8, stable
In-Reply-To: <20251122173712.76397-1-eslam.medhat1993@gmail.com>
On 22. 11. 25, 18:37, Eslam Khafagy wrote:
> Function dualshock4_get_calibration_data allocates memory to pointer
> buf .However, the function may exit prematurely due to transfer_failure
> in this case it does not handle freeing memory.
>
> This patch handles memory deallocation at exit.
Isn't this fixed already by:
commit 8513c154f8ad7097653dd9bf43d6155e5aad4ab3
Author: Abdun Nihaal <nihaal@cse.iitm.ac.in>
Date: Mon Nov 10 22:45:50 2025 +0530
HID: playstation: Fix memory leak in dualshock4_get_calibration_data()
?
Anyway, this is a typical use-case for __free(). Why not to use that?
> Reported-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/691560c4.a70a0220.3124cb.0019.GAE@google.com/T/
> Tested-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com
> Fixes: 947992c7fa9e0 ("HID: playstation: DS4: Fix calibration workaround for clone devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eslam Khafagy <eslam.medhat1993@gmail.com>
> ---
> v3:
> * Address issues reported by checkpatch and re-format commit message
> for better readability
> * kfree() is safe so no need to check for NULL pointer
>
> v2: https://lore.kernel.org/all/20251116022723.29857-1-eslam.medhat1993@gmail.com/
> * Adding tag "Cc: stable@vger.kernel.org"
>
> v1: https://lore.kernel.org/all/20251115022323.1395726-1-eslam.medhat1993@gmail.com/
>
>
> drivers/hid/hid-playstation.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 128aa6abd10b..05a8522ace4f 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -1994,9 +1994,6 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
> acc_z_plus = get_unaligned_le16(&buf[31]);
> acc_z_minus = get_unaligned_le16(&buf[33]);
>
> - /* Done parsing the buffer, so let's free it. */
> - kfree(buf);
> -
> /*
> * Set gyroscope calibration and normalization parameters.
> * Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s.
> @@ -2043,6 +2040,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
> ds4->accel_calib_data[2].sens_denom = range_2g;
>
> transfer_failed:
> + /* First free buf if still allocated */
> + kfree(buf);
> +
> /*
> * Sanity check gyro calibration data. This is needed to prevent crashes
> * during report handling of virtual, clone or broken devices not implementing
--
js
suse labs
^ permalink raw reply
* Re: [git pull] Input updates for v6.18-rc6
From: pr-tracker-bot @ 2025-11-22 18:29 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <s7ye6uffvflxyk543emnmjsqyjc6d22scy6qcr52zegssmdtwj@wswhzowey34g>
The pull request you sent on Fri, 21 Nov 2025 22:47:12 -0800:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.18-rc6
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ebd975458deada5eadcc9f4e0f63fc4ce6aa90ea
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v3] HID: memory leak in dualshock4_get_calibration_data
From: Markus Elfring @ 2025-11-22 18:15 UTC (permalink / raw)
To: Eslam Khafagy, linux-input, Benjamin Tissoires, Jiri Kosina,
Max Staudt, Roderick Colenbrander
Cc: linux-kernel, stable, syzbot+4f5f81e1456a1f645bf8
In-Reply-To: <20251122173712.76397-1-eslam.medhat1993@gmail.com>
> Function dualshock4_get_calibration_data allocates memory to pointer
> buf .However, the function may exit prematurely due to transfer_failure
transfer failure.?
> in this case it does not handle freeing memory.
>
> This patch handles memory deallocation at exit.
Would a corresponding imperative wording become helpful for an improved change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc6#n94
Would a summary phrase like “Prevent memory leak in dualshock4_get_calibration_data()”
be nicer?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v3] HID: memory leak in dualshock4_get_calibration_data
From: Max Staudt @ 2025-11-22 18:09 UTC (permalink / raw)
To: Eslam Khafagy, roderick.colenbrander, jikos, bentiss
Cc: linux-input, linux-kernel, syzbot+4f5f81e1456a1f645bf8, stable
In-Reply-To: <20251122173712.76397-1-eslam.medhat1993@gmail.com>
On 11/23/25 2:37 AM, Eslam Khafagy wrote:
> Function dualshock4_get_calibration_data allocates memory to pointer
> buf .However, the function may exit prematurely due to transfer_failure
> in this case it does not handle freeing memory.
>
> This patch handles memory deallocation at exit.
Thanks, Eslam!
Reviewed-by: Max Staudt <max@enpas.org>
^ permalink raw reply
* [PATCH v3] HID: memory leak in dualshock4_get_calibration_data
From: Eslam Khafagy @ 2025-11-22 17:37 UTC (permalink / raw)
To: roderick.colenbrander, jikos, bentiss, max
Cc: linux-input, linux-kernel, Eslam Khafagy,
syzbot+4f5f81e1456a1f645bf8, stable
Function dualshock4_get_calibration_data allocates memory to pointer
buf .However, the function may exit prematurely due to transfer_failure
in this case it does not handle freeing memory.
This patch handles memory deallocation at exit.
Reported-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/691560c4.a70a0220.3124cb.0019.GAE@google.com/T/
Tested-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com
Fixes: 947992c7fa9e0 ("HID: playstation: DS4: Fix calibration workaround for clone devices")
Cc: stable@vger.kernel.org
Signed-off-by: Eslam Khafagy <eslam.medhat1993@gmail.com>
---
v3:
* Address issues reported by checkpatch and re-format commit message
for better readability
* kfree() is safe so no need to check for NULL pointer
v2: https://lore.kernel.org/all/20251116022723.29857-1-eslam.medhat1993@gmail.com/
* Adding tag "Cc: stable@vger.kernel.org"
v1: https://lore.kernel.org/all/20251115022323.1395726-1-eslam.medhat1993@gmail.com/
drivers/hid/hid-playstation.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 128aa6abd10b..05a8522ace4f 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1994,9 +1994,6 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
acc_z_plus = get_unaligned_le16(&buf[31]);
acc_z_minus = get_unaligned_le16(&buf[33]);
- /* Done parsing the buffer, so let's free it. */
- kfree(buf);
-
/*
* Set gyroscope calibration and normalization parameters.
* Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s.
@@ -2043,6 +2040,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
ds4->accel_calib_data[2].sens_denom = range_2g;
transfer_failed:
+ /* First free buf if still allocated */
+ kfree(buf);
+
/*
* Sanity check gyro calibration data. This is needed to prevent crashes
* during report handling of virtual, clone or broken devices not implementing
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v10 04/11] HID: asus: fortify keyboard handshake
From: Denis Benato @ 2025-11-22 13:13 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251122110032.4274-5-lkml@antheas.dev>
On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Handshaking with an Asus device involves sending it a feature report
> with the string "ASUS Tech.Inc." and then reading it back to verify the
> handshake was successful, under the feature ID the interaction will
> take place.
>
> Currently, the driver only does the first part. Add the readback to
> verify the handshake was successful. As this could cause breakages,
> allow the verification to fail with a dmesg error until we verify
> all devices work with it (they seem to).
>
> Since the response is more than 16 bytes, increase the buffer size
> to 64 as well to avoid overflow errors.
>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> ---
> drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a36a8beb35..2a412e10f916 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define FEATURE_REPORT_ID 0x0d
> #define INPUT_REPORT_ID 0x5d
> #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
> #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>
> @@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>
> static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> {
> + /*
> + * The handshake is first sent as a set_report, then retrieved
> + * from a get_report. They should be equal.
> + */
> const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> int ret;
>
> ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> - if (ret < 0)
> - hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> + return ret;
> + }
> +
> + u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> + if (!readbuf)
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> + FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> + } else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> + hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> + FEATURE_KBD_REPORT_SIZE, readbuf);
> + /*
> + * Do not return error if handshake is wrong until this is
> + * verified to work for all devices.
> + */
> + }
>
> return ret;
> }
^ permalink raw reply
* [PATCH v10 11/11] HID: asus: add support for the asus-wmi brightness handler
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
If the asus-wmi brightness handler is available, send the
keyboard brightness events to it instead of passing them
to userspace. If it is not, fall back to sending them to it.
Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 475e34187326..8f30d1b9737e 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -324,6 +324,17 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
usage->hid & HID_USAGE);
}
+ if (usage->type == EV_KEY && value) {
+ switch (usage->code) {
+ case KEY_KBDILLUMUP:
+ return !asus_hid_event(ASUS_EV_BRTUP);
+ case KEY_KBDILLUMDOWN:
+ return !asus_hid_event(ASUS_EV_BRTDOWN);
+ case KEY_KBDILLUMTOGGLE:
+ return !asus_hid_event(ASUS_EV_BRTTOGGLE);
+ }
+ }
+
return 0;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
The keyboard brightness control of Asus WMI keyboards is handled in
kernel, which leads to the shortcut going from brightness 0, to 1,
to 2, and 3.
However, for HID keyboards it is exposed as a key and handled by the
user's desktop environment. For the toggle button, this means that
brightness control becomes on/off. In addition, in the absence of a
DE, the keyboard brightness does not work.
Therefore, expose an event handler for the keyboard brightness control
which can then be used by hid-asus. Since this handler is called from
an interrupt context, defer the actual work to a workqueue.
In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
for maximum brightness since it is shared between hid-asus/asus-wmi.
Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/platform/x86/asus-wmi.c | 46 +++++++++++++++++++---
include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
2 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 84cde34ab6a8..a69464e45ca4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
}
}
+/*
+ * This function is called from hid-asus to inform asus-wmi of brightness
+ * changes initiated by the keyboard backlight keys.
+ */
+int asus_hid_event(enum asus_hid_event event)
+{
+ struct asus_wmi *asus;
+ int brightness;
+
+ guard(spinlock_irqsave)(&asus_ref.lock);
+ asus = asus_ref.asus;
+ if (!asus || !asus->kbd_led_registered)
+ return -EBUSY;
+
+ brightness = asus->kbd_led_wk;
+
+ switch (event) {
+ case ASUS_EV_BRTUP:
+ brightness += 1;
+ break;
+ case ASUS_EV_BRTDOWN:
+ brightness -= 1;
+ break;
+ case ASUS_EV_BRTTOGGLE:
+ if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
+ brightness = 0;
+ else
+ brightness += 1;
+ break;
+ }
+
+ asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
+ asus->kbd_led_notify = true;
+ queue_work(asus->led_workqueue, &asus->kbd_led_work);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(asus_hid_event);
+
/*
* These functions actually update the LED's, and are called from a
* workqueue. By doing this as separate work rather than when the LED
@@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
{
struct asus_hid_listener *listener;
struct asus_wmi *asus;
- int max_level;
asus = container_of(led_cdev, struct asus_wmi, kbd_led);
- max_level = asus->kbd_led.max_brightness;
scoped_guard(spinlock_irqsave, &asus_ref.lock)
- asus->kbd_led_wk = clamp_val(value, 0, max_level);
+ asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
if (asus->kbd_led_avail)
kbd_led_update(asus);
@@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
asus->kbd_led.brightness_set = kbd_led_set;
asus->kbd_led.brightness_get = kbd_led_get;
- asus->kbd_led.max_brightness = 3;
+ asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
@@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
return;
}
if (code == NOTIFY_KBD_BRTTOGGLE) {
- if (led_value == asus->kbd_led.max_brightness)
+ if (led_value == ASUS_EV_MAX_BRIGHTNESS)
kbd_led_set_by_kbd(asus, 0);
else
kbd_led_set_by_kbd(asus, led_value + 1);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index d347cffd05d5..7b872b5d0960 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -178,6 +178,14 @@ struct asus_hid_listener {
void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
};
+enum asus_hid_event {
+ ASUS_EV_BRTUP,
+ ASUS_EV_BRTDOWN,
+ ASUS_EV_BRTTOGGLE,
+};
+
+#define ASUS_EV_MAX_BRIGHTNESS 3
+
#if IS_REACHABLE(CONFIG_ASUS_WMI)
void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
void set_ally_mcu_powersave(bool enabled);
@@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
int asus_hid_register_listener(struct asus_hid_listener *cdev);
void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
+int asus_hid_event(enum asus_hid_event event);
#else
static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
{
@@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
{
}
+static inline int asus_hid_event(enum asus_hid_event event)
+{
+ return -ENODEV;
+}
#endif
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.52.0
^ permalink raw reply related
* [PATCH v10 05/11] HID: asus: move vendor initialization to probe
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
ROG NKEY devices have multiple HID endpoints, around 3-4. One of those
endpoints has a usage page of 0xff31, and is the one that emits keyboard
shortcuts and controls RGB/backlight. Currently, this driver places
the usage page check under asus_input_mapping and then inits backlight
in asus_input_configured which is unnecessarily complicated and prevents
probe from performing customizations on the vendor endpoint.
Simplify the logic by introducing an is_vendor variable into probe that
checks for usage page 0xff31. Then, use this variable to move backlight
initialization into probe instead of asus_input_configured, and remove
the backlight check from asus_input_mapping.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 2a412e10f916..faac971794c0 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -48,6 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define T100CHI_MOUSE_REPORT_ID 0x06
#define FEATURE_REPORT_ID 0x0d
#define INPUT_REPORT_ID 0x5d
+#define HID_USAGE_PAGE_VENDOR 0xff310000
#define FEATURE_KBD_REPORT_ID 0x5a
#define FEATURE_KBD_REPORT_SIZE 64
#define FEATURE_KBD_LED_REPORT_ID1 0x5d
@@ -127,7 +128,6 @@ struct asus_drvdata {
struct input_dev *tp_kbd_input;
struct asus_kbd_leds *kbd_backlight;
const struct asus_touchpad_info *tp;
- bool enable_backlight;
struct power_supply *battery;
struct power_supply_desc battery_desc;
int battery_capacity;
@@ -318,7 +318,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
static int asus_event(struct hid_device *hdev, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
- if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
+ if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
(usage->hid & HID_USAGE) != 0x00 &&
(usage->hid & HID_USAGE) != 0xff && !usage->type) {
hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
@@ -941,11 +941,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
drvdata->input = input;
- if (drvdata->enable_backlight &&
- !asus_kbd_wmi_led_control_present(hdev) &&
- asus_kbd_register_leds(hdev))
- hid_warn(hdev, "Failed to initialize backlight.\n");
-
return 0;
}
@@ -1018,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
return -1;
}
- /*
- * Check and enable backlight only on devices with UsagePage ==
- * 0xff31 to avoid initializing the keyboard firmware multiple
- * times on devices with multiple HID descriptors but same
- * PID/VID.
- */
- if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
- drvdata->enable_backlight = true;
-
set_bit(EV_REP, hi->input->evbit);
return 1;
}
@@ -1143,8 +1129,11 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
- int ret;
+ struct hid_report_enum *rep_enum;
struct asus_drvdata *drvdata;
+ struct hid_report *rep;
+ bool is_vendor = false;
+ int ret;
drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
if (drvdata == NULL) {
@@ -1228,12 +1217,24 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ /* Check for vendor for RGB init and handle generic devices properly. */
+ rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
+ list_for_each_entry(rep, &rep_enum->report_list, list) {
+ if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
+ is_vendor = true;
+ }
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "Asus hw start failed: %d\n", ret);
return ret;
}
+ if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
+ !asus_kbd_wmi_led_control_present(hdev) &&
+ asus_kbd_register_leds(hdev))
+ hid_warn(hdev, "Failed to initialize backlight.\n");
+
/*
* Check that input registration succeeded. Checking that
* HID_CLAIMED_INPUT is set prevents a UAF when all input devices
--
2.52.0
^ permalink raw reply related
* [PATCH v10 09/11] platform/x86: asus-wmi: remove unused keyboard backlight quirk
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
The quirk for selecting whether keyboard backlight should be controlled
by HID or WMI is not needed anymore, so remove the file containing it.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
.../platform_data/x86/asus-wmi-leds-ids.h | 50 -------------------
1 file changed, 50 deletions(-)
delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
diff --git a/include/linux/platform_data/x86/asus-wmi-leds-ids.h b/include/linux/platform_data/x86/asus-wmi-leds-ids.h
deleted file mode 100644
index 034a039c4e37..000000000000
--- a/include/linux/platform_data/x86/asus-wmi-leds-ids.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
-#define __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
-
-#include <linux/dmi.h>
-#include <linux/types.h>
-
-/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-#if IS_REACHABLE(CONFIG_ASUS_WMI) || IS_REACHABLE(CONFIG_HID_ASUS)
-static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
- {
- .matches = {
- DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
- },
- },
- {
- .matches = {
- DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
- },
- },
- {
- .matches = {
- DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
- },
- },
- {
- .matches = {
- DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
- },
- },
- {
- .matches = {
- DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
- },
- },
- {
- .matches = {
- DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
- },
- },
- {
- .matches = {
- DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
- },
- },
- { },
-};
-#endif
-
-#endif /* __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H */
--
2.52.0
^ permalink raw reply related
* [PATCH v10 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Some ROG laptops expose multiple interfaces for controlling the
keyboard/RGB brightness. This creates a name conflict under
asus::kbd_brightness, where the second device ends up being
named asus::kbd_brightness_1 and they are both broken.
Therefore, register a listener to the asus-wmi brightness device
instead of creating a new one.
Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 65 +++++++-----------------------------------
1 file changed, 10 insertions(+), 55 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 461b0f8efdfb..475e34187326 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -27,7 +27,6 @@
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/platform_data/x86/asus-wmi.h>
-#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
#include <linux/input/mt.h>
#include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
#include <linux/power_supply.h>
@@ -103,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define TRKID_SGN ((TRKID_MAX + 1) >> 1)
struct asus_kbd_leds {
- struct led_classdev cdev;
+ struct asus_hid_listener listener;
struct hid_device *hdev;
struct work_struct work;
unsigned int brightness;
@@ -493,11 +492,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
spin_unlock_irqrestore(&led->lock, flags);
}
-static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
- enum led_brightness brightness)
+static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
+ int brightness)
{
- struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
- cdev);
+ struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
+ listener);
unsigned long flags;
spin_lock_irqsave(&led->lock, flags);
@@ -507,20 +506,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
asus_schedule_work(led);
}
-static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
-{
- struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
- cdev);
- enum led_brightness brightness;
- unsigned long flags;
-
- spin_lock_irqsave(&led->lock, flags);
- brightness = led->brightness;
- spin_unlock_irqrestore(&led->lock, flags);
-
- return brightness;
-}
-
static void asus_kbd_backlight_work(struct work_struct *work)
{
struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
@@ -537,34 +522,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
}
-/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
- * precedence. We only activate HID-based backlight control when the
- * WMI control is not available.
- */
-static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
-{
- struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
- u32 value;
- int ret;
-
- if (!IS_ENABLED(CONFIG_ASUS_WMI))
- return false;
-
- if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
- dmi_check_system(asus_use_hid_led_dmi_ids)) {
- hid_info(hdev, "using HID for asus::kbd_backlight\n");
- return false;
- }
-
- ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
- ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
- hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
- if (ret)
- return false;
-
- return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
-}
-
/*
* We don't care about any other part of the string except the version section.
* Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
@@ -709,14 +666,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
drvdata->kbd_backlight->removed = false;
drvdata->kbd_backlight->brightness = 0;
drvdata->kbd_backlight->hdev = hdev;
- drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
- drvdata->kbd_backlight->cdev.max_brightness = 3;
- drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
- drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
+ drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
spin_lock_init(&drvdata->kbd_backlight->lock);
- ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
+ 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);
@@ -1105,7 +1059,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
if (drvdata->kbd_backlight) {
const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
- drvdata->kbd_backlight->cdev.brightness };
+ drvdata->kbd_backlight->brightness };
ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
if (ret < 0) {
hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
@@ -1231,7 +1185,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
- !asus_kbd_wmi_led_control_present(hdev) &&
asus_kbd_register_leds(hdev))
hid_warn(hdev, "Failed to initialize backlight.\n");
@@ -1278,6 +1231,8 @@ static void asus_remove(struct hid_device *hdev)
unsigned long flags;
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;
spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
--
2.52.0
^ permalink raw reply related
* [PATCH v10 04/11] HID: asus: fortify keyboard handshake
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Handshaking with an Asus device involves sending it a feature report
with the string "ASUS Tech.Inc." and then reading it back to verify the
handshake was successful, under the feature ID the interaction will
take place.
Currently, the driver only does the first part. Add the readback to
verify the handshake was successful. As this could cause breakages,
allow the verification to fail with a dmesg error until we verify
all devices work with it (they seem to).
Since the response is more than 16 bytes, increase the buffer size
to 64 as well to avoid overflow errors.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index c1a36a8beb35..2a412e10f916 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define FEATURE_REPORT_ID 0x0d
#define INPUT_REPORT_ID 0x5d
#define FEATURE_KBD_REPORT_ID 0x5a
-#define FEATURE_KBD_REPORT_SIZE 16
+#define FEATURE_KBD_REPORT_SIZE 64
#define FEATURE_KBD_LED_REPORT_ID1 0x5d
#define FEATURE_KBD_LED_REPORT_ID2 0x5e
@@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
{
+ /*
+ * The handshake is first sent as a set_report, then retrieved
+ * from a get_report. They should be equal.
+ */
const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
int ret;
ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
- if (ret < 0)
- hid_err(hdev, "Asus failed to send init command: %d\n", ret);
+ if (ret < 0) {
+ hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
+ return ret;
+ }
+
+ u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
+ if (!readbuf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, report_id, readbuf,
+ FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
+ if (ret < 0) {
+ hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
+ } else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
+ hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
+ FEATURE_KBD_REPORT_SIZE, readbuf);
+ /*
+ * Do not return error if handshake is wrong until this is
+ * verified to work for all devices.
+ */
+ }
return ret;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v10 07/11] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Some devices, such as the Z13 have multiple Aura devices connected
to them by USB. In addition, they might have a WMI interface for
RGB. In Windows, Armoury Crate exposes a unified brightness slider
for all of them, with 3 brightness levels.
Therefore, to be synergistic in Linux, and support existing tooling
such as UPower, allow adding listeners to the RGB device of the WMI
interface. If WMI does not exist, lazy initialize the interface.
Since hid-asus and asus-wmi can both interact with the led objects
including from an atomic context, protect the brightness access with a
spinlock and update the values from a workqueue. Use this workqueue to
also process WMI keyboard events, so they are handled asynchronously.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/platform/x86/asus-wmi.c | 183 ++++++++++++++++++---
include/linux/platform_data/x86/asus-wmi.h | 15 ++
2 files changed, 173 insertions(+), 25 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 64cfc0bf98dd..84cde34ab6a8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -31,13 +31,13 @@
#include <linux/pci.h>
#include <linux/pci_hotplug.h>
#include <linux/platform_data/x86/asus-wmi.h>
-#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
#include <linux/platform_device.h>
#include <linux/platform_profile.h>
#include <linux/power_supply.h>
#include <linux/rfkill.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/units.h>
@@ -256,6 +256,9 @@ struct asus_wmi {
int tpd_led_wk;
struct led_classdev kbd_led;
int kbd_led_wk;
+ bool kbd_led_notify;
+ bool kbd_led_avail;
+ bool kbd_led_registered;
struct led_classdev lightbar_led;
int lightbar_led_wk;
struct led_classdev micmute_led;
@@ -264,6 +267,7 @@ struct asus_wmi {
struct work_struct tpd_led_work;
struct work_struct wlan_led_work;
struct work_struct lightbar_led_work;
+ struct work_struct kbd_led_work;
struct asus_rfkill wlan;
struct asus_rfkill bluetooth;
@@ -1615,6 +1619,106 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
/* LEDs ***********************************************************************/
+struct asus_hid_ref {
+ struct list_head listeners;
+ struct asus_wmi *asus;
+ /* Protects concurrent access from hid-asus and asus-wmi to leds */
+ spinlock_t lock;
+};
+
+static struct asus_hid_ref asus_ref = {
+ .listeners = LIST_HEAD_INIT(asus_ref.listeners),
+ .asus = NULL,
+ /*
+ * Protects .asus, .asus.kbd_led_{wk,notify}, and .listener refs. Other
+ * asus variables are read-only after .asus is set.
+ *
+ * The led cdev device is not protected because it calls backlight_get
+ * during initialization, which would result in a nested lock attempt.
+ *
+ * The led cdev is safe to access without a lock because if
+ * kbd_led_avail is true it is initialized before .asus is set and never
+ * changed until .asus is dropped. If kbd_led_avail is false, the led
+ * cdev is registered by the workqueue, which is single-threaded and
+ * cancelled before asus-wmi would access the led cdev to unregister it.
+ *
+ * A spinlock is used, because the protected variables can be accessed
+ * from an IRQ context from asus-hid.
+ */
+ .lock = __SPIN_LOCK_UNLOCKED(asus_ref.lock),
+};
+
+/*
+ * Allows registering hid-asus listeners that want to be notified of
+ * keyboard backlight changes.
+ */
+int asus_hid_register_listener(struct asus_hid_listener *bdev)
+{
+ struct asus_wmi *asus;
+
+ guard(spinlock_irqsave)(&asus_ref.lock);
+ list_add_tail(&bdev->list, &asus_ref.listeners);
+ asus = asus_ref.asus;
+ if (asus)
+ queue_work(asus->led_workqueue, &asus->kbd_led_work);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(asus_hid_register_listener);
+
+/*
+ * Allows unregistering hid-asus listeners that were added with
+ * asus_hid_register_listener().
+ */
+void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
+{
+ guard(spinlock_irqsave)(&asus_ref.lock);
+ list_del(&bdev->list);
+}
+EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
+
+static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
+
+static void kbd_led_update_all(struct work_struct *work)
+{
+ struct asus_wmi *asus;
+ bool registered, notify;
+ int ret, value;
+
+ asus = container_of(work, struct asus_wmi, kbd_led_work);
+
+ scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+ registered = asus->kbd_led_registered;
+ value = asus->kbd_led_wk;
+ notify = asus->kbd_led_notify;
+ }
+
+ if (!registered) {
+ /*
+ * This workqueue runs under asus-wmi, which means probe has
+ * completed and asus-wmi will keep running until it finishes.
+ * Therefore, we can safely register the LED without holding
+ * a spinlock.
+ */
+ ret = devm_led_classdev_register(&asus->platform_device->dev,
+ &asus->kbd_led);
+ if (!ret) {
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ asus->kbd_led_registered = true;
+ } else {
+ pr_warn("Failed to register keyboard backlight LED: %d\n", ret);
+ return;
+ }
+ }
+
+ if (value >= 0)
+ do_kbd_led_set(&asus->kbd_led, value);
+ if (notify) {
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ asus->kbd_led_notify = false;
+ led_classdev_notify_brightness_hw_changed(&asus->kbd_led, value);
+ }
+}
+
/*
* These functions actually update the LED's, and are called from a
* workqueue. By doing this as separate work rather than when the LED
@@ -1661,7 +1765,8 @@ static void kbd_led_update(struct asus_wmi *asus)
{
int ctrl_param = 0;
- ctrl_param = 0x80 | (asus->kbd_led_wk & 0x7F);
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ ctrl_param = 0x80 | (asus->kbd_led_wk & 0x7F);
asus_wmi_set_devstate(ASUS_WMI_DEVID_KBD_BACKLIGHT, ctrl_param, NULL);
}
@@ -1694,14 +1799,23 @@ static int kbd_led_read(struct asus_wmi *asus, int *level, int *env)
static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
{
+ struct asus_hid_listener *listener;
struct asus_wmi *asus;
int max_level;
asus = container_of(led_cdev, struct asus_wmi, kbd_led);
max_level = asus->kbd_led.max_brightness;
- asus->kbd_led_wk = clamp_val(value, 0, max_level);
- kbd_led_update(asus);
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ asus->kbd_led_wk = clamp_val(value, 0, max_level);
+
+ if (asus->kbd_led_avail)
+ kbd_led_update(asus);
+
+ scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+ list_for_each_entry(listener, &asus_ref.listeners, list)
+ listener->brightness_set(listener, asus->kbd_led_wk);
+ }
}
static void kbd_led_set(struct led_classdev *led_cdev,
@@ -1716,10 +1830,11 @@ static void kbd_led_set(struct led_classdev *led_cdev,
static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
{
- struct led_classdev *led_cdev = &asus->kbd_led;
-
- do_kbd_led_set(led_cdev, value);
- led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
+ scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+ asus->kbd_led_wk = value;
+ asus->kbd_led_notify = true;
+ }
+ queue_work(asus->led_workqueue, &asus->kbd_led_work);
}
static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
@@ -1729,10 +1844,18 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
asus = container_of(led_cdev, struct asus_wmi, kbd_led);
+ scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+ if (!asus->kbd_led_avail)
+ return asus->kbd_led_wk;
+ }
+
retval = kbd_led_read(asus, &value, NULL);
if (retval < 0)
return retval;
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ asus->kbd_led_wk = value;
+
return value;
}
@@ -1844,7 +1967,9 @@ static int camera_led_set(struct led_classdev *led_cdev,
static void asus_wmi_led_exit(struct asus_wmi *asus)
{
- led_classdev_unregister(&asus->kbd_led);
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ asus_ref.asus = NULL;
+
led_classdev_unregister(&asus->tpd_led);
led_classdev_unregister(&asus->wlan_led);
led_classdev_unregister(&asus->lightbar_led);
@@ -1882,22 +2007,26 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
goto error;
}
- if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
- pr_info("using asus-wmi for asus::kbd_backlight\n");
- asus->kbd_led_wk = led_val;
- asus->kbd_led.name = "asus::kbd_backlight";
- asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
- asus->kbd_led.brightness_set = kbd_led_set;
- asus->kbd_led.brightness_get = kbd_led_get;
- asus->kbd_led.max_brightness = 3;
+ asus->kbd_led.name = "asus::kbd_backlight";
+ asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
+ asus->kbd_led.brightness_set = kbd_led_set;
+ asus->kbd_led.brightness_get = kbd_led_get;
+ asus->kbd_led.max_brightness = 3;
+ asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
+ INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
+ if (asus->kbd_led_avail) {
+ asus->kbd_led_wk = led_val;
if (num_rgb_groups != 0)
asus->kbd_led.groups = kbd_rgb_mode_groups;
+ } else {
+ asus->kbd_led_wk = -1;
+ }
- rv = led_classdev_register(&asus->platform_device->dev,
- &asus->kbd_led);
- if (rv)
- goto error;
+ scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+ asus_ref.asus = asus;
+ if (asus->kbd_led_avail || !list_empty(&asus_ref.listeners))
+ queue_work(asus->led_workqueue, &asus->kbd_led_work);
}
if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
@@ -4372,6 +4501,7 @@ static int asus_wmi_get_event_code(union acpi_object *obj)
static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
{
+ enum led_brightness led_value;
unsigned int key_value = 1;
bool autorelease = 1;
@@ -4388,19 +4518,22 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
return;
}
+ scoped_guard(spinlock_irqsave, &asus_ref.lock)
+ led_value = asus->kbd_led_wk;
+
if (code == NOTIFY_KBD_BRTUP) {
- kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
+ kbd_led_set_by_kbd(asus, led_value + 1);
return;
}
if (code == NOTIFY_KBD_BRTDWN) {
- kbd_led_set_by_kbd(asus, asus->kbd_led_wk - 1);
+ kbd_led_set_by_kbd(asus, led_value - 1);
return;
}
if (code == NOTIFY_KBD_BRTTOGGLE) {
- if (asus->kbd_led_wk == asus->kbd_led.max_brightness)
+ if (led_value == asus->kbd_led.max_brightness)
kbd_led_set_by_kbd(asus, 0);
else
- kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
+ kbd_led_set_by_kbd(asus, led_value + 1);
return;
}
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 419491d4abca..d347cffd05d5 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -172,12 +172,20 @@ enum asus_ally_mcu_hack {
ASUS_WMI_ALLY_MCU_HACK_DISABLED,
};
+/* Used to notify hid-asus when asus-wmi changes keyboard backlight */
+struct asus_hid_listener {
+ struct list_head list;
+ void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
+};
+
#if IS_REACHABLE(CONFIG_ASUS_WMI)
void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
void set_ally_mcu_powersave(bool enabled);
int asus_wmi_get_devstate_dsts(u32 dev_id, u32 *retval);
int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
+int asus_hid_register_listener(struct asus_hid_listener *cdev);
+void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
#else
static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
{
@@ -198,6 +206,13 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
{
return -ENODEV;
}
+static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
+{
+ return -ENODEV;
+}
+static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
+{
+}
#endif
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.52.0
^ permalink raw reply related
* [PATCH v10 02/11] HID: asus: initialize additional endpoints only for legacy devices
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Currently, ID1/ID2 initializations are performed for all NKEY devices.
However, ID1 initializations are only required for RGB control and are
only supported for RGB capable devices. ID2 initializations are only
required for initializing the Anime display endpoint which is only
supported on devices with an Anime display. Both of these
initializations are out of scope for this driver (this is a brightness
control and keyboard shortcut driver) and they should not be performed
for devices that do not support them in any case.
At the same time, there are older NKEY devices that have only been
tested with these initializations in the kernel and it is not possible
to recheck them. There is a possibility that especially with the ID1
initialization, certain laptop models might have their shortcuts stop
working (currently unproven).
For an abundance of caution, only initialize ID1/ID2 for those older
NKEY devices by introducing a quirk for them and replacing the NKEY
quirk in the block that performs the inits with that.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a82286a427b8..9004814fb0d3 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define QUIRK_ROG_NKEY_KEYBOARD BIT(11)
#define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
#define QUIRK_ROG_ALLY_XPAD BIT(13)
+#define QUIRK_ROG_NKEY_LEGACY BIT(14)
#define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
QUIRK_NO_INIT_REPORTS | \
@@ -652,7 +653,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
return -ENODEV;
- if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
+ if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
if (ret < 0)
return ret;
@@ -1376,10 +1377,10 @@ static const struct hid_device_id asus_devices[] = {
QUIRK_USE_KBD_BACKLIGHT },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
--
2.52.0
^ permalink raw reply related
* [PATCH v10 03/11] HID: asus: use same report_id in response
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Currently, asus_kbd_get_functions prods the device using feature
report report_id, but then is hardcoded to check the response through
FEATURE_KBD_REPORT_ID. This only works if report_id is that value
(currently true). So, use report_id in the response as well to
maintain functionality if that value changes in the future.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
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 9004814fb0d3..c1a36a8beb35 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -424,7 +424,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
if (!readbuf)
return -ENOMEM;
- ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
+ ret = hid_hw_raw_request(hdev, report_id, readbuf,
FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
HID_REQ_GET_REPORT);
if (ret < 0) {
--
2.52.0
^ permalink raw reply related
* [PATCH v10 06/11] HID: asus: early return for ROG devices
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Some ROG devices have a new dynamic backlight interface for control by
Windows. This interface does not create an ->input device, causing the
kernel to print an error message and to eject it. In addition, ROG
devices have proper HID names in their descriptors so renaming them is
not necessary.
Therefore, if a device is identified as ROG, early return from probe to
skip renaming and ->input checks.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index faac971794c0..461b0f8efdfb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1235,6 +1235,13 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
asus_kbd_register_leds(hdev))
hid_warn(hdev, "Failed to initialize backlight.\n");
+ /*
+ * For ROG keyboards, skip rename for consistency and ->input check as
+ * some devices do not have inputs.
+ */
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD)
+ return 0;
+
/*
* Check that input registration succeeded. Checking that
* HID_CLAIMED_INPUT is set prevents a UAF when all input devices
--
2.52.0
^ permalink raw reply related
* [PATCH v10 01/11] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
In-Reply-To: <20251122110032.4274-1-lkml@antheas.dev>
Currently, RGB initialization forks depending on whether a device is
NKEY. However, in reality both initialization forks are the same, other
than the NKEY initialization initializing the LED_REPORT_ID1,
LED_REPORT_ID2 endpoints, and the non-NKEY initialization having a
functionality check which is skipped for the NKEY path.
Therefore, merge the if blocks, gate the ID1/ID2 initializations
behind the NKEY quirk instead, and introduce the functionality check
for NKEY devices (it is supported by them).
There should be no functional change with this patch.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 52 ++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 472bca54642b..a82286a427b8 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -639,13 +639,20 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
unsigned char kbd_func;
int ret;
- if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
- /* Initialize keyboard */
- ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
- if (ret < 0)
- return ret;
+ ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
+ if (ret < 0)
+ return ret;
- /* The LED endpoint is initialised in two HID */
+ /* Get keyboard functions */
+ ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+ if (ret < 0)
+ return ret;
+
+ /* Check for backlight support */
+ if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
+ return -ENODEV;
+
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
if (ret < 0)
return ret;
@@ -653,34 +660,19 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
if (ret < 0)
return ret;
+ }
- if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
- ret = asus_kbd_disable_oobe(hdev);
- if (ret < 0)
- return ret;
- }
-
- if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
- intf = to_usb_interface(hdev->dev.parent);
- udev = interface_to_usbdev(intf);
- validate_mcu_fw_version(hdev,
- le16_to_cpu(udev->descriptor.idProduct));
- }
-
- } else {
- /* Initialize keyboard */
- ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
- if (ret < 0)
- return ret;
-
- /* Get keyboard functions */
- ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+ if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
+ ret = asus_kbd_disable_oobe(hdev);
if (ret < 0)
return ret;
+ }
- /* Check for backlight support */
- if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
- return -ENODEV;
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ intf = to_usb_interface(hdev->dev.parent);
+ udev = interface_to_usbdev(intf);
+ validate_mcu_fw_version(
+ hdev, le16_to_cpu(udev->descriptor.idProduct));
}
drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
--
2.52.0
^ permalink raw reply related
* [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
This is a two part series which does the following:
- Clean-up init sequence
- Unify backlight handling to happen under asus-wmi so that all Aura
devices have synced brightness controls and the backlight button works
properly when it is on a USB laptop keyboard instead of one w/ WMI.
For more context, see cover letter of V1. Since V5, I removed some patches
to make this easier to merge.
---
V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
Changes since V9:
- No functional changes
- Rebase to review-ilpo-next
- Fix armoury series conflict by removing the file asus-wmi-leds-ids on
"remove unused keyboard backlight quirk" + imports
Dismiss Luke's review as this patch diverged
- Reword paragraph in "Add support for multiple kbd led handlers" to be
more verbose
- Use kfree in fortify patch
- Fix minor style quirks from --nonstict checkpatch run
Changes since V8:
- No functional changes
- Move legacy init patch to second, modify first patch so that their
diff is minimized
- Split "prevent binding to all HID devices on ROG" into two patches:
- moving backlight initialization into probe
- early exit to skip ->init check and rename
- Remove skipping vendor fixups for non-vendor devices. It is not possible
to read usages before the report fixups are applied, so it did not work
- In that patch, reword a comment to be single line and make is_vendor a bool
- Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
has drifted too far since he reviewed/tested it.
Changes since V7:
- Readd legacy init quirk for Dennis
- Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
- Fix warning due to enum_backlight receiving negative values
Changes since V6:
- Split initialization refactor into three patches, update commit text
to be clearer in what it does
- Replace spinlock accesses with guard and scoped guard in all patches
- Add missing includes mentioned by Ilpo
- Reflow, tweak comment in prevent binding to all HID devices on ROG
- Replace asus_ref.asus with local reference in all patches
- Add missing kernel doc comments
- Other minor nits from Ilpo
- User reported warning due to scheduling work while holding a spinlock.
Restructure patch for multiple handlers to limit when spinlock is held to
variable access only. In parallel, setup a workqueue to handle registration
of led device and setting brightness. This is required as registering the
led device triggers kbd_led_get which needs to hold the spinlock to
protect the led_wk value. The workqueue is also required for the hid
event passthrough to avoid scheduling work while holding the spinlock.
Apply the workqueue to wmi brightness buttons as well, as that was
omitted before this series and WMI access was performed.
- On "HID: asus: prevent binding to all HID devices on ROG", rename
quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
This allows other quirks to apply (applies quirk that fixes keyboard
being named as a pointer device).
Changes since V5:
- It's been a long time
- Remove addition of RGB as that had some comments I need to work on
- Remove folio patch (already merged)
- Remove legacy fix patch 11 from V4. There is a small chance that
without this patch, some old NKEY keyboards might not respond to
RGB commands according to Luke, but the kernel driver does not do
RGB currently. The 0x5d init is done by Armoury crate software in
Windows. If an issue is found, we can re-add it or just remove patches
1/2 before merging. However, init could use the cleanup.
Changes since V4:
- Fix KConfig (reported by kernel robot)
- Fix Ilpo's nits, if I missed anything lmk
Changes since V3:
- Add initializer for 0x5d for old NKEY keyboards until it is verified
that it is not needed for their media keys to function.
- Cover init in asus-wmi with spinlock as per Hans
- If asus-wmi registers WMI handler with brightness, init the brightness
in USB Asus keyboards, per Hans.
- Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
- Fix oops when unregistering asus-wmi by moving unregister outside of
the spin lock (but after the asus reference is set to null)
Changes since V2:
- Check lazy init succeds in asus-wmi before setting register variable
- make explicit check in asus_hid_register_listener for listener existing
to avoid re-init
- rename asus_brt to asus_hid in most places and harmonize everything
- switch to a spinlock instead of a mutex to avoid kernel ooops
- fixup hid device quirks to avoid multiple RGB devices while still exposing
all input vendor devices. This includes moving rgb init to probe
instead of the input_configured callbacks.
- Remove fan key (during retest it appears to be 0xae that is already
supported by hid-asus)
- Never unregister asus::kbd_backlight while asus-wmi is active, as that
- removes fds from userspace and breaks backlight functionality. All
- current mainline drivers do not support backlight hotplugging, so most
userspace software (e.g., KDE, UPower) is built with that assumption.
For the Ally, since it disconnects its controller during sleep, this
caused the backlight slider to not work in KDE.
Changes since V1:
- Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
- Fix ifdef else having an invalid signature (reported by kernel robot)
- Restore input arguments to init and keyboard function so they can
be re-used for RGB controls.
- Remove Z13 delay (it did not work to fix the touchpad) and replace it
with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
keyboard rename into it.
- Unregister brightness listener before removing work queue to avoid
a race condition causing corruption
- Remove spurious mutex unlock in asus_brt_event
- Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
relocking the mutex and causing a deadlock when unregistering leds
- Add extra check during unregistering to avoid calling unregister when
no led device is registered.
- Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
the driver to create 4 RGB handlers per device. I also suspect some
extra events sneak through (KDE had the @@@@@@).
Antheas Kapenekakis (11):
HID: asus: simplify RGB init sequence
HID: asus: initialize additional endpoints only for legacy devices
HID: asus: use same report_id in response
HID: asus: fortify keyboard handshake
HID: asus: move vendor initialization to probe
HID: asus: early return for ROG devices
platform/x86: asus-wmi: Add support for multiple kbd led handlers
HID: asus: listen to the asus-wmi brightness device instead of
creating one
platform/x86: asus-wmi: remove unused keyboard backlight quirk
platform/x86: asus-wmi: add keyboard brightness event handler
HID: asus: add support for the asus-wmi brightness handler
drivers/hid/hid-asus.c | 205 ++++++++--------
drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
.../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
include/linux/platform_data/x86/asus-wmi.h | 28 +++
4 files changed, 322 insertions(+), 184 deletions(-)
delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
--
2.52.0
^ permalink raw reply
* [git pull] Input updates for v6.18-rc6
From: Dmitry Torokhov @ 2025-11-22 6:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.18-rc6
to receive updates for the input subsystem. You will get:
- INPUT_PROP_HAPTIC_TOUCHPAD definition added early in 6.18 cycle has
been renamed to INPUT_PROP_PRESSUREPAD to better reflect the kind of
devices it is supposed to be set for
- a new ID for a touchscreen found in Ayaneo Flip DS in Goodix driver
- Goodix driver no longer tries to set reset pin as "input" as it causes
issues when there is no pull up resistor installed on the board
- fixes for cros_ec_keyb, imx_sc_key, and pegasus-notetaker drivers to
deal with potential out-of-bounds access and memory corruption issues
Changelog:
---------
Dan Carpenter (1):
Input: imx_sc_key - fix memory corruption on unload
Hans de Goede (1):
Input: goodix - add support for ACPI ID GDIX1003
Martyn Welch (1):
Input: goodix - remove setting of RST pin to input
Peter Hutterer (1):
Input: rename INPUT_PROP_HAPTIC_TOUCHPAD to INPUT_PROP_PRESSUREPAD
Seungjin Bae (1):
Input: pegasus-notetaker - fix potential out-of-bounds access
Tzung-Bi Shih (1):
Input: cros_ec_keyb - fix an invalid memory access
Diffstat:
--------
Documentation/input/event-codes.rst | 25 ++++++++++++++++++-------
driveruntime/detectrs/hid/hid-haptic.c | 2 +-
drivers/input/keyboard/cros_ec_keyb.c | 6 ++++++
drivers/input/keyboard/imx_sc_key.c | 2 +-
drivers/input/tablet/pegasus_notetaker.c | 9 +++++++++
drivers/input/touchscreen/goodix.c | 28 ++--------------------------
drivers/input/touchscreen/goodix.h | 1 -
include/uapi/linux/input-event-codes.h | 2 +-
8 files changed, 38 insertions(+), 37 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xpad - Change buttons the D-Pad gets mapped as to BTN_DPAD_*
From: Vicki Pfau @ 2025-11-22 2:23 UTC (permalink / raw)
To: Antheas Kapenekakis, Dmitry Torokhov; +Cc: Cameron Gutman, linux-input
In-Reply-To: <CAGwozwHfnuDrYHK557oDcRNxQvKx4BRSSgkL+ev1xuKNY39JVw@mail.gmail.com>
I might as well throw in my hat while I'm at it. I have several issues with how evdev handles gamepads that I've been meaning to bring up, so now's as good a time as any.
On 11/12/25 6:16 AM, Antheas Kapenekakis wrote:
> On Wed, 5 Nov 2025 at 06:04, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>>
>> On Sat, Nov 01, 2025 at 11:23:51AM -0500, Cameron Gutman wrote:
>>> On Sat, Nov 1, 2025 at 12:52 AM Vicki Pfau <vi@endrift.com> wrote:
>>>>
>>>>
>>>>
>>>> On 10/31/25 8:18 PM, Cameron Gutman wrote:
>>>>> On Sun, Jul 27, 2025 at 3:23 AM Dmitry Torokhov
>>>>> <dmitry.torokhov@gmail.com> wrote:
>>>>>>
>>>>>> On Tue, Jul 01, 2025 at 08:47:40PM -0700, Vicki Pfau wrote:
>>>>>>> Since dance pads can have both up/down or left/right pressed at the same time,
>>>>>>> by design, they are not suitable for mapping the buttons to axes. Historically,
>>>>>>> this driver mapped the D-pad to BTN_TRIGGER_HAPPY1-4 in these cases, and before
>>>>>>> that as mouse buttons. However, BTN_DPAD_* exists for this and makes far more
>>>>>>> sense than the arbitrary mapping it was before.
>>>>>>>
>>>>>>> Signed-off-by: Vicki Pfau <vi@endrift.com>
>>>>>>
>>>>>> This unfortunately changes existing mappings, but I guess new events are
>>>>>> better than old ones...
>>>>>>
>>>>>> Applied, thank you.
>>>>>
>>>>> Unfortunately this mapping change caused major userspace breakages.
>>>>
>>>> This is unfortunate, however,>
>>>>> I think it needs to be reverted.
>>>>>
>>>>> [0]: https://blog.dwickham.me.uk/why-your-xbox-360-wireless-controllers-d-pad-is-now-wrong-and-how-to-fix-it
>>>>> [1]: https://github.com/libsdl-org/SDL/issues/14324
>>>>> [2]: https://github.com/libsdl-org/SDL/pull/14339
>>>>
>>>> None of these pages suggest reverting the changes. The blog post suggests that these mappings are correct, and that the libraries need to be fixed. Reverting it is mentioned as an absolute last case scenario. The SDL issues say they need more testing, not an outright reversion.
>>>
>>> Breaking userspace is the #1 thing we cannot do as Linux kernel developers.
>>>
>>> The fact that it broke enough people to get a blog post is reason
>>> enough to revert.
>>>
>>>>
>>>> Reverting this is solely reverting to an old, uncomfortable status quo instead of actually fixing the things that broke. If this gets reverted, presumably more stuff will ship this broken code that will break if we ever fix this issue again, instead of things getting progressively fixed.
>>>>
>>>
>>> The "broken" 360 wireless mapping has been used since wireless adapter
>>> support was first implemented in 2008 [0].
>>>
>>> There's decades of software out there that already has workarounds for
>>> these devices that we must not break.
>>>
>>> We can send the proper button codes for new controllers, but I think
>>> we have to leave existing devices alone.
>>>
>>> [0]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99de0912be6f384fc31c8e8e7ba0850d0d670385
>>>
>>>> The abandonware being broken is unfortunate, but things like Steam Input exist specifically to work around issues like that.
>>>>
>>>
>>> This isn't just breaking abandonware. It's breaking the *latest*
>>> releases of SDL2 and SDL3.
>>
>> I agree that keeping the old mapping is not great idea, especially if we
>> start having newer devices using new mapping while other devices use the
>> old one. However we can't really be breaking existing user setup and now
>> that we are aware of such breakages we have to react. I am open to
>> suggestions.
>>
>> From my side I see a few options:
>>
>> 1. Introduce a module option, and allow controlling the behavior. Hope
>> distributions will flip it when updating to new version of SDL that can
>> hopefully handle this. Eventually flip the default to be the new
>> mapping.
>>
>> 2. Allow overriding mapping from userspace. Allow [main] user (SDL or
>> not) load an updated keymap if they desire to do so.
>>
>> 3. Maybe we can send both old and new events at the same time and let
>> consumers decide which ones to act upon? Eventually we can drop the old
>> mapping (maybe again having an option to use old one or implement option
>> 2 and allow loading old mapping via udev or a utility).
>>
>> 4. ???
>
> Hi,
> jumping on this to give some background.
>
> Currently, the way the kernel handles evdev gamepads is a bit of a
> mess, specifically when it comes to DInput.
>
> Essentially, gamepads are consumed using the legacy js API which does
> not have event names but only button indexes. Or an evdev wrapper if
> using an SDL-like backend that does something similar.
>
> There are no specific mappings for each index, this is different for
> every gamepad. Userspace software is supposed to know the mapping for
> each gamepad based on a heuristic. The most widespread one is the SDL
> mappings, which are also available in the form of a community database
> [1].
>
> You can see in the button index code in the kernel that it starts with
> buttons from index BTN_JOYSTICK, goes up to KEY_MAX, then loops around
> and registers the buttons until BTN_MISC (which skips most keyboard
> buttons). Those are the buttons available in the legacy API. SDL is
> different, in that it keeps registering the buttons after BTN_MISC
> which is important for some gamepads that use KEY_MENU for e.g., the
> Xbox button. This means that it is possible to use SDL mappings with
> the joydev API while missing some buttons, but since those buttons are
> at the end and usually special functions it is a NOOP.
>
> Dinput is where it gets interesting and why it is kind of a fruitless
> endeavor to try to rename buttons. In [3], the kernel starts
> registering buttons with HID_GD_JOYSTICK from BTN_JOYSTICK onwards, so
> all HID gamepads that do not bind to a driver have completely invalid
> evdev event names. And this is most non-XInput gamepads (around 400
> models) so no one in their right mind would rely on evdev names from
> userspace and they would instead use that mapping.
>
> WIth that in mind, it is very important to not shift the gamepad
> button order when adding new mappings. By default, the Xbox 360 order
> is used. E.g., dont use BTN_BASE for an extra utility button like the
> hid-steam driver (which otherwise has an xbox 360 order), as because
> it is before BTN_GAMEPAD this shifts the index of all buttons by 1 and
> breaks all userspace software without a mapping (this is fixed in
> userspace now).
This itself is a huge problem. It shows a mismatch between the expectations of how evdev works and how game controllers work. evdev has named inputs and dinput/joydev has numbered. But since the ordering of the names is arbitrary, the numbering becomes a useless mess too. If adding a name in the middle breaks numbering, that highlights issues with extensibility, especially considering TRIGGER_HAPPY is at the very end of the space.
As an aside, TRIGGER_HAPPY itself is confusingly named and the name appears to be some sort of joke. I would be happy if we renamed them to BTN_EXTRA* and had the old names be aliases for the new names.
This impedance mismatch means we cannot rename things or add buttons in the middle. That is a huge issue, e.g. when new models of the same controller come out with new buttons, like when the Xbox Series controllers added the share button. This button is somewhat unceremoniously mapped to KEY_RECORD (though the HID subsystem tends to map it from arbitrary controllers as KEY_SYSRQ since it's the same as print screen). There are also guidelines saying new types of button should get new names, which seems to have been not done for quite a while, e.g. BTN_MODE being used for the guide buttons in various controllers when that has nothing to do with the mode button on older controllers.
I've been working on bringing up a new Xbox One controller driver (which has been submitted but unreviewed twice...I'm still working on improvements so it's not a huge issue, at least), but one of the biggest issues with some of the controller types is that they can support up to 64 extra buttons and 12 extra axes. There's no sensible place to map these within the evdev KEY/BTN range or the ABS range. Adding more TRIGGER_HAPPY buttons is suboptimal, and adding enough axes to the ABS range is untenable without growing the range. There just isn't a place for arbitrary extra event types whose semantic meaning is unknown or arbitrary.
The input subsystem seems to need a major rethinking of how controller mapping works. It's historically been not very important due to there not being much gaming on Linux, but with the new products with Valve over the last few years, this is a thing that needs to be done sooner rather than later.
>
> With that in mind, commit [4] that adds button grips is also
> problematic, as those buttons are BELOW trigger-happy which used to do
> dpad buttons. If you revert just this commit, the grips will become
> dpad buttons for devices that have both.
>
> So both commits would have to be reverted. With them together or
> without either, the button order is maintained. However, they have a
> different order within them so dpad is broken. I suspect the grip
> buttons for Xbox Elite might have a similar issue... Although I have
> not had a bug report about it.
>
> I already received and merged a PR that fixes this from user space[5],
> and I saw that the SDL3 fix is not coming to SDL2, but i think that
> fix might leave that config stuck in the user's Steam config dir, so
> perhaps it would be prudent for me to undo it and instead just revert
> both commits.
>
> Antheas
>
> [1] https://github.com/mdqinc/SDL_GameControllerDB
> [2] drivers/input/joydev.c
> [3] drivers/hid/hid-input.c
> [4] Input: xpad - use new BTN_GRIP* buttons
> [5] https://github.com/ublue-os/bazzite/issues/3399
>
>> Thanks.
>>
>> --
>> Dmitry
>
Vicki
^ permalink raw reply
* Re: [PATCH v1] HID: Fix Report Descriptor for Evision Wireless Receiver 320f:226f
From: Artem @ 2025-11-21 22:58 UTC (permalink / raw)
To: Terry Junge; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20251120014931.580340-1-linuxhid@cosmicgizmosystems.com>
Hello Terry,
I have successfully compiled and tested the patch.
It works perfectly! The side buttons (Forward/Back) are now correctly
detected and functioning immediately after loading the patched module.
I haven't encountered any issues afterwards, other functionality is
unchanged.
dmesg confirms the fix is applied:
[10307.732966] evision 0003:320F:226F.0009: fixing EVision:TeLink
Receiver report descriptor
Thank you very much for your quick help and the fix.
Tested-by: Artem <temabiill@gmail.com>
Best regards,
Btema2
^ 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