Message-ID: <89a67ace-6fef-72e0-4ef3-be828dd85e3d@linux.intel.com>
Date: Tue, 20 Jan 2026 15:56:37 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Antheas Kapenekakis <lkml@...heas.dev>
cc: Denis Benato <denis.benato@...ux.dev>, platform-driver-x86@...r.kernel.org, 
    linux-input@...r.kernel.org, LKML <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>
Subject: Re: [PATCH v11 02/11] HID: asus: initialize additional endpoints
 only for legacy devices

On Sat, 17 Jan 2026, Antheas Kapenekakis wrote:
> 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

Can you name it e.g. something that should be neutral such as:

QUIRK_ROG_NKEY_ID1_ID2_INIT

I'm not sure if ROG NKEY should still be included into the name based on 
what Denis mentioned about recent models but at least it gets rid of the 
"legacy" connotation. If wider scope is necessary you could use just
QUIRK_ID1_ID2_INIT.

--
 i.

Powered by blists - more mailing lists