[PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[Antheas Kapenekakis]

This is a three part series that does the following: first, it cleans up
the hid-asus driver initialization, preventing excess renames and dmesg
errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
key. Finally, the bigger piece of this series is the unification of the
backlight controls between hid-asus and asus-wmi.

This requires some context. First, some ROG devices expose both WMI and
HID controls for RGB. In addition, some ROG devices (such as the Z13)
have two AURA devices where both have backlight controls (lightbar and
keyboard). Under Windows, Armoury Crate exposes a single brightness control
for all Aura devices.

However, currently in the linux kernel this is not the case, with asus-wmi
and hid-asus relying on a quirk system to figure out which should control
the backlight. But what about the other one? There might be silent
regressions such as part of the RGB of the device not responding properly.

In the Z13, this is the case, with a race condition causing the lightbar
to control the asus::kbd_backlight device most of the time, with the
keyboard being renamed to asus::kbd_backlight_1 and not doing anything
under KDE controls.

Here, we should note that most backlight handlers are hardcoded to check
for backlight once, and for one backlight, during boot, so any other
solution would require a large rewrite of userspace.

Even when brightness controls are fixed, we still have the problem of the
backlight key being on/off when controlled by KDE and 0/33/66/100 when
the device has a WMI keyboard. Ideally, we would like the 0/33/66/100 to
be done under hid as well, regardless of whether the backlight of the
device is controlled by WMI or HID.

Therefore, this is what the third part of this series does. It sets up
asus-wmi to expose accepting listeners for the asus::kbd_backlight device
and being the one that sets it up. Then, it makes hid-asus devices
register a listener there, so that all of them are controlled by
asus::kbd_backlight. Finally, it adds an event handler for keyboard keys,
so that HID led controls are handled by the kernel instead of userspace.
This way, even when userspace is not active the key works, and we get the
desired behavior of 0/33/66/100 across all Aura devices (currently, that
is keyboards, and embedded devices such as lightbars). This results
removing the quirk system as well, eliminating a point of failure.

I tested this on an Asus Z13 2025, and testing by other devices would be
appreciated for sure. This series is designed to be transparent to
userspace behavior-wise compared previous kernels, with all existing
laptops either having the same behavior or being better.

The Z13 keyboard folio RGB controls work beautifully, with KDE led
notifications working and doing 0/33/66/100 as expected. This also happens
with hotplugging, as the lightbar is always available and keeps the
endpoint alive from boot, even if the folio is not connected (a quirk
can be added later if there is a device where this is not the case).

The first two parts of the series can also be merged independently of the
third part, so we can iterate on that more. Perhaps there is a better way
to handle this cohesion,

Oh, by the way Luke, I developed this series with a variant of
your Armoury series merged, and only switched to 6.14-v7 for this
submission. You will be happy to know that there are no conflicts :)
(at least with that version from ~January). Also, please factcheck
my initialization sequence is correct in the 0x5d and 0x5e devices
you added when you made that refactor last year. Are those handshakes
needed?

Antheas Kapenekakis (11):
  HID: asus: refactor init sequence per spec
  HID: asus: cleanup keyboard backlight check
  HID: asus: prevent binding to all HID devices on ROG
  HID: asus: rename keyboard3 to Z13_FOLIO
  HID: asus: add Asus Z13 2025 Fan key
  HID: asus: introduce small delay on Asus Z13 RGB init
  platform/x86: asus-wmi: Add support for multiple kbd RGB 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                     | 220 ++++++++++++---------
 drivers/hid/hid-ids.h                      |   2 +-
 drivers/platform/x86/asus-wmi.c            | 137 +++++++++++--
 include/linux/platform_data/x86/asus-wmi.h |  66 +++----
 4 files changed, 279 insertions(+), 146 deletions(-)


base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1
-- 
2.48.1

[PATCH 01/11] HID: asus: refactor init sequence per spec

[Antheas Kapenekakis]

Currently, asus_kbd_init() uses a reverse engineered init sequence
from Windows, which contains the handshakes from multiple programs.
Keep the main one, which is 0x5a (meant for drivers).

In addition, perform a get_response and check if the response is the
same. To avoid regressions, print an error if the response does not
match instead of rejecting device.

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

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 46e3e42f9eb5f..aa4a481dc4f27 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_REPORT_ID 0x0d
 #define INPUT_REPORT_ID 0x5d
 #define FEATURE_KBD_REPORT_ID 0x5a
-#define FEATURE_KBD_REPORT_SIZE 16
+#define FEATURE_KBD_REPORT_SIZE 64
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
@@ -386,16 +386,43 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
 	return ret;
 }
 
-static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
+static int asus_kbd_init(struct hid_device *hdev)
 {
-	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
-		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
+	/*
+	 * Asus handshake identifying us as a driver (0x5A)
+	 * 0x5A then ASCII for "ASUS Tech.Inc."
+	 * 0x5D is for userspace Windows applications.
+	 *
+	 * The handshake is first sent as a set_report, then retrieved
+	 * from a get_report to verify the response.
+	 */
+	const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20,
+		0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
+	u8 *readbuf;
 	int ret;
 
 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
-	if (ret < 0)
-		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
+	if (ret < 0) {
+		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
+		return ret;
+	}
 
+	readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
+	if (!readbuf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, FEATURE_KBD_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_err(hdev, "Asus handshake returned invalid response: %*ph\n",
+			FEATURE_KBD_REPORT_SIZE, readbuf);
+		// Do not return error if handshake is wrong to avoid regressions
+	}
+
+	kfree(readbuf);
 	return ret;
 }
 
@@ -540,42 +567,25 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	unsigned char kbd_func;
 	int ret;
 
-	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
-		/* Initialize keyboard */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
-		if (ret < 0)
-			return ret;
-
-		/* The LED endpoint is initialised in two HID */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
-		if (ret < 0)
-			return ret;
-
-		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
-		if (ret < 0)
-			return ret;
+	ret = asus_kbd_init(hdev);
+	if (ret < 0)
+		return ret;
 
-		if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
-			ret = asus_kbd_disable_oobe(hdev);
-			if (ret < 0)
-				return ret;
-		}
-	} else {
-		/* Initialize keyboard */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
-		if (ret < 0)
-			return ret;
+	/* Get keyboard functions */
+	ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	if (ret < 0)
+		return ret;
 
-		/* Get keyboard functions */
-		ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
+		ret = asus_kbd_disable_oobe(hdev);
 		if (ret < 0)
 			return ret;
-
-		/* Check for backlight support */
-		if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
-			return -ENODEV;
 	}
 
+	/* Check for backlight support */
+	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
+		return -ENODEV;
+
 	drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
 					      sizeof(struct asus_kbd_leds),
 					      GFP_KERNEL);
-- 
2.48.1

[PATCH 02/11] HID: asus: cleanup keyboard backlight check

[Antheas Kapenekakis]

Currently, asus_kbd_get_functions does a set_report
to query for the keyboard backlight status using a varying
report id. However, it then does get_report with the fixed
id 0x5a. The ids of get_report and set_report should always
match. Therefore, remove the calling argument and force 0x5a.

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

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index aa4a481dc4f27..bcc317057b465 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -427,10 +427,9 @@ static int asus_kbd_init(struct hid_device *hdev)
 }
 
 static int asus_kbd_get_functions(struct hid_device *hdev,
-				  unsigned char *kbd_func,
-				  u8 report_id)
+				  unsigned char *kbd_func)
 {
-	const u8 buf[] = { report_id, 0x05, 0x20, 0x31, 0x00, 0x08 };
+	const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x05, 0x20, 0x31, 0x00, 0x08 };
 	u8 *readbuf;
 	int ret;
 
@@ -572,7 +571,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 		return ret;
 
 	/* Get keyboard functions */
-	ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	ret = asus_kbd_get_functions(hdev, &kbd_func);
 	if (ret < 0)
 		return ret;
 
-- 
2.48.1

[PATCH 03/11] HID: asus: prevent binding to all HID devices on ROG

[Antheas Kapenekakis]

ROG keyboards are HID compliant. We only care about the endpoint that
produces vendor events (e.g., fan mode) and has the keyboard backlight.
If we attach to all the endpoints, we end up generating errors during
probe for two of them because they are missing the ->input attribute
and risk side effects during input fixups.

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

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index bcc317057b465..490a7ea369961 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -84,6 +84,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_MEDION_E1239T		BIT(10)
 #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
 #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
+#define QUIRK_HANDLE_GENERIC		BIT(13)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -326,6 +327,10 @@ static int asus_raw_event(struct hid_device *hdev,
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		/* NOOP on generic HID devices to avoid side effects. */
+		return 0;
+
 	if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
 		return asus_report_battery(drvdata, data, size);
 
@@ -773,6 +778,10 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	struct input_dev *input = hi->input;
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		/* NOOP on generic HID devices to avoid side effects. */
+		return 0;
+
 	/* T100CHI uses MULTI_INPUT, bind the touchpad to the mouse hid_input */
 	if (drvdata->quirks & QUIRK_T100CHI &&
 	    hi->report->id != T100CHI_MOUSE_REPORT_ID)
@@ -850,6 +859,10 @@ static int asus_input_mapping(struct hid_device *hdev,
 		return -1;
 	}
 
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		/* NOOP on generic HID devices to avoid side effects. */
+		return 0;
+
 	/*
 	 * Ignore a bunch of bogus collections in the T100CHI descriptor.
 	 * This avoids a bunch of non-functional hid_input devices getting
@@ -1025,8 +1038,10 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
 
 static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-	int ret;
+	struct hid_report_enum *rep_enum;
 	struct asus_drvdata *drvdata;
+	struct hid_report *rep;
+	int ret, found = 0;
 
 	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
 	if (drvdata == NULL) {
@@ -1110,6 +1125,37 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
+	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
+		/*
+		 * The only application we care about on ROG NKEY keyboards is
+		 * 0xff310076. This is meant Asus drivers and uses report 0x54.
+		 */
+		rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
+		list_for_each_entry(rep, &rep_enum->report_list, list) {
+			if (rep->application == 0xff310076)
+				found = true;
+		}
+
+		/*
+		 * If we didn't find the application, block hid-asus fixups
+		 * to prevent side effects on generic endpoints.
+		 *
+		 * We cannot -ENODEV here, as hid-generic checked our id_table
+		 * on its match and bailed so it will not take over the device.
+		 * We have to handle it transparently as part of this driver.
+		 */
+		if (!found)
+			drvdata->quirks |= QUIRK_HANDLE_GENERIC;
+
+		/*
+		 * Start all endpoints normally. Include the RGB endpoint
+		 * as it being the only one renamed looks out of place.
+		 * The ->input bail causes regressions in endpoints without
+		 * an input dev and is a NOOP on the RGB endpoint.
+		 */
+		return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	}
+
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret) {
 		hid_err(hdev, "Asus hw start failed: %d\n", ret);
@@ -1166,6 +1212,10 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		/* NOOP on generic HID devices to avoid side effects. */
+		return rdesc;
+
 	if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
 			*rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65) {
 		hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
-- 
2.48.1

[PATCH 04/11] HID: asus: rename keyboard3 to Z13_FOLIO

[Antheas Kapenekakis]

Rename the generic keyboard3 to Z13_FOLIO as it refers to the folio of
the Z13. Both 2023 and 2025 variants.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 2 +-
 drivers/hid/hid-ids.h  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 490a7ea369961..cdd9d9c4fc95f 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1332,7 +1332,7 @@ static const struct hid_device_id asus_devices[] = {
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
-	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
+	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7e400624908e3..b1fe7582324ff 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -209,7 +209,7 @@
 #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD	0x1866
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2	0x19b6
-#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3	0x1a30
+#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO		0x1a30
 #define USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR		0x18c6
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY		0x1abe
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X		0x1b4c
-- 
2.48.1

[PATCH 05/11] HID: asus: add Asus Z13 2025 Fan key

[Antheas Kapenekakis]

The ASUS Z13 2025 uses the vendor code 0xec for
its Fn+F5 fan key. Add a quirk for it.

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

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index cdd9d9c4fc95f..85ae75478b796 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -897,6 +897,7 @@ static int asus_input_mapping(struct hid_device *hdev,
 		case 0x5c: asus_map_key_clear(KEY_PROG3);	break; /* Fn+Space Power4Gear */
 		case 0x99: asus_map_key_clear(KEY_PROG4);	break; /* Fn+F5 "fan" symbol */
 		case 0xae: asus_map_key_clear(KEY_PROG4);	break; /* Fn+F5 "fan" symbol */
+		case 0xec: asus_map_key_clear(KEY_PROG4);	break; /* Fn+F5 "fan" symbol (Z13 2025) */
 		case 0x92: asus_map_key_clear(KEY_CALC);	break; /* Fn+Ret "Calc" symbol */
 		case 0xb2: asus_map_key_clear(KEY_PROG2);	break; /* Fn+Left previous aura */
 		case 0xb3: asus_map_key_clear(KEY_PROG3);	break; /* Fn+Left next aura */
-- 
2.48.1

[PATCH 06/11] HID: asus: introduce small delay on Asus Z13 RGB init

[Antheas Kapenekakis]

The folio keyboard of the Z13 can get stuck in its BIOS mode, where the
touchpad behaves like a mouse and the keyboard start button is not
reliable if we perform the initialization too quickly. This mostly
happens during boot, and can be verified to be caused by hid-asus
through simple blacklisting. A small delay fixes it.

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

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 85ae75478b796..5b75ee83ae290 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -571,6 +571,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	unsigned char kbd_func;
 	int ret;
 
+	/* Wait a bit before init to prevent locking the keyboard */
+	if (dmi_match(DMI_PRODUCT_FAMILY, "ROG Flow Z13"))
+		msleep(500);
+
 	ret = asus_kbd_init(hdev);
 	if (ret < 0)
 		return ret;
-- 
2.48.1

[PATCH 07/11] platform/x86: asus-wmi: Add support for multiple kbd RGB 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.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 38ef778e8c19b..0cb1cf3c25a28 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -254,6 +254,8 @@ struct asus_wmi {
 	int tpd_led_wk;
 	struct led_classdev kbd_led;
 	int kbd_led_wk;
+	bool kbd_led_avail;
+	bool kbd_led_registered;
 	struct led_classdev lightbar_led;
 	int lightbar_led_wk;
 	struct led_classdev micmute_led;
@@ -1487,6 +1489,46 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
 
 /* LEDs ***********************************************************************/
 
+LIST_HEAD(asus_brt_listeners);
+DEFINE_MUTEX(asus_brt_lock);
+struct asus_wmi *asus_brt_ref;
+
+int asus_brt_register_listener(struct asus_brt_listener *bdev)
+{
+	int ret;
+
+	mutex_lock(&asus_brt_lock);
+	list_add_tail(&bdev->list, &asus_brt_listeners);
+	if (asus_brt_ref) {
+		if (asus_brt_ref->kbd_led_registered && asus_brt_ref->kbd_led_wk >= 0)
+			bdev->notify(bdev, asus_brt_ref->kbd_led_wk);
+		else {
+			asus_brt_ref->kbd_led_registered = true;
+			ret = led_classdev_register(
+				&asus_brt_ref->platform_device->dev,
+				&asus_brt_ref->kbd_led);
+		}
+	}
+	mutex_unlock(&asus_brt_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(asus_brt_register_listener);
+
+void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
+{
+	mutex_lock(&asus_brt_lock);
+	list_del(&bdev->list);
+
+	if (asus_brt_ref && list_empty(&asus_brt_listeners) &&
+	    !asus_brt_ref->kbd_led_avail) {
+		led_classdev_unregister(&asus_brt_ref->kbd_led);
+		asus_brt_ref->kbd_led_registered = false;
+	}
+	mutex_unlock(&asus_brt_lock);
+}
+EXPORT_SYMBOL_GPL(asus_brt_unregister_listener);
+
 /*
  * These functions actually update the LED's, and are called from a
  * workqueue. By doing this as separate work rather than when the LED
@@ -1566,6 +1608,7 @@ 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_brt_listener *listener;
 	struct asus_wmi *asus;
 	int max_level;
 
@@ -1573,25 +1616,37 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
 	max_level = asus->kbd_led.max_brightness;
 
 	asus->kbd_led_wk = clamp_val(value, 0, max_level);
-	kbd_led_update(asus);
+
+	if (asus->kbd_led_avail)
+		kbd_led_update(asus);
+
+	list_for_each_entry(listener, &asus_brt_listeners, list)
+		listener->notify(listener, asus->kbd_led_wk);
 }
 
 static void kbd_led_set(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
+	mutex_lock(&asus_brt_lock);
+
 	/* Prevent disabling keyboard backlight on module unregister */
 	if (led_cdev->flags & LED_UNREGISTERING)
 		return;
 
 	do_kbd_led_set(led_cdev, value);
+	mutex_unlock(&asus_brt_lock);
 }
 
 static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
 {
-	struct led_classdev *led_cdev = &asus->kbd_led;
+	struct led_classdev *led_cdev;
+
+	mutex_lock(&asus_brt_lock);
+	led_cdev = &asus->kbd_led;
 
 	do_kbd_led_set(led_cdev, value);
 	led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
+	mutex_unlock(&asus_brt_lock);
 }
 
 static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
@@ -1601,6 +1656,9 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
 
 	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
 
+	if (!asus->kbd_led_avail)
+		return asus->kbd_led_wk;
+
 	retval = kbd_led_read(asus, &value, NULL);
 	if (retval < 0)
 		return retval;
@@ -1716,6 +1774,10 @@ static int camera_led_set(struct led_classdev *led_cdev,
 
 static void asus_wmi_led_exit(struct asus_wmi *asus)
 {
+	mutex_lock(&asus_brt_lock);
+	asus_brt_ref = NULL;
+	mutex_unlock(&asus_brt_lock);
+
 	led_classdev_unregister(&asus->kbd_led);
 	led_classdev_unregister(&asus->tpd_led);
 	led_classdev_unregister(&asus->wlan_led);
@@ -1730,6 +1792,7 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
 static int asus_wmi_led_init(struct asus_wmi *asus)
 {
 	int rv = 0, num_rgb_groups = 0, led_val;
+	bool has_listeners;
 
 	if (asus->kbd_rgb_dev)
 		kbd_rgb_mode_groups[num_rgb_groups++] = &kbd_rgb_mode_group;
@@ -1754,24 +1817,37 @@ 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.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);
+
+	if (asus->kbd_led_avail)
 		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;
+	else
+		asus->kbd_led_wk = -1;
+
+	if (asus->kbd_led_avail && num_rgb_groups != 0)
+		asus->kbd_led.groups = kbd_rgb_mode_groups;
 
-		if (num_rgb_groups != 0)
-			asus->kbd_led.groups = kbd_rgb_mode_groups;
+	mutex_lock(&asus_brt_lock);
+	has_listeners = !list_empty(&asus_brt_listeners);
+	mutex_unlock(&asus_brt_lock);
 
+	if (asus->kbd_led_avail || has_listeners) {
+		asus->kbd_led_registered = true;
 		rv = led_classdev_register(&asus->platform_device->dev,
 					   &asus->kbd_led);
 		if (rv)
 			goto error;
 	}
 
+	mutex_lock(&asus_brt_lock);
+	asus_brt_ref = asus;
+	mutex_unlock(&asus_brt_lock);
+
 	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
 			&& (asus->driver->quirks->wapf > 0)) {
 		INIT_WORK(&asus->wlan_led_work, wlan_led_update);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 783e2a336861b..42e963b70acdb 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -157,14 +157,30 @@
 #define ASUS_WMI_DSTS_MAX_BRIGTH_MASK	0x0000FF00
 #define ASUS_WMI_DSTS_LIGHTBAR_MASK	0x0000000F
 
+struct asus_brt_listener {
+	struct list_head list;
+	void (*notify)(struct asus_brt_listener *listener, int brightness);
+};
+
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
+
+int asus_brt_register_listener(struct asus_brt_listener *cdev);
+void asus_brt_unregister_listener(struct asus_brt_listener *cdev);
 #else
 static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
 					   u32 *retval)
 {
 	return -ENODEV;
 }
+
+static inline int asus_brt_register_listener(struct asus_brt_listener *bdev)
+{
+	return -ENODEV;
+}
+static inline void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
+{
+}
 #endif
 
 /* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-- 
2.48.1

[PATCH 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.

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

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 5b75ee83ae290..5aae4466c0d63 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -96,7 +96,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_brt_listener listener;
 	struct hid_device *hdev;
 	struct work_struct work;
 	unsigned int brightness;
@@ -493,11 +493,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_brt_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 +507,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 +523,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);
-}
-
 static int asus_kbd_register_leds(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -603,14 +561,12 @@ 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.notify = 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_brt_register_listener(&drvdata->kbd_backlight->listener);
+
 	if (ret < 0) {
 		/* No need to have this still around */
 		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
@@ -840,7 +796,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");
 
@@ -1019,7 +974,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);
@@ -1202,6 +1157,8 @@ static void asus_remove(struct hid_device *hdev)
 		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
 
 		cancel_work_sync(&drvdata->kbd_backlight->work);
+
+		asus_brt_unregister_listener(&drvdata->kbd_backlight->listener);
 	}
 
 	hid_hw_stop(hdev);
-- 
2.48.1

[PATCH 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 it.

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

diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 42e963b70acdb..add04524031d8 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -183,44 +183,4 @@ static inline void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
 }
 #endif
 
-/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
-		},
-	},
-	{ },
-};
-
 #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
-- 
2.48.1

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

[Antheas Kapenekakis]

Currenlty, the keyboard brightness control of Asus WMI keyboards is
handled in the 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.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 0cb1cf3c25a28..2a394f56e44c8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1529,6 +1529,45 @@ void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
 }
 EXPORT_SYMBOL_GPL(asus_brt_unregister_listener);
 
