Re: [PATCH v11 02/11] HID: asus: initialize additional endpoints only for legacy devices

[Denis Benato <denis.benato@linux.dev>]

On 1/17/26 17:10, Antheas Kapenekakis wrote:
> On Sat, 17 Jan 2026 at 16:13, Denis Benato <denis.benato@linux.dev> wrote:
>>
>> On 1/17/26 16:07, Antheas Kapenekakis wrote:
>>> On Sat, 17 Jan 2026 at 14:51, Denis Benato <denis.benato@linux.dev> wrote:
>>>> On 1/17/26 00:10, Antheas Kapenekakis wrote:
>>>>> On Fri, 16 Jan 2026 at 21:44, Denis Benato <denis.benato@linux.dev> wrote:
>>>>>> On 1/16/26 14:31, Antheas Kapenekakis wrote:
>>>>>>
>>>>>>> Currently, ID1/ID2 initializations are performed for all NKEY devices.
>>>>>>> However, ID1 initializations are only required for RGB control and are
>>>>>>> only supported for RGB capable devices. ID2 initializations are only
>>>>>>> required for initializing the Anime display endpoint which is only
>>>>>>> supported on devices with an Anime display. Both of these
>>>>>>> initializations are out of scope for this driver (this is a brightness
>>>>>>> control and keyboard shortcut driver) and they should not be performed
>>>>>>> for devices that do not support them in any case.
>>>>>>>
>>>>>>> At the same time, there are older NKEY devices that have only been
>>>>>>> tested with these initializations in the kernel and it is not possible
>>>>>>> to recheck them. There is a possibility that especially with the ID1
>>>>>>> initialization, certain laptop models might have their shortcuts stop
>>>>>>> working (currently unproven).
>>>>>>>
>>>>>>> For an abundance of caution, only initialize ID1/ID2 for those older
>>>>>>> NKEY devices by introducing a quirk for them and replacing the NKEY
>>>>>>> quirk in the block that performs the inits with that.
>>>>>>>
>>>>>>> In addition, as these initializations might not be supported by the
>>>>>>> affected devices, change the function to not bail if they fail.
>>>>>>>
>>>>>>> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>>>>> ---
>>>>>>>  drivers/hid/hid-asus.c | 16 ++++++----------
>>>>>>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>>>>>> index 323e6302bac5..dc7af12cf31a 100644
>>>>>>> --- a/drivers/hid/hid-asus.c
>>>>>>> +++ b/drivers/hid/hid-asus.c
>>>>>>> @@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>>>>>>>  #define QUIRK_ROG_NKEY_KEYBOARD              BIT(11)
>>>>>>>  #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
>>>>>>>  #define QUIRK_ROG_ALLY_XPAD          BIT(13)
>>>>>>> +#define QUIRK_ROG_NKEY_LEGACY                BIT(14)
>>>>>> These past days I have taken a look at new 2025 models and they do make use of ID2,
>>>>>> and won't do harm sending ID1 either. I think you can safely remove the if and send regardless.
>>>>> Hi Denis,
>>>>> it is not the responsibility of this driver. ID2 is used by Anime
>>>>> models. It is a concession to make sure that we do not cause a
>>>>> regression that will cause warnings for a lot of users.
>>>> Who decided it is a concession?
>>> I would rather remove the extra calls unless they are shown to be
>>> needed, which they might be for these PIDs.
>> They are needed on older laptop and to not regress userspace.
>>
>> You just named _LEGACY an usb pid that is not legacy.
>>> The quirk is named legacy because we can't retest these devices. If we
>>> can, then we could remove the quirk and the inits if not needed.
>> We can't retest every device, and that pid is used in pre-2021 models,
>> and these are the unknown, I am criticizing the name of the quirk here,
>> not what it does.
> If you can test whether your device needs them that would be great.
That is pointless.
>> I am also questioning if the quirk is even needed since sending
>> those commands to (at least) recent hardware that doesn't use
>> those endpoints carries no downsides, while removing them
>> surely does.
> We have not found a device yet that needs them. I do not want to keep
> sending unneeded commands. It could cause obscure bugs or interfere
> with userspace software such as the one you maintain. So at least for
> new hardware that is possible to test we should remove them.
There is new hardware that needs them, as I said, including 2025 models. 
> Antheas
>
>>> Antheas
>>>
>>>> Anyway I will move relevant code tied to these two to this driver,
>>>> so it doesn't make sense to remove them anyway.
>>>>>> At least 2023 models like mine that don't support ID2 will simply reply with 0xFF 0xFF and the rest 0x00.
>>>>>> No consequences.
>>>>> In your laptop. In the other user's laptop, the get feature report fails
>>>> for the response to be a failure (as it is supposed to be in mine and other models)
>>>> and to cause problems are two different things. Here I am saying that the hardware
>>>> correctly reports "unsupported" and nothing bad happens (if you ignore the return value).
>>>>>> Regardless the name is wrong: mine is a 2023 rog strix with
>>>>>> ID 0b05:19b6ASUSTek Computer, Inc. N-KEY Device
>>>>>> and surely isn't legacy.
>>>>> Sure, can you try removing the if block?
>>>> I have asked to distribute a kernel that init ID1 and ID2 regardless
>>>> of that quirk. We will soon know if it causes problems or not.
>>>>> If it works in your laptop, that is one less reason to keep it for 19b6
>>>> If it works in my laptop one more reason not to exclude code that
>>>> works and haven't caused any problem ever.
>>>>> Antheas
>>>>>
>>>>>>>  #define I2C_KEYBOARD_QUIRKS                  (QUIRK_FIX_NOTEBOOK_REPORT | \
>>>>>>>                                                QUIRK_NO_INIT_REPORTS | \
>>>>>>> @@ -652,14 +653,9 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>>>>>       if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>>>>               return -ENODEV;
>>>>>>>
>>>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>>>> -             if (ret < 0)
>>>>>>> -                     return ret;
>>>>>>> -
>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>>>> -             if (ret < 0)
>>>>>>> -                     return ret;
>>>>>>> +     if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
>>>>>>> +             asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>>>> +             asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>>>>       }
>>>>>>>
>>>>>>>       if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>>>> @@ -1376,10 +1372,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 },
>>>>> On Fri, 16 Jan 2026 at 21:44, Denis Benato <denis.benato@linux.dev> wrote:
>>>>>> On 1/16/26 14:31, Antheas Kapenekakis wrote:
>>>>>>
>>>>>>> Currently, ID1/ID2 initializations are performed for all NKEY devices.
>>>>>>> However, ID1 initializations are only required for RGB control and are
>>>>>>> only supported for RGB capable devices. ID2 initializations are only
>>>>>>> required for initializing the Anime display endpoint which is only
>>>>>>> supported on devices with an Anime display. Both of these
>>>>>>> initializations are out of scope for this driver (this is a brightness
>>>>>>> control and keyboard shortcut driver) and they should not be performed
>>>>>>> for devices that do not support them in any case.
>>>>>>>
>>>>>>> At the same time, there are older NKEY devices that have only been
>>>>>>> tested with these initializations in the kernel and it is not possible
>>>>>>> to recheck them. There is a possibility that especially with the ID1
>>>>>>> initialization, certain laptop models might have their shortcuts stop
>>>>>>> working (currently unproven).
>>>>>>>
>>>>>>> For an abundance of caution, only initialize ID1/ID2 for those older
>>>>>>> NKEY devices by introducing a quirk for them and replacing the NKEY
>>>>>>> quirk in the block that performs the inits with that.
>>>>>>>
>>>>>>> In addition, as these initializations might not be supported by the
>>>>>>> affected devices, change the function to not bail if they fail.
>>>>>>>
>>>>>>> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>>>>> ---
>>>>>>>  drivers/hid/hid-asus.c | 16 ++++++----------
>>>>>>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>>>>>> index 323e6302bac5..dc7af12cf31a 100644
>>>>>>> --- a/drivers/hid/hid-asus.c
>>>>>>> +++ b/drivers/hid/hid-asus.c
>>>>>>> @@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>>>>>>>  #define QUIRK_ROG_NKEY_KEYBOARD              BIT(11)
>>>>>>>  #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
>>>>>>>  #define QUIRK_ROG_ALLY_XPAD          BIT(13)
>>>>>>> +#define QUIRK_ROG_NKEY_LEGACY                BIT(14)
>>>>>> These past days I have taken a look at new 2025 models and they do make use of ID2,
>>>>>> and won't do harm sending ID1 either. I think you can safely remove the if and send regardless.
>>>>>>
>>>>>> At least 2023 models like mine that don't support ID2 will simply reply with 0xFF 0xFF and the rest 0x00.
>>>>>> No consequences.
>>>>>>
>>>>>> Regardless the name is wrong: mine is a 2023 rog strix with
>>>>>> ID 0b05:19b6ASUSTek Computer, Inc. N-KEY Device
>>>>>> and surely isn't legacy.
>>>>>>>  #define I2C_KEYBOARD_QUIRKS                  (QUIRK_FIX_NOTEBOOK_REPORT | \
>>>>>>>                                                QUIRK_NO_INIT_REPORTS | \
>>>>>>> @@ -652,14 +653,9 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>>>>>       if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>>>>               return -ENODEV;
>>>>>>>
>>>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>>>> -             if (ret < 0)
>>>>>>> -                     return ret;
>>>>>>> -
>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>>>> -             if (ret < 0)
>>>>>>> -                     return ret;
>>>>>>> +     if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
>>>>>>> +             asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>>>> +             asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>>>>       }
>>>>>>>
>>>>>>>       if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>>>> @@ -1376,10 +1372,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 },