Message-ID: <4671d267-d823-4bf7-af30-b587e67dec49@gmail.com>
Date: Thu, 13 Nov 2025 02:14:05 +0100
From: Denis Benato <benato.denis96@...il.com>
To: Antheas Kapenekakis <lkml@...heas.dev>
Cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 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 v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard
 backlight handling


On 11/12/25 23:08, Antheas Kapenekakis wrote:
> On Wed, 12 Nov 2025 at 20:51, Denis Benato <benato.denis96@...il.com> wrote:
>>
>> On 11/12/25 14:41, Antheas Kapenekakis wrote:
>>> On Wed, 12 Nov 2025 at 14:22, Ilpo Järvinen
>>> <ilpo.jarvinen@...ux.intel.com> wrote:
>>>> On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
>>>>
>>>>> On Sat, 1 Nov 2025 at 11:47, Antheas Kapenekakis <lkml@...heas.dev> wrote:
>>>>>> This is a two part series which does the following:
>>>>>>   - Clean-up init sequence
>>>>>>   - Unify backlight handling to happen under asus-wmi so that all Aura
>>>>>>     devices have synced brightness controls and the backlight button works
>>>>>>     properly when it is on a USB laptop keyboard instead of one w/ WMI.
>>>>>>
>>>>>> For more context, see cover letter of V1. Since V5, I removed some patches
>>>>>> to make this easier to merge.
>>>>> Small bump for this.
>>>> I looked at v8 earlier but then got an impression some of Denis' comments
>>>> against v7 were not taken into account in v8, which implies there will be
>>>> delay until I've time to delve into the details (I need to understand
>>>> things pretty deeply in such a case, which does take lots of time).
>>>>
>>>> Alternatively, if Denis says v8 is acceptable, then I don't need to spend
>>>> so much time on it, but somehow I've a feeling he isn't happy with v8
>>>> but just hasn't voiced it again...
>>>>
>>>> Please do realize that ignoring reviewer feedback without a very very good
>>>> reason always risks adding delay or friction into getting things
>>>> upstreamed. Especially, when the review comes from a person who has been
>>>> around for me to learn to trust their reviews or from a maintainer of the
>>>> code in question.
>>> Sure, sorry if it came out this way. Dennis had two comments on the V7
>>> version of the series.
>>>
>>> The first one was that asusctl has a hang bug, which he hasn't had
>>> time to look into yet. This should have been fixed by dropping the
>>> HID_QUIRK_INPUT_PER_APP. I retested the series and that QUIRK was a
>>> bit of a NOOP that does not need to be added in the future.
>> So it is supposed to not regress it now, correct?
>>> The second is he is concerned with dropping the 0x5d/0x5e inits. Luke
>>> said (back in March) that it is fine to drop 0x5e because it is only
>>> used for ANIME displays. However, for 0x5d, it is hard to verify some
>>> of the older laptops because they have only been tested with 0x5d and
>>> we do not have the hardware in question to test.
>>>
>>> For this series, I re-added "initialize LED endpoint early for old
>>> NKEY keyboards" that re-adds 0x5d for the keyboards that cannot be
>>> tested again so this comment should be resolved too. With that in
>>> mind, we do end up with an additional quirk/command that may be
>>> unneeded and will remain there forever, but since it was a point of
>>> contention, it is not worth arguing over.
>>>
>>> So both comments should be resolved
>> The driver should also not late-initialize anything.
>>
>> Windows doesn't do it and the official asus application
>> can freely send LEDs changing commands to either WMI or USB
>> so I don't see any reason to do things differently [than windows]
>> and not prepare every USB endpoint to receive commands,
>> this has not been addressed unless I confused v7 and v8?
> Yes, it's been added on v8. 0x5d is init for the laptops it is
> problematic for. Not because it does not work, but because it has not
> been verified to work for those laptops.
I am not sure I am reading this right:
are you telling me that on recent models the windows driver
doesn't issue 0x5d?
>>> @Denis: can give an ack if this is the case?
>>>
>>> As for Derek's comment, he has a PR for his project where he removes
>>> the name match for Ally devices with ample time for it to be merged
>>> until kernel 6.19 is released. In addition, that specific software for
>>> full functionality relies on OOT drivers on the distros it ships with,
>>> so it is minimally affected in either case.
>> The part we are talking about depends on this driver (hid-asus)
>> and there are people on asus-linux community using inputplumber
>> for non-ally devices (the OOT driver is only for ally devices)
>> therefore it is very important to us (and various other distributions)
>> not to break that software in any way.
> This driver is only used for Ally devices. If you mean that people
> remap their keyboards using inputplumber I guess they could but I have
> not seen it.
I meant people remap keyboards using IP. I am sure there were
(and very probably still are) people doing that.
>> Weighting pros and cons of changing the name I am not sure
>> changing name brings any benefit? Or am I missing something here?
>> It's simply used by userspace so the hardware should be loading
>> regardless of the name...
> Users see the name of their devices in their settings menu. They
> should be accurate. Also, the early entry needs to be added anyway to
> prevent kicking devices.
If it's just aesthetics I don't see much reasons in changing the name.

