[PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Antheas Kapenekakis]

This is a two part series which does the following:
  - Clean-up init sequence
  - Unify backlight handling to happen under asus-wmi so that all Aura
    devices have synced brightness controls and the backlight button works
    properly when it is on a USB laptop keyboard instead of one w/ WMI.

For more context, see cover letter of V1. Since V5, I removed some patches
to make this easier to merge.

---
V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/

Changes since V9:
  - No functional changes
  - Rebase to review-ilpo-next
  - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
    "remove unused keyboard backlight quirk" + imports
    Dismiss Luke's review as this patch diverged
  - Reword paragraph in "Add support for multiple kbd led handlers" to be
    more verbose
  - Use kfree in fortify patch
  - Fix minor style quirks from --nonstict checkpatch run

Changes since V8:
  - No functional changes
  - Move legacy init patch to second, modify first patch so that their
    diff is minimized
  - Split "prevent binding to all HID devices on ROG" into two patches:
    - moving backlight initialization into probe
    - early exit to skip ->init check and rename
    - Remove skipping vendor fixups for non-vendor devices. It is not possible
      to read usages before the report fixups are applied, so it did not work
  - In that patch, reword a comment to be single line and make is_vendor a bool
  - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
    has drifted too far since he reviewed/tested it.

Changes since V7:
  - Readd legacy init quirk for Dennis
  - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
  - Fix warning due to enum_backlight receiving negative values

Changes since V6:
  - Split initialization refactor into three patches, update commit text
    to be clearer in what it does
  - Replace spinlock accesses with guard and scoped guard in all patches
  - Add missing includes mentioned by Ilpo
  - Reflow, tweak comment in prevent binding to all HID devices on ROG
  - Replace asus_ref.asus with local reference in all patches
  - Add missing kernel doc comments
  - Other minor nits from Ilpo
  - User reported warning due to scheduling work while holding a spinlock.
    Restructure patch for multiple handlers to limit when spinlock is held to
    variable access only. In parallel, setup a workqueue to handle registration
    of led device and setting brightness. This is required as registering the
    led device triggers kbd_led_get which needs to hold the spinlock to
    protect the led_wk value. The workqueue is also required for the hid
    event passthrough to avoid scheduling work while holding the spinlock.
    Apply the workqueue to wmi brightness buttons as well, as that was
    omitted before this series and WMI access was performed.
  - On "HID: asus: prevent binding to all HID devices on ROG", rename
    quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
    This allows other quirks to apply (applies quirk that fixes keyboard
    being named as a pointer device).

Changes since V5:
  - It's been a long time
  - Remove addition of RGB as that had some comments I need to work on
  - Remove folio patch (already merged)
  - Remove legacy fix patch 11 from V4. There is a small chance that
    without this patch, some old NKEY keyboards might not respond to
    RGB commands according to Luke, but the kernel driver does not do
    RGB currently. The 0x5d init is done by Armoury crate software in
    Windows. If an issue is found, we can re-add it or just remove patches
    1/2 before merging. However, init could use the cleanup.

Changes since V4:
  - Fix KConfig (reported by kernel robot)
  - Fix Ilpo's nits, if I missed anything lmk

Changes since V3:
  - Add initializer for 0x5d for old NKEY keyboards until it is verified
    that it is not needed for their media keys to function.
  - Cover init in asus-wmi with spinlock as per Hans
  - If asus-wmi registers WMI handler with brightness, init the brightness
    in USB Asus keyboards, per Hans.
  - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
  - Fix oops when unregistering asus-wmi by moving unregister outside of
    the spin lock (but after the asus reference is set to null)

Changes since V2:
  - Check lazy init succeds in asus-wmi before setting register variable
  - make explicit check in asus_hid_register_listener for listener existing
    to avoid re-init
  - rename asus_brt to asus_hid in most places and harmonize everything
  - switch to a spinlock instead of a mutex to avoid kernel ooops
  - fixup hid device quirks to avoid multiple RGB devices while still exposing
    all input vendor devices. This includes moving rgb init to probe
    instead of the input_configured callbacks.
  - Remove fan key (during retest it appears to be 0xae that is already
    supported by hid-asus)
  - Never unregister asus::kbd_backlight while asus-wmi is active, as that
  - removes fds from userspace and breaks backlight functionality. All
  - current mainline drivers do not support backlight hotplugging, so most
    userspace software (e.g., KDE, UPower) is built with that assumption.
    For the Ally, since it disconnects its controller during sleep, this
    caused the backlight slider to not work in KDE.

Changes since V1:
  - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
  - Fix ifdef else having an invalid signature (reported by kernel robot)
  - Restore input arguments to init and keyboard function so they can
    be re-used for RGB controls.
  - Remove Z13 delay (it did not work to fix the touchpad) and replace it
    with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
    keyboard rename into it.
  - Unregister brightness listener before removing work queue to avoid
    a race condition causing corruption
  - Remove spurious mutex unlock in asus_brt_event
  - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
    relocking the mutex and causing a deadlock when unregistering leds
  - Add extra check during unregistering to avoid calling unregister when
    no led device is registered.
  - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
    the driver to create 4 RGB handlers per device. I also suspect some
    extra events sneak through (KDE had the @@@@@@).

Antheas Kapenekakis (11):
  HID: asus: simplify RGB init sequence
  HID: asus: initialize additional endpoints only for legacy devices
  HID: asus: use same report_id in response
  HID: asus: fortify keyboard handshake
  HID: asus: move vendor initialization to probe
  HID: asus: early return for ROG devices
  platform/x86: asus-wmi: Add support for multiple kbd led handlers
  HID: asus: listen to the asus-wmi brightness device instead of
    creating one
  platform/x86: asus-wmi: remove unused keyboard backlight quirk
  platform/x86: asus-wmi: add keyboard brightness event handler
  HID: asus: add support for the asus-wmi brightness handler

 drivers/hid/hid-asus.c                        | 205 ++++++++--------
 drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
 .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
 include/linux/platform_data/x86/asus-wmi.h    |  28 +++
 4 files changed, 322 insertions(+), 184 deletions(-)
 delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h


base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
-- 
2.52.0

[PATCH v10 01/11] HID: asus: simplify RGB init sequence

[Antheas Kapenekakis]

Currently, RGB initialization forks depending on whether a device is
NKEY. However, in reality both initialization forks are the same, other
than the NKEY initialization initializing the LED_REPORT_ID1,
LED_REPORT_ID2 endpoints, and the non-NKEY initialization having a
functionality check which is skipped for the NKEY path.

Therefore, merge the if blocks, gate the ID1/ID2 initializations
behind the NKEY quirk instead, and introduce the functionality check
for NKEY devices (it is supported by them).

There should be no functional change with this patch.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 52 ++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 472bca54642b..a82286a427b8 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -639,13 +639,20 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	unsigned char kbd_func;
 	int ret;
 
-	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
-		/* Initialize keyboard */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
-		if (ret < 0)
-			return ret;
+	ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
+	if (ret < 0)
+		return ret;
 
-		/* The LED endpoint is initialised in two HID */
+	/* Get keyboard functions */
+	ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	if (ret < 0)
+		return ret;
+
+	/* Check for backlight support */
+	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
+		return -ENODEV;
+
+	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
 		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
 		if (ret < 0)
 			return ret;
@@ -653,34 +660,19 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
 		if (ret < 0)
 			return ret;
+	}
 
-		if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
-			ret = asus_kbd_disable_oobe(hdev);
-			if (ret < 0)
-				return ret;
-		}
-
-		if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
-			intf = to_usb_interface(hdev->dev.parent);
-			udev = interface_to_usbdev(intf);
-			validate_mcu_fw_version(hdev,
-				le16_to_cpu(udev->descriptor.idProduct));
-		}
-
-	} else {
-		/* Initialize keyboard */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
-		if (ret < 0)
-			return ret;
-
-		/* Get keyboard functions */
-		ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
+		ret = asus_kbd_disable_oobe(hdev);
 		if (ret < 0)
 			return ret;
+	}
 
-		/* Check for backlight support */
-		if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
-			return -ENODEV;
+	if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+		intf = to_usb_interface(hdev->dev.parent);
+		udev = interface_to_usbdev(intf);
+		validate_mcu_fw_version(
+			hdev, le16_to_cpu(udev->descriptor.idProduct));
 	}
 
 	drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
-- 
2.52.0

[PATCH v10 02/11] HID: asus: initialize additional endpoints only for legacy devices

[Antheas Kapenekakis]

Currently, ID1/ID2 initializations are performed for all NKEY devices.
However, ID1 initializations are only required for RGB control and are
only supported for RGB capable devices. ID2 initializations are only
required for initializing the Anime display endpoint which is only
supported on devices with an Anime display. Both of these
initializations are out of scope for this driver (this is a brightness
control and keyboard shortcut driver) and they should not be performed
for devices that do not support them in any case.

At the same time, there are older NKEY devices that have only been
tested with these initializations in the kernel and it is not possible
to recheck them. There is a possibility that especially with the ID1
initialization, certain laptop models might have their shortcuts stop
working (currently unproven).

For an abundance of caution, only initialize ID1/ID2 for those older
NKEY devices by introducing a quirk for them and replacing the NKEY
quirk in the block that performs the inits with that.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a82286a427b8..9004814fb0d3 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
 #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
 #define QUIRK_ROG_ALLY_XPAD		BIT(13)
+#define QUIRK_ROG_NKEY_LEGACY		BIT(14)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -652,7 +653,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
 		return -ENODEV;
 
-	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
+	if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
 		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
 		if (ret < 0)
 			return ret;
@@ -1376,10 +1377,10 @@ static const struct hid_device_id asus_devices[] = {
 	  QUIRK_USE_KBD_BACKLIGHT },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
-- 
2.52.0

[PATCH v10 03/11] HID: asus: use same report_id in response

[Antheas Kapenekakis]

Currently, asus_kbd_get_functions prods the device using feature
report report_id, but then is hardcoded to check the response through
FEATURE_KBD_REPORT_ID. This only works if report_id is that value
(currently true). So, use report_id in the response as well to
maintain functionality if that value changes in the future.

Reviewed-by: Denis Benato <benato.denis96@gmail.com>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 9004814fb0d3..c1a36a8beb35 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -424,7 +424,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
 	if (!readbuf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
+	ret = hid_hw_raw_request(hdev, report_id, readbuf,
 				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
 				 HID_REQ_GET_REPORT);
 	if (ret < 0) {
-- 
2.52.0

[PATCH v10 04/11] HID: asus: fortify keyboard handshake

[Antheas Kapenekakis]

Handshaking with an Asus device involves sending it a feature report
with the string "ASUS Tech.Inc." and then reading it back to verify the
handshake was successful, under the feature ID the interaction will
take place.

Currently, the driver only does the first part. Add the readback to
verify the handshake was successful. As this could cause breakages,
allow the verification to fail with a dmesg error until we verify
all devices work with it (they seem to).

Since the response is more than 16 bytes, increase the buffer size
to 64 as well to avoid overflow errors.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index c1a36a8beb35..2a412e10f916 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_REPORT_ID 0x0d
 #define INPUT_REPORT_ID 0x5d
 #define FEATURE_KBD_REPORT_ID 0x5a
-#define FEATURE_KBD_REPORT_SIZE 16
+#define FEATURE_KBD_REPORT_SIZE 64
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
@@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
 
 static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
 {
+	/*
+	 * The handshake is first sent as a set_report, then retrieved
+	 * from a get_report. They should be equal.
+	 */
 	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
 		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
 	int ret;
 
 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
-	if (ret < 0)
-		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
+	if (ret < 0) {
+		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
+		return ret;
+	}
+
+	u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
+	if (!readbuf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, report_id, readbuf,
+				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
+				 HID_REQ_GET_REPORT);
+	if (ret < 0) {
+		hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
+	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
+		hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
+			 FEATURE_KBD_REPORT_SIZE, readbuf);
+		/*
+		 * Do not return error if handshake is wrong until this is
+		 * verified to work for all devices.
+		 */
+	}
 
 	return ret;
 }
-- 
2.52.0

[PATCH v10 05/11] HID: asus: move vendor initialization to probe

[Antheas Kapenekakis]

ROG NKEY devices have multiple HID endpoints, around 3-4. One of those
endpoints has a usage page of 0xff31, and is the one that emits keyboard
shortcuts and controls RGB/backlight. Currently, this driver places
the usage page check under asus_input_mapping and then inits backlight
in asus_input_configured which is unnecessarily complicated and prevents
probe from performing customizations on the vendor endpoint.

Simplify the logic by introducing an is_vendor variable into probe that
checks for usage page 0xff31. Then, use this variable to move backlight
initialization into probe instead of asus_input_configured, and remove
the backlight check from asus_input_mapping.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 2a412e10f916..faac971794c0 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -48,6 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define T100CHI_MOUSE_REPORT_ID 0x06
 #define FEATURE_REPORT_ID 0x0d
 #define INPUT_REPORT_ID 0x5d
+#define HID_USAGE_PAGE_VENDOR 0xff310000
 #define FEATURE_KBD_REPORT_ID 0x5a
 #define FEATURE_KBD_REPORT_SIZE 64
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
@@ -127,7 +128,6 @@ struct asus_drvdata {
 	struct input_dev *tp_kbd_input;
 	struct asus_kbd_leds *kbd_backlight;
 	const struct asus_touchpad_info *tp;
-	bool enable_backlight;
 	struct power_supply *battery;
 	struct power_supply_desc battery_desc;
 	int battery_capacity;
@@ -318,7 +318,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
 static int asus_event(struct hid_device *hdev, struct hid_field *field,
 		      struct hid_usage *usage, __s32 value)
 {
-	if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
+	if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
 	    (usage->hid & HID_USAGE) != 0x00 &&
 	    (usage->hid & HID_USAGE) != 0xff && !usage->type) {
 		hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
@@ -941,11 +941,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 
 	drvdata->input = input;
 
-	if (drvdata->enable_backlight &&
-	    !asus_kbd_wmi_led_control_present(hdev) &&
-	    asus_kbd_register_leds(hdev))
-		hid_warn(hdev, "Failed to initialize backlight.\n");
-
 	return 0;
 }
 
@@ -1018,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
 			return -1;
 		}
 
-		/*
-		 * Check and enable backlight only on devices with UsagePage ==
-		 * 0xff31 to avoid initializing the keyboard firmware multiple
-		 * times on devices with multiple HID descriptors but same
-		 * PID/VID.
-		 */
-		if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
-			drvdata->enable_backlight = true;
-
 		set_bit(EV_REP, hi->input->evbit);
 		return 1;
 	}
@@ -1143,8 +1129,11 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
 
 static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-	int ret;
+	struct hid_report_enum *rep_enum;
 	struct asus_drvdata *drvdata;
+	struct hid_report *rep;
+	bool is_vendor = false;
+	int ret;
 
 	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
 	if (drvdata == NULL) {
@@ -1228,12 +1217,24 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
+	/* Check for vendor for RGB init and handle generic devices properly. */
+	rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
+	list_for_each_entry(rep, &rep_enum->report_list, list) {
+		if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
+			is_vendor = true;
+	}
+
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret) {
 		hid_err(hdev, "Asus hw start failed: %d\n", ret);
 		return ret;
 	}
 
+	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
+	    !asus_kbd_wmi_led_control_present(hdev) &&
+	    asus_kbd_register_leds(hdev))
+		hid_warn(hdev, "Failed to initialize backlight.\n");
+
 	/*
 	 * Check that input registration succeeded. Checking that
 	 * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
-- 
2.52.0

[PATCH v10 06/11] HID: asus: early return for ROG devices

[Antheas Kapenekakis]

Some ROG devices have a new dynamic backlight interface for control by
Windows. This interface does not create an ->input device, causing the
kernel to print an error message and to eject it. In addition, ROG
devices have proper HID names in their descriptors so renaming them is
not necessary.

Therefore, if a device is identified as ROG, early return from probe to
skip renaming and ->input checks.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index faac971794c0..461b0f8efdfb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1235,6 +1235,13 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	    asus_kbd_register_leds(hdev))
 		hid_warn(hdev, "Failed to initialize backlight.\n");
 
+	/*
+	 * For ROG keyboards, skip rename for consistency and ->input check as
+	 * some devices do not have inputs.
+	 */
+	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD)
+		return 0;
+
 	/*
 	 * Check that input registration succeeded. Checking that
 	 * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
-- 
2.52.0

[PATCH v10 07/11] platform/x86: asus-wmi: Add support for multiple kbd led handlers

[Antheas Kapenekakis]

Some devices, such as the Z13 have multiple Aura devices connected
to them by USB. In addition, they might have a WMI interface for
RGB. In Windows, Armoury Crate exposes a unified brightness slider
for all of them, with 3 brightness levels.

Therefore, to be synergistic in Linux, and support existing tooling
such as UPower, allow adding listeners to the RGB device of the WMI
interface. If WMI does not exist, lazy initialize the interface.

Since hid-asus and asus-wmi can both interact with the led objects
including from an atomic context, protect the brightness access with a
spinlock and update the values from a workqueue. Use this workqueue to
also process WMI keyboard events, so they are handled asynchronously.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/asus-wmi.c            | 183 ++++++++++++++++++---
 include/linux/platform_data/x86/asus-wmi.h |  15 ++
 2 files changed, 173 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 64cfc0bf98dd..84cde34ab6a8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -31,13 +31,13 @@
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
 #include <linux/platform_data/x86/asus-wmi.h>
-#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
 #include <linux/power_supply.h>
 #include <linux/rfkill.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/units.h>
 
@@ -256,6 +256,9 @@ struct asus_wmi {
 	int tpd_led_wk;
 	struct led_classdev kbd_led;
 	int kbd_led_wk;
+	bool kbd_led_notify;
+	bool kbd_led_avail;
+	bool kbd_led_registered;
 	struct led_classdev lightbar_led;
 	int lightbar_led_wk;
 	struct led_classdev micmute_led;
@@ -264,6 +267,7 @@ struct asus_wmi {
 	struct work_struct tpd_led_work;
 	struct work_struct wlan_led_work;
 	struct work_struct lightbar_led_work;
+	struct work_struct kbd_led_work;
 
 	struct asus_rfkill wlan;
 	struct asus_rfkill bluetooth;
@@ -1615,6 +1619,106 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
 
 /* LEDs ***********************************************************************/
 