+static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
+
+int asus_brt_event(enum asus_brt_event event)
+{
+	int brightness;
+
+	mutex_lock(&asus_brt_lock);
+	if (!asus_brt_ref || !asus_brt_ref->kbd_led_registered) {
+		mutex_unlock(&asus_brt_lock);
+		return -EBUSY;
+	}
+	brightness = asus_brt_ref->kbd_led_wk;
+	mutex_unlock(&asus_brt_lock);
+
+	switch (event) {
+	case ASUS_BRT_UP:
+		brightness += 1;
+		break;
+	case ASUS_BRT_DOWN:
+		brightness -= 1;
+		break;
+	case ASUS_BRT_TOGGLE:
+		if (brightness >= 3)
+			brightness = 0;
+		else
+			brightness += 1;
+		break;
+	}
+
+	do_kbd_led_set(&asus_brt_ref->kbd_led, brightness);
+	led_classdev_notify_brightness_hw_changed(&asus_brt_ref->kbd_led,
+						  asus_brt_ref->kbd_led_wk);
+
+	mutex_unlock(&asus_brt_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asus_brt_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
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index add04524031d8..c683492be5de5 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -162,11 +162,18 @@ struct asus_brt_listener {
 	void (*notify)(struct asus_brt_listener *listener, int brightness);
 };
 
+enum asus_brt_event {
+	ASUS_BRT_UP,
+	ASUS_BRT_DOWN,
+	ASUS_BRT_TOGGLE,
+};
+
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
 
 int asus_brt_register_listener(struct asus_brt_listener *cdev);
 void asus_brt_unregister_listener(struct asus_brt_listener *cdev);
+int asus_brt_event(enum asus_brt_event event);
 #else
 static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
 					   u32 *retval)
@@ -181,6 +188,9 @@ static inline int asus_brt_register_listener(struct asus_brt_listener *bdev)
 static inline void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
 {
 }
+static inline void asus_brt_event(enum asus_brt_event event)
+{
+}
 #endif
 
 #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
-- 
2.48.1

[PATCH 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.

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 5aae4466c0d63..b1ae0716005d2 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -319,6 +319,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_brt_event(ASUS_BRT_UP);
+		case KEY_KBDILLUMDOWN:
+			return !asus_brt_event(ASUS_BRT_DOWN);
+		case KEY_KBDILLUMTOGGLE:
+			return !asus_brt_event(ASUS_BRT_TOGGLE);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.48.1

Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[Antheas Kapenekakis]

On Wed, 19 Mar 2025 at 22:38, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> This is a three part series that does the following: first, it cleans up
> the hid-asus driver initialization, preventing excess renames and dmesg
> errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
> by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
> key. Finally, the bigger piece of this series is the unification of the
> backlight controls between hid-asus and asus-wmi.
>
> <snip>
> --
> 2.48.1
>

Update on this. I made a minor refactor today to fix some edge cases
where it was obvious the mutex was not guarding stuff and ended up
locking during unregistering in this series. So skip testing it.

I'll send the fixed version tomorrow, in the meantime you can find it
here [1]. Fixes were very minor, moving the mutexes around in 2
places, so feel free to comment on this series.

Also, the init delay seems to not have worked for the mouse touchpad
bug. Seems like 5 reboots were not enough testing. I will say it is
peculiar that hid-generic works properly, when this series makes
hid-asus mimic it very closely. Also, one rmmod to hid-asus fixes
this, regardless of how many times hid-asus is reloaded afterwards.

Antheas

+CC Mario in case you want to review

[1] https://github.com/bazzite-org/patchwork/tree/upstream/asusrgb

Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[Luke Jones]

Hi Antheas,

On Thu, 20 Mar 2025, at 8:13 AM, Antheas Kapenekakis wrote:
> This is a three part series that does the following: first, it cleans up
> the hid-asus driver initialization, preventing excess renames and dmesg
> errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
> by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
> key. Finally, the bigger piece of this series is the unification of the
> backlight controls between hid-asus and asus-wmi.
>
> This requires some context. First, some ROG devices expose both WMI and
> HID controls for RGB. In addition, some ROG devices (such as the Z13)
> have two AURA devices where both have backlight controls (lightbar and
> keyboard). Under Windows, Armoury Crate exposes a single brightness control
> for all Aura devices.
>
> However, currently in the linux kernel this is not the case, with asus-wmi
> and hid-asus relying on a quirk system to figure out which should control
> the backlight. But what about the other one? There might be silent
> regressions such as part of the RGB of the device not responding properly.
>
> In the Z13, this is the case, with a race condition causing the lightbar
> to control the asus::kbd_backlight device most of the time, with the
> keyboard being renamed to asus::kbd_backlight_1 and not doing anything
> under KDE controls.
>
> Here, we should note that most backlight handlers are hardcoded to check
> for backlight once, and for one backlight, during boot, so any other
> solution would require a large rewrite of userspace.

This makes me wish there was better standardization. Maybe filing some reports upstream to those various projects could get the ball rolling?

> Even when brightness controls are fixed, we still have the problem of the
> backlight key being on/off when controlled by KDE and 0/33/66/100 when
> the device has a WMI keyboard. Ideally, we would like the 0/33/66/100 to
> be done under hid as well, regardless of whether the backlight of the
> device is controlled by WMI or HID.
>
> Therefore, this is what the third part of this series does. It sets up
> asus-wmi to expose accepting listeners for the asus::kbd_backlight device
> and being the one that sets it up. Then, it makes hid-asus devices
> register a listener there, so that all of them are controlled by
> asus::kbd_backlight. Finally, it adds an event handler for keyboard keys,
> so that HID led controls are handled by the kernel instead of userspace.
> This way, even when userspace is not active the key works, and we get the
> desired behavior of 0/33/66/100 across all Aura devices (currently, that
> is keyboards, and embedded devices such as lightbars). This results
> removing the quirk system as well, eliminating a point of failure.

Nice, I'd been looking at doing something similar but unfortunately hadn't the time for it, nor the device appropriate for testing (keyboard, detachable, lightbar). TBH I wish there was a much better way in kernel to handle these sorts of lighting situations, especially given that we have laptops across vendors and models with different modes, zones, per-key, MCU mode vs software mode etc etc. There is a *very* long thread on lkml bikeshedding it all too - see https://lore.kernel.org/lkml/20231011190017.1230898-1-wse@tuxedocomputers.com/

The LampArray thing is out of scope for this, but I thought maybe worth mentioning in case you weren't aware. The major pitfall of it is that per-key devices update per-row and when you do a single key update to update a whole keyboard it sends N-Key amounts of packets..

Off-topic though. But if you have some ideas please email me.

> I tested this on an Asus Z13 2025, and testing by other devices would be
> appreciated for sure. This series is designed to be transparent to
> userspace behavior-wise compared previous kernels, with all existing
> laptops either having the same behavior or being better.

I have a handful of laptops I can test, including my old GA501, I'll get on it.

> The Z13 keyboard folio RGB controls work beautifully, with KDE led
> notifications working and doing 0/33/66/100 as expected. This also happens
> with hotplugging, as the lightbar is always available and keeps the
> endpoint alive from boot, even if the folio is not connected (a quirk
> can be added later if there is a device where this is not the case).

Very good. This will make a lot of folks happy, I suspect the Z13 is going to be a very popular device.

> The first two parts of the series can also be merged independently of the
> third part, so we can iterate on that more. Perhaps there is a better way
> to handle this cohesion,

After a quick cursory look, this looks good so far. Perhaps after review and iteration you could submit as an independent series to get those parts in quicker - but hey we can cross that when we get to it.

> Oh, by the way Luke, I developed this series with a variant of
> your Armoury series merged, and only switched to 6.14-v7 for this
> submission. You will be happy to know that there are no conflicts :)
> (at least with that version from ~January). Also, please factcheck
> my initialization sequence is correct in the 0x5d and 0x5e devices
> you added when you made that refactor last year. Are those handshakes
> needed?

I would hope the armoury driver stays out of the way of most things, I tried to make it independent. The handshakes are needed for sure, depending on device it may be partial or more, but it's always been the same ASCII right back to when I first started on this with a 2018 laptop - we never bothered with the response check though. I do forget what required the 0x5e init, I'll need to check through some old notes.

I'll apologize in advance for the time it might take me to review - I'll attempt some now for the smaller patches, but hopefully I can get some time in this weekend and we can work together to make asus stuff even better.

Cheers,
Luke.

> Antheas Kapenekakis (11):
>   HID: asus: refactor init sequence per spec
>   HID: asus: cleanup keyboard backlight check
>   HID: asus: prevent binding to all HID devices on ROG
>   HID: asus: rename keyboard3 to Z13_FOLIO
>   HID: asus: add Asus Z13 2025 Fan key
>   HID: asus: introduce small delay on Asus Z13 RGB init
>   platform/x86: asus-wmi: Add support for multiple kbd RGB 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                     | 220 ++++++++++++---------
>  drivers/hid/hid-ids.h                      |   2 +-
>  drivers/platform/x86/asus-wmi.c            | 137 +++++++++++--
>  include/linux/platform_data/x86/asus-wmi.h |  66 +++----
>  4 files changed, 279 insertions(+), 146 deletions(-)
>
>
> base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1
> -- 
> 2.48.1

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

[Luke D. Jones]

On 20/03/25 08:13, Antheas Kapenekakis wrote:
> The quirk for selecting whether keyboard backlight should be controlled
> by HID or WMI is not needed anymore, so remove it.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   include/linux/platform_data/x86/asus-wmi.h | 40 ----------------------
>   1 file changed, 40 deletions(-)
> 
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 42e963b70acdb..add04524031d8 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -183,44 +183,4 @@ static inline void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
>   }
>   #endif
>   
> -/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
> -static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
> -		},
> -	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> -		},
> -	},
> -	{ },
> -};
> -
>   #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */

I think this can be squashed in to patch 8 as it looks likely to cause 
an "unused" warning or error? I'll defer to others though.

Cheers,
Luke.

Re: [PATCH 06/11] HID: asus: introduce small delay on Asus Z13 RGB init

[Luke D. Jones]

On 20/03/25 08:13, Antheas Kapenekakis wrote:
> The folio keyboard of the Z13 can get stuck in its BIOS mode, where the
> touchpad behaves like a mouse and the keyboard start button is not
> reliable if we perform the initialization too quickly. This mostly
> happens during boot, and can be verified to be caused by hid-asus
> through simple blacklisting. A small delay fixes it.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/hid/hid-asus.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 85ae75478b796..5b75ee83ae290 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -571,6 +571,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>   	unsigned char kbd_func;
>   	int ret;
>   
> +	/* Wait a bit before init to prevent locking the keyboard */
> +	if (dmi_match(DMI_PRODUCT_FAMILY, "ROG Flow Z13"))
> +		msleep(500);
> +
>   	ret = asus_kbd_init(hdev);
>   	if (ret < 0)
>   		return ret;

See my comment on patch 1 about trying a loop with the init 
request/response as a hopeful way to test readiness.

Cheers,
Luke.

Re: [PATCH 01/11] HID: asus: refactor init sequence per spec

[Luke D. Jones]

On 20/03/25 08:13, Antheas Kapenekakis wrote:
> Currently, asus_kbd_init() uses a reverse engineered init sequence
> from Windows, which contains the handshakes from multiple programs.
> Keep the main one, which is 0x5a (meant for drivers).

0x5A is also used for Ally setup commands, used from userspace in 
Windows. Only a nit but I don't think stating it's only for drivers is 
accurate but then again asus kind of blurs the line a bit.

> In addition, perform a get_response and check if the response is the
> same. To avoid regressions, print an error if the response does not
> match instead of rejecting device.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/hid/hid-asus.c | 82 +++++++++++++++++++++++-------------------
>   1 file changed, 46 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 46e3e42f9eb5f..aa4a481dc4f27 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>   #define FEATURE_REPORT_ID 0x0d
>   #define INPUT_REPORT_ID 0x5d
>   #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
>   #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>   #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>   
> @@ -386,16 +386,43 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>   	return ret;
>   }
>   
> -static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> +static int asus_kbd_init(struct hid_device *hdev)
>   {
> -	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> -		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> +	/*
> +	 * Asus handshake identifying us as a driver (0x5A)
> +	 * 0x5A then ASCII for "ASUS Tech.Inc."
> +	 * 0x5D is for userspace Windows applications.

0x5D is the report ID used for commands such as RGB modes. Probably 
don't need to mention it here, and only where it is used.

> +	 * The handshake is first sent as a set_report, then retrieved
> +	 * from a get_report to verify the response.
> +	 */
> +	const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20,
> +		0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> +	u8 *readbuf;
>   	int ret;
>   
>   	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> -	if (ret < 0)
> -		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> +	if (ret < 0) {
> +		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> +		return ret;
> +	}
>   
> +	readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> +	if (!readbuf)
> +		return -ENOMEM;
> +
> +	ret = hid_hw_raw_request(hdev, FEATURE_KBD_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_err(hdev, "Asus handshake returned invalid response: %*ph\n",
> +			FEATURE_KBD_REPORT_SIZE, readbuf);
> +		// Do not return error if handshake is wrong to avoid regressions

I'll have to test this on the oldest model I have. Hopefully it's a 
non-issue and this can return error instead.

Side-note: I notice you're using a msleep to try and work around an 
issue in a later patch - it might be worth trying replacing that with a 
retry/count loop with an inner of small msleep + a call to this init, 
see if it still responds to this during that critical period.

> +	}
> +
> +	kfree(readbuf);
>   	return ret;
>   }
>   
> @@ -540,42 +567,25 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>   	unsigned char kbd_func;
>   	int ret;
>   
> -	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> -		/* Initialize keyboard */
> -		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> -		if (ret < 0)
> -			return ret;
> -
> -		/* The LED endpoint is initialised in two HID */
> -		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> -		if (ret < 0)
> -			return ret;
> -
> -		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> -		if (ret < 0)
> -			return ret;

Ah, I recall now. Some devices like the Slash or AniMe Matrix required 
the 0x5E and 0x5D report ID (device dependent) however these are 
currently being done via userspace due to not being HID devices.

There *are* some older laptops still in use that require init on 0x5E or 
0x5D for RGB to be usable, from memory. It's been over 5 years so I'll 
pull out the laptop I have with 0x1866 PID MCU and see if that is 
actually true and not just my imagination.

> +	ret = asus_kbd_init(hdev);
> +	if (ret < 0)
> +		return ret;
>   
> -		if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> -			ret = asus_kbd_disable_oobe(hdev);
> -			if (ret < 0)
> -				return ret;
> -		}
> -	} else {
> -		/* Initialize keyboard */
> -		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> -		if (ret < 0)
> -			return ret;
> +	/* Get keyboard functions */
> +	ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> +	if (ret < 0)
> +		return ret;
>   
> -		/* Get keyboard functions */
> -		ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> +	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> +		ret = asus_kbd_disable_oobe(hdev);
>   		if (ret < 0)
>   			return ret;
> -
> -		/* Check for backlight support */
> -		if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> -			return -ENODEV;
>   	}
>   
> +	/* Check for backlight support */
> +	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> +		return -ENODEV;
> +
>   	drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>   					      sizeof(struct asus_kbd_leds),
>   					      GFP_KERNEL);

I've left only small comments on a few patches for now. I'll review in 
full after I get testing done on a variety of devices whcih I'm aiming 
for this weekend. Overall impression so far is everything looks good and 
this is a nice improvement. Thank you for taking the time to implement it.

Cheers,
Luke.

Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[Antheas Kapenekakis]

On Thu, 20 Mar 2025 at 07:10, Luke Jones <luke@ljones.dev> wrote:
>
> Hi Antheas,
>
> On Thu, 20 Mar 2025, at 8:13 AM, Antheas Kapenekakis wrote:
> > This is a three part series that does the following: first, it cleans up
> > the hid-asus driver initialization, preventing excess renames and dmesg
> > errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
> > by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
> > key. Finally, the bigger piece of this series is the unification of the
> > backlight controls between hid-asus and asus-wmi.
> >
> > This requires some context. First, some ROG devices expose both WMI and
> > HID controls for RGB. In addition, some ROG devices (such as the Z13)
> > have two AURA devices where both have backlight controls (lightbar and
> > keyboard). Under Windows, Armoury Crate exposes a single brightness control
> > for all Aura devices.
> >
> > However, currently in the linux kernel this is not the case, with asus-wmi
> > and hid-asus relying on a quirk system to figure out which should control
> > the backlight. But what about the other one? There might be silent
> > regressions such as part of the RGB of the device not responding properly.
> >
> > In the Z13, this is the case, with a race condition causing the lightbar
> > to control the asus::kbd_backlight device most of the time, with the
> > keyboard being renamed to asus::kbd_backlight_1 and not doing anything
> > under KDE controls.
> >
> > Here, we should note that most backlight handlers are hardcoded to check
> > for backlight once, and for one backlight, during boot, so any other
> > solution would require a large rewrite of userspace.
>
> This makes me wish there was better standardization. Maybe filing some reports upstream to those various projects could get the ball rolling?

I think KDE has some improvements for it for multi-device support. But
specifically for brightness, it seems that all currently supported
manufacturers work fine with this so it would be a lot of work just
for asus to do this through userspace.

> > Even when brightness controls are fixed, we still have the problem of the
> > backlight key being on/off when controlled by KDE and 0/33/66/100 when
> > the device has a WMI keyboard. Ideally, we would like the 0/33/66/100 to
> > be done under hid as well, regardless of whether the backlight of the
> > device is controlled by WMI or HID.
> >
> > Therefore, this is what the third part of this series does. It sets up
> > asus-wmi to expose accepting listeners for the asus::kbd_backlight device
> > and being the one that sets it up. Then, it makes hid-asus devices
> > register a listener there, so that all of them are controlled by
> > asus::kbd_backlight. Finally, it adds an event handler for keyboard keys,
> > so that HID led controls are handled by the kernel instead of userspace.
> > This way, even when userspace is not active the key works, and we get the
> > desired behavior of 0/33/66/100 across all Aura devices (currently, that
> > is keyboards, and embedded devices such as lightbars). This results
> > removing the quirk system as well, eliminating a point of failure.
>
> Nice, I'd been looking at doing something similar but unfortunately hadn't the time for it, nor the device appropriate for testing (keyboard, detachable, lightbar). TBH I wish there was a much better way in kernel to handle these sorts of lighting situations, especially given that we have laptops across vendors and models with different modes, zones, per-key, MCU mode vs software mode etc etc. There is a *very* long thread on lkml bikeshedding it all too - see https://lore.kernel.org/lkml/20231011190017.1230898-1-wse@tuxedocomputers.com/
>
> The LampArray thing is out of scope for this, but I thought maybe worth mentioning in case you weren't aware. The major pitfall of it is that per-key devices update per-row and when you do a single key update to update a whole keyboard it sends N-Key amounts of packets..
>
> Off-topic though. But if you have some ideas please email me.

For me, I think when it comes to Asus, getting the brightness to work
is 90% of the way. Then, getting simple RGB that works with KDE but if
the KDE option is ticked off it defers to other userspace programs
such as your own is the other 10%.

And for that, I think having hid-asus create its own handlers in
addition to the one for backlight in WMI would be appropriate

> > I tested this on an Asus Z13 2025, and testing by other devices would be
> > appreciated for sure. This series is designed to be transparent to
> > userspace behavior-wise compared previous kernels, with all existing
> > laptops either having the same behavior or being better.
>
> I have a handful of laptops I can test, including my old GA501, I'll get on it.
>
> > The Z13 keyboard folio RGB controls work beautifully, with KDE led
> > notifications working and doing 0/33/66/100 as expected. This also happens
> > with hotplugging, as the lightbar is always available and keeps the
> > endpoint alive from boot, even if the folio is not connected (a quirk
> > can be added later if there is a device where this is not the case).
>
> Very good. This will make a lot of folks happy, I suspect the Z13 is going to be a very popular device.
>
> > The first two parts of the series can also be merged independently of the
> > third part, so we can iterate on that more. Perhaps there is a better way
> > to handle this cohesion,
>
> After a quick cursory look, this looks good so far. Perhaps after review and iteration you could submit as an independent series to get those parts in quicker - but hey we can cross that when we get to it.
>
> > Oh, by the way Luke, I developed this series with a variant of
> > your Armoury series merged, and only switched to 6.14-v7 for this
> > submission. You will be happy to know that there are no conflicts :)
> > (at least with that version from ~January). Also, please factcheck
> > my initialization sequence is correct in the 0x5d and 0x5e devices
> > you added when you made that refactor last year. Are those handshakes
> > needed?
>
> I would hope the armoury driver stays out of the way of most things, I tried to make it independent. The handshakes are needed for sure, depending on device it may be partial or more, but it's always been the same ASCII right back to when I first started on this with a 2018 laptop - we never bothered with the response check though. I do forget what required the 0x5e init, I'll need to check through some old notes.
>
> I'll apologize in advance for the time it might take me to review - I'll attempt some now for the smaller patches, but hopefully I can get some time in this weekend and we can work together to make asus stuff even better.
>
> Cheers,
> Luke.
>
> > Antheas Kapenekakis (11):
> >   HID: asus: refactor init sequence per spec
> >   HID: asus: cleanup keyboard backlight check
> >   HID: asus: prevent binding to all HID devices on ROG
> >   HID: asus: rename keyboard3 to Z13_FOLIO
> >   HID: asus: add Asus Z13 2025 Fan key
> >   HID: asus: introduce small delay on Asus Z13 RGB init
> >   platform/x86: asus-wmi: Add support for multiple kbd RGB 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                     | 220 ++++++++++++---------
> >  drivers/hid/hid-ids.h                      |   2 +-
> >  drivers/platform/x86/asus-wmi.c            | 137 +++++++++++--
> >  include/linux/platform_data/x86/asus-wmi.h |  66 +++----
> >  4 files changed, 279 insertions(+), 146 deletions(-)
> >
> >
> > base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1
> > --
> > 2.48.1

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

[Antheas Kapenekakis]

On Thu, 20 Mar 2025 at 08:11, Luke D. Jones <luke@ljones.dev> wrote:
>
> On 20/03/25 08:13, Antheas Kapenekakis wrote:
> > The quirk for selecting whether keyboard backlight should be controlled
> > by HID or WMI is not needed anymore, so remove it.
> >
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >   include/linux/platform_data/x86/asus-wmi.h | 40 ----------------------
> >   1 file changed, 40 deletions(-)
> >
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index 42e963b70acdb..add04524031d8 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -183,44 +183,4 @@ static inline void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
> >   }
> >   #endif
> >
> > -/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
> > -static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
> > -             },
> > -     },
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
> > -             },
> > -     },
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
> > -             },
> > -     },
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
> > -             },
> > -     },
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
> > -             },
> > -     },
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
> > -             },
> > -     },
> > -     {
> > -             .matches = {
> > -                     DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> > -             },
> > -     },
> > -     { },
> > -};
> > -
> >   #endif      /* __PLATFORM_DATA_X86_ASUS_WMI_H */
>
> I think this can be squashed in to patch 8 as it looks likely to cause
> an "unused" warning or error? I'll defer to others though.

I tried to avoid going cross-platform for these three patches. If
someone has a better suggestion to do it I would be happy to do that
instead, incl maybe adding a __maybe_unused as part of the previous
patch.

> Cheers,
> Luke.
>

Antheas

Re: [PATCH 06/11] HID: asus: introduce small delay on Asus Z13 RGB init

[Antheas Kapenekakis]

On Thu, 20 Mar 2025 at 08:12, Luke D. Jones <luke@ljones.dev> wrote:
>
> On 20/03/25 08:13, Antheas Kapenekakis wrote:
> > The folio keyboard of the Z13 can get stuck in its BIOS mode, where the
> > touchpad behaves like a mouse and the keyboard start button is not
> > reliable if we perform the initialization too quickly. This mostly
> > happens during boot, and can be verified to be caused by hid-asus
> > through simple blacklisting. A small delay fixes it.
> >
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >   drivers/hid/hid-asus.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 85ae75478b796..5b75ee83ae290 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -571,6 +571,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >       unsigned char kbd_func;
> >       int ret;
> >
> > +     /* Wait a bit before init to prevent locking the keyboard */
> > +     if (dmi_match(DMI_PRODUCT_FAMILY, "ROG Flow Z13"))
> > +             msleep(500);
> > +
> >       ret = asus_kbd_init(hdev);
> >       if (ret < 0)
> >               return ret;
>
> See my comment on patch 1 about trying a loop with the init
> request/response as a hopeful way to test readiness.
>
> Cheers,
> Luke.

Turns out there isn't an init problem. I have removed this patch from V2.

It was hid-asus taking control of the touchpad from hid-multitouch. So
adding HID_GROUP_GENERIC fixes this. I replaced it with that and
squashed the rename patch alongside it.

Antheas

Re: [PATCH 01/11] HID: asus: refactor init sequence per spec

[Antheas Kapenekakis]

On Thu, 20 Mar 2025 at 08:19, Luke D. Jones <luke@ljones.dev> wrote:
>
>
> On 20/03/25 08:13, Antheas Kapenekakis wrote:
> > Currently, asus_kbd_init() uses a reverse engineered init sequence
> > from Windows, which contains the handshakes from multiple programs.
> > Keep the main one, which is 0x5a (meant for drivers).
>
> 0x5A is also used for Ally setup commands, used from userspace in
> Windows. Only a nit but I don't think stating it's only for drivers is
> accurate but then again asus kind of blurs the line a bit.

ROG devices contain a HID USB endpoint that exposes multiple
applications. On my Z13, that is 4 hiddev devices.

However, we only care about two. Those are:

Application / Report ID:
0xff310076 / 0x5a meant for Asus drivers
0xff310079 / 0x5d meant for Asus applications

Both require the same handshake when they start. Well, in theory. But
as you say in some of the Anime stuff requires it in practice too.

The handshake is set_report 0x5X + "Asus...", then get_report with the
same ID which should return the asus string.

In hiddraw, they appear under the same endpoint, both on the Ally and
the Z13. But in hiddev (with hid-asus disabled or with this series),
they appear as separate.

I cannot comment on the Aura protocol, because I don't know, but for
the basic sticky RGB mode that supports set and apply, they _should_
behave identically. I use 0x5d in my userspace software for everything
now [1]. Previously, I used 0x5a but I am not a driver.

They do behave identically on the Ally X and the Z13 2025 though.

I do not know about 0x5e. Perhaps Asus made a special endpoint for
their Anime creation app.

> > In addition, perform a get_response and check if the response is the
> > same. To avoid regressions, print an error if the response does not
> > match instead of rejecting device.
> >
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >   drivers/hid/hid-asus.c | 82 +++++++++++++++++++++++-------------------
> >   1 file changed, 46 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 46e3e42f9eb5f..aa4a481dc4f27 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> >   #define FEATURE_REPORT_ID 0x0d
> >   #define INPUT_REPORT_ID 0x5d
> >   #define FEATURE_KBD_REPORT_ID 0x5a
> > -#define FEATURE_KBD_REPORT_SIZE 16
> > +#define FEATURE_KBD_REPORT_SIZE 64
> >   #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> >   #define FEATURE_KBD_LED_REPORT_ID2 0x5e
> >
> > @@ -386,16 +386,43 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
> >       return ret;
> >   }
> >
> > -static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> > +static int asus_kbd_init(struct hid_device *hdev)
> >   {
> > -     const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> > -                  0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> > +     /*
> > +      * Asus handshake identifying us as a driver (0x5A)
> > +      * 0x5A then ASCII for "ASUS Tech.Inc."
> > +      * 0x5D is for userspace Windows applications.
>
> 0x5D is the report ID used for commands such as RGB modes. Probably
> don't need to mention it here, and only where it is used.

Yep, see above. Not required for basic RGB. Maybe it is for Aura, but
I'd leave that to userspace.

> > +      * The handshake is first sent as a set_report, then retrieved
> > +      * from a get_report to verify the response.
> > +      */
> > +     const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20,
> > +             0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> > +     u8 *readbuf;
> >       int ret;
> >
> >       ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> > -     if (ret < 0)
> > -             hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> > +     if (ret < 0) {
> > +             hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> > +             return ret;
> > +     }
> >
> > +     readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> > +     if (!readbuf)
> > +             return -ENOMEM;
> > +
> > +     ret = hid_hw_raw_request(hdev, FEATURE_KBD_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_err(hdev, "Asus handshake returned invalid response: %*ph\n",
> > +                     FEATURE_KBD_REPORT_SIZE, readbuf);
> > +             // Do not return error if handshake is wrong to avoid regressions
>
> I'll have to test this on the oldest model I have. Hopefully it's a
> non-issue and this can return error instead.
>
> Side-note: I notice you're using a msleep to try and work around an
> issue in a later patch - it might be worth trying replacing that with a
> retry/count loop with an inner of small msleep + a call to this init,
> see if it still responds to this during that critical period.

The call did not fail. I was thinking it was because the device needs
some time to warm up (it happens with certain devices).

Turns out it was hid-multitouch not attaching.

> > +     }
> > +
> > +     kfree(readbuf);
> >       return ret;
> >   }
> >
> > @@ -540,42 +567,25 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >       unsigned char kbd_func;
> >       int ret;
> >
> > -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> > -             /* Initialize keyboard */
> > -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > -             if (ret < 0)
> > -                     return ret;
> > -
> > -             /* The LED endpoint is initialised in two HID */
> > -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> > -             if (ret < 0)
> > -                     return ret;
> > -
> > -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> > -             if (ret < 0)
> > -                     return ret;
>
> Ah, I recall now. Some devices like the Slash or AniMe Matrix required
> the 0x5E and 0x5D report ID (device dependent) however these are
> currently being done via userspace due to not being HID devices.
>
> There *are* some older laptops still in use that require init on 0x5E or
> 0x5D for RGB to be usable, from memory. It's been over 5 years so I'll
> pull out the laptop I have with 0x1866 PID MCU and see if that is
> actually true and not just my imagination.

Hopefully you handshake with these devices over userspace, so they
will not be affected.

> > +     ret = asus_kbd_init(hdev);
> > +     if (ret < 0)
> > +             return ret;
> >
> > -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > -                     ret = asus_kbd_disable_oobe(hdev);
> > -                     if (ret < 0)
> > -                             return ret;
> > -             }
> > -     } else {
> > -             /* Initialize keyboard */
> > -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > -             if (ret < 0)
> > -                     return ret;
> > +     /* Get keyboard functions */
> > +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > +     if (ret < 0)
> > +             return ret;
> >
> > -             /* Get keyboard functions */
> > -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > +             ret = asus_kbd_disable_oobe(hdev);
> >               if (ret < 0)
> >                       return ret;
> > -
> > -             /* Check for backlight support */
> > -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > -                     return -ENODEV;
> >       }
> >
> > +     /* Check for backlight support */
> > +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > +             return -ENODEV;
> > +
> >       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> >                                             sizeof(struct asus_kbd_leds),
> >                                             GFP_KERNEL);
>
> I've left only small comments on a few patches for now. I'll review in
> full after I get testing done on a variety of devices whcih I'm aiming
> for this weekend. Overall impression so far is everything looks good and
> this is a nice improvement. Thank you for taking the time to implement it.
>
> Cheers,
> Luke.

I'll try to have V2 out today. I finished it yesterday and fixed all
the lockups and the hid-multitouch issue. Just needs a good
lookthrough.

Perhaps I will also do a small multi-intensity endpoint that works
with KDE and only applies the colors when asked. This way our programs
are not affected and normal laptop users get basic RGB OOTB.

If I do that, I will make the quirk for the Ally in a separate patch,
so that you can nack it if you'd rather introduce RGB support with
your driver, so that it does not need to be reverted.

Antheas

[1] https://github.com/hhd-dev/hhd/blob/d3bbd7fa25fe9a4838896a2c5cfda460abe48dc6/src/hhd/device/rog_ally/const.py#L5-L8

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

[kernel test robot]

Hi Antheas,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4701f33a10702d5fc577c32434eb62adde0a1ae1]

url:    https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/HID-asus-refactor-init-sequence-per-spec/20250320-031740
base:   4701f33a10702d5fc577c32434eb62adde0a1ae1
patch link:    https://lore.kernel.org/r/20250319191320.10092-12-lkml%40antheas.dev
patch subject: [PATCH 11/11] HID: asus: add support for the asus-wmi brightness handler
config: s390-randconfig-002-20250320 (https://download.01.org/0day-ci/archive/20250320/202503201739.4NJJCyeZ-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250320/202503201739.4NJJCyeZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503201739.4NJJCyeZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/hid/hid-asus.c: In function 'asus_event':
>> drivers/hid/hid-asus.c:325:11: error: invalid use of void expression
       return !asus_brt_event(ASUS_BRT_UP);
              ^
   drivers/hid/hid-asus.c:327:11: error: invalid use of void expression
       return !asus_brt_event(ASUS_BRT_DOWN);
              ^
   drivers/hid/hid-asus.c:329:11: error: invalid use of void expression
       return !asus_brt_event(ASUS_BRT_TOGGLE);
              ^


vim +325 drivers/hid/hid-asus.c

   311	
   312	static int asus_event(struct hid_device *hdev, struct hid_field *field,
   313			      struct hid_usage *usage, __s32 value)
   314	{
   315		if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
   316		    (usage->hid & HID_USAGE) != 0x00 &&
   317		    (usage->hid & HID_USAGE) != 0xff && !usage->type) {
   318			hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
   319				 usage->hid & HID_USAGE);
   320		}
   321	
   322		if (usage->type == EV_KEY && value) {
   323			switch (usage->code) {
   324			case KEY_KBDILLUMUP:
 > 325				return !asus_brt_event(ASUS_BRT_UP);
   326			case KEY_KBDILLUMDOWN:
   327				return !asus_brt_event(ASUS_BRT_DOWN);
   328			case KEY_KBDILLUMTOGGLE:
   329				return !asus_brt_event(ASUS_BRT_TOGGLE);
   330			}
   331		}
   332	
   333		return 0;
   334	}
   335	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 01/11] HID: asus: refactor init sequence per spec

[Antheas Kapenekakis]

On Thu, 20 Mar 2025 at 10:50, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Thu, 20 Mar 2025 at 08:19, Luke D. Jones <luke@ljones.dev> wrote:
> >
> >
> > On 20/03/25 08:13, Antheas Kapenekakis wrote:
> > > Currently, asus_kbd_init() uses a reverse engineered init sequence
> > > from Windows, which contains the handshakes from multiple programs.
> > > Keep the main one, which is 0x5a (meant for drivers).
> >
> > 0x5A is also used for Ally setup commands, used from userspace in
> > Windows. Only a nit but I don't think stating it's only for drivers is
> > accurate but then again asus kind of blurs the line a bit.
>
> ROG devices contain a HID USB endpoint that exposes multiple
> applications. On my Z13, that is 4 hiddev devices.
>
> However, we only care about two. Those are:
>
> Application / Report ID:
> 0xff310076 / 0x5a meant for Asus drivers
> 0xff310079 / 0x5d meant for Asus applications
>
> Both require the same handshake when they start. Well, in theory. But
> as you say in some of the Anime stuff requires it in practice too.
>
> The handshake is set_report 0x5X + "Asus...", then get_report with the
> same ID which should return the asus string.

I might have misread here and only 0x5d can be used for RGB. Actually
I probably misread. Let me get back to you on that.

But brightness is both 0x5d and 0x5a for sure. If I do RGB I will
defer initializing 0x5d until userspace tries to write the color.

Antheas

