From: Denis Benato <denis.benato@linux.dev>
To: Antheas Kapenekakis <lkml@antheas.dev>
Cc: platform-driver-x86@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, "Jiri Kosina" <jikos@kernel.org>,
"Benjamin Tissoires" <bentiss@kernel.org>,
"Corentin Chary" <corentin.chary@gmail.com>,
"Luke D . Jones" <luke@ljones.dev>,
"Hans de Goede" <hansg@kernel.org>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH v11 02/11] HID: asus: initialize additional endpoints only for legacy devices
Date: Sat, 17 Jan 2026 14:51:48 +0100 [thread overview]
Message-ID: <fe66c416-161c-489e-a38f-9dc7729c6ed7@linux.dev> (raw)
In-Reply-To: <CAGwozwGyUpBq4GGvyDHj089a9-vxNOnqgSBys3-CC_+tKDywaA@mail.gmail.com>
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?
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 },
next prev parent reply other threads:[~2026-01-17 13:51 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-16 13:31 [PATCH v11 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 01/11] HID: asus: simplify RGB init sequence Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 02/11] HID: asus: initialize additional endpoints only for legacy devices Antheas Kapenekakis
2026-01-16 20:44 ` Denis Benato
2026-01-16 23:10 ` Antheas Kapenekakis
2026-01-17 13:51 ` Denis Benato [this message]
2026-01-17 15:07 ` Antheas Kapenekakis
2026-01-17 15:13 ` Denis Benato
2026-01-17 16:10 ` Antheas Kapenekakis
2026-01-17 16:12 ` Denis Benato
2026-01-17 16:16 ` Antheas Kapenekakis
2026-01-17 17:05 ` Denis Benato
2026-01-17 19:17 ` Antheas Kapenekakis
2026-01-20 13:56 ` Ilpo Järvinen
2026-01-20 17:41 ` Antheas Kapenekakis
2026-01-20 21:03 ` Denis Benato
2026-01-20 21:15 ` Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 03/11] HID: asus: use same report_id in response Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 04/11] HID: asus: fortify keyboard handshake Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 05/11] HID: asus: move vendor initialization to probe Antheas Kapenekakis
2026-01-16 21:08 ` Denis Benato
2026-01-16 13:31 ` [PATCH v11 06/11] HID: asus: early return for ROG devices Antheas Kapenekakis
2026-01-16 21:13 ` Denis Benato
2026-01-16 13:31 ` [PATCH v11 07/11] platform/x86: asus-wmi: Add support for multiple kbd led handlers Antheas Kapenekakis
2026-01-17 13:16 ` Denis Benato
2026-01-17 13:49 ` Antheas Kapenekakis
2026-01-17 13:52 ` Antheas Kapenekakis
2026-01-17 13:56 ` Denis Benato
2026-01-17 14:11 ` Antheas Kapenekakis
2026-01-17 14:48 ` Denis Benato
2026-01-19 9:53 ` Ilpo Järvinen
2026-01-19 11:34 ` Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 09/11] platform/x86: asus-wmi: remove unused keyboard backlight quirk Antheas Kapenekakis
2026-01-16 13:31 ` [PATCH v11 10/11] platform/x86: asus-wmi: add keyboard brightness event handler Antheas Kapenekakis
2026-01-17 13:45 ` Denis Benato
2026-01-16 13:31 ` [PATCH v11 11/11] HID: asus: add support for the asus-wmi brightness handler Antheas Kapenekakis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fe66c416-161c-489e-a38f-9dc7729c6ed7@linux.dev \
--to=denis.benato@linux.dev \
--cc=bentiss@kernel.org \
--cc=corentin.chary@gmail.com \
--cc=hansg@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkml@antheas.dev \
--cc=luke@ljones.dev \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox