* Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation
From: Shawn Lin @ 2026-02-24 8:21 UTC (permalink / raw)
To: phasta
Cc: shawn.lin, Bjorn Helgaas, Vaibhaav Ram T . L,
Kumaravel Thiagarajan, Even Xu, Xinpeng Sun, Srinivas Pandruvada,
Jiri Kosina, Alexandre Belloni, Zhou Wang, Longfang Liu,
Vinod Koul, Lee Jones, Jijie Shao, Jian Shen, Sunil Goutham,
Andrew Lunn, Heiner Kallweit, David S . Miller, Jeff Hugo,
Oded Gabbay, Maciej Falkowski, Karol Wachowski, Min Ma, Lizhi Hou,
Andreas Noever, Mika Westerberg, Tomasz Jeznach, Will Deacon,
Xinliang Liu, Tian Tao, Davidlohr Bueso, Jonathan Cameron,
Srujana Challa, Bharat Bhushan, Antoine Tenart, Herbert Xu,
Raag Jadav, Hans de Goede, Greg Kroah-Hartman, Jiri Slaby,
Andy Shevchenko, Manivannan Sadhasivam, Mika Westerberg,
Andi Shyti, Robert Richter, Mark Brown, Nirmal Patel,
Kurt Schwemmer, Logan Gunthorpe, Linus Walleij,
Bartosz Golaszewski, Sakari Ailus, Bingbu Cao, Ulf Hansson,
Arnd Bergmann, Benjamin Tissoires, linux-input, linux-i3c,
dmaengine, netdev, nic_swsd, linux-arm-msm, dri-devel, linux-usb,
iommu, linux-riscv, David Airlie, Simona Vetter, linux-cxl,
linux-crypto, platform-driver-x86, linux-serial, mhi,
Andy Shevchenko, Jan Dabros, linux-i2c, Daniel Mack,
Haojian Zhuang, linux-spi, Jonathan Derrick, linux-pci,
linux-gpio, Mauro Carvalho Chehab, linux-media, linux-mmc,
Jakub Kicinski
In-Reply-To: <07fc896007d86b731cbfb3cf6bbdf4e5315d7a77.camel@mailbox.org>
在 2026/02/24 星期二 15:47, Philipp Stanner 写道:
> On Tue, 2026-02-24 at 10:08 +0800, Shawn Lin wrote:
>> 在 2026/02/24 星期二 8:04, Jakub Kicinski 写道:
>>> On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
>>>> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
>>>> pci device drivers which rely on the devres machinery to help cleanup the IRQ
>>>> vectors.
>>>
>>> If you can please add this API with just a few users, and then convert
>>> remaining users via the subsystem trees in the next cycle.
>>> There's no need to risk wasting maintainer time on conflicts with
>>> conversions like this.
>>
>> Thanks for the suggestion, Jakub. I have little experience with
>> cross-subsystem cleanups like this, so your suggestion is very helpful.
>
>
> When I removed the hybrid nature of pci_request_region() et al., I
> concluded that there were so few users that doing them all in one run
> was sufficient.
>
> For larger reworks, like removing pcim_iomap_table(), a slower step-by-
> step strategy is necessary for the reasons that Jakub details.
>
> It is then smart to omit an easy to port subsystem / driver for the
> ultimate patch series where one then removes the hybrid behavior from
> PCI itself, after porting the last driver.
>
> In general, as Jakub details, those step-by-step cleanups are a bit
> safer, since you can proof valid behavior early on and in case of an
> explosion they are very easy to revert.
>
Thank you, Philipp. I wish I had attended your talk at FOSDEM 2025 on
removing pcim_iomap_table earlier. This first version was perhaps a bit
too aggressive. For v2, I think the plan should start with addressing
the switchtec and vmd drivers, since both of those, along with the new
API additions, can be handled entirely within the PCI subsystem scope.
>
> P.
>
^ permalink raw reply
* Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation
From: Philipp Stanner @ 2026-02-24 8:32 UTC (permalink / raw)
To: Shawn Lin, phasta
Cc: Bjorn Helgaas, Vaibhaav Ram T . L, Kumaravel Thiagarajan, Even Xu,
Xinpeng Sun, Srinivas Pandruvada, Jiri Kosina, Alexandre Belloni,
Zhou Wang, Longfang Liu, Vinod Koul, Lee Jones, Jijie Shao,
Jian Shen, Sunil Goutham, Andrew Lunn, Heiner Kallweit,
David S . Miller, Jeff Hugo, Oded Gabbay, Maciej Falkowski,
Karol Wachowski, Min Ma, Lizhi Hou, Andreas Noever,
Mika Westerberg, Tomasz Jeznach, Will Deacon, Xinliang Liu,
Tian Tao, Davidlohr Bueso, Jonathan Cameron, Srujana Challa,
Bharat Bhushan, Antoine Tenart, Herbert Xu, Raag Jadav,
Hans de Goede, Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko,
Manivannan Sadhasivam, Mika Westerberg, Andi Shyti,
Robert Richter, Mark Brown, Nirmal Patel, Kurt Schwemmer,
Logan Gunthorpe, Linus Walleij, Bartosz Golaszewski, Sakari Ailus,
Bingbu Cao, Ulf Hansson, Arnd Bergmann, Benjamin Tissoires,
linux-input, linux-i3c, dmaengine, netdev, nic_swsd,
linux-arm-msm, dri-devel, linux-usb, iommu, linux-riscv,
David Airlie, Simona Vetter, linux-cxl, linux-crypto,
platform-driver-x86, linux-serial, mhi, Andy Shevchenko,
Jan Dabros, linux-i2c, Daniel Mack, Haojian Zhuang, linux-spi,
Jonathan Derrick, linux-pci, linux-gpio, Mauro Carvalho Chehab,
linux-media, linux-mmc, Jakub Kicinski
In-Reply-To: <d601ec05-ef38-5e8e-c643-c05010717ebe@rock-chips.com>
On Tue, 2026-02-24 at 16:21 +0800, Shawn Lin wrote:
> 在 2026/02/24 星期二 15:47, Philipp Stanner 写道:
> > On Tue, 2026-02-24 at 10:08 +0800, Shawn Lin wrote:
> > > 在 2026/02/24 星期二 8:04, Jakub Kicinski 写道:
> > > > On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
> > > > > pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> > > > > pci device drivers which rely on the devres machinery to help cleanup the IRQ
> > > > > vectors.
> > > >
> > > > If you can please add this API with just a few users, and then convert
> > > > remaining users via the subsystem trees in the next cycle.
> > > > There's no need to risk wasting maintainer time on conflicts with
> > > > conversions like this.
> > >
> > > Thanks for the suggestion, Jakub. I have little experience with
> > > cross-subsystem cleanups like this, so your suggestion is very helpful.
> >
> >
> > When I removed the hybrid nature of pci_request_region() et al., I
> > concluded that there were so few users that doing them all in one run
> > was sufficient.
> >
> > For larger reworks, like removing pcim_iomap_table(), a slower step-by-
> > step strategy is necessary for the reasons that Jakub details.
> >
> > It is then smart to omit an easy to port subsystem / driver for the
> > ultimate patch series where one then removes the hybrid behavior from
> > PCI itself, after porting the last driver.
> >
> > In general, as Jakub details, those step-by-step cleanups are a bit
> > safer, since you can proof valid behavior early on and in case of an
> > explosion they are very easy to revert.
> >
>
> Thank you, Philipp. I wish I had attended your talk at FOSDEM 2025 on
> removing pcim_iomap_table earlier. This first version was perhaps a bit
> too aggressive.
No worries at all, it's very cool that you pick this work up!
> For v2, I think the plan should start with addressing
> the switchtec and vmd drivers, since both of those, along with the new
> API additions, can be handled entirely within the PCI subsystem scope.
Sounds reasonable to me.
Regards
Philipp
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Jiri Kosina @ 2026-02-24 8:42 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Lee Jones, David Rheinsberg, linux-input, linux-kernel, stable
In-Reply-To: <172q4775-616s-p7s4-7n80-p8579n0r3516@xreary.bet>
On Sat, 21 Feb 2026, Jiri Kosina wrote:
> > > Since the report ID is located within the data buffer, overwriting it
> > > would mean that any subsequent matching could cause a disparity in
> > > assumed allocated buffer size. This in turn could trivially result in
> > > an out-of-bounds condition. To mitigate this issue, let's refuse to
> > > overwrite a given report's data area if the ID in get_report_reply
> > > doesn't match.
> >
> > That's a strong assumption and a breakage of the userspace FWIW. The CI
> > is now full of errors:
> > https://gitlab.freedesktop.org/bentiss/hid/-/commits/for-7.0/upstream-fixes
> >
> > It is pretty common to allocate the buffer and not initialize it in
> > get_report operations.
> >
> > It was a bad API choice to have rnum and data[0] for all HID requests
> > (internally, externally), but we should stick to it. The CI breakage in
> > itself is not a big issue TBH, but if it breaks here, it will probably
> > break existing users.
>
> Lee,
>
> was this found via code inspection, fuzzing, or is there some real-world
> report behind it?
For now I've dropped this from for-7.0/upstream-fixes until it's all
clarified.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] HID: apple: Add EPOMAKER TH87 to the non-apple keyboards list
From: Takashi Iwai @ 2026-02-24 9:00 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel
EPOMAKER TH87 has the very same ID as Apple Aluminum keyboard
(05ac:024f) although it doesn't work as expected in compatible way.
Put three entries to the non-apple keyboards list to exclude this
device: one for BT ("TH87"), one for USB ("HFD Epomaker TH87") and one
for dongle ("2.4G Wireless Receiver").
Link: https://bugzilla.suse.com/show_bug.cgi?id=1258455
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/hid/hid-apple.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index b949b767cf08..72d98a086647 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -365,6 +365,9 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
{ "A3R" },
{ "hfd.cn" },
{ "WKB603" },
+ { "TH87" }, /* EPOMAKER TH87 BT mode */
+ { "HFD Epomaker TH87" }, /* EPOMAKER TH87 USB mode */
+ { "2.4G Wireless Receiver" }, /* EPOMAKER TH87 dongle */
};
static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free
From: Andy Shevchenko @ 2026-02-24 9:12 UTC (permalink / raw)
To: phasta
Cc: Simon Richter, Shawn Lin, Bjorn Helgaas, Vaibhaav Ram T . L,
Kumaravel Thiagarajan, Even Xu, Xinpeng Sun, Srinivas Pandruvada,
Jiri Kosina, Alexandre Belloni, Zhou Wang, Longfang Liu,
Vinod Koul, Lee Jones, Jijie Shao, Jian Shen, Sunil Goutham,
Andrew Lunn, Heiner Kallweit, David S . Miller, Jeff Hugo,
Oded Gabbay, Maciej Falkowski, Karol Wachowski, Min Ma, Lizhi Hou,
Andreas Noever, Mika Westerberg, Tomasz Jeznach, Will Deacon,
Xinliang Liu, Tian Tao, Davidlohr Bueso, Jonathan Cameron,
Srujana Challa, Bharat Bhushan, Antoine Tenart, Herbert Xu,
Raag Jadav, Hans de Goede, Greg Kroah-Hartman, Jiri Slaby,
Andy Shevchenko, Manivannan Sadhasivam, Mika Westerberg,
Andi Shyti, Robert Richter, Mark Brown, Nirmal Patel,
Kurt Schwemmer, Logan Gunthorpe, Linus Walleij,
Bartosz Golaszewski, Sakari Ailus, Bingbu Cao, Ulf Hansson,
Arnd Bergmann, Benjamin Tissoires, linux-input, linux-i3c,
dmaengine, netdev, nic_swsd, linux-arm-msm, dri-devel, linux-usb,
iommu, linux-riscv, David Airlie, Simona Vetter, linux-cxl,
linux-crypto, platform-driver-x86, linux-serial, mhi, Jan Dabros,
linux-i2c, Daniel Mack, Haojian Zhuang, linux-spi,
Jonathan Derrick, linux-pci, linux-gpio, Mauro Carvalho Chehab,
linux-media, linux-mmc
In-Reply-To: <7ca512d133f7a3bcfe00e9b0b2af5fe5f147ad77.camel@mailbox.org>
On Tue, Feb 24, 2026 at 08:39:43AM +0100, Philipp Stanner wrote:
> On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:
> > On 2/24/26 12:29 AM, Shawn Lin wrote:
> > > When such a driver also uses `pcim_enable_device()`, the devres framework may
> > > attempt to free the IRQ vectors a second time upon device release, leading to
> > > a double-free. Analysis of the tree shows this hazardous pattern exists widely,
> > > while 35 other drivers correctly rely solely on the implicit cleanup.
> >
> > Would it make sense to have a function pcim_free_irq_vectors(), to allow
> > explicit freeing even if the device is otherwise managed, analogous to
> > pcim_iounmap()?
>
> We used to add those. In part because it is easier to port old users.
>
> Nowadays I tend to think that those APIs were more on the too-complex
> than too-simple side for a long time. As an expert or as the API
> designer you wouldn't expect it, but there are actually far too many
> users who came to believe they always have to use pcim_iounmap() and
> counter parts.
>
> If I could design it from scratch I would probably try to tell users to
> use the unmanaged versions instead of revoking the devres consequence.
+many.
> Devres is actually about your consequence always happening whenever the
> driver unloads, for whatever reason.
I believe you meant "unbinds". The device<-->driver link can be broken
without unloading the driver.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free
From: Philipp Stanner @ 2026-02-24 10:30 UTC (permalink / raw)
To: Andy Shevchenko, phasta
Cc: Simon Richter, Shawn Lin, Bjorn Helgaas, Vaibhaav Ram T . L,
Kumaravel Thiagarajan, Even Xu, Xinpeng Sun, Srinivas Pandruvada,
Jiri Kosina, Alexandre Belloni, Zhou Wang, Longfang Liu,
Vinod Koul, Lee Jones, Jijie Shao, Jian Shen, Sunil Goutham,
Andrew Lunn, Heiner Kallweit, David S . Miller, Jeff Hugo,
Oded Gabbay, Maciej Falkowski, Karol Wachowski, Min Ma, Lizhi Hou,
Andreas Noever, Mika Westerberg, Tomasz Jeznach, Will Deacon,
Xinliang Liu, Tian Tao, Davidlohr Bueso, Jonathan Cameron,
Srujana Challa, Bharat Bhushan, Antoine Tenart, Herbert Xu,
Raag Jadav, Hans de Goede, Greg Kroah-Hartman, Jiri Slaby,
Andy Shevchenko, Manivannan Sadhasivam, Mika Westerberg,
Andi Shyti, Robert Richter, Mark Brown, Nirmal Patel,
Kurt Schwemmer, Logan Gunthorpe, Linus Walleij,
Bartosz Golaszewski, Sakari Ailus, Bingbu Cao, Ulf Hansson,
Arnd Bergmann, Benjamin Tissoires, linux-input, linux-i3c,
dmaengine, netdev, nic_swsd, linux-arm-msm, dri-devel, linux-usb,
iommu, linux-riscv, David Airlie, Simona Vetter, linux-cxl,
linux-crypto, platform-driver-x86, linux-serial, mhi, Jan Dabros,
linux-i2c, Daniel Mack, Haojian Zhuang, linux-spi,
Jonathan Derrick, linux-pci, linux-gpio, Mauro Carvalho Chehab,
linux-media, linux-mmc
In-Reply-To: <aZ1rb8zoqmQmakDP@smile.fi.intel.com>
On Tue, 2026-02-24 at 11:12 +0200, Andy Shevchenko wrote:
> On Tue, Feb 24, 2026 at 08:39:43AM +0100, Philipp Stanner wrote:
> > On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:
> > > On 2/24/26 12:29 AM, Shawn Lin wrote:
>
> > > > When such a driver also uses `pcim_enable_device()`, the devres framework may
> > > > attempt to free the IRQ vectors a second time upon device release, leading to
> > > > a double-free. Analysis of the tree shows this hazardous pattern exists widely,
> > > > while 35 other drivers correctly rely solely on the implicit cleanup.
> > >
> > > Would it make sense to have a function pcim_free_irq_vectors(), to allow
> > > explicit freeing even if the device is otherwise managed, analogous to
> > > pcim_iounmap()?
> >
> > We used to add those. In part because it is easier to port old users.
> >
> > Nowadays I tend to think that those APIs were more on the too-complex
> > than too-simple side for a long time. As an expert or as the API
> > designer you wouldn't expect it, but there are actually far too many
> > users who came to believe they always have to use pcim_iounmap() and
> > counter parts.
> >
> > If I could design it from scratch I would probably try to tell users to
> > use the unmanaged versions instead of revoking the devres consequence.
>
> +many.
hm?
>
> > Devres is actually about your consequence always happening whenever the
> > driver unloads, for whatever reason.
>
> I believe you meant "unbinds". The device<-->driver link can be broken
> without unloading the driver.
Yes, thx for pointing that out. Greg KH AFAIK always calls it "driver
detach".
P.
^ permalink raw reply
* Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free
From: Andy Shevchenko @ 2026-02-24 10:39 UTC (permalink / raw)
To: phasta
Cc: Simon Richter, Shawn Lin, Bjorn Helgaas, Vaibhaav Ram T . L,
Kumaravel Thiagarajan, Even Xu, Xinpeng Sun, Srinivas Pandruvada,
Jiri Kosina, Alexandre Belloni, Zhou Wang, Longfang Liu,
Vinod Koul, Lee Jones, Jijie Shao, Jian Shen, Sunil Goutham,
Andrew Lunn, Heiner Kallweit, David S . Miller, Jeff Hugo,
Oded Gabbay, Maciej Falkowski, Karol Wachowski, Min Ma, Lizhi Hou,
Andreas Noever, Mika Westerberg, Tomasz Jeznach, Will Deacon,
Xinliang Liu, Tian Tao, Davidlohr Bueso, Jonathan Cameron,
Srujana Challa, Bharat Bhushan, Antoine Tenart, Herbert Xu,
Raag Jadav, Hans de Goede, Greg Kroah-Hartman, Jiri Slaby,
Andy Shevchenko, Manivannan Sadhasivam, Mika Westerberg,
Andi Shyti, Robert Richter, Mark Brown, Nirmal Patel,
Kurt Schwemmer, Logan Gunthorpe, Linus Walleij,
Bartosz Golaszewski, Sakari Ailus, Bingbu Cao, Ulf Hansson,
Arnd Bergmann, Benjamin Tissoires, linux-input, linux-i3c,
dmaengine, netdev, nic_swsd, linux-arm-msm, dri-devel, linux-usb,
iommu, linux-riscv, David Airlie, Simona Vetter, linux-cxl,
linux-crypto, platform-driver-x86, linux-serial, mhi, Jan Dabros,
linux-i2c, Daniel Mack, Haojian Zhuang, linux-spi,
Jonathan Derrick, linux-pci, linux-gpio, Mauro Carvalho Chehab,
linux-media, linux-mmc
In-Reply-To: <48297cc524736e7452def05448ece84260a4fd83.camel@mailbox.org>
On Tue, Feb 24, 2026 at 11:30:28AM +0100, Philipp Stanner wrote:
> On Tue, 2026-02-24 at 11:12 +0200, Andy Shevchenko wrote:
> > On Tue, Feb 24, 2026 at 08:39:43AM +0100, Philipp Stanner wrote:
> > > On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:
...
> > > If I could design it from scratch I would probably try to tell users to
> > > use the unmanaged versions instead of revoking the devres consequence.
> >
> > +many.
> hm?
I'm supporting you with many hands up (more than I possess)!
> > > Devres is actually about your consequence always happening whenever the
> > > driver unloads, for whatever reason.
> >
> > I believe you meant "unbinds". The device<-->driver link can be broken
> > without unloading the driver.
>
> Yes, thx for pointing that out. Greg KH AFAIK always calls it "driver
> detach".
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] ARM: dts: qcom: msm8960: expressatt: Add coreriver,tc360-touchkey
From: Konrad Dybcio @ 2026-02-24 11:28 UTC (permalink / raw)
To: Rudraksha Gupta, Dmitry Torokhov
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel, beomho.seo,
jcsing.lee, linux-input, nick.reitemeyer
In-Reply-To: <41b4f4c1-f0c3-4ce7-8267-039bb77ea953@gmail.com>
On 2/24/26 2:00 AM, Rudraksha Gupta wrote:
>
> On 2/22/26 19:54, Dmitry Torokhov wrote:
>> Hi Rudraksha,
>>
>> On Thu, Feb 19, 2026 at 08:33:43PM -0800, Rudraksha Gupta wrote:
>>> Hello all,
>>>
>>>
>>> Top posting for once (context below).
>>>
>>> Not too sure what the next steps are to get the tm2 touchkey in. Should I
>>> resend the patch, contact someone else that can help provide guidance, or
>>> something else?
>>>
>>>
>>> Adding Dmitry Torokhov (official maintainer) and Nick Reitemeyer (person who
>>> introduced this variant).
>> Sorry, I am not sure what the question is... It seems that you made the
>> driver work without any additional changes?
>
> I believe this patch is blocked on Konrad's comment:
>
>> This driver mentions a register called CYPRESS_MODULE_VER - maybe
> it could help confirm the model?
>
>
> This was in response to me saying that the "coreriver,tc360-touchkey" tm2 variant works as is on my device, but I can't tell for sure if this is actually the variant that is on my device. There isn't really any documentation for how this peripheral works and I was primarily relying on others in this thread to provide details to confirm that this is the actual variant being used.
>
> If I'm mistaken that this is a blocker, please let me know.
That was a suggestion - if we're not able to find out, then we're not
able to find out!
Konrad
^ permalink raw reply
* Re: [PATCH 1/9] dt-bindings: mfd: mt6397: Add bindings for MT6392 PMIC
From: Alexandre Belloni @ 2026-02-24 14:50 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Fabien Parent, Val Packett, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sen Chu,
Sean Wang, Macpaul Lin, Lee Jones, Matthias Brugger,
AngeloGioacchino Del Regno, Liam Girdwood, Mark Brown,
Eddie Huang, Gary Bisson, Julien Massot, Louis-Alexis Eyraud,
Chen Zhong, linux-input, devicetree, linux-kernel, linux-pm,
linux-arm-kernel, linux-rtc
In-Reply-To: <c5ce038359583c5318c2d5ee341287c213aef880.1771865015.git.l.scorcia@gmail.com>
On 23/02/2026 17:12:40+0000, Luca Leonardo Scorcia wrote:
> From: Fabien Parent <parent.f@gmail.com>
>
> Add the currently supported bindings for the MT6392 PMIC.
>
> Signed-off-by: Fabien Parent <parent.f@gmail.com>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> index 6a89b479d10f..5f422d311d4d 100644
> --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> @@ -39,6 +39,7 @@ properties:
> - mediatek,mt6328
> - mediatek,mt6358
> - mediatek,mt6359
> + - mediatek,mt6392
> - mediatek,mt6397
> - items:
> - enum:
> @@ -67,6 +68,7 @@ properties:
> - mediatek,mt6323-rtc
> - mediatek,mt6331-rtc
> - mediatek,mt6358-rtc
> + - mediatek,mt6392-rtc
Shouldn't you rather use "mediatek,mt6392-rtc", "mediatek,mt6397-rtc" as
the compatible so you don't have to change anything in the driver?
> - mediatek,mt6397-rtc
> - items:
> - enum:
> --
> 2.43.0
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Benjamin Tissoires @ 2026-02-24 15:57 UTC (permalink / raw)
To: Jiri Kosina
Cc: Lee Jones, David Rheinsberg, linux-input, linux-kernel, stable
In-Reply-To: <47ro00po-r74n-870q-q178-67s8rpsss12q@xreary.bet>
On Feb 24 2026, Jiri Kosina wrote:
> On Sat, 21 Feb 2026, Jiri Kosina wrote:
>
> > > > Since the report ID is located within the data buffer, overwriting it
> > > > would mean that any subsequent matching could cause a disparity in
> > > > assumed allocated buffer size. This in turn could trivially result in
> > > > an out-of-bounds condition. To mitigate this issue, let's refuse to
> > > > overwrite a given report's data area if the ID in get_report_reply
> > > > doesn't match.
> > >
> > > That's a strong assumption and a breakage of the userspace FWIW. The CI
> > > is now full of errors:
> > > https://gitlab.freedesktop.org/bentiss/hid/-/commits/for-7.0/upstream-fixes
> > >
> > > It is pretty common to allocate the buffer and not initialize it in
> > > get_report operations.
> > >
> > > It was a bad API choice to have rnum and data[0] for all HID requests
> > > (internally, externally), but we should stick to it. The CI breakage in
> > > itself is not a big issue TBH, but if it breaks here, it will probably
> > > break existing users.
> >
> > Lee,
> >
> > was this found via code inspection, fuzzing, or is there some real-world
> > report behind it?
>
> For now I've dropped this from for-7.0/upstream-fixes until it's all
> clarified.
>
> Thanks,
>
So I've debugged today the error I was seeing. First, my statement is
slightly exagerated, because we are talking here about a get_report
function, and Lee's patch checks for the return buffer, not the incoming
buffer.
So I had a small time where I was wondering if I was not wrong and
something was off in the test suite.
However, it seems the bug is caused by the PS3 emulation:
https://gitlab.freedesktop.org/libevdev/hid-tools/-/blob/master/hidtools/device/sony_gamepad.py?ref_type=heads#L256-257
During the initialization of the PS3 controller, hid-sony.c calls
several GET_REPORT on 0xF2 and 0xF5. Both of these feature reports are
hidden in the report descriptor (a few lines above in the
sony_gamepad.py file). However, the emulation returns `[0x01, 0x00,
0x18, 0x5E, 0x0F, 0x71, 0xA4, 0xBB]` when we request a 0xf5 report,
which seems to be bogus.
So I digged out the PS3 controller from a drawer, and looked at the
incoming data from it.
And it turns out that the controller reply that exact sequence when
requested about the 0xf5 feature.
So that means that the emulation is correct, and we can have devices
which report a different report number. Arguably this is wrong, but we
are in the peripheral world where every vendor does what it wants :(
Long story short: that patch is too intrusive as it makes assumption on
the behavior of the device. We need to understand where/if the bug was
spotted and fix the caller of hid_hw_raw_request, not the uhid
implementation.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Jiri Kosina @ 2026-02-24 16:12 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Lee Jones, David Rheinsberg, linux-input, linux-kernel, stable
In-Reply-To: <aZ3IKiL91Ya7_iIM@plouf>
On Tue, 24 Feb 2026, Benjamin Tissoires wrote:
> Long story short: that patch is too intrusive as it makes assumption on
> the behavior of the device. We need to understand where/if the bug was
> spotted and fix the caller of hid_hw_raw_request, not the uhid
> implementation.
Thanks a lot for the analysis, Benjamin!
I asked about that here:
https://lore.kernel.org/all/172q4775-616s-p7s4-7n80-p8579n0r3516@xreary.bet/
So let's wait for Lee to clarify. Until that, the patch stays out of the
branch.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation
From: Jonathan Cameron @ 2026-02-24 16:20 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, Vaibhaav Ram T . L, Kumaravel Thiagarajan, Even Xu,
Xinpeng Sun, Srinivas Pandruvada, Jiri Kosina, Alexandre Belloni,
Zhou Wang, Longfang Liu, Vinod Koul, Lee Jones, Jijie Shao,
Jian Shen, Sunil Goutham, Andrew Lunn, Heiner Kallweit,
David S . Miller, Jeff Hugo, Oded Gabbay, Maciej Falkowski,
Karol Wachowski, Min Ma, Lizhi Hou, Andreas Noever,
Mika Westerberg, Tomasz Jeznach, Will Deacon, Xinliang Liu,
Tian Tao, Davidlohr Bueso, Srujana Challa, Bharat Bhushan,
Antoine Tenart, Herbert Xu, Raag Jadav, Hans de Goede,
Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko,
Manivannan Sadhasivam, Mika Westerberg, Andi Shyti,
Robert Richter, Mark Brown, Nirmal Patel, Kurt Schwemmer,
Logan Gunthorpe, Linus Walleij, Bartosz Golaszewski, Sakari Ailus,
Bingbu Cao, Ulf Hansson, Arnd Bergmann, Benjamin Tissoires,
linux-input, linux-i3c, dmaengine, Philipp Stanner, netdev,
nic_swsd, linux-arm-msm, dri-devel, linux-usb, iommu, linux-riscv,
David Airlie, Simona Vetter, linux-cxl, linux-crypto,
platform-driver-x86, linux-serial, mhi, Andy Shevchenko,
Jan Dabros, linux-i2c, Daniel Mack, Haojian Zhuang, linux-spi,
Jonathan Derrick, linux-pci, linux-gpio, Mauro Carvalho Chehab,
linux-media, linux-mmc
In-Reply-To: <1771860581-82092-2-git-send-email-shawn.lin@rock-chips.com>
On Mon, 23 Feb 2026 23:29:40 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> pci device drivers which rely on the devres machinery to help cleanup the IRQ
> vectors.
It might be worth adding some details on why we need the is_msi_managed
flag in the first place vs just doing conventional devm_add_action_or_reset()
with pci_free_irq_vectors().
^ permalink raw reply
* Re: [PATCH] HID: sony: add support for Rock Band 2 instruments
From: Antheas Kapenekakis @ 2026-02-24 18:01 UTC (permalink / raw)
To: appsforartists
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Sanjay Govind, Rosalie Wanders, Vicki Pfau,
Nícolas F . R . A . Prado
In-Reply-To: <20260220165047.2844568-1-appsforartists@google.com>
Hi Brenton,
Let me give you a quirk review. Nice to hear from you again.
On Fri, 20 Feb 2026 at 17:50, <appsforartists@google.com> wrote:
>
> From: Brenton Simpson <appsforartists@google.com>
>
> Rock Band 2 for the Nintendo Wii and for the Sony PlayStation 3 use the
> same mapping as later games:
>
> Green: SOUTH (A)
> Red: EAST (B)
> Yellow: NORTH (Y)
> Blue: WEST (X)
> Orange/pedal: TL (L1)
> Solo flag: TL2 (L2)
> Tilt: TR (R1)
> Pad flag: THUMBL (L3)
> Instrument button: MODE (Steam/Xbox)
> Whammy bar: ABS_Z (Axis 4)
> Effects switch: Z (Axis 5)
>
> As documented at https://github.com/TheNathannator/PlasticBand/blob/main/Docs/.
The checkpatch hits this line. Remove this line and replace with see below.
>
> The guitar and drums both use the same mapping. Tested using the Wii
> versions of the instruments.
>
A link tag.
Link: https://github.com/TheNathannator/PlasticBand/blob/main/Docs/
> Signed-off-by: Brenton Simpson <appsforartists@google.com>
> ---
> drivers/hid/hid-ids.h | 8 +++++++-
> drivers/hid/hid-sony.c | 45 +++++++++++++++++++++++++++++-------------
> 2 files changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 3e299a30dcde..644d3c4df144 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -664,6 +664,10 @@
> #define USB_DEVICE_ID_UGCI_FLYING 0x0020
> #define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
>
> +#define USB_VENDOR_ID_HARMONIX 0x1bad
> +#define USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE 0x3010
> +#define USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE 0x3110
> +
> #define USB_VENDOR_ID_HP 0x03f0
> #define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
> #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
> @@ -1299,7 +1303,9 @@
>
> #define USB_VENDOR_ID_SONY_RHYTHM 0x12ba
> #define USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE 0x074b
> -#define USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE 0x0100
> +#define USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE 0x0100
> +#define USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE 0x0200
> +#define USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE 0x0210
Indentation on first glance seems incorrect. But this file seems to
follow the rule of vendor gets one tab, and others are aligned. And
your additions match that.
But other than that, it is true that it is a mess and everyone does
what they do.
> #define USB_VENDOR_ID_SINO_LITE 0x1345
> #define USB_DEVICE_ID_SINO_LITE_CONTROLLER 0x3008
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index a89af14e4acc..f6975f6ae882 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -62,9 +62,10 @@
> #define GH_GUITAR_CONTROLLER BIT(14)
> #define GHL_GUITAR_PS3WIIU BIT(15)
> #define GHL_GUITAR_PS4 BIT(16)
> -#define RB4_GUITAR_PS4_USB BIT(17)
> -#define RB4_GUITAR_PS4_BT BIT(18)
> -#define RB4_GUITAR_PS5 BIT(19)
> +#define RB2_INSTRUMENT BIT(17)
> +#define RB4_GUITAR_PS4_USB BIT(18)
> +#define RB4_GUITAR_PS4_BT BIT(19)
> +#define RB4_GUITAR_PS5 BIT(20)
I thought these were not formatted correctly. But they are. You added
RB2_INSTRUMENT above. I would suggest pulling it down and making it
BIT(20) instead. It minimizes the diff.
>
> #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT)
> #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT)
> @@ -422,12 +423,12 @@ static const unsigned int sixaxis_keymap[] = {
> [0x11] = BTN_MODE, /* PS */
> };
>
> -static const unsigned int rb4_absmap[] = {
> +static const unsigned int rb_absmap[] = {
> [0x30] = ABS_X,
> [0x31] = ABS_Y,
> };
>
> -static const unsigned int rb4_keymap[] = {
> +static const unsigned int rb_keymap[] = {
> [0x1] = BTN_WEST, /* Square */
> [0x2] = BTN_SOUTH, /* Cross */
> [0x3] = BTN_EAST, /* Circle */
> @@ -625,17 +626,17 @@ static int gh_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> return 0;
> }
>
> -static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> +static int rb_instrument_mapping(struct hid_device *hdev, struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> {
> if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
> unsigned int key = usage->hid & HID_USAGE;
>
> - if (key >= ARRAY_SIZE(rb4_keymap))
> + if (key >= ARRAY_SIZE(rb_keymap))
> return 0;
>
> - key = rb4_keymap[key];
> + key = rb_keymap[key];
> hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> return 1;
> } else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_GENDESK) {
> @@ -645,10 +646,10 @@ static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> if (usage->hid == HID_GD_HATSWITCH)
> return 0;
>
> - if (abs >= ARRAY_SIZE(rb4_absmap))
> + if (abs >= ARRAY_SIZE(rb_absmap))
> return 0;
>
> - abs = rb4_absmap[abs];
> + abs = rb_absmap[abs];
> hid_map_usage_clear(hi, usage, bit, max, EV_ABS, abs);
> return 1;
> }
> @@ -1101,11 +1102,14 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
> if (sc->quirks & GH_GUITAR_CONTROLLER)
> return gh_guitar_mapping(hdev, hi, field, usage, bit, max);
>
> + if (sc->quirks & RB2_INSTRUMENT)
> + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> +
> if (sc->quirks & (RB4_GUITAR_PS4_USB | RB4_GUITAR_PS4_BT))
> - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
>
> if (sc->quirks & RB4_GUITAR_PS5)
> - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
>
> /* Let hid-core decide for the others */
> return 0;
> @@ -2369,12 +2373,25 @@ static const struct hid_device_id sony_devices[] = {
> /* Guitar Hero PC Guitar Dongle */
> { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE),
> .driver_data = GH_GUITAR_CONTROLLER },
> - /* Guitar Hero PS3 World Tour Guitar Dongle */
> - { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE),
> + /* Guitar Hero World Tour PS3 Guitar Dongle */
> + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE),
> .driver_data = GH_GUITAR_CONTROLLER },
> /* Guitar Hero Live PS4 guitar dongles */
> { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE),
> .driver_data = GHL_GUITAR_PS4 | GH_GUITAR_CONTROLLER },
> + /* Rock Band 2 instruments
> + * Nintendo Wii instruments are included in `hid-sony` because `hid-nintendo`
> + * is for the newer Nintendo Switch, and the Wii instruments use the same
> + * protocol as their Sony PlayStation 3 cousins.
> + */
I would simplify it to "Rock Band 2 Instruments for Wii U. They use
the hid-sony protocol.". Readers do not need more context.
Other than that, I would drop all the renames from your patch. They
triple the diff and just make it harder to merge. Without those, the
patch is essentially the hunk below, the new PIDs, and the quirk. The
justification of the quirk is that you do not want your code to hit
rb4_ps4_guitar_parse_report(sc, rd, size); (?)
This patch is very reasonable to merge as it makes no code changes
after you clean it up a bit.
As for why the buttons are jumbled, this is standard HID gamepad
handling. You are not supposed to read them directly, but instead use
something like [1] or PR to [2]. If the gamepad does not have
vibration, there is no functionality loss either. But... this patch is
also very welcome.
Best,
Antheas
[1] https://github.com/mdqinc/SDL_GameControllerDB
[2] https://github.com/libsdl-org/SDL
> + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE),
> + .driver_data = RB2_INSTRUMENT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE),
> + .driver_data = RB2_INSTRUMENT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE),
> + .driver_data = RB2_INSTRUMENT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE),
> + .driver_data = RB2_INSTRUMENT },
> /* Rock Band 4 PS4 guitars */
> { HID_USB_DEVICE(USB_VENDOR_ID_PDP, USB_DEVICE_ID_PDP_PS4_RIFFMASTER),
> .driver_data = RB4_GUITAR_PS4_USB },
> --
> 2.53.0.414.gf7e9f6c205-goog
>
>
^ permalink raw reply
* Re: [PATCH] HID: sony: add support for Rock Band 2 instruments
From: Brenton Simpson @ 2026-02-24 18:47 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Sanjay Govind, Rosalie Wanders, Vicki Pfau,
Nícolas F . R . A . Prado
In-Reply-To: <CAGwozwHDGc=2hyUSQA7Awoa-3Gj=2BQ8JPqz0vEUPOpNiqY+RQ@mail.gmail.com>
Thanks Antheas!
Sanjay, Rosalie, and I got in touch off-thread. We'll be submitting a
patch that handles Rock Band instruments more holistically (not just
the Wii ones) shortly.
Preview here:
https://github.com/Rosalie241/hid-sony/tree/rb2-instruments
On Tue, Feb 24, 2026 at 1:01 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> Hi Brenton,
>
> Let me give you a quirk review. Nice to hear from you again.
>
> On Fri, 20 Feb 2026 at 17:50, <appsforartists@google.com> wrote:
> >
> > From: Brenton Simpson <appsforartists@google.com>
> >
> > Rock Band 2 for the Nintendo Wii and for the Sony PlayStation 3 use the
> > same mapping as later games:
> >
> > Green: SOUTH (A)
> > Red: EAST (B)
> > Yellow: NORTH (Y)
> > Blue: WEST (X)
> > Orange/pedal: TL (L1)
> > Solo flag: TL2 (L2)
> > Tilt: TR (R1)
> > Pad flag: THUMBL (L3)
> > Instrument button: MODE (Steam/Xbox)
> > Whammy bar: ABS_Z (Axis 4)
> > Effects switch: Z (Axis 5)
> >
> > As documented at https://github.com/TheNathannator/PlasticBand/blob/main/Docs/.
>
> The checkpatch hits this line. Remove this line and replace with see below.
>
> >
> > The guitar and drums both use the same mapping. Tested using the Wii
> > versions of the instruments.
> >
>
> A link tag.
>
> Link: https://github.com/TheNathannator/PlasticBand/blob/main/Docs/
> > Signed-off-by: Brenton Simpson <appsforartists@google.com>
> > ---
> > drivers/hid/hid-ids.h | 8 +++++++-
> > drivers/hid/hid-sony.c | 45 +++++++++++++++++++++++++++++-------------
> > 2 files changed, 38 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 3e299a30dcde..644d3c4df144 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -664,6 +664,10 @@
> > #define USB_DEVICE_ID_UGCI_FLYING 0x0020
> > #define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
> >
> > +#define USB_VENDOR_ID_HARMONIX 0x1bad
> > +#define USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE 0x3010
> > +#define USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE 0x3110
> > +
> > #define USB_VENDOR_ID_HP 0x03f0
> > #define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
> > #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
> > @@ -1299,7 +1303,9 @@
> >
> > #define USB_VENDOR_ID_SONY_RHYTHM 0x12ba
> > #define USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE 0x074b
> > -#define USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE 0x0100
> > +#define USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE 0x0100
> > +#define USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE 0x0200
> > +#define USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE 0x0210
>
> Indentation on first glance seems incorrect. But this file seems to
> follow the rule of vendor gets one tab, and others are aligned. And
> your additions match that.
>
> But other than that, it is true that it is a mess and everyone does
> what they do.
>
> > #define USB_VENDOR_ID_SINO_LITE 0x1345
> > #define USB_DEVICE_ID_SINO_LITE_CONTROLLER 0x3008
> > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> > index a89af14e4acc..f6975f6ae882 100644
> > --- a/drivers/hid/hid-sony.c
> > +++ b/drivers/hid/hid-sony.c
> > @@ -62,9 +62,10 @@
> > #define GH_GUITAR_CONTROLLER BIT(14)
> > #define GHL_GUITAR_PS3WIIU BIT(15)
> > #define GHL_GUITAR_PS4 BIT(16)
> > -#define RB4_GUITAR_PS4_USB BIT(17)
> > -#define RB4_GUITAR_PS4_BT BIT(18)
> > -#define RB4_GUITAR_PS5 BIT(19)
> > +#define RB2_INSTRUMENT BIT(17)
> > +#define RB4_GUITAR_PS4_USB BIT(18)
> > +#define RB4_GUITAR_PS4_BT BIT(19)
> > +#define RB4_GUITAR_PS5 BIT(20)
>
> I thought these were not formatted correctly. But they are. You added
> RB2_INSTRUMENT above. I would suggest pulling it down and making it
> BIT(20) instead. It minimizes the diff.
>
> >
> > #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT)
> > #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT)
> > @@ -422,12 +423,12 @@ static const unsigned int sixaxis_keymap[] = {
> > [0x11] = BTN_MODE, /* PS */
> > };
> >
> > -static const unsigned int rb4_absmap[] = {
> > +static const unsigned int rb_absmap[] = {
> > [0x30] = ABS_X,
> > [0x31] = ABS_Y,
> > };
> >
> > -static const unsigned int rb4_keymap[] = {
> > +static const unsigned int rb_keymap[] = {
> > [0x1] = BTN_WEST, /* Square */
> > [0x2] = BTN_SOUTH, /* Cross */
> > [0x3] = BTN_EAST, /* Circle */
> > @@ -625,17 +626,17 @@ static int gh_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > return 0;
> > }
> >
> > -static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > +static int rb_instrument_mapping(struct hid_device *hdev, struct hid_input *hi,
> > struct hid_field *field, struct hid_usage *usage,
> > unsigned long **bit, int *max)
> > {
> > if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
> > unsigned int key = usage->hid & HID_USAGE;
> >
> > - if (key >= ARRAY_SIZE(rb4_keymap))
> > + if (key >= ARRAY_SIZE(rb_keymap))
> > return 0;
> >
> > - key = rb4_keymap[key];
> > + key = rb_keymap[key];
> > hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> > return 1;
> > } else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_GENDESK) {
> > @@ -645,10 +646,10 @@ static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > if (usage->hid == HID_GD_HATSWITCH)
> > return 0;
> >
> > - if (abs >= ARRAY_SIZE(rb4_absmap))
> > + if (abs >= ARRAY_SIZE(rb_absmap))
> > return 0;
> >
> > - abs = rb4_absmap[abs];
> > + abs = rb_absmap[abs];
> > hid_map_usage_clear(hi, usage, bit, max, EV_ABS, abs);
> > return 1;
> > }
> > @@ -1101,11 +1102,14 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
> > if (sc->quirks & GH_GUITAR_CONTROLLER)
> > return gh_guitar_mapping(hdev, hi, field, usage, bit, max);
> >
> > + if (sc->quirks & RB2_INSTRUMENT)
> > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > +
> > if (sc->quirks & (RB4_GUITAR_PS4_USB | RB4_GUITAR_PS4_BT))
> > - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> >
> > if (sc->quirks & RB4_GUITAR_PS5)
> > - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> >
> > /* Let hid-core decide for the others */
> > return 0;
> > @@ -2369,12 +2373,25 @@ static const struct hid_device_id sony_devices[] = {
> > /* Guitar Hero PC Guitar Dongle */
> > { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE),
> > .driver_data = GH_GUITAR_CONTROLLER },
> > - /* Guitar Hero PS3 World Tour Guitar Dongle */
> > - { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE),
> > + /* Guitar Hero World Tour PS3 Guitar Dongle */
> > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE),
> > .driver_data = GH_GUITAR_CONTROLLER },
> > /* Guitar Hero Live PS4 guitar dongles */
> > { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE),
> > .driver_data = GHL_GUITAR_PS4 | GH_GUITAR_CONTROLLER },
> > + /* Rock Band 2 instruments
> > + * Nintendo Wii instruments are included in `hid-sony` because `hid-nintendo`
> > + * is for the newer Nintendo Switch, and the Wii instruments use the same
> > + * protocol as their Sony PlayStation 3 cousins.
> > + */
>
> I would simplify it to "Rock Band 2 Instruments for Wii U. They use
> the hid-sony protocol.". Readers do not need more context.
>
> Other than that, I would drop all the renames from your patch. They
> triple the diff and just make it harder to merge. Without those, the
> patch is essentially the hunk below, the new PIDs, and the quirk. The
> justification of the quirk is that you do not want your code to hit
> rb4_ps4_guitar_parse_report(sc, rd, size); (?)
>
> This patch is very reasonable to merge as it makes no code changes
> after you clean it up a bit.
>
> As for why the buttons are jumbled, this is standard HID gamepad
> handling. You are not supposed to read them directly, but instead use
> something like [1] or PR to [2]. If the gamepad does not have
> vibration, there is no functionality loss either. But... this patch is
> also very welcome.
>
> Best,
> Antheas
>
> [1] https://github.com/mdqinc/SDL_GameControllerDB
> [2] https://github.com/libsdl-org/SDL
>
> > + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE),
> > + .driver_data = RB2_INSTRUMENT },
> > + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE),
> > + .driver_data = RB2_INSTRUMENT },
> > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE),
> > + .driver_data = RB2_INSTRUMENT },
> > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE),
> > + .driver_data = RB2_INSTRUMENT },
> > /* Rock Band 4 PS4 guitars */
> > { HID_USB_DEVICE(USB_VENDOR_ID_PDP, USB_DEVICE_ID_PDP_PS4_RIFFMASTER),
> > .driver_data = RB4_GUITAR_PS4_USB },
> > --
> > 2.53.0.414.gf7e9f6c205-goog
> >
> >
>
^ permalink raw reply
* Re: [PATCH] HID: sony: add support for Rock Band 2 instruments
From: Antheas Kapenekakis @ 2026-02-24 19:19 UTC (permalink / raw)
To: Brenton Simpson
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Sanjay Govind, Rosalie Wanders, Vicki Pfau,
Nícolas F . R . A . Prado
In-Reply-To: <CAGwozwE+zUc9W00o89+CTpF3-XDmp6U+sNYBspBEcT6-A0Hr5g@mail.gmail.com>
On Tue, 24 Feb 2026 at 20:19, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Tue, 24 Feb 2026 at 19:48, Brenton Simpson <appsforartists@google.com> wrote:
> >
> > Thanks Antheas!
> >
> > Sanjay, Rosalie, and I got in touch off-thread. We'll be submitting a
> > patch that handles Rock Band instruments more holistically (not just
> > the Wii ones) shortly.
> >
> > Preview here:
> > https://github.com/Rosalie241/hid-sony/tree/rb2-instruments
>
> I reviewed that. If you want to merge quickly, I'd suggest less
> renames though. You are making changes there that are hard to review.
>
> Renames are typically a separate patch. If you truly believe in them
> and want to battle it out, that's where they'll go.
>
> I'd suggest you apply my suggestions to your patch anyhow and keep it
> separate. It is self contained and can merge quickly.
>
> I will leave it up to you if you want to have it as 1st patch of a
> series that Rosalie submits, or submit it on your own, get it merged
> to HID, then send the other patches on top of that remote.
>
> My recommendation would be the former.
*latter
>
> Best,
> Antheas
>
> > On Tue, Feb 24, 2026 at 1:01 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > >
> > > Hi Brenton,
> > >
> > > Let me give you a quirk review. Nice to hear from you again.
> > >
> > > On Fri, 20 Feb 2026 at 17:50, <appsforartists@google.com> wrote:
> > > >
> > > > From: Brenton Simpson <appsforartists@google.com>
> > > >
> > > > Rock Band 2 for the Nintendo Wii and for the Sony PlayStation 3 use the
> > > > same mapping as later games:
> > > >
> > > > Green: SOUTH (A)
> > > > Red: EAST (B)
> > > > Yellow: NORTH (Y)
> > > > Blue: WEST (X)
> > > > Orange/pedal: TL (L1)
> > > > Solo flag: TL2 (L2)
> > > > Tilt: TR (R1)
> > > > Pad flag: THUMBL (L3)
> > > > Instrument button: MODE (Steam/Xbox)
> > > > Whammy bar: ABS_Z (Axis 4)
> > > > Effects switch: Z (Axis 5)
> > > >
> > > > As documented at https://github.com/TheNathannator/PlasticBand/blob/main/Docs/.
> > >
> > > The checkpatch hits this line. Remove this line and replace with see below.
> > >
> > > >
> > > > The guitar and drums both use the same mapping. Tested using the Wii
> > > > versions of the instruments.
> > > >
> > >
> > > A link tag.
> > >
> > > Link: https://github.com/TheNathannator/PlasticBand/blob/main/Docs/
> > > > Signed-off-by: Brenton Simpson <appsforartists@google.com>
> > > > ---
> > > > drivers/hid/hid-ids.h | 8 +++++++-
> > > > drivers/hid/hid-sony.c | 45 +++++++++++++++++++++++++++++-------------
> > > > 2 files changed, 38 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > > index 3e299a30dcde..644d3c4df144 100644
> > > > --- a/drivers/hid/hid-ids.h
> > > > +++ b/drivers/hid/hid-ids.h
> > > > @@ -664,6 +664,10 @@
> > > > #define USB_DEVICE_ID_UGCI_FLYING 0x0020
> > > > #define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
> > > >
> > > > +#define USB_VENDOR_ID_HARMONIX 0x1bad
> > > > +#define USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE 0x3010
> > > > +#define USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE 0x3110
> > > > +
> > > > #define USB_VENDOR_ID_HP 0x03f0
> > > > #define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
> > > > #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
> > > > @@ -1299,7 +1303,9 @@
> > > >
> > > > #define USB_VENDOR_ID_SONY_RHYTHM 0x12ba
> > > > #define USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE 0x074b
> > > > -#define USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE 0x0100
> > > > +#define USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE 0x0100
> > > > +#define USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE 0x0200
> > > > +#define USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE 0x0210
> > >
> > > Indentation on first glance seems incorrect. But this file seems to
> > > follow the rule of vendor gets one tab, and others are aligned. And
> > > your additions match that.
> > >
> > > But other than that, it is true that it is a mess and everyone does
> > > what they do.
> > >
> > > > #define USB_VENDOR_ID_SINO_LITE 0x1345
> > > > #define USB_DEVICE_ID_SINO_LITE_CONTROLLER 0x3008
> > > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> > > > index a89af14e4acc..f6975f6ae882 100644
> > > > --- a/drivers/hid/hid-sony.c
> > > > +++ b/drivers/hid/hid-sony.c
> > > > @@ -62,9 +62,10 @@
> > > > #define GH_GUITAR_CONTROLLER BIT(14)
> > > > #define GHL_GUITAR_PS3WIIU BIT(15)
> > > > #define GHL_GUITAR_PS4 BIT(16)
> > > > -#define RB4_GUITAR_PS4_USB BIT(17)
> > > > -#define RB4_GUITAR_PS4_BT BIT(18)
> > > > -#define RB4_GUITAR_PS5 BIT(19)
> > > > +#define RB2_INSTRUMENT BIT(17)
> > > > +#define RB4_GUITAR_PS4_USB BIT(18)
> > > > +#define RB4_GUITAR_PS4_BT BIT(19)
> > > > +#define RB4_GUITAR_PS5 BIT(20)
> > >
> > > I thought these were not formatted correctly. But they are. You added
> > > RB2_INSTRUMENT above. I would suggest pulling it down and making it
> > > BIT(20) instead. It minimizes the diff.
> > >
> > > >
> > > > #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT)
> > > > #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT)
> > > > @@ -422,12 +423,12 @@ static const unsigned int sixaxis_keymap[] = {
> > > > [0x11] = BTN_MODE, /* PS */
> > > > };
> > > >
> > > > -static const unsigned int rb4_absmap[] = {
> > > > +static const unsigned int rb_absmap[] = {
> > > > [0x30] = ABS_X,
> > > > [0x31] = ABS_Y,
> > > > };
> > > >
> > > > -static const unsigned int rb4_keymap[] = {
> > > > +static const unsigned int rb_keymap[] = {
> > > > [0x1] = BTN_WEST, /* Square */
> > > > [0x2] = BTN_SOUTH, /* Cross */
> > > > [0x3] = BTN_EAST, /* Circle */
> > > > @@ -625,17 +626,17 @@ static int gh_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > > return 0;
> > > > }
> > > >
> > > > -static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > > +static int rb_instrument_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > > struct hid_field *field, struct hid_usage *usage,
> > > > unsigned long **bit, int *max)
> > > > {
> > > > if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
> > > > unsigned int key = usage->hid & HID_USAGE;
> > > >
> > > > - if (key >= ARRAY_SIZE(rb4_keymap))
> > > > + if (key >= ARRAY_SIZE(rb_keymap))
> > > > return 0;
> > > >
> > > > - key = rb4_keymap[key];
> > > > + key = rb_keymap[key];
> > > > hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> > > > return 1;
> > > > } else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_GENDESK) {
> > > > @@ -645,10 +646,10 @@ static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > > if (usage->hid == HID_GD_HATSWITCH)
> > > > return 0;
> > > >
> > > > - if (abs >= ARRAY_SIZE(rb4_absmap))
> > > > + if (abs >= ARRAY_SIZE(rb_absmap))
> > > > return 0;
> > > >
> > > > - abs = rb4_absmap[abs];
> > > > + abs = rb_absmap[abs];
> > > > hid_map_usage_clear(hi, usage, bit, max, EV_ABS, abs);
> > > > return 1;
> > > > }
> > > > @@ -1101,11 +1102,14 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > > if (sc->quirks & GH_GUITAR_CONTROLLER)
> > > > return gh_guitar_mapping(hdev, hi, field, usage, bit, max);
> > > >
> > > > + if (sc->quirks & RB2_INSTRUMENT)
> > > > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > > > +
> > > > if (sc->quirks & (RB4_GUITAR_PS4_USB | RB4_GUITAR_PS4_BT))
> > > > - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> > > > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > > >
> > > > if (sc->quirks & RB4_GUITAR_PS5)
> > > > - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> > > > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > > >
> > > > /* Let hid-core decide for the others */
> > > > return 0;
> > > > @@ -2369,12 +2373,25 @@ static const struct hid_device_id sony_devices[] = {
> > > > /* Guitar Hero PC Guitar Dongle */
> > > > { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE),
> > > > .driver_data = GH_GUITAR_CONTROLLER },
> > > > - /* Guitar Hero PS3 World Tour Guitar Dongle */
> > > > - { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE),
> > > > + /* Guitar Hero World Tour PS3 Guitar Dongle */
> > > > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE),
> > > > .driver_data = GH_GUITAR_CONTROLLER },
> > > > /* Guitar Hero Live PS4 guitar dongles */
> > > > { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE),
> > > > .driver_data = GHL_GUITAR_PS4 | GH_GUITAR_CONTROLLER },
> > > > + /* Rock Band 2 instruments
> > > > + * Nintendo Wii instruments are included in `hid-sony` because `hid-nintendo`
> > > > + * is for the newer Nintendo Switch, and the Wii instruments use the same
> > > > + * protocol as their Sony PlayStation 3 cousins.
> > > > + */
> > >
> > > I would simplify it to "Rock Band 2 Instruments for Wii U. They use
> > > the hid-sony protocol.". Readers do not need more context.
> > >
> > > Other than that, I would drop all the renames from your patch. They
> > > triple the diff and just make it harder to merge. Without those, the
> > > patch is essentially the hunk below, the new PIDs, and the quirk. The
> > > justification of the quirk is that you do not want your code to hit
> > > rb4_ps4_guitar_parse_report(sc, rd, size); (?)
> > >
> > > This patch is very reasonable to merge as it makes no code changes
> > > after you clean it up a bit.
> > >
> > > As for why the buttons are jumbled, this is standard HID gamepad
> > > handling. You are not supposed to read them directly, but instead use
> > > something like [1] or PR to [2]. If the gamepad does not have
> > > vibration, there is no functionality loss either. But... this patch is
> > > also very welcome.
> > >
> > > Best,
> > > Antheas
> > >
> > > [1] https://github.com/mdqinc/SDL_GameControllerDB
> > > [2] https://github.com/libsdl-org/SDL
> > >
> > > > + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE),
> > > > + .driver_data = RB2_INSTRUMENT },
> > > > + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE),
> > > > + .driver_data = RB2_INSTRUMENT },
> > > > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE),
> > > > + .driver_data = RB2_INSTRUMENT },
> > > > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE),
> > > > + .driver_data = RB2_INSTRUMENT },
> > > > /* Rock Band 4 PS4 guitars */
> > > > { HID_USB_DEVICE(USB_VENDOR_ID_PDP, USB_DEVICE_ID_PDP_PS4_RIFFMASTER),
> > > > .driver_data = RB4_GUITAR_PS4_USB },
> > > > --
> > > > 2.53.0.414.gf7e9f6c205-goog
> > > >
> > > >
> > >
> >
^ permalink raw reply
* Re: [PATCH] HID: sony: add support for Rock Band 2 instruments
From: Antheas Kapenekakis @ 2026-02-24 19:19 UTC (permalink / raw)
To: Brenton Simpson
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Sanjay Govind, Rosalie Wanders, Vicki Pfau,
Nícolas F . R . A . Prado
In-Reply-To: <CAAL3-=-0e6PqNyR64oCrBcA+kGwAfBVMoFc9zOpUVMoeXo_Z-A@mail.gmail.com>
On Tue, 24 Feb 2026 at 19:48, Brenton Simpson <appsforartists@google.com> wrote:
>
> Thanks Antheas!
>
> Sanjay, Rosalie, and I got in touch off-thread. We'll be submitting a
> patch that handles Rock Band instruments more holistically (not just
> the Wii ones) shortly.
>
> Preview here:
> https://github.com/Rosalie241/hid-sony/tree/rb2-instruments
I reviewed that. If you want to merge quickly, I'd suggest less
renames though. You are making changes there that are hard to review.
Renames are typically a separate patch. If you truly believe in them
and want to battle it out, that's where they'll go.
I'd suggest you apply my suggestions to your patch anyhow and keep it
separate. It is self contained and can merge quickly.
I will leave it up to you if you want to have it as 1st patch of a
series that Rosalie submits, or submit it on your own, get it merged
to HID, then send the other patches on top of that remote.
My recommendation would be the former.
Best,
Antheas
> On Tue, Feb 24, 2026 at 1:01 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > Hi Brenton,
> >
> > Let me give you a quirk review. Nice to hear from you again.
> >
> > On Fri, 20 Feb 2026 at 17:50, <appsforartists@google.com> wrote:
> > >
> > > From: Brenton Simpson <appsforartists@google.com>
> > >
> > > Rock Band 2 for the Nintendo Wii and for the Sony PlayStation 3 use the
> > > same mapping as later games:
> > >
> > > Green: SOUTH (A)
> > > Red: EAST (B)
> > > Yellow: NORTH (Y)
> > > Blue: WEST (X)
> > > Orange/pedal: TL (L1)
> > > Solo flag: TL2 (L2)
> > > Tilt: TR (R1)
> > > Pad flag: THUMBL (L3)
> > > Instrument button: MODE (Steam/Xbox)
> > > Whammy bar: ABS_Z (Axis 4)
> > > Effects switch: Z (Axis 5)
> > >
> > > As documented at https://github.com/TheNathannator/PlasticBand/blob/main/Docs/.
> >
> > The checkpatch hits this line. Remove this line and replace with see below.
> >
> > >
> > > The guitar and drums both use the same mapping. Tested using the Wii
> > > versions of the instruments.
> > >
> >
> > A link tag.
> >
> > Link: https://github.com/TheNathannator/PlasticBand/blob/main/Docs/
> > > Signed-off-by: Brenton Simpson <appsforartists@google.com>
> > > ---
> > > drivers/hid/hid-ids.h | 8 +++++++-
> > > drivers/hid/hid-sony.c | 45 +++++++++++++++++++++++++++++-------------
> > > 2 files changed, 38 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > index 3e299a30dcde..644d3c4df144 100644
> > > --- a/drivers/hid/hid-ids.h
> > > +++ b/drivers/hid/hid-ids.h
> > > @@ -664,6 +664,10 @@
> > > #define USB_DEVICE_ID_UGCI_FLYING 0x0020
> > > #define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
> > >
> > > +#define USB_VENDOR_ID_HARMONIX 0x1bad
> > > +#define USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE 0x3010
> > > +#define USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE 0x3110
> > > +
> > > #define USB_VENDOR_ID_HP 0x03f0
> > > #define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
> > > #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
> > > @@ -1299,7 +1303,9 @@
> > >
> > > #define USB_VENDOR_ID_SONY_RHYTHM 0x12ba
> > > #define USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE 0x074b
> > > -#define USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE 0x0100
> > > +#define USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE 0x0100
> > > +#define USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE 0x0200
> > > +#define USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE 0x0210
> >
> > Indentation on first glance seems incorrect. But this file seems to
> > follow the rule of vendor gets one tab, and others are aligned. And
> > your additions match that.
> >
> > But other than that, it is true that it is a mess and everyone does
> > what they do.
> >
> > > #define USB_VENDOR_ID_SINO_LITE 0x1345
> > > #define USB_DEVICE_ID_SINO_LITE_CONTROLLER 0x3008
> > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> > > index a89af14e4acc..f6975f6ae882 100644
> > > --- a/drivers/hid/hid-sony.c
> > > +++ b/drivers/hid/hid-sony.c
> > > @@ -62,9 +62,10 @@
> > > #define GH_GUITAR_CONTROLLER BIT(14)
> > > #define GHL_GUITAR_PS3WIIU BIT(15)
> > > #define GHL_GUITAR_PS4 BIT(16)
> > > -#define RB4_GUITAR_PS4_USB BIT(17)
> > > -#define RB4_GUITAR_PS4_BT BIT(18)
> > > -#define RB4_GUITAR_PS5 BIT(19)
> > > +#define RB2_INSTRUMENT BIT(17)
> > > +#define RB4_GUITAR_PS4_USB BIT(18)
> > > +#define RB4_GUITAR_PS4_BT BIT(19)
> > > +#define RB4_GUITAR_PS5 BIT(20)
> >
> > I thought these were not formatted correctly. But they are. You added
> > RB2_INSTRUMENT above. I would suggest pulling it down and making it
> > BIT(20) instead. It minimizes the diff.
> >
> > >
> > > #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT)
> > > #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT)
> > > @@ -422,12 +423,12 @@ static const unsigned int sixaxis_keymap[] = {
> > > [0x11] = BTN_MODE, /* PS */
> > > };
> > >
> > > -static const unsigned int rb4_absmap[] = {
> > > +static const unsigned int rb_absmap[] = {
> > > [0x30] = ABS_X,
> > > [0x31] = ABS_Y,
> > > };
> > >
> > > -static const unsigned int rb4_keymap[] = {
> > > +static const unsigned int rb_keymap[] = {
> > > [0x1] = BTN_WEST, /* Square */
> > > [0x2] = BTN_SOUTH, /* Cross */
> > > [0x3] = BTN_EAST, /* Circle */
> > > @@ -625,17 +626,17 @@ static int gh_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > return 0;
> > > }
> > >
> > > -static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > +static int rb_instrument_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > struct hid_field *field, struct hid_usage *usage,
> > > unsigned long **bit, int *max)
> > > {
> > > if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
> > > unsigned int key = usage->hid & HID_USAGE;
> > >
> > > - if (key >= ARRAY_SIZE(rb4_keymap))
> > > + if (key >= ARRAY_SIZE(rb_keymap))
> > > return 0;
> > >
> > > - key = rb4_keymap[key];
> > > + key = rb_keymap[key];
> > > hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> > > return 1;
> > > } else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_GENDESK) {
> > > @@ -645,10 +646,10 @@ static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > if (usage->hid == HID_GD_HATSWITCH)
> > > return 0;
> > >
> > > - if (abs >= ARRAY_SIZE(rb4_absmap))
> > > + if (abs >= ARRAY_SIZE(rb_absmap))
> > > return 0;
> > >
> > > - abs = rb4_absmap[abs];
> > > + abs = rb_absmap[abs];
> > > hid_map_usage_clear(hi, usage, bit, max, EV_ABS, abs);
> > > return 1;
> > > }
> > > @@ -1101,11 +1102,14 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
> > > if (sc->quirks & GH_GUITAR_CONTROLLER)
> > > return gh_guitar_mapping(hdev, hi, field, usage, bit, max);
> > >
> > > + if (sc->quirks & RB2_INSTRUMENT)
> > > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > > +
> > > if (sc->quirks & (RB4_GUITAR_PS4_USB | RB4_GUITAR_PS4_BT))
> > > - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> > > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > >
> > > if (sc->quirks & RB4_GUITAR_PS5)
> > > - return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
> > > + return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
> > >
> > > /* Let hid-core decide for the others */
> > > return 0;
> > > @@ -2369,12 +2373,25 @@ static const struct hid_device_id sony_devices[] = {
> > > /* Guitar Hero PC Guitar Dongle */
> > > { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE),
> > > .driver_data = GH_GUITAR_CONTROLLER },
> > > - /* Guitar Hero PS3 World Tour Guitar Dongle */
> > > - { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE),
> > > + /* Guitar Hero World Tour PS3 Guitar Dongle */
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE),
> > > .driver_data = GH_GUITAR_CONTROLLER },
> > > /* Guitar Hero Live PS4 guitar dongles */
> > > { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE),
> > > .driver_data = GHL_GUITAR_PS4 | GH_GUITAR_CONTROLLER },
> > > + /* Rock Band 2 instruments
> > > + * Nintendo Wii instruments are included in `hid-sony` because `hid-nintendo`
> > > + * is for the newer Nintendo Switch, and the Wii instruments use the same
> > > + * protocol as their Sony PlayStation 3 cousins.
> > > + */
> >
> > I would simplify it to "Rock Band 2 Instruments for Wii U. They use
> > the hid-sony protocol.". Readers do not need more context.
> >
> > Other than that, I would drop all the renames from your patch. They
> > triple the diff and just make it harder to merge. Without those, the
> > patch is essentially the hunk below, the new PIDs, and the quirk. The
> > justification of the quirk is that you do not want your code to hit
> > rb4_ps4_guitar_parse_report(sc, rd, size); (?)
> >
> > This patch is very reasonable to merge as it makes no code changes
> > after you clean it up a bit.
> >
> > As for why the buttons are jumbled, this is standard HID gamepad
> > handling. You are not supposed to read them directly, but instead use
> > something like [1] or PR to [2]. If the gamepad does not have
> > vibration, there is no functionality loss either. But... this patch is
> > also very welcome.
> >
> > Best,
> > Antheas
> >
> > [1] https://github.com/mdqinc/SDL_GameControllerDB
> > [2] https://github.com/libsdl-org/SDL
> >
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE),
> > > + .driver_data = RB2_INSTRUMENT },
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE),
> > > + .driver_data = RB2_INSTRUMENT },
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE),
> > > + .driver_data = RB2_INSTRUMENT },
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE),
> > > + .driver_data = RB2_INSTRUMENT },
> > > /* Rock Band 4 PS4 guitars */
> > > { HID_USB_DEVICE(USB_VENDOR_ID_PDP, USB_DEVICE_ID_PDP_PS4_RIFFMASTER),
> > > .driver_data = RB4_GUITAR_PS4_USB },
> > > --
> > > 2.53.0.414.gf7e9f6c205-goog
> > >
> > >
> >
>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-02-24 20:40 UTC (permalink / raw)
To: Rob Herring
Cc: hvilleneuve, dmitry.torokhov, krzk+dt, conor+dt, linux-input,
devicetree, linux-kernel
In-Reply-To: <CAL_JsqJNASirEqqcT-Sv8h9JC74e+XJSRsAki1ZWeKY8j2zbfw@mail.gmail.com>
Hi Rob,
On Mon, 23 Feb 2026 17:23:33 -0600
Rob Herring <robh@kernel.org> wrote:
> On Mon, Feb 23, 2026 at 12:47 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > Hi Rob,
> >
> > On Mon, 23 Feb 2026 11:57:06 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >
> > > On Fri, Feb 13, 2026 at 12:14:25PM -0500, Hugo Villeneuve wrote:
> > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > >
> > > > Add DT bindings for GPIO charlieplex keypad.
> > > >
> > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > ---
> > > > .../input/gpio-charlieplex-keypad.yaml | 82 +++++++++++++++++++
> > > > 1 file changed, 82 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > new file mode 100644
> > > > index 0000000000000..1672491a75a85
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > @@ -0,0 +1,82 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +
> > > > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: GPIO charlieplex keypad
> > > > +
> > > > +maintainers:
> > > > + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > +
> > > > +description:
> > > > + The charlieplex keypad supports N^2)-N different key combinations (where N is
> > > > + the number of lines). Key presses and releases are detected by configuring
> > > > + only one line as output at a time, and reading other line states. This process
> > > > + is repeated for each line.
> > > > + This mechanism doesn't allow to detect simultaneous key presses.
> > > > +
> > > > +allOf:
> > > > + - $ref: input.yaml#
> > > > + - $ref: /schemas/input/matrix-keymap.yaml#
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: gpio-charlieplex-keypad
> > > > +
> > > > + autorepeat: true
> > > > +
> > > > + line-scan-delay-us:
> > > > + description:
> > > > + Delay, measured in microseconds, that is needed
> > > > + before we can scan keypad after activating one line.
> > > > + default: 0
> > >
> > > Isn't this the same as "col-scan-delay-us" in gpio-matrix-keypad.yaml?
> > > If so, move it to matrix-keymap.yaml to re-use it here.
> >
> > It is used in a similar fashion, but for charlieplex keyboard, there is
> > no concept of "rows" and "columns". There are only
> > lines, which are all equivalent in functionality.
> >
> > > If not, there's a bunch of other scan delay properties just from
> > > grepping "delay" in the input bindings. Surely we can define something
> > > common.
> >
> > Most of those delays refer to something quite different than what
> > "col-scan-delay-us" or "line-scan-delay-us" are used for (it is a delay
> > that we wait when activating a GPIO before we can safely/reliably read
> > other GPIOs connected thru its circuitry).
> >
> > Maybe "col-scan-delay-us" and "line-scan-delay-us" could be
> > combined into a common "line-scan-delay-us" ("line" is more generic
> > than column), and defined in matrix-keymap.yaml.
>
> What about "scan-delay-us"? I would assume all the scan delay
> properties are just the delay after changing the outputs to reading
> the inputs.
They are for gpio-matrix-keypad.yaml and this binding, but not for
others. Most scan delay properties refer to the period or
interval between successive scans.
So for my binding, "settling-time-us" would be more accurate and a
better property name (it is also used in adc.yaml).
Looking into a common place to define this new property, I stumbled
upon gpio-delay.yaml, so maybe I do not need this new property at all
and simply define a gpio-delay node if needed (and add it to this
binding example)?
I tested this and it works, although it requires a patch to the
gpio-aggregator driver, because for now it respect the delay only
when changing the output value, not when switching between input and
output like I do in my driver.
With my patch, it works ok.
> > Then would it be ok to remove "col-scan-delay-us" from
> > gpio-matrix-keypad.yaml and use "line-scan-delay-us" (ABI change) ?
>
> No!
>
> Rob
--
Hugo Villeneuve
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: add GPIO charlieplex keypad
From: Rob Herring @ 2026-02-24 21:06 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: hvilleneuve, dmitry.torokhov, krzk+dt, conor+dt, linux-input,
devicetree, linux-kernel
In-Reply-To: <20260224154027.0f81b1aa13fe779776e6d58f@hugovil.com>
On Tue, Feb 24, 2026 at 2:40 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> Hi Rob,
>
> On Mon, 23 Feb 2026 17:23:33 -0600
> Rob Herring <robh@kernel.org> wrote:
>
> > On Mon, Feb 23, 2026 at 12:47 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Mon, 23 Feb 2026 11:57:06 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >
> > > > On Fri, Feb 13, 2026 at 12:14:25PM -0500, Hugo Villeneuve wrote:
> > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > >
> > > > > Add DT bindings for GPIO charlieplex keypad.
> > > > >
> > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > ---
> > > > > .../input/gpio-charlieplex-keypad.yaml | 82 +++++++++++++++++++
> > > > > 1 file changed, 82 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > new file mode 100644
> > > > > index 0000000000000..1672491a75a85
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > @@ -0,0 +1,82 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +
> > > > > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: GPIO charlieplex keypad
> > > > > +
> > > > > +maintainers:
> > > > > + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > +
> > > > > +description:
> > > > > + The charlieplex keypad supports N^2)-N different key combinations (where N is
> > > > > + the number of lines). Key presses and releases are detected by configuring
> > > > > + only one line as output at a time, and reading other line states. This process
> > > > > + is repeated for each line.
> > > > > + This mechanism doesn't allow to detect simultaneous key presses.
> > > > > +
> > > > > +allOf:
> > > > > + - $ref: input.yaml#
> > > > > + - $ref: /schemas/input/matrix-keymap.yaml#
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + const: gpio-charlieplex-keypad
> > > > > +
> > > > > + autorepeat: true
> > > > > +
> > > > > + line-scan-delay-us:
> > > > > + description:
> > > > > + Delay, measured in microseconds, that is needed
> > > > > + before we can scan keypad after activating one line.
> > > > > + default: 0
> > > >
> > > > Isn't this the same as "col-scan-delay-us" in gpio-matrix-keypad.yaml?
> > > > If so, move it to matrix-keymap.yaml to re-use it here.
> > >
> > > It is used in a similar fashion, but for charlieplex keyboard, there is
> > > no concept of "rows" and "columns". There are only
> > > lines, which are all equivalent in functionality.
> > >
> > > > If not, there's a bunch of other scan delay properties just from
> > > > grepping "delay" in the input bindings. Surely we can define something
> > > > common.
> > >
> > > Most of those delays refer to something quite different than what
> > > "col-scan-delay-us" or "line-scan-delay-us" are used for (it is a delay
> > > that we wait when activating a GPIO before we can safely/reliably read
> > > other GPIOs connected thru its circuitry).
> > >
> > > Maybe "col-scan-delay-us" and "line-scan-delay-us" could be
> > > combined into a common "line-scan-delay-us" ("line" is more generic
> > > than column), and defined in matrix-keymap.yaml.
> >
> > What about "scan-delay-us"? I would assume all the scan delay
> > properties are just the delay after changing the outputs to reading
> > the inputs.
>
> They are for gpio-matrix-keypad.yaml and this binding, but not for
> others. Most scan delay properties refer to the period or
> interval between successive scans.
>
> So for my binding, "settling-time-us" would be more accurate and a
> better property name (it is also used in adc.yaml).
Let's go with that.
>
> Looking into a common place to define this new property, I stumbled
> upon gpio-delay.yaml, so maybe I do not need this new property at all
> and simply define a gpio-delay node if needed (and add it to this
> binding example)?
>
> I tested this and it works, although it requires a patch to the
> gpio-aggregator driver, because for now it respect the delay only
> when changing the output value, not when switching between input and
> output like I do in my driver.
>
> With my patch, it works ok.
I would not use gpio-delay here.
Rob
^ permalink raw reply
* Re: [PATCH v2] HID: playstation: Center initial joystick axes to prevent spurious events
From: Guenter Roeck @ 2026-02-24 21:10 UTC (permalink / raw)
To: Siarhei Vishniakou
Cc: Jiri Kosina, Benjamin Tissoires, Roderick Colenbrander,
linux-input, linux-kernel
In-Reply-To: <20251111234519.369652-1-svv@google.com>
On Tue, Nov 11, 2025 at 03:45:19PM -0800, Siarhei Vishniakou wrote:
> When a new PlayStation gamepad (DualShock 4 or DualSense) is initialized,
> the input subsystem sets the default value for its absolute axes (e.g.,
> ABS_X, ABS_Y) to 0.
>
> However, the hardware's actual neutral/resting state for these joysticks
> is 128 (0x80). This creates a mismatch.
>
> When the first HID report arrives from the device, the driver sees the
> resting value of 128. The kernel compares this to its initial state of 0
> and incorrectly interprets this as a delta (0 -> 128). Consequently, it
> generates EV_ABS events for this initial, non-existent movement.
>
> This behavior can fail userspace 'sanity check' tests (e.g., in
> Android CTS) that correctly assert no motion events should be generated
> from a device that is already at rest.
>
> This patch fixes the issue by explicitly setting the initial value of the
> main joystick axes (e.g., ABS_X, ABS_Y, ABS_RX, ABS_RY) to 128 (0x80)
> in the common ps_gamepad_create() function.
>
> This aligns the kernel's initial state with the hardware's expected
> neutral state, ensuring that the first report (at 128) produces no
> delta and thus, no spurious event.
>
> Signed-off-by: Siarhei Vishniakou <svv@google.com>
> Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> drivers/hid/hid-playstation.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 1468fb11e39d..a145b5ea4405 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -718,11 +718,16 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
> if (IS_ERR(gamepad))
> return ERR_CAST(gamepad);
>
> + /* Set initial resting state for joysticks to 128 (center) */
> input_set_abs_params(gamepad, ABS_X, 0, 255, 0, 0);
> + gamepad->absinfo[ABS_X].value = 128;
input_set_abs_params() can fail to allocate absinfo. If that happens,
this will crash. AI suggests setting the value with input_abs_set_val()
to ensure that the value is checked before dereferencing.
Guenter
> input_set_abs_params(gamepad, ABS_Y, 0, 255, 0, 0);
> + gamepad->absinfo[ABS_Y].value = 128;
> input_set_abs_params(gamepad, ABS_Z, 0, 255, 0, 0);
> input_set_abs_params(gamepad, ABS_RX, 0, 255, 0, 0);
> + gamepad->absinfo[ABS_RX].value = 128;
> input_set_abs_params(gamepad, ABS_RY, 0, 255, 0, 0);
> + gamepad->absinfo[ABS_RY].value = 128;
> input_set_abs_params(gamepad, ABS_RZ, 0, 255, 0, 0);
>
> input_set_abs_params(gamepad, ABS_HAT0X, -1, 1, 0, 0);
> --
> 2.51.2.1041.gc1ab5b90ca-goo
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-02-24 22:32 UTC (permalink / raw)
To: Rob Herring
Cc: hvilleneuve, dmitry.torokhov, krzk+dt, conor+dt, linux-input,
devicetree, linux-kernel
In-Reply-To: <CAL_JsqJtwM33G4qdzpnp6nNtc+tNr6VOKvTB8y6Xv04GTKxymA@mail.gmail.com>
On Tue, 24 Feb 2026 15:06:40 -0600
Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 24, 2026 at 2:40 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > Hi Rob,
> >
> > On Mon, 23 Feb 2026 17:23:33 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >
> > > On Mon, Feb 23, 2026 at 12:47 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Mon, 23 Feb 2026 11:57:06 -0600
> > > > Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > > On Fri, Feb 13, 2026 at 12:14:25PM -0500, Hugo Villeneuve wrote:
> > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > >
> > > > > > Add DT bindings for GPIO charlieplex keypad.
> > > > > >
> > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > ---
> > > > > > .../input/gpio-charlieplex-keypad.yaml | 82 +++++++++++++++++++
> > > > > > 1 file changed, 82 insertions(+)
> > > > > > create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > > new file mode 100644
> > > > > > index 0000000000000..1672491a75a85
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > > @@ -0,0 +1,82 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +
> > > > > > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: GPIO charlieplex keypad
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > +
> > > > > > +description:
> > > > > > + The charlieplex keypad supports N^2)-N different key combinations (where N is
> > > > > > + the number of lines). Key presses and releases are detected by configuring
> > > > > > + only one line as output at a time, and reading other line states. This process
> > > > > > + is repeated for each line.
> > > > > > + This mechanism doesn't allow to detect simultaneous key presses.
> > > > > > +
> > > > > > +allOf:
> > > > > > + - $ref: input.yaml#
> > > > > > + - $ref: /schemas/input/matrix-keymap.yaml#
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + const: gpio-charlieplex-keypad
> > > > > > +
> > > > > > + autorepeat: true
> > > > > > +
> > > > > > + line-scan-delay-us:
> > > > > > + description:
> > > > > > + Delay, measured in microseconds, that is needed
> > > > > > + before we can scan keypad after activating one line.
> > > > > > + default: 0
> > > > >
> > > > > Isn't this the same as "col-scan-delay-us" in gpio-matrix-keypad.yaml?
> > > > > If so, move it to matrix-keymap.yaml to re-use it here.
> > > >
> > > > It is used in a similar fashion, but for charlieplex keyboard, there is
> > > > no concept of "rows" and "columns". There are only
> > > > lines, which are all equivalent in functionality.
> > > >
> > > > > If not, there's a bunch of other scan delay properties just from
> > > > > grepping "delay" in the input bindings. Surely we can define something
> > > > > common.
> > > >
> > > > Most of those delays refer to something quite different than what
> > > > "col-scan-delay-us" or "line-scan-delay-us" are used for (it is a delay
> > > > that we wait when activating a GPIO before we can safely/reliably read
> > > > other GPIOs connected thru its circuitry).
> > > >
> > > > Maybe "col-scan-delay-us" and "line-scan-delay-us" could be
> > > > combined into a common "line-scan-delay-us" ("line" is more generic
> > > > than column), and defined in matrix-keymap.yaml.
> > >
> > > What about "scan-delay-us"? I would assume all the scan delay
> > > properties are just the delay after changing the outputs to reading
> > > the inputs.
> >
> > They are for gpio-matrix-keypad.yaml and this binding, but not for
> > others. Most scan delay properties refer to the period or
> > interval between successive scans.
> >
> > So for my binding, "settling-time-us" would be more accurate and a
> > better property name (it is also used in adc.yaml).
>
> Let's go with that.
Ok, will do.
>
> >
> > Looking into a common place to define this new property, I stumbled
> > upon gpio-delay.yaml, so maybe I do not need this new property at all
> > and simply define a gpio-delay node if needed (and add it to this
> > binding example)?
> >
> > I tested this and it works, although it requires a patch to the
> > gpio-aggregator driver, because for now it respect the delay only
> > when changing the output value, not when switching between input and
> > output like I do in my driver.
> >
> > With my patch, it works ok.
>
> I would not use gpio-delay here.
Ok.
I also observed that debounce-delay-ms is re-defined by a few bindings,
including mine. I assume that I could move all these identical
definitions to input.yaml and reduce duplication (in a separate patch,
of course).
While testing that, I found an odd situation: if I reference these four
dummy (undefined anywhere) properties in the gpio-matrix-keypad
binding, I am expecting four warnings when validating it. However I get
just three:
==================
diff --git
a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml index
20b5371fa21c..733458bf4d13 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -36,7 +36,10 @@ properties:
linux,keymap: true
+ bogus1-delay-ms: true
+ bogus2-delay-ms-bogus2: true
+ bogus3-my-property: true
+ bogus4: true
linux,no-autorepeat:
type: boolean
==================
Validation result:
...
SCHEMA Documentation/devicetree/bindings/processed-schema.json
...
...input/gpio-matrix-keypad.yaml:
bogus2-delay-ms-bogus2: missing type definition
...input/gpio-matrix-keypad.yaml:
bogus3-my-property: missing type definition
...input/gpio-matrix-keypad.yaml:
bogus4: missing type definition
any idea why?
Thank you,
Hugo.
--
Hugo Villeneuve
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: add GPIO charlieplex keypad
From: Rob Herring @ 2026-02-24 22:36 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: hvilleneuve, dmitry.torokhov, krzk+dt, conor+dt, linux-input,
devicetree, linux-kernel
In-Reply-To: <20260224173247.65ac1b195ff8bab4e8af3163@hugovil.com>
On Tue, Feb 24, 2026 at 4:32 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> On Tue, 24 Feb 2026 15:06:40 -0600
> Rob Herring <robh@kernel.org> wrote:
>
> > On Tue, Feb 24, 2026 at 2:40 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Mon, 23 Feb 2026 17:23:33 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >
> > > > On Mon, Feb 23, 2026 at 12:47 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Mon, 23 Feb 2026 11:57:06 -0600
> > > > > Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > > On Fri, Feb 13, 2026 at 12:14:25PM -0500, Hugo Villeneuve wrote:
> > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > >
> > > > > > > Add DT bindings for GPIO charlieplex keypad.
> > > > > > >
> > > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > ---
> > > > > > > .../input/gpio-charlieplex-keypad.yaml | 82 +++++++++++++++++++
> > > > > > > 1 file changed, 82 insertions(+)
> > > > > > > create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > > > new file mode 100644
> > > > > > > index 0000000000000..1672491a75a85
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > > > > > > @@ -0,0 +1,82 @@
> > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > +%YAML 1.2
> > > > > > > +---
> > > > > > > +
> > > > > > > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > +
> > > > > > > +title: GPIO charlieplex keypad
> > > > > > > +
> > > > > > > +maintainers:
> > > > > > > + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > +
> > > > > > > +description:
> > > > > > > + The charlieplex keypad supports N^2)-N different key combinations (where N is
> > > > > > > + the number of lines). Key presses and releases are detected by configuring
> > > > > > > + only one line as output at a time, and reading other line states. This process
> > > > > > > + is repeated for each line.
> > > > > > > + This mechanism doesn't allow to detect simultaneous key presses.
> > > > > > > +
> > > > > > > +allOf:
> > > > > > > + - $ref: input.yaml#
> > > > > > > + - $ref: /schemas/input/matrix-keymap.yaml#
> > > > > > > +
> > > > > > > +properties:
> > > > > > > + compatible:
> > > > > > > + const: gpio-charlieplex-keypad
> > > > > > > +
> > > > > > > + autorepeat: true
> > > > > > > +
> > > > > > > + line-scan-delay-us:
> > > > > > > + description:
> > > > > > > + Delay, measured in microseconds, that is needed
> > > > > > > + before we can scan keypad after activating one line.
> > > > > > > + default: 0
> > > > > >
> > > > > > Isn't this the same as "col-scan-delay-us" in gpio-matrix-keypad.yaml?
> > > > > > If so, move it to matrix-keymap.yaml to re-use it here.
> > > > >
> > > > > It is used in a similar fashion, but for charlieplex keyboard, there is
> > > > > no concept of "rows" and "columns". There are only
> > > > > lines, which are all equivalent in functionality.
> > > > >
> > > > > > If not, there's a bunch of other scan delay properties just from
> > > > > > grepping "delay" in the input bindings. Surely we can define something
> > > > > > common.
> > > > >
> > > > > Most of those delays refer to something quite different than what
> > > > > "col-scan-delay-us" or "line-scan-delay-us" are used for (it is a delay
> > > > > that we wait when activating a GPIO before we can safely/reliably read
> > > > > other GPIOs connected thru its circuitry).
> > > > >
> > > > > Maybe "col-scan-delay-us" and "line-scan-delay-us" could be
> > > > > combined into a common "line-scan-delay-us" ("line" is more generic
> > > > > than column), and defined in matrix-keymap.yaml.
> > > >
> > > > What about "scan-delay-us"? I would assume all the scan delay
> > > > properties are just the delay after changing the outputs to reading
> > > > the inputs.
> > >
> > > They are for gpio-matrix-keypad.yaml and this binding, but not for
> > > others. Most scan delay properties refer to the period or
> > > interval between successive scans.
> > >
> > > So for my binding, "settling-time-us" would be more accurate and a
> > > better property name (it is also used in adc.yaml).
> >
> > Let's go with that.
>
> Ok, will do.
>
> >
> > >
> > > Looking into a common place to define this new property, I stumbled
> > > upon gpio-delay.yaml, so maybe I do not need this new property at all
> > > and simply define a gpio-delay node if needed (and add it to this
> > > binding example)?
> > >
> > > I tested this and it works, although it requires a patch to the
> > > gpio-aggregator driver, because for now it respect the delay only
> > > when changing the output value, not when switching between input and
> > > output like I do in my driver.
> > >
> > > With my patch, it works ok.
> >
> > I would not use gpio-delay here.
>
> Ok.
>
> I also observed that debounce-delay-ms is re-defined by a few bindings,
> including mine. I assume that I could move all these identical
> definitions to input.yaml and reduce duplication (in a separate patch,
> of course).
Yes, that would be good.
> While testing that, I found an odd situation: if I reference these four
> dummy (undefined anywhere) properties in the gpio-matrix-keypad
> binding, I am expecting four warnings when validating it. However I get
> just three:
>
> ==================
> diff --git
> a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml index
> 20b5371fa21c..733458bf4d13 100644
> --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> @@ -36,7 +36,10 @@ properties:
>
> linux,keymap: true
>
> + bogus1-delay-ms: true
> + bogus2-delay-ms-bogus2: true
> + bogus3-my-property: true
> + bogus4: true
>
> linux,no-autorepeat:
> type: boolean
> ==================
>
> Validation result:
>
> ...
> SCHEMA Documentation/devicetree/bindings/processed-schema.json
> ...
> ...input/gpio-matrix-keypad.yaml:
> bogus2-delay-ms-bogus2: missing type definition
> ...input/gpio-matrix-keypad.yaml:
> bogus3-my-property: missing type definition
> ...input/gpio-matrix-keypad.yaml:
> bogus4: missing type definition
>
> any idea why?
Properties with a unit suffix like "-ms" always have a type hence why
they don't ever define a type.
Rob
^ permalink raw reply
* Re: [PATCH 2/2] input: misc: add driver for max16150
From: Dmitry Torokhov @ 2026-02-24 22:42 UTC (permalink / raw)
To: marcpaolo.sosa
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel
In-Reply-To: <20260223-max16150-v1-2-38e2a4f0d0f1@analog.com>
Hi Marc,
On Mon, Feb 23, 2026 at 07:03:40PM +0800, Marc Paolo Sosa via B4 Relay wrote:
> From: Marc Paolo Sosa <marcpaolo.sosa@analog.com>
>
> MAX16150/MAX16169 nanoPower Pushbutton On/Off Controller
>
> Signed-off-by: Marc Paolo Sosa <marcpaolo.sosa@analog.com>
> ---
> drivers/input/misc/Kconfig | 9 +++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/max16150.c | 161 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 171 insertions(+)
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 94a753fcb64f..a31d3d2a7fd6 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -178,6 +178,15 @@ config INPUT_E3X0_BUTTON
> To compile this driver as a module, choose M here: the
> module will be called e3x0_button.
>
> +config INPUT_MAX16150_PWRBUTTON
> + tristate "MAX16150/MAX16169 Pushbutton driver"
> + help
> + Say Y here if you want to enable power key reporting via
> + MAX16150/MAX16169 nanoPower Pushbutton On/Off Controller.
> +
> + To compile this driver as a module, choose M here. The module will
> + be called max16150.
> +
> config INPUT_PCSPKR
> tristate "PC Speaker support"
> depends on PCSPKR_PLATFORM
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 415fc4e2918b..c2c1c45f2df6 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_INPUT_IQS7222) += iqs7222.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> +obj-$(CONFIG_INPUT_MAX16150_PWRBUTTON) += max16150.o
> obj-$(CONFIG_INPUT_MAX7360_ROTARY) += max7360-rotary.o
> obj-$(CONFIG_INPUT_MAX77650_ONKEY) += max77650-onkey.o
> obj-$(CONFIG_INPUT_MAX77693_HAPTIC) += max77693-haptic.o
> diff --git a/drivers/input/misc/max16150.c b/drivers/input/misc/max16150.c
> new file mode 100644
> index 000000000000..ae353b926afc
> --- /dev/null
> +++ b/drivers/input/misc/max16150.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Analog Devices MAX16150/MAX16169 Pushbutton Driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +
> +#define MAX16150_LONG_INTERRUPT 120000000
> +
> +struct max16150_chip_info {
> + bool has_clr_gpio;
> +};
> +
> +struct max16150_device {
> + struct input_dev *input;
> + struct gpio_desc *gpiod;
> + struct gpio_desc *clr_gpiod;
> + const struct max16150_chip_info *chip_info;
> + u64 low, high, duration;
I do not think you need to store "high" and "duration", just "press"
time. I also do not think you need nanosecond resilution, jiffies will
do.
> + unsigned int keycode;
> +};
> +
> +static irqreturn_t max16150_irq_handler(int irq, void *_max16150)
> +{
> + struct max16150_device *max16150 = _max16150;
> + int value;
> +
> + value = gpiod_get_value(max16150->gpiod);
> +
> + if (!value) {
> + max16150->low = ktime_get_ns();
> + return IRQ_HANDLED;
> + }
> +
> + max16150->high = ktime_get_ns();
> + if (max16150->low) {
> + max16150->duration = max16150->high - max16150->low;
> +
> + if (max16150->duration > MAX16150_LONG_INTERRUPT) {
time_after() is probably what you need here.
> + gpiod_set_value(max16150->clr_gpiod, 1);
> + input_report_key(max16150->input, max16150->keycode, 1);
> + input_sync(max16150->input);
Why is press not reported right away?
> + input_report_key(max16150->input, max16150->keycode, 0);
> + input_sync(max16150->input);
> + }
> +
> + max16150->low = 0;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct max16150_chip_info max16150_variant_a = {
> + .has_clr_gpio = true,
> +};
> +
> +static const struct max16150_chip_info max16150_variant_b = {
> + .has_clr_gpio = false,
> +};
> +
> +static int max16150_probe(struct platform_device *pdev)
> +{
> + const struct max16150_chip_info *chip_info;
> + struct max16150_device *max16150;
> + struct device *dev = &pdev->dev;
> + int err, irq, ret;
Why do you need both err and ret?
> + u32 keycode;
> +
> + chip_info = device_get_match_data(dev);
> + if (!chip_info)
> + return -EINVAL;
> +
> + max16150 = devm_kzalloc(dev, sizeof(*max16150), GFP_KERNEL);
> + if (!max16150)
> + return -ENOMEM;
> +
> + max16150->chip_info = chip_info;
> +
> + max16150->input = devm_input_allocate_device(dev);
> + if (!max16150->input)
> + return -ENOMEM;
> +
> + max16150->input->name = "MAX16150 Pushbutton";
> + max16150->input->phys = "max16150/input0";
> + max16150->input->id.bustype = BUS_HOST;
> +
> + keycode = KEY_POWER;
> + ret = device_property_read_u32(dev, "linux,code", &keycode);
err = ...
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get keycode\n");
> +
> + max16150->keycode = keycode;
> +
> + input_set_capability(max16150->input, EV_KEY, max16150->keycode);
> +
> + max16150->gpiod = devm_gpiod_get(dev, "interrupt", GPIOD_IN);
> + if (IS_ERR(max16150->gpiod))
> + return dev_err_probe(dev, PTR_ERR(max16150->gpiod),
> + "Failed to get interrupt GPIO\n");
> +
> + if (chip_info->has_clr_gpio) {
> + max16150->clr_gpiod = devm_gpiod_get(dev, "clr", GPIOD_OUT_HIGH);
> + if (IS_ERR(max16150->clr_gpiod))
> + return dev_err_probe(dev, PTR_ERR(max16150->clr_gpiod),
> + "Failed to get clr GPIO\n");
> +
> + if (!max16150->clr_gpiod)
How would we end up here? You are using devm_gpiod_get() which will
never return NULL GPIO descriptor.
> + return dev_err_probe(dev, -ENODEV,
> + "clr GPIO is mandatory\n");
> +
> + if (max16150->clr_gpiod) {
> + fsleep(1000);
> + gpiod_set_value(max16150->clr_gpiod, 0);
> + }
> + }
> +
> + irq = gpiod_to_irq(max16150->gpiod);
> + if (irq < 0)
> + return dev_err_probe(dev, irq,
> + "MAX16150: Failed to map GPIO to IRQ");
As Rob said in DT binding review use interrupts property and separate
IRQ and GPIO handling.
> +
> + err = devm_request_irq(dev, irq, max16150_irq_handler,
> + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> + "max16150_irq", max16150);
> + if (err)
> + return err;
> +
> + return input_register_device(max16150->input);
err = input_register_device(...);
if (err)
return err;
return 0;
> +}
> +
> +static const struct of_device_id max16150_of_match[] = {
> + { .compatible = "adi,max16150a", .data = &max16150_variant_a },
> + { .compatible = "adi,max16150b", .data = &max16150_variant_b },
> + { .compatible = "adi,max16169a", .data = &max16150_variant_a },
> + { .compatible = "adi,max16169b", .data = &max16150_variant_b },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max16150_of_match);
> +
> +static struct platform_driver max16150_driver = {
> + .probe = max16150_probe,
> + .driver = {
> + .name = "max16150",
> + .of_match_table = max16150_of_match,
> + },
> +};
> +module_platform_driver(max16150_driver);
> +
> +MODULE_AUTHOR("Marc Paolo Sosa <marcpaolo.sosa@analog.com>");
> +MODULE_DESCRIPTION("MAX16150/MAX16169 Pushbutton Driver");
> +MODULE_LICENSE("GPL");
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: playstation: Center initial joystick axes to prevent spurious events
From: Siarhei Vishniakou @ 2026-02-24 22:44 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jiri Kosina, Benjamin Tissoires, Roderick Colenbrander,
linux-input, linux-kernel
In-Reply-To: <fcaa494a-43ba-410e-ac2b-162f5b4814bc@roeck-us.net>
вт, 24 февр. 2026 г. в 13:10, Guenter Roeck <linux@roeck-us.net>:
>
> On Tue, Nov 11, 2025 at 03:45:19PM -0800, Siarhei Vishniakou wrote:
> > When a new PlayStation gamepad (DualShock 4 or DualSense) is initialized,
> > the input subsystem sets the default value for its absolute axes (e.g.,
> > ABS_X, ABS_Y) to 0.
> >
> > However, the hardware's actual neutral/resting state for these joysticks
> > is 128 (0x80). This creates a mismatch.
> >
> > When the first HID report arrives from the device, the driver sees the
> > resting value of 128. The kernel compares this to its initial state of 0
> > and incorrectly interprets this as a delta (0 -> 128). Consequently, it
> > generates EV_ABS events for this initial, non-existent movement.
> >
> > This behavior can fail userspace 'sanity check' tests (e.g., in
> > Android CTS) that correctly assert no motion events should be generated
> > from a device that is already at rest.
> >
> > This patch fixes the issue by explicitly setting the initial value of the
> > main joystick axes (e.g., ABS_X, ABS_Y, ABS_RX, ABS_RY) to 128 (0x80)
> > in the common ps_gamepad_create() function.
> >
> > This aligns the kernel's initial state with the hardware's expected
> > neutral state, ensuring that the first report (at 128) produces no
> > delta and thus, no spurious event.
> >
> > Signed-off-by: Siarhei Vishniakou <svv@google.com>
> > Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > drivers/hid/hid-playstation.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> > index 1468fb11e39d..a145b5ea4405 100644
> > --- a/drivers/hid/hid-playstation.c
> > +++ b/drivers/hid/hid-playstation.c
> > @@ -718,11 +718,16 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
> > if (IS_ERR(gamepad))
> > return ERR_CAST(gamepad);
> >
> > + /* Set initial resting state for joysticks to 128 (center) */
> > input_set_abs_params(gamepad, ABS_X, 0, 255, 0, 0);
> > + gamepad->absinfo[ABS_X].value = 128;
>
> input_set_abs_params() can fail to allocate absinfo. If that happens,
> this will crash. AI suggests setting the value with input_abs_set_val()
> to ensure that the value is checked before dereferencing.
>
> Guenter
>
> > input_set_abs_params(gamepad, ABS_Y, 0, 255, 0, 0);
> > + gamepad->absinfo[ABS_Y].value = 128;
> > input_set_abs_params(gamepad, ABS_Z, 0, 255, 0, 0);
> > input_set_abs_params(gamepad, ABS_RX, 0, 255, 0, 0);
> > + gamepad->absinfo[ABS_RX].value = 128;
> > input_set_abs_params(gamepad, ABS_RY, 0, 255, 0, 0);
> > + gamepad->absinfo[ABS_RY].value = 128;
> > input_set_abs_params(gamepad, ABS_RZ, 0, 255, 0, 0);
> >
> > input_set_abs_params(gamepad, ABS_HAT0X, -1, 1, 0, 0);
> > --
> > 2.51.2.1041.gc1ab5b90ca-goo
The function `input_set_abs_params` is ill-formed. If this call can
fail, then it should return some kind of error result instead of void.
In addition, the function name doesn't suggest that it's going to be
doing any allocations.
So the (short-term) fix should be something like this:
/* Set initial resting state for joysticks to 128 (center) */
input_set_abs_params(gamepad, ABS_X, 0, 255, 0, 0);
+ if (gamepad->absinfo == NULL) {
+ // input_set_abs_params allocates "absinfo" struct, which could fail.
+ return ERR_CAST(gamepad);
+ }
A better long-term option would be to change the signature of
input_set_abs_params to return a bool, or to add an explicit
allocation step (looking at code search, there aren't too many places
using this API today, but this might break 3-rd party stuff).
Curious to hear maintainers' opinions on this.
^ permalink raw reply
* Re: [PATCH v2 4/5] input: drv260x: Fix unbalanced regulator_disable() call
From: Dmitry Torokhov @ 2026-02-24 22:45 UTC (permalink / raw)
To: Yauhen Kharuzhy; +Cc: linux-input, linux-kernel, Hans de Goede
In-Reply-To: <aZbg2w2lIahy_sM9@jekhomev>
On Thu, Feb 19, 2026 at 12:11:47PM +0200, Yauhen Kharuzhy wrote:
> On Wed, Feb 18, 2026 at 06:43:30PM -0800, Dmitry Torokhov wrote:
> > Hi Yauhen,
> >
> > On Sun, Feb 15, 2026 at 04:14:34PM +0200, Yauhen Kharuzhy wrote:
> > > +static void drv260x_remove(struct i2c_client *client)
> > > +{
> > > + struct drv260x_data *haptics = i2c_get_clientdata(client);
> > > +
> > > + regulator_disable(haptics->regulator);
> > > }
> >
> > This will result in power being shut off the first thing during
> > unbinding, which is too early.
> >
> > I switched this to devm_add_action_or_reset() and applied.
>
> Do you mean that, in this case, regulator_disable() may be called before any
> devm_* registered actions? Good point, thanks.
>
Yes, all devm* actions are run after remove() completes.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: playstation: Center initial joystick axes to prevent spurious events
From: Guenter Roeck @ 2026-02-24 23:34 UTC (permalink / raw)
To: Siarhei Vishniakou
Cc: Jiri Kosina, Benjamin Tissoires, Roderick Colenbrander,
linux-input, linux-kernel
In-Reply-To: <CAKF84v0eVCSM7L-EvtmoquN87sSS0GA1yX-ewGyzr-hqO7tAGA@mail.gmail.com>
On 2/24/26 14:44, Siarhei Vishniakou wrote:
> вт, 24 февр. 2026 г. в 13:10, Guenter Roeck <linux@roeck-us.net>:
>>
>> On Tue, Nov 11, 2025 at 03:45:19PM -0800, Siarhei Vishniakou wrote:
>>> When a new PlayStation gamepad (DualShock 4 or DualSense) is initialized,
>>> the input subsystem sets the default value for its absolute axes (e.g.,
>>> ABS_X, ABS_Y) to 0.
>>>
>>> However, the hardware's actual neutral/resting state for these joysticks
>>> is 128 (0x80). This creates a mismatch.
>>>
>>> When the first HID report arrives from the device, the driver sees the
>>> resting value of 128. The kernel compares this to its initial state of 0
>>> and incorrectly interprets this as a delta (0 -> 128). Consequently, it
>>> generates EV_ABS events for this initial, non-existent movement.
>>>
>>> This behavior can fail userspace 'sanity check' tests (e.g., in
>>> Android CTS) that correctly assert no motion events should be generated
>>> from a device that is already at rest.
>>>
>>> This patch fixes the issue by explicitly setting the initial value of the
>>> main joystick axes (e.g., ABS_X, ABS_Y, ABS_RX, ABS_RY) to 128 (0x80)
>>> in the common ps_gamepad_create() function.
>>>
>>> This aligns the kernel's initial state with the hardware's expected
>>> neutral state, ensuring that the first report (at 128) produces no
>>> delta and thus, no spurious event.
>>>
>>> Signed-off-by: Siarhei Vishniakou <svv@google.com>
>>> Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
>>> ---
>>> drivers/hid/hid-playstation.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
>>> index 1468fb11e39d..a145b5ea4405 100644
>>> --- a/drivers/hid/hid-playstation.c
>>> +++ b/drivers/hid/hid-playstation.c
>>> @@ -718,11 +718,16 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
>>> if (IS_ERR(gamepad))
>>> return ERR_CAST(gamepad);
>>>
>>> + /* Set initial resting state for joysticks to 128 (center) */
>>> input_set_abs_params(gamepad, ABS_X, 0, 255, 0, 0);
>>> + gamepad->absinfo[ABS_X].value = 128;
>>
>> input_set_abs_params() can fail to allocate absinfo. If that happens,
>> this will crash. AI suggests setting the value with input_abs_set_val()
>> to ensure that the value is checked before dereferencing.
>>
>> Guenter
>>
>>> input_set_abs_params(gamepad, ABS_Y, 0, 255, 0, 0);
>>> + gamepad->absinfo[ABS_Y].value = 128;
>>> input_set_abs_params(gamepad, ABS_Z, 0, 255, 0, 0);
>>> input_set_abs_params(gamepad, ABS_RX, 0, 255, 0, 0);
>>> + gamepad->absinfo[ABS_RX].value = 128;
>>> input_set_abs_params(gamepad, ABS_RY, 0, 255, 0, 0);
>>> + gamepad->absinfo[ABS_RY].value = 128;
>>> input_set_abs_params(gamepad, ABS_RZ, 0, 255, 0, 0);
>>>
>>> input_set_abs_params(gamepad, ABS_HAT0X, -1, 1, 0, 0);
>>> --
>>> 2.51.2.1041.gc1ab5b90ca-goo
>
> The function `input_set_abs_params` is ill-formed. If this call can
> fail, then it should return some kind of error result instead of void.
> In addition, the function name doesn't suggest that it's going to be
> doing any allocations.
> So the (short-term) fix should be something like this:
>
> /* Set initial resting state for joysticks to 128 (center) */
> input_set_abs_params(gamepad, ABS_X, 0, 255, 0, 0);
> + if (gamepad->absinfo == NULL) {
> + // input_set_abs_params allocates "absinfo" struct, which could fail.
> + return ERR_CAST(gamepad);
> + }
>
That function is called about 870+ times.
> A better long-term option would be to change the signature of
> input_set_abs_params to return a bool, or to add an explicit
> allocation step (looking at code search, there aren't too many places
> using this API today, but this might break 3-rd party stuff).
>
> Curious to hear maintainers' opinions on this.
The same would apply to the following macro.
static inline void input_abs_set_##_suffix(struct input_dev *dev, \
unsigned int axis, int val) \
{ \
input_alloc_absinfo(dev); \
if (dev->absinfo) \
dev->absinfo[axis]._item = val; \
}
which is called some 170 times with no error check.
Overall, input_abs_set_val() and input_alloc_absinfo() are widely used
in the kernel. Changing that would be monumental. While I agree that the
current code is less than perfect, I think the immediate fix should be
to avoid the crash and use the same mechanism that is used by all other
drivers.
Of course, I am not the maintainer, so that is just my personal opinion.
Thanks,
Guenter
^ permalink raw reply
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