* Re: [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: luke @ 2025-11-13 22:09 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Denis Benato, Ilpo Järvinen, platform-driver-x86,
linux-input, LKML, Jiri Kosina, Benjamin Tissoires,
Corentin Chary, Hans de Goede
In-Reply-To: <CAGwozwGwtivKnC6ucCGwJiu2yLLbddFXG+jYTaMTwU4Zt=CAfQ@mail.gmail.com>
> On 14 Nov 2025, at 10:17, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Thu, 13 Nov 2025 at 21:23, <luke@ljones.dev> wrote:
>>
>>
>>> On 13 Nov 2025, at 21:44, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>
>>> On Thu, 13 Nov 2025 at 02:14, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>
>>>>
>>>> On 11/12/25 23:08, Antheas Kapenekakis wrote:
>>>>> On Wed, 12 Nov 2025 at 20:51, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>>
>>>>>> On 11/12/25 14:41, Antheas Kapenekakis wrote:
>>>>>>> On Wed, 12 Nov 2025 at 14:22, Ilpo Järvinen
>>>>>>> <ilpo.jarvinen@linux.intel.com> wrote:
>>>>>>>> On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
>>>>>>>>
>>>>>>>>> On Sat, 1 Nov 2025 at 11:47, Antheas Kapenekakis <lkml@antheas.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?
>>>
>>> Try to add spaces in your replies. This is hard to follow.
>>>
>>> Do not conflate driver with software. 0x5a (over the application
>>> 0xff310076) has traditionally been used by a driver in Windows to
>>> control the backlight level, as it is done in this driver. 0x5d (over
>>> the application 0xff310079) is only used by laptops with RGB by
>>> Armoury crate. But this driver does not do RGB. No device
>>> functionality relies on it being sent for any device I've seen. The
>>> device remembers its Windows settings, incl. the backlight color, in
>>> the absence of a driver.
>>>
>>> Laptops without RGB such as the Duo series which I would like to add
>>> support for next only have a 0x5a endpoint. But, they are sent garbage
>>> inits for 0x5d and 0x5e currently. This should be fixed.
>>>
>>> Moreso, it seems that Armoury crate on the Xbox Ally series uses
>>> exclusively 0x5a commands and if you use 0x5d it ignores them (perhaps
>>> RGB still works though). With the previous generation, commands worked
>>> for either report id.
>>>
>>>>>>> @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.
>>>
>>> Early exit-
>>>
>>>>>> 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...
>>>
>>> In [1] Derek removes the name matches
>>>
>>> There are no other name matches concerning this driver in it.
>>>
>>> The data flow is not changed in this series; you should go through the
>>> patches once again if you think that. The only difference is 0x5e is
>>> not sent, and 0x5d is not sent for newer devices.
>>>
>>> [1] https://github.com/ShadowBlip/InputPlumber/pull/453
>>>
>>>>>> 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).
Ah… this was an annoyance. In the hid-asus driver I did I ended up defaulting the LED control to the lamp-array style because it enabled faster per-led control. Then on sleep/resume it applied static colour matching first LED to keep some consistency. It works fine and there is no noticeable delay between switching to/from since the LampArray commands are instant (no set/apply required).
Anyhow, that driver created proper new LED device just for LED control. But it could only do that by taking the Ally HID off of the current driver and managing the whole lot. The end result I thought was much cleaner and separated the actual endpoints out to specific functions instead of how the current driver takes *all* the endpoints and tries to work off usage pages or report ID only.
For example use endpoint 0x83 for configuration (of gamepad) and LED. 0x87 is typically keyboard press events etc.
It did make me wonder if a newer cleaner driver for new MCU 0x19b6 onwards would have worked better instead of trying to shoehorn stuff in to the current driver constantly. It’s dead easy to bring up a new driver for this as an experiment. Maybe both you and Denis could investigate doing so?
>>>>>> 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.
>>>
>>> Yes, we know their specific PIDs, so if you see the patch that adds
>>> the 0x5d init, it is only added for those models.
>>
>> I’m only half keeping up to date on this. I do recall however that the 0x5D init was definitely required for the first ASUS laptop I worked on, and old GX502 - the PID for keyboard is 0x1866 and I think that was the last of that generation MCU. All the previous MCU also required it.
>
> You recall if it was needed to enable the RGB commands or was it only
> for keyboard shortcuts? If it is needed for keyboard shortcuts it is
> correct for it to stay. If RGB does not turn on where it has been
> enabled before, it should also stay.
It was for shortcuts and the ROG buttons above the keyboard. There may have been some laptops using the same MCU that required it to enable LEDs too.
>
>> I saw some messages in perhaps another thread where it was mentioned that 0x5E init should be removed? That I agreed with that?
>> I know there are AniMe and Slash versions that use that init, and they are on the same MCU as the keyboard. I had expected that just one init (on 0x5A or whatever) would work but it doesn’t - what I don’t recall is if an incomplete init affected the keyboard features.
>
> Well, the way these devices work is that there are three hiddev
> devices, usually nested within the same hid endpoint under up to three
> collections. Each has one report ID. 0x5a is for brightness controls,
> 0x5d is for RGB, and 0x5e is for anime. For the first two, I know the
> usages are 76 and 79 (see above). I am not sure what the usage for
> anime is because I do not have a hid descriptor for that device.
>
> In order to start talking to one of the hiddev devices, you are
> supposed to start with an init. The init is bidirectional, so after
> reading it back software knows it is talking to an Asus device (as it
> is done in this series). Likewise, even though it is not the case for
> all MCUs, the MCUs themselves can use it to verify they are talking to
> an Asus application (as you said) and reject commands if it is not
> sent.
Yes, I know.
Before I stopped on all this I built up a rather large (untidy) collection of dumps for various things here https://gitlab.com/asus-linux/reverse-engineering/-/tree/master
>
> For this reason, I think it is a good idea before asusctl starts
> controlling RGB, to always start with a 0x5d init to verify it is
> talking to an Asus device. And before Anime, with a 0x5e init (if the
> specific application for it is available). So since Dennis you are the
> new maintainer, you should try to work that in. Sending it twice does
> not hurt, even if not ideal.
>
> Similarly, because this driver does not do Anime currently, there is
> no reason for it to send 0x5e. It also does not do RGB, so there is no
> reason to send 0x5D (unless not sending it causes issues). For the RGB
> patch I did, I delayed the init purposely until userspace interacts
> with the sysfs RGB endpoint, partly to interfere with userspace
> software less as well. So if the user does not use the sysfs RGB e.g.
> asusctl is completely unaffected.
>
>> In all reality unless the full set of init is causing issues it’s best to leave them in. If it is then I guess this driver is going to become a little more complex and have a few more quirks.
>>
>> Unfortunately I didn’t keep good records of my findings on this so it’s just my remembered observations that you’ll have to take at my word.
>>
>> It would be a good idea for you both to perhaps collaborate with Sergei from ghelper, he has put a huge amount of effort in to that tool and due to it being windows he gets a hell of a lot more use and bug reports/data than this driver does. There’s no shame in looking to others for inspiration, ideas, or guidance.
>
> Good idea. From a quick look, indeed slash/anime is 0x5e. We could
> interact with him more in the future.
>
> Although looking into it, to find the correct endpoint he does a dirty
> check for report length being more than 128[1]. SIgh
Sergei would appreciate any friendly hints for sure. He’s a very nice guy.
>
> I think it would be productive to try to merge this for 6.19. So
> Dennis can you try to be a bit more cooperative?
He’s not being intentionally recalcitrant. You both have vested interest in the outcomes of this review process and from what I’ve seen he has provided some excellent feedback. If something isn’t going the way you want it doesn’t mean it’s personal. You will both converge on acceptable solutions through good faith and communication.
I’ll try to get a look in early on the next patch version and help a little if I can - it would be good to get this work in kernel and you both build off it.
>
> I already have 6 more patches for duo keyboards. Although the keyboard
> brightness button on those seems to not work (?)[2]. I am waiting on a
> reply for that. Perhaps it uses a slightly different ID code. However,
> it seems that brightness works even when disconnecting and connecting
> the keyboard. Which is great.
Do the keys emit any codes? Maybe checking the raw output before it all gets filtered by the driver could help (like printing the raw array as hex) in asus_raw_event(). If there is nothing it could be emitting WMI events. ASUS did a dirty on some laptops and left the default WMI (I probably misremember, but ACPI event at least) in the ACPI, but made them emit nothing and used HID for brightness control instead.
That was this patch: https://github.com/torvalds/linux/commit/a720dee5e039238a44c0142dfccdc0e35c1125f7
Seems likely that because it appears to be a single button brightness cycle it could be a new code. In any case, debug printing the raw array as hex will show it if it’s being emitted.
While I remember, if you ever start playing with per-key RGB I mapped a lot of laptops https://gitlab.com/asus-linux/reverse-engineering/-/blob/master/keyboard/per_key_raw_bytes.ods?ref_type=heads - something to note is that each packet takes 1ms, but due to kernel internals it may attempt a burst of a few, or there could be up to 5ms delay. So a full sequence per row can be 8-20ms or more.
Oh, small reminder: if any patch changes dramatically from what I reviewed my tags should be removed.
>
> Antheas
>
> [1] https://github.com/seerge/g-helper/blob/610b11749b4da97346012e5d47f0a9bbc93b94af/app/AnimeMatrix/Communication/Platform/WindowsUsbProvider.cs#L37
> [2] https://github.com/bazzite-org/kernel-bazzite/issues/35
>
>> Cheers,
>> Luke.
>>
>>>
>>>> No disable commands unless we find hard evidence those are strictly needed.
>>>
>>> They are needed for the Xbox Ally series, but since this driver does
>>> not do RGB it is out of scope.
>>>
>>>>> 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
>>
>>
>>
>
^ permalink raw reply
* Re: [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-11-13 21:17 UTC (permalink / raw)
To: luke
Cc: Denis Benato, Ilpo Järvinen, platform-driver-x86,
linux-input, LKML, Jiri Kosina, Benjamin Tissoires,
Corentin Chary, Hans de Goede
In-Reply-To: <AC058796-5255-41AE-93A1-1D5382F8FDDE@ljones.dev>
On Thu, 13 Nov 2025 at 21:23, <luke@ljones.dev> wrote:
>
>
> > On 13 Nov 2025, at 21:44, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Thu, 13 Nov 2025 at 02:14, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >>
> >> On 11/12/25 23:08, Antheas Kapenekakis wrote:
> >>> On Wed, 12 Nov 2025 at 20:51, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>
> >>>> On 11/12/25 14:41, Antheas Kapenekakis wrote:
> >>>>> On Wed, 12 Nov 2025 at 14:22, Ilpo Järvinen
> >>>>> <ilpo.jarvinen@linux.intel.com> wrote:
> >>>>>> On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
> >>>>>>
> >>>>>>> On Sat, 1 Nov 2025 at 11:47, Antheas Kapenekakis <lkml@antheas.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?
> >
> > Try to add spaces in your replies. This is hard to follow.
> >
> > Do not conflate driver with software. 0x5a (over the application
> > 0xff310076) has traditionally been used by a driver in Windows to
> > control the backlight level, as it is done in this driver. 0x5d (over
> > the application 0xff310079) is only used by laptops with RGB by
> > Armoury crate. But this driver does not do RGB. No device
> > functionality relies on it being sent for any device I've seen. The
> > device remembers its Windows settings, incl. the backlight color, in
> > the absence of a driver.
> >
> > Laptops without RGB such as the Duo series which I would like to add
> > support for next only have a 0x5a endpoint. But, they are sent garbage
> > inits for 0x5d and 0x5e currently. This should be fixed.
> >
> > Moreso, it seems that Armoury crate on the Xbox Ally series uses
> > exclusively 0x5a commands and if you use 0x5d it ignores them (perhaps
> > RGB still works though). With the previous generation, commands worked
> > for either report id.
> >
> >>>>> @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.
> >
> > Early exit-
> >
> >>>> 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...
> >
> > In [1] Derek removes the name matches
> >
> > There are no other name matches concerning this driver in it.
> >
> > The data flow is not changed in this series; you should go through the
> > patches once again if you think that. The only difference is 0x5e is
> > not sent, and 0x5d is not sent for newer devices.
> >
> > [1] https://github.com/ShadowBlip/InputPlumber/pull/453
> >
> >>>> 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.
> >
> > Yes, we know their specific PIDs, so if you see the patch that adds
> > the 0x5d init, it is only added for those models.
>
> I’m only half keeping up to date on this. I do recall however that the 0x5D init was definitely required for the first ASUS laptop I worked on, and old GX502 - the PID for keyboard is 0x1866 and I think that was the last of that generation MCU. All the previous MCU also required it.
You recall if it was needed to enable the RGB commands or was it only
for keyboard shortcuts? If it is needed for keyboard shortcuts it is
correct for it to stay. If RGB does not turn on where it has been
enabled before, it should also stay.
> I saw some messages in perhaps another thread where it was mentioned that 0x5E init should be removed? That I agreed with that?
> I know there are AniMe and Slash versions that use that init, and they are on the same MCU as the keyboard. I had expected that just one init (on 0x5A or whatever) would work but it doesn’t - what I don’t recall is if an incomplete init affected the keyboard features.
Well, the way these devices work is that there are three hiddev
devices, usually nested within the same hid endpoint under up to three
collections. Each has one report ID. 0x5a is for brightness controls,
0x5d is for RGB, and 0x5e is for anime. For the first two, I know the
usages are 76 and 79 (see above). I am not sure what the usage for
anime is because I do not have a hid descriptor for that device.
In order to start talking to one of the hiddev devices, you are
supposed to start with an init. The init is bidirectional, so after
reading it back software knows it is talking to an Asus device (as it
is done in this series). Likewise, even though it is not the case for
all MCUs, the MCUs themselves can use it to verify they are talking to
an Asus application (as you said) and reject commands if it is not
sent.
For this reason, I think it is a good idea before asusctl starts
controlling RGB, to always start with a 0x5d init to verify it is
talking to an Asus device. And before Anime, with a 0x5e init (if the
specific application for it is available). So since Dennis you are the
new maintainer, you should try to work that in. Sending it twice does
not hurt, even if not ideal.
Similarly, because this driver does not do Anime currently, there is
no reason for it to send 0x5e. It also does not do RGB, so there is no
reason to send 0x5D (unless not sending it causes issues). For the RGB
patch I did, I delayed the init purposely until userspace interacts
with the sysfs RGB endpoint, partly to interfere with userspace
software less as well. So if the user does not use the sysfs RGB e.g.
asusctl is completely unaffected.
> In all reality unless the full set of init is causing issues it’s best to leave them in. If it is then I guess this driver is going to become a little more complex and have a few more quirks.
>
> Unfortunately I didn’t keep good records of my findings on this so it’s just my remembered observations that you’ll have to take at my word.
>
> It would be a good idea for you both to perhaps collaborate with Sergei from ghelper, he has put a huge amount of effort in to that tool and due to it being windows he gets a hell of a lot more use and bug reports/data than this driver does. There’s no shame in looking to others for inspiration, ideas, or guidance.
Good idea. From a quick look, indeed slash/anime is 0x5e. We could
interact with him more in the future.
Although looking into it, to find the correct endpoint he does a dirty
check for report length being more than 128[1]. SIgh
I think it would be productive to try to merge this for 6.19. So
Dennis can you try to be a bit more cooperative?
I already have 6 more patches for duo keyboards. Although the keyboard
brightness button on those seems to not work (?)[2]. I am waiting on a
reply for that. Perhaps it uses a slightly different ID code. However,
it seems that brightness works even when disconnecting and connecting
the keyboard. Which is great.
Antheas
[1] https://github.com/seerge/g-helper/blob/610b11749b4da97346012e5d47f0a9bbc93b94af/app/AnimeMatrix/Communication/Platform/WindowsUsbProvider.cs#L37
[2] https://github.com/bazzite-org/kernel-bazzite/issues/35
> Cheers,
> Luke.
>
> >
> >> No disable commands unless we find hard evidence those are strictly needed.
> >
> > They are needed for the Xbox Ally series, but since this driver does
> > not do RGB it is out of scope.
> >
> >>> 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
>
>
>
^ permalink raw reply
* Re: [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: luke @ 2025-11-13 20:23 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Denis Benato, Ilpo Järvinen, platform-driver-x86,
linux-input, LKML, Jiri Kosina, Benjamin Tissoires,
Corentin Chary, Hans de Goede
In-Reply-To: <CAGwozwFDm80YuC9AfES2d7Xk2bnCNPjHtgXCz5gZuh7fuajHgg@mail.gmail.com>
> On 13 Nov 2025, at 21:44, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Thu, 13 Nov 2025 at 02:14, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>>
>> On 11/12/25 23:08, Antheas Kapenekakis wrote:
>>> On Wed, 12 Nov 2025 at 20:51, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>
>>>> On 11/12/25 14:41, Antheas Kapenekakis wrote:
>>>>> On Wed, 12 Nov 2025 at 14:22, Ilpo Järvinen
>>>>> <ilpo.jarvinen@linux.intel.com> wrote:
>>>>>> On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
>>>>>>
>>>>>>> On Sat, 1 Nov 2025 at 11:47, Antheas Kapenekakis <lkml@antheas.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?
>
> Try to add spaces in your replies. This is hard to follow.
>
> Do not conflate driver with software. 0x5a (over the application
> 0xff310076) has traditionally been used by a driver in Windows to
> control the backlight level, as it is done in this driver. 0x5d (over
> the application 0xff310079) is only used by laptops with RGB by
> Armoury crate. But this driver does not do RGB. No device
> functionality relies on it being sent for any device I've seen. The
> device remembers its Windows settings, incl. the backlight color, in
> the absence of a driver.
>
> Laptops without RGB such as the Duo series which I would like to add
> support for next only have a 0x5a endpoint. But, they are sent garbage
> inits for 0x5d and 0x5e currently. This should be fixed.
>
> Moreso, it seems that Armoury crate on the Xbox Ally series uses
> exclusively 0x5a commands and if you use 0x5d it ignores them (perhaps
> RGB still works though). With the previous generation, commands worked
> for either report id.
>
>>>>> @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.
>
> Early exit-
>
>>>> 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...
>
> In [1] Derek removes the name matches
>
> There are no other name matches concerning this driver in it.
>
> The data flow is not changed in this series; you should go through the
> patches once again if you think that. The only difference is 0x5e is
> not sent, and 0x5d is not sent for newer devices.
>
> [1] https://github.com/ShadowBlip/InputPlumber/pull/453
>
>>>> 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.
>
> Yes, we know their specific PIDs, so if you see the patch that adds
> the 0x5d init, it is only added for those models.
I’m only half keeping up to date on this. I do recall however that the 0x5D init was definitely required for the first ASUS laptop I worked on, and old GX502 - the PID for keyboard is 0x1866 and I think that was the last of that generation MCU. All the previous MCU also required it.
I saw some messages in perhaps another thread where it was mentioned that 0x5E init should be removed? That I agreed with that?
I know there are AniMe and Slash versions that use that init, and they are on the same MCU as the keyboard. I had expected that just one init (on 0x5A or whatever) would work but it doesn’t - what I don’t recall is if an incomplete init affected the keyboard features.
In all reality unless the full set of init is causing issues it’s best to leave them in. If it is then I guess this driver is going to become a little more complex and have a few more quirks.
Unfortunately I didn’t keep good records of my findings on this so it’s just my remembered observations that you’ll have to take at my word.
It would be a good idea for you both to perhaps collaborate with Sergei from ghelper, he has put a huge amount of effort in to that tool and due to it being windows he gets a hell of a lot more use and bug reports/data than this driver does. There’s no shame in looking to others for inspiration, ideas, or guidance.
Cheers,
Luke.
>
>> No disable commands unless we find hard evidence those are strictly needed.
>
> They are needed for the Xbox Ally series, but since this driver does
> not do RGB it is out of scope.
>
>>> 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
^ permalink raw reply
* Re: [PATCH 1/1] hid: hid-playstation: fix memleak in probe
From: Silvan Jegen @ 2025-11-13 20:04 UTC (permalink / raw)
To: Oliver Neukum; +Cc: roderick.colenbrander, jikos, bentiss, linux-input
In-Reply-To: <20251113192713.537344-1-oneukum@suse.com>
Oliver Neukum <oneukum@suse.com> wrote:
> Free the buffer if we bail out in the error case of
> dualshock4_get_calibration_data()
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
> drivers/hid/hid-playstation.c | 2 ++
> 1 file changed, 2 insertions(+)
This LGTM!
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 63f6eb9030d1..128aa6abd10b 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -1942,6 +1942,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
> "Failed to retrieve DualShock4 calibration info: %d\n",
> ret);
> ret = -EILSEQ;
> + kfree(buf);
> goto transfer_failed;
> } else {
> break;
> @@ -1959,6 +1960,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
>
> if (ret) {
> hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
> + kfree(buf);
> goto transfer_failed;
> }
> }
^ permalink raw reply
* Re: [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device
From: Sebastian Reichel @ 2025-11-13 11:16 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires,
Bastien Nocera, linux-pm
In-Reply-To: <20251113001508.713574-1-lzampier@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 978 bytes --]
Hi,
On Thu, Nov 13, 2025 at 12:15:02AM +0000, Lucas Zampieri wrote:
> ## Request for Comments
>
> Is list-based storage appropriate or would another structure work better?
> Should we support usage-based identification in addition to report ID for
> devices using the same report ID? Is sequential naming (battery-N) sufficient
> or should batteries have semantic role identifiers like "main", "stylus", "dock"?
If unique semantic identifiers are available they are obviously a better
choice.
> To HID maintainers (Jiri Kosina, Benjamin Tissoires): Does this belong in
> hid-input.c or should it be separate? Any concerns about the backwards
> compatibility approach? Meaning, should I have removed the whole
> dev->bat legacy mapping and use the new struct?
>
> To power supply maintainers (Sebastian Reichel): Any issues with multiple
> power_supply devices from a single HID device?
I don't see any issues with that.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/1] hid: hid-playstation: fix memleak in probe
From: Oliver Neukum @ 2025-11-13 19:26 UTC (permalink / raw)
To: roderick.colenbrander, jikos, bentiss, linux-input; +Cc: Oliver Neukum
Free the buffer if we bail out in the error case of
dualshock4_get_calibration_data()
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/hid/hid-playstation.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 63f6eb9030d1..128aa6abd10b 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1942,6 +1942,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
"Failed to retrieve DualShock4 calibration info: %d\n",
ret);
ret = -EILSEQ;
+ kfree(buf);
goto transfer_failed;
} else {
break;
@@ -1959,6 +1960,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
if (ret) {
hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+ kfree(buf);
goto transfer_failed;
}
}
--
2.51.1
^ permalink raw reply related
* Re: [PATCH v4 2/2] HID: i2c-hid: Add FocalTech FT8112
From: Doug Anderson @ 2025-11-13 18:47 UTC (permalink / raw)
To: daniel_peng
Cc: Dmitry Torokhov, linux-input, LKML, Benjamin Tissoires,
Jiri Kosina, Pin-yen Lin
In-Reply-To: <20251113140004.v4.2.I47e9f0b76399e6009854dec06420c20e68e23116@changeid>
Hi,
On Wed, Nov 12, 2025 at 10:01 PM
<daniel_peng@pegatron.corp-partner.google.com> wrote:
>
> From: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
>
> Information for touchscreen model HKO/RB116AS01-2 as below:
> - HID :FTSC1000
> - slave address:0X38
> - Interface:HID over I2C
> - Touch control lC:FT8112
> - I2C ID: PNP0C50
>
> Signed-off-by: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
> ---
>
> Changes in v4:
> - Move change log of v3 under "---" correctly as below 1 item.
>
> - Modified the subject to include the ID of the chip as suggestion only.
>
> drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* Re: [PATCH v2] Apply the quirk HID_QUIRK_ALWAYS_POLL to the Edifier QR30 (2d99:a101).
From: Terry Junge @ 2025-11-13 17:45 UTC (permalink / raw)
To: The-Luga, Alan Stern
Cc: michal.pecio, bentiss, dmitry.torokhov, jikos, linux-input,
linux-sound, linux-usb, linuxsound
In-Reply-To: <CALvgqEBVQsoQ3wewP+37u5Ms398O5gC8YaELm0UJdZSDBHzPPw@mail.gmail.com>
On 11/13/2025 7:45 AM, The-Luga wrote:
>> Vendor ID 0x2d99 belongs to Edifier International Limited not Jieli
>> Can you change to USB_VENDOR_ID_EDIFIER instead and move to the
>> alphabetically correct location?
>
> Could you kindly tell me where this info is?
> Searching the internet I only found this website with this info:
> https://the-sz.com/products/usbid/index.php?v=0x2D99
The official list of valid USB Vendor IDs can be found here:
https://www.usb.org/developers
currently:
https://www.usb.org/sites/default/files/vendor_ids10282025b_1.pdf
>
> I just checked the vendor from `lsusb`, and I was hesitant whether to
> write Jieli or Edifier.
> I also decided to write QR30 instead of Hal0. Should I add a comment
> mentioning Jieli as I did with the device?
The product was made using the Jieli SDK which supports Jieli chips. The
manufacturers string "Jieli Technology" is the default value in the SDK
and should have been changed by Edifier.
No need to mention Jieli at all. The bottom line is that the product is
not 100% USB compliant and does not support suspend mode reliably so
requires the HID_QUIRK_ALWAYS_POLL quirk.
Regards,
Terry
^ permalink raw reply
* [PATCH v6 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
From: David Heidelberg via B4 Relay @ 2025-11-13 16:31 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which do not
expose the function number and the interrupt status in its PDT entries.
OnePlus 6 (original touch IC)
rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2b 22 0d 06 01 01)
OnePlus 6 (aftermarket touch IC)
rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2c 23 0d 06 00 00)
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
[codeflow adjustments, checkpatch fixes, wording]
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_driver.c | 62 +++++++++++++++++++++++++++++++++++------
drivers/input/rmi4/rmi_driver.h | 2 ++
include/linux/rmi.h | 3 ++
3 files changed, 59 insertions(+), 8 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 93a190e333c66..bb1db5bbb3abb 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -462,9 +462,10 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
return 0;
}
-static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
- struct pdt_entry *entry, u16 pdt_address)
+static int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
+ struct pdt_scan_state *state, u16 pdt_address)
{
+ const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
u8 buf[RMI_PDT_ENTRY_SIZE];
int error;
@@ -475,6 +476,21 @@ static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
return error;
}
+ if (pdata->pdt_fallback_size > state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1) {
+ /* Use the description bytes from the driver */
+ buf[5] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS];
+ buf[4] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1];
+
+ error = rmi_read_block(rmi_dev, pdt_address, buf,
+ RMI_PDT_ENTRY_SIZE - 2);
+ if (error) {
+ dev_err(&rmi_dev->dev,
+ "Read PDT entry at %#06x failed, code: %d.\n",
+ pdt_address, error);
+ return error;
+ }
+ }
+
entry->page_start = pdt_address & RMI4_PAGE_MASK;
entry->query_base_addr = buf[0];
entry->command_base_addr = buf[1];
@@ -547,7 +563,7 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int retval;
for (addr = pdt_start; addr >= pdt_end; addr -= RMI_PDT_ENTRY_SIZE) {
- error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, addr);
+ error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, state, addr);
if (error)
return error;
@@ -1024,9 +1040,13 @@ static int rmi_driver_remove(struct device *dev)
}
#ifdef CONFIG_OF
-static int rmi_driver_of_probe(struct device *dev,
- struct rmi_device_platform_data *pdata)
+static const u8 rmi_s3706_fallback_pdt[] = {34, 41, 01, 01, 12, 01};
+
+static int rmi_driver_of_probe(struct rmi_device *rmi_dev,
+ struct rmi_device_platform_data *pdata)
{
+ struct device *dev = rmi_dev->xport->dev;
+ u8 buf[RMI_PDT_ENTRY_SIZE];
int retval;
retval = rmi_of_property_read_u32(dev, &pdata->reset_delay_ms,
@@ -1034,11 +1054,37 @@ static int rmi_driver_of_probe(struct device *dev,
if (retval)
return retval;
+ /*
+ * In some aftermerket touch ICs, the first PDT entry is empty and
+ * the function number register is 0. If so, the driver
+ * may have provide backup PDT entries.
+ */
+
+ retval = rmi_read_block(rmi_dev, PDT_START_SCAN_LOCATION,
+ buf, RMI_PDT_ENTRY_SIZE);
+ if (retval) {
+ dev_err(dev, "Read PDT entry at %#06x failed, code: %d.\n",
+ PDT_START_SCAN_LOCATION, retval);
+ return retval;
+ }
+
+ if (!RMI4_END_OF_PDT(buf[5]))
+ return 0;
+
+ /* List of known PDT entries per compatible. */
+ if (of_device_is_compatible(dev->of_node, "syna,rmi4-s3706b")) {
+ pdata->pdt_fallback_desc = rmi_s3706_fallback_pdt;
+ pdata->pdt_fallback_size = ARRAY_SIZE(rmi_s3706_fallback_pdt);
+ } else {
+ dev_err(dev, "First PDT entry is empty and no backup values provided.\n");
+ return -EINVAL;
+ }
+
return 0;
}
#else
-static inline int rmi_driver_of_probe(struct device *dev,
- struct rmi_device_platform_data *pdata)
+static inline int rmi_driver_of_probe(struct rmi_device *rmi_dev,
+ struct rmi_device_platform_data *pdata)
{
return -ENODEV;
}
@@ -1159,7 +1205,7 @@ static int rmi_driver_probe(struct device *dev)
pdata = rmi_get_platform_data(rmi_dev);
if (rmi_dev->xport->dev->of_node) {
- retval = rmi_driver_of_probe(rmi_dev->xport->dev, pdata);
+ retval = rmi_driver_of_probe(rmi_dev, pdata);
if (retval)
return retval;
}
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index a4ae2af93ce3a..b931f428713bf 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -31,6 +31,8 @@
#define RMI_PDT_FUNCTION_VERSION_MASK 0x60
#define RMI_PDT_INT_SOURCE_COUNT_MASK 0x07
+#define RMI_OF_PDT_DESC_CELLS 2
+
#define PDT_START_SCAN_LOCATION 0x00e9
#define PDT_END_SCAN_LOCATION 0x0005
#define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ab7eea01ab427..4ba2cefac8558 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -214,6 +214,9 @@ struct rmi_device_platform_data {
int reset_delay_ms;
int irq;
+ unsigned int pdt_fallback_size;
+ const u8 *pdt_fallback_desc;
+
struct rmi_device_platform_data_spi spi_data;
/* function handler pdata */
--
2.51.0
^ permalink raw reply related
* [PATCH v6 6/7] Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
From: David Heidelberg via B4 Relay @ 2025-11-13 16:31 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which do not
report the product ID correctly unless we read directly from the
product ID register. Add a check and a fallback read to handle this.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f01.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 47be64284b25e..2278e9b6a9207 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -250,6 +250,20 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
}
}
+ /*
+ * Some aftermarket ICs put garbage into the product id field unless
+ * we read directly from the product id register.
+ */
+ if (props->product_id[0] < 0x20) {
+ ret = rmi_read_block(rmi_dev, query_base_addr + 11,
+ props->product_id, RMI_PRODUCT_ID_LENGTH);
+ if (ret) {
+ dev_err(&rmi_dev->dev,
+ "Failed to read product id: %d\n", ret);
+ return ret;
+ }
+ }
+
return 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v6 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count
From: David Heidelberg via B4 Relay @ 2025-11-13 16:31 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some third party ICs claim to support f55 but report an electrode count
of 0. Catch this and bail out early so that we don't confuse the i2c bus
with 0 sized reads.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
[simplify code, adjust wording]
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f55.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/rmi4/rmi_f55.c b/drivers/input/rmi4/rmi_f55.c
index 488adaca4dd00..776c915b82e72 100644
--- a/drivers/input/rmi4/rmi_f55.c
+++ b/drivers/input/rmi4/rmi_f55.c
@@ -52,6 +52,11 @@ static int rmi_f55_detect(struct rmi_function *fn)
f55->num_rx_electrodes = f55->qry[F55_NUM_RX_OFFSET];
f55->num_tx_electrodes = f55->qry[F55_NUM_TX_OFFSET];
+ if (!f55->num_rx_electrodes || !f55->num_tx_electrodes) {
+ dev_err(&fn->dev, "%s: F55 query returned no electrodes, giving up\n",
+ __func__);
+ return -EINVAL;
+ }
f55->cfg_num_rx_electrodes = f55->num_rx_electrodes;
f55->cfg_num_tx_electrodes = f55->num_rx_electrodes;
--
2.51.0
^ permalink raw reply related
* [PATCH v6 5/7] Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
From: David Heidelberg via B4 Relay @ 2025-11-13 16:31 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which incur a
significant penalty (1-2 seconds) when doing certain unaligned reads.
This is enough to break functionality when it happens in the hot path,
so adjust the interrupt handler to not read from an unaligned address.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_driver.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index c7d2f68e65487..93a190e333c66 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -137,9 +137,14 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
return 0;
if (!data->attn_data.data) {
+ /*
+ * Read the device status register as well and ignore it.
+ * Some aftermarket ICs have issues with interrupt requests
+ * otherwise.
+ */
error = rmi_read_block(rmi_dev,
- data->f01_container->fd.data_base_addr + 1,
- data->irq_status, data->num_of_irq_regs);
+ data->f01_container->fd.data_base_addr,
+ (u8 *)data->irq_status - 1, data->num_of_irq_regs + 1);
if (error < 0) {
dev_err(dev, "Failed to read irqs, code=%d\n", error);
return error;
@@ -1079,16 +1084,17 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
data->num_of_irq_regs = (data->irq_count + 7) / 8;
size = BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long);
- data->irq_memory = devm_kcalloc(dev, size, 4, GFP_KERNEL);
+ data->irq_memory = devm_kzalloc(dev, size * 4 + 1, GFP_KERNEL);
if (!data->irq_memory) {
dev_err(dev, "Failed to allocate memory for irq masks.\n");
return -ENOMEM;
}
- data->irq_status = data->irq_memory + size * 0;
- data->fn_irq_bits = data->irq_memory + size * 1;
- data->current_irq_mask = data->irq_memory + size * 2;
- data->new_irq_mask = data->irq_memory + size * 3;
+ /* The first byte is reserved for the device status register */
+ data->irq_status = data->irq_memory + size * 0 + 1;
+ data->fn_irq_bits = data->irq_memory + size * 1 + 1;
+ data->current_irq_mask = data->irq_memory + size * 2 + 1;
+ data->new_irq_mask = data->irq_memory + size * 3 + 1;
return retval;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v6 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
From: David Heidelberg via B4 Relay @ 2025-11-13 16:30 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which are
devoid of register descriptors. Create a fake data register descriptor
for such ICs and provide hardcoded default values.
It isn't possible to reliably determine if the touch IC is original or
not, so these fallback values are offered as an alternative to the error
path when register descriptors aren't available.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
[changes for readability / codeflow, checkpatch fixes]
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f12.c | 117 +++++++++++++++++++++++++++++++++----------
1 file changed, 91 insertions(+), 26 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 8246fe77114bb..1a103cc5f2235 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -218,6 +218,41 @@ static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, int size)
rmi_2d_sensor_abs_report(sensor, &sensor->objs[i], i);
}
+static void rmi_f12_set_hardcoded_desc(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+ struct rmi_register_desc_item *reg_desc;
+
+ /* We have no f12->data_reg_desc, so the pkt_size is 0, override it with
+ * a somewhat sensible default (this corresponds to 10 fingers).
+ */
+ sensor->pkt_size = 88;
+
+ /*
+ * There are no register descriptors to get these values from.
+ * We set them to high values to either be overwritten by the clip
+ * properties from devicetree, or to just not get in the way.
+ */
+ sensor->max_x = 65535;
+ sensor->max_y = 65535;
+
+ /*
+ * Create the Data1 register descriptor so that touch events
+ * can work properly.
+ */
+ reg_desc = devm_kcalloc(&fn->dev, 1,
+ sizeof(struct rmi_register_desc_item), GFP_KERNEL);
+ reg_desc->reg = 1;
+ reg_desc->reg_size = 80;
+ reg_desc->num_subpackets = 10;
+
+ f12->data1 = reg_desc;
+ f12->data1_offset = 0;
+ sensor->nbr_fingers = reg_desc->num_subpackets;
+ sensor->report_abs = 1;
+ sensor->attn_size += reg_desc->reg_size;
+}
+
static irqreturn_t rmi_f12_attention(int irq, void *ctx)
{
int retval;
@@ -338,6 +373,40 @@ static int rmi_f12_config(struct rmi_function *fn)
return 0;
}
+static int rmi_f12_sensor_init(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+
+ sensor->fn = fn;
+ f12->data_addr = fn->fd.data_base_addr;
+
+ /* On quirky devices that don't have a data_reg_desc we hardcode the packet
+ * in rmi_f12_set_hardcoded_desc(). Make sure not to set it to 0 here.
+ */
+ if (!sensor->pkt_size)
+ sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
+
+ sensor->axis_align =
+ f12->sensor_pdata.axis_align;
+
+ sensor->x_mm = f12->sensor_pdata.x_mm;
+ sensor->y_mm = f12->sensor_pdata.y_mm;
+ sensor->dribble = f12->sensor_pdata.dribble;
+
+ if (sensor->sensor_type == rmi_sensor_default)
+ sensor->sensor_type =
+ f12->sensor_pdata.sensor_type;
+
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
+ sensor->pkt_size);
+
+ sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
+ if (!sensor->data_pkt)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int rmi_f12_probe(struct rmi_function *fn)
{
struct f12_data *f12;
@@ -351,6 +420,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
u16 data_offset = 0;
int mask_size;
+ bool hardcoded_desc_quirk = false;
rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
@@ -365,9 +435,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
++query_addr;
if (!(buf & BIT(0))) {
- dev_err(&fn->dev,
- "Behavior of F12 without register descriptors is undefined.\n");
- return -ENODEV;
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+ "No register descriptors defined for F12, using fallback\n");
+ hardcoded_desc_quirk = true;
}
f12 = devm_kzalloc(&fn->dev, sizeof(struct f12_data) + mask_size * 2,
@@ -375,6 +445,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
if (!f12)
return -ENOMEM;
+ dev_set_drvdata(&fn->dev, f12);
+
f12->abs_mask = (unsigned long *)((char *)f12
+ sizeof(struct f12_data));
f12->rel_mask = (unsigned long *)((char *)f12
@@ -393,6 +465,18 @@ static int rmi_f12_probe(struct rmi_function *fn)
f12->sensor_pdata = pdata->sensor_pdata;
}
+ sensor = &f12->sensor;
+
+ if (hardcoded_desc_quirk) {
+ rmi_f12_set_hardcoded_desc(fn, f12);
+
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
+
+ goto skip_register_desc;
+ }
+
ret = rmi_read_register_desc(rmi_dev, query_addr,
&f12->query_reg_desc);
if (ret) {
@@ -423,29 +507,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
}
query_addr += 3;
- sensor = &f12->sensor;
- sensor->fn = fn;
- f12->data_addr = fn->fd.data_base_addr;
- sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
-
- sensor->axis_align =
- f12->sensor_pdata.axis_align;
-
- sensor->x_mm = f12->sensor_pdata.x_mm;
- sensor->y_mm = f12->sensor_pdata.y_mm;
- sensor->dribble = f12->sensor_pdata.dribble;
-
- if (sensor->sensor_type == rmi_sensor_default)
- sensor->sensor_type =
- f12->sensor_pdata.sensor_type;
-
- rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
- sensor->pkt_size);
- sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
- if (!sensor->data_pkt)
- return -ENOMEM;
-
- dev_set_drvdata(&fn->dev, f12);
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
ret = rmi_f12_read_sensor_tuning(f12);
if (ret)
@@ -543,6 +607,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
data_offset += item->reg_size;
}
+skip_register_desc:
/* allocate the in-kernel tracking buffers */
sensor->tracking_pos = devm_kcalloc(&fn->dev,
sensor->nbr_fingers, sizeof(struct input_mt_pos),
--
2.51.0
^ permalink raw reply related
* [PATCH v6 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
From: David Heidelberg via B4 Relay @ 2025-11-13 16:30 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: Casey Connolly <casey.connolly@linaro.org>
Some third party rmi4-compatible ICs don't expose their PDT entries
very well. Add a few checks to skip duplicate entries as well as entries
for unsupported functions.
This is required to support some phones with third party displays.
Validated on a stock OnePlus 6T (original parts):
manufacturer: Synaptics, product: S3706B, fw id: 2852315
Co-developed-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_driver.c | 42 +++++++++++++++++++++++++++++++++++------
drivers/input/rmi4/rmi_driver.h | 8 ++++++++
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index ccd9338a44dbe..c7d2f68e65487 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -494,12 +494,39 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
fd->function_version = pdt->function_version;
}
+static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
+ struct pdt_scan_state *state, u8 fn)
+{
+ switch (fn) {
+ case 0x01:
+ case 0x03:
+ case 0x11:
+ case 0x12:
+ case 0x30:
+ case 0x34:
+ case 0x3a:
+ case 0x54:
+ case 0x55:
+ if (state->pdts[fn] == true)
+ return false;
+ break;
+ default:
+ rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
+ "PDT has unknown function number %#02x\n", fn);
+ return false;
+ }
+
+ state->pdts[fn] = true;
+ state->pdt_count++;
+ return true;
+}
+
#define RMI_SCAN_CONTINUE 0
#define RMI_SCAN_DONE 1
static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int page,
- int *empty_pages,
+ struct pdt_scan_state *state,
void *ctx,
int (*callback)(struct rmi_device *rmi_dev,
void *ctx,
@@ -522,6 +549,9 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
if (RMI4_END_OF_PDT(pdt_entry.function_number))
break;
+ if (!rmi_pdt_entry_is_valid(rmi_dev, state, pdt_entry.function_number))
+ continue;
+
retval = callback(rmi_dev, ctx, &pdt_entry);
if (retval != RMI_SCAN_CONTINUE)
return retval;
@@ -532,11 +562,11 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
* or more is found, stop scanning.
*/
if (addr == pdt_start)
- ++*empty_pages;
+ ++state->empty_pages;
else
- *empty_pages = 0;
+ state->empty_pages = 0;
- return (data->bootloader_mode || *empty_pages >= 2) ?
+ return (data->bootloader_mode || state->empty_pages >= 2) ?
RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
}
@@ -545,11 +575,11 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
void *ctx, const struct pdt_entry *entry))
{
int page;
- int empty_pages = 0;
+ struct pdt_scan_state state = {0, 0, {0}};
int retval = RMI_SCAN_DONE;
for (page = 0; page <= RMI4_MAX_PAGE; page++) {
- retval = rmi_scan_pdt_page(rmi_dev, page, &empty_pages,
+ retval = rmi_scan_pdt_page(rmi_dev, page, &state,
ctx, callback);
if (retval != RMI_SCAN_CONTINUE)
break;
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index e84495caab151..a4ae2af93ce3a 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -46,6 +46,14 @@ struct pdt_entry {
u8 function_number;
};
+#define RMI_PDT_MAX 0x55
+
+struct pdt_scan_state {
+ u8 empty_pages;
+ u8 pdt_count;
+ bool pdts[RMI_PDT_MAX];
+};
+
#define RMI_REG_DESC_PRESENSE_BITS (32 * BITS_PER_BYTE)
#define RMI_REG_DESC_SUBPACKET_BITS (37 * BITS_PER_BYTE)
--
2.51.0
^ permalink raw reply related
* [PATCH v6 1/7] dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
From: David Heidelberg via B4 Relay @ 2025-11-13 16:30 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg, Krzysztof Kozlowski
In-Reply-To: <20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz>
From: David Heidelberg <david@ixit.cz>
Mostly irrelevant for authentic Synaptics touchscreens, but very important
for applying workarounds to cheap TS knockoffs.
These knockoffs work well with the downstream driver, and since the user
has no way to distinguish them, later in this patch set, we introduce
workarounds to ensure they function as well as possible.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Documentation/devicetree/bindings/input/syna,rmi4.yaml | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index f369385ffaf02..e0e006c9c9ee4 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -18,9 +18,14 @@ description: |
properties:
compatible:
- enum:
- - syna,rmi4-i2c
- - syna,rmi4-spi
+ oneOf:
+ - enum:
+ - syna,rmi4-i2c
+ - syna,rmi4-spi
+ - items:
+ - enum:
+ - syna,rmi4-s3706b # OnePlus 6/6T
+ - const: syna,rmi4-i2c
reg:
maxItems: 1
--
2.51.0
^ permalink raw reply related
* [PATCH v6 0/7] Input: synaptics-rmi4 - add quirks for third party touchscreen controllers
From: David Heidelberg via B4 Relay @ 2025-11-13 16:30 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel, Casey Connolly,
phone-devel, David Heidelberg, Krzysztof Kozlowski
With the growing popularity of running upstream Linux on mobile devices,
we're beginning to run into more and more edgecases. The OnePlus 6 is a
fairly well supported 2018 era smartphone, selling over a million units
in it's first 22 days. With this level of popularity, it's almost
inevitable that we get third party replacement displays, and as a
result, replacement touchscreen controllers.
The OnePlus 6 shipped with an extremely usecase specific touchscreen
driver, it implemented only the bare minimum parts of the highly generic
rmi4 protocol, instead hardcoding most of the register addresses.
As a result, the third party touchscreen controllers that are often
found in replacement screens, implement only the registers that the
downstream driver reads from. They additionally have other restrictions
such as heavy penalties on unaligned reads.
This series attempts to implement the necessary workaround to support
some of these chips with the rmi4 driver. Although it's worth noting
that at the time of writing there are other unofficial controllers in
the wild that don't work even with these patches.
We have been shipping these patches in postmarketOS for the last several
years, and they are known to not cause any regressions on the OnePlus
6/6T (with the official Synaptics controller), however I don't own any
other rmi4 hardware to further validate this.
The series is also available (until merged) at
https://gitlab.com/sdm845/sdm845-next/-/commits/b4/synaptics-rmi4
---
Changes in v6:
- Rebased on top of next-20251113.
- No other change since the Rob Herring comment.
- Link to v5: https://lore.kernel.org/r/20250410-synaptics-rmi4-v5-0-b41bb90f78b9@ixit.cz
Changes in v5:
- Removed -i2c suffix from rmi4-s3706b-i2c (Krzysztof).
- Link to v4: https://lore.kernel.org/r/20250402-synaptics-rmi4-v4-0-1bb95959e564@ixit.cz
Changes in v4:
- Replaced patch "dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc"
with patch documenting specific touchscreen model used in OnePlus 6 and 6T.
- Fixed zero electrode return code (Dmitry).
- Switched the duplicate detection algo to bitmap (Dmitry).
- Optimized rmi_device_platform_data struct to avoid unnecessary
padding.
- Changed fallback_size from int to unsigned int.
- Changed SoB from nickname and old address (methanal <baclofen@tuta.io>) to
Kaustabh Chakraborty <kauschluss@disroot.org>.
Verified ownership through the sdm845 chatroom on Matrix.
- Link to v3: https://lore.kernel.org/r/20250308-synaptics-rmi4-v3-0-215d3e7289a2@ixit.cz
Changes in v3:
- reworded dt-bindings property description
- fixed the rmi_driver_of_probe definition for non device-tree builds.
- fixed some indentation issues reported by checkpatch
- change rmi_pdt_entry_is_valid() variable to unsigned
- Link to v2: https://lore.kernel.org/all/20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org
Changes in v2:
- Improve dt-bindings patch (thanks Rob)
- Add missing cast in patch 5 to fix the pointer arithmetic
- Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org
---
Casey Connolly (1):
Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
David Heidelberg (1):
dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
Kaustabh Chakraborty (5):
Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
Input: synaptics-rmi4 - f55: handle zero electrode count
Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
.../devicetree/bindings/input/syna,rmi4.yaml | 11 +-
drivers/input/rmi4/rmi_driver.c | 124 +++++++++++++++++----
drivers/input/rmi4/rmi_driver.h | 10 ++
drivers/input/rmi4/rmi_f01.c | 14 +++
drivers/input/rmi4/rmi_f12.c | 117 ++++++++++++++-----
drivers/input/rmi4/rmi_f55.c | 5 +
include/linux/rmi.h | 3 +
7 files changed, 234 insertions(+), 50 deletions(-)
---
base-commit: 6d7e7251d03f98f26f2ee0dfd21bb0a0480a2178
change-id: 20250308-synaptics-rmi4-c832b2f73ceb
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply
* Re: (subset) [PATCH v4 1/3] dt-bindings: mfd: twl: enable power button also for twl603x
From: Lee Jones @ 2025-11-13 16:07 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andreas Kemnade, Dmitry Torokhov, Tony Lindgren, Kevin Hilman,
akemnade
Cc: devicetree, linux-kernel, linux-input, linux-omap
In-Reply-To: <20251106-twl6030-button-v4-1-fdf1aa6e1e9a@kernel.org>
On Thu, 06 Nov 2025 10:00:25 +0100, akemnade@kernel.org wrote:
> TWL603x has also a power button function, so add the corresponding subnode.
> As not in all cases there is a power button connected to the corresponding
> pad of the TWL603x, the functionality can be disabled by
> status = "disabled" or simply not adding the subnode.
> To keep things simple, follow the established design pattern of using const
> interrupts as used also by the other subdevices.
>
> [...]
Applied, thanks!
[1/3] dt-bindings: mfd: twl: enable power button also for twl603x
commit: 65a4ee23e057f60d25c3436acd88f2f169f780c2
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH v8 2/2] arm64: dts: qcom: sdm845-oneplus: Add alert-slider
From: David Heidelberg via B4 Relay @ 2025-11-13 16:02 UTC (permalink / raw)
To: Dmitry Torokhov, Jonathan Corbet, Jiri Kosina, Benjamin Tissoires,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Casey Connolly, Guido Günther
Cc: linux-input, linux-doc, linux-kernel, linux-arm-msm, devicetree,
phone-devel, Gergo Koteles, Casey Connolly, David Heidelberg,
Konrad Dybcio
In-Reply-To: <20251113-op6-tri-state-v8-0-54073f3874bc@ixit.cz>
From: Gergo Koteles <soyer@irl.hu>
The alert-slider is a tri-state sound profile switch found on the
OnePlus 6, Android maps the states to "silent", "vibrate" and "ring".
Expose them as ABS_SND_PROFILE events.
The previous GPIO numbers were wrong. Update them to the correct ones.
Co-developed-by: Casey Connolly <casey@connolly.tech>
Signed-off-by: Casey Connolly <casey@connolly.tech>
Signed-off-by: Gergo Koteles <soyer@irl.hu>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Tested-by: Guido Günther <agx@sigxcpu.org> # oneplus,fajita & oneplus,enchilada
Reviewed-by: Guido Günther <agx@sigxcpu.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
index db6dd04c51bb5..963dc4cc3bbb8 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
@@ -25,6 +25,41 @@ / {
chassis-type = "handset";
qcom,msm-id = <QCOM_ID_SDM845 0x20001>;
+ alert-slider {
+ compatible = "gpio-keys";
+ label = "Alert slider";
+
+ pinctrl-0 = <&alert_slider_default>;
+ pinctrl-names = "default";
+
+ switch-top {
+ label = "Silent";
+ linux,input-type = <EV_ABS>;
+ linux,code = <ABS_SND_PROFILE>;
+ linux,input-value = <SND_PROFILE_SILENT>;
+ gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
+ linux,can-disable;
+ };
+
+ switch-middle {
+ label = "Vibrate";
+ linux,input-type = <EV_ABS>;
+ linux,code = <ABS_SND_PROFILE>;
+ linux,input-value = <SND_PROFILE_VIBRATE>;
+ gpios = <&tlmm 52 GPIO_ACTIVE_LOW>;
+ linux,can-disable;
+ };
+
+ switch-bottom {
+ label = "Ring";
+ linux,input-type = <EV_ABS>;
+ linux,code = <ABS_SND_PROFILE>;
+ linux,input-value = <SND_PROFILE_RING>;
+ gpios = <&tlmm 24 GPIO_ACTIVE_LOW>;
+ linux,can-disable;
+ };
+ };
+
aliases {
serial0 = &uart9;
serial1 = &uart6;
--
2.51.0
^ permalink raw reply related
* [PATCH v8 0/2] Add support for sound profile switching and leverage for OnePlus slider
From: David Heidelberg via B4 Relay @ 2025-11-13 16:02 UTC (permalink / raw)
To: Dmitry Torokhov, Jonathan Corbet, Jiri Kosina, Benjamin Tissoires,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Casey Connolly, Guido Günther
Cc: linux-input, linux-doc, linux-kernel, linux-arm-msm, devicetree,
phone-devel, Gergo Koteles, David Heidelberg, Casey Connolly,
Konrad Dybcio
This series add initial support for OnePlus 6 and 6T, but other OnePlus
phones contains same mechanism to switch sound profiles.
This code was tested for two years within the downstream Snapdragon 845 tree.
It is now perfectly integrated with feedbackd in the Phosh environment.
The series is also available (until merged) at
https://gitlab.com/sdm845/sdm845-next/-/commits/b4/op6-tri-state
Changes in v8:
- Rebased against next-20251113
- Added R-b, T-b trailers from v7.
- Link to v7: https://lore.kernel.org/r/20251014-op6-tri-state-v7-0-938a6367197b@ixit.cz
Changes in v7:
- Separated GPIO number fix from the original commit
"arm64: dts: qcom: sdm845-oneplus: Add alert-slider"
- Rebased again next-20251008
- Link to v6: https://lore.kernel.org/r/20250731-op6-tri-state-v6-0-569c25cbc8c2@ixit.cz
Changes in v6:
- Rebased again next-20250731, otherwise just a resent.
- Link to v5: https://lore.kernel.org/r/20250419-op6-tri-state-v5-0-443127078517@ixit.cz
Changes in v5:
- Dropped merged
"Input: gpio-keys - add support for linux,input-value DTS property"
- Link to v4: https://lore.kernel.org/all/cover.1677022414.git.soyer@irl.hu/
Changes in v4:
- DTS: use default debounce-interval, order alphabetically
- Link to v3: https://lore.kernel.org/lkml/cover.1676850819.git.soyer@irl.hu/
Changes in v3:
- rename tri-state-key to alert-slider, fix DTS warnings,
Changes in v2:
- rebase to qcom/for-next
add SND_PROFILE_* identifiers to input-event-codes.h
---
Gergo Koteles (2):
Input: add ABS_SND_PROFILE
arm64: dts: qcom: sdm845-oneplus: Add alert-slider
Documentation/input/event-codes.rst | 6 ++++
.../arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 35 ++++++++++++++++++++++
drivers/hid/hid-debug.c | 1 +
include/uapi/linux/input-event-codes.h | 9 ++++++
4 files changed, 51 insertions(+)
---
base-commit: 6d7e7251d03f98f26f2ee0dfd21bb0a0480a2178
change-id: 20250419-op6-tri-state-ed1a05a11125
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply
* [PATCH v8 1/2] Input: add ABS_SND_PROFILE
From: David Heidelberg via B4 Relay @ 2025-11-13 16:02 UTC (permalink / raw)
To: Dmitry Torokhov, Jonathan Corbet, Jiri Kosina, Benjamin Tissoires,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Casey Connolly, Guido Günther
Cc: linux-input, linux-doc, linux-kernel, linux-arm-msm, devicetree,
phone-devel, Gergo Koteles, David Heidelberg
In-Reply-To: <20251113-op6-tri-state-v8-0-54073f3874bc@ixit.cz>
From: Gergo Koteles <soyer@irl.hu>
ABS_SND_PROFILE used to describe the state of a multi-value sound profile
switch. This will be used for the alert-slider on OnePlus phones or other
phones.
Profile values added as SND_PROFLE_(SILENT|VIBRATE|RING) identifiers
to input-event-codes.h so they can be used from DTS.
Signed-off-by: Gergo Koteles <soyer@irl.hu>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Tested-by: Guido Günther <agx@sigxcpu.org> # oneplus,fajita & oneplus,enchilada
Reviewed-by: Guido Günther <agx@sigxcpu.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Documentation/input/event-codes.rst | 6 ++++++
drivers/hid/hid-debug.c | 1 +
include/uapi/linux/input-event-codes.h | 9 +++++++++
3 files changed, 16 insertions(+)
diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index 1ead9bb8d9c64..e4f065dd5a1da 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -241,6 +241,12 @@ A few EV_ABS codes have special meanings:
emitted only when the selected profile changes, indicating the newly
selected profile value.
+* ABS_SND_PROFILE:
+
+ - Used to describe the state of a multi-value sound profile switch.
+ An event is emitted only when the selected profile changes,
+ indicating the newly selected profile value.
+
* ABS_MT_<name>:
- Used to describe multitouch input events. Please see
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 337d2dc81b4ca..c5865b0d2aaaf 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -3513,6 +3513,7 @@ static const char *absolutes[ABS_CNT] = {
[ABS_DISTANCE] = "Distance", [ABS_TILT_X] = "XTilt",
[ABS_TILT_Y] = "YTilt", [ABS_TOOL_WIDTH] = "ToolWidth",
[ABS_VOLUME] = "Volume", [ABS_PROFILE] = "Profile",
+ [ABS_SND_PROFILE] = "SoundProfile",
[ABS_MISC] = "Misc",
[ABS_MT_SLOT] = "MTSlot",
[ABS_MT_TOUCH_MAJOR] = "MTMajor",
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 9cd89bcc1d9c0..430552f036616 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -891,6 +891,7 @@
#define ABS_VOLUME 0x20
#define ABS_PROFILE 0x21
+#define ABS_SND_PROFILE 0x22
#define ABS_MISC 0x28
@@ -1000,4 +1001,12 @@
#define SND_MAX 0x07
#define SND_CNT (SND_MAX+1)
+/*
+ * ABS_SND_PROFILE values
+ */
+
+#define SND_PROFILE_SILENT 0x00
+#define SND_PROFILE_VIBRATE 0x01
+#define SND_PROFILE_RING 0x02
+
#endif
--
2.51.0
^ permalink raw reply related
* [PATCH v2 5/5] Input: gpio_decoder - don't use "proxy" headers
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
In-Reply-To: <20251113154616.3107676-1-andriy.shevchenko@linux.intel.com>
Update header inclusions to follow IWYU (Include What You Use)
principle.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/misc/gpio_decoder.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index 057717de9849..f0759dd39b35 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -7,16 +7,17 @@
*/
#include <linux/bitmap.h>
-#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/device/devres.h>
#include <linux/err.h>
#include <linux/gpio/consumer.h>
#include <linux/input.h>
-#include <linux/kernel.h>
#include <linux/minmax.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/property.h>
+#include <linux/types.h>
struct gpio_decoder {
struct gpio_descs *input_gpios;
--
2.50.1
^ permalink raw reply related
* [PATCH v2 2/5] Input: gpio_decoder - unify messages with help of dev_err_probe()
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
In-Reply-To: <20251113154616.3107676-1-andriy.shevchenko@linux.intel.com>
Unify error messages that might appear during probe phase by
switching to use dev_err_probe().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/misc/gpio_decoder.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index 459abc749a49..a2ae400790f9 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -7,6 +7,7 @@
*/
#include <linux/device.h>
+#include <linux/err.h>
#include <linux/gpio/consumer.h>
#include <linux/input.h>
#include <linux/kernel.h>
@@ -73,15 +74,12 @@ static int gpio_decoder_probe(struct platform_device *pdev)
device_property_read_u32(dev, "linux,axis", &decoder->axis);
decoder->input_gpios = devm_gpiod_get_array(dev, NULL, GPIOD_IN);
- if (IS_ERR(decoder->input_gpios)) {
- dev_err(dev, "unable to acquire input gpios\n");
- return PTR_ERR(decoder->input_gpios);
- }
+ if (IS_ERR(decoder->input_gpios))
+ return dev_err_probe(dev, PTR_ERR(decoder->input_gpios),
+ "unable to acquire input gpios\n");
- if (decoder->input_gpios->ndescs < 2) {
- dev_err(dev, "not enough gpios found\n");
- return -EINVAL;
- }
+ if (decoder->input_gpios->ndescs < 2)
+ return dev_err_probe(dev, -EINVAL, "not enough gpios found\n");
if (device_property_read_u32(dev, "decoder-max-value", &max))
max = (1U << decoder->input_gpios->ndescs) - 1;
@@ -97,16 +95,12 @@ static int gpio_decoder_probe(struct platform_device *pdev)
input_set_abs_params(input, decoder->axis, 0, max, 0, 0);
err = input_setup_polling(input, gpio_decoder_poll_gpios);
- if (err) {
- dev_err(dev, "failed to set up polling\n");
- return err;
- }
+ if (err)
+ return dev_err_probe(dev, err, "failed to set up polling\n");
err = input_register_device(input);
- if (err) {
- dev_err(dev, "failed to register input device\n");
- return err;
- }
+ if (err)
+ return dev_err_probe(dev, err, "failed to register input device\n");
return 0;
}
--
2.50.1
^ permalink raw reply related
* [PATCH v2 4/5] Input: gpio_decoder - make use of the macros from bits.h
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
In-Reply-To: <20251113154616.3107676-1-andriy.shevchenko@linux.intel.com>
Make use of BIT() where it makes sense.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/misc/gpio_decoder.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index 7f319b932550..057717de9849 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -60,7 +60,7 @@ static int gpio_decoder_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct gpio_decoder *decoder;
struct input_dev *input;
- u32 max;
+ u32 max;
int err;
decoder = devm_kzalloc(dev, sizeof(*decoder), GFP_KERNEL);
@@ -82,7 +82,7 @@ static int gpio_decoder_probe(struct platform_device *pdev)
return dev_err_probe(dev, -EINVAL, "too many gpios found\n");
if (device_property_read_u32(dev, "decoder-max-value", &max))
- max = (1U << decoder->input_gpios->ndescs) - 1;
+ max = BIT(decoder->input_gpios->ndescs) - 1;
input = devm_input_allocate_device(dev);
if (!input)
--
2.50.1
^ permalink raw reply related
* [PATCH v2 3/5] Input: gpio_decoder - replace custom loop by gpiod_get_array_value_cansleep()
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
In-Reply-To: <20251113154616.3107676-1-andriy.shevchenko@linux.intel.com>
There is a custom loop that repeats parts of gpiod_get_array_value_cansleep().
Use that in conjunction with bitmap API to make code shorter and easier to
follow.
With this done, add an upper check for amount of GPIOs given based on
the driver's code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/misc/gpio_decoder.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index a2ae400790f9..7f319b932550 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -6,11 +6,13 @@
* encoded numeric value into an input event.
*/
+#include <linux/bitmap.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/gpio/consumer.h>
#include <linux/input.h>
#include <linux/kernel.h>
+#include <linux/minmax.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -26,23 +28,18 @@ struct gpio_decoder {
static int gpio_decoder_get_gpios_state(struct gpio_decoder *decoder)
{
struct gpio_descs *gpios = decoder->input_gpios;
- unsigned int ret = 0;
- int i, val;
+ DECLARE_BITMAP(values, 32);
+ unsigned int size;
+ int err;
- for (i = 0; i < gpios->ndescs; i++) {
- val = gpiod_get_value_cansleep(gpios->desc[i]);
- if (val < 0) {
- dev_err(decoder->dev,
- "Error reading gpio %d: %d\n",
- desc_to_gpio(gpios->desc[i]), val);
- return val;
- }
-
- val = !!val;
- ret = (ret << 1) | val;
+ size = min(gpios->ndescs, 32U);
+ err = gpiod_get_array_value_cansleep(size, gpios->desc, gpios->info, values);
+ if (err) {
+ dev_err(decoder->dev, "Error reading GPIO: %d\n", err);
+ return err;
}
- return ret;
+ return bitmap_read(values, 0, size);
}
static void gpio_decoder_poll_gpios(struct input_dev *input)
@@ -81,6 +78,9 @@ static int gpio_decoder_probe(struct platform_device *pdev)
if (decoder->input_gpios->ndescs < 2)
return dev_err_probe(dev, -EINVAL, "not enough gpios found\n");
+ if (decoder->input_gpios->ndescs > 31)
+ return dev_err_probe(dev, -EINVAL, "too many gpios found\n");
+
if (device_property_read_u32(dev, "decoder-max-value", &max))
max = (1U << decoder->input_gpios->ndescs) - 1;
--
2.50.1
^ permalink raw reply related
* [PATCH v2 1/5] Input: gpio_decoder - make use of device properties
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
In-Reply-To: <20251113154616.3107676-1-andriy.shevchenko@linux.intel.com>
Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/misc/gpio_decoder.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index ee668eba302f..459abc749a49 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -10,9 +10,10 @@
#include <linux/gpio/consumer.h>
#include <linux/input.h>
#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
-#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
struct gpio_decoder {
struct gpio_descs *input_gpios;
@@ -110,19 +111,17 @@ static int gpio_decoder_probe(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_OF
static const struct of_device_id gpio_decoder_of_match[] = {
{ .compatible = "gpio-decoder", },
- { },
+ { }
};
MODULE_DEVICE_TABLE(of, gpio_decoder_of_match);
-#endif
static struct platform_driver gpio_decoder_driver = {
.probe = gpio_decoder_probe,
.driver = {
.name = "gpio-decoder",
- .of_match_table = of_match_ptr(gpio_decoder_of_match),
+ .of_match_table = gpio_decoder_of_match,
}
};
module_platform_driver(gpio_decoder_driver);
--
2.50.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox