On some laptops the report_id used for LED brightness control must be
0x5D instead of 0x5A.
Signed-off-by: Luke D. Jones <[email protected]>
---
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;
+}
+
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;
--
2.45.1
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 <[email protected]>
> ---
> 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 <[email protected]>
> > ---
> > 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
>
>
>