+struct asus_hid_ref {
+	struct list_head listeners;
+	struct asus_wmi *asus;
+	/* Protects concurrent access from hid-asus and asus-wmi to leds */
+	spinlock_t lock;
+};
+
+static struct asus_hid_ref asus_ref = {
+	.listeners = LIST_HEAD_INIT(asus_ref.listeners),
+	.asus = NULL,
+	/*
+	 * Protects .asus, .asus.kbd_led_{wk,notify}, and .listener refs. Other
+	 * asus variables are read-only after .asus is set.
+	 *
+	 * The led cdev device is not protected because it calls backlight_get
+	 * during initialization, which would result in a nested lock attempt.
+	 *
+	 * The led cdev is safe to access without a lock because if
+	 * kbd_led_avail is true it is initialized before .asus is set and never
+	 * changed until .asus is dropped. If kbd_led_avail is false, the led
+	 * cdev is registered by the workqueue, which is single-threaded and
+	 * cancelled before asus-wmi would access the led cdev to unregister it.
+	 *
+	 * A spinlock is used, because the protected variables can be accessed
+	 * from an IRQ context from asus-hid.
+	 */
+	.lock = __SPIN_LOCK_UNLOCKED(asus_ref.lock),
+};
+
+/*
+ * Allows registering hid-asus listeners that want to be notified of
+ * keyboard backlight changes.
+ */
+int asus_hid_register_listener(struct asus_hid_listener *bdev)
+{
+	struct asus_wmi *asus;
+
+	guard(spinlock_irqsave)(&asus_ref.lock);
+	list_add_tail(&bdev->list, &asus_ref.listeners);
+	asus = asus_ref.asus;
+	if (asus)
+		queue_work(asus->led_workqueue, &asus->kbd_led_work);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asus_hid_register_listener);
+
+/*
+ * Allows unregistering hid-asus listeners that were added with
+ * asus_hid_register_listener().
+ */
+void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
+{
+	guard(spinlock_irqsave)(&asus_ref.lock);
+	list_del(&bdev->list);
+}
+EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
+
+static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
+
+static void kbd_led_update_all(struct work_struct *work)
+{
+	struct asus_wmi *asus;
+	bool registered, notify;
+	int ret, value;
+
+	asus = container_of(work, struct asus_wmi, kbd_led_work);
+
+	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+		registered = asus->kbd_led_registered;
+		value = asus->kbd_led_wk;
+		notify = asus->kbd_led_notify;
+	}
+
+	if (!registered) {
+		/*
+		 * This workqueue runs under asus-wmi, which means probe has
+		 * completed and asus-wmi will keep running until it finishes.
+		 * Therefore, we can safely register the LED without holding
+		 * a spinlock.
+		 */
+		ret = devm_led_classdev_register(&asus->platform_device->dev,
+						 &asus->kbd_led);
+		if (!ret) {
+			scoped_guard(spinlock_irqsave, &asus_ref.lock)
+				asus->kbd_led_registered = true;
+		} else {
+			pr_warn("Failed to register keyboard backlight LED: %d\n", ret);
+			return;
+		}
+	}
+
+	if (value >= 0)
+		do_kbd_led_set(&asus->kbd_led, value);
+	if (notify) {
+		scoped_guard(spinlock_irqsave, &asus_ref.lock)
+			asus->kbd_led_notify = false;
+		led_classdev_notify_brightness_hw_changed(&asus->kbd_led, value);
+	}
+}
+
 /*
  * These functions actually update the LED's, and are called from a
  * workqueue. By doing this as separate work rather than when the LED
@@ -1661,7 +1765,8 @@ static void kbd_led_update(struct asus_wmi *asus)
 {
 	int ctrl_param = 0;
 
-	ctrl_param = 0x80 | (asus->kbd_led_wk & 0x7F);
+	scoped_guard(spinlock_irqsave, &asus_ref.lock)
+		ctrl_param = 0x80 | (asus->kbd_led_wk & 0x7F);
 	asus_wmi_set_devstate(ASUS_WMI_DEVID_KBD_BACKLIGHT, ctrl_param, NULL);
 }
 
@@ -1694,14 +1799,23 @@ static int kbd_led_read(struct asus_wmi *asus, int *level, int *env)
 
 static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
 {
+	struct asus_hid_listener *listener;
 	struct asus_wmi *asus;
 	int max_level;
 
 	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
 	max_level = asus->kbd_led.max_brightness;
 
-	asus->kbd_led_wk = clamp_val(value, 0, max_level);
-	kbd_led_update(asus);
+	scoped_guard(spinlock_irqsave, &asus_ref.lock)
+		asus->kbd_led_wk = clamp_val(value, 0, max_level);
+
+	if (asus->kbd_led_avail)
+		kbd_led_update(asus);
+
+	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+		list_for_each_entry(listener, &asus_ref.listeners, list)
+			listener->brightness_set(listener, asus->kbd_led_wk);
+	}
 }
 
 static void kbd_led_set(struct led_classdev *led_cdev,
@@ -1716,10 +1830,11 @@ static void kbd_led_set(struct led_classdev *led_cdev,
 
 static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
 {
-	struct led_classdev *led_cdev = &asus->kbd_led;
-
-	do_kbd_led_set(led_cdev, value);
-	led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
+	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+		asus->kbd_led_wk = value;
+		asus->kbd_led_notify = true;
+	}
+	queue_work(asus->led_workqueue, &asus->kbd_led_work);
 }
 
 static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
@@ -1729,10 +1844,18 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
 
 	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
 
+	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+		if (!asus->kbd_led_avail)
+			return asus->kbd_led_wk;
+	}
+
 	retval = kbd_led_read(asus, &value, NULL);
 	if (retval < 0)
 		return retval;
 
+	scoped_guard(spinlock_irqsave, &asus_ref.lock)
+		asus->kbd_led_wk = value;
+
 	return value;
 }
 
@@ -1844,7 +1967,9 @@ static int camera_led_set(struct led_classdev *led_cdev,
 
 static void asus_wmi_led_exit(struct asus_wmi *asus)
 {
-	led_classdev_unregister(&asus->kbd_led);
+	scoped_guard(spinlock_irqsave, &asus_ref.lock)
+		asus_ref.asus = NULL;
+
 	led_classdev_unregister(&asus->tpd_led);
 	led_classdev_unregister(&asus->wlan_led);
 	led_classdev_unregister(&asus->lightbar_led);
@@ -1882,22 +2007,26 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 			goto error;
 	}
 
-	if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
-		pr_info("using asus-wmi for asus::kbd_backlight\n");
-		asus->kbd_led_wk = led_val;
-		asus->kbd_led.name = "asus::kbd_backlight";
-		asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
-		asus->kbd_led.brightness_set = kbd_led_set;
-		asus->kbd_led.brightness_get = kbd_led_get;
-		asus->kbd_led.max_brightness = 3;
+	asus->kbd_led.name = "asus::kbd_backlight";
+	asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
+	asus->kbd_led.brightness_set = kbd_led_set;
+	asus->kbd_led.brightness_get = kbd_led_get;
+	asus->kbd_led.max_brightness = 3;
+	asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
+	INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
 
+	if (asus->kbd_led_avail) {
+		asus->kbd_led_wk = led_val;
 		if (num_rgb_groups != 0)
 			asus->kbd_led.groups = kbd_rgb_mode_groups;
+	} else {
+		asus->kbd_led_wk = -1;
+	}
 
-		rv = led_classdev_register(&asus->platform_device->dev,
-					   &asus->kbd_led);
-		if (rv)
-			goto error;
+	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+		asus_ref.asus = asus;
+		if (asus->kbd_led_avail || !list_empty(&asus_ref.listeners))
+			queue_work(asus->led_workqueue, &asus->kbd_led_work);
 	}
 
 	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
@@ -4372,6 +4501,7 @@ static int asus_wmi_get_event_code(union acpi_object *obj)
 
 static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 {
+	enum led_brightness led_value;
 	unsigned int key_value = 1;
 	bool autorelease = 1;
 
@@ -4388,19 +4518,22 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	scoped_guard(spinlock_irqsave, &asus_ref.lock)
+		led_value = asus->kbd_led_wk;
+
 	if (code == NOTIFY_KBD_BRTUP) {
-		kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
+		kbd_led_set_by_kbd(asus, led_value + 1);
 		return;
 	}
 	if (code == NOTIFY_KBD_BRTDWN) {
-		kbd_led_set_by_kbd(asus, asus->kbd_led_wk - 1);
+		kbd_led_set_by_kbd(asus, led_value - 1);
 		return;
 	}
 	if (code == NOTIFY_KBD_BRTTOGGLE) {
-		if (asus->kbd_led_wk == asus->kbd_led.max_brightness)
+		if (led_value == asus->kbd_led.max_brightness)
 			kbd_led_set_by_kbd(asus, 0);
 		else
-			kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
+			kbd_led_set_by_kbd(asus, led_value + 1);
 		return;
 	}
 
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 419491d4abca..d347cffd05d5 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -172,12 +172,20 @@ enum asus_ally_mcu_hack {
 	ASUS_WMI_ALLY_MCU_HACK_DISABLED,
 };
 
+/* Used to notify hid-asus when asus-wmi changes keyboard backlight */
+struct asus_hid_listener {
+	struct list_head list;
+	void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
+};
+
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
 void set_ally_mcu_powersave(bool enabled);
 int asus_wmi_get_devstate_dsts(u32 dev_id, u32 *retval);
 int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
+int asus_hid_register_listener(struct asus_hid_listener *cdev);
+void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
 #else
 static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
 {
@@ -198,6 +206,13 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
 {
 	return -ENODEV;
 }
+static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
+{
+	return -ENODEV;
+}
+static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
+{
+}
 #endif
 
 #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
-- 
2.52.0

[PATCH v10 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one

[Antheas Kapenekakis]

Some ROG laptops expose multiple interfaces for controlling the
keyboard/RGB brightness. This creates a name conflict under
asus::kbd_brightness, where the second device ends up being
named asus::kbd_brightness_1 and they are both broken.

Therefore, register a listener to the asus-wmi brightness device
instead of creating a new one.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 65 +++++++-----------------------------------
 1 file changed, 10 insertions(+), 55 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 461b0f8efdfb..475e34187326 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -27,7 +27,6 @@
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/platform_data/x86/asus-wmi.h>
-#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
 #include <linux/input/mt.h>
 #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
 #include <linux/power_supply.h>
@@ -103,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
 
 struct asus_kbd_leds {
-	struct led_classdev cdev;
+	struct asus_hid_listener listener;
 	struct hid_device *hdev;
 	struct work_struct work;
 	unsigned int brightness;
@@ -493,11 +492,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
 	spin_unlock_irqrestore(&led->lock, flags);
 }
 
-static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
-				   enum led_brightness brightness)
+static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
+				   int brightness)
 {
-	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
-						 cdev);
+	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
+						 listener);
 	unsigned long flags;
 
 	spin_lock_irqsave(&led->lock, flags);
@@ -507,20 +506,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
 	asus_schedule_work(led);
 }
 
-static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
-{
-	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
-						 cdev);
-	enum led_brightness brightness;
-	unsigned long flags;
-
-	spin_lock_irqsave(&led->lock, flags);
-	brightness = led->brightness;
-	spin_unlock_irqrestore(&led->lock, flags);
-
-	return brightness;
-}
-
 static void asus_kbd_backlight_work(struct work_struct *work)
 {
 	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
@@ -537,34 +522,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
 		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
 }
 
-/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
- * precedence. We only activate HID-based backlight control when the
- * WMI control is not available.
- */
-static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
-{
-	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
-	u32 value;
-	int ret;
-
-	if (!IS_ENABLED(CONFIG_ASUS_WMI))
-		return false;
-
-	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
-			dmi_check_system(asus_use_hid_led_dmi_ids)) {
-		hid_info(hdev, "using HID for asus::kbd_backlight\n");
-		return false;
-	}
-
-	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
-				       ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
-	hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
-	if (ret)
-		return false;
-
-	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
-}
-
 /*
  * We don't care about any other part of the string except the version section.
  * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
@@ -709,14 +666,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	drvdata->kbd_backlight->removed = false;
 	drvdata->kbd_backlight->brightness = 0;
 	drvdata->kbd_backlight->hdev = hdev;
-	drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
-	drvdata->kbd_backlight->cdev.max_brightness = 3;
-	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
-	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
+	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
 	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
 	spin_lock_init(&drvdata->kbd_backlight->lock);
 
-	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
+	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
 	if (ret < 0) {
 		/* No need to have this still around */
 		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
@@ -1105,7 +1059,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
 
 	if (drvdata->kbd_backlight) {
 		const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
-				drvdata->kbd_backlight->cdev.brightness };
+				drvdata->kbd_backlight->brightness };
 		ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
 		if (ret < 0) {
 			hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
@@ -1231,7 +1185,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
-	    !asus_kbd_wmi_led_control_present(hdev) &&
 	    asus_kbd_register_leds(hdev))
 		hid_warn(hdev, "Failed to initialize backlight.\n");
 
@@ -1278,6 +1231,8 @@ static void asus_remove(struct hid_device *hdev)
 	unsigned long flags;
 
 	if (drvdata->kbd_backlight) {
+		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
+
 		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
 		drvdata->kbd_backlight->removed = true;
 		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
-- 
2.52.0

[PATCH v10 09/11] platform/x86: asus-wmi: remove unused keyboard backlight quirk

[Antheas Kapenekakis]

The quirk for selecting whether keyboard backlight should be controlled
by HID or WMI is not needed anymore, so remove the file containing it.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 .../platform_data/x86/asus-wmi-leds-ids.h     | 50 -------------------
 1 file changed, 50 deletions(-)
 delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h

diff --git a/include/linux/platform_data/x86/asus-wmi-leds-ids.h b/include/linux/platform_data/x86/asus-wmi-leds-ids.h
deleted file mode 100644
index 034a039c4e37..000000000000
--- a/include/linux/platform_data/x86/asus-wmi-leds-ids.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
-#define __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
-
-#include <linux/dmi.h>
-#include <linux/types.h>
-
-/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-#if IS_REACHABLE(CONFIG_ASUS_WMI) || IS_REACHABLE(CONFIG_HID_ASUS)
-static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
-		},
-	},
-	{ },
-};
-#endif
-
-#endif	/* __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H */
-- 
2.52.0

[PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Antheas Kapenekakis]

The keyboard brightness control of Asus WMI keyboards is handled in
kernel, which leads to the shortcut going from brightness 0, to 1,
to 2, and 3.

However, for HID keyboards it is exposed as a key and handled by the
user's desktop environment. For the toggle button, this means that
brightness control becomes on/off. In addition, in the absence of a
DE, the keyboard brightness does not work.

Therefore, expose an event handler for the keyboard brightness control
which can then be used by hid-asus. Since this handler is called from
an interrupt context, defer the actual work to a workqueue.

In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
for maximum brightness since it is shared between hid-asus/asus-wmi.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
 include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 84cde34ab6a8..a69464e45ca4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
 	}
 }
 
+/*
+ * This function is called from hid-asus to inform asus-wmi of brightness
+ * changes initiated by the keyboard backlight keys.
+ */
+int asus_hid_event(enum asus_hid_event event)
+{
+	struct asus_wmi *asus;
+	int brightness;
+
+	guard(spinlock_irqsave)(&asus_ref.lock);
+	asus = asus_ref.asus;
+	if (!asus || !asus->kbd_led_registered)
+		return -EBUSY;
+
+	brightness = asus->kbd_led_wk;
+
+	switch (event) {
+	case ASUS_EV_BRTUP:
+		brightness += 1;
+		break;
+	case ASUS_EV_BRTDOWN:
+		brightness -= 1;
+		break;
+	case ASUS_EV_BRTTOGGLE:
+		if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
+			brightness = 0;
+		else
+			brightness += 1;
+		break;
+	}
+
+	asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
+	asus->kbd_led_notify = true;
+	queue_work(asus->led_workqueue, &asus->kbd_led_work);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asus_hid_event);
+
 /*
  * These functions actually update the LED's, and are called from a
  * workqueue. By doing this as separate work rather than when the LED
@@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
 {
 	struct asus_hid_listener *listener;
 	struct asus_wmi *asus;
-	int max_level;
 
 	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
-	max_level = asus->kbd_led.max_brightness;
 
 	scoped_guard(spinlock_irqsave, &asus_ref.lock)
-		asus->kbd_led_wk = clamp_val(value, 0, max_level);
+		asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
 
 	if (asus->kbd_led_avail)
 		kbd_led_update(asus);
@@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
 	asus->kbd_led.brightness_set = kbd_led_set;
 	asus->kbd_led.brightness_get = kbd_led_get;
-	asus->kbd_led.max_brightness = 3;
+	asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
 	asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
 	INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
 
@@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 	if (code == NOTIFY_KBD_BRTTOGGLE) {
-		if (led_value == asus->kbd_led.max_brightness)
+		if (led_value == ASUS_EV_MAX_BRIGHTNESS)
 			kbd_led_set_by_kbd(asus, 0);
 		else
 			kbd_led_set_by_kbd(asus, led_value + 1);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index d347cffd05d5..7b872b5d0960 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -178,6 +178,14 @@ struct asus_hid_listener {
 	void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
 };
 
+enum asus_hid_event {
+	ASUS_EV_BRTUP,
+	ASUS_EV_BRTDOWN,
+	ASUS_EV_BRTTOGGLE,
+};
+
+#define ASUS_EV_MAX_BRIGHTNESS 3
+
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
 void set_ally_mcu_powersave(bool enabled);
@@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
 int asus_hid_register_listener(struct asus_hid_listener *cdev);
 void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
+int asus_hid_event(enum asus_hid_event event);
 #else
 static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
 {
@@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
 static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
 {
 }
+static inline int asus_hid_event(enum asus_hid_event event)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
-- 
2.52.0

[PATCH v10 11/11] HID: asus: add support for the asus-wmi brightness handler

[Antheas Kapenekakis]

If the asus-wmi brightness handler is available, send the
keyboard brightness events to it instead of passing them
to userspace. If it is not, fall back to sending them to it.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 475e34187326..8f30d1b9737e 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -324,6 +324,17 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
 			 usage->hid & HID_USAGE);
 	}
 
+	if (usage->type == EV_KEY && value) {
+		switch (usage->code) {
+		case KEY_KBDILLUMUP:
+			return !asus_hid_event(ASUS_EV_BRTUP);
+		case KEY_KBDILLUMDOWN:
+			return !asus_hid_event(ASUS_EV_BRTDOWN);
+		case KEY_KBDILLUMTOGGLE:
+			return !asus_hid_event(ASUS_EV_BRTTOGGLE);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.52.0

Re: [PATCH v10 04/11] HID: asus: fortify keyboard handshake

[Denis Benato]

On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Handshaking with an Asus device involves sending it a feature report
> with the string "ASUS Tech.Inc." and then reading it back to verify the
> handshake was successful, under the feature ID the interaction will
> take place.
>
> Currently, the driver only does the first part. Add the readback to
> verify the handshake was successful. As this could cause breakages,
> allow the verification to fail with a dmesg error until we verify
> all devices work with it (they seem to).
>
> Since the response is more than 16 bytes, increase the buffer size
> to 64 as well to avoid overflow errors.
>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> ---
>  drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a36a8beb35..2a412e10f916 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define FEATURE_REPORT_ID 0x0d
>  #define INPUT_REPORT_ID 0x5d
>  #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
>  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>  #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>  
> @@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>  
>  static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>  {
> +	/*
> +	 * The handshake is first sent as a set_report, then retrieved
> +	 * from a get_report. They should be equal.
> +	 */
>  	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
>  		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>  	int ret;
>  
>  	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> -	if (ret < 0)
> -		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> +		return ret;
> +	}
> +
> +	u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> +	if (!readbuf)
> +		return -ENOMEM;
> +
> +	ret = hid_hw_raw_request(hdev, report_id, readbuf,
> +				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> +				 HID_REQ_GET_REPORT);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> +	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> +		hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> +			 FEATURE_KBD_REPORT_SIZE, readbuf);
> +		/*
> +		 * Do not return error if handshake is wrong until this is
> +		 * verified to work for all devices.
> +		 */
> +	}
>  
>  	return ret;
>  }

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Antheas Kapenekakis]

On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> 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.

Slight bump on this. It addresses both of the remarks Denis made in
the previous version.

I begrudgingly argued a bit for those because I did not want to resend
the series and they were not functional changes, so sorry about that.
But they are fixed in this version incl. with the conflict with the
armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
Add support for multiple kbd led handlers" even though I addressed
your comment, so you may want to add that.

As for "HID: asus: early return for ROG devices" changing the name of
the devices of this driver, I will veto backporting it if it happens,
so inputplumber will have the two full months to remove the name
match. This is not a breaking change in the sense that software cannot
be made to work on both previous and latter versions and there is no
other software to my knowledge relying on name matches for Asus
keyboards. Moreover, an early exit is needed to prevent ejecting HID
endpoints without an ->input parameter so it is a needed fix anyway.
Postponing it will prevent Xbox Ally users from having RGB control
through userspace on a stock kernel but it is also not worth arguing
about

It is also fine for me for this series to merge for 6.20, but I'd
rather we handle it now since there will be some turbulence for asus
users due to armoury merging so it makes sense to have this transition
once.

Antheas

> ---
> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>
> Changes since V9:
>   - No functional changes
>   - Rebase to review-ilpo-next
>   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
>     "remove unused keyboard backlight quirk" + imports
>     Dismiss Luke's review as this patch diverged
>   - Reword paragraph in "Add support for multiple kbd led handlers" to be
>     more verbose
>   - Use kfree in fortify patch
>   - Fix minor style quirks from --nonstict checkpatch run
>
> Changes since V8:
>   - No functional changes
>   - Move legacy init patch to second, modify first patch so that their
>     diff is minimized
>   - Split "prevent binding to all HID devices on ROG" into two patches:
>     - moving backlight initialization into probe
>     - early exit to skip ->init check and rename
>     - Remove skipping vendor fixups for non-vendor devices. It is not possible
>       to read usages before the report fixups are applied, so it did not work
>   - In that patch, reword a comment to be single line and make is_vendor a bool
>   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
>     has drifted too far since he reviewed/tested it.
>
> Changes since V7:
>   - Readd legacy init quirk for Dennis
>   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
>   - Fix warning due to enum_backlight receiving negative values
>
> Changes since V6:
>   - Split initialization refactor into three patches, update commit text
>     to be clearer in what it does
>   - Replace spinlock accesses with guard and scoped guard in all patches
>   - Add missing includes mentioned by Ilpo
>   - Reflow, tweak comment in prevent binding to all HID devices on ROG
>   - Replace asus_ref.asus with local reference in all patches
>   - Add missing kernel doc comments
>   - Other minor nits from Ilpo
>   - User reported warning due to scheduling work while holding a spinlock.
>     Restructure patch for multiple handlers to limit when spinlock is held to
>     variable access only. In parallel, setup a workqueue to handle registration
>     of led device and setting brightness. This is required as registering the
>     led device triggers kbd_led_get which needs to hold the spinlock to
>     protect the led_wk value. The workqueue is also required for the hid
>     event passthrough to avoid scheduling work while holding the spinlock.
>     Apply the workqueue to wmi brightness buttons as well, as that was
>     omitted before this series and WMI access was performed.
>   - On "HID: asus: prevent binding to all HID devices on ROG", rename
>     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
>     This allows other quirks to apply (applies quirk that fixes keyboard
>     being named as a pointer device).
>
> Changes since V5:
>   - It's been a long time
>   - Remove addition of RGB as that had some comments I need to work on
>   - Remove folio patch (already merged)
>   - Remove legacy fix patch 11 from V4. There is a small chance that
>     without this patch, some old NKEY keyboards might not respond to
>     RGB commands according to Luke, but the kernel driver does not do
>     RGB currently. The 0x5d init is done by Armoury crate software in
>     Windows. If an issue is found, we can re-add it or just remove patches
>     1/2 before merging. However, init could use the cleanup.
>
> Changes since V4:
>   - Fix KConfig (reported by kernel robot)
>   - Fix Ilpo's nits, if I missed anything lmk
>
> Changes since V3:
>   - Add initializer for 0x5d for old NKEY keyboards until it is verified
>     that it is not needed for their media keys to function.
>   - Cover init in asus-wmi with spinlock as per Hans
>   - If asus-wmi registers WMI handler with brightness, init the brightness
>     in USB Asus keyboards, per Hans.
>   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
>   - Fix oops when unregistering asus-wmi by moving unregister outside of
>     the spin lock (but after the asus reference is set to null)
>
> Changes since V2:
>   - Check lazy init succeds in asus-wmi before setting register variable
>   - make explicit check in asus_hid_register_listener for listener existing
>     to avoid re-init
>   - rename asus_brt to asus_hid in most places and harmonize everything
>   - switch to a spinlock instead of a mutex to avoid kernel ooops
>   - fixup hid device quirks to avoid multiple RGB devices while still exposing
>     all input vendor devices. This includes moving rgb init to probe
>     instead of the input_configured callbacks.
>   - Remove fan key (during retest it appears to be 0xae that is already
>     supported by hid-asus)
>   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
>   - removes fds from userspace and breaks backlight functionality. All
>   - current mainline drivers do not support backlight hotplugging, so most
>     userspace software (e.g., KDE, UPower) is built with that assumption.
>     For the Ally, since it disconnects its controller during sleep, this
>     caused the backlight slider to not work in KDE.
>
> Changes since V1:
>   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
>   - Fix ifdef else having an invalid signature (reported by kernel robot)
>   - Restore input arguments to init and keyboard function so they can
>     be re-used for RGB controls.
>   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
>     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
>     keyboard rename into it.
>   - Unregister brightness listener before removing work queue to avoid
>     a race condition causing corruption
>   - Remove spurious mutex unlock in asus_brt_event
>   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
>     relocking the mutex and causing a deadlock when unregistering leds
>   - Add extra check during unregistering to avoid calling unregister when
>     no led device is registered.
>   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
>     the driver to create 4 RGB handlers per device. I also suspect some
>     extra events sneak through (KDE had the @@@@@@).
>
> Antheas Kapenekakis (11):
>   HID: asus: simplify RGB init sequence
>   HID: asus: initialize additional endpoints only for legacy devices
>   HID: asus: use same report_id in response
>   HID: asus: fortify keyboard handshake
>   HID: asus: move vendor initialization to probe
>   HID: asus: early return for ROG devices
>   platform/x86: asus-wmi: Add support for multiple kbd led handlers
>   HID: asus: listen to the asus-wmi brightness device instead of
>     creating one
>   platform/x86: asus-wmi: remove unused keyboard backlight quirk
>   platform/x86: asus-wmi: add keyboard brightness event handler
>   HID: asus: add support for the asus-wmi brightness handler
>
>  drivers/hid/hid-asus.c                        | 205 ++++++++--------
>  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
>  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
>  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
>  4 files changed, 322 insertions(+), 184 deletions(-)
>  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>
>
> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> --
> 2.52.0
>
>

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Ilpo Järvinen]

