* [PATCH v2 10/10] arm64: dts: renesas: r9a08g046l48-smarc: Add USB2.0 support
From: Biju @ 2026-06-16 10:44 UTC (permalink / raw)
To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260616104459.410743-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Enable USB2.0 on the RZ/G3L SMARC board (r9a08g046l48-smarc).
Port 0 (ehci0, ohci0, usb2_phy0) is configured as OTG with
usb0_pins pinmux (USB20_OVRCUR, USB20_VBUSEN) and usb0_vbus_otg
as the VBUS supply. Port 1 (ehci1, ohci1, usb2_phy1) is configured
as host-only with usb1_pins pinmux (USB21_OVRCUR, USB21_VBUSEN),
usb1_vbus_otg as the VBUS supply, and renesas,no-otg-pins set to
indicate no OTG pin routing. The phyrst USB PHY reset controller is
also enabled.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Updated commit description.
---
.../boot/dts/renesas/r9a08g046l48-smarc.dts | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
index 96cc7ee46a6a..b189ae8e808d 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
@@ -87,6 +87,16 @@ vqmmc_sd1_pvdd: regulator-vqmmc-sd1-pvdd {
#endif
};
+&ehci0 {
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&ehci1 {
+ dr_mode = "host";
+ status = "okay";
+};
+
&i2c2 {
pinctrl-0 = <&i2c2_pins>;
pinctrl-names = "default";
@@ -138,6 +148,20 @@ &keys {
#endif
};
+&ohci0 {
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&ohci1 {
+ dr_mode = "host";
+ status = "okay";
+};
+
+&phyrst {
+ status = "okay";
+};
+
&pinctrl {
audio_clk_pins: audio-clock {
pinmux = <RZG3L_PORT_PINMUX(H, 4, 6)>, /* AUDIO_CLK_B */
@@ -259,6 +283,16 @@ ssi0_pins: ssi0 {
<RZG3L_PORT_PINMUX(H, 2, 9)>, /* SSIF0_RCK */
<RZG3L_PORT_PINMUX(H, 3, 9)>; /* SSIF0_TXD */
};
+
+ usb0_pins: usb0 {
+ pinmux = <RZG3L_PORT_PINMUX(3, 0, 12)>, /* USB20_OVRCUR */
+ <RZG3L_PORT_PINMUX(3, 1, 12)>; /* USB20_VBUSEN */
+ };
+
+ usb1_pins: usb1 {
+ pinmux = <RZG3L_PORT_PINMUX(3, 4, 12)>, /* USB21_OVRCUR */
+ <RZG3L_PORT_PINMUX(3, 5, 12)>; /* USB21_VBUSEN */
+ };
};
#if SW_SER0_PMOD
@@ -330,3 +364,18 @@ &ssi0 {
status = "okay";
};
#endif
+
+&usb2_phy0 {
+ pinctrl-0 = <&usb0_pins>;
+ pinctrl-names = "default";
+ vbus-supply = <&usb0_vbus_otg>;
+ status = "okay";
+};
+
+&usb2_phy1 {
+ pinctrl-0 = <&usb1_pins>;
+ pinctrl-names = "default";
+ vbus-supply = <&usb1_vbus_otg>;
+ renesas,no-otg-pins;
+ status = "okay";
+};
--
2.43.0
^ permalink raw reply related
* [PATCH v2 09/10] arm64: dts: renesas: r9a08g046: Add USB2.0 device nodes
From: Biju @ 2026-06-16 10:44 UTC (permalink / raw)
To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260616104459.410743-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Add USB2.0 device nodes to the RZ/G3L (r9a08g046) SoC DTSI, covering
the USB PHY controller, OHCI/EHCI host controllers, and USB2 PHYs for
both ports.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Updated commit description.
* Added regulators group node and its children.
---
arch/arm64/boot/dts/renesas/r9a08g046.dtsi | 105 +++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
index 85e409ac8d5c..926a81cec37e 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
@@ -1198,6 +1198,111 @@ queue3 {
};
};
+ phyrst: usbphy-ctrl@11e00000 {
+ compatible = "renesas,r9a08g046-usbphy-ctrl";
+ reg = <0 0x11e00000 0 0x10000>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>;
+ resets = <&cpg R9A08G046_USB_PRESETN>;
+ power-domains = <&cpg>;
+ #reset-cells = <1>;
+ renesas,sysc-pwrrdy = <&sysc 0xd70 0x1>;
+ status = "disabled";
+
+ regulators {
+ usb0_vbus_otg: vbus0 {
+ regulator-name = "usb0_vbus";
+ };
+
+ usb1_vbus_otg: vbus1 {
+ regulator-name = "usb1_vbus";
+ };
+ };
+ };
+
+ ohci0: usb@11e10000 {
+ compatible = "generic-ohci";
+ reg = <0 0x11e10000 0 0x100>;
+ interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
+ <&cpg CPG_MOD R9A08G046_USB_U2H0_HCLK>;
+ resets = <&phyrst 0>,
+ <&cpg R9A08G046_USB_U2H0_HRESETN>;
+ phys = <&usb2_phy0 1>;
+ phy-names = "usb";
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
+ ohci1: usb@11e90000 {
+ compatible = "generic-ohci";
+ reg = <0 0x11e90000 0 0x100>;
+ interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
+ <&cpg CPG_MOD R9A08G046_USB_U2H1_HCLK>;
+ resets = <&phyrst 1>,
+ <&cpg R9A08G046_USB_U2H1_HRESETN>;
+ phys = <&usb2_phy1 1>;
+ phy-names = "usb";
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
+ ehci0: usb@11e10100 {
+ compatible = "generic-ehci";
+ reg = <0 0x11e10100 0 0x100>;
+ interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
+ <&cpg CPG_MOD R9A08G046_USB_U2H0_HCLK>;
+ resets = <&phyrst 0>,
+ <&cpg R9A08G046_USB_U2H0_HRESETN>;
+ phys = <&usb2_phy0 2>;
+ phy-names = "usb";
+ companion = <&ohci0>;
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
+ ehci1: usb@11e90100 {
+ compatible = "generic-ehci";
+ reg = <0 0x11e90100 0 0x100>;
+ interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
+ <&cpg CPG_MOD R9A08G046_USB_U2H1_HCLK>;
+ resets = <&phyrst 1>,
+ <&cpg R9A08G046_USB_U2H1_HRESETN>;
+ phys = <&usb2_phy1 2>;
+ phy-names = "usb";
+ companion = <&ohci1>;
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
+ usb2_phy0: usb-phy@11e10200 {
+ compatible = "renesas,usb2-phy-r9a08g046";
+ reg = <0 0x11e10200 0 0x700>;
+ interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
+ <&cpg CPG_MOD R9A08G046_USB_U2H0_HCLK>;
+ resets = <&phyrst 0>,
+ <&cpg R9A08G046_USB_U2H0_HRESETN>;
+ #phy-cells = <1>;
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
+ usb2_phy1: usb-phy@11e90200 {
+ compatible = "renesas,usb2-phy-r9a08g046";
+ reg = <0 0x11e90200 0 0x700>;
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
+ <&cpg CPG_MOD R9A08G046_USB_U2H1_HCLK>;
+ resets = <&phyrst 1>,
+ <&cpg R9A08G046_USB_U2H1_HRESETN>;
+ #phy-cells = <1>;
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
pcie: pcie@11e40000 {
reg = <0 0x11e40000 0 0x10000>;
ranges = <0x02000000 0 0x30000000 0 0x30000000 0 0x08000000>;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 02/10] dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings
From: Biju @ 2026-06-16 10:44 UTC (permalink / raw)
To: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Neil Armstrong, Yoshihiro Shimoda, linux-phy,
devicetree, linux-kernel, linux-renesas-soc,
Prabhakar Mahadev Lad, Biju Das, Krzysztof Kozlowski
In-Reply-To: <20260616104459.410743-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Add device tree binding support for the RZ/G3L (r9a08g046) USB2 PHY.
The RZ/G3L USB PHY is almost identical to the RZ/G3S USB PHY, the
difference being 2 OTG blocks on RZ/G3L compared to 1 on RZ/G3S.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Collected tag.
---
Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml b/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
index 9740e5b335f9..d6b9d08ceec6 100644
--- a/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml
@@ -16,6 +16,7 @@ properties:
- enum:
- renesas,usb2-phy-r8a77470 # RZ/G1C
- renesas,usb2-phy-r9a08g045 # RZ/G3S
+ - renesas,usb2-phy-r9a08g046 # RZ/G3L
- renesas,usb2-phy-r9a09g057 # RZ/V2H(P)
- items:
@@ -132,6 +133,7 @@ allOf:
enum:
- renesas,usb2-phy-r9a09g057
- renesas,usb2-phy-r9a08g045
+ - renesas,usb2-phy-r9a08g046
- renesas,rzg2l-usb2-phy
then:
properties:
--
2.43.0
^ permalink raw reply related
* [PATCH v2 01/10] dt-bindings: reset: renesas,rzg2l-usbphy-ctrl: Document RZ/G3L support
From: Biju @ 2026-06-16 10:44 UTC (permalink / raw)
To: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, devicetree, linux-kernel, linux-renesas-soc,
Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260616104459.410743-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Document Renesas RZ/G3L (r9a08g046) USB PHY controller bindings.
The RZ/G3L USB PHY block is similar to RZ/G3S but differs in that each
port has its own OTG controller, whereas RZ/G3S only has one on port 1.
To reflect this, RZ/G3L uses a regulators sub-node with per-port vbus0
and vbus1 entries instead of the single regulator-vbus property used
by other compatible SoCs.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Updated commit description.
* Added enum instead of const in the compatible section.
* Dropped regulator1-vbus and added a regulators group node.
* Updated schema check.
---
.../reset/renesas,rzg2l-usbphy-ctrl.yaml | 49 +++++++++++++++++--
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.yaml b/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.yaml
index c83469a1b379..12da48d069e5 100644
--- a/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.yaml
+++ b/Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.yaml
@@ -16,13 +16,17 @@ description:
properties:
compatible:
oneOf:
+ - items:
+ - enum:
+ - renesas,r9a08g045-usbphy-ctrl # RZ/G3S
+ - renesas,r9a08g046-usbphy-ctrl # RZ/G3L
+
- items:
- enum:
- renesas,r9a07g043-usbphy-ctrl # RZ/G2UL and RZ/Five
- renesas,r9a07g044-usbphy-ctrl # RZ/G2{L,LC}
- renesas,r9a07g054-usbphy-ctrl # RZ/V2L
- const: renesas,rzg2l-usbphy-ctrl
- - const: renesas,r9a08g045-usbphy-ctrl # RZ/G3S
reg:
maxItems: 1
@@ -50,6 +54,28 @@ properties:
$ref: /schemas/regulator/regulator.yaml#
unevaluatedProperties: false
+ regulators:
+ type: object
+ description:
+ list of vbus regulators provided by this controller.
+
+ properties:
+ vbus0:
+ type: object
+ description: Port 1 USB VBUS regulator
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+
+ vbus1:
+ type: object
+ description: Port 2 USB VBUS regulator
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+
+ required:
+ - vbus0
+ - vbus1
+
renesas,sysc-pwrrdy:
description:
The system controller PWRRDY indicates to the USB PHY if the power supply
@@ -71,14 +97,15 @@ required:
- resets
- power-domains
- '#reset-cells'
- - regulator-vbus
allOf:
- if:
properties:
compatible:
contains:
- const: renesas,r9a08g045-usbphy-ctrl
+ enum:
+ - renesas,r9a08g045-usbphy-ctrl
+ - renesas,r9a08g046-usbphy-ctrl
then:
required:
- renesas,sysc-pwrrdy
@@ -86,6 +113,22 @@ allOf:
properties:
renesas,sysc-pwrrdy: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,r9a08g046-usbphy-ctrl
+ then:
+ properties:
+ regulator-vbus: false
+ required:
+ - regulators
+ else:
+ properties:
+ regulators: false
+ required:
+ - regulator-vbus
+
additionalProperties: false
examples:
--
2.43.0
^ permalink raw reply related
* [PATCH v2 00/10] Add RZ/G3L USB2.0 host support
From: Biju @ 2026-06-16 10:44 UTC (permalink / raw)
To: Philipp Zabel, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Michael Turquette, Stephen Boyd,
Liam Girdwood, Mark Brown, Magnus Damm
Cc: Biju Das, Neil Armstrong, Yoshihiro Shimoda, linux-phy,
devicetree, linux-kernel, linux-clk, linux-renesas-soc,
Prabhakar Mahadev Lad, Biju Das
From: Biju Das <biju.das.jz@bp.renesas.com>
Add device tree binding support for the RZ/G3L (r9a08g046) USB PHY
controller. The RZ/G3L USB PHY block is similar to RZ/G3S, but each port
has an OTG controller, unlike RZ/G3S, which has an OTG controller only on
port 1.
v1->v2:
* Dropped patch#6(Introduce helper for regulator registration)
* Passing pointer to an array of regulators to make it scalable.
* Dropped regulator1-vbus and added a regulators group node.
* Updated commit description for patch#1,#4,#6,#7,#8,#9 and #10.
* Added enum instead of const in the compatible section.
* Updated schema check.
* Collected tag for PHY binding patch.
in the regulator driver.
* Added regulators group node and its children in SoC dtsi.
Biju Das (10):
dt-bindings: reset: renesas,rzg2l-usbphy-ctrl: Document RZ/G3L support
dt-bindings: phy: renesas,usb2-phy: Document RZ/G3L PHY bindings
clk: renesas: r9a08g046: Add USB2.0 clock and reset entries
reset: rzg2l-usbphy-ctrl: Introduce info struct for match data
reset: rzg2l-usbphy-ctrl: Add RZ/G3L support
regulator: renesas-usb-vbus-regulator: Add RZ/G3L VBUS regulator
support
phy: renesas: phy-rcar-gen3-usb2: Add RZ/G3L support
phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for
disabled VBUS regulator
arm64: dts: renesas: r9a08g046: Add USB2.0 device nodes
arm64: dts: renesas: r9a08g046l48-smarc: Add USB2.0 support
.../bindings/phy/renesas,usb2-phy.yaml | 2 +
.../reset/renesas,rzg2l-usbphy-ctrl.yaml | 49 +++++++-
arch/arm64/boot/dts/renesas/r9a08g046.dtsi | 105 ++++++++++++++++++
.../boot/dts/renesas/r9a08g046l48-smarc.dts | 49 ++++++++
drivers/clk/renesas/r9a08g046-cpg.c | 15 +++
drivers/phy/renesas/phy-rcar-gen3-usb2.c | 20 ++--
.../regulator/renesas-usb-vbus-regulator.c | 53 +++++++++
drivers/reset/reset-rzg2l-usbphy-ctrl.c | 44 +++++---
8 files changed, 312 insertions(+), 25 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()
From: Andy Shevchenko @ 2026-06-16 10:43 UTC (permalink / raw)
To: rodrigo.alencar
Cc: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Kees Cook, Gustavo A. R. Silva,
Maxwell Doose, Joshua Crofts
In-Reply-To: <20260616-ad5686-new-features-v3-11-f829fb7e9262@analog.com>
On Tue, Jun 16, 2026 at 09:21:17AM +0100, Rodrigo Alencar via B4 Relay wrote:
> Use guarded mutex lock to facilitate code review when adding new
> attributes. This will allow for early returns, avoiding error-prone
> locking and unlocking in error paths. This also adds missing include
> linux/cleanup.h. Gain-control support will allow the scale attribute
> to be configurable.
What about interrupt handler? You just added yet another mutex lock/unlock
there. Perhaps this patch should be done before the previous one?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 10/12] iio: dac: ad5686: add triggered buffer support
From: Andy Shevchenko @ 2026-06-16 10:42 UTC (permalink / raw)
To: rodrigo.alencar
Cc: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Kees Cook, Gustavo A. R. Silva
In-Reply-To: <20260616-ad5686-new-features-v3-10-f829fb7e9262@analog.com>
On Tue, Jun 16, 2026 at 09:21:16AM +0100, Rodrigo Alencar via B4 Relay wrote:
> Implement trigger handler by leveraging the LDAC gpio to update all DAC
> channels at once when it is available. Also, the multiple channel writes
> can be flushed at once with the sync() operation.
...
> +static irqreturn_t ad5686_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct iio_buffer *buffer = indio_dev->buffer;
> + struct ad5686_state *st = iio_priv(indio_dev);
> + u16 val[AD5686_MAX_CHANNELS] = { };
> + unsigned int scan_count;
> + int ret, ch, i = 0;
Decouple assignment and definition. Also do 'i' and 'ch' need to be signed?
> + bool async_update;
> + u8 cmd;
> +
> + ret = iio_pop_from_buffer(buffer, val);
> + if (ret)
> + goto out_notify_done;
> +
> + mutex_lock(&st->lock);
> +
> + scan_count = bitmap_weight(indio_dev->active_scan_mask,
> + iio_get_masklength(indio_dev));
> + async_update = st->ldac_gpio && scan_count > 1;
> + if (async_update) {
> + /* use LDAC to update all channels simultaneously */
> + cmd = AD5686_CMD_WRITE_INPUT_N;
> + gpiod_set_value_cansleep(st->ldac_gpio, 0);
> + } else {
> + cmd = AD5686_CMD_WRITE_INPUT_N_UPDATE_N;
> + }
> +
> + iio_for_each_active_channel(indio_dev, ch) {
> + ret = st->ops->write(st, cmd, indio_dev->channels[ch].address, val[i++]);
> + if (ret)
> + break;
> + }
> +
> + /*
> + * If sync() is available, it is called here regardless of write
> + * failure to allow bus implementation to reset. In that case, partial
> + * writes are unlikely as the write operations would just queue up
> + * the transfers.
> + */
> + if (st->ops->sync)
> + ret = st->ops->sync(st); /* flush all pending transfers */
'ret' is set but not used. Always compile your code with `make W=1` and both
GCC and clang.
> + if (async_update)
> + gpiod_set_value_cansleep(st->ldac_gpio, 1);
> +
> + mutex_unlock(&st->lock);
> +out_notify_done:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v9 1/2] dt-bindings: ufs: Document static TX Equalization settings properties
From: Peter Wang (王信友) @ 2026-06-16 10:36 UTC (permalink / raw)
To: beanhuo@micron.com, mani@kernel.org, can.guo@oss.qualcomm.com,
bvanassche@acm.org, krzk@kernel.org, martin.petersen@oracle.com
Cc: avri.altman@wdc.com, zhml@posteo.com, linux-scsi@vger.kernel.org,
robh@kernel.org, linux-kernel@vger.kernel.org, krzk+dt@kernel.org,
alim.akhtar@samsung.com, quic_rdwivedi@quicinc.com,
conor+dt@kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20260615132834.2985346-2-can.guo@oss.qualcomm.com>
On Mon, 2026-06-15 at 06:28 -0700, Can Guo wrote:
> UFS v5.0/UFSHCI v5.0 adds HS-G6 support (46.6 Gbps/lane) via UniPro
> v3.0 and M-PHY v6.0. These specs define TX Equalization for all
> High-Speed Gears (not only HS-G6) to compensate channel loss and
> improve signal integrity at high speed.
>
> For HS-G6, M-PHY uses PAM4 1b1b line coding. Pre-Coding may also be
> required depending on channel characteristics.
>
> Document vendor-neutral properties in ufs-common.yaml:
> - txeq-preshoot-g[1-6]
> - txeq-deemphasis-g[1-6]
> - tx-precode-enable-g6
>
> Values are per-lane Host/Device tuples (2 values for x1, 4 values for
> x2). PreShoot/DeEmphasis range from 0..7, and Precode is 0/1.
>
> These are board-specific signal-integrity tuning values. They depend
> on
> channel SI/PHY characterization and validation (host PHY, device PHY,
> package, and board routing), and are determined by HW/PHY designers.
>
> Although UFSHCI v5.0 supports TX Equalization Training via UniPro
> v3.0,
> which allows host software to determine optimal TX Equalization at
> runtime, static board-specific TX Equalization settings in the Device
> Tree are still necessary because:
> - TX Equalization Training is not supported for HS-G3 and below
> - TX Equalization Training is disabled on some platforms
>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
^ permalink raw reply
* Re: [PATCH v3 09/12] iio: dac: ad5686: implement new sync() op for the spi bus
From: Andy Shevchenko @ 2026-06-16 10:35 UTC (permalink / raw)
To: rodrigo.alencar
Cc: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Kees Cook, Gustavo A. R. Silva
In-Reply-To: <20260616-ad5686-new-features-v3-9-f829fb7e9262@analog.com>
On Tue, Jun 16, 2026 at 09:21:15AM +0100, Rodrigo Alencar via B4 Relay wrote:
> Use of local SPI bus data to manage a collection of SPI transfers and
> flush them to the SPI platform driver with the sync() operation. This
> allows for faster handling of multiple channel DAC writes, avoiding kernel
> overhead per spi_sync() call, which will be helpful when enabling
> triggered buffer support.
...
> static int ad5686_spi_probe(struct spi_device *spi)
> {
> - return ad5686_probe(&spi->dev, spi_get_device_match_data(spi),
> - spi->modalias, &ad5686_spi_ops);
> + const struct ad5686_chip_info *info = spi_get_device_match_data(spi);
Sashiko might still complain on driver_override case, which means we need to
add a NULL check (or is NULL fine?).
> + struct ad5686_spi_data *bus_data;
> + struct device *dev = &spi->dev;
> + unsigned int capacity;
> +
> + /* read operation requires at least 2 transfers */
> + capacity = max(info->num_channels, 2);
> + bus_data = devm_kzalloc(dev, struct_size(bus_data, xfers, capacity),
> + GFP_KERNEL);
> + if (!bus_data)
> + return -ENOMEM;
> +
> + bus_data->capacity = capacity;
> +
> + return ad5686_probe(dev, info, spi->modalias, &ad5686_spi_ops, bus_data);
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 07/12] iio: dac: ad5686: add ldac gpio
From: Andy Shevchenko @ 2026-06-16 10:32 UTC (permalink / raw)
To: rodrigo.alencar
Cc: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Kees Cook, Gustavo A. R. Silva
In-Reply-To: <20260616-ad5686-new-features-v3-7-f829fb7e9262@analog.com>
On Tue, Jun 16, 2026 at 09:21:13AM +0100, Rodrigo Alencar via B4 Relay wrote:
> If wired LDAC, should be held low when unused (pin is active-low), which
> allows for synchronous DAC updates. This will be used to update all the
> channels at the same time when adding buffer support.
...
> +struct gpio_desc;
+ blank line to decouple generic declarations from local ones?
> struct ad5686_state;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 2/3] counter: add GPIO-based quadrature encoder driver
From: William Breathitt Gray @ 2026-06-16 10:30 UTC (permalink / raw)
To: Wadim Mueller
Cc: William Breathitt Gray, Krzysztof Kozlowski, Rob Herring,
Conor Dooley, linux-iio, devicetree, linux-kernel
In-Reply-To: <2u2lqetlfph2leoggp6d7sxqv4toxpnsts2f2zgq3pwt7ae4ol@ploty2jwm3t3>
On Sun, May 24, 2026 at 09:35:45PM +0200, Wadim Mueller wrote:
> On 2026-05-21 09:26, William Breathitt Gray wrote:
> > From: Wadim Mueller <wafgo01@gmail.com>
> >
> > On Wed, May 20, 2026 at 01:45:20PM +0900, William Breathitt Gray wrote:
> > > On Fri, May 15, 2026 at 05:36:15PM +0200, Wadim Mueller wrote:
> > > > +static int gpio_qenc_a_delta(struct gpio_qenc_priv *priv, int a, int b,
> > > > + int prev_a, int prev_b)
> > > > +{
> > > > + int state = CREATE_QE_STATE(prev_a, prev_b, a, b);
> > > > +
> > > > + switch (priv->function) {
> > > > + case COUNTER_FUNCTION_QUADRATURE_X4:
> > > > + return gpio_qenc_quad_x4_table[state];
> > > > +
> > > > + case COUNTER_FUNCTION_QUADRATURE_X2_A:
> > > > + /* Both edges of A; sign comes from current A vs B. */
> > > > + return (a == b) ? -1 : 1;
> > > > +
> > > > + case COUNTER_FUNCTION_QUADRATURE_X1_A:
> > > > + /* Rising edge of A only. */
> > > > + if (!prev_a && a)
> > > > + return b ? -1 : 1;
> > > > + return 0;
> > >
> > > Quadrature X1 count modes trigger on the falling edge when the direction
> > > is backward. This isn't simply a requirement by definition, but
> > > necessary for the proper interpretation of the quadrature encoding.
> > >
> > > Let's evaluate an incremental encoder used in a positioning application
> > > as typical use case.[^1] These are commonly implemented using a rotating
> > > shaft with a quadrature-offset pattern; aligned sensors detect the
> > > physical A/B pattern as the shaft rotates.[^2] As the shaft rotates a
> > > quadrature encoding emerges whose A-B phase difference allows us to
> > > determine direction: forward when rising edge of signal A leads B, and
> > > backward when it trails.[^3]
> > >
> > > Now consider what happens to the signals when the rotation changes
> > > direction: there is a phase change between Signals A and B.[^4] The A/B
> > > pattern on the shaft is physically present so it has not changed; rather
> > > the pattern is now fed backwards to the sensors due to the direction
> > > reversal. The key point is the physical boundaries of the pattern are
> > > located in the same shaft positions they have always been, yet the
> > > signal edges representing those boundaries have flipped as a result of
> > > the direction change: positions marked by rising edges now appear as
> > > falling edges.
> > >
> > > In Quadrature X4 and X2, the pattern reversal doesn't affect positioning
> > > because we count on both edges, so swapping rising and falling edges
> > > nets the same position count. Quadrature X1 presents a problem because
> > > we count on a single edge type, so a phase-difference in the encoding
> > > results in a physical shift in real-life position. The way to account
> > > for that phase shift is to swap counting to the other edge type when the
> > > direction changes. That's how dedicated quadrature encoder devices solve
> > > this problem.
> > >
> > > I'm not sure of the best way to solve the Quadrature X1 problem in this
> > > driver. Right now we fire off interrupts on both edges, so perhaps
> > > there's a way for us to determine whether we're firing on a rising edge
> > > or falling edge and evaluate accordingly. Does the GPIO subsystem
> > > provide an indication for which edge triggered the interrupt? Or would
> > > it make sense to provide two interrupt service routines (one on rising
> > > edge and one on falling edge) and handle it that way?
> >
> > The simplest method might be to evaluate the current GPIO level to
> > determine the edge polarity. Because we trigger on both edges, we can
> > assume a high level means a low-high transition (rising edge) and a low
> > level means a high-low transition (falling edge).
> >
> > Using that assumption, we can implement the Quadrature X1 case by
> > checking the current state and direction, and adjusting the counting
> > accordingly when applicable: count up if rising edge and forward
> > direction, and count down if falling edge and backward direction.
> >
>
> Implemented in following v5 as suggested in both signal-A and signal-B ISRs.
Hello Wadim,
I apologize again for the delays in my responses. I'm currently
reviewing your v5 submission, but I do have a question below.
> One
> caveat I called out in the source: in pure X1 mode the driver never
> sees both edges of both signals, so direction is whatever the last
> X4/X2 sample produced (or whatever userspace set via sysfs). In
> practice X1 fits applications that already know the direction or
> that have just calibrated in X4.
Wouldn't the driver see both edges of both signals in X1 mode? The
ISR callbacks (gpio_counter_a_isr and gpio_counter_b_isr) execute on
both edges of their respective Signals (A and B). The driver can use
GPIO_COUNTER_GET_DIRECTION() to get the current quadrature direction and
update priv->direction accordingly during each ISR.
In that way, any quadrature mode selected will always be capable of
knowing its current direction regardless of whether it's X4, X2, or X1.
William Breathitt Gray
^ permalink raw reply
* Re: [PATCH v4 11/12] hwmon: spd5118: Add I3C support
From: sashiko-bot @ 2026-06-16 10:30 UTC (permalink / raw)
To: Akhil R; +Cc: robh, devicetree, conor+dt, linux-hwmon, linux-i3c, Frank.Li
In-Reply-To: <20260616095429.3947205-12-akhilrajeev@nvidia.com>
> Add a regmap config and a probe function to support I3C-based
> communication with SPD5118 devices.
>
> On an I3C bus, SPD5118 devices are enumerated via SETAASA and always
> require an ACPI or device tree entry. Device matching is hence through
> the OF match tables only and does not need an I3C class match table. The
> device identity is verified in the type registers before proceeding to
> the common probe function.
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616095429.3947205-1-akhilrajeev@nvidia.com?part=11
^ permalink raw reply
* Re: [PATCH v3 06/12] iio: dac: ad5686: consume optional reset signal
From: Andy Shevchenko @ 2026-06-16 10:30 UTC (permalink / raw)
To: rodrigo.alencar
Cc: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Kees Cook, Gustavo A. R. Silva
In-Reply-To: <20260616-ad5686-new-features-v3-6-f829fb7e9262@analog.com>
On Tue, Jun 16, 2026 at 09:21:12AM +0100, Rodrigo Alencar via B4 Relay wrote:
> Add RESET pin GPIO support through an optional reset control, which is
> local to the probe function. A reset pulse is manually generated after
> the device is powered up.
...
> + fsleep(1); /* reset pulse: comfortably bigger than the spec */
What spec? Same comment basically, extend with the reference to the datasheet.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 05/12] iio: dac: ad5686: add support for missing power supplies
From: Andy Shevchenko @ 2026-06-16 10:29 UTC (permalink / raw)
To: Joshua Crofts
Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, Michael Auchter,
linux, linux-iio, devicetree, linux-kernel, linux-hardening,
Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva
In-Reply-To: <20260616113322.00005273@gmail.com>
On Tue, Jun 16, 2026 at 11:33:22AM +0200, Joshua Crofts wrote:
> On Tue, 16 Jun 2026 09:21:11 +0100
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
...
> > st->use_internal_vref = ret == -ENODEV;
> > st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;
> I'd use a macro from <linux/units.h> instead of just doing / 1000.
In this case we use (MICRO / MILLI) as we don't have (yet?) the
MILLIVOLTS_PER_VOLT and so on. Feeling like you want to add those?
...
> > + fsleep(5); /* power-up time */
>
> I'd recommend using a macro to define the sleep time, removing
> the need for a comment.
OTOH macro might hide the actual value. The comment is fine, but I would extend
it with a reference to a data sheet page, table, section, et cetera.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2 3/3] irqchip/qcom-pdc: Add puwra compatible for PDC secondary mode
From: Maulik Shah @ 2026-06-16 10:27 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-kernel, devicetree, Konrad Dybcio,
Maulik Shah
In-Reply-To: <20260616-purwa-pdc-v2-0-8dda7ef25ce5@oss.qualcomm.com>
X1P42100 (Purwa) and X1E80100 (Hamoa) shares the same PDC and windows
firmware sets the PDC to secondary mode for X1P42100 too. Add support
to reset the PDC to pass through mode using qcom_scm_io_writel() similar
to X1E80100.
Make sure x1e_quirk to workaround a hardware bug is set only for X1E80100
as X1P42100 have this fixed in sillicon.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/irqchip/qcom-pdc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index c6f2935ff788..0f638556d4d0 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -704,7 +704,10 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
}
pdc->x1e_quirk = true;
+ }
+ if (of_device_is_compatible(node, "qcom,x1e80100-pdc") ||
+ of_device_is_compatible(node, "qcom,x1p42100-pdc")) {
if (!qcom_scm_is_available())
return -EPROBE_DEFER;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] arm64: dts: qcom: purwa: Drop the Hamoa workaround for PDC
From: Maulik Shah @ 2026-06-16 10:27 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-kernel, devicetree, Konrad Dybcio,
Maulik Shah
In-Reply-To: <20260616-purwa-pdc-v2-0-8dda7ef25ce5@oss.qualcomm.com>
X1P42100 (Purwa) shares the X1E80100 (Hamoa) PDC device, but the hardware
register bug addressed in commit e9a48ea4d90b ("irqchip/qcom-pdc:
Workaround hardware register bug on X1E80100") is already fixed in
X1P42100 silicon.
X1E80100 compatible forces the software workaround. Use the X1P42100
specific compatible string for the PDC node to remove the workaround.
Fixes: f08edb529916 ("arm64: dts: qcom: Add X1P42100 SoC and CRD")
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/purwa.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/purwa.dtsi b/arch/arm64/boot/dts/qcom/purwa.dtsi
index 9ab4f26b35f2..0db8c561e7ba 100644
--- a/arch/arm64/boot/dts/qcom/purwa.dtsi
+++ b/arch/arm64/boot/dts/qcom/purwa.dtsi
@@ -166,6 +166,11 @@ &pcie6a_phy {
compatible = "qcom,x1p42100-qmp-gen4x4-pcie-phy";
};
+/* X1P42100 PDC is same as X1E80100, but without hardware register bug */
+&pdc {
+ compatible = "qcom,x1p42100-pdc", "qcom,pdc";
+};
+
&qfprom {
gpu_speed_bin: gpu-speed-bin@119 {
reg = <0x119 0x2>;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: interrupt-controller: qcom,pdc: Document Purwa PDC
From: Maulik Shah @ 2026-06-16 10:27 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-kernel, devicetree, Konrad Dybcio,
Maulik Shah
In-Reply-To: <20260616-purwa-pdc-v2-0-8dda7ef25ce5@oss.qualcomm.com>
X1P42100 (Purwa) shares the X1E80100 (Hamoa) PDC device, but the hardware
register bug addressed in commit e9a48ea4d90b ("irqchip/qcom-pdc:
Workaround hardware register bug on X1E80100") is already fixed in
X1P42100 silicon.
X1E80100 compatible forces the software workaround. Add PDC compatible
for purwa as "qcom,x1p42100-pdc" to remove the workaround from Purwa.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
index 07a46c5457a4..420421863328 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
@@ -57,6 +57,7 @@ properties:
- qcom,sm8650-pdc
- qcom,sm8750-pdc
- qcom,x1e80100-pdc
+ - qcom,x1p42100-pdc
- const: qcom,pdc
reg:
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 4/4] arm64: dts: qcom: Add PMIH0108 haptics device node
From: Konrad Dybcio @ 2026-06-16 10:27 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-4-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add haptics device node in the PMIH0108 PMIC base dtsi files, and enable
> it on several boards according to the LRA (Linear Resonant Actuator)
> component mounted on each of them.
>
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/kaanapali-mtp.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/kaanapali-qrd.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi | 9 +++++++++
> arch/arm64/boot/dts/qcom/pmih0108.dtsi | 9 +++++++++
> arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 7 +++++++
One commit per board, please
> 6 files changed, 46 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> index 07247dc98b70..7e3f59fc008e 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> @@ -952,6 +952,13 @@ wifi@0 {
> };
> };
>
> +&pmih0108_e1_haptics {
> + status = "okay";
'status' should go last
> +
> + qcom,lra-period-us = <6667>;
> + qcom,vmax-mv = <3600>;
Do these properties depend on the physical characteristics on what's
connected to the other end of the haptics driver?
Konrad
^ permalink raw reply
* [PATCH v2 0/3] purwa: Drop the Hamoa PDC workaround from purwa
From: Maulik Shah @ 2026-06-16 10:27 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-kernel, devicetree, Konrad Dybcio,
Maulik Shah
Purwa shares the Hamoa PDC compatible. Hamoa had the software workaround
added due to hardware register bug mentioned in commit [1] which used
Hamoa/X1E80100 compatible to apply the workaround.
As the hardware bug mentioned in [1] is fixed on purwa silicon add purwa
compatible to avoid applying workaround meant for hamoa.
[1] https://lore.kernel.org/all/20250218-x1e80100-pdc-hw-wa-v2-1-29be4c98e355@linaro.org/
---
Changes in v2:
- Remove Fixes tag from bindings
- Rename $subject to use hamoa and purwa name
- Add comment above compatible in devicetree
- Rebase and mark dependency
- Add new change to invoke SMC call for purwa PDC mode setting
- Link to v1: https://lore.kernel.org/r/20251231-purwa_pdc-v1-0-2b4979dd88ad@oss.qualcomm.com
---
To: Thomas Gleixner <tglx@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
Maulik Shah (3):
dt-bindings: interrupt-controller: qcom,pdc: Document Purwa PDC
arm64: dts: qcom: purwa: Drop the Hamoa workaround for PDC
irqchip/qcom-pdc: Add puwra compatible for PDC secondary mode
Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml | 1 +
arch/arm64/boot/dts/qcom/purwa.dtsi | 5 +++++
drivers/irqchip/qcom-pdc.c | 3 +++
3 files changed, 9 insertions(+)
---
base-commit: 8d6dbbbe3ba62de0a63e962ee004afb848c8e3ac
change-id: 20260616-purwa-pdc-198063b482b9
prerequisite-message-id: <20260616-hamoa_pdc_v3-v3-0-4d8e1504ea75@oss.qualcomm.com>
prerequisite-patch-id: 060eef1520c1c4dd8dac1be913d3a3807e2e4297
prerequisite-patch-id: 8f3baf0a17cf193edc0a3dd072a1e148970d1818
prerequisite-patch-id: 00e0419592d6b43eb8d17bbe2ef24ccf708f15c3
prerequisite-patch-id: e6711f15783ab50499e556618767b3450e2f1367
prerequisite-patch-id: 531aa20d79ee018e28c86567c11ea88f665cb2e5
prerequisite-patch-id: 085301f3a331a482e3f16cdd4b588139cf1f2ae5
prerequisite-patch-id: aa6592f80552d5be8f73cf2068d31f1bc214c923
prerequisite-patch-id: 501c83baa6ed0870e2f535e09b64d7510e261def
Best regards,
--
Maulik Shah <maulik.shah@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Konrad Dybcio @ 2026-06-16 10:25 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-3-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add an initial driver for the Qualcomm PMIH010x PMIC haptics module,
> named as HAP530_HV. This module supports several play modes, including
> DIRECT_PLAY, FIFO, PAT_MEM, and SWR, each with distinct data sourcing
> and hardware data handling logic. Currently, the driver provides support
> for two play modes using the input force-feedback framework: FF_CONSTANT
> effect for DIRECT_PLAY mode and FF_PERIODIC effect with FF_CUSTOM
> waveform for FIFO mode.
>
> Assisted-by: Claude:claude-4-6-sonnet
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
[...]
> +static int cfg_write(struct qcom_haptics *h, u32 off, u32 val)
static inline
although I have mixed feelings about having so many accessors
[...]
> +static int haptics_write_fifo_chunk(struct qcom_haptics *h,
> + const s8 *data, u32 len)
> +{
> + u32 i, bulk_len = ALIGN_DOWN(len, 4);
Please avoid mixing multiple declarations and assignments
> + int ret;
> +
> + for (i = 0; i < bulk_len; i += 4) {
You can do 'int i' in loops nowadays
> + ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
> + if (ret)
> + return ret;
> + }
> +
> + for (; i < len; i++) {
> + ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
> + if (ret)
> + return ret;
> + }
So if i'm reading this right, the first loop will always write
4*(len//4) bytes and the second one will be entered at most once,
to write len rem 4 bytes.. should this be an if instead?
> +
> + return 0;
> +}
> +
> +/*
> + * Configure the hardware FIFO memory boundary.
> + * FIFO occupies addresses [0, fifo_len).
> + */
> +static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
> +{
> + u32 fifo_len, fifo_units;
> +
> + /* Config all memory space for FIFO usage for now */
What's the not-"for now" endgame for this?
> + fifo_len = HAP530_MEM_TOTAL_BYTES;
> + fifo_len = ALIGN_DOWN(fifo_len, 64);
> + fifo_units = fifo_len / 64;
> + h->fifo_len = fifo_len;
> +
> + return ptn_write(h, HAP_PTN_MMAP_FIFO_REG,
> + MMAP_FIFO_EXIST_BIT |
> + FIELD_PREP(MMAP_FIFO_LEN_MASK, fifo_units - 1));
> +}
> +
> +static u32 haptics_gain_scaled_vmax(struct qcom_haptics *h, u32 vmax_mv)
> +{
> + u32 v = (u32)((u64)vmax_mv * h->gain / 0xFFFF);
mult_frac()
> +
> + return max_t(u32, v, VMAX_STEP_MV);
> +}
> +
> +static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
> +{
> + if (h->irq_enabled == enable)
> + return;
> +
> + if (enable)
> + enable_irq(h->fifo_empty_irq);
> + else
> + disable_irq_nosync(h->fifo_empty_irq);
This is called in the .remove() path, I think you may need the
sync variant as the underlying device may be destroyed before the
ISR completes if there's a late interrupt
[...]
> +static int haptics_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> +
> + h->cur_effect_id = effect_id;
> + h->play_request = (val > 0);
> + schedule_work(&h->play_work);
> + return 0;
nit: \n before return is 'nice'
[...]
> + ret = device_property_read_u32_array(&pdev->dev, "reg", regs,
> + ARRAY_SIZE(regs));
Here you use device_property_
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to read 'reg' property\n");
> +
> + h->cfg_base = regs[0];
> + h->ptn_base = regs[1];
> +
> + ret = of_property_read_u32(h->dev->of_node, "qcom,lra-period-us",
> + &h->t_lra_us);
And here you use of_property_ (please use device_)
[...]
> + input->name = "qcom-spmi-haptics";
Odd whitespace
Konrad
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: sashiko-bot @ 2026-06-16 10:23 UTC (permalink / raw)
To: Fenglin Wu; +Cc: robh, linux-input, conor+dt, devicetree, dmitry.torokhov
In-Reply-To: <20260616-qcom-spmi-haptics-v1-3-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] Repeated playback requests for an actively playing effect leak runtime PM usage counters, permanently preventing the device from suspending.
- [High] The threaded IRQ handler sleeps in atomic context by executing sleepable regmap operations while holding a spinlock.
- [High] Incorrect teardown ordering in `remove()` and `suspend()` allows a work item to be re-queued and executed on freed memory (Use-After-Free).
- [High] Erasing or updating a force feedback effect frees the effect's data buffer synchronously, causing a Use-After-Free in the IRQ handler.
- [High] A failed memory allocation during effect upload leaves internal state corrupted, guaranteeing a NULL pointer dereference upon playback.
- [High] A lockless race condition between the input playback callback and the FIFO empty IRQ handler can cause new playback requests to be silently dropped.
- [High] A data race on `h->cur_effect_id` allows userspace to misdirect the stop sequence, leaving hardware IRQs enabled while dropping PM references.
- [Medium] Dynamically allocated custom waveform buffers are permanently leaked when the device is unbound.
- [Medium] Missing `abs()` on `s16` magnitude level causes massive arithmetic overflow for negative effects.
- [Medium] Registering the input device before PM runtime is fully configured creates a race condition for early playback events.
--
commit 84c087855458c41a5a109b1ffb5594db116b5170
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
input: misc: Add Qualcomm SPMI PMIC haptics driver
Add an initial driver for the Qualcomm PMIH010x PMIC haptics module,
named as HAP530_HV. This module supports several play modes, including
DIRECT_PLAY, FIFO, PAT_MEM, and SWR, each with distinct data sourcing
and hardware data handling logic. Currently, the driver provides support
for two play modes using the input force-feedback framework: FF_CONSTANT
effect for DIRECT_PLAY mode and FF_PERIODIC effect with FF_CUSTOM
waveform for FIFO mode.
> diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
> new file mode 100644
> index 0000000000000..75b3e338b54eb
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
> @@ -0,0 +1,831 @@
[ ... ]
> +static int haptics_start_direct_play(struct qcom_haptics *h, int effect_id)
> +{
> + struct ff_effect *ffe = &h->input->ff->effects[effect_id];
> + u8 amplitude = (u8)((u32)ffe->u.constant.level * 255 / 0x7FFF);
[Severity: Medium]
Does this cast cause an arithmetic overflow for negative effect levels?
The ffe->u.constant.level is an s16 which can be negative to represent
opposite-direction effects. Casting a negative s16 directly to u32
sign-extends it, resulting in a large unsigned value. Multiplying this by
255 overflows the 32-bit boundary, producing a garbage amplitude for the
hardware register.
Consider taking the absolute value of the level before casting and scaling.
[ ... ]
> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
> +{
> + struct qcom_haptics *h = dev_id;
> + unsigned long flags;
> + u32 sts, to_write;
> + int ret;
> +
> + ret = regmap_read(h->regmap,
> + h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
> + if (ret || !(sts & FIFO_EMPTY_BIT))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(&h->fifo_lock, flags);
[Severity: High]
Is it safe to hold a spinlock while calling sleepable regmap operations?
The haptics_write_fifo_chunk() function maps to regmap_bulk_write(). For an
SPMI PMIC, the regmap operates over a slow bus and takes a mutex to sleep
during transactions. Calling these sleepable functions while holding
fifo_lock results in a scheduling while atomic bug.
[ ... ]
> + if (h->data_done) {
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> + h->fifo_data = NULL;
> + h->play_request = false;
> + schedule_work(&h->play_work);
[Severity: High]
Can a lockless race condition here cause new playback requests to be silently
dropped?
When an effect completes, this sets play_request to false. Concurrently, if a
new playback request arrives, haptics_playback() sets it to true. If the IRQ
handler runs immediately after haptics_playback(), it overwrites the new true
value with false.
When haptics_play_work() executes, it would observe play_request as false,
stop the hardware, and drop the newly queued playback request.
> + spin_unlock_irqrestore(&h->fifo_lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> + /* Refill: write the next chunk, conservatively sized to the threshold */
> + to_write = min_t(u32, h->data_len - h->data_written,
> + h->fifo_len - FIFO_EMPTY_THRESH);
> + haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
[Severity: High]
Is there a risk of a use-after-free here if an effect is erased while playing?
The input framework allows effect upload or erase to occur concurrently with
playback. In haptics_erase() and haptics_upload_effect(), the driver calls
kfree(priv->fifo_data) synchronously.
However, the stop request only schedules an asynchronous work item to stop
playback. Because the hardware stop is deferred, the hardware may still be
actively playing and asserting the FIFO-empty interrupt. The IRQ handler
will then attempt to read from the freed fifo_data.
[ ... ]
> +static void haptics_play_work(struct work_struct *work)
> +{
> + struct qcom_haptics *h = container_of(work, struct qcom_haptics, play_work);
> + int id, ret;
> +
> + mutex_lock(&h->play_lock);
> +
> + if (!h->play_request) {
> + haptics_stop_locked(h);
> + if (h->pm_ref_held) {
> + pm_runtime_mark_last_busy(h->dev);
> + pm_runtime_put_autosuspend(h->dev);
> + h->pm_ref_held = false;
> + }
> + goto unlock;
> + }
> +
> + ret = pm_runtime_resume_and_get(h->dev);
> + if (ret < 0) {
> + dev_err(h->dev, "failed to resume device: %d\n", ret);
> + goto unlock;
> + }
> + h->pm_ref_held = true;
[Severity: High]
Can this leak runtime PM usage counters?
The pm_ref_held flag is set to true without checking if it is already true.
If userspace issues multiple start events without interleaved stops (for
example, to update parameters), the PM usage counter is incremented multiple
times.
When playback is eventually stopped, the single check drops only one
reference via pm_runtime_put_autosuspend(), permanently leaking all other
references and preventing the device from suspending.
[ ... ]
> +static int haptics_upload_effect(struct input_dev *dev,
> + struct ff_effect *effect,
> + struct ff_effect *old)
> +{
[ ... ]
> + priv->play_rate = (u8)buf[CUSTOM_DATA_RATE_IDX];
> + priv->vmax_mv = (u32)clamp_val(buf[CUSTOM_DATA_VMAX_IDX], 0, VMAX_MV_MAX);
> + priv->data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
> +
> + kfree(priv->fifo_data);
> + priv->fifo_data = kmalloc(priv->data_len, GFP_KERNEL);
> + if (!priv->fifo_data) {
> + kfree(buf);
> + return -ENOMEM;
> + }
[Severity: High]
Could a failed memory allocation leave the internal state corrupted?
When updating an existing FF_PERIODIC effect, data_len is updated to a new
size before kmalloc() is called. If kmalloc() fails, the function returns
-ENOMEM, leaving data_len non-zero while fifo_data is NULL.
The input core keeps the existing effect ID valid. If userspace subsequently
requests to play this effect, haptics_start_fifo() computes a non-zero
init_len and passes the NULL pointer to haptics_write_fifo_chunk(), causing
a crash.
[ ... ]
> +static int haptics_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> +
> + h->cur_effect_id = effect_id;
> + h->play_request = (val > 0);
[Severity: High]
Could this lockless update of cur_effect_id misdirect the stop sequence?
If a FIFO effect is actively playing and userspace issues a playback request
for a different DIRECT_PLAY effect, cur_effect_id is updated asynchronously.
When play_work eventually processes the stop/start sequence,
haptics_stop_locked() checks the newly updated mode:
if (h->effects[h->cur_effect_id].mode == HAPTICS_FIFO) {
ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
Seeing DIRECT_PLAY, it would skip disabling the FIFO IRQ and clearing
fifo_data, while still dropping the PM reference. The device auto-suspends,
but the IRQ remains enabled and fires spuriously.
[ ... ]
> +static int qcom_haptics_probe(struct platform_device *pdev)
> +{
[ ... ]
> + ff = input->ff;
> + ff->upload = haptics_upload_effect;
> + ff->playback = haptics_playback;
> + ff->erase = haptics_erase;
> + ff->set_gain = haptics_set_gain;
[Severity: Medium]
Should the ff->destroy callback be implemented to prevent memory leaks?
Custom waveform buffers are allocated using kmalloc() and stored in
fifo_data. This memory is normally freed in haptics_erase(). However, when
the driver is unbound and the input device is unregistered, the input core
destroys the ff device but does not automatically invoke erase() for
surviving effects.
Without a destroy callback to clean up the effects array, the allocated
buffers appear to be permanently leaked.
> +
> + ret = input_register_device(input);
> + if (ret) {
> + input_ff_destroy(input);
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register input device\n");
> + }
> +
> + platform_set_drvdata(pdev, h);
> +
> + /*
> + * Grab a reference on behalf of probe (usage_count → 1), mark the
> + * device active, then enable runtime PM.
> + */
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, HAPTICS_AUTOSUSPEND_MS);
> + devm_pm_runtime_set_active_enabled(&pdev->dev);
[Severity: Medium]
Is there a race condition by registering the input device before PM runtime
is fully configured?
This call makes the force-feedback device immediately visible to userspace.
If an application sends a play request before the probe function finishes,
haptics_play_work() will run and call pm_runtime_resume_and_get(). Since PM
runtime is not yet fully enabled, the PM runtime API will return an error
and fail the playback.
[ ... ]
> +static void qcom_haptics_remove(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> +
> + cancel_work_sync(&h->play_work);
> + mutex_lock(&h->play_lock);
> + haptics_stop_locked(h);
> + haptics_enable_module(h, false);
> + mutex_unlock(&h->play_lock);
> +
> + input_unregister_device(h->input);
[Severity: High]
Does this teardown ordering allow a work item to be re-queued and executed
on freed memory?
During driver removal, cancel_work_sync() is called before the input device
is unregistered. This creates a race window where the input device is still
visible to userspace, allowing applications to issue playback events that
call schedule_work().
Once remove() completes, the devres framework frees the qcom_haptics memory.
The mistakenly re-queued background work will then execute, dereferencing
the freed driver context.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH v4 04/12] i3c: master: Add support for devices using SETAASA
From: sashiko-bot @ 2026-06-16 10:19 UTC (permalink / raw)
To: Akhil R; +Cc: robh, Frank.Li, linux-i3c, devicetree, linux-hwmon, conor+dt
In-Reply-To: <20260616095429.3947205-5-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Missing CCC command whitelisting in I3C master drivers causes entire bus initialization to fail.
- [High] Premature SETAASA broadcast preempts and breaks SETDASA initialization.
- [High] Missing physical device address reset (RSTDAA) on SETAASA attach failure allows address collisions.
Pre-existing issues:
- [High] Bypassing PID validation allows duplicate PIDs, triggering a pre-existing Use-After-Free during teardown.
--
commit ef82dd52670ca8654bff7067823722f8e77933d1
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: master: Add support for devices using SETAASA
Add support for devices using SETAASA, such as SPD5118 and SPD5108
attached to DDR5 memory modules that do not support ENTDAA. Follow the
guidelines proposed by the MIPI Discovery and Configuration
Specification [1] for discovering such devices.
SETAASA (Set All Addresses to Static Address) differs from standard I3C
address assignment that uses ENTDAA or SETDASA to assign dynamic
addresses. Devices using SETAASA assign their pre-defined static addresses
as their dynamic addresses during DAA, and it is not mandatory for these
devices to implement standard CCC commands like GETPID, GETDCR, or GETBCR.
For such devices, it is generally recommended to issue SETHID (specified
by JEDEC JESD300) as a prerequisite for SETAASA to stop HID bit flipping.
Link: https://www.mipi.org/mipi-disco-for-i3c-download
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f0e05bcac26da..557332d932576 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -1852,6 +1898,22 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
> if (ret)
> goto err_free_dev;
>
> + /*
> + * For devices using SETAASA instead of ENTDAA, the address is statically
> + * assigned. Update the dynamic address to the provided static address.
> + * Reattach the I3C device after updating the dynamic address with the same
> + * static address. It is not mandatory for such devices to implement CCC
> + * commands like GETPID, GETDCR etc. Hence, we can return after reattaching.
> + */
> + if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> + i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
> + ret = i3c_master_reattach_i3c_dev(i3cdev, 0);
> + if (ret)
> + goto err_detach_dev;
^^^^^^^^
[Severity: High]
Does skipping the physical device address reset (RSTDAA) here allow address
collisions?
If i3c_master_reattach_i3c_dev() fails for a SETAASA device, the code jumps to
err_detach_dev. Unlike the SETDASA error path which jumps to err_rstdaa to
issue a directed RSTDAA command and clear the dynamic address, this path
leaves the physical device active at that address.
Meanwhile, i3c_master_detach_i3c_dev() frees the address slot in the master's
software state. During subsequent Dynamic Address Assignment (DAA), the master
may assign this supposedly "free" address to another device, resulting in two
physical devices responding to the same dynamic address.
[ ... ]
> @@ -2206,6 +2268,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> if (ret)
> goto err_bus_cleanup;
>
> + if (master->addr_method & I3C_ADDR_METHOD_SETAASA) {
> + ret = i3c_master_setaasa_locked(master);
> + if (ret)
> + goto err_bus_cleanup;
> + }
> +
[Severity: High]
Will this unconditional broadcast of SETAASA cause the entire bus initialization
to fail on certain controllers?
Several I3C master drivers (e.g., Cadence, SVC, Renesas) implement a strict
whitelist via the supports_ccc_cmd callback. Because these whitelists were
not updated to allow I3C_CCC_SETAASA (0x29) and I3C_CCC_VENDOR(0, true)
(SETHID), the master driver returns -EOPNOTSUPP. This error propagates up
through i3c_master_setaasa_locked(), causing i3c_master_bus_init() to
immediately abort.
[Severity: High]
Does this premature SETAASA broadcast break SETDASA initialization?
By broadcasting SETAASA before processing devices that require SETDASA:
list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
...
if (i3cboardinfo->static_addr)
i3c_master_early_i3c_dev_add(master, i3cboardinfo);
}
any device with a static address will accept the broadcast and adopt its
static address as its dynamic address. If a device was configured to use
SETDASA, it will erroneously accept the SETAASA broadcast.
When i3c_master_early_i3c_dev_add() subsequently attempts to issue the
directed SETDASA command, the device already has an assigned dynamic address
and will ignore the command. This causes the software state to desynchronize
from the physical device state. The MIPI I3C spec explicitly mandates that
SETDASA commands must be sent prior to the SETAASA command.
> /*
> * Reserve init_dyn_addr first, and then try to pre-assign dynamic
[ ... ]
> @@ -2724,6 +2799,9 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
> return -EINVAL;
> }
>
> + /* Update the address methods required for device discovery */
> + master->addr_method |= boardinfo->static_addr_method;
> +
> boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
>
> if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
^^^^^^^^
[Severity: High]
This isn't a bug introduced by this patch, but does allowing duplicate PIDs
(such as PID=0 for SETAASA devices) trigger a pre-existing Use-After-Free during
teardown?
Since SETAASA devices don't require standard CCC commands like GETPID,
configuring multiple SETAASA devices with an identical dummy PID (like PID=0)
passes the validation here.
When i3c_master_register_new_i3c_devs() is called, dev_set_name() assigns
identical sysfs names to these duplicate devices. device_register() then fails
with -EEXIST. In the error path, put_device() is called, freeing desc->dev,
but desc->dev is not set to NULL.
During bus teardown in i3c_master_unregister_i3c_devs(), the core iterates
over all devices, sees the non-NULL dangling pointer, and calls
device_is_registered(&desc->dev->dev), leading to a Use-After-Free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616095429.3947205-1-akhilrajeev@nvidia.com?part=4
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
From: sashiko-bot @ 2026-06-16 10:18 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, robh, dmitry.torokhov, linux-input, devicetree
In-Reply-To: <20260616-qcom-spmi-haptics-v1-1-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree binding description contains Linux-specific driver implementation details, violating the OS-agnostic principle.
- [Low] Stale reference to an undefined property 'qcom,wf-play-rate' in the binding description.
--
commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
Add binding document for the haptics module inside Qualcomm PMIH010X.
> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
[ ... ]
> + In FIFO streaming mode, hardware-based LRA auto-resonance tracking is
> + disabled by default. Because this mode is intended to drive arbitrary
> + waveforms that may not follow the resonant frequency, autonomous hardware
> + resonance correction would interfere with the intended output.
> +
> + In the driver, FIFO streaming is implemented using an FF_PERIODIC effect
> + with an FF_CUSTOM waveform. The expected custom data layout is:
[Severity: Low]
Are these Linux-specific driver implementation details, such as FF_PERIODIC
and FF_CUSTOM, appropriate for this binding document?
Device tree bindings are typically expected to be strictly OS-agnostic
hardware descriptions. Should the documentation for the driver's ioctl payload
and custom_data memory layout be moved to the Linux driver documentation
instead?
> + custom_data[0] = play rate code (see qcom,wf-play-rate values below)
[Severity: Low]
Is there a missing property definition for qcom,wf-play-rate?
This line references qcom,wf-play-rate values, but the property doesn't appear
to be defined anywhere in the schema's properties section. Does this need to
be added, or is this a stale reference that should be removed?
> + custom_data[1] = vmax in mV; 0 = use device default (qcom,vmax-mv)
> + custom_data[2..] = signed 8-bit PCM samples (at least one required)
> +
> +properties:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=1
^ permalink raw reply
* Re: [PATCH v4 05/12] i3c: master: Add support for devices without PID
From: sashiko-bot @ 2026-06-16 10:17 UTC (permalink / raw)
To: Akhil R; +Cc: linux-i3c, conor+dt, robh, linux-hwmon, devicetree, Frank.Li
In-Reply-To: <20260616095429.3947205-6-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The fallback logic added to `i3c_master_attach_boardinfo` for matching devices without a PID is completely unreachable dead code.
Pre-existing issues:
- [High] Dangling pointer in `i3c_master_register_new_i3c_devs()` error path leads to Use-After-Free and Double-Free.
--
commit 02ce08803703bc5626ea5224b66caedd628224e4
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: master: Add support for devices without PID
Devices using SETAASA for address assignment are not required to have
a 48-bit PID according to the I3C specification. Allow such devices to
register and use the static address where PID was required.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 557332d932576..bcc9c2d29c342 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1963,8 +1963,17 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> desc->dev->dev.type = &i3c_device_type;
> desc->dev->dev.bus = &i3c_bus_type;
> desc->dev->dev.release = i3c_device_release;
> - dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
> - desc->info.pid);
> +
> + /*
> + * For devices without PID (e.g., SETAASA devices), use
> + * static address for naming instead.
> + */
> + if (desc->info.pid)
> + dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
> + desc->info.pid);
> + else
> + dev_set_name(&desc->dev->dev, "%d-%02x", master->bus.id,
> + desc->info.static_addr);
>
> if (desc->boardinfo)
> device_set_node(&desc->dev->dev,
> fwnode_handle_get(desc->boardinfo->fwnode));
>
> ret = device_register(&desc->dev->dev);
> if (ret) {
> dev_err(&master->dev,
> "Failed to add I3C device (err = %d)\n", ret);
> put_device(&desc->dev->dev);
> }
[Severity: High]
This is a pre-existing issue, but does this error path leave a dangling
pointer that can cause memory corruption?
If device_register() fails, put_device() drops the reference and frees
desc->dev. However, desc->dev is not set to NULL.
When the I3C master later unregisters, i3c_master_unregister_i3c_devs()
iterates through the bus devices. Since the pointer is left dangling, it
passes the if (!i3cdev->dev) check, leading to write-after-free when
setting i3cdev->dev->desc = NULL, followed by a double-free when calling
put_device() again.
Should desc->dev be set to NULL after calling put_device() here?
[ ... ]
> @@ -2357,8 +2366,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
> struct i3c_dev_boardinfo *i3cboardinfo;
>
> list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> - if (i3cdev->info.pid != i3cboardinfo->pid)
> - continue;
> + /*
> + * For devices without PID (e.g., SETAASA devices), match by
> + * static address. For devices with PID, match by PID.
> + */
> + if (i3cboardinfo->pid) {
> + if (i3cdev->info.pid != i3cboardinfo->pid)
> + continue;
> + } else {
> + if (!i3cboardinfo->static_addr ||
> + i3cdev->info.static_addr != i3cboardinfo->static_addr)
> + continue;
> + }
[Severity: Low]
Is this else block reachable?
Looking at i3c_master_add_i3c_boardinfo(), it requires a valid PID for all
non-SETAASA devices. Since SETAASA devices bypass
i3c_master_attach_boardinfo() entirely as they are attached early via
i3c_master_early_i3c_dev_add(), wouldn't i3cboardinfo->pid be strictly
non-zero for any device reaching this function?
Additionally, for dynamically discovered devices, wouldn't
i3cdev->info.static_addr be 0 prior to this call, causing
i3cdev->info.static_addr != i3cboardinfo->static_addr to always be true
anyway?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616095429.3947205-1-akhilrajeev@nvidia.com?part=5
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: sashiko-bot @ 2026-06-16 10:17 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, robh, linux-input, dmitry.torokhov, devicetree
In-Reply-To: <20260616-qcom-spmi-haptics-v1-2-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Use of a wildcard ('x') in the compatible string.
--
commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
Add binding document for the haptics module inside Qualcomm PMIH010X.
> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
[ ... ]
> +properties:
> + compatible:
> + const: qcom,pmih010x-haptics
[Severity: Low]
Does this introduce a regression by using a wildcard character ('x') in the
compatible string?
Device tree specifications generally forbid wildcards in compatible strings.
Should the specific model name of the first chip in the family
(e.g., "qcom,pmih0108-haptics") be used instead of a wildcard?
[ ... ]
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pmic {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + haptics@f000 {
> + compatible = "qcom,pmih010x-haptics";
[Severity: Low]
Does this example share the same regression with the wildcard compatible
string?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=2
^ 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