"the early entry needs to be added anyway ...." has no meaning to me,
please rephrase. Sorry.

>> Along with IP and your tool and asusctl there is also openrgb,
>> and a newborn application for asus devices (I don't have contacts
>> with that dev nor I remember the name of the tool)
>> and I am not even that sure these are all asus-related
>> applications.
> My tool never checked for names, it briefly did for around a month
> after its creation for some devices until capability matches. Around
> 6.1 and 6.7 the kernel changed the names of most USB devices and that
> caused issues. It currently only uses name matches for VID/PID 0/0
> devices created by the kernel. Specifically, WMI and AT Keyboards. I
> am not sure there is a workaround for those. Asusctl also does not use
> names either.
But IP does, so I would like to hear confirmation from at least Derek
before the merge that there won't be future issues.

Interpret what I say here as a broad topic, not just name/PER_APP flag:
avoid changing data flow on older models...

>> Excercise EXTRA care touching this area as these are enough changes
>> to make it difficult to understand what exactly is the problem if
>> someone shows up with LEDs malfunctioning, laptop not entering sleep
>> anymore or something else entirely. Plus over time
>> ASUS has used various workarounds for windows problems
>> and I am not eager to find out what those are since there is only
>> a realistic way it's going to happen....
> These changes are not doing something extraordinary. It's just a minor cleanup.
>
>>> Moreover, that specific commit is needed for Xbox Ally devices anyway,
>>> as the kernel kicks one of the RGB endpoints because it does not
>>> register an input device (the check skipped by early return) so
>>> userspace becomes unable to control RGB on a stock kernel
>>> (hidraw/hiddev nodes are gone). For more context here, this specific
>>> endpoint implements the RGB Lamparray protocol for Windows dynamic
>>> lighting, and I think in an attempt to make it work properly in
>>> Windows, Asus made it so Windows has to first disable dynamic lighting
>>> for armoury crate RGB commands to work (the 0x5a ones over the 0xff31
>>> hid page).
>> Yes once ASUS introduces something new it sticks with that for
>> future models so it's expected more and more laptops will have
>> the same problem: I am not questioning if these patches are needed
>> as they very clearly are; I am questioning if everything that these
>> patches are doing are worth doing and aren't breaking/regressing
>> either tools or the flow of actions between the EC and these USB devices.
> Well, this series is needed to account for that. Sending the disable
> command is out of scope for now though.
Here I apologize for confusion: my comments were mostly about
older models: I absolutely don't want to break those, but if you find a way
to distinguish them from newer models that would give you more freedom with those.

No disable commands unless we find hard evidence those are strictly needed.

> Antheas
>
>>> Hopefully this clears things up
>>>
>>> Antheas
>>>
>>>>> Unrelated but I was b4ing this series on Ubuntu 24 and got BADSIG:
>>>>> DKIM/antheas.dev. Is there a reference for fixing this on my host?
>>>>> Perhaps it would help with spam
>>>> I see BADSIG very often these days from b4 (thanks to gmail expiring
>>>> things after 7 days or so, I recall hearing somewhere), I just ignore them
>>>> entirely.
>>>>
>>>> AFAIK, that has never caused any delay to any patch in pdx86 domain so if
>>>> that is what you thought is happening here, it's not the case.
>>>> If your patch does appear in the pdx86 patchwork, there's even less reason
>>>> to worry as I mostly pick patches to process using patchwork's list.
>>> Turns out I had to update my DNS records. It should be good now.
>>>
>>>> --
>>>>  i.
>> snipp
>>>>>> 2.51.2
>>>>>>
>>>>>>

Powered by blists - more mailing lists