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 <[email protected]>
---
drivers/hid/hid-asus.c | 19 ++++++++++++-
drivers/platform/x86/asus-wmi.c | 3 ++-
include/linux/platform_data/x86/asus-wmi.h | 31 ++++++++++++++++++++++
3 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 02de2bf4f790..9389a3e733e3 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -101,6 +101,7 @@ struct asus_kbd_leds {
unsigned int brightness;
spinlock_t lock;
bool removed;
+ int report_id;
};
struct asus_touchpad_info {
@@ -473,7 +474,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
static void asus_kbd_backlight_work(struct work_struct *work)
{
struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
- u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
+ u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
int ret;
unsigned long flags;
@@ -492,12 +493,18 @@ 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 && asus_use_hidraw_led()) {
+ hid_info(hdev, "using hidraw 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);
@@ -507,6 +514,12 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
}
+static bool asus_kbd_is_input_led(void)
+{
+ return dmi_match(DMI_PRODUCT_NAME, "GU605")
+ || dmi_match(DMI_PRODUCT_NAME, "GA403");
+}
+
static int asus_kbd_register_leds(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -549,6 +562,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (!drvdata->kbd_backlight)
return -ENOMEM;
+ drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
+ drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
+
drvdata->kbd_backlight->removed = false;
drvdata->kbd_backlight->brightness = 0;
drvdata->kbd_backlight->hdev = hdev;
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3f9b6285c9a6..a58df18a70ad 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) && !asus_use_hidraw_led()) {
+ 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..79a50102440d 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -160,4 +160,35 @@ 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 */
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+static bool asus_use_hidraw_led(void)
+{
+ const char *product, *board;
+
+ product = dmi_get_system_info(DMI_PRODUCT_FAMILY);
+ if (!product)
+ return false;
+
+ /* These product ranges should all be using HID for keyboard LED */
+ if (strstr(product, "ROG Zephyrus")
+ || strstr(product, "ROG Strix")
+ || strstr(product, "ROG Flow")
+ || strstr(product, "GA403")
+ || strstr(product, "GU605"))
+ return true;
+
+ board = dmi_get_system_info(DMI_BOARD_NAME);
+ if (!board)
+ return false;
+
+ return strstr(board, "RC71L"); /* ROG Ally specific */
+}
+#else
+static inline bool asus_use_hidraw_led(void)
+{
+ return true;
+}
+#endif
+
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.45.1
On Tue, 28 May 2024, 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 <[email protected]>
> ---
> drivers/hid/hid-asus.c | 19 ++++++++++++-
> drivers/platform/x86/asus-wmi.c | 3 ++-
> include/linux/platform_data/x86/asus-wmi.h | 31 ++++++++++++++++++++++
> 3 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 02de2bf4f790..9389a3e733e3 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -101,6 +101,7 @@ struct asus_kbd_leds {
> unsigned int brightness;
> spinlock_t lock;
> bool removed;
> + int report_id;
> };
>
> struct asus_touchpad_info {
> @@ -473,7 +474,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> static void asus_kbd_backlight_work(struct work_struct *work)
> {
> struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> - u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
> + u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
> int ret;
> unsigned long flags;
>
> @@ -492,12 +493,18 @@ 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 && asus_use_hidraw_led()) {
> + hid_info(hdev, "using hidraw 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);
> @@ -507,6 +514,12 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> }
>
> +static bool asus_kbd_is_input_led(void)
> +{
> + return dmi_match(DMI_PRODUCT_NAME, "GU605")
> + || dmi_match(DMI_PRODUCT_NAME, "GA403");
> +}
> +
> static int asus_kbd_register_leds(struct hid_device *hdev)
> {
> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> @@ -549,6 +562,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (!drvdata->kbd_backlight)
> return -ENOMEM;
>
> + drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
> + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
> + drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
> +
> drvdata->kbd_backlight->removed = false;
> drvdata->kbd_backlight->brightness = 0;
> drvdata->kbd_backlight->hdev = hdev;
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 3f9b6285c9a6..a58df18a70ad 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) && !asus_use_hidraw_led()) {
> + 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..79a50102440d 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -160,4 +160,35 @@ 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 */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +static bool asus_use_hidraw_led(void)
Since it's in a header, it's missing inline. However, this function looks
quite complicated so putting it into a header file is questionable to
begin with so I'd prefer it to be in a .c file.
> +{
> + const char *product, *board;
> +
> + product = dmi_get_system_info(DMI_PRODUCT_FAMILY);
> + if (!product)
> + return false;
> +
> + /* These product ranges should all be using HID for keyboard LED */
> + if (strstr(product, "ROG Zephyrus")
> + || strstr(product, "ROG Strix")
> + || strstr(product, "ROG Flow")
> + || strstr(product, "GA403")
> + || strstr(product, "GU605"))
Please align these properly but consider using array and loop.
> + return true;
> +
> + board = dmi_get_system_info(DMI_BOARD_NAME);
> + if (!board)
> + return false;
> +
> + return strstr(board, "RC71L"); /* ROG Ally specific */
> +}
> +#else
> +static inline bool asus_use_hidraw_led(void)
> +{
> + return true;
> +}
> +#endif
> +
> #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
>
--
i.
On Tue, 28 May 2024, at 8:36 PM, Ilpo Järvinen wrote:
> On Tue, 28 May 2024, 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 <[email protected]>
> > ---
> > drivers/hid/hid-asus.c | 19 ++++++++++++-
> > drivers/platform/x86/asus-wmi.c | 3 ++-
> > include/linux/platform_data/x86/asus-wmi.h | 31 ++++++++++++++++++++++
> > 3 files changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 02de2bf4f790..9389a3e733e3 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -101,6 +101,7 @@ struct asus_kbd_leds {
> > unsigned int brightness;
> > spinlock_t lock;
> > bool removed;
> > + int report_id;
> > };
> >
> > struct asus_touchpad_info {
> > @@ -473,7 +474,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> > static void asus_kbd_backlight_work(struct work_struct *work)
> > {
> > struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> > - u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
> > + u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
> > int ret;
> > unsigned long flags;
> >
> > @@ -492,12 +493,18 @@ 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 && asus_use_hidraw_led()) {
> > + hid_info(hdev, "using hidraw 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);
> > @@ -507,6 +514,12 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> > return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> > }
> >
> > +static bool asus_kbd_is_input_led(void)
> > +{
> > + return dmi_match(DMI_PRODUCT_NAME, "GU605")
> > + || dmi_match(DMI_PRODUCT_NAME, "GA403");
> > +}
> > +
> > static int asus_kbd_register_leds(struct hid_device *hdev)
> > {
> > struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> > @@ -549,6 +562,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> > if (!drvdata->kbd_backlight)
> > return -ENOMEM;
> >
> > + drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
> > + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
> > + drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
> > +
> > drvdata->kbd_backlight->removed = false;
> > drvdata->kbd_backlight->brightness = 0;
> > drvdata->kbd_backlight->hdev = hdev;
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 3f9b6285c9a6..a58df18a70ad 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) && !asus_use_hidraw_led()) {
> > + 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..79a50102440d 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -160,4 +160,35 @@ 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 */
> > +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> > +static bool asus_use_hidraw_led(void)
>
> Since it's in a header, it's missing inline. However, this function looks
> quite complicated so putting it into a header file is questionable to
> begin with so I'd prefer it to be in a .c file.
Thanks for the review y'all. All recommendations implemented including this and splitting to two commits.
> > +{
> > + const char *product, *board;
> > +
> > + product = dmi_get_system_info(DMI_PRODUCT_FAMILY);
> > + if (!product)
> > + return false;
> > +
> > + /* These product ranges should all be using HID for keyboard LED */
> > + if (strstr(product, "ROG Zephyrus")
> > + || strstr(product, "ROG Strix")
> > + || strstr(product, "ROG Flow")
> > + || strstr(product, "GA403")
> > + || strstr(product, "GU605"))
>
> Please align these properly but consider using array and loop.
>
> > + return true;
> > +
> > + board = dmi_get_system_info(DMI_BOARD_NAME);
> > + if (!board)
> > + return false;
> > +
> > + return strstr(board, "RC71L"); /* ROG Ally specific */
> > +}
> > +#else
> > +static inline bool asus_use_hidraw_led(void)
> > +{
> > + return true;
> > +}
> > +#endif
> > +
> > #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> >
>
> --
> i.
>
>