These keyboards have always had initialization in the kernel for 0x5d.
At this point, it is hard to verify again and we risk regressions by
removing this. Therefore, initialize with 0x5d as well.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/hid/hid-asus.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 726f5d8e22d1..221c7195e885 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -91,6 +91,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
#define QUIRK_ROG_ALLY_XPAD BIT(13)
#define QUIRK_SKIP_REPORT_FIXUP BIT(14)
+#define QUIRK_ROG_NKEY_LEGACY BIT(15)
#define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
QUIRK_NO_INIT_REPORTS | \
@@ -669,6 +670,16 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (ret < 0)
return ret;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
+ /*
+ * These keyboards might need 0x5d for shortcuts to work.
+ * As it has been more than 5 years, it is hard to verify.
+ */
+ ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
+ if (ret < 0)
+ return ret;
+ }
+
/* Get keyboard functions */
ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
if (ret < 0)
@@ -1409,10 +1420,10 @@ static const struct hid_device_id asus_devices[] = {
QUIRK_USE_KBD_BACKLIGHT },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
--
2.51.2On Sat, 1 Nov 2025, Antheas Kapenekakis wrote:
> These keyboards have always had initialization in the kernel for 0x5d.
> At this point, it is hard to verify again and we risk regressions by
> removing this. Therefore, initialize with 0x5d as well.
>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/hid/hid-asus.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 726f5d8e22d1..221c7195e885 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -91,6 +91,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> #define QUIRK_SKIP_REPORT_FIXUP BIT(14)
> +#define QUIRK_ROG_NKEY_LEGACY BIT(15)
>
> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
> QUIRK_NO_INIT_REPORTS | \
> @@ -669,6 +670,16 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (ret < 0)
> return ret;
>
> + if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
> + /*
> + * These keyboards might need 0x5d for shortcuts to work.
> + * As it has been more than 5 years, it is hard to verify.
> + */
> + ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> + if (ret < 0)
> + return ret;
> + }
> +
> /* Get keyboard functions */
> ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> if (ret < 0)
> @@ -1409,10 +1420,10 @@ static const struct hid_device_id asus_devices[] = {
> QUIRK_USE_KBD_BACKLIGHT },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
You should do FEATURE_KBD_LED_REPORT_ID1 refactoring together, not remove
+ add back in different patches.
I suppose the cleanest would be to add a new patch as first which moves
asus_kbd_init() outside of if/else so you can make this refactoring of
FEATURE_KBD_LED_REPORT_ID1 in the 2nd patch.
I note there's still contention with this series overall.
--
i.
On 11/18/25 13:10, Ilpo Järvinen wrote:
> On Sat, 1 Nov 2025, Antheas Kapenekakis wrote:
>
>> These keyboards have always had initialization in the kernel for 0x5d.
>> At this point, it is hard to verify again and we risk regressions by
>> removing this. Therefore, initialize with 0x5d as well.
See patch 1: unless I missed something you can retain the two
FEATURE_KBD_LED_REPORT_IDx behind the same exact quirk:
why are we adding a quirk to replace a quirk that was removed
in patch 1?
You are basically doing the pretty-much-but-not-quite
equivalent of what the driver was doing before.
>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>> ---
>> drivers/hid/hid-asus.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index 726f5d8e22d1..221c7195e885 100644
>> --- a/drivers/hid/hid-asus.c
>> +++ b/drivers/hid/hid-asus.c
>> @@ -91,6 +91,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
>> #define QUIRK_ROG_ALLY_XPAD BIT(13)
>> #define QUIRK_SKIP_REPORT_FIXUP BIT(14)
>> +#define QUIRK_ROG_NKEY_LEGACY BIT(15)
>>
>> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
>> QUIRK_NO_INIT_REPORTS | \
>> @@ -669,6 +670,16 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>> if (ret < 0)
>> return ret;
>>
>> + if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
>> + /*
>> + * These keyboards might need 0x5d for shortcuts to work.
>> + * As it has been more than 5 years, it is hard to verify.
>> + */
>> + ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> /* Get keyboard functions */
>> ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>> if (ret < 0)
>> @@ -1409,10 +1420,10 @@ static const struct hid_device_id asus_devices[] = {
>> QUIRK_USE_KBD_BACKLIGHT },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
>> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
>> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>> USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
>> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> You should do FEATURE_KBD_LED_REPORT_ID1 refactoring together, not remove
> + add back in different patches.
Granted I still have no idea why that was removed in the first place?
Then re-added but losing FEATURE_KBD_LED_REPORT_ID1 ?
What's the problem with FEATURE_KBD_LED_REPORT_ID1?
> I suppose the cleanest would be to add a new patch as first which moves
> asus_kbd_init() outside of if/else so you can make this refactoring of
> FEATURE_KBD_LED_REPORT_ID1 in the 2nd patch.
Again I am missing the point in moving these...
> I note there's still contention with this series overall.
>
There are a few things that have pretty much the potential of making
some laptops act funny due to tinkering with initializations commands.
The rename will break some tools, but other than that, granted I have yet
to check the rest of the patchset, looks reasonable to me.
Perhaps I am not entirely happy with how things are worded in
a few instances, but it's a minor issue.
On Wed, 19 Nov 2025, Denis Benato wrote:
> On 11/18/25 13:10, Ilpo Järvinen wrote:
> > On Sat, 1 Nov 2025, Antheas Kapenekakis wrote:
> >
> >> These keyboards have always had initialization in the kernel for 0x5d.
> >> At this point, it is hard to verify again and we risk regressions by
> >> removing this. Therefore, initialize with 0x5d as well.
> See patch 1: unless I missed something you can retain the two
> FEATURE_KBD_LED_REPORT_IDx behind the same exact quirk:
> why are we adding a quirk to replace a quirk that was removed
> in patch 1?
>
> You are basically doing the pretty-much-but-not-quite
> equivalent of what the driver was doing before.
> >> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> >> ---
> >> drivers/hid/hid-asus.c | 15 +++++++++++++--
> >> 1 file changed, 13 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >> index 726f5d8e22d1..221c7195e885 100644
> >> --- a/drivers/hid/hid-asus.c
> >> +++ b/drivers/hid/hid-asus.c
> >> @@ -91,6 +91,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> >> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> >> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> >> #define QUIRK_SKIP_REPORT_FIXUP BIT(14)
> >> +#define QUIRK_ROG_NKEY_LEGACY BIT(15)
> >>
> >> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
> >> QUIRK_NO_INIT_REPORTS | \
> >> @@ -669,6 +670,16 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >> if (ret < 0)
> >> return ret;
> >>
> >> + if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
> >> + /*
> >> + * These keyboards might need 0x5d for shortcuts to work.
> >> + * As it has been more than 5 years, it is hard to verify.
> >> + */
> >> + ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> >> + if (ret < 0)
> >> + return ret;
> >> + }
> >> +
> >> /* Get keyboard functions */
> >> ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >> if (ret < 0)
> >> @@ -1409,10 +1420,10 @@ static const struct hid_device_id asus_devices[] = {
> >> QUIRK_USE_KBD_BACKLIGHT },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> >> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
> >> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> >> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> >> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> >> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> >> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> >> USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
> >> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> > You should do FEATURE_KBD_LED_REPORT_ID1 refactoring together, not remove
> > + add back in different patches.
> Granted I still have no idea why that was removed in the first place?
> Then re-added but losing FEATURE_KBD_LED_REPORT_ID1 ?
Did you mean losing ID2 not ID1 as I don't understand what you meant
otherwise?
And my suggestion was to not "remove [it] in the first place". In a reply
to patch 1, Antheas seemed to be agreeable to not remove it first and also
to not remove ID2 but instead introduce the quirk earlier in the series.
> What's the problem with FEATURE_KBD_LED_REPORT_ID1?
>
> > I suppose the cleanest would be to add a new patch as first which moves
> > asus_kbd_init() outside of if/else so you can make this refactoring of
> > FEATURE_KBD_LED_REPORT_ID1 in the 2nd patch.
>
> Again I am missing the point in moving these...
Antheas wants to consolidate the asus_kbd_register_leds() if/else
branches. That consolidation requires "moving" code one level up
indentation-wise. The series is just misordered currently (in v8 and
before) so that code is first removed and then reintroduced later,
whereas correct approach to order the series would ensure there are
no intermediate step within series that (can) result in lacking something.
My understanding is that this ordering problem is going to be corrected in
v9.
> > I note there's still contention with this series overall.
> >
> There are a few things that have pretty much the potential of making
> some laptops act funny due to tinkering with initializations commands.
>
> The rename will break some tools, but other than that, granted I have yet
> to check the rest of the patchset, looks reasonable to me.
>
> Perhaps I am not entirely happy with how things are worded in
> a few instances, but it's a minor issue.
I'd prefer we address nits as well.
--
i.
© 2016 - 2026 Red Hat, Inc.