Message-ID: 
 <CAGwozwF--y3vsqhdqYFV_x1ff7jfSkYaQHeV1OV70yue7Huk=A@mail.gmail.com>
Date: Sat, 17 Jan 2026 20:17:22 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Denis Benato <denis.benato@...ux.dev>
Cc: platform-driver-x86@...r.kernel.org, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, Jiri Kosina <jikos@...nel.org>,
	Benjamin Tissoires <bentiss@...nel.org>,
 Corentin Chary <corentin.chary@...il.com>,
	"Luke D . Jones" <luke@...nes.dev>, Hans de Goede <hansg@...nel.org>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v11 02/11] HID: asus: initialize additional endpoints only
 for legacy devices

On Sat, 17 Jan 2026 at 18:05, Denis Benato <denis.benato@...ux.dev> wrote:
>
>
> On 1/17/26 17:16, Antheas Kapenekakis wrote:
> > On Sat, 17 Jan 2026 at 17:12, Denis Benato <denis.benato@...ux.dev> wrote:
> >>
> >> On 1/17/26 17:10, Antheas Kapenekakis wrote:
> >>> On Sat, 17 Jan 2026 at 16:13, Denis Benato <denis.benato@...ux.dev> wrote:
> >>>> On 1/17/26 16:07, Antheas Kapenekakis wrote:
> >>>>> On Sat, 17 Jan 2026 at 14:51, Denis Benato <denis.benato@...ux.dev> wrote:
> >>>>>> On 1/17/26 00:10, Antheas Kapenekakis wrote:
> >>>>>>> On Fri, 16 Jan 2026 at 21:44, Denis Benato <denis.benato@...ux.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@...nel.org>
> >>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@...heas.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.
> > I was not aware of that. As far as I know they are not needed. Do you
> > have a bug report with a specific laptop model I can look at?
> There is current effort to integrate commands that requires those
> initializations on 2025 laptop, why would I strip out a command
> that I already know is required anyway?

Hi,
yes ID1 is required for RGB, I have a draft patch for it that would
lazily do it if RGB is supported.

I recall now a previous discussion about it being required for some
laptop shortcuts but we never found a laptop that needs it so I forgot

> No, this is not the way to go to knowingly and willingly cause
> troubles (both known and unknown) to others just because
> you think it's better this way.
>
> Change the name of _LEGACY to something else, have this accepted
> and then if I see it's appropriate  to remove the if and send those
> regardless I will.

Sure, up to you if you want to change the name. What would you like it
be? I would like this series to merge

Antheas

> > Antheas
> >
> >>> 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@...ux.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@...nel.org>
> >>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@...heas.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 },
>

Powered by blists - more mailing lists