> In hiddraw, they appear under the same endpoint, both on the Ally and
> the Z13. But in hiddev (with hid-asus disabled or with this series),
> they appear as separate.
>
> I cannot comment on the Aura protocol, because I don't know, but for
> the basic sticky RGB mode that supports set and apply, they _should_
> behave identically. I use 0x5d in my userspace software for everything
> now [1]. Previously, I used 0x5a but I am not a driver.
>
> They do behave identically on the Ally X and the Z13 2025 though.
>
> I do not know about 0x5e. Perhaps Asus made a special endpoint for
> their Anime creation app.
>
> > > In addition, perform a get_response and check if the response is the
> > > same. To avoid regressions, print an error if the response does not
> > > match instead of rejecting device.
> > >
> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > >   drivers/hid/hid-asus.c | 82 +++++++++++++++++++++++-------------------
> > >   1 file changed, 46 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > > index 46e3e42f9eb5f..aa4a481dc4f27 100644
> > > --- a/drivers/hid/hid-asus.c
> > > +++ b/drivers/hid/hid-asus.c
> > > @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> > >   #define FEATURE_REPORT_ID 0x0d
> > >   #define INPUT_REPORT_ID 0x5d
> > >   #define FEATURE_KBD_REPORT_ID 0x5a
> > > -#define FEATURE_KBD_REPORT_SIZE 16
> > > +#define FEATURE_KBD_REPORT_SIZE 64
> > >   #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> > >   #define FEATURE_KBD_LED_REPORT_ID2 0x5e
> > >
> > > @@ -386,16 +386,43 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
> > >       return ret;
> > >   }
> > >
> > > -static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> > > +static int asus_kbd_init(struct hid_device *hdev)
> > >   {
> > > -     const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> > > -                  0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> > > +     /*
> > > +      * Asus handshake identifying us as a driver (0x5A)
> > > +      * 0x5A then ASCII for "ASUS Tech.Inc."
> > > +      * 0x5D is for userspace Windows applications.
> >
> > 0x5D is the report ID used for commands such as RGB modes. Probably
> > don't need to mention it here, and only where it is used.
>
> Yep, see above. Not required for basic RGB. Maybe it is for Aura, but
> I'd leave that to userspace.
>
> > > +      * The handshake is first sent as a set_report, then retrieved
> > > +      * from a get_report to verify the response.
> > > +      */
> > > +     const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20,
> > > +             0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> > > +     u8 *readbuf;
> > >       int ret;
> > >
> > >       ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> > > -     if (ret < 0)
> > > -             hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> > > +     if (ret < 0) {
> > > +             hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> > > +             return ret;
> > > +     }
> > >
> > > +     readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> > > +     if (!readbuf)
> > > +             return -ENOMEM;
> > > +
> > > +     ret = hid_hw_raw_request(hdev, FEATURE_KBD_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_err(hdev, "Asus handshake returned invalid response: %*ph\n",
> > > +                     FEATURE_KBD_REPORT_SIZE, readbuf);
> > > +             // Do not return error if handshake is wrong to avoid regressions
> >
> > I'll have to test this on the oldest model I have. Hopefully it's a
> > non-issue and this can return error instead.
> >
> > Side-note: I notice you're using a msleep to try and work around an
> > issue in a later patch - it might be worth trying replacing that with a
> > retry/count loop with an inner of small msleep + a call to this init,
> > see if it still responds to this during that critical period.
>
> The call did not fail. I was thinking it was because the device needs
> some time to warm up (it happens with certain devices).
>
> Turns out it was hid-multitouch not attaching.
>
> > > +     }
> > > +
> > > +     kfree(readbuf);
> > >       return ret;
> > >   }
> > >
> > > @@ -540,42 +567,25 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> > >       unsigned char kbd_func;
> > >       int ret;
> > >
> > > -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> > > -             /* Initialize keyboard */
> > > -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > > -             if (ret < 0)
> > > -                     return ret;
> > > -
> > > -             /* The LED endpoint is initialised in two HID */
> > > -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> > > -             if (ret < 0)
> > > -                     return ret;
> > > -
> > > -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> > > -             if (ret < 0)
> > > -                     return ret;
> >
> > Ah, I recall now. Some devices like the Slash or AniMe Matrix required
> > the 0x5E and 0x5D report ID (device dependent) however these are
> > currently being done via userspace due to not being HID devices.
> >
> > There *are* some older laptops still in use that require init on 0x5E or
> > 0x5D for RGB to be usable, from memory. It's been over 5 years so I'll
> > pull out the laptop I have with 0x1866 PID MCU and see if that is
> > actually true and not just my imagination.
>
> Hopefully you handshake with these devices over userspace, so they
> will not be affected.
>
> > > +     ret = asus_kbd_init(hdev);
> > > +     if (ret < 0)
> > > +             return ret;
> > >
> > > -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > > -                     ret = asus_kbd_disable_oobe(hdev);
> > > -                     if (ret < 0)
> > > -                             return ret;
> > > -             }
> > > -     } else {
> > > -             /* Initialize keyboard */
> > > -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > > -             if (ret < 0)
> > > -                     return ret;
> > > +     /* Get keyboard functions */
> > > +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > > +     if (ret < 0)
> > > +             return ret;
> > >
> > > -             /* Get keyboard functions */
> > > -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > > +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > > +             ret = asus_kbd_disable_oobe(hdev);
> > >               if (ret < 0)
> > >                       return ret;
> > > -
> > > -             /* Check for backlight support */
> > > -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > > -                     return -ENODEV;
> > >       }
> > >
> > > +     /* Check for backlight support */
> > > +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > > +             return -ENODEV;
> > > +
> > >       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> > >                                             sizeof(struct asus_kbd_leds),
> > >                                             GFP_KERNEL);
> >
> > I've left only small comments on a few patches for now. I'll review in
> > full after I get testing done on a variety of devices whcih I'm aiming
> > for this weekend. Overall impression so far is everything looks good and
> > this is a nice improvement. Thank you for taking the time to implement it.
> >
> > Cheers,
> > Luke.
>
> I'll try to have V2 out today. I finished it yesterday and fixed all
> the lockups and the hid-multitouch issue. Just needs a good
> lookthrough.
>
> Perhaps I will also do a small multi-intensity endpoint that works
> with KDE and only applies the colors when asked. This way our programs
> are not affected and normal laptop users get basic RGB OOTB.
>
> If I do that, I will make the quirk for the Ally in a separate patch,
> so that you can nack it if you'd rather introduce RGB support with
> your driver, so that it does not need to be reverted.
>
> Antheas
>
> [1] https://github.com/hhd-dev/hhd/blob/d3bbd7fa25fe9a4838896a2c5cfda460abe48dc6/src/hhd/device/rog_ally/const.py#L5-L8

Re: [PATCH 01/11] HID: asus: refactor init sequence per spec

[Luke D. Jones]

On 20/03/25 22:50, Antheas Kapenekakis wrote:
> On Thu, 20 Mar 2025 at 08:19, Luke D. Jones <luke@ljones.dev> wrote:
>>
>>
>> On 20/03/25 08:13, Antheas Kapenekakis wrote:
>>> Currently, asus_kbd_init() uses a reverse engineered init sequence
>>> from Windows, which contains the handshakes from multiple programs.
>>> Keep the main one, which is 0x5a (meant for drivers).
>>
>> 0x5A is also used for Ally setup commands, used from userspace in
>> Windows. Only a nit but I don't think stating it's only for drivers is
>> accurate but then again asus kind of blurs the line a bit.
> 
> ROG devices contain a HID USB endpoint that exposes multiple
> applications. On my Z13, that is 4 hiddev devices.
> 
> However, we only care about two. Those are:
> 
> Application / Report ID:
> 0xff310076 / 0x5a meant for Asus drivers
> 0xff310079 / 0x5d meant for Asus applications

I'm curious how you determined that. From what I've seen asus are 
consistent until they're not - typically it's when they have a weird 
device like the Slash or Anime. The slash on the G16 uses the same MCU 
and endpoint as the keyboard, but with a new report ID, while the slash 
on a G14 is a separate MCU (or at least device exposed by MCU) with own 
endpoints and hid report.

It's not important here yet, I'm just trying to add context and provide 
some extra information for you.

> Both require the same handshake when they start. Well, in theory. But
> as you say in some of the Anime stuff requires it in practice too.

Yeah as above, the G14 slash needs it, so does the older Anime dispaly. 
But the G16 slash being on the same MCU doesn't. In any case those 
gadgets are being handled in userspace just fine regardless of the 
driver state. Still, not really relevant here and I'm just adding context.

> The handshake is set_report 0x5X + "Asus...", then get_report with the
> same ID which should return the asus string.
> 
> In hiddraw, they appear under the same endpoint, both on the Ally and
> the Z13. But in hiddev (with hid-asus disabled or with this series),
> they appear as separate.
> 
> I cannot comment on the Aura protocol, because I don't know, but for
> the basic sticky RGB mode that supports set and apply, they _should_
> behave identically. I use 0x5d in my userspace software for everything
> now [1]. Previously, I used 0x5a but I am not a driver.
> 
> They do behave identically on the Ally X and the Z13 2025 though.

It is something I do have to test, and I'll add your v2 to kernels to 
get some feedback from my discord group too. I know for sure that there 
were some laptops that didn't accept 0x5D for brightness - I'm sorry I'm 
so vague on this part, I really don't remember the details and I lost 
notes so it will need to be tested.

> I do not know about 0x5e. Perhaps Asus made a special endpoint for
> their Anime creation app.

It is only for that yes.

Cheers,
Luke.

