[PATCH v3 0/1] hid-asus: use hid for keyboard brightness control

[PATCH v3 0/1] hid-asus: use hid for keyboard brightness control

[Luke D. Jones]

Refactored V1 of patch to remove the report_id change as it isn't required.

Changelog:
- V3
  - Fix an issue with array initialisation
- V2
  - Split patch in two (use hid, and change report_id)
  - Drop the report_id changing as not required
  - Use dmi_check_system() for DMI matching

History:
- https://lore.kernel.org/linux-input/20240528013959.14661-1-luke@ljones.dev/T/#u

Luke D. Jones (1):
  hid-asus: use hid for brightness control on keyboard

 drivers/hid/hid-asus.c                     |  7 +++++
 drivers/platform/x86/asus-wmi.c            |  3 +-
 include/linux/platform_data/x86/asus-wmi.h | 36 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

-- 
2.45.2

[PATCH v3 1/1] hid-asus: use hid for brightness control on keyboard

[Luke D. Jones]

On almost all ASUS ROG series laptops the MCU used for the USB keyboard
also has a HID packet used for setting the brightness. This is usually
the same as the WMI method. But in some laptops the WMI method either
is missing or doesn't work, so we should default to the HID control.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/hid/hid-asus.c                     |  7 +++++
 drivers/platform/x86/asus-wmi.c            |  3 +-
 include/linux/platform_data/x86/asus-wmi.h | 36 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 37e6d25593c2..af57a5f03193 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -492,12 +492,19 @@ static void asus_kbd_backlight_work(struct work_struct *work)
  */
 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);
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3f9b6285c9a6..799d928c7d3d 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1681,7 +1681,8 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 			goto error;
 	}
 
-	if (!kbd_led_read(asus, &led_val, NULL)) {
+	if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
+		pr_info("using asus-wmi for asus::kbd_backlight\n");
 		asus->kbd_led_wk = led_val;
 		asus->kbd_led.name = "asus::kbd_backlight";
 		asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 3eb5cd6773ad..74b32e1d6735 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -4,6 +4,7 @@
 
 #include <linux/errno.h>
 #include <linux/types.h>
+#include <linux/dmi.h>
 
 /* WMI Methods */
 #define ASUS_WMI_METHODID_SPEC	        0x43455053 /* BIOS SPECification */
@@ -160,4 +161,39 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
 }
 #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_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.45.2

Re: [PATCH v3 1/1] hid-asus: use hid for brightness control on keyboard

[Hans de Goede]

Hi,

On 7/13/24 9:47 AM, Luke D. Jones wrote:
> On almost all ASUS ROG series laptops the MCU used for the USB keyboard
> also has a HID packet used for setting the brightness. This is usually
> the same as the WMI method. But in some laptops the WMI method either
> is missing or doesn't work, so we should default to the HID control.
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans




> ---
>  drivers/hid/hid-asus.c                     |  7 +++++
>  drivers/platform/x86/asus-wmi.c            |  3 +-
>  include/linux/platform_data/x86/asus-wmi.h | 36 ++++++++++++++++++++++
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 37e6d25593c2..af57a5f03193 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -492,12 +492,19 @@ static void asus_kbd_backlight_work(struct work_struct *work)
>   */
>  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);
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 3f9b6285c9a6..799d928c7d3d 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -1681,7 +1681,8 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
>  			goto error;
>  	}
>  
> -	if (!kbd_led_read(asus, &led_val, NULL)) {
> +	if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
> +		pr_info("using asus-wmi for asus::kbd_backlight\n");
>  		asus->kbd_led_wk = led_val;
>  		asus->kbd_led.name = "asus::kbd_backlight";
>  		asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 3eb5cd6773ad..74b32e1d6735 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -4,6 +4,7 @@
>  
>  #include <linux/errno.h>
>  #include <linux/types.h>
> +#include <linux/dmi.h>
>  
>  /* WMI Methods */
>  #define ASUS_WMI_METHODID_SPEC	        0x43455053 /* BIOS SPECification */
> @@ -160,4 +161,39 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
>  }
>  #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_BOARD_NAME, "GA403U"),
> +		},
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
> +		},
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> +		},
> +	},
> +	{ },
> +};
> +
>  #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */

Re: [PATCH v3 1/1] hid-asus: use hid for brightness control on keyboard

[Josh Leivenzon]

Hello,

Thanks for this patch. I'm looking at doing something similar for the Zenbook 
Duo 2024 keyboard, which, I'm discovering, is similar to the ROG_NKEY_KEYBOARD 
devices in a number of ways.

Something unique about the Zenbook Duo keyboard is that it has an external 
data-capable USB-C port, meaning it can be used as a regular USB keyboard for 
any device. This means that even if WMI features work, it is highly preferable 
to use HID alternatives where possible in order to allow those features to work 
on any Linux device that the keyboard is plugged in to.

This leads me to my question: Why the DMI check on the HID side? This is a 
little problematic for me, as it blocks functionallity on other devices. Would 
it be sufficient to use quirks only in that case? The important part, to my 
understanding, is that the WMI module disables itself properly, which is still 
covered by its own DMI check.

Are there devices that use the ROG_NKEY_KEYBOARDs where HID doesn't work?

Re: [PATCH v3 1/1] hid-asus: use hid for brightness control on keyboard

[Luke Jones]

On Mon, 13 Jan 2025, at 1:18 AM, Josh Leivenzon wrote:
> Hello,
>
> Thanks for this patch. I'm looking at doing something similar for the Zenbook 
> Duo 2024 keyboard, which, I'm discovering, is similar to the ROG_NKEY_KEYBOARD 
> devices in a number of ways.
>
> Something unique about the Zenbook Duo keyboard is that it has an external 
> data-capable USB-C port, meaning it can be used as a regular USB keyboard for 
> any device. This means that even if WMI features work, it is highly preferable 
> to use HID alternatives where possible in order to allow those features to work 
> on any Linux device that the keyboard is plugged in to.
>
> This leads me to my question: Why the DMI check on the HID side? This is a 

It's because I couldn't see a good way to both disable the WMI backlight and use HID. On some laptops both WMI and HID work, but on some WMI methods exist but don't work at all. The DMI check ends up being something of a blanket check across those family or models as they all used different generations of the MCU at one point but all have HID brightness control.

The key part is we need to tell asus-wmi not to use WMI for brightness on those product ranges. I have at least 4 years of data in this rather disorganised repo if you're curious about other models (And I always take contributions) - https://gitlab.com/asus-linux/reverse-engineering

> little problematic for me, as it blocks functionallity on other devices. Would 
> it be sufficient to use quirks only in that case? The important part, to my 
> understanding, is that the WMI module disables itself properly, which is still 
> covered by its own DMI check.

asus-wmi is still used for many things, so it's really only the LED handling that needs disabling when required.

> Are there devices that use the ROG_NKEY_KEYBOARDs where HID doesn't work?

Not that I know of, and I think this goes right back to 2019 with the:
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD	0x1866

These ones span 2015-2019 I think, and are rare to find in use still, but from what I've managed to find and from the odd report back they also use HID:
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1 0x1854
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD2 0x1837
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822

I would love to have someone withe the hardware help to make this device better for all users. Some issues that may be useful or related are here:

https://github.com/flukejones/asusctl/issues/25

https://github.com/alesya-h/zenbook-duo-2024-ux8406ma-linux/pull/12

I also have a WIP branch here https://github.com/flukejones/linux/tree/wip/ally-6.13 (ignore name, it covers all asus)

If there is anything I can do to further help you please ping whenever you need.