[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <36479176-9583-478b-b16a-03ba725b3f81@linux.dev>
Date: Tue, 20 Jan 2026 22:03:01 +0100
From: Denis Benato <denis.benato@...ux.dev>
To: Antheas Kapenekakis <lkml@...heas.dev>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: 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 1/20/26 18:41, Antheas Kapenekakis wrote:
> 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.
Generally speaking yeah a positive answer (ASUSTEK ...) means
the feature is supported. A negative answer (generally 0xFF 0xFF 0x00 ....)
means the feature is not supported.
Those messages are also an init for models that supports the feature,
and generally asus has hooked functions not directly connected
to the main function of the endpoint to those handshakes
(the only example on my mind is 5a [] making some fn keys behaving
different than booting/pre-os, basically for UEFI settings I guess).
The disassembled AsusKeyboardFilter.sys seems to call those regardless,
but a list of devices is nowhere to be found, probably because the .inf
already lists the vid and pid for the driver to load.
>>> 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
Perhaps, I can't find it on my notes. In general keyboard configuration
has been scattered on a lot of "devices": certain laptops have an additional
i2c device, that is separate from the keyboard device, that does things
like locking certain keys... don't be surprised if one command
does more than what you are used to.
On the ally 5a also changes gamepad configuration, not just leds
as usual.
Before the bootloader if you press the vol+ a boot menu will appear
and the device will enter keyboard mode (ASUS implemented this to
allow easier UEFI usage), but as soon as you init the device [5a handshake]
it re-enters its default state (xbox360), exiting the keyboard state.
>>>> 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)
Given that those devices are the main way to control the hardware
(for things that are out of scope of wmi) you could also call those
_SYSTEM or something that denotate the importance of those devices
in controlling what the firmware does.
I am fine even with what Ilpo suggested, I simply don't want
to associate those vid:pid with something that is necessarily
"old" as they have been in use for a long time and at least on ROG
virtually all of them have those vid:pid...
The overwhelming majority: so much so that the easiest way to
tell models apart is to check for DMI since software cannot easily
distinguish them from USB alone.
> 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