>>> In addition, perform a get_response and check if the response is the
>>> same. To avoid regressions, print an error if the response does not
>>> match instead of rejecting device.
>>>
>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>> ---
>>>    drivers/hid/hid-asus.c | 82 +++++++++++++++++++++++-------------------
>>>    1 file changed, 46 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>> index 46e3e42f9eb5f..aa4a481dc4f27 100644
>>> --- a/drivers/hid/hid-asus.c
>>> +++ b/drivers/hid/hid-asus.c
>>> @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>>>    #define FEATURE_REPORT_ID 0x0d
>>>    #define INPUT_REPORT_ID 0x5d
>>>    #define FEATURE_KBD_REPORT_ID 0x5a
>>> -#define FEATURE_KBD_REPORT_SIZE 16
>>> +#define FEATURE_KBD_REPORT_SIZE 64
>>>    #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>>>    #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>>>
>>> @@ -386,16 +386,43 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>>>        return ret;
>>>    }
>>>
>>> -static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>>> +static int asus_kbd_init(struct hid_device *hdev)
>>>    {
>>> -     const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
>>> -                  0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>>> +     /*
>>> +      * Asus handshake identifying us as a driver (0x5A)
>>> +      * 0x5A then ASCII for "ASUS Tech.Inc."
>>> +      * 0x5D is for userspace Windows applications.
>>
>> 0x5D is the report ID used for commands such as RGB modes. Probably
>> don't need to mention it here, and only where it is used.
> 
> Yep, see above. Not required for basic RGB. Maybe it is for Aura, but
> I'd leave that to userspace.
> 
>>> +      * The handshake is first sent as a set_report, then retrieved
>>> +      * from a get_report to verify the response.
>>> +      */
>>> +     const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20,
>>> +             0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>>> +     u8 *readbuf;
>>>        int ret;
>>>
>>>        ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>>> -     if (ret < 0)
>>> -             hid_err(hdev, "Asus failed to send init command: %d\n", ret);
>>> +     if (ret < 0) {
>>> +             hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
>>> +             return ret;
>>> +     }
>>>
>>> +     readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
>>> +     if (!readbuf)
>>> +             return -ENOMEM;
>>> +
>>> +     ret = hid_hw_raw_request(hdev, FEATURE_KBD_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_err(hdev, "Asus handshake returned invalid response: %*ph\n",
>>> +                     FEATURE_KBD_REPORT_SIZE, readbuf);
>>> +             // Do not return error if handshake is wrong to avoid regressions
>>
>> I'll have to test this on the oldest model I have. Hopefully it's a
>> non-issue and this can return error instead.
>>
>> Side-note: I notice you're using a msleep to try and work around an
>> issue in a later patch - it might be worth trying replacing that with a
>> retry/count loop with an inner of small msleep + a call to this init,
>> see if it still responds to this during that critical period.
> 
> The call did not fail. I was thinking it was because the device needs
> some time to warm up (it happens with certain devices).
> 
> Turns out it was hid-multitouch not attaching.
> 
>>> +     }
>>> +
>>> +     kfree(readbuf);
>>>        return ret;
>>>    }
>>>
>>> @@ -540,42 +567,25 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>        unsigned char kbd_func;
>>>        int ret;
>>>
>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>> -             /* Initialize keyboard */
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> -
>>> -             /* The LED endpoint is initialised in two HID */
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> -
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>> -             if (ret < 0)
>>> -                     return ret;
>>
>> Ah, I recall now. Some devices like the Slash or AniMe Matrix required
>> the 0x5E and 0x5D report ID (device dependent) however these are
>> currently being done via userspace due to not being HID devices.
>>
>> There *are* some older laptops still in use that require init on 0x5E or
>> 0x5D for RGB to be usable, from memory. It's been over 5 years so I'll
>> pull out the laptop I have with 0x1866 PID MCU and see if that is
>> actually true and not just my imagination.
> 
> Hopefully you handshake with these devices over userspace, so they
> will not be affected.
> 
>>> +     ret = asus_kbd_init(hdev);
>>> +     if (ret < 0)
>>> +             return ret;
>>>
>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>> -                     ret = asus_kbd_disable_oobe(hdev);
>>> -                     if (ret < 0)
>>> -                             return ret;
>>> -             }
>>> -     } else {
>>> -             /* Initialize keyboard */
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> +     /* Get keyboard functions */
>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>> +     if (ret < 0)
>>> +             return ret;
>>>
>>> -             /* Get keyboard functions */
>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>> +             ret = asus_kbd_disable_oobe(hdev);
>>>                if (ret < 0)
>>>                        return ret;
>>> -
>>> -             /* Check for backlight support */
>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>> -                     return -ENODEV;
>>>        }
>>>
>>> +     /* Check for backlight support */
>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>> +             return -ENODEV;
>>> +
>>>        drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>>>                                              sizeof(struct asus_kbd_leds),
>>>                                              GFP_KERNEL);
>>
>> I've left only small comments on a few patches for now. I'll review in
>> full after I get testing done on a variety of devices whcih I'm aiming
>> for this weekend. Overall impression so far is everything looks good and
>> this is a nice improvement. Thank you for taking the time to implement it.
>>
>> Cheers,
>> Luke.
> 
> I'll try to have V2 out today. I finished it yesterday and fixed all
> the lockups and the hid-multitouch issue. Just needs a good
> lookthrough.
> 
> Perhaps I will also do a small multi-intensity endpoint that works
> with KDE and only applies the colors when asked. This way our programs
> are not affected and normal laptop users get basic RGB OOTB.
> 
> If I do that, I will make the quirk for the Ally in a separate patch,
> so that you can nack it if you'd rather introduce RGB support with
> your driver, so that it does not need to be reverted.
> 
> Antheas
> 
> [1] https://github.com/hhd-dev/hhd/blob/d3bbd7fa25fe9a4838896a2c5cfda460abe48dc6/src/hhd/device/rog_ally/const.py#L5-L8

Re: [PATCH 06/11] HID: asus: introduce small delay on Asus Z13 RGB init

[Luke D. Jones]

On 20/03/25 21:30, Antheas Kapenekakis wrote:
> On Thu, 20 Mar 2025 at 08:12, Luke D. Jones <luke@ljones.dev> wrote:
>>
>> On 20/03/25 08:13, Antheas Kapenekakis wrote:
>>> The folio keyboard of the Z13 can get stuck in its BIOS mode, where the
>>> touchpad behaves like a mouse and the keyboard start button is not
>>> reliable if we perform the initialization too quickly. This mostly
>>> happens during boot, and can be verified to be caused by hid-asus
>>> through simple blacklisting. A small delay fixes it.
>>>
>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>> ---
>>>    drivers/hid/hid-asus.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>> index 85ae75478b796..5b75ee83ae290 100644
>>> --- a/drivers/hid/hid-asus.c
>>> +++ b/drivers/hid/hid-asus.c
>>> @@ -571,6 +571,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>        unsigned char kbd_func;
>>>        int ret;
>>>
>>> +     /* Wait a bit before init to prevent locking the keyboard */
>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ROG Flow Z13"))
>>> +             msleep(500);
>>> +
>>>        ret = asus_kbd_init(hdev);
>>>        if (ret < 0)
>>>                return ret;
>>
>> See my comment on patch 1 about trying a loop with the init
>> request/response as a hopeful way to test readiness.
>>
>> Cheers,
>> Luke.
> 
> Turns out there isn't an init problem. I have removed this patch from V2.
> 
> It was hid-asus taking control of the touchpad from hid-multitouch. So
> adding HID_GROUP_GENERIC fixes this. I replaced it with that and
> squashed the rename patch alongside it.
> 
> Antheas

Awesome. Is this the one on `#define INPUT_REPORT_ID 0x5d`?

Re: [PATCH 01/11] HID: asus: refactor init sequence per spec

[Antheas Kapenekakis]

On Thu, 20 Mar 2025 at 22:01, Luke D. Jones <luke@ljones.dev> wrote:
>
> On 20/03/25 22:50, Antheas Kapenekakis wrote:
> > On Thu, 20 Mar 2025 at 08:19, Luke D. Jones <luke@ljones.dev> wrote:
> >>
> >>
> >> On 20/03/25 08:13, Antheas Kapenekakis wrote:
> >>> Currently, asus_kbd_init() uses a reverse engineered init sequence
> >>> from Windows, which contains the handshakes from multiple programs.
> >>> Keep the main one, which is 0x5a (meant for drivers).
> >>
> >> 0x5A is also used for Ally setup commands, used from userspace in
> >> Windows. Only a nit but I don't think stating it's only for drivers is
> >> accurate but then again asus kind of blurs the line a bit.
> >
> > ROG devices contain a HID USB endpoint that exposes multiple
> > applications. On my Z13, that is 4 hiddev devices.
> >
> > However, we only care about two. Those are:
> >
> > Application / Report ID:
> > 0xff310076 / 0x5a meant for Asus drivers
> > 0xff310079 / 0x5d meant for Asus applications
>
> I'm curious how you determined that. From what I've seen asus are
> consistent until they're not - typically it's when they have a weird
> device like the Slash or Anime. The slash on the G16 uses the same MCU
> and endpoint as the keyboard, but with a new report ID, while the slash
> on a G14 is a separate MCU (or at least device exposed by MCU) with own
> endpoints and hid report.
>
> It's not important here yet, I'm just trying to add context and provide
> some extra information for you.
>
> > Both require the same handshake when they start. Well, in theory. But
> > as you say in some of the Anime stuff requires it in practice too.
>
> Yeah as above, the G14 slash needs it, so does the older Anime dispaly.
> But the G16 slash being on the same MCU doesn't. In any case those
> gadgets are being handled in userspace just fine regardless of the
> driver state. Still, not really relevant here and I'm just adding context.
>
> > The handshake is set_report 0x5X + "Asus...", then get_report with the
> > same ID which should return the asus string.
> >
> > In hiddraw, they appear under the same endpoint, both on the Ally and
> > the Z13. But in hiddev (with hid-asus disabled or with this series),
> > they appear as separate.
> >
> > I cannot comment on the Aura protocol, because I don't know, but for
> > the basic sticky RGB mode that supports set and apply, they _should_
> > behave identically. I use 0x5d in my userspace software for everything
> > now [1]. Previously, I used 0x5a but I am not a driver.
> >
> > They do behave identically on the Ally X and the Z13 2025 though.
>
> It is something I do have to test, and I'll add your v2 to kernels to
> get some feedback from my discord group too. I know for sure that there
> were some laptops that didn't accept 0x5D for brightness - I'm sorry I'm
> so vague on this part, I really don't remember the details and I lost
> notes so it will need to be tested.

Well, different ODMs different years stuff changes, theory is theory.
I looked it up and I still use 0x5a for the Ally for RGB.

I am almost done with an initial version of the RGB patch. KDE seems
to not be cooperating and setting the brightness to 0 though. I would
not say it is ready to consume yet. The rest of the series as a V2
should work well though. Should post it in around an hour.

Antheas

> > I do not know about 0x5e. Perhaps Asus made a special endpoint for
> > their Anime creation app.
>
> It is only for that yes.
>
> Cheers,
> Luke.
>
> >>> In addition, perform a get_response and check if the response is the
> >>> same. To avoid regressions, print an error if the response does not
> >>> match instead of rejecting device.
> >>>
> >>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> >>> ---
> >>>    drivers/hid/hid-asus.c | 82 +++++++++++++++++++++++-------------------
> >>>    1 file changed, 46 insertions(+), 36 deletions(-)
> >>>
> >>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >>> index 46e3e42f9eb5f..aa4a481dc4f27 100644
> >>> --- a/drivers/hid/hid-asus.c
> >>> +++ b/drivers/hid/hid-asus.c
> >>> @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> >>>    #define FEATURE_REPORT_ID 0x0d
> >>>    #define INPUT_REPORT_ID 0x5d
> >>>    #define FEATURE_KBD_REPORT_ID 0x5a
> >>> -#define FEATURE_KBD_REPORT_SIZE 16
> >>> +#define FEATURE_KBD_REPORT_SIZE 64
> >>>    #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> >>>    #define FEATURE_KBD_LED_REPORT_ID2 0x5e
> >>>
> >>> @@ -386,16 +386,43 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
> >>>        return ret;
> >>>    }
> >>>
> >>> -static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> >>> +static int asus_kbd_init(struct hid_device *hdev)
> >>>    {
> >>> -     const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> >>> -                  0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> >>> +     /*
> >>> +      * Asus handshake identifying us as a driver (0x5A)
> >>> +      * 0x5A then ASCII for "ASUS Tech.Inc."
> >>> +      * 0x5D is for userspace Windows applications.
> >>
> >> 0x5D is the report ID used for commands such as RGB modes. Probably
> >> don't need to mention it here, and only where it is used.
> >
> > Yep, see above. Not required for basic RGB. Maybe it is for Aura, but
> > I'd leave that to userspace.
> >
> >>> +      * The handshake is first sent as a set_report, then retrieved
> >>> +      * from a get_report to verify the response.
> >>> +      */
> >>> +     const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20,
> >>> +             0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> >>> +     u8 *readbuf;
> >>>        int ret;
> >>>
> >>>        ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> >>> -     if (ret < 0)
> >>> -             hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> >>> +     if (ret < 0) {
> >>> +             hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> >>> +             return ret;
> >>> +     }
> >>>
> >>> +     readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> >>> +     if (!readbuf)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     ret = hid_hw_raw_request(hdev, FEATURE_KBD_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_err(hdev, "Asus handshake returned invalid response: %*ph\n",
> >>> +                     FEATURE_KBD_REPORT_SIZE, readbuf);
> >>> +             // Do not return error if handshake is wrong to avoid regressions
> >>
> >> I'll have to test this on the oldest model I have. Hopefully it's a
> >> non-issue and this can return error instead.
> >>
> >> Side-note: I notice you're using a msleep to try and work around an
> >> issue in a later patch - it might be worth trying replacing that with a
> >> retry/count loop with an inner of small msleep + a call to this init,
> >> see if it still responds to this during that critical period.
> >
> > The call did not fail. I was thinking it was because the device needs
> > some time to warm up (it happens with certain devices).
> >
> > Turns out it was hid-multitouch not attaching.
> >
> >>> +     }
> >>> +
> >>> +     kfree(readbuf);
> >>>        return ret;
> >>>    }
> >>>
> >>> @@ -540,42 +567,25 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >>>        unsigned char kbd_func;
> >>>        int ret;
> >>>
> >>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> >>> -             /* Initialize keyboard */
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> -
> >>> -             /* The LED endpoint is initialised in two HID */
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> -
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>
> >> Ah, I recall now. Some devices like the Slash or AniMe Matrix required
> >> the 0x5E and 0x5D report ID (device dependent) however these are
> >> currently being done via userspace due to not being HID devices.
> >>
> >> There *are* some older laptops still in use that require init on 0x5E or
> >> 0x5D for RGB to be usable, from memory. It's been over 5 years so I'll
> >> pull out the laptop I have with 0x1866 PID MCU and see if that is
> >> actually true and not just my imagination.
> >
> > Hopefully you handshake with these devices over userspace, so they
> > will not be affected.
> >
> >>> +     ret = asus_kbd_init(hdev);
> >>> +     if (ret < 0)
> >>> +             return ret;
> >>>
> >>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>> -                     ret = asus_kbd_disable_oobe(hdev);
> >>> -                     if (ret < 0)
> >>> -                             return ret;
> >>> -             }
> >>> -     } else {
> >>> -             /* Initialize keyboard */
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> +     /* Get keyboard functions */
> >>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>> +     if (ret < 0)
> >>> +             return ret;
> >>>
> >>> -             /* Get keyboard functions */
> >>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>> +             ret = asus_kbd_disable_oobe(hdev);
> >>>                if (ret < 0)
> >>>                        return ret;
> >>> -
> >>> -             /* Check for backlight support */
> >>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>> -                     return -ENODEV;
> >>>        }
> >>>
> >>> +     /* Check for backlight support */
> >>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>> +             return -ENODEV;
> >>> +
> >>>        drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> >>>                                              sizeof(struct asus_kbd_leds),
> >>>                                              GFP_KERNEL);
> >>
> >> I've left only small comments on a few patches for now. I'll review in
> >> full after I get testing done on a variety of devices whcih I'm aiming
> >> for this weekend. Overall impression so far is everything looks good and
> >> this is a nice improvement. Thank you for taking the time to implement it.
> >>
> >> Cheers,
> >> Luke.
> >
> > I'll try to have V2 out today. I finished it yesterday and fixed all
> > the lockups and the hid-multitouch issue. Just needs a good
> > lookthrough.
> >
> > Perhaps I will also do a small multi-intensity endpoint that works
> > with KDE and only applies the colors when asked. This way our programs
> > are not affected and normal laptop users get basic RGB OOTB.
> >
> > If I do that, I will make the quirk for the Ally in a separate patch,
> > so that you can nack it if you'd rather introduce RGB support with
> > your driver, so that it does not need to be reverted.
> >
> > Antheas
> >
> > [1] https://github.com/hhd-dev/hhd/blob/d3bbd7fa25fe9a4838896a2c5cfda460abe48dc6/src/hhd/device/rog_ally/const.py#L5-L8
>

