| Message ID | 20250322102804.418000-3-lkml@antheas.dev (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | HID: asus: Add RGB Support to Asus Z13, Ally, unify backlight asus-wmi, and Z13 QOL | expand |
On 22/03/25 23:27, Antheas Kapenekakis wrote: > ROG keyboards are HID compliant and only care about the endpoint that > produces vendor events (e.g., fan mode) and has the keyboard backlight. > > Therefore, handle all of the endpoints of ROG keyboards as compliant, > by adding HID_QUIRK_INPUT_PER_APP and, for devices other than the vendor > one, by adding QUIRK_HANDLE_GENERIC to stop mutating them. > > Due to HID_QUIRK_INPUT_PER_APP, rgb register is moved into probe, as > the input_* functions are called multiple times (4 for the Z13). > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> > --- > drivers/hid/hid-asus.c | 69 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 53 insertions(+), 16 deletions(-) > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > index 8d4df1b6f143b..96461321c191c 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 | \ > @@ -120,7 +121,6 @@ struct asus_drvdata { > struct input_dev *tp_kbd_input; > struct asus_kbd_leds *kbd_backlight; > const struct asus_touchpad_info *tp; > - bool enable_backlight; > struct power_supply *battery; > struct power_supply_desc battery_desc; > int battery_capacity; > @@ -326,6 +326,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); > > @@ -774,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) > @@ -827,11 +835,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"); > - > return 0; > } > > @@ -851,6 +854,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 > @@ -901,15 +908,6 @@ static int asus_input_mapping(struct hid_device *hdev, > return -1; > } > > - /* > - * Check and enable backlight only on devices with UsagePage == > - * 0xff31 to avoid initializing the keyboard firmware multiple > - * times on devices with multiple HID descriptors but same > - * PID/VID. > - */ > - if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) > - drvdata->enable_backlight = true; > - > set_bit(EV_REP, hi->input->evbit); > return 1; > } > @@ -1026,8 +1024,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, is_vendor = 0; > > drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL); > if (drvdata == NULL) { > @@ -1111,12 +1111,45 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id) > return ret; > } > > + /* > + * Check for the vendor interface (0xff31) to init the RGB. > + * and handle generic devices properly. > + */ > + rep_enum = &hdev->report_enum[HID_INPUT_REPORT]; > + list_for_each_entry(rep, &rep_enum->report_list, list) { > + if ((rep->application & HID_USAGE_PAGE) == 0xff310000) > + is_vendor = true; > + } > + > + /* > + * For ROG keyboards, make them hid compliant by > + * creating one input per application. For interfaces other than > + * the vendor one, disable hid-asus handlers. > + */ > + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) { > + if (!is_vendor) > + drvdata->quirks |= QUIRK_HANDLE_GENERIC; > + hdev->quirks |= HID_QUIRK_INPUT_PER_APP; > + } > + > ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); > if (ret) { > hid_err(hdev, "Asus hw start failed: %d\n", ret); > return ret; > } > > + if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) && > + !asus_kbd_wmi_led_control_present(hdev) && > + asus_kbd_register_leds(hdev)) > + hid_warn(hdev, "Failed to initialize backlight.\n"); > + > + /* > + * For ROG keyboards, skip rename for consistency and > + * ->input check as some devices do not have inputs. > + */ > + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) > + return 0; > + > if (!drvdata->input) { > hid_err(hdev, "Asus input not registered\n"); > ret = -ENOMEM; > @@ -1167,6 +1200,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"); Ilpo might have some feedback. I'm otherwise happy with this. Reviewed-by: Luke D. Jones <luke@ljones.dev>
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 8d4df1b6f143b..96461321c191c 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 | \ @@ -120,7 +121,6 @@ struct asus_drvdata { struct input_dev *tp_kbd_input; struct asus_kbd_leds *kbd_backlight; const struct asus_touchpad_info *tp; - bool enable_backlight; struct power_supply *battery; struct power_supply_desc battery_desc; int battery_capacity; @@ -326,6 +326,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); @@ -774,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) @@ -827,11 +835,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"); - return 0; } @@ -851,6 +854,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 @@ -901,15 +908,6 @@ static int asus_input_mapping(struct hid_device *hdev, return -1; } - /* - * Check and enable backlight only on devices with UsagePage == - * 0xff31 to avoid initializing the keyboard firmware multiple - * times on devices with multiple HID descriptors but same - * PID/VID. - */ - if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) - drvdata->enable_backlight = true; - set_bit(EV_REP, hi->input->evbit); return 1; } @@ -1026,8 +1024,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, is_vendor = 0; drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL); if (drvdata == NULL) { @@ -1111,12 +1111,45 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id) return ret; } + /* + * Check for the vendor interface (0xff31) to init the RGB. + * and handle generic devices properly. + */ + rep_enum = &hdev->report_enum[HID_INPUT_REPORT]; + list_for_each_entry(rep, &rep_enum->report_list, list) { + if ((rep->application & HID_USAGE_PAGE) == 0xff310000) + is_vendor = true; + } + + /* + * For ROG keyboards, make them hid compliant by + * creating one input per application. For interfaces other than + * the vendor one, disable hid-asus handlers. + */ + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) { + if (!is_vendor) + drvdata->quirks |= QUIRK_HANDLE_GENERIC; + hdev->quirks |= HID_QUIRK_INPUT_PER_APP; + } + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); if (ret) { hid_err(hdev, "Asus hw start failed: %d\n", ret); return ret; } + if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) && + !asus_kbd_wmi_led_control_present(hdev) && + asus_kbd_register_leds(hdev)) + hid_warn(hdev, "Failed to initialize backlight.\n"); + + /* + * For ROG keyboards, skip rename for consistency and + * ->input check as some devices do not have inputs. + */ + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) + return 0; + if (!drvdata->input) { hid_err(hdev, "Asus input not registered\n"); ret = -ENOMEM; @@ -1167,6 +1200,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");
ROG keyboards are HID compliant and only care about the endpoint that produces vendor events (e.g., fan mode) and has the keyboard backlight. Therefore, handle all of the endpoints of ROG keyboards as compliant, by adding HID_QUIRK_INPUT_PER_APP and, for devices other than the vendor one, by adding QUIRK_HANDLE_GENERIC to stop mutating them. Due to HID_QUIRK_INPUT_PER_APP, rgb register is moved into probe, as the input_* functions are called multiple times (4 for the Z13). Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> --- drivers/hid/hid-asus.c | 69 ++++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 16 deletions(-)