| Message ID | 20240529012827.146005-3-luke@ljones.dev (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | asus wmi and hid: use HID LED for brightness | expand |
Hi, On 5/29/24 3:28 AM, Luke D. Jones wrote: > On some laptops the report_id used for LED brightness control must be > 0x5D instead of 0x5A. > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > --- > drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > index 4cba8e143031..ec3556cc4eef 100644 > --- a/drivers/hid/hid-asus.c > +++ b/drivers/hid/hid-asus.c > @@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad"); > > #define TRKID_SGN ((TRKID_MAX + 1) >> 1) > > +static const char * const use_alt_led_report_id[] = { "GU605", "GA403" }; > + > struct asus_kbd_leds { > struct led_classdev cdev; > struct hid_device *hdev; > @@ -101,6 +103,7 @@ struct asus_kbd_leds { > unsigned int brightness; > spinlock_t lock; > bool removed; > + int report_id; > }; > > struct asus_touchpad_info { > @@ -473,7 +476,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; > > @@ -513,6 +516,23 @@ 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) > +{ > + const char *product; > + int i; > + > + product = dmi_get_system_info(DMI_PRODUCT_NAME); > + if (!product) > + return false; > + > + for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) { > + if (strstr(product, use_alt_led_report_id[i])) > + return true; > + } > + > + return false; > +} > + This again feels like you are re-inventing dmi_check_system() please change use_alt_led_report_id to a dmi_system_id array and drop this function (you can directly call dmi_check_system(use_alt_led_report_id) instead). > static int asus_kbd_register_leds(struct hid_device *hdev) > { > struct asus_drvdata *drvdata = hid_get_drvdata(hdev); > @@ -555,6 +575,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; Regards, Hans
On Wed, 29 May 2024, at 8:25 PM, Hans de Goede wrote: > Hi, > > On 5/29/24 3:28 AM, Luke D. Jones wrote: > > On some laptops the report_id used for LED brightness control must be > > 0x5D instead of 0x5A. > > > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > > --- > > drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > > index 4cba8e143031..ec3556cc4eef 100644 > > --- a/drivers/hid/hid-asus.c > > +++ b/drivers/hid/hid-asus.c > > @@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad"); > > > > #define TRKID_SGN ((TRKID_MAX + 1) >> 1) > > > > +static const char * const use_alt_led_report_id[] = { "GU605", "GA403" }; > > + > > struct asus_kbd_leds { > > struct led_classdev cdev; > > struct hid_device *hdev; > > @@ -101,6 +103,7 @@ struct asus_kbd_leds { > > unsigned int brightness; > > spinlock_t lock; > > bool removed; > > + int report_id; > > }; > > > > struct asus_touchpad_info { > > @@ -473,7 +476,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; > > > > @@ -513,6 +516,23 @@ 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) > > +{ > > + const char *product; > > + int i; > > + > > + product = dmi_get_system_info(DMI_PRODUCT_NAME); > > + if (!product) > > + return false; > > + > > + for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) { > > + if (strstr(product, use_alt_led_report_id[i])) > > + return true; > > + } > > + > > + return false; > > +} > > + > > > This again feels like you are re-inventing dmi_check_system() please > change use_alt_led_report_id to a dmi_system_id array and drop this > function (you can directly call dmi_check_system(use_alt_led_report_id) > instead). I wasn't actually aware of that, so i guess I've learned a whole lot of new stuff today :) Thanks mate, I'll get round to a v2 in the next few days. > > > static int asus_kbd_register_leds(struct hid_device *hdev) > > { > > struct asus_drvdata *drvdata = hid_get_drvdata(hdev); > > @@ -555,6 +575,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; > > Regards, > > Hans > > >
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 4cba8e143031..ec3556cc4eef 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c @@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad"); #define TRKID_SGN ((TRKID_MAX + 1) >> 1) +static const char * const use_alt_led_report_id[] = { "GU605", "GA403" }; + struct asus_kbd_leds { struct led_classdev cdev; struct hid_device *hdev; @@ -101,6 +103,7 @@ struct asus_kbd_leds { unsigned int brightness; spinlock_t lock; bool removed; + int report_id; }; struct asus_touchpad_info { @@ -473,7 +476,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; @@ -513,6 +516,23 @@ 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) +{ + const char *product; + int i; + + product = dmi_get_system_info(DMI_PRODUCT_NAME); + if (!product) + return false; + + for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) { + if (strstr(product, use_alt_led_report_id[i])) + return true; + } + + return false; +} + static int asus_kbd_register_leds(struct hid_device *hdev) { struct asus_drvdata *drvdata = hid_get_drvdata(hdev); @@ -555,6 +575,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;
On some laptops the report_id used for LED brightness control must be 0x5D instead of 0x5A. Signed-off-by: Luke D. Jones <luke@ljones.dev> --- drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)