[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<CAGwozwFfQdivRkb3uSQOOZa6chRAiQjuHTSBGLxyr1JqJ=zVFA@mail.gmail.com>
Date: Mon, 3 Nov 2025 10:15:46 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: luke@...nes.dev
Cc: "Derek J. Clark" <derekjohn.clark@...il.com>,
Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>,
Corentin Chary <corentin.chary@...il.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Denis Benato <benato.denis96@...il.com>, kernel test robot <lkp@...el.com>,
platform-driver-x86@...r.kernel.org, linux-input@...r.kernel.org,
oe-kbuild-all@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 5/9] platform/x86: asus-wmi: Add support for multiple
kbd led handlers
On Mon, 3 Nov 2025 at 10:06, <luke@...nes.dev> wrote:
>
>
>
> > On 3 Nov 2025, at 21:48, Antheas Kapenekakis <lkml@...heas.dev> wrote:
> >
> > On Mon, 3 Nov 2025 at 09:38, <luke@...nes.dev> wrote:
> >>
> >>
> >>> On 3 Nov 2025, at 20:36, Antheas Kapenekakis <lkml@...heas.dev> wrote:
> >>>
> >>> On Mon, 3 Nov 2025 at 05:29, Derek J. Clark <derekjohn.clark@...il.com> wrote:
> >>>>
> >>>>> On Fri, 31 Oct 2025 at 09:27, Jiri Kosina <jikos@...nel.org> wrote:
> >>>>>>
> >>>>>> On Thu, 23 Oct 2025, Antheas Kapenekakis wrote:
> >>>>>>
> >>>>>>>> 1589
> >>>>>>>> 1590 static void kbd_led_update_all(struct work_struct *work)
> >>>>>>>> 1591 {
> >>>>>>>> 1592 enum led_brightness value;
> >>>>>>>> 1593 struct asus_wmi *asus;
> >>>>>>>> 1594 bool registered, notify;
> >>>>>>>> 1595 int ret;
> >>>>>>> /\ value should have been an int and
> >>>>>>> placed here. It can take the value -1 hence the check
> >>>>>>
> >>>>>> Thanks, that needs to be fixed before the final merge.
> >>>>>>
> >>>>>>> Are there any other comments on the series?
> >>>>>>>
> >>>>>>> The only issue I am aware of is that Denis identified a bug in asusd
> >>>>>>> (asusctl userspace program daemon) in certain Asus G14/G16 laptops
> >>>>>>> that cause laptop keys to become sticky, I have had users also report
> >>>>>>> that bug in previous versions of the series. WIthout asusd running,
> >>>>>>> keyboards work fine incl. with brightness control (did not work
> >>>>>>> before). Given it will take two months for this to reach mainline, I
> >>>>>>> think it is a fair amount of time to address the bug.
> >>>>>>
> >>>>>> One thing that is not clear to me about this -- is this causing a visible
> >>>>>> user-space behavior regression before vs. after the patchset with asusctl?
> >>>>>>
> >>>>>> If so, I am afraid this needs to be root-caused and fixed before the set
> >>>>>> can be considered for inclusion.
> >>>>
> >>>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on
> >>>>> ROG") adds HID_QUIRK_INPUT_PER_APP and the extra devices seem to
> >>>>> confuse asusd. Since the devices are the same as with hid-asus not
> >>>>> loaded, it is specific to that program.
> >>>>>
> >>>>>
> >>>> Hi Antheas.
> >>>>
> >>>> While you have previously expressed to me directly that you wish InputPlumber
> >>>> didn't exist, it still very much does, in fact, exist. I also know that you are
> >>>> explicitly aware that InputPlumber is a consumer of this interface, so your
> >>>> comment that asusctl is the only affected program is something you know to be
> >>>> false. This is not even the first time you have renamed an input device that
> >>>> you knew InputPlumber was a consumer of without notifying me[1].
> >>>>
> >>>> I can't abide you outright lying to the maintainers here and I'm sick and tired
> >>>> of having to watch your every move on the LKML. Either become a good citizen of
> >>>> kernel maintenance, or get out of it.
> >>>
> >>> Hi Derek,
> >>> I am not aware if your software is affected or not by this series as I
> >>> have not received reports about it.
> >>>
> >>> If it is, please add:
> >>> "ASUSTeK Computer Inc. N-KEY Device"
> >>>
> >>> As a name match to your software (should only take 5 minutes).
> >>>
> >>> I worked with Luke and Dennis on it for the better part of a year so
> >>> hopefully they forwarded to you if it affects you or not.
> >>>
> >>> Your software relies on OOT drivers for most devices incl. the Ally so
> >>> I am unsure if it is affected in reality. E.g., it would not be
> >>> affected in SteamOS and CachyOS. In the future, it would be good to
> >>> avoid name matches for your software as it makes it very fragile,
> >>> which is a discussion we have had before. I do not think device evdev
> >>> names constitute an ABI technically.
> >>
> >> Taking no sides here.
> >>
> >> An unfortunate reality is that there is stuff out there that does use name matches (and yes I agree they shouldn’t because it is *not* an ABI and many many devices have had name changes over the decades).
> >>
> >> While name strings aren't a formal ABI, when a change affects known downstream users, a heads-up helps the ecosystem adapt smoothly—even if the technical stance is that they shouldn't rely on names.
> >>
> >> In general it also needs to be justified such as:
> >> - "Matches updated product branding"
> >> - "Current string is misleading (says 'mouse' but it's a keyboard)"
> >> - "Fixing spelling error"
> >> - "Aligning with USB-IF device class names"
> >>
> >> I always advocated use of evdev libraries to discover devices rather than the shortcut of name matching as it is much more powerful and reliable (which is how asusctl does dynamic add/remove of LED dev dbus interfaces). It’s much better to use evdev with vid/pid, device sub/classes, into descriptors and so on - you can be as open or restrictive as required by use case. This particular issue illustrates why this approach is preferable.
> >>
> >> If the name change is a result of something I said or missed then I apologise to both Derek and yourself. Likely I missed it as I’ve never relied on name strings for userspace.
> >>
> >> Regarding the OOT ally drivers I started, these will of course get upstreamed in the future (by Denis in this case when he can). They are getting very heavily battled tested in the mean time. Please do contribute to them if there is anything required to be addressed or chat to Denis, after all they are made only to benefit all users (there is no *race to be first* here.
> >>
> >> As I no longer work on Asus laptops/handhelds and don't have hardware left to test with, I can't contribute further to this discussion. Best of luck resolving this.
> >>
> >> I'm out.
> >> Luke.
> >
> > Hi Luke,
> > good luck to your future endeavors.
> >
> > The OOT reference was not to disparage the drivers, just to note that
> > in kernels that use those, hid-asus is stubbed for the Ally so there
> > is no change there.
> >
> > As for reasons, see below.
> >
> >> - "Matches updated product branding"
> >
> > Reason for [1]
> >
> >> - "Aligning with USB-IF device class names"
> >
> > Reason for the change in this patch
>
> Ack. Thank you, appreciate the clarification, this is the appropriate reason. Might be worth mentioning in change cover letter or commit message (if not already, sorry if I missed).
>
> >
> > If you would like me to stop cc'ing you in future asus changes let me
> > know. I am preparing some additional patches for the Duo class of
> > laptops.
>
> Leave me in for the moment. I’ll submit a patch to remove myself from maintainers once Denis is comfortable in the role.
>
> Regarding Duo, I assume you mean the larger type with two full-size screens and not the older models with a thin screen above the keyboard?
> There was an issue with the older ones which I could not solve until I had the physical hardware to test due to it being difficult for testers to describe the exact behaviour. The way the power-off and brightness control works is funky. I submitted the patch here https://lore.kernel.org/all/20250525204214.104030-1-luke@ljones.dev/
>
> Hopefully that’s of some help to you. It’s very unlikely I will resubmit that as a split series.
>
> If you need anything from me related to above or otherwise, do please reach out off-chain to avoid us further hitting the CC.
Yes, the two screen type. I am focusing on the keyboard for now. I am
waiting for a user to test the changes. This series helps with
maintaining keyboard brightness on that. Hopefully it will work as the
keyboard transitions from USB to bluetooth.
If there are issues with the screen brightness, I will rereview your
patch and if it is needed adapt it and coby you.
Best,
Antheas
> >
> > Best,
> > Antheas
> >
> >>>
> >>> Best,
> >>> Antheas
> >>>
> >>>> Commit 591ba2074337 ("HID: asus: prevent binding to all HID devices on ROG")
> >>>> Nacked-By: Derek J. Clark <derekjohn.clark@...il.com>
> >>>>
> >>>> - Derek
> >>>>
> >>>> [1] https://lore.kernel.org/linux-input/Z74vZD7ZtKBTDlwy@google.com/
> >>>>
> >>>>> We can delay that patch until Denis who took over maintenance of the
> >>>>> program can have a deeper look. I will still keep the last part of
> >>>>> that patch that skips the input check, because that causes errors in
> >>>>> devices that do not create an input device (e.g., lightbar).
> >>>>>
> >>>>> Antheas
>
>
>
Powered by blists - more mailing lists