* [PATCH v8 01/10] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-1-lkml@antheas.dev>
Currently, RGB initialization forks depending on whether a device is
NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
endpoints, and non-NKEY devices with 0x5a and then a
backlight check, which is omitted for NKEY devices.
Remove the fork, using a common initialization sequence for both,
where they are both only initialized with 0x5a, then checked for
backlight support. This patch should not affect existing functionality.
0x5d and 0x5e endpoint initializations are performed by Windows
userspace programs associated with different usages that reside under
the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
controls RGB and 0x5e by an animation program for certain Asus laptops.
Neither is used currently in the driver.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
1 file changed, 19 insertions(+), 37 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a444d41e53b6..7ea1037c3979 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -638,50 +638,32 @@ 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;
-
- /* The LED endpoint is initialised in two HID */
- ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
- if (ret < 0)
- return ret;
-
- 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));
- }
+ ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
+ if (ret < 0)
+ return ret;
- } 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 (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));
}
+ /* Check for backlight support */
+ if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
+ return -ENODEV;
+
drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
sizeof(struct asus_kbd_leds),
GFP_KERNEL);
--
2.51.2
^ permalink raw reply related
* [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-11-01 10:47 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.
---
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 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 (10):
HID: asus: simplify RGB init sequence
HID: asus: use same report_id in response
HID: asus: fortify keyboard handshake
HID: asus: prevent binding to all HID devices on ROG
HID: asus: initialize LED endpoint early for old NKEY keyboards
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 | 222 +++++++++++----------
drivers/platform/x86/asus-wmi.c | 214 +++++++++++++++++---
include/linux/platform_data/x86/asus-wmi.h | 70 +++----
3 files changed, 331 insertions(+), 175 deletions(-)
base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
--
2.51.2
^ permalink raw reply
* [PATCH v8 02/10] HID: asus: use same report_id in response
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-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.
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 7ea1037c3979..4676b7f20caf 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -422,7 +422,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.51.2
^ permalink raw reply related
* [PATCH v8 03/10] HID: asus: fortify keyboard handshake
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-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 | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 4676b7f20caf..03f0d86936fc 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -48,7 +48,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
@@ -393,14 +393,40 @@ 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 };
+ u8 *readbuf;
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;
+ }
+
+ readbuf = 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.
+ */
+ }
+ kfree(readbuf);
return ret;
}
--
2.51.2
^ permalink raw reply related
* [PATCH v8 04/10] HID: asus: prevent binding to all HID devices on ROG
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-1-lkml@antheas.dev>
Currently, when hid-asus is not loaded, NKEY keyboards load as ~6
event devices with a pretty ASUSTEK name. When it loads, it concatenates
all applications per HID endpoint, renames them, and prints errors
when some of them do not have an input device.
Therefore, change probe so that this is no longer the case. Stop
renaming the devices, omit the check for .input which causes errors
on e.g., the Z13 for some hiddev only devices, and move RGB checks
into probe.
Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 52 ++++++++++++++++++++++++++++--------------
1 file changed, 35 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 03f0d86936fc..726f5d8e22d1 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -47,6 +47,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
@@ -89,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_SKIP_REPORT_FIXUP BIT(14)
#define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
QUIRK_NO_INIT_REPORTS | \
@@ -125,7 +127,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;
@@ -316,7 +317,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",
@@ -931,11 +932,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;
}
@@ -1008,15 +1004,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;
}
@@ -1133,8 +1120,10 @@ 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;
+ int ret, is_vendor = 0;
drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
if (drvdata == NULL) {
@@ -1218,12 +1207,37 @@ 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;
+ }
+
+ /*
+ * For ROG keyboards, disable fixups except vendor devices.
+ */
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && !is_vendor)
+ drvdata->quirks |= QUIRK_SKIP_REPORT_FIXUP;
+
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");
+
+ /*
+ * 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
@@ -1352,6 +1366,10 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
rdesc = new_rdesc;
}
+ /* Vendor fixups should only apply to NKEY vendor devices. */
+ if (drvdata->quirks & QUIRK_SKIP_REPORT_FIXUP)
+ return rdesc;
+
if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
*rsize == 331 && rdesc[190] == 0x85 && rdesc[191] == 0x5a &&
rdesc[204] == 0x95 && rdesc[205] == 0x05) {
--
2.51.2
^ permalink raw reply related
* [PATCH v8 05/10] HID: asus: initialize LED endpoint early for old NKEY keyboards
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-1-lkml@antheas.dev>
These keyboards have always had initialization in the kernel for 0x5d.
At this point, it is hard to verify again and we risk regressions by
removing this. Therefore, initialize with 0x5d as well.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 726f5d8e22d1..221c7195e885 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -91,6 +91,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
#define QUIRK_ROG_ALLY_XPAD BIT(13)
#define QUIRK_SKIP_REPORT_FIXUP BIT(14)
+#define QUIRK_ROG_NKEY_LEGACY BIT(15)
#define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
QUIRK_NO_INIT_REPORTS | \
@@ -669,6 +670,16 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (ret < 0)
return ret;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
+ /*
+ * These keyboards might need 0x5d for shortcuts to work.
+ * As it has been more than 5 years, it is hard to verify.
+ */
+ ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
+ if (ret < 0)
+ return ret;
+ }
+
/* Get keyboard functions */
ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
if (ret < 0)
@@ -1409,10 +1420,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.51.2
^ permalink raw reply related
* [PATCH v8 06/10] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-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 both 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 process WMI keyboard events as well, so they
are processed asynchronously.
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 | 174 ++++++++++++++++++---
include/linux/platform_data/x86/asus-wmi.h | 17 ++
2 files changed, 167 insertions(+), 24 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index e72a2b5d158e..5f23aedbf34f 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -36,6 +36,7 @@
#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>
@@ -258,6 +259,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;
@@ -266,6 +270,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;
@@ -1530,6 +1535,99 @@ 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. Except the led cdev
+ * device if not kbd_led_avail. That becomes read-only after the
+ * first hid-asus listener registers and triggers the work queue. It is
+ * then not referenced again until unregistering, which happens after
+ * .asus ref is dropped. Since .asus needs to be accessed by hid-asus
+ * IRQs to check if forwarding events is possible, a spinlock is used.
+ */
+ .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
@@ -1576,7 +1674,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);
}
@@ -1609,14 +1708,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,
@@ -1631,10 +1739,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)
@@ -1644,10 +1753,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;
}
@@ -1759,7 +1876,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);
@@ -1797,22 +1916,25 @@ 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)
@@ -4272,6 +4394,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;
@@ -4288,19 +4411,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 8a515179113d..1165039013b1 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -163,11 +163,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_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)
{
@@ -184,6 +193,14 @@ 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
/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
--
2.51.2
^ permalink raw reply related
* [PATCH v8 07/10] HID: asus: listen to the asus-wmi brightness device instead of creating one
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-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 | 64 +++++++-----------------------------------
1 file changed, 10 insertions(+), 54 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 221c7195e885..e5d3f28c1fad 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -103,7 +103,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;
@@ -495,11 +495,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);
@@ -509,20 +509,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);
@@ -539,34 +525,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
@@ -711,14 +669,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);
@@ -1107,7 +1062,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);
@@ -1238,7 +1193,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");
@@ -1285,6 +1239,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.51.2
^ permalink raw reply related
* [PATCH v8 08/10] platform/x86: asus-wmi: remove unused keyboard backlight quirk
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-1-lkml@antheas.dev>
The quirk for selecting whether keyboard backlight should be controlled
by HID or WMI is not needed anymore, so remove it.
Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
include/linux/platform_data/x86/asus-wmi.h | 40 ----------------------
1 file changed, 40 deletions(-)
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 1165039013b1..d8c5269854b0 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -203,44 +203,4 @@ static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
}
#endif
-/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-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 /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.51.2
^ permalink raw reply related
* [PATCH v8 09/10] platform/x86: asus-wmi: add keyboard brightness event handler
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-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 5f23aedbf34f..a7482c97cc5b 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1628,6 +1628,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
@@ -1710,13 +1748,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);
@@ -1920,7 +1956,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);
@@ -4423,7 +4459,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 d8c5269854b0..3f679598b629 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -169,6 +169,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);
@@ -177,6 +185,7 @@ 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)
{
@@ -201,6 +210,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.51.2
^ permalink raw reply related
* [PATCH v8 10/10] HID: asus: add support for the asus-wmi brightness handler
From: Antheas Kapenekakis @ 2025-11-01 10:47 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: <20251101104712.8011-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>
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 e5d3f28c1fad..de64451e315d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -325,6 +325,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.51.2
^ permalink raw reply related
* [PATCH] Input: imx_sc_key - Fix memory corruption on unload
From: Dan Carpenter @ 2025-11-01 13:25 UTC (permalink / raw)
To: Anson Huang
Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-input,
linux-kernel, imx, linux-arm-kernel, kernel-janitors
This is supposed to be "priv" but we accidentally pass "&priv" which is
an address in the stack and so it will lead to memory corruption when
the imx_sc_key_action() function is called. Remove the &.
Fixes: 768062fd1284 ("Input: imx_sc_key - use devm_add_action_or_reset() to handle all cleanups")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/input/keyboard/imx_sc_key.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/input/keyboard/imx_sc_key.c b/drivers/input/keyboard/imx_sc_key.c
index d18839f1f4f6..b620cd310cdb 100644
--- a/drivers/input/keyboard/imx_sc_key.c
+++ b/drivers/input/keyboard/imx_sc_key.c
@@ -158,7 +158,7 @@ static int imx_sc_key_probe(struct platform_device *pdev)
return error;
}
- error = devm_add_action_or_reset(&pdev->dev, imx_sc_key_action, &priv);
+ error = devm_add_action_or_reset(&pdev->dev, imx_sc_key_action, priv);
if (error)
return error;
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] Input: xpad - Change buttons the D-Pad gets mapped as to BTN_DPAD_*
From: Cameron Gutman @ 2025-11-01 16:23 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Dmitry Torokhov, linux-input
In-Reply-To: <b8933438-fd43-4e58-a26d-0febb27911c4@endrift.com>
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.
We cannot depend on third-party software like Steam Input to avoid
breaking userspace,
nor can we depend on distros to update their SDL versions in lockstep
with the kernel.
> >
> >>
> >> --
> >> Dmitry
> >>
>
> Vicki
>
^ permalink raw reply
* Re: [PATCH v2 0/5] Input: xbox_gip - Add new driver for Xbox GIP
From: Shengyu Qu @ 2025-11-01 17:45 UTC (permalink / raw)
To: Vicki Pfau, Dmitry Torokhov, linux-input; +Cc: wiagn233
In-Reply-To: <20250917011937.1649481-1-vi@endrift.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 3076 bytes --]
Ping. Could anyone give this series a review?
在 2025/9/17 9:19, Vicki Pfau 写道:
> This introduces a new driver for the Xbox One/Series controller protocol,
> officially known as the Gaming Input Protocol, or GIP for short.
>
> Microsoft released documentation on (some of) GIP in late 2024, upon which
> this driver is based. Though the documentation was incomplete, it still
> provided enough information to warrant a clean start over the previous,
> incomplete implementation.
>
> This driver is already at feature parity with the GIP support in xpad,
> along with several more enhancements:
>
> - Proper support for parsing message length and fragmented messages
> - Metadata parsing, allowing for auto-detection on various parameters,
> including the presence and location in the message of the share button,
> as well as detection of specific device types
> - Controllable LED support
> - HID passthrough for the Chatpad
> - Preliminary support for racing wheels
>
> The framework set out in this driver also allows future expansion for
> specialized device types and additional features more cleanly than xpad.
>
> Future plans include:
>
> - Flight stick support
> - Improved support for racing wheels, including force feedback support
> - Support for the security handshake, which is required for devices that use
> wireless dongles
> - Exposing a raw character device to enable sending vendor-specific commands
> from userspace
> - Event logging to either sysfs or dmesg
> - Support for the headphone jack
> - Splitting the driver into separate drivers treating gip as a bus with each
> attachment being able to have its own gip_driver defined by a preferred type
> and/or GUID
>
> Also included in this series is the addition of three new ABS input types, with
> the two relevant ones to HID added to the mappings
>
> v2 of this series is mostly the same as v1 rebased onto dtor/master so it
> actually applies cleanly, with one major difference: flight stick support has
> been omitted, as I was unhappy with how mapping worked and want to discuss it
> further before having a patch readied.
>
> Vicki Pfau (5):
> Input: xbox_gip - Add new driver for Xbox GIP
> Input: xpad - Remove Xbox One support
> Input: Add ABS_CLUTCH, HANDBRAKE, and SHIFTER
> HID: Map more automobile simulation inputs
> Input: xbox_gip - Add wheel support
>
> Documentation/input/devices/xpad.rst | 17 +-
> MAINTAINERS | 6 +
> drivers/hid/hid-debug.c | 16 +-
> drivers/hid/hid-input.c | 2 +
> drivers/input/joystick/Kconfig | 26 +
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/xbox_gip.c | 3314 ++++++++++++++++++++++++
> drivers/input/joystick/xpad.c | 634 +----
> include/uapi/linux/input-event-codes.h | 3 +
> 9 files changed, 3372 insertions(+), 647 deletions(-)
> create mode 100644 drivers/input/joystick/xbox_gip.c
>
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6977 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply
* Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: Derek J. Clark @ 2025-11-03 4:28 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Jiri Kosina, Benjamin Tissoires, Corentin Chary, Luke D . Jones,
Hans de Goede, Ilpo Järvinen, Denis Benato,
kernel test robot, platform-driver-x86, linux-input,
oe-kbuild-all, linux-kernel, Derek J. Clark
In-Reply-To: <CAGwozwFgd91n2HnHn0VEL3BTGkj8QCRnp2jfCsMB38JqK8znNg@mail.gmail.com>
>On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@kernel.org> wrote:
>>
>> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
>>
>> > > 1589
>> > > 1590 static void kbd_led_update_all(struct work_struct *work)
>> > > 1591 {
>> > > 1592 enum led_brightness value;
>> > > 1593 struct asus_wmi *asus;
>> > > 1594 bool registered, notify;
>> > > 1595 int ret;
>> > /\ value should have been an int and
>> > placed here. It can take the value -1 hence the check
>>
>> Thanks, that needs to be fixed before the final merge.
>>
>> > Are there any other comments on the series?
>> >
>> > The only issue I am aware of is that Denis identified a bug in asusd
>> > (asusctl userspace program daemon) in certain Asus G14/G16 laptops
>> > that cause laptop keys to become sticky, I have had users also report
>> > that bug in previous versions of the series. WIthout asusd running,
>> > keyboards work fine incl. with brightness control (did not work
>> > before). Given it will take two months for this to reach mainline, I
>> > think it is a fair amount of time to address the bug.
>>
>> One thing that is not clear to me about this -- is this causing a visible
>> user-space behavior regression before vs. after the patchset with asusctl?
>>
>> If so, I am afraid this needs to be root-caused and fixed before the set
>> can be considered for inclusion.
>Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
>ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
>confuse asusd. Since the devices are the same as with hid-asus not
>loaded, it is specific to that program.
>
>
Hi Antheas.
While you have previously expressed to me directly that you wish InputPlumber
didn't exist, it still very much does, in fact, exist. I also know that you are
explicitly aware that InputPlumber is a consumer of this interface, so your
comment that asusctl is the only affected program is something you know to be
false. This is not even the first time you have renamed an input device that
you knew InputPlumber was a consumer of without notifying me[1].
I can't abide you outright lying to the maintainers here and I'm sick and tired
of having to watch your every move on the LKML. Either become a good citizen of
kernel maintenance, or get out of it.
Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
Nacked-By: Derek J. Clark <derekjohn.clark@gmail.com>
- Derek
[1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
>We can delay that patch until Denis who took over maintenance of the
>program can have a deeper look. I will still keep the last part of
>that patch that skips the input check, because that causes errors in
>devices that do not create an input device (e.g., lightbar).
>
>Antheas
^ permalink raw reply
* Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: Antheas Kapenekakis @ 2025-11-03 7:36 UTC (permalink / raw)
To: Derek J. Clark
Cc: Jiri Kosina, Benjamin Tissoires, Corentin Chary, Luke D . Jones,
Hans de Goede, Ilpo Järvinen, Denis Benato,
kernel test robot, platform-driver-x86, linux-input,
oe-kbuild-all, linux-kernel
In-Reply-To: <20251103042848.9302-1-derekjohn.clark@gmail.com>
On Mon, 3 Nov 2025 at 05:29, Derek J. Clark <derekjohn.clark@gmail.com> wrote:
>
> >On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@kernel.org> wrote:
> >>
> >> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
> >>
> >> > > 1589
> >> > > 1590 static void kbd_led_update_all(struct work_struct *work)
> >> > > 1591 {
> >> > > 1592 enum led_brightness value;
> >> > > 1593 struct asus_wmi *asus;
> >> > > 1594 bool registered, notify;
> >> > > 1595 int ret;
> >> > /\ value should have been an int and
> >> > placed here. It can take the value -1 hence the check
> >>
> >> Thanks, that needs to be fixed before the final merge.
> >>
> >> > Are there any other comments on the series?
> >> >
> >> > The only issue I am aware of is that Denis identified a bug in asusd
> >> > (asusctl userspace program daemon) in certain Asus G14/G16 laptops
> >> > that cause laptop keys to become sticky, I have had users also report
> >> > that bug in previous versions of the series. WIthout asusd running,
> >> > keyboards work fine incl. with brightness control (did not work
> >> > before). Given it will take two months for this to reach mainline, I
> >> > think it is a fair amount of time to address the bug.
> >>
> >> One thing that is not clear to me about this -- is this causing a visible
> >> user-space behavior regression before vs. after the patchset with asusctl?
> >>
> >> If so, I am afraid this needs to be root-caused and fixed before the set
> >> can be considered for inclusion.
>
> >Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
> >ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
> >confuse asusd. Since the devices are the same as with hid-asus not
> >loaded, it is specific to that program.
> >
> >
> Hi Antheas.
>
> While you have previously expressed to me directly that you wish InputPlumber
> didn't exist, it still very much does, in fact, exist. I also know that you are
> explicitly aware that InputPlumber is a consumer of this interface, so your
> comment that asusctl is the only affected program is something you know to be
> false. This is not even the first time you have renamed an input device that
> you knew InputPlumber was a consumer of without notifying me[1].
>
> I can't abide you outright lying to the maintainers here and I'm sick and tired
> of having to watch your every move on the LKML. Either become a good citizen of
> kernel maintenance, or get out of it.
Hi Derek,
I am not aware if your software is affected or not by this series as I
have not received reports about it.
If it is, please add:
"ASUSTeK Computer Inc. N-KEY Device"
As a name match to your software (should only take 5 minutes).
I worked with Luke and Dennis on it for the better part of a year so
hopefully they forwarded to you if it affects you or not.
Your software relies on OOT drivers for most devices incl. the Ally so
I am unsure if it is affected in reality. E.g., it would not be
affected in SteamOS and CachyOS. In the future, it would be good to
avoid name matches for your software as it makes it very fragile,
which is a discussion we have had before. I do not think device evdev
names constitute an ABI technically.
Best,
Antheas
> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
> Nacked-By: Derek J. Clark <derekjohn.clark@gmail.com>
>
> - Derek
>
> [1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
>
> >We can delay that patch until Denis who took over maintenance of the
> >program can have a deeper look. I will still keep the last part of
> >that patch that skips the input check, because that causes errors in
> >devices that do not create an input device (e.g., lightbar).
> >
> >Antheas
>
>
^ permalink raw reply
* Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: luke @ 2025-11-03 8:37 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Derek J. Clark, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Hans de Goede, Ilpo Järvinen, Denis Benato,
kernel test robot, platform-driver-x86, linux-input,
oe-kbuild-all, linux-kernel
In-Reply-To: <CAGwozwHCnHwOVw08ZJ4LOFD8kDv+kevvF1-PkjBq2+VMBBx9TQ@mail.gmail.com>
> On 3 Nov 2025, at 20:36, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Mon, 3 Nov 2025 at 05:29, Derek J. Clark <derekjohn.clark@gmail.com> wrote:
>>
>>> On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@kernel.org> wrote:
>>>>
>>>> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
>>>>
>>>>>> 1589
>>>>>> 1590 static void kbd_led_update_all(struct work_struct *work)
>>>>>> 1591 {
>>>>>> 1592 enum led_brightness value;
>>>>>> 1593 struct asus_wmi *asus;
>>>>>> 1594 bool registered, notify;
>>>>>> 1595 int ret;
>>>>> /\ value should have been an int and
>>>>> placed here. It can take the value -1 hence the check
>>>>
>>>> Thanks, that needs to be fixed before the final merge.
>>>>
>>>>> Are there any other comments on the series?
>>>>>
>>>>> The only issue I am aware of is that Denis identified a bug in asusd
>>>>> (asusctl userspace program daemon) in certain Asus G14/G16 laptops
>>>>> that cause laptop keys to become sticky, I have had users also report
>>>>> that bug in previous versions of the series. WIthout asusd running,
>>>>> keyboards work fine incl. with brightness control (did not work
>>>>> before). Given it will take two months for this to reach mainline, I
>>>>> think it is a fair amount of time to address the bug.
>>>>
>>>> One thing that is not clear to me about this -- is this causing a visible
>>>> user-space behavior regression before vs. after the patchset with asusctl?
>>>>
>>>> If so, I am afraid this needs to be root-caused and fixed before the set
>>>> can be considered for inclusion.
>>
>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
>>> ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
>>> confuse asusd. Since the devices are the same as with hid-asus not
>>> loaded, it is specific to that program.
>>>
>>>
>> Hi Antheas.
>>
>> While you have previously expressed to me directly that you wish InputPlumber
>> didn't exist, it still very much does, in fact, exist. I also know that you are
>> explicitly aware that InputPlumber is a consumer of this interface, so your
>> comment that asusctl is the only affected program is something you know to be
>> false. This is not even the first time you have renamed an input device that
>> you knew InputPlumber was a consumer of without notifying me[1].
>>
>> I can't abide you outright lying to the maintainers here and I'm sick and tired
>> of having to watch your every move on the LKML. Either become a good citizen of
>> kernel maintenance, or get out of it.
>
> Hi Derek,
> I am not aware if your software is affected or not by this series as I
> have not received reports about it.
>
> If it is, please add:
> "ASUSTeK Computer Inc. N-KEY Device"
>
> As a name match to your software (should only take 5 minutes).
>
> I worked with Luke and Dennis on it for the better part of a year so
> hopefully they forwarded to you if it affects you or not.
>
> Your software relies on OOT drivers for most devices incl. the Ally so
> I am unsure if it is affected in reality. E.g., it would not be
> affected in SteamOS and CachyOS. In the future, it would be good to
> avoid name matches for your software as it makes it very fragile,
> which is a discussion we have had before. I do not think device evdev
> names constitute an ABI technically.
Taking no sides here.
An unfortunate reality is that there is stuff out there that does use name matches (and yes I agree they shouldn’t because it is *not* an ABI and many many devices have had name changes over the decades).
While name strings aren't a formal ABI, when a change affects known downstream users, a heads-up helps the ecosystem adapt smoothly—even if the technical stance is that they shouldn't rely on names.
In general it also needs to be justified such as:
- "Matches updated product branding"
- "Current string is misleading (says 'mouse' but it's a keyboard)"
- "Fixing spelling error"
- "Aligning with USB-IF device class names"
I always advocated use of evdev libraries to discover devices rather than the shortcut of name matching as it is much more powerful and reliable (which is how asusctl does dynamic add/remove of LED dev dbus interfaces). It’s much better to use evdev with vid/pid, device sub/classes, into descriptors and so on - you can be as open or restrictive as required by use case. This particular issue illustrates why this approach is preferable.
If the name change is a result of something I said or missed then I apologise to both Derek and yourself. Likely I missed it as I’ve never relied on name strings for userspace.
Regarding the OOT ally drivers I started, these will of course get upstreamed in the future (by Denis in this case when he can). They are getting very heavily battled tested in the mean time. Please do contribute to them if there is anything required to be addressed or chat to Denis, after all they are made only to benefit all users (there is no *race to be first* here.
As I no longer work on Asus laptops/handhelds and don't have hardware left to test with, I can't contribute further to this discussion. Best of luck resolving this.
I'm out.
Luke.
>
> Best,
> Antheas
>
>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
>> Nacked-By: Derek J. Clark <derekjohn.clark@gmail.com>
>>
>> - Derek
>>
>> [1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
>>
>>> We can delay that patch until Denis who took over maintenance of the
>>> program can have a deeper look. I will still keep the last part of
>>> that patch that skips the input check, because that causes errors in
>>> devices that do not create an input device (e.g., lightbar).
>>>
>>> Antheas
^ permalink raw reply
* Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: Antheas Kapenekakis @ 2025-11-03 8:48 UTC (permalink / raw)
To: luke
Cc: Derek J. Clark, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Hans de Goede, Ilpo Järvinen, Denis Benato,
kernel test robot, platform-driver-x86, linux-input,
oe-kbuild-all, linux-kernel
In-Reply-To: <52B61B78-4177-474B-9D40-471ED918C951@ljones.dev>
On Mon, 3 Nov 2025 at 09:38, <luke@ljones.dev> wrote:
>
>
> > On 3 Nov 2025, at 20:36, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Mon, 3 Nov 2025 at 05:29, Derek J. Clark <derekjohn.clark@gmail.com> wrote:
> >>
> >>> On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@kernel.org> wrote:
> >>>>
> >>>> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
> >>>>
> >>>>>> 1589
> >>>>>> 1590 static void kbd_led_update_all(struct work_struct *work)
> >>>>>> 1591 {
> >>>>>> 1592 enum led_brightness value;
> >>>>>> 1593 struct asus_wmi *asus;
> >>>>>> 1594 bool registered, notify;
> >>>>>> 1595 int ret;
> >>>>> /\ value should have been an int and
> >>>>> placed here. It can take the value -1 hence the check
> >>>>
> >>>> Thanks, that needs to be fixed before the final merge.
> >>>>
> >>>>> Are there any other comments on the series?
> >>>>>
> >>>>> The only issue I am aware of is that Denis identified a bug in asusd
> >>>>> (asusctl userspace program daemon) in certain Asus G14/G16 laptops
> >>>>> that cause laptop keys to become sticky, I have had users also report
> >>>>> that bug in previous versions of the series. WIthout asusd running,
> >>>>> keyboards work fine incl. with brightness control (did not work
> >>>>> before). Given it will take two months for this to reach mainline, I
> >>>>> think it is a fair amount of time to address the bug.
> >>>>
> >>>> One thing that is not clear to me about this -- is this causing a visible
> >>>> user-space behavior regression before vs. after the patchset with asusctl?
> >>>>
> >>>> If so, I am afraid this needs to be root-caused and fixed before the set
> >>>> can be considered for inclusion.
> >>
> >>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
> >>> ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
> >>> confuse asusd. Since the devices are the same as with hid-asus not
> >>> loaded, it is specific to that program.
> >>>
> >>>
> >> Hi Antheas.
> >>
> >> While you have previously expressed to me directly that you wish InputPlumber
> >> didn't exist, it still very much does, in fact, exist. I also know that you are
> >> explicitly aware that InputPlumber is a consumer of this interface, so your
> >> comment that asusctl is the only affected program is something you know to be
> >> false. This is not even the first time you have renamed an input device that
> >> you knew InputPlumber was a consumer of without notifying me[1].
> >>
> >> I can't abide you outright lying to the maintainers here and I'm sick and tired
> >> of having to watch your every move on the LKML. Either become a good citizen of
> >> kernel maintenance, or get out of it.
> >
> > Hi Derek,
> > I am not aware if your software is affected or not by this series as I
> > have not received reports about it.
> >
> > If it is, please add:
> > "ASUSTeK Computer Inc. N-KEY Device"
> >
> > As a name match to your software (should only take 5 minutes).
> >
> > I worked with Luke and Dennis on it for the better part of a year so
> > hopefully they forwarded to you if it affects you or not.
> >
> > Your software relies on OOT drivers for most devices incl. the Ally so
> > I am unsure if it is affected in reality. E.g., it would not be
> > affected in SteamOS and CachyOS. In the future, it would be good to
> > avoid name matches for your software as it makes it very fragile,
> > which is a discussion we have had before. I do not think device evdev
> > names constitute an ABI technically.
>
> Taking no sides here.
>
> An unfortunate reality is that there is stuff out there that does use name matches (and yes I agree they shouldn’t because it is *not* an ABI and many many devices have had name changes over the decades).
>
> While name strings aren't a formal ABI, when a change affects known downstream users, a heads-up helps the ecosystem adapt smoothly—even if the technical stance is that they shouldn't rely on names.
>
> In general it also needs to be justified such as:
> - "Matches updated product branding"
> - "Current string is misleading (says 'mouse' but it's a keyboard)"
> - "Fixing spelling error"
> - "Aligning with USB-IF device class names"
>
> I always advocated use of evdev libraries to discover devices rather than the shortcut of name matching as it is much more powerful and reliable (which is how asusctl does dynamic add/remove of LED dev dbus interfaces). It’s much better to use evdev with vid/pid, device sub/classes, into descriptors and so on - you can be as open or restrictive as required by use case. This particular issue illustrates why this approach is preferable.
>
> If the name change is a result of something I said or missed then I apologise to both Derek and yourself. Likely I missed it as I’ve never relied on name strings for userspace.
>
> Regarding the OOT ally drivers I started, these will of course get upstreamed in the future (by Denis in this case when he can). They are getting very heavily battled tested in the mean time. Please do contribute to them if there is anything required to be addressed or chat to Denis, after all they are made only to benefit all users (there is no *race to be first* here.
>
> As I no longer work on Asus laptops/handhelds and don't have hardware left to test with, I can't contribute further to this discussion. Best of luck resolving this.
>
> I'm out.
> Luke.
Hi Luke,
good luck to your future endeavors.
The OOT reference was not to disparage the drivers, just to note that
in kernels that use those, hid-asus is stubbed for the Ally so there
is no change there.
As for reasons, see below.
> - "Matches updated product branding"
Reason for [1]
> - "Aligning with USB-IF device class names"
Reason for the change in this patch
If you would like me to stop cc'ing you in future asus changes let me
know. I am preparing some additional patches for the Duo class of
laptops.
Best,
Antheas
> >
> > Best,
> > Antheas
> >
> >> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
> >> Nacked-By: Derek J. Clark <derekjohn.clark@gmail.com>
> >>
> >> - Derek
> >>
> >> [1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
> >>
> >>> We can delay that patch until Denis who took over maintenance of the
> >>> program can have a deeper look. I will still keep the last part of
> >>> that patch that skips the input check, because that causes errors in
> >>> devices that do not create an input device (e.g., lightbar).
> >>>
> >>> Antheas
>
>
>
^ permalink raw reply
* Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: luke @ 2025-11-03 9:05 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Derek J. Clark, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Hans de Goede, Ilpo Järvinen, Denis Benato,
kernel test robot, platform-driver-x86, linux-input,
oe-kbuild-all, linux-kernel
In-Reply-To: <CAGwozwGJUPQLB2UrZubUj9Sd-MijM0ADepVwEaNo6MNRafD=Gg@mail.gmail.com>
> On 3 Nov 2025, at 21:48, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Mon, 3 Nov 2025 at 09:38, <luke@ljones.dev> wrote:
>>
>>
>>> On 3 Nov 2025, at 20:36, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>
>>> On Mon, 3 Nov 2025 at 05:29, Derek J. Clark <derekjohn.clark@gmail.com> wrote:
>>>>
>>>>> On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@kernel.org> wrote:
>>>>>>
>>>>>> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
>>>>>>
>>>>>>>> 1589
>>>>>>>> 1590 static void kbd_led_update_all(struct work_struct *work)
>>>>>>>> 1591 {
>>>>>>>> 1592 enum led_brightness value;
>>>>>>>> 1593 struct asus_wmi *asus;
>>>>>>>> 1594 bool registered, notify;
>>>>>>>> 1595 int ret;
>>>>>>> /\ value should have been an int and
>>>>>>> placed here. It can take the value -1 hence the check
>>>>>>
>>>>>> Thanks, that needs to be fixed before the final merge.
>>>>>>
>>>>>>> Are there any other comments on the series?
>>>>>>>
>>>>>>> The only issue I am aware of is that Denis identified a bug in asusd
>>>>>>> (asusctl userspace program daemon) in certain Asus G14/G16 laptops
>>>>>>> that cause laptop keys to become sticky, I have had users also report
>>>>>>> that bug in previous versions of the series. WIthout asusd running,
>>>>>>> keyboards work fine incl. with brightness control (did not work
>>>>>>> before). Given it will take two months for this to reach mainline, I
>>>>>>> think it is a fair amount of time to address the bug.
>>>>>>
>>>>>> One thing that is not clear to me about this -- is this causing a visible
>>>>>> user-space behavior regression before vs. after the patchset with asusctl?
>>>>>>
>>>>>> If so, I am afraid this needs to be root-caused and fixed before the set
>>>>>> can be considered for inclusion.
>>>>
>>>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
>>>>> ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
>>>>> confuse asusd. Since the devices are the same as with hid-asus not
>>>>> loaded, it is specific to that program.
>>>>>
>>>>>
>>>> Hi Antheas.
>>>>
>>>> While you have previously expressed to me directly that you wish InputPlumber
>>>> didn't exist, it still very much does, in fact, exist. I also know that you are
>>>> explicitly aware that InputPlumber is a consumer of this interface, so your
>>>> comment that asusctl is the only affected program is something you know to be
>>>> false. This is not even the first time you have renamed an input device that
>>>> you knew InputPlumber was a consumer of without notifying me[1].
>>>>
>>>> I can't abide you outright lying to the maintainers here and I'm sick and tired
>>>> of having to watch your every move on the LKML. Either become a good citizen of
>>>> kernel maintenance, or get out of it.
>>>
>>> Hi Derek,
>>> I am not aware if your software is affected or not by this series as I
>>> have not received reports about it.
>>>
>>> If it is, please add:
>>> "ASUSTeK Computer Inc. N-KEY Device"
>>>
>>> As a name match to your software (should only take 5 minutes).
>>>
>>> I worked with Luke and Dennis on it for the better part of a year so
>>> hopefully they forwarded to you if it affects you or not.
>>>
>>> Your software relies on OOT drivers for most devices incl. the Ally so
>>> I am unsure if it is affected in reality. E.g., it would not be
>>> affected in SteamOS and CachyOS. In the future, it would be good to
>>> avoid name matches for your software as it makes it very fragile,
>>> which is a discussion we have had before. I do not think device evdev
>>> names constitute an ABI technically.
>>
>> Taking no sides here.
>>
>> An unfortunate reality is that there is stuff out there that does use name matches (and yes I agree they shouldn’t because it is *not* an ABI and many many devices have had name changes over the decades).
>>
>> While name strings aren't a formal ABI, when a change affects known downstream users, a heads-up helps the ecosystem adapt smoothly—even if the technical stance is that they shouldn't rely on names.
>>
>> In general it also needs to be justified such as:
>> - "Matches updated product branding"
>> - "Current string is misleading (says 'mouse' but it's a keyboard)"
>> - "Fixing spelling error"
>> - "Aligning with USB-IF device class names"
>>
>> I always advocated use of evdev libraries to discover devices rather than the shortcut of name matching as it is much more powerful and reliable (which is how asusctl does dynamic add/remove of LED dev dbus interfaces). It’s much better to use evdev with vid/pid, device sub/classes, into descriptors and so on - you can be as open or restrictive as required by use case. This particular issue illustrates why this approach is preferable.
>>
>> If the name change is a result of something I said or missed then I apologise to both Derek and yourself. Likely I missed it as I’ve never relied on name strings for userspace.
>>
>> Regarding the OOT ally drivers I started, these will of course get upstreamed in the future (by Denis in this case when he can). They are getting very heavily battled tested in the mean time. Please do contribute to them if there is anything required to be addressed or chat to Denis, after all they are made only to benefit all users (there is no *race to be first* here.
>>
>> As I no longer work on Asus laptops/handhelds and don't have hardware left to test with, I can't contribute further to this discussion. Best of luck resolving this.
>>
>> I'm out.
>> Luke.
>
> Hi Luke,
> good luck to your future endeavors.
>
> The OOT reference was not to disparage the drivers, just to note that
> in kernels that use those, hid-asus is stubbed for the Ally so there
> is no change there.
>
> As for reasons, see below.
>
>> - "Matches updated product branding"
>
> Reason for [1]
>
>> - "Aligning with USB-IF device class names"
>
> Reason for the change in this patch
Ack. Thank you, appreciate the clarification, this is the appropriate reason. Might be worth mentioning in change cover letter or commit message (if not already, sorry if I missed).
>
> If you would like me to stop cc'ing you in future asus changes let me
> know. I am preparing some additional patches for the Duo class of
> laptops.
Leave me in for the moment. I’ll submit a patch to remove myself from maintainers once Denis is comfortable in the role.
Regarding Duo, I assume you mean the larger type with two full-size screens and not the older models with a thin screen above the keyboard?
There was an issue with the older ones which I could not solve until I had the physical hardware to test due to it being difficult for testers to describe the exact behaviour. The way the power-off and brightness control works is funky. I submitted the patch here https://lore.kernel.org/all/20250525204214.104030-1-luke@ljones.dev/
Hopefully that’s of some help to you. It’s very unlikely I will resubmit that as a split series.
If you need anything from me related to above or otherwise, do please reach out off-chain to avoid us further hitting the CC.
>
> Best,
> Antheas
>
>>>
>>> Best,
>>> Antheas
>>>
>>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
>>>> Nacked-By: Derek J. Clark <derekjohn.clark@gmail.com>
>>>>
>>>> - Derek
>>>>
>>>> [1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
>>>>
>>>>> We can delay that patch until Denis who took over maintenance of the
>>>>> program can have a deeper look. I will still keep the last part of
>>>>> that patch that skips the input check, because that causes errors in
>>>>> devices that do not create an input device (e.g., lightbar).
>>>>>
>>>>> Antheas
^ permalink raw reply
* Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple kbd led handlers
From: Antheas Kapenekakis @ 2025-11-03 9:15 UTC (permalink / raw)
To: luke
Cc: Derek J. Clark, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Ilpo Järvinen, Denis Benato, kernel test robot,
platform-driver-x86, linux-input, oe-kbuild-all, linux-kernel
In-Reply-To: <4A46B714-E4D8-43B3-A4F0-53FF76E41BE4@ljones.dev>
On Mon, 3 Nov 2025 at 10:06, <luke@ljones.dev> wrote:
>
>
>
> > On 3 Nov 2025, at 21:48, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Mon, 3 Nov 2025 at 09:38, <luke@ljones.dev> wrote:
> >>
> >>
> >>> On 3 Nov 2025, at 20:36, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >>>
> >>> On Mon, 3 Nov 2025 at 05:29, Derek J. Clark <derekjohn.clark@gmail.com> wrote:
> >>>>
> >>>>> On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@kernel.org> wrote:
> >>>>>>
> >>>>>> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
> >>>>>>
> >>>>>>>> 1589
> >>>>>>>> 1590 static void kbd_led_update_all(struct work_struct *work)
> >>>>>>>> 1591 {
> >>>>>>>> 1592 enum led_brightness value;
> >>>>>>>> 1593 struct asus_wmi *asus;
> >>>>>>>> 1594 bool registered, notify;
> >>>>>>>> 1595 int ret;
> >>>>>>> /\ value should have been an int and
> >>>>>>> placed here. It can take the value -1 hence the check
> >>>>>>
> >>>>>> Thanks, that needs to be fixed before the final merge.
> >>>>>>
> >>>>>>> Are there any other comments on the series?
> >>>>>>>
> >>>>>>> The only issue I am aware of is that Denis identified a bug in asusd
> >>>>>>> (asusctl userspace program daemon) in certain Asus G14/G16 laptops
> >>>>>>> that cause laptop keys to become sticky, I have had users also report
> >>>>>>> that bug in previous versions of the series. WIthout asusd running,
> >>>>>>> keyboards work fine incl. with brightness control (did not work
> >>>>>>> before). Given it will take two months for this to reach mainline, I
> >>>>>>> think it is a fair amount of time to address the bug.
> >>>>>>
> >>>>>> One thing that is not clear to me about this -- is this causing a visible
> >>>>>> user-space behavior regression before vs. after the patchset with asusctl?
> >>>>>>
> >>>>>> If so, I am afraid this needs to be root-caused and fixed before the set
> >>>>>> can be considered for inclusion.
> >>>>
> >>>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
> >>>>> ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
> >>>>> confuse asusd. Since the devices are the same as with hid-asus not
> >>>>> loaded, it is specific to that program.
> >>>>>
> >>>>>
> >>>> Hi Antheas.
> >>>>
> >>>> While you have previously expressed to me directly that you wish InputPlumber
> >>>> didn't exist, it still very much does, in fact, exist. I also know that you are
> >>>> explicitly aware that InputPlumber is a consumer of this interface, so your
> >>>> comment that asusctl is the only affected program is something you know to be
> >>>> false. This is not even the first time you have renamed an input device that
> >>>> you knew InputPlumber was a consumer of without notifying me[1].
> >>>>
> >>>> I can't abide you outright lying to the maintainers here and I'm sick and tired
> >>>> of having to watch your every move on the LKML. Either become a good citizen of
> >>>> kernel maintenance, or get out of it.
> >>>
> >>> Hi Derek,
> >>> I am not aware if your software is affected or not by this series as I
> >>> have not received reports about it.
> >>>
> >>> If it is, please add:
> >>> "ASUSTeK Computer Inc. N-KEY Device"
> >>>
> >>> As a name match to your software (should only take 5 minutes).
> >>>
> >>> I worked with Luke and Dennis on it for the better part of a year so
> >>> hopefully they forwarded to you if it affects you or not.
> >>>
> >>> Your software relies on OOT drivers for most devices incl. the Ally so
> >>> I am unsure if it is affected in reality. E.g., it would not be
> >>> affected in SteamOS and CachyOS. In the future, it would be good to
> >>> avoid name matches for your software as it makes it very fragile,
> >>> which is a discussion we have had before. I do not think device evdev
> >>> names constitute an ABI technically.
> >>
> >> Taking no sides here.
> >>
> >> An unfortunate reality is that there is stuff out there that does use name matches (and yes I agree they shouldn’t because it is *not* an ABI and many many devices have had name changes over the decades).
> >>
> >> While name strings aren't a formal ABI, when a change affects known downstream users, a heads-up helps the ecosystem adapt smoothly—even if the technical stance is that they shouldn't rely on names.
> >>
> >> In general it also needs to be justified such as:
> >> - "Matches updated product branding"
> >> - "Current string is misleading (says 'mouse' but it's a keyboard)"
> >> - "Fixing spelling error"
> >> - "Aligning with USB-IF device class names"
> >>
> >> I always advocated use of evdev libraries to discover devices rather than the shortcut of name matching as it is much more powerful and reliable (which is how asusctl does dynamic add/remove of LED dev dbus interfaces). It’s much better to use evdev with vid/pid, device sub/classes, into descriptors and so on - you can be as open or restrictive as required by use case. This particular issue illustrates why this approach is preferable.
> >>
> >> If the name change is a result of something I said or missed then I apologise to both Derek and yourself. Likely I missed it as I’ve never relied on name strings for userspace.
> >>
> >> Regarding the OOT ally drivers I started, these will of course get upstreamed in the future (by Denis in this case when he can). They are getting very heavily battled tested in the mean time. Please do contribute to them if there is anything required to be addressed or chat to Denis, after all they are made only to benefit all users (there is no *race to be first* here.
> >>
> >> As I no longer work on Asus laptops/handhelds and don't have hardware left to test with, I can't contribute further to this discussion. Best of luck resolving this.
> >>
> >> I'm out.
> >> Luke.
> >
> > Hi Luke,
> > good luck to your future endeavors.
> >
> > The OOT reference was not to disparage the drivers, just to note that
> > in kernels that use those, hid-asus is stubbed for the Ally so there
> > is no change there.
> >
> > As for reasons, see below.
> >
> >> - "Matches updated product branding"
> >
> > Reason for [1]
> >
> >> - "Aligning with USB-IF device class names"
> >
> > Reason for the change in this patch
>
> Ack. Thank you, appreciate the clarification, this is the appropriate reason. Might be worth mentioning in change cover letter or commit message (if not already, sorry if I missed).
>
> >
> > If you would like me to stop cc'ing you in future asus changes let me
> > know. I am preparing some additional patches for the Duo class of
> > laptops.
>
> Leave me in for the moment. I’ll submit a patch to remove myself from maintainers once Denis is comfortable in the role.
>
> Regarding Duo, I assume you mean the larger type with two full-size screens and not the older models with a thin screen above the keyboard?
> There was an issue with the older ones which I could not solve until I had the physical hardware to test due to it being difficult for testers to describe the exact behaviour. The way the power-off and brightness control works is funky. I submitted the patch here https://lore.kernel.org/all/20250525204214.104030-1-luke@ljones.dev/
>
> Hopefully that’s of some help to you. It’s very unlikely I will resubmit that as a split series.
>
> If you need anything from me related to above or otherwise, do please reach out off-chain to avoid us further hitting the CC.
Yes, the two screen type. I am focusing on the keyboard for now. I am
waiting for a user to test the changes. This series helps with
maintaining keyboard brightness on that. Hopefully it will work as the
keyboard transitions from USB to bluetooth.
If there are issues with the screen brightness, I will rereview your
patch and if it is needed adapt it and coby you.
Best,
Antheas
> >
> > Best,
> > Antheas
> >
> >>>
> >>> Best,
> >>> Antheas
> >>>
> >>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
> >>>> Nacked-By: Derek J. Clark <derekjohn.clark@gmail.com>
> >>>>
> >>>> - Derek
> >>>>
> >>>> [1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
> >>>>
> >>>>> We can delay that patch until Denis who took over maintenance of the
> >>>>> program can have a deeper look. I will still keep the last part of
> >>>>> that patch that skips the input check, because that causes errors in
> >>>>> devices that do not create an input device (e.g., lightbar).
> >>>>>
> >>>>> Antheas
>
>
>
^ permalink raw reply
* Re: [PATCH 2/2] Input: Add support for Wacom W9000-series penabled touchscreens
From: Kernel GMX @ 2025-11-03 10:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Hendrik Noack, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
In-Reply-To: <20251028-funky-rose-rook-3ccab5@kuoka>
28.10.2025 08:44:34 Krzysztof Kozlowski <krzk@kernel.org>:
> On Mon, Oct 27, 2025 at 10:25:35PM +0100, Hendrik Noack wrote:
>> +
>> +static const struct of_device_id wacom_w9000_of_match[] = {
>> + { .compatible = "wacom,w9007a_lt03", .data = &wacom_w9007a_lt03, },
>> + { .compatible = "wacom,w9007a_v1", .data = &wacom_w9007a_v1, },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, wacom_w9000_of_match);
>
> This goes next to other ID table, around the probe.
>
>> +
>> +static int wacom_w9000_read(struct i2c_client *client, u8 command, int len, char *data)
>> +{
>> + struct i2c_msg xfer[2];
>> + bool retried = false;
>> + int ret;
>> +
>
> ...
>
>> +
>> +static int wacom_w9000_probe(struct i2c_client *client)
>> +{
>> + struct device *dev = &client->dev;
>> + struct wacom_w9000_data *wacom_data;
>> + struct input_dev *input_dev;
>> + int ret;
>> + u32 val;
>> +
>> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> + dev_err(dev, "i2c_check_functionality error\n");
>> + return -EIO;
>> + }
>> +
>> + wacom_data = devm_kzalloc(dev, sizeof(struct wacom_w9000_data), GFP_KERNEL);
>
> sizeof(*)
>
> Please use existing kernel coding style, not some downstream version.
>
Okay, I was orienting myself on the atmel_mxt_ts driver and it had it like this.
>> + if (!wacom_data)
>> + return -ENOMEM;
>> +
>> + wacom_data->variant = i2c_get_match_data(client);
>> +
>> + wacom_data->client = client;
>> +
>> + input_dev = devm_input_allocate_device(dev);
>> + if (!input_dev)
>> + return -ENOMEM;
>> + wacom_data->input_dev = input_dev;
>> +
>> + wacom_data->irq = client->irq;
>> + i2c_set_clientdata(client, wacom_data);
>> +
>> + wacom_data->regulator = devm_regulator_get(dev, "vdd");
>> + if (IS_ERR(wacom_data->regulator)) {
>> + ret = PTR_ERR(wacom_data->regulator);
>> + dev_err(dev, "Failed to get regulators %d\n", ret);
>> + return ret;
>
> Nope. Look at all other drivers. Syntax is since some years return
> dev_err_probe.
>
>> + }
>> +
>> + /* Request flash-mode line and don't go into flash mode */
>> + wacom_data->flash_mode_gpio = devm_gpiod_get_optional(dev, "flash-mode", GPIOD_OUT_LOW);
>> + if (IS_ERR(wacom_data->flash_mode_gpio)) {
>> + ret = PTR_ERR(wacom_data->flash_mode_gpio);
>> + dev_err(dev, "Failed to get flash-mode gpio: %d\n", ret);
>> + return ret;
>
> You must handle deferred probe. Please look at all other drivers how
> they do it.
>
>> + }
>> +
>> + /* Request pdct line */
>> + wacom_data->pen_detect_gpio = devm_gpiod_get_optional(dev, "pdct", GPIOD_IN);
>> + if (IS_ERR(wacom_data->pen_detect_gpio)) {
>> + ret = PTR_ERR(wacom_data->pen_detect_gpio);
>> + dev_err(dev, "Failed to get pdct gpio: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* Request pen-insert line */
>> + wacom_data->pen_inserted_gpio = devm_gpiod_get_optional(dev, "pen-inserted", GPIOD_IN);
>> + if (IS_ERR(wacom_data->pen_inserted_gpio)) {
>> + ret = PTR_ERR(wacom_data->pen_inserted_gpio);
>> + dev_err(dev, "Failed to get pen-insert gpio: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = regulator_enable(wacom_data->regulator);
>> + if (ret) {
>> + dev_err(dev, "Failed to enable regulators: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + msleep(200);
>> +
>> + ret = wacom_w9000_query(wacom_data);
>> + if (ret)
>> + goto err_disable_regulators;
>> +
>> + input_dev->name = wacom_data->variant->name;
>> + input_dev->id.bustype = BUS_I2C;
>> + input_dev->dev.parent = dev;
>> + input_dev->id.vendor = 0x56a;
>> + input_dev->id.version = wacom_data->fw_version;
>> + input_dev->open = wacom_w9000_open;
>> + input_dev->close = wacom_w9000_close;
>> +
>> + __set_bit(EV_KEY, input_dev->evbit);
>> + __set_bit(EV_ABS, input_dev->evbit);
>> + __set_bit(BTN_TOUCH, input_dev->keybit);
>> + __set_bit(BTN_TOOL_PEN, input_dev->keybit);
>> + __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
>> + __set_bit(BTN_STYLUS, input_dev->keybit);
>> +
>> + // Calculate x and y resolution from size in devicetree
>> + ret = device_property_read_u32(dev, "touchscreen-x-mm", &val);
>> + if (ret)
>> + input_abs_set_res(input_dev, ABS_X, 100);
>> + else
>> + input_abs_set_res(input_dev, ABS_X, wacom_data->prop.max_x / val);
>> + ret = device_property_read_u32(dev, "touchscreen-y-mm", &val);
>> + if (ret)
>> + input_abs_set_res(input_dev, ABS_Y, 100);
>> + else
>> + input_abs_set_res(input_dev, ABS_Y, wacom_data->prop.max_y / val);
>> +
>> + input_set_abs_params(input_dev, ABS_X, 0, wacom_data->prop.max_x, 4, 0);
>> + input_set_abs_params(input_dev, ABS_Y, 0, wacom_data->prop.max_y, 4, 0);
>> + input_set_abs_params(input_dev, ABS_PRESSURE, 0, wacom_data->max_pressure, 0, 0);
>> + input_set_abs_params(input_dev, ABS_DISTANCE, 0, 255, 0, 0);
>> +
>> + touchscreen_parse_properties(input_dev, false, &wacom_data->prop);
>> +
>> + ret = devm_request_threaded_irq(dev, wacom_data->irq, NULL, wacom_w9000_interrupt,
>> + IRQF_ONESHOT | IRQF_NO_AUTOEN, client->name, wacom_data);
>> + if (ret) {
>> + dev_err(dev, "Failed to register interrupt\n");
>> + goto err_disable_regulators;
>> + }
>> +
>> + if (wacom_data->pen_detect_gpio) {
>> + wacom_data->pen_detect_irq = gpiod_to_irq(wacom_data->pen_detect_gpio);
>
> Why is this a GPIO? Your binding said this is GPIO, your code says this
> is an interrupt.
You are right, it's not necessary here.
>> + ret = devm_request_threaded_irq(dev, wacom_data->pen_detect_irq, NULL,
>> + wacom_w9000_interrupt_pen_detect, IRQF_ONESHOT |
>> + IRQF_NO_AUTOEN | IRQF_TRIGGER_RISING |
>> + IRQF_TRIGGER_FALLING, "wacom_pdct", wacom_data);
>> + if (ret) {
>> + dev_err(dev, "Failed to register pdct interrupt\n");
>> + goto err_disable_regulators;
>> + }
>> + }
>> +
>> + if (wacom_data->pen_inserted_gpio) {
>> + input_dev->evbit[0] |= BIT_MASK(EV_SW);
>> + input_set_capability(input_dev, EV_SW, SW_PEN_INSERTED);
>> + wacom_data->pen_insert_irq = gpiod_to_irq(wacom_data->pen_inserted_gpio);
>
> Same question here.
>
The driver needs to know the GPIO state to determine if the pen is inserted
so that it can enable or disable the regulator and main interrupt accordingly.
An interrupt for this GPIO is necessary for this evaluation to occur
on every signal change.
>> + ret = devm_request_threaded_irq(dev, wacom_data->pen_insert_irq, NULL,
>> + wacom_w9000_interrupt_pen_insert, IRQF_ONESHOT |
>> + IRQF_NO_AUTOEN | IRQF_TRIGGER_RISING |
>> + IRQF_TRIGGER_FALLING, "wacom_pen_insert",
>> + wacom_data);
>> + if (ret) {
>> + dev_err(dev, "Failed to register pen-insert interrupt\n");
>> + goto err_disable_regulators;
>> + }
>> + }
>> +
...
Thank you for you feedback, I will address the feedback in my next iteration.
Best regards,
Hendrik
^ permalink raw reply
* [PATCH] HID: elecom: Add support for ELECOM M-XT3URBK (018F)
From: Naoki Ueki @ 2025-11-03 12:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: Naoki Ueki, linux-input, linux-kernel
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>
---
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 69771fd35006..981d1b6e9658 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 0723b4b1c9ec..2245d85cacf5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -449,7 +449,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 bcd4bccf1a7c..5ec5eded5f0b 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -410,7 +410,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.2
^ permalink raw reply related
* [PATCH] HID: corsair-void: Use %pe for printing PTR_ERR
From: Stuart Hayhurst @ 2025-11-03 14:21 UTC (permalink / raw)
To: jikos
Cc: Stuart Hayhurst, Benjamin Tissoires, linux-input, linux-kernel,
kernel test robot, Julia Lawall
Use %pe to print a PTR_ERR to silence a cocci warning
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@inria.fr>
Closes: https://lore.kernel.org/r/202510300342.WtPn2jF3-lkp@intel.com/
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-corsair-void.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
index fee134a7eba3..5e9a5b8f7f16 100644
--- a/drivers/hid/hid-corsair-void.c
+++ b/drivers/hid/hid-corsair-void.c
@@ -553,9 +553,8 @@ static void corsair_void_add_battery(struct corsair_void_drvdata *drvdata)
if (IS_ERR(new_supply)) {
hid_err(drvdata->hid_dev,
- "failed to register battery '%s' (reason: %ld)\n",
- drvdata->battery_desc.name,
- PTR_ERR(new_supply));
+ "failed to register battery '%s' (reason: %pe)\n",
+ drvdata->battery_desc.name, new_supply);
return;
}
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] Input: imx_sc_key - Fix memory corruption on unload
From: Frank Li @ 2025-11-03 15:12 UTC (permalink / raw)
To: Dan Carpenter
Cc: Anson Huang, Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-input, linux-kernel, imx, linux-arm-kernel, kernel-janitors
In-Reply-To: <aQYKR75r2VMFJutT@stanley.mountain>
On Sat, Nov 01, 2025 at 04:25:27PM +0300, Dan Carpenter wrote:
> This is supposed to be "priv" but we accidentally pass "&priv" which is
> an address in the stack and so it will lead to memory corruption when
> the imx_sc_key_action() function is called. Remove the &.
>
> Fixes: 768062fd1284 ("Input: imx_sc_key - use devm_add_action_or_reset() to handle all cleanups")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/input/keyboard/imx_sc_key.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/keyboard/imx_sc_key.c b/drivers/input/keyboard/imx_sc_key.c
> index d18839f1f4f6..b620cd310cdb 100644
> --- a/drivers/input/keyboard/imx_sc_key.c
> +++ b/drivers/input/keyboard/imx_sc_key.c
> @@ -158,7 +158,7 @@ static int imx_sc_key_probe(struct platform_device *pdev)
> return error;
> }
>
> - error = devm_add_action_or_reset(&pdev->dev, imx_sc_key_action, &priv);
> + error = devm_add_action_or_reset(&pdev->dev, imx_sc_key_action, priv);
> if (error)
> return error;
>
> --
> 2.51.0
>
^ permalink raw reply
* [PATCH] HID: mcp2221: fix slab out-of-bounds in mcp2221_raw_event
From: Atharv Dubey @ 2025-11-03 18:25 UTC (permalink / raw)
To: jikos
Cc: bentiss, linux-input, linux-kernel, Atharv Dubey,
syzbot+1018672fe70298606e5f
Fixes an out-of-bounds read triggered by malformed HID input reports.
Fixes: 3a8660878839 ("HID: mcp2221: add support for MCP2221 HID adapter")
Reported-by: syzbot+1018672fe70298606e5f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
Signed-off-by: Atharv Dubey <atharvd440@gmail.com>
---
drivers/hid/hid-mcp2221.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index a9fd7648515d..c97f0acbac8b 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -945,7 +945,7 @@ static int mcp2221_raw_event(struct hid_device *hdev,
switch (data[1]) {
case MCP2221_SUCCESS:
if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
- (mcp->gp_idx > 0 &&data[mcp->gp_idx - 1] == MCP2221_ALT_F_NOT_GPIOV)) {
+ (mcp->gp_idx > 0 && data[mcp->gp_idx - 1] == MCP2221_ALT_F_NOT_GPIOV)) {
mcp->status = -ENOENT;
} else {
mcp->status = 0;
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox