Message-ID: 
 <CAGwozwGdDe+M36RE_CMjXePBX-CVRORjHZbOroS+e_Dc_Am2vg@mail.gmail.com>
Date: Tue, 20 Jan 2026 18:41:03 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
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 Tue, 20 Jan 2026 at 14:56, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com> wrote:
>
> 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.

It's NKEY only so QUIRK_ROG_NKEY_ID1ID2_INIT would be more appropriate
(all recent models are NKEY)

Sounds like a plan, I will resend tomorrow. If anyone wants to leave
any more comments now is the time.

Antheas

> --
>  i.
>

Powered by blists - more mailing lists