On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:

> On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > 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.
> 
> Slight bump on this. It addresses both of the remarks Denis made in
> the previous version.
> 
> I begrudgingly argued a bit for those because I did not want to resend
> the series and they were not functional changes, so sorry about that.
> But they are fixed in this version incl. with the conflict with the
> armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> Add support for multiple kbd led handlers" even though I addressed
> your comment, so you may want to add that.

FYI, there's no direct relation that mandates a person to give a rev-by 
even if all his/her comments were addressed.

But generally yes, it would be useful to hear whether Denis is fine with 
v10, especially those patches that had contention earlier but you've 
modified post-v8.

> As for "HID: asus: early return for ROG devices" changing the name of
> the devices of this driver, I will veto backporting it if it happens,
> so inputplumber will have the two full months to remove the name
> match. This is not a breaking change in the sense that software cannot
> be made to work on both previous and latter versions and there is no
> other software to my knowledge relying on name matches for Asus
> keyboards.

Did Hans give some opinion about this rename earlier, at least I don't 
remember nor could find from lore archives?

--
 i.

> Moreover, an early exit is needed to prevent ejecting HID
> endpoints without an ->input parameter so it is a needed fix anyway.
> Postponing it will prevent Xbox Ally users from having RGB control
> through userspace on a stock kernel but it is also not worth arguing
> about
> 
> It is also fine for me for this series to merge for 6.20, but I'd
> rather we handle it now since there will be some turbulence for asus
> users due to armoury merging so it makes sense to have this transition
> once.
>
> > ---
> > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> >
> > Changes since V9:
> >   - No functional changes
> >   - Rebase to review-ilpo-next
> >   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> >     "remove unused keyboard backlight quirk" + imports
> >     Dismiss Luke's review as this patch diverged
> >   - Reword paragraph in "Add support for multiple kbd led handlers" to be
> >     more verbose
> >   - Use kfree in fortify patch
> >   - Fix minor style quirks from --nonstict checkpatch run
> >
> > Changes since V8:
> >   - No functional changes
> >   - Move legacy init patch to second, modify first patch so that their
> >     diff is minimized
> >   - Split "prevent binding to all HID devices on ROG" into two patches:
> >     - moving backlight initialization into probe
> >     - early exit to skip ->init check and rename
> >     - Remove skipping vendor fixups for non-vendor devices. It is not possible
> >       to read usages before the report fixups are applied, so it did not work
> >   - In that patch, reword a comment to be single line and make is_vendor a bool
> >   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> >     has drifted too far since he reviewed/tested it.
> >
> > Changes since V7:
> >   - Readd legacy init quirk for Dennis
> >   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> >   - Fix warning due to enum_backlight receiving negative values
> >
> > Changes since V6:
> >   - Split initialization refactor into three patches, update commit text
> >     to be clearer in what it does
> >   - Replace spinlock accesses with guard and scoped guard in all patches
> >   - Add missing includes mentioned by Ilpo
> >   - Reflow, tweak comment in prevent binding to all HID devices on ROG
> >   - Replace asus_ref.asus with local reference in all patches
> >   - Add missing kernel doc comments
> >   - Other minor nits from Ilpo
> >   - User reported warning due to scheduling work while holding a spinlock.
> >     Restructure patch for multiple handlers to limit when spinlock is held to
> >     variable access only. In parallel, setup a workqueue to handle registration
> >     of led device and setting brightness. This is required as registering the
> >     led device triggers kbd_led_get which needs to hold the spinlock to
> >     protect the led_wk value. The workqueue is also required for the hid
> >     event passthrough to avoid scheduling work while holding the spinlock.
> >     Apply the workqueue to wmi brightness buttons as well, as that was
> >     omitted before this series and WMI access was performed.
> >   - On "HID: asus: prevent binding to all HID devices on ROG", rename
> >     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> >     This allows other quirks to apply (applies quirk that fixes keyboard
> >     being named as a pointer device).
> >
> > Changes since V5:
> >   - It's been a long time
> >   - Remove addition of RGB as that had some comments I need to work on
> >   - Remove folio patch (already merged)
> >   - Remove legacy fix patch 11 from V4. There is a small chance that
> >     without this patch, some old NKEY keyboards might not respond to
> >     RGB commands according to Luke, but the kernel driver does not do
> >     RGB currently. The 0x5d init is done by Armoury crate software in
> >     Windows. If an issue is found, we can re-add it or just remove patches
> >     1/2 before merging. However, init could use the cleanup.
> >
> > Changes since V4:
> >   - Fix KConfig (reported by kernel robot)
> >   - Fix Ilpo's nits, if I missed anything lmk
> >
> > Changes since V3:
> >   - Add initializer for 0x5d for old NKEY keyboards until it is verified
> >     that it is not needed for their media keys to function.
> >   - Cover init in asus-wmi with spinlock as per Hans
> >   - If asus-wmi registers WMI handler with brightness, init the brightness
> >     in USB Asus keyboards, per Hans.
> >   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> >   - Fix oops when unregistering asus-wmi by moving unregister outside of
> >     the spin lock (but after the asus reference is set to null)
> >
> > Changes since V2:
> >   - Check lazy init succeds in asus-wmi before setting register variable
> >   - make explicit check in asus_hid_register_listener for listener existing
> >     to avoid re-init
> >   - rename asus_brt to asus_hid in most places and harmonize everything
> >   - switch to a spinlock instead of a mutex to avoid kernel ooops
> >   - fixup hid device quirks to avoid multiple RGB devices while still exposing
> >     all input vendor devices. This includes moving rgb init to probe
> >     instead of the input_configured callbacks.
> >   - Remove fan key (during retest it appears to be 0xae that is already
> >     supported by hid-asus)
> >   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> >   - removes fds from userspace and breaks backlight functionality. All
> >   - current mainline drivers do not support backlight hotplugging, so most
> >     userspace software (e.g., KDE, UPower) is built with that assumption.
> >     For the Ally, since it disconnects its controller during sleep, this
> >     caused the backlight slider to not work in KDE.
> >
> > Changes since V1:
> >   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> >   - Fix ifdef else having an invalid signature (reported by kernel robot)
> >   - Restore input arguments to init and keyboard function so they can
> >     be re-used for RGB controls.
> >   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> >     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> >     keyboard rename into it.
> >   - Unregister brightness listener before removing work queue to avoid
> >     a race condition causing corruption
> >   - Remove spurious mutex unlock in asus_brt_event
> >   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> >     relocking the mutex and causing a deadlock when unregistering leds
> >   - Add extra check during unregistering to avoid calling unregister when
> >     no led device is registered.
> >   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> >     the driver to create 4 RGB handlers per device. I also suspect some
> >     extra events sneak through (KDE had the @@@@@@).
> >
> > Antheas Kapenekakis (11):
> >   HID: asus: simplify RGB init sequence
> >   HID: asus: initialize additional endpoints only for legacy devices
> >   HID: asus: use same report_id in response
> >   HID: asus: fortify keyboard handshake
> >   HID: asus: move vendor initialization to probe
> >   HID: asus: early return for ROG devices
> >   platform/x86: asus-wmi: Add support for multiple kbd led handlers
> >   HID: asus: listen to the asus-wmi brightness device instead of
> >     creating one
> >   platform/x86: asus-wmi: remove unused keyboard backlight quirk
> >   platform/x86: asus-wmi: add keyboard brightness event handler
> >   HID: asus: add support for the asus-wmi brightness handler
> >
> >  drivers/hid/hid-asus.c                        | 205 ++++++++--------
> >  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
> >  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
> >  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
> >  4 files changed, 322 insertions(+), 184 deletions(-)
> >  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> >
> >
> > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > --
> > 2.52.0
> >
> >
> 

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Antheas Kapenekakis]

On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>
> > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > >
> > > 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.
> >
> > Slight bump on this. It addresses both of the remarks Denis made in
> > the previous version.
> >
> > I begrudgingly argued a bit for those because I did not want to resend
> > the series and they were not functional changes, so sorry about that.
> > But they are fixed in this version incl. with the conflict with the
> > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > Add support for multiple kbd led handlers" even though I addressed
> > your comment, so you may want to add that.
>
> FYI, there's no direct relation that mandates a person to give a rev-by
> even if all his/her comments were addressed.

True, this is just a reminder because I did not hear from him and
since he added a rev-by on the kfree patch. There's no obligation from
my side.

> But generally yes, it would be useful to hear whether Denis is fine with
> v10, especially those patches that had contention earlier but you've
> modified post-v8.
>
> > As for "HID: asus: early return for ROG devices" changing the name of
> > the devices of this driver, I will veto backporting it if it happens,
> > so inputplumber will have the two full months to remove the name
> > match. This is not a breaking change in the sense that software cannot
> > be made to work on both previous and latter versions and there is no
> > other software to my knowledge relying on name matches for Asus
> > keyboards.
>
> Did Hans give some opinion about this rename earlier, at least I don't
> remember nor could find from lore archives?

Hans jumped in on the ayaneo controller patch. I don't think I saw
activity on this series

Antheas

> --
>  i.
>
> > Moreover, an early exit is needed to prevent ejecting HID
> > endpoints without an ->input parameter so it is a needed fix anyway.
> > Postponing it will prevent Xbox Ally users from having RGB control
> > through userspace on a stock kernel but it is also not worth arguing
> > about
> >
> > It is also fine for me for this series to merge for 6.20, but I'd
> > rather we handle it now since there will be some turbulence for asus
> > users due to armoury merging so it makes sense to have this transition
> > once.
> >
> > > ---
> > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > >
> > > Changes since V9:
> > >   - No functional changes
> > >   - Rebase to review-ilpo-next
> > >   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > >     "remove unused keyboard backlight quirk" + imports
> > >     Dismiss Luke's review as this patch diverged
> > >   - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > >     more verbose
> > >   - Use kfree in fortify patch
> > >   - Fix minor style quirks from --nonstict checkpatch run
> > >
> > > Changes since V8:
> > >   - No functional changes
> > >   - Move legacy init patch to second, modify first patch so that their
> > >     diff is minimized
> > >   - Split "prevent binding to all HID devices on ROG" into two patches:
> > >     - moving backlight initialization into probe
> > >     - early exit to skip ->init check and rename
> > >     - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > >       to read usages before the report fixups are applied, so it did not work
> > >   - In that patch, reword a comment to be single line and make is_vendor a bool
> > >   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > >     has drifted too far since he reviewed/tested it.
> > >
> > > Changes since V7:
> > >   - Readd legacy init quirk for Dennis
> > >   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > >   - Fix warning due to enum_backlight receiving negative values
> > >
> > > Changes since V6:
> > >   - Split initialization refactor into three patches, update commit text
> > >     to be clearer in what it does
> > >   - Replace spinlock accesses with guard and scoped guard in all patches
> > >   - Add missing includes mentioned by Ilpo
> > >   - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > >   - Replace asus_ref.asus with local reference in all patches
> > >   - Add missing kernel doc comments
> > >   - Other minor nits from Ilpo
> > >   - User reported warning due to scheduling work while holding a spinlock.
> > >     Restructure patch for multiple handlers to limit when spinlock is held to
> > >     variable access only. In parallel, setup a workqueue to handle registration
> > >     of led device and setting brightness. This is required as registering the
> > >     led device triggers kbd_led_get which needs to hold the spinlock to
> > >     protect the led_wk value. The workqueue is also required for the hid
> > >     event passthrough to avoid scheduling work while holding the spinlock.
> > >     Apply the workqueue to wmi brightness buttons as well, as that was
> > >     omitted before this series and WMI access was performed.
> > >   - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > >     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > >     This allows other quirks to apply (applies quirk that fixes keyboard
> > >     being named as a pointer device).
> > >
> > > Changes since V5:
> > >   - It's been a long time
> > >   - Remove addition of RGB as that had some comments I need to work on
> > >   - Remove folio patch (already merged)
> > >   - Remove legacy fix patch 11 from V4. There is a small chance that
> > >     without this patch, some old NKEY keyboards might not respond to
> > >     RGB commands according to Luke, but the kernel driver does not do
> > >     RGB currently. The 0x5d init is done by Armoury crate software in
> > >     Windows. If an issue is found, we can re-add it or just remove patches
> > >     1/2 before merging. However, init could use the cleanup.
> > >
> > > Changes since V4:
> > >   - Fix KConfig (reported by kernel robot)
> > >   - Fix Ilpo's nits, if I missed anything lmk
> > >
> > > Changes since V3:
> > >   - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > >     that it is not needed for their media keys to function.
> > >   - Cover init in asus-wmi with spinlock as per Hans
> > >   - If asus-wmi registers WMI handler with brightness, init the brightness
> > >     in USB Asus keyboards, per Hans.
> > >   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > >   - Fix oops when unregistering asus-wmi by moving unregister outside of
> > >     the spin lock (but after the asus reference is set to null)
> > >
> > > Changes since V2:
> > >   - Check lazy init succeds in asus-wmi before setting register variable
> > >   - make explicit check in asus_hid_register_listener for listener existing
> > >     to avoid re-init
> > >   - rename asus_brt to asus_hid in most places and harmonize everything
> > >   - switch to a spinlock instead of a mutex to avoid kernel ooops
> > >   - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > >     all input vendor devices. This includes moving rgb init to probe
> > >     instead of the input_configured callbacks.
> > >   - Remove fan key (during retest it appears to be 0xae that is already
> > >     supported by hid-asus)
> > >   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > >   - removes fds from userspace and breaks backlight functionality. All
> > >   - current mainline drivers do not support backlight hotplugging, so most
> > >     userspace software (e.g., KDE, UPower) is built with that assumption.
> > >     For the Ally, since it disconnects its controller during sleep, this
> > >     caused the backlight slider to not work in KDE.
> > >
> > > Changes since V1:
> > >   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > >   - Fix ifdef else having an invalid signature (reported by kernel robot)
> > >   - Restore input arguments to init and keyboard function so they can
> > >     be re-used for RGB controls.
> > >   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > >     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > >     keyboard rename into it.
> > >   - Unregister brightness listener before removing work queue to avoid
> > >     a race condition causing corruption
> > >   - Remove spurious mutex unlock in asus_brt_event
> > >   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > >     relocking the mutex and causing a deadlock when unregistering leds
> > >   - Add extra check during unregistering to avoid calling unregister when
> > >     no led device is registered.
> > >   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > >     the driver to create 4 RGB handlers per device. I also suspect some
> > >     extra events sneak through (KDE had the @@@@@@).
> > >
> > > Antheas Kapenekakis (11):
> > >   HID: asus: simplify RGB init sequence
> > >   HID: asus: initialize additional endpoints only for legacy devices
> > >   HID: asus: use same report_id in response
> > >   HID: asus: fortify keyboard handshake
> > >   HID: asus: move vendor initialization to probe
> > >   HID: asus: early return for ROG devices
> > >   platform/x86: asus-wmi: Add support for multiple kbd led handlers
> > >   HID: asus: listen to the asus-wmi brightness device instead of
> > >     creating one
> > >   platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > >   platform/x86: asus-wmi: add keyboard brightness event handler
> > >   HID: asus: add support for the asus-wmi brightness handler
> > >
> > >  drivers/hid/hid-asus.c                        | 205 ++++++++--------
> > >  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
> > >  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
> > >  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
> > >  4 files changed, 322 insertions(+), 184 deletions(-)
> > >  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > >
> > >
> > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > --
> > > 2.52.0
> > >
> > >
> >
>

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Antheas Kapenekakis]

On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> >
> > > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > >
> > > > 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.
> > >
> > > Slight bump on this. It addresses both of the remarks Denis made in
> > > the previous version.
> > >
> > > I begrudgingly argued a bit for those because I did not want to resend
> > > the series and they were not functional changes, so sorry about that.
> > > But they are fixed in this version incl. with the conflict with the
> > > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > > Add support for multiple kbd led handlers" even though I addressed
> > > your comment, so you may want to add that.
> >
> > FYI, there's no direct relation that mandates a person to give a rev-by
> > even if all his/her comments were addressed.
>
> True, this is just a reminder because I did not hear from him and
> since he added a rev-by on the kfree patch. There's no obligation from
> my side.
>
> > But generally yes, it would be useful to hear whether Denis is fine with
> > v10, especially those patches that had contention earlier but you've
> > modified post-v8.
> >
> > > As for "HID: asus: early return for ROG devices" changing the name of
> > > the devices of this driver, I will veto backporting it if it happens,
> > > so inputplumber will have the two full months to remove the name
> > > match. This is not a breaking change in the sense that software cannot
> > > be made to work on both previous and latter versions and there is no
> > > other software to my knowledge relying on name matches for Asus
> > > keyboards.
> >
> > Did Hans give some opinion about this rename earlier, at least I don't
> > remember nor could find from lore archives?
>
> Hans jumped in on the ayaneo controller patch. I don't think I saw
> activity on this series

Hans had some feedback around half a year ago for the latter part of
this series that binds the devices together