Re: [PATCH 04/11] HID: asus: rename keyboard3 to Z13_FOLIO

[Luke D. Jones]

On 20/03/25 08:13, Antheas Kapenekakis wrote:
> Rename the generic keyboard3 to Z13_FOLIO as it refers to the folio of
> the Z13. Both 2023 and 2025 variants.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/hid/hid-asus.c | 2 +-
>   drivers/hid/hid-ids.h  | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 490a7ea369961..cdd9d9c4fc95f 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1332,7 +1332,7 @@ static const struct hid_device_id asus_devices[] = {
>   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
>   	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> -	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
> +	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
>   	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 7e400624908e3..b1fe7582324ff 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -209,7 +209,7 @@
>   #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822
>   #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD	0x1866
>   #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2	0x19b6
> -#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3	0x1a30
> +#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO		0x1a30

Hi Antheas,

I checked history of the Z13 and use of the 0x1a30 PID and it appears 
only the Z13 folio ever used this PID.


Reviewed-by: Luke D. Jones <luke@ljones.dev>


>   #define USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR		0x18c6
>   #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY		0x1abe
>   #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X		0x1b4c

Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[Hans de Goede]

Hi Antheas,

On 19-Mar-25 20:13, Antheas Kapenekakis wrote:
> This is a three part series that does the following: first, it cleans up
> the hid-asus driver initialization, preventing excess renames and dmesg
> errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
> by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
> key. Finally, the bigger piece of this series is the unification of the
> backlight controls between hid-asus and asus-wmi.

Thank you for your work on this.

> This requires some context. First, some ROG devices expose both WMI and
> HID controls for RGB. In addition, some ROG devices (such as the Z13)
> have two AURA devices where both have backlight controls (lightbar and
> keyboard). Under Windows, Armoury Crate exposes a single brightness control
> for all Aura devices.
> 
> However, currently in the linux kernel this is not the case, with asus-wmi
> and hid-asus relying on a quirk system to figure out which should control
> the backlight. But what about the other one? There might be silent
> regressions such as part of the RGB of the device not responding properly.
> 
> In the Z13, this is the case, with a race condition causing the lightbar
> to control the asus::kbd_backlight device most of the time, with the
> keyboard being renamed to asus::kbd_backlight_1 and not doing anything
> under KDE controls.
> 
> Here, we should note that most backlight handlers are hardcoded to check
> for backlight once, and for one backlight, during boot, so any other
> solution would require a large rewrite of userspace.

Note that work is actually ongoing to add support for multiple kbd
backlights to upower:

https://gitlab.freedesktop.org/upower/upower/-/merge_requests/258

But that is intended for when there are 2 kbds with a controllable backlight,
e.g. a docked laptop with a gaming kbd with RGB backlight connected to the dock.

Where as here we seem to have 2 controls which ideally should be set to
the same value if I understand things correctly ?

> Even when brightness controls are fixed, we still have the problem of the
> backlight key being on/off when controlled by KDE and 0/33/66/100 when
> the device has a WMI keyboard. Ideally, we would like the 0/33/66/100 to
> be done under hid as well, regardless of whether the backlight of the
> device is controlled by WMI or HID.

Hmm, ideally we want this sort of policy to be in userspace, this sounds
more like it is a keycode problem and we maybe need KEY_KBDILLUMCYCLE next
to the existing KEY_KBDILLUMTOGGLE. For the existing toggle doing on/off
obviously is the correct userspace behavior.

Anyways I can see how Asus is special here and on laptops the cycling is
typically handled by the EC and we have chosen to emulate EC behavior in
the kernel before to keep things consistent amongst models.

Still generally speaking we do prefer to just send keypresses when possible
and let userspace set the policy, but I guess we can make an exception here.

> Therefore, this is what the third part of this series does. It sets up
> asus-wmi to expose accepting listeners for the asus::kbd_backlight device
> and being the one that sets it up. Then, it makes hid-asus devices
> register a listener there, so that all of them are controlled by
> asus::kbd_backlight. Finally, it adds an event handler for keyboard keys,
> so that HID led controls are handled by the kernel instead of userspace.
> This way, even when userspace is not active the key works, and we get the
> desired behavior of 0/33/66/100 across all Aura devices (currently, that
> is keyboards, and embedded devices such as lightbars). This results
> removing the quirk system as well, eliminating a point of failure.

I've taken a quick look at the new API between asus-wmi and asus-hid and
this looks good to me, thank you for your work on this.

Regards,

Hans

Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements

[Antheas Kapenekakis]

On Mon, 24 Mar 2025 at 13:10, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Antheas,
>
> On 19-Mar-25 20:13, Antheas Kapenekakis wrote:
> > This is a three part series that does the following: first, it cleans up
> > the hid-asus driver initialization, preventing excess renames and dmesg
> > errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
> > by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
> > key. Finally, the bigger piece of this series is the unification of the
> > backlight controls between hid-asus and asus-wmi.
>
> Thank you for your work on this.
>
> > This requires some context. First, some ROG devices expose both WMI and
> > HID controls for RGB. In addition, some ROG devices (such as the Z13)
> > have two AURA devices where both have backlight controls (lightbar and
> > keyboard). Under Windows, Armoury Crate exposes a single brightness control
> > for all Aura devices.
> >
> > However, currently in the linux kernel this is not the case, with asus-wmi
> > and hid-asus relying on a quirk system to figure out which should control
> > the backlight. But what about the other one? There might be silent
> > regressions such as part of the RGB of the device not responding properly.
> >
> > In the Z13, this is the case, with a race condition causing the lightbar
> > to control the asus::kbd_backlight device most of the time, with the
> > keyboard being renamed to asus::kbd_backlight_1 and not doing anything
> > under KDE controls.
> >
> > Here, we should note that most backlight handlers are hardcoded to check
> > for backlight once, and for one backlight, during boot, so any other
> > solution would require a large rewrite of userspace.
>
> Note that work is actually ongoing to add support for multiple kbd
> backlights to upower:
>
> https://gitlab.freedesktop.org/upower/upower/-/merge_requests/258
>
> But that is intended for when there are 2 kbds with a controllable backlight,
> e.g. a docked laptop with a gaming kbd with RGB backlight connected to the dock.
>
> Where as here we seem to have 2 controls which ideally should be set to
> the same value if I understand things correctly ?

Yes, there can be a HID device and a WMI device or multiple HID
devices, and currently the driver is quirked to either select HID or
WMI based on laptop model. There is also a deviation between how WMI
is handled and how HID is handled. This way we unify all of it.

In addition, on the Z13, we have a lightbar and a keyboard backlight
(both HID/separate USB devices). And the keyboard is removable. On the
Ally, we have a backlight but the controller disappears before sleep
because WIndows does not support selective suspend for xinput. By
placing the handler on WMI we ensure it is always active and the state
does not get lost.

> > Even when brightness controls are fixed, we still have the problem of the
> > backlight key being on/off when controlled by KDE and 0/33/66/100 when
> > the device has a WMI keyboard. Ideally, we would like the 0/33/66/100 to
> > be done under hid as well, regardless of whether the backlight of the
> > device is controlled by WMI or HID.
>
> Hmm, ideally we want this sort of policy to be in userspace, this sounds
> more like it is a keycode problem and we maybe need KEY_KBDILLUMCYCLE next
> to the existing KEY_KBDILLUMTOGGLE. For the existing toggle doing on/off
> obviously is the correct userspace behavior.
>
> Anyways I can see how Asus is special here and on laptops the cycling is
> typically handled by the EC and we have chosen to emulate EC behavior in
> the kernel before to keep things consistent amongst models.
>
> Still generally speaking we do prefer to just send keypresses when possible
> and let userspace set the policy, but I guess we can make an exception here.

Yeah I agree with this, but now the WMI driver sets a bit of a precedent.

> > Therefore, this is what the third part of this series does. It sets up
> > asus-wmi to expose accepting listeners for the asus::kbd_backlight device
> > and being the one that sets it up. Then, it makes hid-asus devices
> > register a listener there, so that all of them are controlled by
> > asus::kbd_backlight. Finally, it adds an event handler for keyboard keys,
> > so that HID led controls are handled by the kernel instead of userspace.
> > This way, even when userspace is not active the key works, and we get the
> > desired behavior of 0/33/66/100 across all Aura devices (currently, that
> > is keyboards, and embedded devices such as lightbars). This results
> > removing the quirk system as well, eliminating a point of failure.
>
> I've taken a quick look at the new API between asus-wmi and asus-hid and
> this looks good to me, thank you for your work on this.
>
> Regards,
>
> Hans
>
>