Message-ID: 
 <CAGwozwEKqqJxxmtjJhy2MzNVhmBTMmy8xG5TZGkKJqJCgK=X5w@mail.gmail.com>
Date: Wed, 12 Nov 2025 23:08:00 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Denis Benato <benato.denis96@...il.com>
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 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.

> > @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.

> 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.

> 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.

> 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.

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