> Antheas
>
> > --
> >  i.
> >
> > > Moreover, an early exit is needed to prevent ejecting HID
> > > endpoints without an ->input parameter so it is a needed fix anyway.
> > > Postponing it will prevent Xbox Ally users from having RGB control
> > > through userspace on a stock kernel but it is also not worth arguing
> > > about
> > >
> > > It is also fine for me for this series to merge for 6.20, but I'd
> > > rather we handle it now since there will be some turbulence for asus
> > > users due to armoury merging so it makes sense to have this transition
> > > once.
> > >
> > > > ---
> > > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > >
> > > > Changes since V9:
> > > >   - No functional changes
> > > >   - Rebase to review-ilpo-next
> > > >   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > >     "remove unused keyboard backlight quirk" + imports
> > > >     Dismiss Luke's review as this patch diverged
> > > >   - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > >     more verbose
> > > >   - Use kfree in fortify patch
> > > >   - Fix minor style quirks from --nonstict checkpatch run
> > > >
> > > > Changes since V8:
> > > >   - No functional changes
> > > >   - Move legacy init patch to second, modify first patch so that their
> > > >     diff is minimized
> > > >   - Split "prevent binding to all HID devices on ROG" into two patches:
> > > >     - moving backlight initialization into probe
> > > >     - early exit to skip ->init check and rename
> > > >     - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > >       to read usages before the report fixups are applied, so it did not work
> > > >   - In that patch, reword a comment to be single line and make is_vendor a bool
> > > >   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > >     has drifted too far since he reviewed/tested it.
> > > >
> > > > Changes since V7:
> > > >   - Readd legacy init quirk for Dennis
> > > >   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > >   - Fix warning due to enum_backlight receiving negative values
> > > >
> > > > Changes since V6:
> > > >   - Split initialization refactor into three patches, update commit text
> > > >     to be clearer in what it does
> > > >   - Replace spinlock accesses with guard and scoped guard in all patches
> > > >   - Add missing includes mentioned by Ilpo
> > > >   - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > >   - Replace asus_ref.asus with local reference in all patches
> > > >   - Add missing kernel doc comments
> > > >   - Other minor nits from Ilpo
> > > >   - User reported warning due to scheduling work while holding a spinlock.
> > > >     Restructure patch for multiple handlers to limit when spinlock is held to
> > > >     variable access only. In parallel, setup a workqueue to handle registration
> > > >     of led device and setting brightness. This is required as registering the
> > > >     led device triggers kbd_led_get which needs to hold the spinlock to
> > > >     protect the led_wk value. The workqueue is also required for the hid
> > > >     event passthrough to avoid scheduling work while holding the spinlock.
> > > >     Apply the workqueue to wmi brightness buttons as well, as that was
> > > >     omitted before this series and WMI access was performed.
> > > >   - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > >     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > >     This allows other quirks to apply (applies quirk that fixes keyboard
> > > >     being named as a pointer device).
> > > >
> > > > Changes since V5:
> > > >   - It's been a long time
> > > >   - Remove addition of RGB as that had some comments I need to work on
> > > >   - Remove folio patch (already merged)
> > > >   - Remove legacy fix patch 11 from V4. There is a small chance that
> > > >     without this patch, some old NKEY keyboards might not respond to
> > > >     RGB commands according to Luke, but the kernel driver does not do
> > > >     RGB currently. The 0x5d init is done by Armoury crate software in
> > > >     Windows. If an issue is found, we can re-add it or just remove patches
> > > >     1/2 before merging. However, init could use the cleanup.
> > > >
> > > > Changes since V4:
> > > >   - Fix KConfig (reported by kernel robot)
> > > >   - Fix Ilpo's nits, if I missed anything lmk
> > > >
> > > > Changes since V3:
> > > >   - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > >     that it is not needed for their media keys to function.
> > > >   - Cover init in asus-wmi with spinlock as per Hans
> > > >   - If asus-wmi registers WMI handler with brightness, init the brightness
> > > >     in USB Asus keyboards, per Hans.
> > > >   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > >   - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > >     the spin lock (but after the asus reference is set to null)
> > > >
> > > > Changes since V2:
> > > >   - Check lazy init succeds in asus-wmi before setting register variable
> > > >   - make explicit check in asus_hid_register_listener for listener existing
> > > >     to avoid re-init
> > > >   - rename asus_brt to asus_hid in most places and harmonize everything
> > > >   - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > >   - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > >     all input vendor devices. This includes moving rgb init to probe
> > > >     instead of the input_configured callbacks.
> > > >   - Remove fan key (during retest it appears to be 0xae that is already
> > > >     supported by hid-asus)
> > > >   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > >   - removes fds from userspace and breaks backlight functionality. All
> > > >   - current mainline drivers do not support backlight hotplugging, so most
> > > >     userspace software (e.g., KDE, UPower) is built with that assumption.
> > > >     For the Ally, since it disconnects its controller during sleep, this
> > > >     caused the backlight slider to not work in KDE.
> > > >
> > > > Changes since V1:
> > > >   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > > >   - Fix ifdef else having an invalid signature (reported by kernel robot)
> > > >   - Restore input arguments to init and keyboard function so they can
> > > >     be re-used for RGB controls.
> > > >   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > > >     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > > >     keyboard rename into it.
> > > >   - Unregister brightness listener before removing work queue to avoid
> > > >     a race condition causing corruption
> > > >   - Remove spurious mutex unlock in asus_brt_event
> > > >   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > > >     relocking the mutex and causing a deadlock when unregistering leds
> > > >   - Add extra check during unregistering to avoid calling unregister when
> > > >     no led device is registered.
> > > >   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > > >     the driver to create 4 RGB handlers per device. I also suspect some
> > > >     extra events sneak through (KDE had the @@@@@@).
> > > >
> > > > Antheas Kapenekakis (11):
> > > >   HID: asus: simplify RGB init sequence
> > > >   HID: asus: initialize additional endpoints only for legacy devices
> > > >   HID: asus: use same report_id in response
> > > >   HID: asus: fortify keyboard handshake
> > > >   HID: asus: move vendor initialization to probe
> > > >   HID: asus: early return for ROG devices
> > > >   platform/x86: asus-wmi: Add support for multiple kbd led handlers
> > > >   HID: asus: listen to the asus-wmi brightness device instead of
> > > >     creating one
> > > >   platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > >   platform/x86: asus-wmi: add keyboard brightness event handler
> > > >   HID: asus: add support for the asus-wmi brightness handler
> > > >
> > > >  drivers/hid/hid-asus.c                        | 205 ++++++++--------
> > > >  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
> > > >  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
> > > >  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
> > > >  4 files changed, 322 insertions(+), 184 deletions(-)
> > > >  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > > >
> > > >
> > > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > > --
> > > > 2.52.0
> > > >
> > > >
> > >
> >

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 13042 bytes --]

On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:

> On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> > >
> > > > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > > >
> > > > > 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.
> > > >
> > > > Slight bump on this. It addresses both of the remarks Denis made in
> > > > the previous version.
> > > >
> > > > I begrudgingly argued a bit for those because I did not want to resend
> > > > the series and they were not functional changes, so sorry about that.
> > > > But they are fixed in this version incl. with the conflict with the
> > > > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > > > Add support for multiple kbd led handlers" even though I addressed
> > > > your comment, so you may want to add that.
> > >
> > > FYI, there's no direct relation that mandates a person to give a rev-by
> > > even if all his/her comments were addressed.
> >
> > True, this is just a reminder because I did not hear from him and
> > since he added a rev-by on the kfree patch. There's no obligation from
> > my side.
> >
> > > But generally yes, it would be useful to hear whether Denis is fine with
> > > v10, especially those patches that had contention earlier but you've
> > > modified post-v8.
> > >
> > > > As for "HID: asus: early return for ROG devices" changing the name of
> > > > the devices of this driver, I will veto backporting it if it happens,
> > > > so inputplumber will have the two full months to remove the name
> > > > match. This is not a breaking change in the sense that software cannot
> > > > be made to work on both previous and latter versions and there is no
> > > > other software to my knowledge relying on name matches for Asus
> > > > keyboards.
> > >
> > > Did Hans give some opinion about this rename earlier, at least I don't
> > > remember nor could find from lore archives?
> >
> > Hans jumped in on the ayaneo controller patch. I don't think I saw
> > activity on this series
> 
> Hans had some feedback around half a year ago for the latter part of
> this series that binds the devices together

Yes but I was interested on specifically about this userspace visible 
change. He has been around much much longer than me so his insight on 
userspace visible changes is way more valuable than mine.

--
 i.

> 
> > Antheas
> >
> > > --
> > >  i.
> > >
> > > > Moreover, an early exit is needed to prevent ejecting HID
> > > > endpoints without an ->input parameter so it is a needed fix anyway.
> > > > Postponing it will prevent Xbox Ally users from having RGB control
> > > > through userspace on a stock kernel but it is also not worth arguing
> > > > about
> > > >
> > > > It is also fine for me for this series to merge for 6.20, but I'd
> > > > rather we handle it now since there will be some turbulence for asus
> > > > users due to armoury merging so it makes sense to have this transition
> > > > once.
> > > >
> > > > > ---
> > > > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > > >
> > > > > Changes since V9:
> > > > >   - No functional changes
> > > > >   - Rebase to review-ilpo-next
> > > > >   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > > >     "remove unused keyboard backlight quirk" + imports
> > > > >     Dismiss Luke's review as this patch diverged
> > > > >   - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > > >     more verbose
> > > > >   - Use kfree in fortify patch
> > > > >   - Fix minor style quirks from --nonstict checkpatch run
> > > > >
> > > > > Changes since V8:
> > > > >   - No functional changes
> > > > >   - Move legacy init patch to second, modify first patch so that their
> > > > >     diff is minimized
> > > > >   - Split "prevent binding to all HID devices on ROG" into two patches:
> > > > >     - moving backlight initialization into probe
> > > > >     - early exit to skip ->init check and rename
> > > > >     - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > > >       to read usages before the report fixups are applied, so it did not work
> > > > >   - In that patch, reword a comment to be single line and make is_vendor a bool
> > > > >   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > > >     has drifted too far since he reviewed/tested it.
> > > > >
> > > > > Changes since V7:
> > > > >   - Readd legacy init quirk for Dennis
> > > > >   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > > >   - Fix warning due to enum_backlight receiving negative values
> > > > >
> > > > > Changes since V6:
> > > > >   - Split initialization refactor into three patches, update commit text
> > > > >     to be clearer in what it does
> > > > >   - Replace spinlock accesses with guard and scoped guard in all patches
> > > > >   - Add missing includes mentioned by Ilpo
> > > > >   - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > > >   - Replace asus_ref.asus with local reference in all patches
> > > > >   - Add missing kernel doc comments
> > > > >   - Other minor nits from Ilpo
> > > > >   - User reported warning due to scheduling work while holding a spinlock.
> > > > >     Restructure patch for multiple handlers to limit when spinlock is held to
> > > > >     variable access only. In parallel, setup a workqueue to handle registration
> > > > >     of led device and setting brightness. This is required as registering the
> > > > >     led device triggers kbd_led_get which needs to hold the spinlock to
> > > > >     protect the led_wk value. The workqueue is also required for the hid
> > > > >     event passthrough to avoid scheduling work while holding the spinlock.
> > > > >     Apply the workqueue to wmi brightness buttons as well, as that was
> > > > >     omitted before this series and WMI access was performed.
> > > > >   - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > > >     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > > >     This allows other quirks to apply (applies quirk that fixes keyboard
> > > > >     being named as a pointer device).
> > > > >
> > > > > Changes since V5:
> > > > >   - It's been a long time
> > > > >   - Remove addition of RGB as that had some comments I need to work on
> > > > >   - Remove folio patch (already merged)
> > > > >   - Remove legacy fix patch 11 from V4. There is a small chance that
> > > > >     without this patch, some old NKEY keyboards might not respond to
> > > > >     RGB commands according to Luke, but the kernel driver does not do
> > > > >     RGB currently. The 0x5d init is done by Armoury crate software in
> > > > >     Windows. If an issue is found, we can re-add it or just remove patches
> > > > >     1/2 before merging. However, init could use the cleanup.
> > > > >
> > > > > Changes since V4:
> > > > >   - Fix KConfig (reported by kernel robot)
> > > > >   - Fix Ilpo's nits, if I missed anything lmk
> > > > >
> > > > > Changes since V3:
> > > > >   - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > > >     that it is not needed for their media keys to function.
> > > > >   - Cover init in asus-wmi with spinlock as per Hans
> > > > >   - If asus-wmi registers WMI handler with brightness, init the brightness
> > > > >     in USB Asus keyboards, per Hans.
> > > > >   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > > >   - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > > >     the spin lock (but after the asus reference is set to null)
> > > > >
> > > > > Changes since V2:
> > > > >   - Check lazy init succeds in asus-wmi before setting register variable
> > > > >   - make explicit check in asus_hid_register_listener for listener existing
> > > > >     to avoid re-init
> > > > >   - rename asus_brt to asus_hid in most places and harmonize everything
> > > > >   - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > > >   - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > > >     all input vendor devices. This includes moving rgb init to probe
> > > > >     instead of the input_configured callbacks.
> > > > >   - Remove fan key (during retest it appears to be 0xae that is already
> > > > >     supported by hid-asus)
> > > > >   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > > >   - removes fds from userspace and breaks backlight functionality. All
> > > > >   - current mainline drivers do not support backlight hotplugging, so most
> > > > >     userspace software (e.g., KDE, UPower) is built with that assumption.
> > > > >     For the Ally, since it disconnects its controller during sleep, this
> > > > >     caused the backlight slider to not work in KDE.
> > > > >
> > > > > Changes since V1:
> > > > >   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > > > >   - Fix ifdef else having an invalid signature (reported by kernel robot)
> > > > >   - Restore input arguments to init and keyboard function so they can
> > > > >     be re-used for RGB controls.
> > > > >   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > > > >     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > > > >     keyboard rename into it.
> > > > >   - Unregister brightness listener before removing work queue to avoid
> > > > >     a race condition causing corruption
> > > > >   - Remove spurious mutex unlock in asus_brt_event
> > > > >   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > > > >     relocking the mutex and causing a deadlock when unregistering leds
> > > > >   - Add extra check during unregistering to avoid calling unregister when
> > > > >     no led device is registered.
> > > > >   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > > > >     the driver to create 4 RGB handlers per device. I also suspect some
> > > > >     extra events sneak through (KDE had the @@@@@@).
> > > > >
> > > > > Antheas Kapenekakis (11):
> > > > >   HID: asus: simplify RGB init sequence
> > > > >   HID: asus: initialize additional endpoints only for legacy devices
> > > > >   HID: asus: use same report_id in response
> > > > >   HID: asus: fortify keyboard handshake
> > > > >   HID: asus: move vendor initialization to probe
> > > > >   HID: asus: early return for ROG devices
> > > > >   platform/x86: asus-wmi: Add support for multiple kbd led handlers
> > > > >   HID: asus: listen to the asus-wmi brightness device instead of
> > > > >     creating one
> > > > >   platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > > >   platform/x86: asus-wmi: add keyboard brightness event handler
> > > > >   HID: asus: add support for the asus-wmi brightness handler
> > > > >
> > > > >  drivers/hid/hid-asus.c                        | 205 ++++++++--------
> > > > >  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
> > > > >  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
> > > > >  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
> > > > >  4 files changed, 322 insertions(+), 184 deletions(-)
> > > > >  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > > > >
> > > > >
> > > > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > > > --
> > > > > 2.52.0
> > > > >
> > > > >
> > > >
> > >
> 

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Hans de Goede]

Hi,

On 26-Nov-25 4:23 PM, Ilpo Järvinen wrote:
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:

...

>> As for "HID: asus: early return for ROG devices" changing the name of
>> the devices of this driver, I will veto backporting it if it happens,
>> so inputplumber will have the two full months to remove the name
>> match. This is not a breaking change in the sense that software cannot
>> be made to work on both previous and latter versions and there is no
>> other software to my knowledge relying on name matches for Asus
>> keyboards.
> 
> Did Hans give some opinion about this rename earlier, at least I don't 
> remember nor could find from lore archives?

I don't remember commenting on this myself either.

So generally speaking there are plenty of cases where /dev/input/event#
nodes for a specific device have their name changed by some kernel patches.

Typically HID input devices are matched in userspace by their
bus:vend-id:prod-id triplet not by the name. The name might even
change by a fwupdate of the device itself.

So I'm not overly worried about this and inputplumber seems nice
enough and already is very much not a plug-and-play tool.

One possible concern with laptop keyboard input-device name changes
though is hwdb entries to fixup scancode -> ev-key-code mappings.

See: /lib/udev/hwdb.d/60-keyboard.hwdb on any standard Linux systems
an then the big comment at the top.

An input-device name change might break this match pattern:

#  - Input driver device name and DMI data match:
#      evdev:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
#    <input device name> is the name device specified by the
#    driver, <vendor> is the firmware-provided string exported
#    by the kernel DMI modalias, see /sys/class/dmi/id/modalias

As well as the extended version of this and for laptops with USB
keyboards this is the only match type which allows a DMI match
which is what we want for laptop kbd mappings. Looking at the Asus
section of the upstream 60-keyboard.hwdb I do not see any such
matches though.

There not being such matches kinda make sense since for USB-HID
devices any special scancode -> ev-key-code mappings are typically
handled in a vendor specific HID driver like hid-asus.

TL;DR: I think that the input-device name should be fine.

Regards,

Hans

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Denis Benato]

On 11/26/25 18:34, Hans de Goede wrote:
> Hi,
>
> On 26-Nov-25 4:23 PM, Ilpo Järvinen wrote:
>> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> ...
>
>>> As for "HID: asus: early return for ROG devices" changing the name of
>>> the devices of this driver, I will veto backporting it if it happens,
>>> so inputplumber will have the two full months to remove the name
>>> match. This is not a breaking change in the sense that software cannot
>>> be made to work on both previous and latter versions and there is no
>>> other software to my knowledge relying on name matches for Asus
>>> keyboards.
>> Did Hans give some opinion about this rename earlier, at least I don't 
>> remember nor could find from lore archives?
> I don't remember commenting on this myself either.
>
> So generally speaking there are plenty of cases where /dev/input/event#
> nodes for a specific device have their name changed by some kernel patches.
>
> Typically HID input devices are matched in userspace by their
> bus:vend-id:prod-id triplet not by the name. The name might even
> change by a fwupdate of the device itself.
>
> So I'm not overly worried about this and inputplumber seems nice
> enough and already is very much not a plug-and-play tool.
>
> One possible concern with laptop keyboard input-device name changes
> though is hwdb entries to fixup scancode -> ev-key-code mappings.
>
> See: /lib/udev/hwdb.d/60-keyboard.hwdb on any standard Linux systems
> an then the big comment at the top.
>
> An input-device name change might break this match pattern:
>
> #  - Input driver device name and DMI data match:
> #      evdev:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
> #    <input device name> is the name device specified by the
> #    driver, <vendor> is the firmware-provided string exported
> #    by the kernel DMI modalias, see /sys/class/dmi/id/modalias
>
> As well as the extended version of this and for laptops with USB
> keyboards this is the only match type which allows a DMI match
> which is what we want for laptop kbd mappings. Looking at the Asus
> section of the upstream 60-keyboard.hwdb I do not see any such
> matches though.
>
> There not being such matches kinda make sense since for USB-HID
> devices any special scancode -> ev-key-code mappings are typically
> handled in a vendor specific HID driver like hid-asus.
>
> TL;DR: I think that the input-device name should be fine.
Thank you very much for this write-up! Now I feel much more confident
in giving approval!
> Regards,
>
> Hans
Thank you,
Denis

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Denis Benato]

