* Re: [PATCH v6 0/7] Add support for MT6392 PMIC
From: Luca Leonardo Scorcia @ 2026-06-13 6:46 UTC (permalink / raw)
To: linux-mediatek
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Sen Chu, Sean Wang, Macpaul Lin, Lee Jones, Matthias Brugger,
AngeloGioacchino Del Regno, Linus Walleij, Julien Massot,
Louis-Alexis Eyraud, Val Packett, Fabien Parent, Akari Tsuyukusa,
Chen Zhong, linux-input, devicetree, linux-kernel, linux-pm,
linux-arm-kernel, linux-gpio
In-Reply-To: <20260612200717.361018-1-l.scorcia@gmail.com>
Please ignore this patch set. As correctly described by sashiko-bot,
the binding with no regulator node would not be usable, even if it
passed dtbs_check. I need to complete work on that and then resubmit.
Sorry about the confusion.
Il giorno ven 12 giu 2026 alle ore 22:26 Luca Leonardo Scorcia
<l.scorcia@gmail.com> ha scritto:
>
> The MediaTek MT6392 PMIC is usually found on devices powered by
> the MT8516/MT8167 SoC and is yet another MT6323/MT6397 variant.
>
> This series is mostly based around patches submitted a couple
> years ago by Fabien Parent and not merged and from Val Packett's
> submission from Jan 2025 that included extra cleanups, fixes, and a
> new dtsi file similar to ones that exist for other PMICs. Some
> comments weren't addressed and the series was ultimately not merged.
>
> These patches only enable three functions: keys, pinctrl and RTC.
> Regulators and speaker amp will follow later as I still need to further
> improve those two, but getting the main PMIC in will make the series
> easier to review.
>
> I added a handful of device tree improvements to fix some dtbs_check
> errors, added support for the pinctrl device and addressed the comments
> from last year's reviews.
>
> Please note that patch 0005 depends on patch 0004 as they both need the
> registers.h file, but they belong to different driver areas. I'm not sure
> if I'm supposed to squash them even if they belong to different driver
> areas of if it's fine like this. Any advice is welcome.
>
> The series has been tested on Xiaomi Mi Smart Clock X04G and on the
> Lenovo Smart Clock 2.
>
> Changes in v6:
> - Dropped the regulators driver for the moment
> - Explained the FCHR key name origin in the commit message
> - Introduced the MFD_CELL_* macro in the sub-devices definitions.
> A separate, independent commit introduced MFD_CELL_* to all the
> subdevices in the mt6397-core.c file for consistency
> - Replaced of_device_get_match_data with device_get_match_data
> - Removed the mfd_match_data enum in favor of the preexisting
> chip_id enum
> - Adjusted the error message if the device is unsupported
>
> Changes in v5 [5]:
> - Double checked regulator driver with data sheet and Android sources.
> The data sheet I have misses a lot of register descriptions, but
> Android sources have been helpful to fill the gaps
> - Reintroduced the required attribute for the regulator compatible
> in the bindings
> - Fixed the missing reference to the MT6392 schema
> - Fixed casts/unused vars reported by kernel test robot
> - Removed Reviewed-by tags from the regulator patches as they have been
> modified in this version
>
> Changes in v4 [4]:
> - Dropped usage of the regulator compatible
> - Fixed commit messages text to properly reference the target subsystem
> - Added supply rails to the regulator
> - Reworked the regulator schema and PMIC dtsi. Now all supplies are
> documented and the schema no longer includes voltage information
> - Removed redundant ldo- / buck- prefixes
> - Renamed the pinfunc header to mediatek,mt6392-pinfunc.h
> - Modified the MFD driver to use a simple identifier in the of_match
> data properties
>
> Changes in v3 [3]:
> - Added pinctrl device
> - Changed mt6397-rtc fallback to mt6323-rtc
> - Added schema for regulators
> - Fixed checkpatch issues
>
> Changes in v2 [2]:
> - Replaced explicit compatibles with fallbacks
>
> Initial version: [1]
>
> [1] https://lore.kernel.org/linux-mediatek/cover.1771865014.git.l.scorcia@gmail.com/
> [2] https://lore.kernel.org/linux-mediatek/20260306120521.163654-1-l.scorcia@gmail.com/
> [3] https://lore.kernel.org/linux-mediatek/20260317184507.523060-1-l.scorcia@gmail.com/
> [4] https://lore.kernel.org/linux-mediatek/20260330083429.359819-1-l.scorcia@gmail.com/
> [5] https://lore.kernel.org/linux-mediatek/20260420213529.1645560-1-l.scorcia@gmail.com/
>
> Fabien Parent (3):
> dt-bindings: mfd: mt6397: Add MT6392 PMIC
> dt-bindings: input: mtk-pmic-keys: Add MT6392 PMIC keys
> mfd: mt6397: Add support for MT6392 PMIC
>
> Luca Leonardo Scorcia (2):
> mfd: mt6397: Use MFD_CELL_* to describe sub-devices
> pinctrl: mediatek: mt6397: Add MediaTek MT6392
>
> Val Packett (2):
> input: keyboard: mtk-pmic-keys: Add MT6392 support
> arm64: dts: mediatek: Add MediaTek MT6392 PMIC dtsi
>
> .../bindings/input/mediatek,pmic-keys.yaml | 1 +
> .../bindings/mfd/mediatek,mt6397.yaml | 8 +
> arch/arm64/boot/dts/mediatek/mt6392.dtsi | 41 ++
> drivers/input/keyboard/mtk-pmic-keys.c | 17 +
> drivers/mfd/mt6397-core.c | 294 +++++------
> drivers/mfd/mt6397-irq.c | 8 +
> drivers/pinctrl/mediatek/pinctrl-mt6397.c | 37 +-
> drivers/pinctrl/mediatek/pinctrl-mtk-mt6392.h | 64 +++
> include/linux/mfd/mt6392/core.h | 43 ++
> include/linux/mfd/mt6392/registers.h | 488 ++++++++++++++++++
> include/linux/mfd/mt6397/core.h | 1 +
> 11 files changed, 840 insertions(+), 162 deletions(-)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6392.dtsi
> create mode 100644 drivers/pinctrl/mediatek/pinctrl-mtk-mt6392.h
> create mode 100644 include/linux/mfd/mt6392/core.h
> create mode 100644 include/linux/mfd/mt6392/registers.h
>
> --
> 2.43.0
>
--
Luca Leonardo Scorcia
l.scorcia@gmail.com
^ permalink raw reply
* Re: [PATCH v4 1/7] arm64: defconfig: Enable Allwinner LRADC input driver
From: Jernej Škrabec @ 2026-06-13 8:12 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi, Alexander Sverdlin
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Samuel Holland, Hans de Goede,
Dmitry Torokhov, Andre Przywara, Jun Yan, Lukas Schmid,
J. Neuschäfer, Eric Biggers, Michal Simek, Luca Weiss,
Sven Peter, Maxime Ripard, devicetree, linux-kernel, linux-input
In-Reply-To: <20260605070923.3045073-2-alexander.sverdlin@gmail.com>
Dne petek, 5. junij 2026 ob 09:09:15 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> Enable Allwinner LRADC input driver as module to support buttons on Baijie
> HelperBoard A133.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej Skrabec
^ permalink raw reply
* Re: [PATCH v4 5/7] arm64: dts: allwinner: a100: Add LRADC node
From: Jernej Škrabec @ 2026-06-13 8:20 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi, Alexander Sverdlin
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Samuel Holland, Hans de Goede,
Dmitry Torokhov, Andre Przywara, Jun Yan, Lukas Schmid,
J. Neuschäfer, Eric Biggers, Michal Simek, Luca Weiss,
Sven Peter, Maxime Ripard, devicetree, linux-kernel, linux-input
In-Reply-To: <20260605070923.3045073-6-alexander.sverdlin@gmail.com>
Dne petek, 5. junij 2026 ob 09:09:19 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> A100/A133 SoCs feature a Low Rate ADC (LRADC) for Key application.
>
> Specs:
> - Power supply voltage: 1.8 V
> - Reference voltage: 1.35 V
> - Interrupt support
> - Support Hold Key and General Key
> - Support normal, continue and single work mode
> - 6-bits resolution, sample rate up to 2 kHz
> - Voltage input range between 0 and 1.35 V
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> ---
> Changelog:
> v4:
> - added allwinner,sun50i-a100-lradc compatible
> v3:
> - new patch
>
> arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> index b3fb1e0ee796..7cb06b19b5a5 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> @@ -466,6 +466,15 @@ ths: thermal-sensor@5070400 {
> #thermal-sensor-cells = <1>;
> };
>
> + lradc: lradc@5070800 {
> + compatible = "allwinner,sun50i-a100-lradc", "allwinner,sun50i-r329-lradc";
Break this into two lines, like usb_otg below. With that:
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej Skrabec
> + reg = <0x05070800 0x400>;
> + interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&ccu CLK_BUS_LRADC>;
> + resets = <&ccu RST_BUS_LRADC>;
> + status = "disabled";
> + };
> +
> usb_otg: usb@5100000 {
> compatible = "allwinner,sun50i-a100-musb",
> "allwinner,sun8i-a33-musb";
>
^ permalink raw reply
* Re: [PATCH v4 6/7] arm64: dts: allwinner: a100: reserve RAM for ATF
From: Jernej Škrabec @ 2026-06-13 8:22 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi, Alexander Sverdlin
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Samuel Holland, Hans de Goede,
Dmitry Torokhov, Andre Przywara, Jun Yan, Lukas Schmid,
J. Neuschäfer, Eric Biggers, Michal Simek, Luca Weiss,
Sven Peter, Maxime Ripard, devicetree, linux-kernel, linux-input
In-Reply-To: <20260605070923.3045073-7-alexander.sverdlin@gmail.com>
Dne petek, 5. junij 2026 ob 09:09:20 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> Add reserved-memory node carving out Trusted Firmware-A region spanning
> fixed 256K from physical address 0x40000000. Even though Allwinner ATF
> itself passes the address range in the fdt to U-Boot, U-Boot currently
> only reserves this memory internally, but doesn't carve out the region
> in the fdt passed to Linux.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
NAK. It is job of boot procedure to properly inject TF-A reserved node.
Any issue should be fixed there.
Best regards,
Jernej Skrabec
> ---
> Changelog:
> v4:
> - new patch
>
> arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> index 7cb06b19b5a5..d8391663fd1d 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> @@ -87,6 +87,22 @@ osc32k: osc32k-clk {
> #clock-cells = <0>;
> };
>
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + /*
> + * 256 KiB reserved for Trusted Firmware-A (BL31).
> + * This is added by BL31 itself, but some bootloaders fail
> + * to propagate this into the DTB handed to kernels.
> + */
> + secmon@40000000 {
> + reg = <0x0 0x40000000 0x0 0x40000>;
> + no-map;
> + };
> + };
> +
> timer {
> compatible = "arm,armv8-timer";
> interrupts = <GIC_PPI 13
>
^ permalink raw reply
* Re: [PATCH v4 6/7] arm64: dts: allwinner: a100: reserve RAM for ATF
From: Alexander Sverdlin @ 2026-06-13 8:34 UTC (permalink / raw)
To: Jernej Škrabec, linux-arm-kernel, linux-sunxi
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Hans de Goede, Dmitry Torokhov, Andre Przywara,
Jun Yan, Lukas Schmid, J. Neuschäfer, Eric Biggers,
Michal Simek, Luca Weiss, Sven Peter, Maxime Ripard, devicetree,
linux-kernel, linux-input
In-Reply-To: <_6NEZ78hR8a_3V2bhBAGoQ@gmail.com>
Hi Jernej,
On Sat, 2026-06-13 at 10:22 +0200, Jernej Škrabec wrote:
> Dne petek, 5. junij 2026 ob 09:09:20 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> > Add reserved-memory node carving out Trusted Firmware-A region spanning
> > fixed 256K from physical address 0x40000000. Even though Allwinner ATF
> > itself passes the address range in the fdt to U-Boot, U-Boot currently
> > only reserves this memory internally, but doesn't carve out the region
> > in the fdt passed to Linux.
> >
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
>
> NAK. It is job of boot procedure to properly inject TF-A reserved node.
> Any issue should be fixed there.
like in commit 0d17c865118881609ea7e381c7cadbb7979cc596
("arm64: dts: allwinner: Add Allwinner H616 .dtsi file")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
? ;-)
> Best regards,
> Jernej Skrabec
>
> > ---
> > Changelog:
> > v4:
> > - new patch
> >
> > arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> > index 7cb06b19b5a5..d8391663fd1d 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> > @@ -87,6 +87,22 @@ osc32k: osc32k-clk {
> > #clock-cells = <0>;
> > };
> >
> > + reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + /*
> > + * 256 KiB reserved for Trusted Firmware-A (BL31).
> > + * This is added by BL31 itself, but some bootloaders fail
> > + * to propagate this into the DTB handed to kernels.
> > + */
> > + secmon@40000000 {
> > + reg = <0x0 0x40000000 0x0 0x40000>;
> > + no-map;
> > + };
> > + };
> > +
> > timer {
> > compatible = "arm,armv8-timer";
> > interrupts = <GIC_PPI 13
--
Alexander Sverdlin.
^ permalink raw reply
* Re: [PATCH v4 6/7] arm64: dts: allwinner: a100: reserve RAM for ATF
From: Jernej Škrabec @ 2026-06-13 9:38 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi, Alexander Sverdlin
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Hans de Goede, Dmitry Torokhov, Andre Przywara,
Jun Yan, Lukas Schmid, J. Neuschäfer, Eric Biggers,
Michal Simek, Luca Weiss, Sven Peter, Maxime Ripard, devicetree,
linux-kernel, linux-input
In-Reply-To: <b428d57ba5464f1226daf099877f4c25fa4fc191.camel@gmail.com>
Dne sobota, 13. junij 2026 ob 10:34:42 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> Hi Jernej,
>
> On Sat, 2026-06-13 at 10:22 +0200, Jernej Škrabec wrote:
> > Dne petek, 5. junij 2026 ob 09:09:20 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> > > Add reserved-memory node carving out Trusted Firmware-A region spanning
> > > fixed 256K from physical address 0x40000000. Even though Allwinner ATF
> > > itself passes the address range in the fdt to U-Boot, U-Boot currently
> > > only reserves this memory internally, but doesn't carve out the region
> > > in the fdt passed to Linux.
> > >
> > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> >
> > NAK. It is job of boot procedure to properly inject TF-A reserved node.
> > Any issue should be fixed there.
>
>
> like in commit 0d17c865118881609ea7e381c7cadbb7979cc596
> ("arm64: dts: allwinner: Add Allwinner H616 .dtsi file")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
>
> ? ;-)
I was against that too.
It's time to be fixed in bootloader.
Best regards,
Jernej
>
> > Best regards,
> > Jernej Skrabec
> >
> > > ---
> > > Changelog:
> > > v4:
> > > - new patch
> > >
> > > arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 16 ++++++++++++++++
> > > 1 file changed, 16 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> > > index 7cb06b19b5a5..d8391663fd1d 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
> > > @@ -87,6 +87,22 @@ osc32k: osc32k-clk {
> > > #clock-cells = <0>;
> > > };
> > >
> > > + reserved-memory {
> > > + #address-cells = <2>;
> > > + #size-cells = <2>;
> > > + ranges;
> > > +
> > > + /*
> > > + * 256 KiB reserved for Trusted Firmware-A (BL31).
> > > + * This is added by BL31 itself, but some bootloaders fail
> > > + * to propagate this into the DTB handed to kernels.
> > > + */
> > > + secmon@40000000 {
> > > + reg = <0x0 0x40000000 0x0 0x40000>;
> > > + no-map;
> > > + };
> > > + };
> > > +
> > > timer {
> > > compatible = "arm,armv8-timer";
> > > interrupts = <GIC_PPI 13
>
>
^ permalink raw reply
* Re: [PATCH v2 8/9] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: Jun Yan @ 2026-06-13 9:53 UTC (permalink / raw)
To: conor
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101, krzk+dt,
linux-input, linux-kernel, robh
In-Reply-To: <20260612-plethora-debatable-d00cb679277b@spud>
> On Fri, Jun 12, 2026 at 03:22:14PM +0800, Jun Yan wrote:
> > CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
> > and hardware reset support.
> >=20
> > Add the compatible string for CAP1114, add its datasheet URL,
> > update the maximum of LED channel reg, and add constraint for
> > linux,keycodes.
> >=20
> > Previously, the LED reg property had a default maximum of 7 for CAP1188.
> > With the addition of CAP1114, the default maximum is now 11.
> > An if-then constraint is added to limit the LED count for CAP1188.
> >=20
> > Update description for microchip,input-threshold: CAP1114 only provides
> > eight threshold entries, which does not match its total channel count.
> >=20
> > CAP1114 does not support microchip,signal-guard and
> > microchip,calib-sensitivity.
> >=20
> > Add CAP1114 to the unsupported enum list.
> >=20
> > Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> > ---
> > .../bindings/input/microchip,cap11xx.yaml | 32 ++++++++++++++++++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >=20
> > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > index 778ec6d659a8..0e9a1a8a3f3e 100644
> > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > @@ -12,6 +12,7 @@ description: |
> > =20
> > For more product information please see the links below:
> > CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.=
> pdf
> > + CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.=
> pdf
> > CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.=
> pdf
> > CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.=
> pdf
> > CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.=
> pdf
> > @@ -26,6 +27,7 @@ properties:
> > compatible:
> > enum:
> > - microchip,cap1106
> > + - microchip,cap1114
> > - microchip,cap1126
> > - microchip,cap1188
> > - microchip,cap1203
> > @@ -122,6 +124,8 @@ properties:
> > is required for a touch to be registered, making the touch sensor =
> less
> > sensitive.
> > The number of entries must correspond to the number of channels.
> > + CAP1114 is an exception where channels 8~14 reuse the eighth entry=
> 's
> > + threshold, so counts differ.
> > =20
> > microchip,calib-sensitivity:
> > $ref: /schemas/types.yaml#/definitions/uint32-array
> > @@ -149,7 +153,7 @@ patternProperties:
> > reg:
> > description: LED channel number
> > minimum: 0
> > - maximum: 7
> > + maximum: 10
> > =20
> > label: true
> > =20
> > @@ -178,6 +182,18 @@ allOf:
> > properties:
> > reset-gpios: false
> > =20
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - microchip,cap1114
> > + then:
> > + properties:
> > + linux,keycodes:
> > + minItems: 14
> > + maxItems: 14
>
> Sashiko complaint here is valid.
> You need to increase the outer constraint to max 14, and only set the
> min here.
> Then you need to add an else that sets maxitems to 8.
Thanks for the review. I'll fix this in v3.
> pw-bot: changes-requested
>
> Cheers,
> Conor.
^ permalink raw reply
* Re: [PATCH v2 6/9] Input: cap11xx - add reset gpio support
From: Jun Yan @ 2026-06-13 10:17 UTC (permalink / raw)
To: sashiko-bot
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101,
linux-input, robh, sashiko-reviews
In-Reply-To: <20260612073150.185DB1F000E9@smtp.kernel.org>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(=
> s) to consider:
>
> New issues:
> - [Medium] Unintended glitch on the reset line during probe due to GPIOD_OU=
> T_LOW initialization.
>
> Pre-existing issues:
> - [High] Unvalidated keycodes from Device Tree cause an out-of-bounds heap =
> write.
> --
>
> commit 5befbfbb7019f474fe2d2d2e0e3da23976b75ada
> Author: Jun Yan <jerrysteve1101@gmail.com>
>
> Input: cap11xx - add reset gpio support
>
> Some CAP11xx devices (CAP1126/CAP1188) have a dedicated RESET pin. Add
> hardware reset operation to improve device reliability and ensure proper
> initialization on probe.
>
> > diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/ca=
> p11xx.c
> > index 686174722204c..75746a8a22332 100644
> > --- a/drivers/input/keyboard/cap11xx.c
> > +++ b/drivers/input/keyboard/cap11xx.c
>
> [ ... ]
>
> > @@ -452,6 +457,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_=
> client)
> > if (IS_ERR(priv->regmap))
> > return PTR_ERR(priv->regmap);
> > =20
> > + priv->reset_gpio =3D devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LO=
> W);
> > + if (IS_ERR(priv->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
> > + "Failed to get 'reset' GPIO\n");
> > + if (priv->reset_gpio) {
> > + gpiod_set_value_cansleep(priv->reset_gpio, 1);
>
> [Severity: Medium]
> Does initializing the reset GPIO with GPIOD_OUT_LOW cause an unintended
> glitch on the reset line?
>
> When devm_gpiod_get_optional() is called with GPIOD_OUT_LOW, the GPIO line
> is immediately driven to its logical low (inactive) state. If the hardware
> or bootloader previously left the device in a reset state, this prematurely
> pulls it out of reset.
>
> The driver then immediately calls gpiod_set_value_cansleep() to assert
> the reset. This sequence creates an extremely short, unintended
> de-assert/assert glitch on the reset pin, which could violate hardware
> timing requirements.
>
> Would it be safer to request the reset GPIO with GPIOD_OUT_HIGH (or
> GPIOD_ASIS) to guarantee it starts in the logically asserted state before
> we hold it for the required minimum duration?
Agreed, this also makes the code cleaner.
>
> [Severity: High]
> This is a pre-existing issue, but does this code overflow the keybit array
> if a device tree provides unvalidated keycodes?
>
> Later in cap11xx_i2c_probe(), the driver loops over the keycodes read from
> the device tree:
>
> for (i =3D 0; i < cap->num_channels; i++)
> __set_bit(priv->keycodes[i], priv->idev->keybit);
>
> Since __set_bit() does not perform bounds checking, if a device tree
> specifies a keycode value greater than or equal to KEY_MAX, this will
> write outside the bounds of the keybit array in the input_dev structure,
> clobbering adjacent memory in the kernel heap.
>
> Should we add a check against KEY_MAX before setting the bit?
>
> > + usleep_range(CAP11XX_T_RST_FILT_MIN_US, CAP11XX_T_RST_FILT_MIN_US * 2);
> > + gpiod_set_value_cansleep(priv->reset_gpio, 0);
> > + msleep(CAP11XX_T_RST_ON_MIN_MS);
> > + }
> > +
> > error =3D regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
>
> --=20
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260612072237.1177=
> 304-1-jerrysteve1101@gmail.com?part=3D6
>
^ permalink raw reply
* Re: [PATCH v4 7/7] arm64: dts: allwinner: A133: add support for Baijie Helper A133 board
From: Jernej Škrabec @ 2026-06-13 11:01 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi, Alexander Sverdlin
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Samuel Holland, Hans de Goede,
Dmitry Torokhov, Andre Przywara, Jun Yan, Lukas Schmid,
J. Neuschäfer, Eric Biggers, Michal Simek, Luca Weiss,
Sven Peter, Maxime Ripard, devicetree, linux-kernel, linux-input
In-Reply-To: <20260605070923.3045073-8-alexander.sverdlin@gmail.com>
Dne petek, 5. junij 2026 ob 09:09:21 Srednjeevropski poletni čas je Alexander Sverdlin napisal(a):
> Baijie Helper A133 board is a development board around Baijie A133 Core
> SBC. Features:
>
> - 1/2/4GiB LPDDR4 DRAM
> - 8/16/32GiB eMMC
> - AXP707 PMIC
> - USB-C OTG port in peripheral mode (via onboard hub)
> - 2 USB 2.0 ports
> - MicroSD slot and on-board eMMC module
> - Gigabit Ethernet
> - Bluetooth
> - WiFi
>
> Add initial support for both the Helper and Core boards, including UART,
> PMU, eMMC, USB, Ethernet, LRADC-connected buttons.
>
> UART1 can only be used for Bluetooth module, but BT-WiFi combo Allwinner
> AW869A chip has no mainline driver currently.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Schema validation passes, so:
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
^ permalink raw reply
* Re: [PATCH v2 9/9] Input: cap11xx - add support for CAP1114
From: Jun Yan @ 2026-06-13 11:12 UTC (permalink / raw)
To: sashiko-bot
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101,
linux-input, robh, sashiko-reviews
In-Reply-To: <20260612074238.4C3371F000E9@smtp.kernel.org>
> Thank you for your contribution! Sashiko AI review found 4 potential issue(=
> s) to consider:
> - [High] Buffer overflow when parsing the 'microchip,calib-sensitivity' dev=
> ice tree property for devices with more than 8 channels.
> - [Medium] Integer truncation silently drops signal guard configurations fo=
> r channels 8-13.
> - [Low] Out-of-bounds `GENMASK` generates an 11-bit mask for an 8-bit LED c=
> ontrol register.
> - [Low] Unnecessary 9th threshold register added to regmap defaults.
> --
>
> commit a843fc20f78c2766e479b33ffb3dcd6238d1f3f0
> Author: Jun Yan <jerrysteve1101@gmail.com>
>
> Input: cap11xx - add support for CAP1114
>
> CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
> and hardware reset support. The CAP1114 uses two control registers
> for LED output management and requires two button status registers
> for touch input state reporting.
>
> > diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/ca=
> p11xx.c
> > index d45bb231d7a1f..e4da0e26c34a8 100644
> > --- a/drivers/input/keyboard/cap11xx.c
> > +++ b/drivers/input/keyboard/cap11xx.c
> [ ... ]
> > @@ -98,6 +105,9 @@ static const struct reg_default cap11xx_reg_defaults[]=
> =3D {
> > { CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
> > { CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
> > { CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
> > + { CAP11XX_REG_SENSOR_THRESH(6), 0x40 },
> > + { CAP11XX_REG_SENSOR_THRESH(7), 0x40 },
> > + { CAP11XX_REG_SENSOR_THRESH(8), 0x40 },
>
> [Severity: Low]
> Does this code inadvertently add a 9th threshold register?
>
> Since the indexing is 0-based, an 8-threshold device should only need indic=
> es
> 0 through 7.
>
> Register index 8 appears to map to an unused register.
Good catch, CAP11XX_REG_SENSOR_THRESH(8) should be dropped.
>
> > { CAP11XX_REG_CONFIG2, 0x40 },
> > };
> [ ... ]
> > @@ -394,6 +442,14 @@ static int cap11xx_init_leds(struct device *dev,
> > if (error)
> > return error;
> > =20
> > + if (num_leds > 8) {
> > + error =3D regmap_update_bits(priv->regmap,
> > + priv->model->led_output_control_reg_base + 1,
> > + GENMASK(num_leds - 8 - 1, 0), 0);
>
> [Severity: Low]
> Can this result in an out-of-bounds mask generation when num_leds > 8?
>
> In cap11xx_init_leds() just prior to this addition, the code calls:
>
> error =3D regmap_update_bits(priv->regmap,
> priv->model->led_output_control_reg_base,
> GENMASK(num_leds - 1, 0), 0);
>
> When num_leds is 11 for the CAP1114 model, this generates an 11-bit mask
> to update an 8-bit register.
>
> While the regmap core safely masks this, should the mask be clamped to
> min(num_leds, 8) to accurately reflect the hardware register's capacity?
Agreed, thanks for the correction.
>
> > + if (error)
> > + return error;
> > + }
> [ ... ]
> > @@ -572,6 +628,14 @@ static const struct cap11xx_hw_model cap1106_model =
> =3D {
> > .has_repeat_en =3D true,
> > };
> > =20
> > +static const struct cap11xx_hw_model cap1114_model =3D {
> > + .product_id =3D 0x3a,
> > + .num_channels =3D 14, .num_leds =3D 11, .num_sensor_thresholds =3D 8,
>
> [Severity: High]
> Does this code overflow the calib_sensitivities array during initialization?
>
> In struct cap11xx_priv, the array is rigidly sized to 8 elements:
>
> u32 calib_sensitivities[8];
> u32 keycodes[];
>
> During cap11xx_init_keys(), when parsing "microchip,calib-sensitivity", the
> driver reads priv->model->num_channels elements:
>
> of_property_read_u32_array(node, "microchip,calib-sensitivity",
> priv->calib_sensitivities,
> priv->model->num_channels)
>
> With num_channels set to 14, won't this write 14 elements into an 8-element
> array, clobbering the adjacent keycodes flexible array member and potential=
> ly
> causing memory corruption or a kernel panic?
Good point, I'll fix that in v3.
>
>
> [Severity: Medium]
> Will this configuration lead to integer truncation when parsing the
> "microchip,signal-guard" property?
>
> In struct cap11xx_priv, the mask is defined as an 8-bit value:
>
> u8 signal_guard_inputs_mask;
>
> During cap11xx_init_keys(), a loop iterates up to num_channels (14) and shi=
> fts
> the bits:
>
> priv->signal_guard_inputs_mask |=3D 0x01 << i;
>
> For channels 8 to 13, won't this bitwise shift exceed the 8-bit variable wi=
> dth
> and silently truncate to 0?
Good point, I'll fix that in v3.
>
> Even if the CAP1114 doesn't support signal guards, this drops the bitmask f=
> or
> the upper channels, bypassing the capability check and failing to emit the
> intended 'unsupported' warning.
>
> > + .led_output_control_reg_base =3D CAP1114_REG_LED_OUTPUT_CONTROL1,
> > + .sensor_input_reg_base =3D CAP1114_REG_BUTTON_STATUS1,
> > + .has_grouped_sensors =3D true,
> > +};
>
> --=20
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260612072237.1177=
> 304-1-jerrysteve1101@gmail.com?part=3D9
>
^ permalink raw reply
* Re: [PATCH v4 6/7] arm64: dts: allwinner: a100: reserve RAM for ATF
From: Alexander Sverdlin @ 2026-06-13 12:23 UTC (permalink / raw)
To: Jernej Škrabec, linux-arm-kernel, linux-sunxi
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Hans de Goede, Dmitry Torokhov, Andre Przywara,
Jun Yan, Lukas Schmid, J. Neuschäfer, Eric Biggers,
Michal Simek, Luca Weiss, Sven Peter, Maxime Ripard, devicetree,
linux-kernel, linux-input
In-Reply-To: <FE7Vh4yfTmGMM24i18Wwwg@gmail.com>
Hi Jernej,
On Sat, 2026-06-13 at 11:38 +0200, Jernej Škrabec wrote:
> > > > Add reserved-memory node carving out Trusted Firmware-A region spanning
> > > > fixed 256K from physical address 0x40000000. Even though Allwinner ATF
> > > > itself passes the address range in the fdt to U-Boot, U-Boot currently
> > > > only reserves this memory internally, but doesn't carve out the region
> > > > in the fdt passed to Linux.
> > > >
> > > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > >
> > > NAK. It is job of boot procedure to properly inject TF-A reserved node.
> > > Any issue should be fixed there.
> >
> >
> > like in commit 0d17c865118881609ea7e381c7cadbb7979cc596
> > ("arm64: dts: allwinner: Add Allwinner H616 .dtsi file")
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> >
> > ? ;-)
>
> I was against that too.
>
> It's time to be fixed in bootloader.
I agree, I was wondering about this shortcut as well... I'll implement
some sort of generic reserved memory handling in U-Boot.
--
Alexander Sverdlin.
^ permalink raw reply
* Re: [PATCH v2 4/4] HID: asus: add i2c entry for FA808UM and other TUFs
From: Denis Benato @ 2026-06-13 13:22 UTC (permalink / raw)
To: sashiko-reviews
Cc: dmitry.torokhov, linux-input, connorbelli2003, Jiri Kosina,
Benjamin Tissoires
In-Reply-To: <20260612143750.26B531F000E9@smtp.kernel.org>
On 6/12/26 16:37, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Improper Workqueue Initialization / Memory Corruption via Re-initialization
> - [High] Use-After-Free due to Race Condition in `asus_remove()`
> - [High] Missing NULL check for `devm_kasprintf` allocation leading to Kernel Oops
> --
>
> commit ee67627a03893a0ccf0a7653ac62191a687b7821
> Author: Denis Benato <denis.benato@linux.dev>
>
> HID: asus: add i2c entry for FA808UM and other TUFs
>
> On newer TUF laptops the keyboard HID device uses the same PID/VID of a
> USB device that was found in ROG laptops: add it to hid-asus as i2c too.
>
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index 0f78242b6a1db..7964fc96196e5 100644
>> --- a/drivers/hid/hid-asus.c
>> +++ b/drivers/hid/hid-asus.c
>> @@ -1565,6 +1565,9 @@ static const struct hid_device_id asus_devices[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
>> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
>> + { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
>> + USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
>> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
> [Severity: Critical]
> This is a pre-existing issue, but since this patch enables QUIRK_HID_FN_LOCK
> for a new device, could the fn_lock_sync_work initialization cause workqueue
> corruption?
>
> In asus_input_configured(), INIT_WORK(&drvdata->fn_lock_sync_work, ...) is
> called per-input. For devices with multiple inputs, if hid_hw_start() enables
> interrupts before all inputs are configured, an early KEY_FN_ESC event could
> schedule the work before the next input is configured.
>
> The next call to asus_input_configured() would then re-execute INIT_WORK() on
> the already queued work item, clearing its list pointers and causing a panic.
>
> Additionally, for devices with 0 inputs, it is never initialized, causing a
> warning in cancel_work_sync().
This is implemented in f631011e36b87b173b71c7592b558ad05d791228
("HID: hid-asus: Implement fn lock for Asus ProArt P16") and I see a
asus_kbd_set_fn_lock(hdev, true);
things
asus_kbd_set_fn_lock(hdev, true);
Shouldn't one of the two (probably the latest) be:
asus_kbd_set_fn_lock(hdev, false);
Moreover I see from the commit message that this implementation has
been copied from hid-lenovo, so does hid-lenovo suffers from the same issue?
Since I don't have the affected hardware and I'm much confused by this double true
I won't try to fix it in this patchset unless absolutely necessary or I have more info,
but ideally I would like the patch to come from who can actually test the fix.
> [Severity: High]
> This is a pre-existing issue, but is there a potential use-after-free during
> device removal when QUIRK_HID_FN_LOCK is enabled?
>
> In asus_remove(), cancel_work_sync(&drvdata->fn_lock_sync_work) is called
> before hid_hw_stop(hdev). Since the hardware is still active, a late
> KEY_FN_ESC event can trigger asus_event() and unconditionally call
> schedule_work() after the work was supposedly canceled.
>
> Once the driver is removed and drvdata is freed by devres, could the
> workqueue execute asus_sync_fn_lock() and dereference the freed drvdata?
>
> [Severity: High]
> This is a pre-existing issue, but does the XGM LED name allocation need a
> NULL check?
>
> In asus_probe(), devm_kasprintf() is used to allocate the name for
> drvdata->xgm_led->cdev.name:
>
> drvdata->xgm_led->cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
> "asus:xgm-%s:led",
> strlen(hdev->uniq) ?
> hdev->uniq : dev_name(&hdev->dev));
> drvdata->xgm_led->cdev.brightness = 1;
>
> If devm_kasprintf() fails under memory pressure, it returns NULL. This NULL
> pointer is then passed to led_classdev_register(), which passes it to
> led_classdev_next_name() and eventually to strscpy() as the source string.
> Since strscpy() does not check for a NULL source, this would cause an
> immediate NULL pointer dereference.
>
>> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>> USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
>> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
^ permalink raw reply
* Re: [PATCH v2 3/4] HID: asus: add support for xgm led
From: Denis Benato @ 2026-06-13 14:01 UTC (permalink / raw)
To: Antheas Kapenekakis, Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll
In-Reply-To: <CAGwozwE-EOdbJgKwUXth3S5Tzw4AcxJaMCjP=r0YUP5+TNnOkQ@mail.gmail.com>
On 6/13/26 02:30, Antheas Kapenekakis wrote:
> On Fri, 12 Jun 2026 at 17:56, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 6/12/26 16:39, Antheas Kapenekakis wrote:
>>> On Fri, 12 Jun 2026 at 16:23, Denis Benato <denis.benato@linux.dev> wrote:
>>>> XG mobile stations have very bright leds behind the fan that can be
>>>> turned either ON or OFF: add a cled interface to allow controlling the
>>>> brightness of those red leds.
>>>>
>>>> Signed-off-by: Denis Benato <denis.benato@linux.dev>
>>>> ---
>>>> drivers/hid/hid-asus.c | 70 ++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 70 insertions(+)
>>>>
>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>>> index 323dc0b7f3ff..21c4a60d224e 100644
>>>> --- a/drivers/hid/hid-asus.c
>>>> +++ b/drivers/hid/hid-asus.c
>>>> @@ -51,6 +51,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>>>> #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>>>> #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>>>>
>>>> +#define ROG_XGM_REPORT_SIZE 300
>>>> +
>>>> #define ROG_ALLY_REPORT_SIZE 64
>>>> #define ROG_ALLY_X_MIN_MCU 313
>>>> #define ROG_ALLY_MIN_MCU 319
>>>> @@ -118,6 +120,11 @@ struct asus_kbd_leds {
>>>> bool removed;
>>>> };
>>>>
>>>> +struct asus_xgm_led {
>>>> + struct led_classdev cdev;
>>>> + struct hid_device *hdev;
>>>> +};
>>>> +
>>>> struct asus_touchpad_info {
>>>> int max_x;
>>>> int max_y;
>>>> @@ -143,6 +150,7 @@ struct asus_drvdata {
>>>> unsigned long battery_next_query;
>>>> struct work_struct fn_lock_sync_work;
>>>> bool fn_lock;
>>>> + struct asus_xgm_led *xgm_led;
>>>> };
>>>>
>>>> static int asus_report_battery(struct asus_drvdata *, u8 *, int);
>>>> @@ -941,6 +949,23 @@ static int asus_battery_probe(struct hid_device *hdev)
>>>> return ret;
>>>> }
>>>>
>>>> +static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
>>>> +{
>>>> + const u8 buf[ROG_XGM_REPORT_SIZE] = {
>>>> + FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
>>>> + };
>>>> + struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
>>>> + int ret;
>>>> +
>>>> + ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
>>>> + if (ret != ROG_XGM_REPORT_SIZE) {
>>>> + hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>>>> {
>>>> struct input_dev *input = hi->input;
>>>> @@ -1184,6 +1209,14 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
> I might have confused the hunk here and you are right, input
> configured is above.
>
>>>> }
>>>> }
>>>>
>>>> + if (drvdata->xgm_led) {
>>>> + ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
>>>> + if (ret) {
>>>> + hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
>>>> + goto asus_resume_err;
>>>> + }
>>>> + }
>>>> +
>>>> asus_resume_err:
>>>> return ret;
>>>> }
>>>> @@ -1310,6 +1343,40 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>>> }
>>>> }
>>>>
>>>> + if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
>>>> + ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
>>>> + (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
>>>> + drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
>>>> + if (drvdata->xgm_led == NULL) {
>>>> + ret = -ENOMEM;
>>>> + goto err_stop_hw;
>>>> + }
>>>> + drvdata->xgm_led->hdev = hdev;
>>>> + drvdata->xgm_led->cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
>>>> + "asus:xgm-%s:led",
>>>> + strlen(hdev->uniq) ?
>>>> + hdev->uniq : dev_name(&hdev->dev));
>>>> + drvdata->xgm_led->cdev.brightness = 1;
>>>> + drvdata->xgm_led->cdev.max_brightness = 1;
>>>> + drvdata->xgm_led->cdev.brightness_set_blocking = asus_xgm_led_set;
>>>> +
>>>> + /*
>>>> + * Sometimes after a reboot LEDs are ON, sometimes are OFF: set them to
>>>> + * what the default brightness resets when doing a cold boot.
>>>> + */
>>> I think this is set by the driver, so you should reformat the comment
>>> above, so you should trim the comment.
>>>
>>> Perhaps, "The LED state is arbitrary on boot, therefore default to the
>>> initial brightness set above". This way it does not become outdated if
>>> cdev.brightness changes.
>> yeah better spelling. I agree.
>>>> + ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
>>>> + if (ret) {
>>>> + hid_err(hdev, "Asus failed to set xgm led: %d\n", ret);
>>>> + goto err_stop_hw;
>>>> + }
>>> You already do this in asus_input_configured so you do it twice?
>>> Perhaps skip one if you end up keeping them? I think that it's better
>>> to keep this block.
>> In asus_input_configured? Will take a look in the next days. I tought
>> the other was in asus_resume since at resume they resets back...
>>> Or even better return an error in _get so that on boot it is
>>> ambiguous? I assume the leds remain to the state they had prior to the
>>> reboot? With this change, imagine a user that turned off the leds in
>>> windows, permabooted into Linux, and now has the lights always turn on
>>> during boot.
>> Cold boot sets them to ON, while rebooting keep them at what they were.
>>
>> After exiting from sleep they are always ON, but this is on an ally,
>> I don't know if on an old rog flow it's the same.
>>> Moreover, can systemd restore this or is it out of scope for its led
>>> handler? Perhaps it is an ambitious idea though, and better skipped.
>> I don't see realistic for this to fail if it was successful at probe so it
>> shouldn't matter. As for systemd restoring them it would have to
>> be informed that they changed (but there is no read-back) so either
>> way something has to happen at resume, but doing this means no
>> additional software is necessary and user preference is being
>> respected regardless of anything else.
> Ok, so xg mobile is the first generation gpu dock with the wide
> connector, which is why it is i2c. It's probably from the side port.
You are correct in the connector: it is a pci-e custom connector with
an usb-c on the side.
That's not i2c: it's usb-c and it is basically a usb-c hub with others
usb connectors on the dock, a 2.5Gbps ethernet and the controller
device.
> So, when it initializes in general it enables the leds. And it
> initializes on boot and on wake. And it has no memory.
From what I can see I can say I have no idea, if this was 100% correct
I wouldn't see leds staying OFF after a reboot since the driver
does a reinit of the device.
These devices have leds entering a blinking pattern while the main device
is in s2idle and the ACPI has specific code for these devices.
It's what egpu_enable enables: the pci-e part of the device (usb-c
part is always enabled).
These devices heavily rely on ACPI, even if they are external so I'm
trying to reduce assumptions here.
> For not setting the led, you could return an error on reads if the
> brightness has not been written yet and if it has return a cached
> value / restore on awake. You can store the brightness in the struct
> below xgm_led, e.g., xgm_led_val, and initialize it to -1. If it is
> -1, get would return the same error you did for battery capacity in
> asus-wmi
Yes, I could do that, but considering this is just a simple ON/OFF switch,
where the device has no memory (even sleeping changes the status),
three times (even on windows):
sleep enter: whatever -> blinking
sleep exit: blinking -> ON
after some milliseconds: ON -> whatever was set before
I don't see anything to be gained by not setting it in _probe:
no memory write to spare, no delayed effect.
The cost of additional code that would be the same as the default _get
implementation just to throw an error is one I can't justify.
> In that case, you would remove the init from probe. Otherwise, the
> current patch with a small tweak to the comment if you do a revision
> is fine by me
>
> Reviewed by: Antheas Kapenekakis <lkml@antheas.dev>
Alright, thank you
>>>> +
>>>> + ret = led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
>>>> + if (ret) {
>>>> + hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
>>>> + goto err_stop_hw;
>>>> + }
>>>> + }
>>>> +
>>>> /* Laptops keyboard backlight is always at 0x5a */
>>>> if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
>>>> (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
>>>> @@ -1366,6 +1433,9 @@ static void asus_remove(struct hid_device *hdev)
>>>> if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>>>> cancel_work_sync(&drvdata->fn_lock_sync_work);
>>>>
>>>> + if (drvdata->xgm_led)
>>>> + led_classdev_unregister(&drvdata->xgm_led->cdev);
>>>> +
>>>> hid_hw_stop(hdev);
>>>> }
>>>>
>>>> --
>>>> 2.47.3
>>>>
>>>>
>> Thanks,
>> Denis
>>
^ permalink raw reply
* [PATCH v3 0/8] HID: asus: security fixes and more hardware support
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli
Hi all,
I have added support for controlling the (way too bright) XG mobile
LEDs in hid-asus and added the i2c version of already supported
hardware that was probed only when it's a USB: these are two separate
features changes and are the only two that are not fixes for
pre-existing issue (see below).
Auto-review bot has spotted a bunch of pre-existing problems alongside
problems in my own code therefore at this point I am going to fix the
more problems I can and including those fixes and improvements in this
patchset.
Link: https://lore.kernel.org/all/20260612124830.984592-1-denis.benato@linux.dev/
Link: https://lore.kernel.org/all/30258f28-4036-4e99-9494-031212544dc2@linux.dev/
Best regards,
Denis Benato
Link v2: https://lore.kernel.org/all/20260612142326.1704858-1-denis.benato@linux.dev/
Changelog:
- v1
- Initial submission (as separate patches)
- v2
- HID: asus: add i2c entry for FA808UM and other TUFs
- Joined to the patchset
- HID: asus: add support for xgm led
- Use the correct callback set_brightness_blocking
- Stop hardcoding the cled name
- HID: asus: prevent wrong pointer cast
- Added
- HID: asus: mitigate possible use-after-free
- Added
- v3
- HID: asus: add support for xgm led
- check devm_kasprintf result
- HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
- Added
- HID: asus: avoid sleeping calls in atomic context
- Added
- HID: asus: cleanup keyboard listener on failure: avoid use-after-free
- Added
Denis Benato (8):
HID: asus: mitigate possible use-after-free
HID: asus: prevent wrong pointer cast
HID: asus: add support for xgm led
HID: asus: cleanup keyboard listener on failure: avoid use-after-free
HID: asus: avoid sleeping calls in atomic context
HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
HID: asus: add i2c entry for FA808UM and other TUFs
HID: asus: remove unnecessary OOM message
drivers/hid/hid-asus.c | 183 +++++++++++++++++++++++++++++++++++++----
1 file changed, 165 insertions(+), 18 deletions(-)
--
2.47.3
^ permalink raw reply
* [PATCH v3 1/8] HID: asus: mitigate possible use-after-free
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sashiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
The kbd_backlight pointer is not being set to NULL after being freed in
asus_kbd_register_leds and other portions of the driver checks that field,
therefore set the pointer to NULL after being freed.
Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/
Suggested-by: sashiko-bot@kernel.org
Reviewed-by: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d34d74df3dc0..4d7110b892bd 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -777,6 +777,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (ret < 0) {
/* No need to have this still around */
devm_kfree(&hdev->dev, drvdata->kbd_backlight);
+ drvdata->kbd_backlight = NULL;
}
return ret;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 2/8] HID: asus: prevent wrong pointer cast
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sashiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
The firmware version check for ROG ally devices assumes the parent device
is a USB device, therefore prevent possible out-of-bounds access by using
hid_is_usb() to mitigate possible out-of-bounds access.
Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/
Fixes: 00e005c952f7 ("hid-asus: check ROG Ally MCU version and warn")
Suggested-by: sashiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 4d7110b892bd..323dc0b7f3ff 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -753,7 +753,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
return ret;
}
- if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ if (hid_is_usb(hdev) && (drvdata->quirks & QUIRK_ROG_ALLY_XPAD)) {
intf = to_usb_interface(hdev->dev.parent);
udev = interface_to_usbdev(intf);
validate_mcu_fw_version(hdev,
--
2.47.3
^ permalink raw reply related
* [PATCH v3 3/8] HID: asus: add support for xgm led
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.
Reviewed by: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 323dc0b7f3ff..0a6c97155549 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -51,6 +51,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define FEATURE_KBD_LED_REPORT_ID1 0x5d
#define FEATURE_KBD_LED_REPORT_ID2 0x5e
+#define ROG_XGM_REPORT_SIZE 300
+
#define ROG_ALLY_REPORT_SIZE 64
#define ROG_ALLY_X_MIN_MCU 313
#define ROG_ALLY_MIN_MCU 319
@@ -118,6 +120,11 @@ struct asus_kbd_leds {
bool removed;
};
+struct asus_xgm_led {
+ struct led_classdev cdev;
+ struct hid_device *hdev;
+};
+
struct asus_touchpad_info {
int max_x;
int max_y;
@@ -143,6 +150,7 @@ struct asus_drvdata {
unsigned long battery_next_query;
struct work_struct fn_lock_sync_work;
bool fn_lock;
+ struct asus_xgm_led *xgm_led;
};
static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -941,6 +949,23 @@ static int asus_battery_probe(struct hid_device *hdev)
return ret;
}
+static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+ const u8 buf[ROG_XGM_REPORT_SIZE] = {
+ FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
+ };
+ struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
+ int ret;
+
+ ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
+ if (ret != ROG_XGM_REPORT_SIZE) {
+ hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
{
struct input_dev *input = hi->input;
@@ -1169,6 +1194,51 @@ static int asus_start_multitouch(struct hid_device *hdev)
return 0;
}
+static int asus_xgm_init(struct hid_device *hdev, struct asus_drvdata *drvdata)
+{
+ const char *name;
+ int ret;
+
+ drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
+ if (drvdata->xgm_led == NULL)
+ return -ENOMEM;
+
+ name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "asus:xgm-%s:led",
+ strlen(hdev->uniq) ? hdev->uniq : dev_name(&hdev->dev));
+
+ if (name == NULL) {
+ ret = -ENOMEM;
+ goto asus_xgm_init_err;
+ }
+
+ drvdata->xgm_led->hdev = hdev;
+ drvdata->xgm_led->cdev.name = name;
+ drvdata->xgm_led->cdev.brightness = 1;
+ drvdata->xgm_led->cdev.max_brightness = 1;
+ drvdata->xgm_led->cdev.brightness_set_blocking = asus_xgm_led_set;
+
+ /* LED state is arbitrary on boot, set a default */
+ ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+ if (ret) {
+ hid_err(hdev, "Asus failed to set xgm led: %d\n", ret);
+ goto asus_xgm_init_led_err;
+ }
+
+ ret = led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
+ if (ret) {
+ hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
+ goto asus_xgm_init_led_err;
+ }
+
+ return 0;
+asus_xgm_init_led_err:
+ devm_kfree(&hdev->dev, name);
+asus_xgm_init_err:
+ devm_kfree(&hdev->dev, drvdata->xgm_led);
+ drvdata->xgm_led = NULL;
+ return ret;
+}
+
static int __maybe_unused asus_resume(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -1184,6 +1254,14 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
}
}
+ if (drvdata->xgm_led) {
+ ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+ if (ret) {
+ hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
+ goto asus_resume_err;
+ }
+ }
+
asus_resume_err:
return ret;
}
@@ -1310,6 +1388,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
}
+ if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
+ ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
+ (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
+ ret = asus_xgm_init(hdev, drvdata);
+ if (ret) {
+ hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
+ goto err_stop_hw;
+ }
+ }
+
/* Laptops keyboard backlight is always at 0x5a */
if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
(asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
@@ -1366,6 +1454,9 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->quirks & QUIRK_HID_FN_LOCK)
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ if (drvdata->xgm_led)
+ led_classdev_unregister(&drvdata->xgm_led->cdev);
+
hid_hw_stop(hdev);
}
--
2.47.3
^ permalink raw reply related
* [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sahiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
asus_kbd_register_leds(), I noticed it registers a listener to a global list
and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
fails the driver returns without unregistering the listener, and the devres
subsystem will automatically free the memory, leaving a dangling pointer
in the global list.
Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 0a6c97155549..f38b18ad65c6 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (drvdata->tp) {
ret = asus_start_multitouch(hdev);
if (ret)
- goto err_stop_hw;
+ goto err_unregister_backlight;
}
}
return 0;
+err_unregister_backlight:
+ if (drvdata->kbd_backlight) {
+ asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
+ devm_kfree(&hdev->dev, drvdata->kbd_backlight);
+ drvdata->kbd_backlight = NULL;
+ }
err_stop_hw:
hid_hw_stop(hdev);
return ret;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sahiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
asus_raw_event() that is called in atomic context.
This commit changes when methods are called, not method parameters:
the driver behaves as before.
Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
1 file changed, 56 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f38b18ad65c6..a6467172c455 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -150,6 +150,13 @@ struct asus_drvdata {
unsigned long battery_next_query;
struct work_struct fn_lock_sync_work;
bool fn_lock;
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ struct work_struct wmi_work;
+ bool wmi_work_disabled;
+ u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
+ int wmi_size;
+ spinlock_t wmi_lock;
+#endif
struct asus_xgm_led *xgm_led;
};
@@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
return 0;
}
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
/*
* Send events to asus-wmi driver for handling special keys
*/
@@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
return 0;
}
+static void asus_wmi_work(struct work_struct *work)
+{
+ struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
+ u8 report_data[FEATURE_KBD_REPORT_SIZE];
+ int report_size;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&drvdata->wmi_lock, flags);
+ memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
+ report_size = drvdata->wmi_size;
+ spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
+
+ ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
+ if (ret) {
+ if (ret != -ENODEV)
+ hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
+
+ /* Fallback: pass the raw event to the HID core */
+ hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
+ report_data, report_size,
+ report_size, 1);
+ }
+}
+#endif
+
static int asus_event(struct hid_device *hdev, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
@@ -397,6 +431,9 @@ static int asus_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data, int size)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ unsigned long flags;
+#endif
if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
return asus_report_battery(drvdata, data, size);
@@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
* pass to userspace so it can implement its own fan control.
*/
if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
- int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
-
- if (ret == 0) {
- /* Successfully handled by asus-wmi, block event */
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ spin_lock_irqsave(&drvdata->wmi_lock, flags);
+ memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
+ drvdata->wmi_size = size;
+ spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
+
+ if (!drvdata->wmi_work_disabled) {
+ schedule_work(&drvdata->wmi_work);
+ /* Successfully handled asynchronously, block event */
return -1;
}
-
- /*
- * Warn if asus-wmi failed (but not if it's unavailable).
- * Let the event reach userspace in all failure cases.
- */
- if (ret != -ENODEV)
- hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
+#endif
}
/*
@@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
drvdata->hdev = hdev;
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
+ spin_lock_init(&drvdata->wmi_lock);
+#endif
if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
ret = asus_battery_probe(hdev);
@@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->quirks & QUIRK_HID_FN_LOCK)
cancel_work_sync(&drvdata->fn_lock_sync_work);
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ drvdata->wmi_work_disabled = true;
+ cancel_work_sync(&drvdata->wmi_work);
+#endif
+
if (drvdata->xgm_led)
led_classdev_unregister(&drvdata->xgm_led->cdev);
--
2.47.3
^ permalink raw reply related
* [PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sahiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
schedule_work() after the work was supposedly canceled.
Once the driver is removed and drvdata is freed by devres, the workqueue
could execute asus_sync_fn_lock() and dereference the freed drvdata.
Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a6467172c455..a65a28137f0d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -148,6 +148,7 @@ struct asus_drvdata {
int battery_stat;
bool battery_in_query;
unsigned long battery_next_query;
+ bool fn_lock_sync_work_disabled;
struct work_struct fn_lock_sync_work;
bool fn_lock;
#if IS_REACHABLE(CONFIG_ASUS_WMI)
@@ -416,7 +417,7 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
case KEY_KBDILLUMTOGGLE:
return !asus_hid_event(ASUS_EV_BRTTOGGLE);
case KEY_FN_ESC:
- if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+ if (!drvdata->fn_lock_sync_work_disabled && (drvdata->quirks & QUIRK_HID_FN_LOCK)) {
drvdata->fn_lock = !drvdata->fn_lock;
schedule_work(&drvdata->fn_lock_sync_work);
}
@@ -1497,8 +1498,10 @@ static void asus_remove(struct hid_device *hdev)
cancel_work_sync(&drvdata->kbd_backlight->work);
}
- if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+ if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+ drvdata->fn_lock_sync_work_disabled = true;
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ }
#if IS_REACHABLE(CONFIG_ASUS_WMI)
drvdata->wmi_work_disabled = true;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 7/8] HID: asus: add i2c entry for FA808UM and other TUFs
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
On newer TUF laptops the keyboard HID device uses the same PID/VID of a
USB device that was found in ROG laptops: add it to hid-asus as i2c too.
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a65a28137f0d..efad6b623e65 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1646,6 +1646,9 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
--
2.47.3
^ permalink raw reply related
* [PATCH v3 8/8] HID: asus: remove unnecessary OOM message
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
If devm_kzalloc fails an allocation error is already being reported:
there is no need for the driver to repeat it, moreover if this were
new code checkpatch.pl would throw a warning.
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index efad6b623e65..f1fe3358a298 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1322,10 +1322,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
- if (drvdata == NULL) {
- hid_err(hdev, "Can't alloc Asus descriptor\n");
+ if (drvdata == NULL)
return -ENOMEM;
- }
hid_set_drvdata(hdev, drvdata);
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 3/8] HID: asus: add support for xgm led
From: sashiko-bot @ 2026-06-13 15:42 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-4-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing led_classdev_unregister() in the probe error path leads to a use-after-free.
- [Medium] An error during keyboard backlight resume causes the XGM LED resume to be skipped.
- [Medium] Returning a positive short transfer length from brightness_set_blocking instead of a negative error code.
--
commit 201b953e7851d7aae6a4427d79499eefd3b7b433
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for xgm led
XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 52d755659d951..95e7dc6a447d8 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -941,6 +949,23 @@ static int asus_battery_probe(struct hid_device *hdev)
> return ret;
> }
>
> +static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
> +{
> + const u8 buf[ROG_XGM_REPORT_SIZE] = {
> + FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
> + };
> + struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
> + int ret;
> +
> + ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
> + if (ret != ROG_XGM_REPORT_SIZE) {
> + hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
> + return ret;
[Severity: Medium]
If a short transfer occurs, asus_kbd_set_report() will return a positive
number less than ROG_XGM_REPORT_SIZE.
Will returning this positive value cause the LED subsystem API to incorrectly
interpret the short transfer as a success instead of a failure?
> + }
> +
> + return 0;
> +}
> +
[ ... ]
> @@ -1178,6 +1248,14 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
> }
> }
>
[Severity: Medium]
If restoring the keyboard backlight fails earlier in asus_resume(), the
early return via goto asus_resume_err causes this XGM LED resume logic
to be skipped.
Should the device resume function try to resume as many independent
components as possible instead of unconditionally aborting here?
> + if (drvdata->xgm_led) {
> + ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
> + if (ret) {
> + hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
> + goto asus_resume_err;
> + }
> + }
> +
> asus_resume_err:
> return ret;
> }
[ ... ]
> @@ -1304,6 +1382,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
> }
>
> + if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
> + ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
> + (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
> + ret = asus_xgm_init(hdev, drvdata);
[Severity: High]
If asus_probe() encounters an error later on (such as asus_start_multitouch()
failing), the error handling path does not unregister the LED class device.
Because drvdata->xgm_led was allocated with devm_kzalloc(), devres will
free it. Does this lead to a use-after-free if userspace interacts with the
sysfs interface of the leaked cdev structure?
Would using devm_led_classdev_register() in asus_xgm_init() instead of
led_classdev_register() cleanly avoid this issue?
> + if (ret) {
> + hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
> + goto err_stop_hw;
> + }
> + }
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=3
^ permalink raw reply
* Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free
From: sashiko-bot @ 2026-06-13 15:42 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-5-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The newly added `err_unregister_backlight` error path frees `drvdata->kbd_backlight` without canceling its associated workqueue, leading to a Use-After-Free.
- [High] The `asus_probe` error path fails to unregister the dynamically registered `xgm_led` class device, leaving a dangling pointer in the LED subsystem.
Pre-existing issues:
- [High] The `asus_probe` error paths fail to cancel `fn_lock_sync_work` and `wmi_work`, creating a potential Use-After-Free if the works are scheduled during probe.
--
commit 6e04c2967d627dea02497ed7471439a5d35c56bc
Author: Denis Benato <denis.benato@linux.dev>
Subject: HID: asus: cleanup keyboard listener on failure: avoid use-after-free
This commit fixes a potential use-after-free by unregistering the keyboard
listener on probe failure. It explicitly adds an error path to unregister
the listener and free the memory when a subsequent initialization step fails.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 95e7dc6a447d8..e3a03f7cf77d0 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1420,11 +1420,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (drvdata->tp) {
> ret = asus_start_multitouch(hdev);
> if (ret)
> - goto err_stop_hw;
> + goto err_unregister_backlight;
> }
> }
>
> return 0;
> +err_unregister_backlight:
> + if (drvdata->kbd_backlight) {
> + asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> + devm_kfree(&hdev->dev, drvdata->kbd_backlight);
[Severity: High]
Does this code leave pending work items referencing freed memory?
If concurrent events have queued `drvdata->kbd_backlight->work` (e.g.,
via `asus_kbd_backlight_set()` or `asus_hid_event()`), freeing
`kbd_backlight` without synchronizing via `cancel_work_sync()` might
cause a use-after-free when the workqueue executes.
> + drvdata->kbd_backlight = NULL;
> + }
> err_stop_hw:
> hid_hw_stop(hdev);
> return ret;
[Severity: High]
Could failing to unregister `xgm_led` leave a dangling pointer in the LED
subsystem?
If `asus_xgm_init()` succeeds, it registers `drvdata->xgm_led->cdev` using
`led_classdev_register()`, which is not devres-managed. If `asus_probe()`
subsequently fails and jumps here, the code exits without calling
`led_classdev_unregister()`. Since `drvdata->xgm_led` is devres-managed, it
will be freed, but the LED subsystem will retain a pointer to it,
potentially causing a use-after-free.
[Severity: High]
This is a pre-existing issue, but does this error path also need to cancel
`fn_lock_sync_work` and `wmi_work`?
Once the hardware is started via `hid_hw_start()`, incoming input events
can schedule `drvdata->fn_lock_sync_work` or `drvdata->wmi_work`. If
`asus_probe()` fails and exits here, devres will automatically free
`drvdata`. Since `cancel_work_sync()` is omitted from this error path, any
queued work could execute asynchronously after `drvdata` is freed, leading
to a use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=4
^ permalink raw reply
* Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context
From: sashiko-bot @ 2026-06-13 15:43 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-6-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unbounded wmi_size assignment causes a stack buffer overflow and out-of-bounds read in asus_wmi_work().
- [High] Asynchronous invocation of hid_report_raw_event without acquiring driver_input_lock causes a data race.
- [High] Missing cancel_work_sync in asus_probe error paths leaves wmi_work pending, causing a Use-After-Free.
- [Medium] Passing report_size as bufsize to hid_report_raw_event silently drops valid short reports.
--
commit 59c560d9da130f0e811e0abb99b0edf58eef0925
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: avoid sleeping calls in atomic context
Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
asus_raw_event() that is called in atomic context.
This commit changes when methods are called, not method parameters:
the driver behaves as before.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e3a03f7cf77d0..a3437789b2341 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
> return 0;
> }
>
> +static void asus_wmi_work(struct work_struct *work)
> +{
> + struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
> + u8 report_data[FEATURE_KBD_REPORT_SIZE];
> + int report_size;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
> + memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
[Severity: High]
Can this memcpy cause a stack buffer overflow?
In asus_raw_event(), drvdata->wmi_size is assigned the untrusted raw event
size without being clipped to the maximum buffer size. When the asynchronous
worker processes this, it uses that unclipped size as the length for memcpy
into the fixed 64-byte report_data stack array.
A malicious or malfunctioning USB device sending a report larger than
64 bytes could overflow the stack here.
> + report_size = drvdata->wmi_size;
> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> + ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> + if (ret) {
> + if (ret != -ENODEV)
> + hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
> +
> + /* Fallback: pass the raw event to the HID core */
> + hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
> + report_data, report_size,
> + report_size, 1);
[Severity: High]
Is it safe to call hid_report_raw_event() asynchronously without acquiring
hdev->driver_input_lock?
The HID core expects callers of hid_report_raw_event() to hold this lock
to ensure exclusive access when modifying shared state within the HID report
structure. If a concurrent hardware interrupt triggers __hid_input_report(),
it could successfully acquire the lock and mutate the HID report state at the
same time, potentially leading to data corruption or crashes.
[Severity: Medium]
Does passing report_size as the bufsize parameter prevent the HID core from
safely handling short reports?
The HID core requires bufsize to be at least the expected size of the report
so it can safely zero-pad short reports. Because bufsize is constrained here
to the actual payload size, the safety check inside hid_report_raw_event()
might fail, causing valid short reports to be permanently dropped with an
EINVAL error.
> + }
> +}
> +#endif
> +
[ ... ]
> @@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
> * pass to userspace so it can implement its own fan control.
> */
> if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
> - int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> -
> - if (ret == 0) {
> - /* Successfully handled by asus-wmi, block event */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
> + memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
> + drvdata->wmi_size = size;
[Severity: High]
Should drvdata->wmi_size be capped to the size of the buffer?
While the memcpy directly above safely clips the copy using min_t(),
drvdata->wmi_size is assigned the unclipped size, leading to the stack buffer
overflow when it is consumed in asus_wmi_work().
> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> + if (!drvdata->wmi_work_disabled) {
> + schedule_work(&drvdata->wmi_work);
> + /* Successfully handled asynchronously, block event */
> return -1;
> }
[ ... ]
> @@ -1344,6 +1380,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>
> drvdata->hdev = hdev;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
> + spin_lock_init(&drvdata->wmi_lock);
> +#endif
[Severity: High]
Could wmi_work trigger a use-after-free if probe fails during later stages?
In asus_probe(), after wmi_work is initialized, hid_hw_start() is called which
enables hardware interrupts. At this point, incoming events can queue wmi_work.
If a subsequent initialization step fails (for example, asus_xgm_init()),
the function jumps to the err_stop_hw label:
err_stop_hw:
hid_hw_stop(hdev);
return ret;
This returns an error without calling cancel_work_sync(&drvdata->wmi_work).
The workqueue subsystem guidelines state that once a work struct is queued,
shutdown methods must prevent leaking it. The drvdata memory will be freed
by devres, and any pending wmi_work will execute on freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=5
^ 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