On 11/26/25 16:31, Ilpo Järvinen wrote:
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>
>> On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>> On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
>>> <ilpo.jarvinen@linux.intel.com> wrote:
>>>> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>>>>
>>>>> On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>>>> 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.
>>>>> Slight bump on this. It addresses both of the remarks Denis made in
>>>>> the previous version.
>>>>>
>>>>> I begrudgingly argued a bit for those because I did not want to resend
>>>>> the series and they were not functional changes, so sorry about that.
>>>>> But they are fixed in this version incl. with the conflict with the
>>>>> armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
>>>>> Add support for multiple kbd led handlers" even though I addressed
>>>>> your comment, so you may want to add that.
>>>> FYI, there's no direct relation that mandates a person to give a rev-by
>>>> even if all his/her comments were addressed.
>>> True, this is just a reminder because I did not hear from him and
>>> since he added a rev-by on the kfree patch. There's no obligation from
>>> my side.
Lack of time: I will review the rest shortly unless some more unexpected
thing shows up.
>>>> But generally yes, it would be useful to hear whether Denis is fine with
>>>> v10, especially those patches that had contention earlier but you've
>>>> modified post-v8.
>>>>
>>>>> As for "HID: asus: early return for ROG devices" changing the name of
>>>>> the devices of this driver, I will veto backporting it if it happens,
>>>>> so inputplumber will have the two full months to remove the name
>>>>> match. This is not a breaking change in the sense that software cannot
>>>>> be made to work on both previous and latter versions and there is no
>>>>> other software to my knowledge relying on name matches for Asus
>>>>> keyboards.
>>>> Did Hans give some opinion about this rename earlier, at least I don't
>>>> remember nor could find from lore archives?
>>> Hans jumped in on the ayaneo controller patch. I don't think I saw
>>> activity on this series
>> Hans had some feedback around half a year ago for the latter part of
>> this series that binds the devices together
> Yes but I was interested on specifically about this userspace visible 
> change. He has been around much much longer than me so his insight on 
> userspace visible changes is way more valuable than mine.
>
> --
>  i.
>
>>> Antheas
>>>
>>>> --
>>>>  i.
>>>>
>>>>> Moreover, an early exit is needed to prevent ejecting HID
>>>>> endpoints without an ->input parameter so it is a needed fix anyway.
>>>>> Postponing it will prevent Xbox Ally users from having RGB control
>>>>> through userspace on a stock kernel but it is also not worth arguing
>>>>> about
>>>>>
>>>>> It is also fine for me for this series to merge for 6.20, but I'd
>>>>> rather we handle it now since there will be some turbulence for asus
>>>>> users due to armoury merging so it makes sense to have this transition
>>>>> once.
>>>>>
>>>>>> ---
>>>>>> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
>>>>>> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
>>>>>> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
>>>>>> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
>>>>>> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
>>>>>> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
>>>>>> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
>>>>>> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
>>>>>> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>>>>>>
>>>>>> Changes since V9:
>>>>>>   - No functional changes
>>>>>>   - Rebase to review-ilpo-next
>>>>>>   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
>>>>>>     "remove unused keyboard backlight quirk" + imports
>>>>>>     Dismiss Luke's review as this patch diverged
>>>>>>   - Reword paragraph in "Add support for multiple kbd led handlers" to be
>>>>>>     more verbose
>>>>>>   - Use kfree in fortify patch
>>>>>>   - Fix minor style quirks from --nonstict checkpatch run
>>>>>>
>>>>>> Changes since V8:
>>>>>>   - No functional changes
>>>>>>   - Move legacy init patch to second, modify first patch so that their
>>>>>>     diff is minimized
>>>>>>   - Split "prevent binding to all HID devices on ROG" into two patches:
>>>>>>     - moving backlight initialization into probe
>>>>>>     - early exit to skip ->init check and rename
>>>>>>     - Remove skipping vendor fixups for non-vendor devices. It is not possible
>>>>>>       to read usages before the report fixups are applied, so it did not work
>>>>>>   - In that patch, reword a comment to be single line and make is_vendor a bool
>>>>>>   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
>>>>>>     has drifted too far since he reviewed/tested it.
>>>>>>
>>>>>> Changes since V7:
>>>>>>   - Readd legacy init quirk for Dennis
>>>>>>   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
>>>>>>   - Fix warning due to enum_backlight receiving negative values
>>>>>>
>>>>>> Changes since V6:
>>>>>>   - Split initialization refactor into three patches, update commit text
>>>>>>     to be clearer in what it does
>>>>>>   - Replace spinlock accesses with guard and scoped guard in all patches
>>>>>>   - Add missing includes mentioned by Ilpo
>>>>>>   - Reflow, tweak comment in prevent binding to all HID devices on ROG
>>>>>>   - Replace asus_ref.asus with local reference in all patches
>>>>>>   - Add missing kernel doc comments
>>>>>>   - Other minor nits from Ilpo
>>>>>>   - User reported warning due to scheduling work while holding a spinlock.
>>>>>>     Restructure patch for multiple handlers to limit when spinlock is held to
>>>>>>     variable access only. In parallel, setup a workqueue to handle registration
>>>>>>     of led device and setting brightness. This is required as registering the
>>>>>>     led device triggers kbd_led_get which needs to hold the spinlock to
>>>>>>     protect the led_wk value. The workqueue is also required for the hid
>>>>>>     event passthrough to avoid scheduling work while holding the spinlock.
>>>>>>     Apply the workqueue to wmi brightness buttons as well, as that was
>>>>>>     omitted before this series and WMI access was performed.
>>>>>>   - On "HID: asus: prevent binding to all HID devices on ROG", rename
>>>>>>     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
>>>>>>     This allows other quirks to apply (applies quirk that fixes keyboard
>>>>>>     being named as a pointer device).
>>>>>>
>>>>>> Changes since V5:
>>>>>>   - It's been a long time
>>>>>>   - Remove addition of RGB as that had some comments I need to work on
>>>>>>   - Remove folio patch (already merged)
>>>>>>   - Remove legacy fix patch 11 from V4. There is a small chance that
>>>>>>     without this patch, some old NKEY keyboards might not respond to
>>>>>>     RGB commands according to Luke, but the kernel driver does not do
>>>>>>     RGB currently. The 0x5d init is done by Armoury crate software in
>>>>>>     Windows. If an issue is found, we can re-add it or just remove patches
>>>>>>     1/2 before merging. However, init could use the cleanup.
>>>>>>
>>>>>> Changes since V4:
>>>>>>   - Fix KConfig (reported by kernel robot)
>>>>>>   - Fix Ilpo's nits, if I missed anything lmk
>>>>>>
>>>>>> Changes since V3:
>>>>>>   - Add initializer for 0x5d for old NKEY keyboards until it is verified
>>>>>>     that it is not needed for their media keys to function.
>>>>>>   - Cover init in asus-wmi with spinlock as per Hans
>>>>>>   - If asus-wmi registers WMI handler with brightness, init the brightness
>>>>>>     in USB Asus keyboards, per Hans.
>>>>>>   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
>>>>>>   - Fix oops when unregistering asus-wmi by moving unregister outside of
>>>>>>     the spin lock (but after the asus reference is set to null)
>>>>>>
>>>>>> Changes since V2:
>>>>>>   - Check lazy init succeds in asus-wmi before setting register variable
>>>>>>   - make explicit check in asus_hid_register_listener for listener existing
>>>>>>     to avoid re-init
>>>>>>   - rename asus_brt to asus_hid in most places and harmonize everything
>>>>>>   - switch to a spinlock instead of a mutex to avoid kernel ooops
>>>>>>   - fixup hid device quirks to avoid multiple RGB devices while still exposing
>>>>>>     all input vendor devices. This includes moving rgb init to probe
>>>>>>     instead of the input_configured callbacks.
>>>>>>   - Remove fan key (during retest it appears to be 0xae that is already
>>>>>>     supported by hid-asus)
>>>>>>   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
>>>>>>   - removes fds from userspace and breaks backlight functionality. All
>>>>>>   - current mainline drivers do not support backlight hotplugging, so most
>>>>>>     userspace software (e.g., KDE, UPower) is built with that assumption.
>>>>>>     For the Ally, since it disconnects its controller during sleep, this
>>>>>>     caused the backlight slider to not work in KDE.
>>>>>>
>>>>>> Changes since V1:
>>>>>>   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
>>>>>>   - Fix ifdef else having an invalid signature (reported by kernel robot)
>>>>>>   - Restore input arguments to init and keyboard function so they can
>>>>>>     be re-used for RGB controls.
>>>>>>   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
>>>>>>     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
>>>>>>     keyboard rename into it.
>>>>>>   - Unregister brightness listener before removing work queue to avoid
>>>>>>     a race condition causing corruption
>>>>>>   - Remove spurious mutex unlock in asus_brt_event
>>>>>>   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
>>>>>>     relocking the mutex and causing a deadlock when unregistering leds
>>>>>>   - Add extra check during unregistering to avoid calling unregister when
>>>>>>     no led device is registered.
>>>>>>   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
>>>>>>     the driver to create 4 RGB handlers per device. I also suspect some
>>>>>>     extra events sneak through (KDE had the @@@@@@).
>>>>>>
>>>>>> Antheas Kapenekakis (11):
>>>>>>   HID: asus: simplify RGB init sequence
>>>>>>   HID: asus: initialize additional endpoints only for legacy devices
>>>>>>   HID: asus: use same report_id in response
>>>>>>   HID: asus: fortify keyboard handshake
>>>>>>   HID: asus: move vendor initialization to probe
>>>>>>   HID: asus: early return for ROG devices
>>>>>>   platform/x86: asus-wmi: Add support for multiple kbd led handlers
>>>>>>   HID: asus: listen to the asus-wmi brightness device instead of
>>>>>>     creating one
>>>>>>   platform/x86: asus-wmi: remove unused keyboard backlight quirk
>>>>>>   platform/x86: asus-wmi: add keyboard brightness event handler
>>>>>>   HID: asus: add support for the asus-wmi brightness handler
>>>>>>
>>>>>>  drivers/hid/hid-asus.c                        | 205 ++++++++--------
>>>>>>  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
>>>>>>  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
>>>>>>  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
>>>>>>  4 files changed, 322 insertions(+), 184 deletions(-)
>>>>>>  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>>>>>>
>>>>>>
>>>>>> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
>>>>>> --
>>>>>> 2.52.0
>>>>>>
>>>>>>

Re: [PATCH v10 04/11] HID: asus: fortify keyboard handshake

[Denis Benato]

On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Handshaking with an Asus device involves sending it a feature report
> with the string "ASUS Tech.Inc." and then reading it back to verify the
> handshake was successful, under the feature ID the interaction will
> take place.
>
> Currently, the driver only does the first part. Add the readback to
> verify the handshake was successful. As this could cause breakages,
> allow the verification to fail with a dmesg error until we verify
> all devices work with it (they seem to).
>
> Since the response is more than 16 bytes, increase the buffer size
> to 64 as well to avoid overflow errors.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a36a8beb35..2a412e10f916 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define FEATURE_REPORT_ID 0x0d
>  #define INPUT_REPORT_ID 0x5d
>  #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
>  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>  #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>  
> @@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>  
>  static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>  {
> +	/*
> +	 * The handshake is first sent as a set_report, then retrieved
> +	 * from a get_report. They should be equal.
> +	 */
>  	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
>  		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>  	int ret;
>  
>  	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> -	if (ret < 0)
> -		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> +		return ret;
> +	}
> +
> +	u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> +	if (!readbuf)
> +		return -ENOMEM;
> +
> +	ret = hid_hw_raw_request(hdev, report_id, readbuf,
> +				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> +				 HID_REQ_GET_REPORT);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> +	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> +		hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> +			 FEATURE_KBD_REPORT_SIZE, readbuf);
> +		/*
> +		 * Do not return error if handshake is wrong until this is
> +		 * verified to work for all devices.
> +		 */
> +	}
>  
>  	return ret;
>  }

Re: [PATCH v10 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one

[Denis Benato]

On 11/22/25 12:00, Antheas Kapenekakis wrote:
> 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: Denis Benato <benato.denis96@gmail.com>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 65 +++++++-----------------------------------
>  1 file changed, 10 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 461b0f8efdfb..475e34187326 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -27,7 +27,6 @@
>  #include <linux/hid.h>
>  #include <linux/module.h>
>  #include <linux/platform_data/x86/asus-wmi.h>
> -#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
>  #include <linux/input/mt.h>
>  #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
>  #include <linux/power_supply.h>
> @@ -103,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
>  
>  struct asus_kbd_leds {
> -	struct led_classdev cdev;
> +	struct asus_hid_listener listener;
>  	struct hid_device *hdev;
>  	struct work_struct work;
>  	unsigned int brightness;
> @@ -493,11 +492,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
>  	spin_unlock_irqrestore(&led->lock, flags);
>  }
>  
> -static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> -				   enum led_brightness brightness)
> +static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
> +				   int brightness)
>  {
> -	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> -						 cdev);
> +	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
> +						 listener);
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&led->lock, flags);
> @@ -507,20 +506,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
>  	asus_schedule_work(led);
>  }
>  
> -static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> -{
> -	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> -						 cdev);
> -	enum led_brightness brightness;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&led->lock, flags);
> -	brightness = led->brightness;
> -	spin_unlock_irqrestore(&led->lock, flags);
> -
> -	return brightness;
> -}
> -
>  static void asus_kbd_backlight_work(struct work_struct *work)
>  {
>  	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> @@ -537,34 +522,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
>  		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>  }
>  
> -/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
> - * precedence. We only activate HID-based backlight control when the
> - * WMI control is not available.
> - */
> -static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> -{
> -	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> -	u32 value;
> -	int ret;
> -
> -	if (!IS_ENABLED(CONFIG_ASUS_WMI))
> -		return false;
> -
> -	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
> -			dmi_check_system(asus_use_hid_led_dmi_ids)) {
> -		hid_info(hdev, "using HID for asus::kbd_backlight\n");
> -		return false;
> -	}
> -
> -	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
> -				       ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
> -	hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
> -	if (ret)
> -		return false;
> -
> -	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> -}
> -
>  /*
>   * We don't care about any other part of the string except the version section.
>   * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
> @@ -709,14 +666,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	drvdata->kbd_backlight->removed = false;
>  	drvdata->kbd_backlight->brightness = 0;
>  	drvdata->kbd_backlight->hdev = hdev;
> -	drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
> -	drvdata->kbd_backlight->cdev.max_brightness = 3;
> -	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
> -	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
> +	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
>  	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
>  	spin_lock_init(&drvdata->kbd_backlight->lock);
>  
> -	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
> +	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
>  	if (ret < 0) {
>  		/* No need to have this still around */
>  		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> @@ -1105,7 +1059,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
>  
>  	if (drvdata->kbd_backlight) {
>  		const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
> -				drvdata->kbd_backlight->cdev.brightness };
> +				drvdata->kbd_backlight->brightness };
>  		ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>  		if (ret < 0) {
>  			hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> @@ -1231,7 +1185,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	}
>  
>  	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> -	    !asus_kbd_wmi_led_control_present(hdev) &&
>  	    asus_kbd_register_leds(hdev))
>  		hid_warn(hdev, "Failed to initialize backlight.\n");
>  
> @@ -1278,6 +1231,8 @@ static void asus_remove(struct hid_device *hdev)
>  	unsigned long flags;
>  
>  	if (drvdata->kbd_backlight) {
> +		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +
>  		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
>  		drvdata->kbd_backlight->removed = true;
>  		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);

Re: [PATCH v10 09/11] platform/x86: asus-wmi: remove unused keyboard backlight quirk

[Denis Benato]

On 11/22/25 12:00, Antheas Kapenekakis wrote:
> The quirk for selecting whether keyboard backlight should be controlled
> by HID or WMI is not needed anymore, so remove the file containing it.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  .../platform_data/x86/asus-wmi-leds-ids.h     | 50 -------------------
>  1 file changed, 50 deletions(-)
>  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>
> diff --git a/include/linux/platform_data/x86/asus-wmi-leds-ids.h b/include/linux/platform_data/x86/asus-wmi-leds-ids.h
> deleted file mode 100644
> index 034a039c4e37..000000000000
> --- a/include/linux/platform_data/x86/asus-wmi-leds-ids.h
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
> -#define __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
> -
> -#include <linux/dmi.h>
> -#include <linux/types.h>
> -
> -/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
> -#if IS_REACHABLE(CONFIG_ASUS_WMI) || IS_REACHABLE(CONFIG_HID_ASUS)
> -static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> -		},
> -	},
> -	{ },
> -};
> -#endif
> -
> -#endif	/* __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H */

Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Denis Benato]

On 11/22/25 12:00, Antheas Kapenekakis wrote:
> The keyboard brightness control of Asus WMI keyboards is handled in
> kernel, which leads to the shortcut going from brightness 0, to 1,
> to 2, and 3.
>
> However, for HID keyboards it is exposed as a key and handled by the
> user's desktop environment. For the toggle button, this means that
> brightness control becomes on/off. In addition, in the absence of a
> DE, the keyboard brightness does not work.
>
> Therefore, expose an event handler for the keyboard brightness control
> which can then be used by hid-asus. Since this handler is called from
> an interrupt context, defer the actual work to a workqueue.
>
> In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> for maximum brightness since it is shared between hid-asus/asus-wmi.
>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Tested-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
>  include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
>  2 files changed, 54 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 84cde34ab6a8..a69464e45ca4 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
>  	}
>  }
>  
> +/*
> + * This function is called from hid-asus to inform asus-wmi of brightness
> + * changes initiated by the keyboard backlight keys.
> + */
> +int asus_hid_event(enum asus_hid_event event)
> +{
> +	struct asus_wmi *asus;
> +	int brightness;
> +
> +	guard(spinlock_irqsave)(&asus_ref.lock);
> +	asus = asus_ref.asus;
> +	if (!asus || !asus->kbd_led_registered)
> +		return -EBUSY;
> +
> +	brightness = asus->kbd_led_wk;
> +
> +	switch (event) {
> +	case ASUS_EV_BRTUP:
> +		brightness += 1;
> +		break;
> +	case ASUS_EV_BRTDOWN:
> +		brightness -= 1;
> +		break;
> +	case ASUS_EV_BRTTOGGLE:
> +		if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> +			brightness = 0;
> +		else
> +			brightness += 1;
> +		break;
> +	}
> +
> +	asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> +	asus->kbd_led_notify = true;
> +	queue_work(asus->led_workqueue, &asus->kbd_led_work);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_event);
> +
>  /*
>   * These functions actually update the LED's, and are called from a
LEDs as opposed to LED's?
>   * workqueue. By doing this as separate work rather than when the LED
> @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
>  {
>  	struct asus_hid_listener *listener;
>  	struct asus_wmi *asus;
> -	int max_level;
>  
>  	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> -	max_level = asus->kbd_led.max_brightness;
>  
>  	scoped_guard(spinlock_irqsave, &asus_ref.lock)
> -		asus->kbd_led_wk = clamp_val(value, 0, max_level);
> +		asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
>  
>  	if (asus->kbd_led_avail)
>  		kbd_led_update(asus);
> @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
>  	asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
>  	asus->kbd_led.brightness_set = kbd_led_set;
>  	asus->kbd_led.brightness_get = kbd_led_get;
> -	asus->kbd_led.max_brightness = 3;
> +	asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
>  	asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
>  	INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
>  
> @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
>  		return;
>  	}
>  	if (code == NOTIFY_KBD_BRTTOGGLE) {
> -		if (led_value == asus->kbd_led.max_brightness)
> +		if (led_value == ASUS_EV_MAX_BRIGHTNESS)
>  			kbd_led_set_by_kbd(asus, 0);
>  		else
>  			kbd_led_set_by_kbd(asus, led_value + 1);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index d347cffd05d5..7b872b5d0960 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -178,6 +178,14 @@ struct asus_hid_listener {
>  	void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
>  };
>  
> +enum asus_hid_event {
> +	ASUS_EV_BRTUP,
> +	ASUS_EV_BRTDOWN,
> +	ASUS_EV_BRTTOGGLE,
> +};
> +
> +#define ASUS_EV_MAX_BRIGHTNESS 3
> +
>  #if IS_REACHABLE(CONFIG_ASUS_WMI)
>  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
>  void set_ally_mcu_powersave(bool enabled);
> @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
>  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
>  int asus_hid_register_listener(struct asus_hid_listener *cdev);
>  void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> +int asus_hid_event(enum asus_hid_event event);
>  #else
>  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
>  {
> @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
>  static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
>  {
>  }
> +static inline int asus_hid_event(enum asus_hid_event event)
> +{
> +	return -ENODEV;
> +}
>  #endif
>  
>  #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */

Re: [PATCH v10 02/11] HID: asus: initialize additional endpoints only for legacy devices

[Denis Benato]

On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Currently, ID1/ID2 initializations are performed for all NKEY devices.
> However, ID1 initializations are only required for RGB control and are
> only supported for RGB capable devices. ID2 initializations are only
> required for initializing the Anime display endpoint which is only
> supported on devices with an Anime display. Both of these
> initializations are out of scope for this driver (this is a brightness
> control and keyboard shortcut driver) and they should not be performed
> for devices that do not support them in any case.
>
> At the same time, there are older NKEY devices that have only been
> tested with these initializations in the kernel and it is not possible
> to recheck them. There is a possibility that especially with the ID1
> initialization, certain laptop models might have their shortcuts stop
> working (currently unproven).
>
> For an abundance of caution, only initialize ID1/ID2 for those older
> NKEY devices by introducing a quirk for them and replacing the NKEY
> quirk in the block that performs the inits with that.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a82286a427b8..9004814fb0d3 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
>  #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
>  #define QUIRK_ROG_ALLY_XPAD		BIT(13)
> +#define QUIRK_ROG_NKEY_LEGACY		BIT(14)
>  
>  #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
>  						 QUIRK_NO_INIT_REPORTS | \
> @@ -652,7 +653,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>  		return -ENODEV;
>  
> -	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> +	if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
>  		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>  		if (ret < 0)
>  			return ret;
> @@ -1376,10 +1377,10 @@ static const struct hid_device_id asus_devices[] = {
>  	  QUIRK_USE_KBD_BACKLIGHT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
>  	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },

Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Antheas Kapenekakis]

On Wed, 26 Nov 2025 at 21:23, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 11/22/25 12:00, Antheas Kapenekakis wrote:
> > The keyboard brightness control of Asus WMI keyboards is handled in
> > kernel, which leads to the shortcut going from brightness 0, to 1,
> > to 2, and 3.
> >
> > However, for HID keyboards it is exposed as a key and handled by the
> > user's desktop environment. For the toggle button, this means that
> > brightness control becomes on/off. In addition, in the absence of a
> > DE, the keyboard brightness does not work.
> >
> > Therefore, expose an event handler for the keyboard brightness control
> > which can then be used by hid-asus. Since this handler is called from
> > an interrupt context, defer the actual work to a workqueue.
> >
> > In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> > for maximum brightness since it is shared between hid-asus/asus-wmi.
> >
> > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > Tested-by: Luke D. Jones <luke@ljones.dev>
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >  drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
> >  include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> >  2 files changed, 54 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 84cde34ab6a8..a69464e45ca4 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> >       }
> >  }
> >
> > +/*
> > + * This function is called from hid-asus to inform asus-wmi of brightness
> > + * changes initiated by the keyboard backlight keys.
> > + */
> > +int asus_hid_event(enum asus_hid_event event)
> > +{
> > +     struct asus_wmi *asus;
> > +     int brightness;
> > +
> > +     guard(spinlock_irqsave)(&asus_ref.lock);
> > +     asus = asus_ref.asus;
> > +     if (!asus || !asus->kbd_led_registered)
> > +             return -EBUSY;
> > +
> > +     brightness = asus->kbd_led_wk;
> > +
> > +     switch (event) {
> > +     case ASUS_EV_BRTUP:
> > +             brightness += 1;
> > +             break;
> > +     case ASUS_EV_BRTDOWN:
> > +             brightness -= 1;
> > +             break;
> > +     case ASUS_EV_BRTTOGGLE:
> > +             if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > +                     brightness = 0;
> > +             else
> > +                     brightness += 1;
> > +             break;
> > +     }
> > +
> > +     asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> > +     asus->kbd_led_notify = true;
> > +     queue_work(asus->led_workqueue, &asus->kbd_led_work);
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > +
> >  /*
> >   * These functions actually update the LED's, and are called from a
> LEDs as opposed to LED's?

I agree with you, but the author of that line probably wouldn't -
depends on author dialect and both are usually correct.

When making acronyms plural, adding a ' is usually accepted as
correct. But this is not added as part of this series, so you can do a
reword commit if you wish

Antheas

> >   * workqueue. By doing this as separate work rather than when the LED
> > @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> >  {
> >       struct asus_hid_listener *listener;
> >       struct asus_wmi *asus;
> > -     int max_level;
> >
> >       asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> > -     max_level = asus->kbd_led.max_brightness;
> >
> >       scoped_guard(spinlock_irqsave, &asus_ref.lock)
> > -             asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > +             asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
> >
> >       if (asus->kbd_led_avail)
> >               kbd_led_update(asus);
> > @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> >       asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> >       asus->kbd_led.brightness_set = kbd_led_set;
> >       asus->kbd_led.brightness_get = kbd_led_get;
> > -     asus->kbd_led.max_brightness = 3;
> > +     asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> >       asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> >       INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
> >
> > @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> >               return;
> >       }
> >       if (code == NOTIFY_KBD_BRTTOGGLE) {
> > -             if (led_value == asus->kbd_led.max_brightness)
> > +             if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> >                       kbd_led_set_by_kbd(asus, 0);
> >               else
> >                       kbd_led_set_by_kbd(asus, led_value + 1);
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index d347cffd05d5..7b872b5d0960 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -178,6 +178,14 @@ struct asus_hid_listener {
> >       void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> >  };
> >
> > +enum asus_hid_event {
> > +     ASUS_EV_BRTUP,
> > +     ASUS_EV_BRTDOWN,
> > +     ASUS_EV_BRTTOGGLE,
> > +};
> > +
> > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > +
> >  #if IS_REACHABLE(CONFIG_ASUS_WMI)
> >  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> >  void set_ally_mcu_powersave(bool enabled);
> > @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> >  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> >  int asus_hid_register_listener(struct asus_hid_listener *cdev);
> >  void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > +int asus_hid_event(enum asus_hid_event event);
> >  #else
> >  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> >  {
> > @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> >  static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> >  {
> >  }
> > +static inline int asus_hid_event(enum asus_hid_event event)
> > +{
> > +     return -ENODEV;
> > +}
> >  #endif
> >
> >  #endif       /* __PLATFORM_DATA_X86_ASUS_WMI_H */
>

Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Antheas Kapenekakis]

On Wed, 26 Nov 2025 at 21:39, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Wed, 26 Nov 2025 at 21:23, Denis Benato <benato.denis96@gmail.com> wrote:
> >
> >
> > On 11/22/25 12:00, Antheas Kapenekakis wrote:
> > > The keyboard brightness control of Asus WMI keyboards is handled in
> > > kernel, which leads to the shortcut going from brightness 0, to 1,
> > > to 2, and 3.
> > >
> > > However, for HID keyboards it is exposed as a key and handled by the
> > > user's desktop environment. For the toggle button, this means that
> > > brightness control becomes on/off. In addition, in the absence of a
> > > DE, the keyboard brightness does not work.
> > >
> > > Therefore, expose an event handler for the keyboard brightness control
> > > which can then be used by hid-asus. Since this handler is called from
> > > an interrupt context, defer the actual work to a workqueue.
> > >
> > > In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> > > for maximum brightness since it is shared between hid-asus/asus-wmi.
> > >
> > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > >  drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
> > >  include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> > >  2 files changed, 54 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > index 84cde34ab6a8..a69464e45ca4 100644
> > > --- a/drivers/platform/x86/asus-wmi.c
> > > +++ b/drivers/platform/x86/asus-wmi.c
> > > @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> > >       }
> > >  }
> > >
> > > +/*
> > > + * This function is called from hid-asus to inform asus-wmi of brightness
> > > + * changes initiated by the keyboard backlight keys.
> > > + */
> > > +int asus_hid_event(enum asus_hid_event event)
> > > +{
> > > +     struct asus_wmi *asus;
> > > +     int brightness;
> > > +
> > > +     guard(spinlock_irqsave)(&asus_ref.lock);
> > > +     asus = asus_ref.asus;
> > > +     if (!asus || !asus->kbd_led_registered)
> > > +             return -EBUSY;
> > > +
> > > +     brightness = asus->kbd_led_wk;
> > > +
> > > +     switch (event) {
> > > +     case ASUS_EV_BRTUP:
> > > +             brightness += 1;
> > > +             break;
> > > +     case ASUS_EV_BRTDOWN:
> > > +             brightness -= 1;
> > > +             break;
> > > +     case ASUS_EV_BRTTOGGLE:
> > > +             if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > > +                     brightness = 0;
> > > +             else
> > > +                     brightness += 1;
> > > +             break;
> > > +     }
> > > +
> > > +     asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > +     asus->kbd_led_notify = true;
> > > +     queue_work(asus->led_workqueue, &asus->kbd_led_work);
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > > +
> > >  /*
> > >   * These functions actually update the LED's, and are called from a
> > LEDs as opposed to LED's?
>
> I agree with you, but the author of that line probably wouldn't -
> depends on author dialect and both are usually correct.
>
> When making acronyms plural, adding a ' is usually accepted as
> correct. But this is not added as part of this series, so you can do a
> reword commit if you wish
>
> Antheas

Hm, perhaps this was not clear but the part you commented on was not
part of the patch/this series. Do you want to finish reviewing this
patch and add a rev-by?

@Ilpo: with 6.18 releasing yesterday, what is the status on this? is
it for 6.20? Hans commented on patch 5

Thanks,
Antheas

> > >   * workqueue. By doing this as separate work rather than when the LED
> > > @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > >  {
> > >       struct asus_hid_listener *listener;
> > >       struct asus_wmi *asus;
> > > -     int max_level;
> > >
> > >       asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> > > -     max_level = asus->kbd_led.max_brightness;
> > >
> > >       scoped_guard(spinlock_irqsave, &asus_ref.lock)
> > > -             asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > > +             asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
> > >
> > >       if (asus->kbd_led_avail)
> > >               kbd_led_update(asus);
> > > @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > >       asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > >       asus->kbd_led.brightness_set = kbd_led_set;
> > >       asus->kbd_led.brightness_get = kbd_led_get;
> > > -     asus->kbd_led.max_brightness = 3;
> > > +     asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> > >       asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> > >       INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
> > >
> > > @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> > >               return;
> > >       }
> > >       if (code == NOTIFY_KBD_BRTTOGGLE) {
> > > -             if (led_value == asus->kbd_led.max_brightness)
> > > +             if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> > >                       kbd_led_set_by_kbd(asus, 0);
> > >               else
> > >                       kbd_led_set_by_kbd(asus, led_value + 1);
> > > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > > index d347cffd05d5..7b872b5d0960 100644
> > > --- a/include/linux/platform_data/x86/asus-wmi.h
> > > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > > @@ -178,6 +178,14 @@ struct asus_hid_listener {
> > >       void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > >  };
> > >
> > > +enum asus_hid_event {
> > > +     ASUS_EV_BRTUP,
> > > +     ASUS_EV_BRTDOWN,
> > > +     ASUS_EV_BRTTOGGLE,
> > > +};
> > > +
> > > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > > +
> > >  #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > >  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > >  void set_ally_mcu_powersave(bool enabled);
> > > @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> > >  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> > >  int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > >  void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > > +int asus_hid_event(enum asus_hid_event event);
> > >  #else
> > >  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > >  {
> > > @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > >  static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > >  {
> > >  }
> > > +static inline int asus_hid_event(enum asus_hid_event event)
> > > +{
> > > +     return -ENODEV;
> > > +}
> > >  #endif
> > >
> > >  #endif       /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> >

Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Ilpo Järvinen]

On Mon, 1 Dec 2025, Antheas Kapenekakis wrote:
> On Wed, 26 Nov 2025 at 21:39, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > On Wed, 26 Nov 2025 at 21:23, Denis Benato <benato.denis96@gmail.com> wrote:
> > > On 11/22/25 12:00, Antheas Kapenekakis wrote:
> > > > The keyboard brightness control of Asus WMI keyboards is handled in
> > > > kernel, which leads to the shortcut going from brightness 0, to 1,
> > > > to 2, and 3.
> > > >
> > > > However, for HID keyboards it is exposed as a key and handled by the
> > > > user's desktop environment. For the toggle button, this means that
> > > > brightness control becomes on/off. In addition, in the absence of a
> > > > DE, the keyboard brightness does not work.
> > > >
> > > > Therefore, expose an event handler for the keyboard brightness control
> > > > which can then be used by hid-asus. Since this handler is called from
> > > > an interrupt context, defer the actual work to a workqueue.
> > > >
> > > > In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> > > > for maximum brightness since it is shared between hid-asus/asus-wmi.
> > > >
> > > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > > ---
> > > >  drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
> > > >  include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> > > >  2 files changed, 54 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > > index 84cde34ab6a8..a69464e45ca4 100644
> > > > --- a/drivers/platform/x86/asus-wmi.c
> > > > +++ b/drivers/platform/x86/asus-wmi.c
> > > > @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> > > >       }
> > > >  }
> > > >
> > > > +/*
> > > > + * This function is called from hid-asus to inform asus-wmi of brightness
> > > > + * changes initiated by the keyboard backlight keys.
> > > > + */
> > > > +int asus_hid_event(enum asus_hid_event event)
> > > > +{
> > > > +     struct asus_wmi *asus;
> > > > +     int brightness;
> > > > +
> > > > +     guard(spinlock_irqsave)(&asus_ref.lock);
> > > > +     asus = asus_ref.asus;
> > > > +     if (!asus || !asus->kbd_led_registered)
> > > > +             return -EBUSY;
> > > > +
> > > > +     brightness = asus->kbd_led_wk;
> > > > +
> > > > +     switch (event) {
> > > > +     case ASUS_EV_BRTUP:
> > > > +             brightness += 1;
> > > > +             break;
> > > > +     case ASUS_EV_BRTDOWN:
> > > > +             brightness -= 1;
> > > > +             break;
> > > > +     case ASUS_EV_BRTTOGGLE:
> > > > +             if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > > > +                     brightness = 0;
> > > > +             else
> > > > +                     brightness += 1;
> > > > +             break;
> > > > +     }
> > > > +
> > > > +     asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > > +     asus->kbd_led_notify = true;
> > > > +     queue_work(asus->led_workqueue, &asus->kbd_led_work);
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > > > +
> > > >  /*
> > > >   * These functions actually update the LED's, and are called from a
> > > LEDs as opposed to LED's?
> >
> > I agree with you, but the author of that line probably wouldn't -
> > depends on author dialect and both are usually correct.
> >
> > When making acronyms plural, adding a ' is usually accepted as
> > correct. But this is not added as part of this series, so you can do a
> > reword commit if you wish
> >
> > Antheas
> 
> Hm, perhaps this was not clear but the part you commented on was not
> part of the patch/this series. Do you want to finish reviewing this
> patch and add a rev-by?
> 
> @Ilpo: with 6.18 releasing yesterday, what is the status on this? is
> it for 6.20? Hans commented on patch 5

It will have to wait to the 6.20 cycle as it's cross-subsystem series so 
there are other maintainers involved than just me.

-- 
 i.

> Thanks,
> Antheas
> 
> > > >   * workqueue. By doing this as separate work rather than when the LED
> > > > @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > > >  {
> > > >       struct asus_hid_listener *listener;
> > > >       struct asus_wmi *asus;
> > > > -     int max_level;
> > > >
> > > >       asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> > > > -     max_level = asus->kbd_led.max_brightness;
> > > >
> > > >       scoped_guard(spinlock_irqsave, &asus_ref.lock)
> > > > -             asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > > > +             asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > >
> > > >       if (asus->kbd_led_avail)
> > > >               kbd_led_update(asus);
> > > > @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > > >       asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > > >       asus->kbd_led.brightness_set = kbd_led_set;
> > > >       asus->kbd_led.brightness_get = kbd_led_get;
> > > > -     asus->kbd_led.max_brightness = 3;
> > > > +     asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> > > >       asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> > > >       INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
> > > >
> > > > @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> > > >               return;
> > > >       }
> > > >       if (code == NOTIFY_KBD_BRTTOGGLE) {
> > > > -             if (led_value == asus->kbd_led.max_brightness)
> > > > +             if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> > > >                       kbd_led_set_by_kbd(asus, 0);
> > > >               else
> > > >                       kbd_led_set_by_kbd(asus, led_value + 1);
> > > > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > > > index d347cffd05d5..7b872b5d0960 100644
> > > > --- a/include/linux/platform_data/x86/asus-wmi.h
> > > > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > > > @@ -178,6 +178,14 @@ struct asus_hid_listener {
> > > >       void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > > >  };
> > > >
> > > > +enum asus_hid_event {
> > > > +     ASUS_EV_BRTUP,
> > > > +     ASUS_EV_BRTDOWN,
> > > > +     ASUS_EV_BRTTOGGLE,
> > > > +};
> > > > +
> > > > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > > > +
> > > >  #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > > >  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > > >  void set_ally_mcu_powersave(bool enabled);
> > > > @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> > > >  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> > > >  int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > > >  void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > > > +int asus_hid_event(enum asus_hid_event event);
> > > >  #else
> > > >  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > > >  {
> > > > @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > > >  static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > > >  {
> > > >  }
> > > > +static inline int asus_hid_event(enum asus_hid_event event)
> > > > +{
> > > > +     return -ENODEV;
> > > > +}
> > > >  #endif
> > > >
> > > >  #endif       /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> > >
> 

Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Antheas Kapenekakis]

On Mon, 1 Dec 2025 at 10:52, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 1 Dec 2025, Antheas Kapenekakis wrote:
> > On Wed, 26 Nov 2025 at 21:39, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > On Wed, 26 Nov 2025 at 21:23, Denis Benato <benato.denis96@gmail.com> wrote:
> > > > On 11/22/25 12:00, Antheas Kapenekakis wrote:
> > > > > The keyboard brightness control of Asus WMI keyboards is handled in
> > > > > kernel, which leads to the shortcut going from brightness 0, to 1,
> > > > > to 2, and 3.
> > > > >
> > > > > However, for HID keyboards it is exposed as a key and handled by the
> > > > > user's desktop environment. For the toggle button, this means that
> > > > > brightness control becomes on/off. In addition, in the absence of a
> > > > > DE, the keyboard brightness does not work.
> > > > >
> > > > > Therefore, expose an event handler for the keyboard brightness control
> > > > > which can then be used by hid-asus. Since this handler is called from
> > > > > an interrupt context, defer the actual work to a workqueue.
> > > > >
> > > > > In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> > > > > for maximum brightness since it is shared between hid-asus/asus-wmi.
> > > > >
> > > > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > > > ---
> > > > >  drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
> > > > >  include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> > > > >  2 files changed, 54 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > > > index 84cde34ab6a8..a69464e45ca4 100644
> > > > > --- a/drivers/platform/x86/asus-wmi.c
> > > > > +++ b/drivers/platform/x86/asus-wmi.c
> > > > > @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> > > > >       }
> > > > >  }
> > > > >
> > > > > +/*
> > > > > + * This function is called from hid-asus to inform asus-wmi of brightness
> > > > > + * changes initiated by the keyboard backlight keys.
> > > > > + */
> > > > > +int asus_hid_event(enum asus_hid_event event)
> > > > > +{
> > > > > +     struct asus_wmi *asus;
> > > > > +     int brightness;
> > > > > +
> > > > > +     guard(spinlock_irqsave)(&asus_ref.lock);
> > > > > +     asus = asus_ref.asus;
> > > > > +     if (!asus || !asus->kbd_led_registered)
> > > > > +             return -EBUSY;
> > > > > +
> > > > > +     brightness = asus->kbd_led_wk;
> > > > > +
> > > > > +     switch (event) {
> > > > > +     case ASUS_EV_BRTUP:
> > > > > +             brightness += 1;
> > > > > +             break;
> > > > > +     case ASUS_EV_BRTDOWN:
> > > > > +             brightness -= 1;
> > > > > +             break;
> > > > > +     case ASUS_EV_BRTTOGGLE:
> > > > > +             if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > > > > +                     brightness = 0;
> > > > > +             else
> > > > > +                     brightness += 1;
> > > > > +             break;
> > > > > +     }
> > > > > +
> > > > > +     asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > > > +     asus->kbd_led_notify = true;
> > > > > +     queue_work(asus->led_workqueue, &asus->kbd_led_work);
> > > > > +     return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > > > > +
> > > > >  /*
> > > > >   * These functions actually update the LED's, and are called from a
> > > > LEDs as opposed to LED's?
> > >
> > > I agree with you, but the author of that line probably wouldn't -
> > > depends on author dialect and both are usually correct.
> > >
> > > When making acronyms plural, adding a ' is usually accepted as
> > > correct. But this is not added as part of this series, so you can do a
> > > reword commit if you wish
> > >
> > > Antheas
> >
> > Hm, perhaps this was not clear but the part you commented on was not
> > part of the patch/this series. Do you want to finish reviewing this
> > patch and add a rev-by?
> >
> > @Ilpo: with 6.18 releasing yesterday, what is the status on this? is
> > it for 6.20? Hans commented on patch 5
>
> It will have to wait to the 6.20 cycle as it's cross-subsystem series so
> there are other maintainers involved than just me.

Unfortunate. Jiri is cc'd on this series and we had around 2 weeks
where the code was frozen. Was it my responsibility to ask for acks so
it can go through x86?

> --
>  i.
>
> > Thanks,
> > Antheas
> >
> > > > >   * workqueue. By doing this as separate work rather than when the LED
> > > > > @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > > > >  {
> > > > >       struct asus_hid_listener *listener;
> > > > >       struct asus_wmi *asus;
> > > > > -     int max_level;
> > > > >
> > > > >       asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> > > > > -     max_level = asus->kbd_led.max_brightness;
> > > > >
> > > > >       scoped_guard(spinlock_irqsave, &asus_ref.lock)
> > > > > -             asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > > > > +             asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > > >
> > > > >       if (asus->kbd_led_avail)
> > > > >               kbd_led_update(asus);
> > > > > @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > > > >       asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > > > >       asus->kbd_led.brightness_set = kbd_led_set;
> > > > >       asus->kbd_led.brightness_get = kbd_led_get;
> > > > > -     asus->kbd_led.max_brightness = 3;
> > > > > +     asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> > > > >       asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> > > > >       INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
> > > > >
> > > > > @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> > > > >               return;
> > > > >       }
> > > > >       if (code == NOTIFY_KBD_BRTTOGGLE) {
> > > > > -             if (led_value == asus->kbd_led.max_brightness)
> > > > > +             if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> > > > >                       kbd_led_set_by_kbd(asus, 0);
> > > > >               else
> > > > >                       kbd_led_set_by_kbd(asus, led_value + 1);
> > > > > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > > > > index d347cffd05d5..7b872b5d0960 100644
> > > > > --- a/include/linux/platform_data/x86/asus-wmi.h
> > > > > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > > > > @@ -178,6 +178,14 @@ struct asus_hid_listener {
> > > > >       void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > > > >  };
> > > > >
> > > > > +enum asus_hid_event {
> > > > > +     ASUS_EV_BRTUP,
> > > > > +     ASUS_EV_BRTDOWN,
> > > > > +     ASUS_EV_BRTTOGGLE,
> > > > > +};
> > > > > +
> > > > > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > > > > +
> > > > >  #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > > > >  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > > > >  void set_ally_mcu_powersave(bool enabled);
> > > > > @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> > > > >  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> > > > >  int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > > > >  void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > > > > +int asus_hid_event(enum asus_hid_event event);
> > > > >  #else
> > > > >  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > > > >  {
> > > > > @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > > > >  static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > > > >  {
> > > > >  }
> > > > > +static inline int asus_hid_event(enum asus_hid_event event)
> > > > > +{
> > > > > +     return -ENODEV;
> > > > > +}
> > > > >  #endif
> > > > >
> > > > >  #endif       /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> > > >
> >
>

Re: [PATCH v10 04/11] HID: asus: fortify keyboard handshake

[Benjamin Tissoires]

On Nov 22 2025, Antheas Kapenekakis wrote:
> Handshaking with an Asus device involves sending it a feature report
> with the string "ASUS Tech.Inc." and then reading it back to verify the
> handshake was successful, under the feature ID the interaction will
> take place.
> 
> Currently, the driver only does the first part. Add the readback to
> verify the handshake was successful. As this could cause breakages,
> allow the verification to fail with a dmesg error until we verify
> all devices work with it (they seem to).
> 
> Since the response is more than 16 bytes, increase the buffer size
> to 64 as well to avoid overflow errors.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a36a8beb35..2a412e10f916 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define FEATURE_REPORT_ID 0x0d
>  #define INPUT_REPORT_ID 0x5d
>  #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
>  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>  #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>  
> @@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>  
>  static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>  {
> +	/*
> +	 * The handshake is first sent as a set_report, then retrieved
> +	 * from a get_report. They should be equal.
> +	 */
>  	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
>  		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>  	int ret;
>  
>  	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> -	if (ret < 0)
> -		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> +		return ret;
> +	}
> +
> +	u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> +	if (!readbuf)
> +		return -ENOMEM;
> +
> +	ret = hid_hw_raw_request(hdev, report_id, readbuf,
> +				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> +				 HID_REQ_GET_REPORT);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);

Nitpick here: previously we would not check for the answer after the
ping, so the ret value would be positive for those devices where we fail
to receive the handshake.

So basically, there is a mismatch between the commit description which
says that we keep existing devices working when there is a slight change
in the behaviour. OTOH, I guess that if the Windows driver reads back
this value, there are very little chance to return a negative error
code, so this is probably just fine.

But at least we know there is this change :)

Anyway:
Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>

> +	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> +		hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> +			 FEATURE_KBD_REPORT_SIZE, readbuf);
> +		/*
> +		 * Do not return error if handshake is wrong until this is
> +		 * verified to work for all devices.
> +		 */
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.52.0
> 
> 

Re: [PATCH v10 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one

[Benjamin Tissoires]

On Nov 22 2025, Antheas Kapenekakis wrote:
> Some ROG laptops expose multiple interfaces for controlling the
> keyboard/RGB brightness. This creates a name conflict under
> asus::kbd_brightness, where the second device ends up being
> named asus::kbd_brightness_1 and they are both broken.
> 
> Therefore, register a listener to the asus-wmi brightness device
> instead of creating a new one.
> 
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 65 +++++++-----------------------------------
>  1 file changed, 10 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 461b0f8efdfb..475e34187326 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -27,7 +27,6 @@
>  #include <linux/hid.h>
>  #include <linux/module.h>
>  #include <linux/platform_data/x86/asus-wmi.h>
> -#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
>  #include <linux/input/mt.h>
>  #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
>  #include <linux/power_supply.h>
> @@ -103,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
>  
>  struct asus_kbd_leds {
> -	struct led_classdev cdev;
> +	struct asus_hid_listener listener;
>  	struct hid_device *hdev;
>  	struct work_struct work;
>  	unsigned int brightness;
> @@ -493,11 +492,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
>  	spin_unlock_irqrestore(&led->lock, flags);
>  }
>  
> -static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> -				   enum led_brightness brightness)
> +static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
> +				   int brightness)
>  {
> -	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> -						 cdev);
> +	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
> +						 listener);
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&led->lock, flags);
> @@ -507,20 +506,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
>  	asus_schedule_work(led);
>  }
>  
> -static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> -{
> -	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> -						 cdev);
> -	enum led_brightness brightness;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&led->lock, flags);
> -	brightness = led->brightness;
> -	spin_unlock_irqrestore(&led->lock, flags);
> -
> -	return brightness;
> -}
> -
>  static void asus_kbd_backlight_work(struct work_struct *work)
>  {
>  	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> @@ -537,34 +522,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
>  		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>  }
>  
> -/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
> - * precedence. We only activate HID-based backlight control when the
> - * WMI control is not available.
> - */
> -static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> -{
> -	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> -	u32 value;
> -	int ret;
> -
> -	if (!IS_ENABLED(CONFIG_ASUS_WMI))
> -		return false;
> -
> -	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
> -			dmi_check_system(asus_use_hid_led_dmi_ids)) {
> -		hid_info(hdev, "using HID for asus::kbd_backlight\n");
> -		return false;
> -	}
> -
> -	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
> -				       ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
> -	hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
> -	if (ret)
> -		return false;
> -
> -	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> -}
> -
>  /*
>   * We don't care about any other part of the string except the version section.
>   * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
> @@ -709,14 +666,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	drvdata->kbd_backlight->removed = false;
>  	drvdata->kbd_backlight->brightness = 0;
>  	drvdata->kbd_backlight->hdev = hdev;
> -	drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
> -	drvdata->kbd_backlight->cdev.max_brightness = 3;
> -	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
> -	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
> +	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
>  	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
>  	spin_lock_init(&drvdata->kbd_backlight->lock);
>  
> -	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
> +	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
>  	if (ret < 0) {
>  		/* No need to have this still around */
>  		devm_kfree(&hdev->dev, drvdata->kbd_backlight);

Unrelated to your patch (strictly speaking), but still worth fixing
(here on in a follow up patch):
Here we free drvdata->kbd_backlight we just allocated before without
resetting the value to 0.

However, unless I'm mistaken, this function asus_kbd_register_leds() is
called from asus_input_configured() and we simply put a warning and do
not report the return value later on. Which means that further checks
against drvdata->kbd_backlight (like in asus_resume()) will consider the
value initialized and might trigger a use after free.

With or without that:
Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>

Cheers,
Benjamin

> @@ -1105,7 +1059,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
>  
>  	if (drvdata->kbd_backlight) {
>  		const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
> -				drvdata->kbd_backlight->cdev.brightness };
> +				drvdata->kbd_backlight->brightness };
>  		ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>  		if (ret < 0) {
>  			hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> @@ -1231,7 +1185,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	}
>  
>  	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> -	    !asus_kbd_wmi_led_control_present(hdev) &&
>  	    asus_kbd_register_leds(hdev))
>  		hid_warn(hdev, "Failed to initialize backlight.\n");
>  
> @@ -1278,6 +1231,8 @@ static void asus_remove(struct hid_device *hdev)
>  	unsigned long flags;
>  
>  	if (drvdata->kbd_backlight) {
> +		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +
>  		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
>  		drvdata->kbd_backlight->removed = true;
>  		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
> -- 
> 2.52.0
> 
> 

Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler

[Benjamin Tissoires]

On Dec 01 2025, Antheas Kapenekakis wrote:
> On Mon, 1 Dec 2025 at 10:52, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Mon, 1 Dec 2025, Antheas Kapenekakis wrote:
> > > On Wed, 26 Nov 2025 at 21:39, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > > On Wed, 26 Nov 2025 at 21:23, Denis Benato <benato.denis96@gmail.com> wrote:
> > > > > On 11/22/25 12:00, Antheas Kapenekakis wrote:
> > > > > > The keyboard brightness control of Asus WMI keyboards is handled in
> > > > > > kernel, which leads to the shortcut going from brightness 0, to 1,
> > > > > > to 2, and 3.
> > > > > >
> > > > > > However, for HID keyboards it is exposed as a key and handled by the
> > > > > > user's desktop environment. For the toggle button, this means that
> > > > > > brightness control becomes on/off. In addition, in the absence of a
> > > > > > DE, the keyboard brightness does not work.
> > > > > >
> > > > > > Therefore, expose an event handler for the keyboard brightness control
> > > > > > which can then be used by hid-asus. Since this handler is called from
> > > > > > an interrupt context, defer the actual work to a workqueue.
> > > > > >
> > > > > > In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> > > > > > for maximum brightness since it is shared between hid-asus/asus-wmi.
> > > > > >
> > > > > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > > > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > > > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > > > > ---
> > > > > >  drivers/platform/x86/asus-wmi.c            | 46 +++++++++++++++++++---
> > > > > >  include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> > > > > >  2 files changed, 54 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > > > > index 84cde34ab6a8..a69464e45ca4 100644
> > > > > > --- a/drivers/platform/x86/asus-wmi.c
> > > > > > +++ b/drivers/platform/x86/asus-wmi.c
> > > > > > @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> > > > > >       }
> > > > > >  }
> > > > > >
> > > > > > +/*
> > > > > > + * This function is called from hid-asus to inform asus-wmi of brightness
> > > > > > + * changes initiated by the keyboard backlight keys.
> > > > > > + */
> > > > > > +int asus_hid_event(enum asus_hid_event event)
> > > > > > +{
> > > > > > +     struct asus_wmi *asus;
> > > > > > +     int brightness;
> > > > > > +
> > > > > > +     guard(spinlock_irqsave)(&asus_ref.lock);
> > > > > > +     asus = asus_ref.asus;
> > > > > > +     if (!asus || !asus->kbd_led_registered)
> > > > > > +             return -EBUSY;
> > > > > > +
> > > > > > +     brightness = asus->kbd_led_wk;
> > > > > > +
> > > > > > +     switch (event) {
> > > > > > +     case ASUS_EV_BRTUP:
> > > > > > +             brightness += 1;
> > > > > > +             break;
> > > > > > +     case ASUS_EV_BRTDOWN:
> > > > > > +             brightness -= 1;
> > > > > > +             break;
> > > > > > +     case ASUS_EV_BRTTOGGLE:
> > > > > > +             if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > > > > > +                     brightness = 0;
> > > > > > +             else
> > > > > > +                     brightness += 1;
> > > > > > +             break;
> > > > > > +     }
> > > > > > +
> > > > > > +     asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > > > > +     asus->kbd_led_notify = true;
> > > > > > +     queue_work(asus->led_workqueue, &asus->kbd_led_work);
> > > > > > +     return 0;
> > > > > > +}
> > > > > > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > > > > > +
> > > > > >  /*
> > > > > >   * These functions actually update the LED's, and are called from a
> > > > > LEDs as opposed to LED's?
> > > >
> > > > I agree with you, but the author of that line probably wouldn't -
> > > > depends on author dialect and both are usually correct.
> > > >
> > > > When making acronyms plural, adding a ' is usually accepted as
> > > > correct. But this is not added as part of this series, so you can do a
> > > > reword commit if you wish
> > > >
> > > > Antheas
> > >
> > > Hm, perhaps this was not clear but the part you commented on was not
> > > part of the patch/this series. Do you want to finish reviewing this
> > > patch and add a rev-by?
> > >
> > > @Ilpo: with 6.18 releasing yesterday, what is the status on this? is
> > > it for 6.20? Hans commented on patch 5
> >
> > It will have to wait to the 6.20 cycle as it's cross-subsystem series so
> > there are other maintainers involved than just me.
> 
> Unfortunate. Jiri is cc'd on this series and we had around 2 weeks
> where the code was frozen. Was it my responsibility to ask for acks so
> it can go through x86?

Sorry for the current situation. We were following the series without
much involvment as there were still changes requested here and there.

Anyway, we (Jiri and I) are fine for this series to be merged entirely
through the drivers-platform-x86 tree. The only HID changes are solely
located into hid-asus.c, so we can punt to you the patches for this
cycle if there is anything happening until this gets merge into Linus
tree and we can go back to normal.

Cheers,
Benjamin

> 
> > --
> >  i.
> >
> > > Thanks,
> > > Antheas
> > >
> > > > > >   * workqueue. By doing this as separate work rather than when the LED
> > > > > > @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > > > > >  {
> > > > > >       struct asus_hid_listener *listener;
> > > > > >       struct asus_wmi *asus;
> > > > > > -     int max_level;
> > > > > >
> > > > > >       asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> > > > > > -     max_level = asus->kbd_led.max_brightness;
> > > > > >
> > > > > >       scoped_guard(spinlock_irqsave, &asus_ref.lock)
> > > > > > -             asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > > > > > +             asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
> > > > > >
> > > > > >       if (asus->kbd_led_avail)
> > > > > >               kbd_led_update(asus);
> > > > > > @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > > > > >       asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > > > > >       asus->kbd_led.brightness_set = kbd_led_set;
> > > > > >       asus->kbd_led.brightness_get = kbd_led_get;
> > > > > > -     asus->kbd_led.max_brightness = 3;
> > > > > > +     asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> > > > > >       asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> > > > > >       INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
> > > > > >
> > > > > > @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> > > > > >               return;
> > > > > >       }
> > > > > >       if (code == NOTIFY_KBD_BRTTOGGLE) {
> > > > > > -             if (led_value == asus->kbd_led.max_brightness)
> > > > > > +             if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> > > > > >                       kbd_led_set_by_kbd(asus, 0);
> > > > > >               else
> > > > > >                       kbd_led_set_by_kbd(asus, led_value + 1);
> > > > > > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > > > > > index d347cffd05d5..7b872b5d0960 100644
> > > > > > --- a/include/linux/platform_data/x86/asus-wmi.h
> > > > > > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > > > > > @@ -178,6 +178,14 @@ struct asus_hid_listener {
> > > > > >       void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > > > > >  };
> > > > > >
> > > > > > +enum asus_hid_event {
> > > > > > +     ASUS_EV_BRTUP,
> > > > > > +     ASUS_EV_BRTDOWN,
> > > > > > +     ASUS_EV_BRTTOGGLE,
> > > > > > +};
> > > > > > +
> > > > > > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > > > > > +
> > > > > >  #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > > > > >  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > > > > >  void set_ally_mcu_powersave(bool enabled);
> > > > > > @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> > > > > >  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> > > > > >  int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > > > > >  void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > > > > > +int asus_hid_event(enum asus_hid_event event);
> > > > > >  #else
> > > > > >  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > > > > >  {
> > > > > > @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > > > > >  static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > > > > >  {
> > > > > >  }
> > > > > > +static inline int asus_hid_event(enum asus_hid_event event)
> > > > > > +{
> > > > > > +     return -ENODEV;
> > > > > > +}
> > > > > >  #endif
> > > > > >
> > > > > >  #endif       /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> > > > >
> > >
> >
> 

Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling

[Benjamin Tissoires]

On Nov 22 2025, Antheas Kapenekakis wrote:
> 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.

For the HID part:
Acked-by: Benjamin Tissoires <bentiss@kernel.org>

Again, as mentioned in one of the other patch. Feel free to take this
into the platform-x86 tree.

Cheers,
Benjamin

> 
> ---
> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> 
> Changes since V9:
>   - No functional changes
>   - Rebase to review-ilpo-next
>   - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
>     "remove unused keyboard backlight quirk" + imports
>     Dismiss Luke's review as this patch diverged
>   - Reword paragraph in "Add support for multiple kbd led handlers" to be
>     more verbose
>   - Use kfree in fortify patch
>   - Fix minor style quirks from --nonstict checkpatch run
> 
> Changes since V8:
>   - No functional changes
>   - Move legacy init patch to second, modify first patch so that their
>     diff is minimized
>   - Split "prevent binding to all HID devices on ROG" into two patches:
>     - moving backlight initialization into probe
>     - early exit to skip ->init check and rename
>     - Remove skipping vendor fixups for non-vendor devices. It is not possible
>       to read usages before the report fixups are applied, so it did not work
>   - In that patch, reword a comment to be single line and make is_vendor a bool
>   - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
>     has drifted too far since he reviewed/tested it.
> 
> Changes since V7:
>   - Readd legacy init quirk for Dennis
>   - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
>   - Fix warning due to enum_backlight receiving negative values
> 
> Changes since V6:
>   - Split initialization refactor into three patches, update commit text
>     to be clearer in what it does
>   - Replace spinlock accesses with guard and scoped guard in all patches
>   - Add missing includes mentioned by Ilpo
>   - Reflow, tweak comment in prevent binding to all HID devices on ROG
>   - Replace asus_ref.asus with local reference in all patches
>   - Add missing kernel doc comments
>   - Other minor nits from Ilpo
>   - User reported warning due to scheduling work while holding a spinlock.
>     Restructure patch for multiple handlers to limit when spinlock is held to
>     variable access only. In parallel, setup a workqueue to handle registration
>     of led device and setting brightness. This is required as registering the
>     led device triggers kbd_led_get which needs to hold the spinlock to
>     protect the led_wk value. The workqueue is also required for the hid
>     event passthrough to avoid scheduling work while holding the spinlock.
>     Apply the workqueue to wmi brightness buttons as well, as that was
>     omitted before this series and WMI access was performed.
>   - On "HID: asus: prevent binding to all HID devices on ROG", rename
>     quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
>     This allows other quirks to apply (applies quirk that fixes keyboard
>     being named as a pointer device).
> 
> Changes since V5:
>   - It's been a long time
>   - Remove addition of RGB as that had some comments I need to work on
>   - Remove folio patch (already merged)
>   - Remove legacy fix patch 11 from V4. There is a small chance that
>     without this patch, some old NKEY keyboards might not respond to
>     RGB commands according to Luke, but the kernel driver does not do
>     RGB currently. The 0x5d init is done by Armoury crate software in
>     Windows. If an issue is found, we can re-add it or just remove patches
>     1/2 before merging. However, init could use the cleanup.
> 
> Changes since V4:
>   - Fix KConfig (reported by kernel robot)
>   - Fix Ilpo's nits, if I missed anything lmk
> 
> Changes since V3:
>   - Add initializer for 0x5d for old NKEY keyboards until it is verified
>     that it is not needed for their media keys to function.
>   - Cover init in asus-wmi with spinlock as per Hans
>   - If asus-wmi registers WMI handler with brightness, init the brightness
>     in USB Asus keyboards, per Hans.
>   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
>   - Fix oops when unregistering asus-wmi by moving unregister outside of
>     the spin lock (but after the asus reference is set to null)
> 
> Changes since V2:
>   - Check lazy init succeds in asus-wmi before setting register variable
>   - make explicit check in asus_hid_register_listener for listener existing
>     to avoid re-init
>   - rename asus_brt to asus_hid in most places and harmonize everything
>   - switch to a spinlock instead of a mutex to avoid kernel ooops
>   - fixup hid device quirks to avoid multiple RGB devices while still exposing
>     all input vendor devices. This includes moving rgb init to probe
>     instead of the input_configured callbacks.
>   - Remove fan key (during retest it appears to be 0xae that is already
>     supported by hid-asus)
>   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
>   - removes fds from userspace and breaks backlight functionality. All
>   - current mainline drivers do not support backlight hotplugging, so most
>     userspace software (e.g., KDE, UPower) is built with that assumption.
>     For the Ally, since it disconnects its controller during sleep, this
>     caused the backlight slider to not work in KDE.
> 
> Changes since V1:
>   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
>   - Fix ifdef else having an invalid signature (reported by kernel robot)
>   - Restore input arguments to init and keyboard function so they can
>     be re-used for RGB controls.
>   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
>     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
>     keyboard rename into it.
>   - Unregister brightness listener before removing work queue to avoid
>     a race condition causing corruption
>   - Remove spurious mutex unlock in asus_brt_event
>   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
>     relocking the mutex and causing a deadlock when unregistering leds
>   - Add extra check during unregistering to avoid calling unregister when
>     no led device is registered.
>   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
>     the driver to create 4 RGB handlers per device. I also suspect some
>     extra events sneak through (KDE had the @@@@@@).
> 
> Antheas Kapenekakis (11):
>   HID: asus: simplify RGB init sequence
>   HID: asus: initialize additional endpoints only for legacy devices
>   HID: asus: use same report_id in response
>   HID: asus: fortify keyboard handshake
>   HID: asus: move vendor initialization to probe
>   HID: asus: early return for ROG devices
>   platform/x86: asus-wmi: Add support for multiple kbd led handlers
>   HID: asus: listen to the asus-wmi brightness device instead of
>     creating one
>   platform/x86: asus-wmi: remove unused keyboard backlight quirk
>   platform/x86: asus-wmi: add keyboard brightness event handler
>   HID: asus: add support for the asus-wmi brightness handler
> 
>  drivers/hid/hid-asus.c                        | 205 ++++++++--------
>  drivers/platform/x86/asus-wmi.c               | 223 +++++++++++++++---
>  .../platform_data/x86/asus-wmi-leds-ids.h     |  50 ----
>  include/linux/platform_data/x86/asus-wmi.h    |  28 +++
>  4 files changed, 322 insertions(+), 184 deletions(-)
>  delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> 
> 
> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> -- 
> 2.52.0
> 
> 

Re: [PATCH v10 07/11] platform/x86: asus-wmi: Add support for multiple kbd led handlers

[Ilpo Järvinen]

On Sat, 22 Nov 2025, Antheas Kapenekakis wrote:

> Some devices, such as the Z13 have multiple Aura devices connected
> to them by USB. In addition, they might have a WMI interface for
> RGB. In Windows, Armoury Crate exposes a unified brightness slider
> for all of them, with 3 brightness levels.
> 
> Therefore, to be synergistic in Linux, and support existing tooling
> such as UPower, allow adding listeners to the RGB device of the WMI
> interface. If WMI does not exist, lazy initialize the interface.
> 
> Since hid-asus and asus-wmi can both interact with the led objects
> including from an atomic context, protect the brightness access with a
> spinlock and update the values from a workqueue. Use this workqueue to
> also process WMI keyboard events, so they are handled asynchronously.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 183 ++++++++++++++++++---
>  include/linux/platform_data/x86/asus-wmi.h |  15 ++
>  2 files changed, 173 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 64cfc0bf98dd..84cde34ab6a8 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -31,13 +31,13 @@
>  #include <linux/pci.h>
>  #include <linux/pci_hotplug.h>
>  #include <linux/platform_data/x86/asus-wmi.h>
> -#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
>  #include <linux/platform_device.h>
>  #include <linux/platform_profile.h>
>  #include <linux/power_supply.h>
>  #include <linux/rfkill.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> +#include <linux/spinlock.h>
>  #include <linux/types.h>
>  #include <linux/units.h>
>  
> @@ -256,6 +256,9 @@ struct asus_wmi {
>  	int tpd_led_wk;
>  	struct led_classdev kbd_led;
>  	int kbd_led_wk;
> +	bool kbd_led_notify;
> +	bool kbd_led_avail;
> +	bool kbd_led_registered;
>  	struct led_classdev lightbar_led;
>  	int lightbar_led_wk;
>  	struct led_classdev micmute_led;
> @@ -264,6 +267,7 @@ struct asus_wmi {
>  	struct work_struct tpd_led_work;
>  	struct work_struct wlan_led_work;
>  	struct work_struct lightbar_led_work;
> +	struct work_struct kbd_led_work;
>  
>  	struct asus_rfkill wlan;
>  	struct asus_rfkill bluetooth;
> @@ -1615,6 +1619,106 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
>  
>  /* LEDs ***********************************************************************/
>  
> +struct asus_hid_ref {
> +	struct list_head listeners;
> +	struct asus_wmi *asus;
> +	/* Protects concurrent access from hid-asus and asus-wmi to leds */
> +	spinlock_t lock;
> +};
> +
> +static struct asus_hid_ref asus_ref = {
> +	.listeners = LIST_HEAD_INIT(asus_ref.listeners),
> +	.asus = NULL,
> +	/*
> +	 * Protects .asus, .asus.kbd_led_{wk,notify}, and .listener refs. Other
> +	 * asus variables are read-only after .asus is set.
> +	 *
> +	 * The led cdev device is not protected because it calls backlight_get
> +	 * during initialization, which would result in a nested lock attempt.
> +	 *
> +	 * The led cdev is safe to access without a lock because if
> +	 * kbd_led_avail is true it is initialized before .asus is set and never
> +	 * changed until .asus is dropped. If kbd_led_avail is false, the led

It's actually not as straightforward when it comes to visibility of memory 
writes but you seem to end up being safe because you take the spinlock in 
between so that should ensure kbd_led_avail gets set before .asus is 
assigned. If that wouldn't be there, you'd have needed a memory barrier to 
ensure the writes are guaranteed to be visible in the desired ordering.

> +	 * cdev is registered by the workqueue, which is single-threaded and
> +	 * cancelled before asus-wmi would access the led cdev to unregister it.
> +	 *
> +	 * A spinlock is used, because the protected variables can be accessed
> +	 * from an IRQ context from asus-hid.
> +	 */
> +	.lock = __SPIN_LOCK_UNLOCKED(asus_ref.lock),
> +};
> +
> +/*
> + * Allows registering hid-asus listeners that want to be notified of
> + * keyboard backlight changes.
> + */
> +int asus_hid_register_listener(struct asus_hid_listener *bdev)
> +{
> +	struct asus_wmi *asus;
> +
> +	guard(spinlock_irqsave)(&asus_ref.lock);
> +	list_add_tail(&bdev->list, &asus_ref.listeners);
> +	asus = asus_ref.asus;
> +	if (asus)
> +		queue_work(asus->led_workqueue, &asus->kbd_led_work);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_register_listener);
> +
> +/*
> + * Allows unregistering hid-asus listeners that were added with
> + * asus_hid_register_listener().
> + */
> +void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> +{
> +	guard(spinlock_irqsave)(&asus_ref.lock);
> +	list_del(&bdev->list);
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
> +
> +static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
> +
> +static void kbd_led_update_all(struct work_struct *work)
> +{
> +	struct asus_wmi *asus;
> +	bool registered, notify;
> +	int ret, value;
> +
> +	asus = container_of(work, struct asus_wmi, kbd_led_work);
> +
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
> +		registered = asus->kbd_led_registered;
> +		value = asus->kbd_led_wk;
> +		notify = asus->kbd_led_notify;
> +	}
> +
> +	if (!registered) {
> +		/*
> +		 * This workqueue runs under asus-wmi, which means probe has
> +		 * completed and asus-wmi will keep running until it finishes.
> +		 * Therefore, we can safely register the LED without holding
> +		 * a spinlock.
> +		 */
> +		ret = devm_led_classdev_register(&asus->platform_device->dev,
> +						 &asus->kbd_led);
> +		if (!ret) {
> +			scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +				asus->kbd_led_registered = true;
> +		} else {
> +			pr_warn("Failed to register keyboard backlight LED: %d\n", ret);
> +			return;
> +		}
> +	}
> +
> +	if (value >= 0)
> +		do_kbd_led_set(&asus->kbd_led, value);
> +	if (notify) {
> +		scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +			asus->kbd_led_notify = false;
> +		led_classdev_notify_brightness_hw_changed(&asus->kbd_led, value);
> +	}
> +}
> +
>  /*
>   * These functions actually update the LED's, and are called from a
>   * workqueue. By doing this as separate work rather than when the LED
> @@ -1661,7 +1765,8 @@ static void kbd_led_update(struct asus_wmi *asus)
>  {
>  	int ctrl_param = 0;
>  
> -	ctrl_param = 0x80 | (asus->kbd_led_wk & 0x7F);
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +		ctrl_param = 0x80 | (asus->kbd_led_wk & 0x7F);

Unrelated to this series, it would be nice to name that 0x80 and the 0x7F 
field.

-- 
 i.

>  	asus_wmi_set_devstate(ASUS_WMI_DEVID_KBD_BACKLIGHT, ctrl_param, NULL);
>  }
>  
> @@ -1694,14 +1799,23 @@ static int kbd_led_read(struct asus_wmi *asus, int *level, int *env)
>  
>  static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
>  {
> +	struct asus_hid_listener *listener;
>  	struct asus_wmi *asus;
>  	int max_level;
>  
>  	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
>  	max_level = asus->kbd_led.max_brightness;
>  
> -	asus->kbd_led_wk = clamp_val(value, 0, max_level);
> -	kbd_led_update(asus);
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +		asus->kbd_led_wk = clamp_val(value, 0, max_level);
> +
> +	if (asus->kbd_led_avail)
> +		kbd_led_update(asus);
> +
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
> +		list_for_each_entry(listener, &asus_ref.listeners, list)
> +			listener->brightness_set(listener, asus->kbd_led_wk);
> +	}
>  }
>  
>  static void kbd_led_set(struct led_classdev *led_cdev,
> @@ -1716,10 +1830,11 @@ static void kbd_led_set(struct led_classdev *led_cdev,
>  
>  static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
>  {
> -	struct led_classdev *led_cdev = &asus->kbd_led;
> -
> -	do_kbd_led_set(led_cdev, value);
> -	led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
> +		asus->kbd_led_wk = value;
> +		asus->kbd_led_notify = true;
> +	}
> +	queue_work(asus->led_workqueue, &asus->kbd_led_work);
>  }
>  
>  static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
> @@ -1729,10 +1844,18 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
>  
>  	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
>  
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
> +		if (!asus->kbd_led_avail)
> +			return asus->kbd_led_wk;
> +	}
> +
>  	retval = kbd_led_read(asus, &value, NULL);
>  	if (retval < 0)
>  		return retval;
>  
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +		asus->kbd_led_wk = value;
> +
>  	return value;
>  }
>  
> @@ -1844,7 +1967,9 @@ static int camera_led_set(struct led_classdev *led_cdev,
>  
>  static void asus_wmi_led_exit(struct asus_wmi *asus)
>  {
> -	led_classdev_unregister(&asus->kbd_led);
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +		asus_ref.asus = NULL;
> +
>  	led_classdev_unregister(&asus->tpd_led);
>  	led_classdev_unregister(&asus->wlan_led);
>  	led_classdev_unregister(&asus->lightbar_led);
> @@ -1882,22 +2007,26 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
>  			goto error;
>  	}
>  
> -	if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
> -		pr_info("using asus-wmi for asus::kbd_backlight\n");
> -		asus->kbd_led_wk = led_val;
> -		asus->kbd_led.name = "asus::kbd_backlight";
> -		asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> -		asus->kbd_led.brightness_set = kbd_led_set;
> -		asus->kbd_led.brightness_get = kbd_led_get;
> -		asus->kbd_led.max_brightness = 3;
> +	asus->kbd_led.name = "asus::kbd_backlight";
> +	asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> +	asus->kbd_led.brightness_set = kbd_led_set;
> +	asus->kbd_led.brightness_get = kbd_led_get;
> +	asus->kbd_led.max_brightness = 3;
> +	asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> +	INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
>  
> +	if (asus->kbd_led_avail) {
> +		asus->kbd_led_wk = led_val;
>  		if (num_rgb_groups != 0)
>  			asus->kbd_led.groups = kbd_rgb_mode_groups;
> +	} else {
> +		asus->kbd_led_wk = -1;
> +	}
>  
> -		rv = led_classdev_register(&asus->platform_device->dev,
> -					   &asus->kbd_led);
> -		if (rv)
> -			goto error;
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
> +		asus_ref.asus = asus;
> +		if (asus->kbd_led_avail || !list_empty(&asus_ref.listeners))
> +			queue_work(asus->led_workqueue, &asus->kbd_led_work);
>  	}
>  
>  	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
> @@ -4372,6 +4501,7 @@ static int asus_wmi_get_event_code(union acpi_object *obj)
>  
>  static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
>  {
> +	enum led_brightness led_value;
>  	unsigned int key_value = 1;
>  	bool autorelease = 1;
>  
> @@ -4388,19 +4518,22 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
>  		return;
>  	}
>  
> +	scoped_guard(spinlock_irqsave, &asus_ref.lock)
> +		led_value = asus->kbd_led_wk;
> +
>  	if (code == NOTIFY_KBD_BRTUP) {
> -		kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
> +		kbd_led_set_by_kbd(asus, led_value + 1);
>  		return;
>  	}
>  	if (code == NOTIFY_KBD_BRTDWN) {
> -		kbd_led_set_by_kbd(asus, asus->kbd_led_wk - 1);
> +		kbd_led_set_by_kbd(asus, led_value - 1);
>  		return;
>  	}
>  	if (code == NOTIFY_KBD_BRTTOGGLE) {
> -		if (asus->kbd_led_wk == asus->kbd_led.max_brightness)
> +		if (led_value == asus->kbd_led.max_brightness)
>  			kbd_led_set_by_kbd(asus, 0);
>  		else
> -			kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
> +			kbd_led_set_by_kbd(asus, led_value + 1);
>  		return;
>  	}
>  
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 419491d4abca..d347cffd05d5 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -172,12 +172,20 @@ enum asus_ally_mcu_hack {
>  	ASUS_WMI_ALLY_MCU_HACK_DISABLED,
>  };
>  
> +/* Used to notify hid-asus when asus-wmi changes keyboard backlight */
> +struct asus_hid_listener {
> +	struct list_head list;
> +	void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> +};
> +
>  #if IS_REACHABLE(CONFIG_ASUS_WMI)
>  void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
>  void set_ally_mcu_powersave(bool enabled);
>  int asus_wmi_get_devstate_dsts(u32 dev_id, u32 *retval);
>  int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
>  int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> +int asus_hid_register_listener(struct asus_hid_listener *cdev);
> +void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
>  #else
>  static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
>  {
> @@ -198,6 +206,13 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
>  {
>  	return -ENODEV;
>  }
> +static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> +{
> +	return -ENODEV;
> +}
> +static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> +{
> +}
>  #endif
>  
>  #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
> 

Re: [PATCH v10 05/11] HID: asus: move vendor initialization to probe

[Ilpo Järvinen]

On Sat, 22 Nov 2025, Antheas Kapenekakis wrote:

> ROG NKEY devices have multiple HID endpoints, around 3-4. One of those
> endpoints has a usage page of 0xff31, and is the one that emits keyboard
> shortcuts and controls RGB/backlight. Currently, this driver places
> the usage page check under asus_input_mapping and then inits backlight
> in asus_input_configured which is unnecessarily complicated and prevents
> probe from performing customizations on the vendor endpoint.
> 
> Simplify the logic by introducing an is_vendor variable into probe that
> checks for usage page 0xff31. Then, use this variable to move backlight
> initialization into probe instead of asus_input_configured, and remove
> the backlight check from asus_input_mapping.

In the changelogs, please add () after any name that refers to a C 
function so the reader immediately knows you're talking about a function.

> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 2a412e10f916..faac971794c0 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -48,6 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define T100CHI_MOUSE_REPORT_ID 0x06
>  #define FEATURE_REPORT_ID 0x0d
>  #define INPUT_REPORT_ID 0x5d
> +#define HID_USAGE_PAGE_VENDOR 0xff310000
>  #define FEATURE_KBD_REPORT_ID 0x5a
>  #define FEATURE_KBD_REPORT_SIZE 64
>  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> @@ -127,7 +128,6 @@ struct asus_drvdata {
>  	struct input_dev *tp_kbd_input;
>  	struct asus_kbd_leds *kbd_backlight;
>  	const struct asus_touchpad_info *tp;
> -	bool enable_backlight;
>  	struct power_supply *battery;
>  	struct power_supply_desc battery_desc;
>  	int battery_capacity;
> @@ -318,7 +318,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
>  static int asus_event(struct hid_device *hdev, struct hid_field *field,
>  		      struct hid_usage *usage, __s32 value)
>  {
> -	if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
> +	if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
>  	    (usage->hid & HID_USAGE) != 0x00 &&
>  	    (usage->hid & HID_USAGE) != 0xff && !usage->type) {
>  		hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
> @@ -941,11 +941,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>  
>  	drvdata->input = input;
>  
> -	if (drvdata->enable_backlight &&
> -	    !asus_kbd_wmi_led_control_present(hdev) &&
> -	    asus_kbd_register_leds(hdev))
> -		hid_warn(hdev, "Failed to initialize backlight.\n");
> -
>  	return 0;
>  }
>  
> @@ -1018,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
>  			return -1;
>  		}
>  
> -		/*
> -		 * Check and enable backlight only on devices with UsagePage ==
> -		 * 0xff31 to avoid initializing the keyboard firmware multiple
> -		 * times on devices with multiple HID descriptors but same
> -		 * PID/VID.
> -		 */
> -		if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
> -			drvdata->enable_backlight = true;
> -
>  		set_bit(EV_REP, hi->input->evbit);
>  		return 1;
>  	}
> @@ -1143,8 +1129,11 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
>  
>  static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
> -	int ret;
> +	struct hid_report_enum *rep_enum;
>  	struct asus_drvdata *drvdata;
> +	struct hid_report *rep;
> +	bool is_vendor = false;
> +	int ret;
>  
>  	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
>  	if (drvdata == NULL) {
> @@ -1228,12 +1217,24 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		return ret;
>  	}
>  
> +	/* Check for vendor for RGB init and handle generic devices properly. */
> +	rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
> +	list_for_each_entry(rep, &rep_enum->report_list, list) {
> +		if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
> +			is_vendor = true;
> +	}
> +
>  	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>  	if (ret) {
>  		hid_err(hdev, "Asus hw start failed: %d\n", ret);
>  		return ret;
>  	}
>  
> +	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> +	    !asus_kbd_wmi_led_control_present(hdev) &&
> +	    asus_kbd_register_leds(hdev))
> +		hid_warn(hdev, "Failed to initialize backlight.\n");
> +
>  	/*
>  	 * Check that input registration succeeded. Checking that
>  	 * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
> 

-- 
 i.

Re: [PATCH v10 05/11] HID: asus: move vendor initialization to probe

[Antheas Kapenekakis]

On Mon, 8 Dec 2025 at 18:11, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Sat, 22 Nov 2025, Antheas Kapenekakis wrote:
>
> > ROG NKEY devices have multiple HID endpoints, around 3-4. One of those
> > endpoints has a usage page of 0xff31, and is the one that emits keyboard
> > shortcuts and controls RGB/backlight. Currently, this driver places
> > the usage page check under asus_input_mapping and then inits backlight
> > in asus_input_configured which is unnecessarily complicated and prevents
> > probe from performing customizations on the vendor endpoint.
> >
> > Simplify the logic by introducing an is_vendor variable into probe that
> > checks for usage page 0xff31. Then, use this variable to move backlight
> > initialization into probe instead of asus_input_configured, and remove
> > the backlight check from asus_input_mapping.
>
> In the changelogs, please add () after any name that refers to a C
> function so the reader immediately knows you're talking about a function.

Sure. What should be my timeline for fixing this?

I assume we missed the merge window. If not, I can tweak the series
and send it later today

Antheas

> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >  drivers/hid/hid-asus.c | 35 ++++++++++++++++++-----------------
> >  1 file changed, 18 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 2a412e10f916..faac971794c0 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -48,6 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> >  #define T100CHI_MOUSE_REPORT_ID 0x06
> >  #define FEATURE_REPORT_ID 0x0d
> >  #define INPUT_REPORT_ID 0x5d
> > +#define HID_USAGE_PAGE_VENDOR 0xff310000
> >  #define FEATURE_KBD_REPORT_ID 0x5a
> >  #define FEATURE_KBD_REPORT_SIZE 64
> >  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> > @@ -127,7 +128,6 @@ struct asus_drvdata {
> >       struct input_dev *tp_kbd_input;
> >       struct asus_kbd_leds *kbd_backlight;
> >       const struct asus_touchpad_info *tp;
> > -     bool enable_backlight;
> >       struct power_supply *battery;
> >       struct power_supply_desc battery_desc;
> >       int battery_capacity;
> > @@ -318,7 +318,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
> >  static int asus_event(struct hid_device *hdev, struct hid_field *field,
> >                     struct hid_usage *usage, __s32 value)
> >  {
> > -     if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
> > +     if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
> >           (usage->hid & HID_USAGE) != 0x00 &&
> >           (usage->hid & HID_USAGE) != 0xff && !usage->type) {
> >               hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
> > @@ -941,11 +941,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> >
> >       drvdata->input = input;
> >
> > -     if (drvdata->enable_backlight &&
> > -         !asus_kbd_wmi_led_control_present(hdev) &&
> > -         asus_kbd_register_leds(hdev))
> > -             hid_warn(hdev, "Failed to initialize backlight.\n");
> > -
> >       return 0;
> >  }
> >
> > @@ -1018,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
> >                       return -1;
> >               }
> >
> > -             /*
> > -              * Check and enable backlight only on devices with UsagePage ==
> > -              * 0xff31 to avoid initializing the keyboard firmware multiple
> > -              * times on devices with multiple HID descriptors but same
> > -              * PID/VID.
> > -              */
> > -             if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
> > -                     drvdata->enable_backlight = true;
> > -
> >               set_bit(EV_REP, hi->input->evbit);
> >               return 1;
> >       }
> > @@ -1143,8 +1129,11 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
> >
> >  static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >  {
> > -     int ret;
> > +     struct hid_report_enum *rep_enum;
> >       struct asus_drvdata *drvdata;
> > +     struct hid_report *rep;
> > +     bool is_vendor = false;
> > +     int ret;
> >
> >       drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
> >       if (drvdata == NULL) {
> > @@ -1228,12 +1217,24 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >               return ret;
> >       }
> >
> > +     /* Check for vendor for RGB init and handle generic devices properly. */
> > +     rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
> > +     list_for_each_entry(rep, &rep_enum->report_list, list) {
> > +             if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
> > +                     is_vendor = true;
> > +     }
> > +
> >       ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> >       if (ret) {
> >               hid_err(hdev, "Asus hw start failed: %d\n", ret);
> >               return ret;
> >       }
> >
> > +     if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> > +         !asus_kbd_wmi_led_control_present(hdev) &&
> > +         asus_kbd_register_leds(hdev))
> > +             hid_warn(hdev, "Failed to initialize backlight.\n");
> > +
> >       /*
> >        * Check that input registration succeeded. Checking that
> >        * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
> >
>
> --
>  i.
>
>

Re: [PATCH v10 05/11] HID: asus: move vendor initialization to probe

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 5989 bytes --]

On Tue, 9 Dec 2025, Antheas Kapenekakis wrote:
> On Mon, 8 Dec 2025 at 18:11, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Sat, 22 Nov 2025, Antheas Kapenekakis wrote:
> >
> > > ROG NKEY devices have multiple HID endpoints, around 3-4. One of those
> > > endpoints has a usage page of 0xff31, and is the one that emits keyboard
> > > shortcuts and controls RGB/backlight. Currently, this driver places
> > > the usage page check under asus_input_mapping and then inits backlight
> > > in asus_input_configured which is unnecessarily complicated and prevents
> > > probe from performing customizations on the vendor endpoint.
> > >
> > > Simplify the logic by introducing an is_vendor variable into probe that
> > > checks for usage page 0xff31. Then, use this variable to move backlight
> > > initialization into probe instead of asus_input_configured, and remove
> > > the backlight check from asus_input_mapping.
> >
> > In the changelogs, please add () after any name that refers to a C
> > function so the reader immediately knows you're talking about a function.
> 
> Sure. What should be my timeline for fixing this?
> 
> I assume we missed the merge window. If not, I can tweak the series
> and send it later today

This is 6.20 material so no big hurry.

--
 i.

> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > >  drivers/hid/hid-asus.c | 35 ++++++++++++++++++-----------------
> > >  1 file changed, 18 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > > index 2a412e10f916..faac971794c0 100644
> > > --- a/drivers/hid/hid-asus.c
> > > +++ b/drivers/hid/hid-asus.c
> > > @@ -48,6 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> > >  #define T100CHI_MOUSE_REPORT_ID 0x06
> > >  #define FEATURE_REPORT_ID 0x0d
> > >  #define INPUT_REPORT_ID 0x5d
> > > +#define HID_USAGE_PAGE_VENDOR 0xff310000
> > >  #define FEATURE_KBD_REPORT_ID 0x5a
> > >  #define FEATURE_KBD_REPORT_SIZE 64
> > >  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> > > @@ -127,7 +128,6 @@ struct asus_drvdata {
> > >       struct input_dev *tp_kbd_input;
> > >       struct asus_kbd_leds *kbd_backlight;
> > >       const struct asus_touchpad_info *tp;
> > > -     bool enable_backlight;
> > >       struct power_supply *battery;
> > >       struct power_supply_desc battery_desc;
> > >       int battery_capacity;
> > > @@ -318,7 +318,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
> > >  static int asus_event(struct hid_device *hdev, struct hid_field *field,
> > >                     struct hid_usage *usage, __s32 value)
> > >  {
> > > -     if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
> > > +     if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
> > >           (usage->hid & HID_USAGE) != 0x00 &&
> > >           (usage->hid & HID_USAGE) != 0xff && !usage->type) {
> > >               hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
> > > @@ -941,11 +941,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> > >
> > >       drvdata->input = input;
> > >
> > > -     if (drvdata->enable_backlight &&
> > > -         !asus_kbd_wmi_led_control_present(hdev) &&
> > > -         asus_kbd_register_leds(hdev))
> > > -             hid_warn(hdev, "Failed to initialize backlight.\n");
> > > -
> > >       return 0;
> > >  }
> > >
> > > @@ -1018,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
> > >                       return -1;
> > >               }
> > >
> > > -             /*
> > > -              * Check and enable backlight only on devices with UsagePage ==
> > > -              * 0xff31 to avoid initializing the keyboard firmware multiple
> > > -              * times on devices with multiple HID descriptors but same
> > > -              * PID/VID.
> > > -              */
> > > -             if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
> > > -                     drvdata->enable_backlight = true;
> > > -
> > >               set_bit(EV_REP, hi->input->evbit);
> > >               return 1;
> > >       }
> > > @@ -1143,8 +1129,11 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
> > >
> > >  static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > >  {
> > > -     int ret;
> > > +     struct hid_report_enum *rep_enum;
> > >       struct asus_drvdata *drvdata;
> > > +     struct hid_report *rep;
> > > +     bool is_vendor = false;
> > > +     int ret;
> > >
> > >       drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
> > >       if (drvdata == NULL) {
> > > @@ -1228,12 +1217,24 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > >               return ret;
> > >       }
> > >
> > > +     /* Check for vendor for RGB init and handle generic devices properly. */
> > > +     rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
> > > +     list_for_each_entry(rep, &rep_enum->report_list, list) {
> > > +             if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
> > > +                     is_vendor = true;
> > > +     }
> > > +
> > >       ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > >       if (ret) {
> > >               hid_err(hdev, "Asus hw start failed: %d\n", ret);
> > >               return ret;
> > >       }
> > >
> > > +     if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> > > +         !asus_kbd_wmi_led_control_present(hdev) &&
> > > +         asus_kbd_register_leds(hdev))
> > > +             hid_warn(hdev, "Failed to initialize backlight.\n");
> > > +
> > >       /*
> > >        * Check that input registration succeeded. Checking that
> > >        * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
> > >
> >
> > --
> >  i.
> >
> >
>