[PATCH 0/3] hwspinlock: sun6i: Allwinner D1 and T113 support

[PATCH 0/3] hwspinlock: sun6i: Allwinner D1 and T113 support

[Nguyen Minh Tien]

This adds the hardware spinlock of the D1 and T113.

The sun6i driver already handles this block, but it looks up its clock
and reset by name, which the binding doesn't allow, so a node written
to the binding never probes. Patch 1 fixes the driver, as suggested
when this came up for the A64 [1]. Patch 3 needs it for the device to
probe.

Wilken asked then for testing with Crust running [2], as Crust owns
part of this block on the SoCs it supports. That doesn't apply here:
Crust doesn't run on the D1 or T113, no devicetree in mainline has this
node for any other SoC, and patch 1 changes only how the clock and
reset are found, not when the driver enables or asserts them.

I tested it on a MangoPi MQ-Dual (T113-S3), not only between the two
Cortex-A7 cores but also against FreeRTOS on the HiFi4 DSP, the other
side this block is there for:
 - the driver finds 32 locks;
 - for each lock, while Linux or the DSP held it, the other side was
   refused, and got it once it was freed;
 - both A7 cores and the DSP each added 1 to a shared counter a million
   times under one lock, and the total was exact in every run. Without
   the lock, over a third of the updates were lost.
I have no D1 board to test on.

[1] https://lore.kernel.org/f720eaf9-01d0-0e57-f6bf-9aade00aafb8@linaro.org
[2] https://lore.kernel.org/20230216095454.54f4d5ca@posteo.net

Nguyen Minh Tien (3):
  hwspinlock: sun6i: Get the clock and the reset without names
  dt-bindings: hwlock: sun6i: Add compatible for Allwinner D1
  riscv: dts: allwinner: d1-t113: Add the hardware spinlock

 .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml  | 7 ++++++-
 arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi         | 9 +++++++++
 drivers/hwspinlock/sun6i_hwspinlock.c                    | 4 ++--
 3 files changed, 17 insertions(+), 3 deletions(-)


base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14
-- 
2.34.1

[PATCH 1/3] hwspinlock: sun6i: Get the clock and the reset without names

[Nguyen Minh Tien]

The driver looks up its clock and reset by the name "ahb", but the
binding doesn't define clock-names or reset-names, so a node that
follows the binding fails to probe:

  sun6i_hwspinlock 3005000.hwlock: unable to get AHB clock (-2)

There is only one of each, so get them without a name. That is what
was asked for when the names were added to an A64 node, and nodes that
do set "ahb" keep working. For the reset, call the explicit
devm_reset_control_get_exclusive(), which devm_reset_control_get()
only wraps.

Fixes: 3c881e05c814 ("hwspinlock: add sun6i hardware spinlock support")
Link: https://lore.kernel.org/f720eaf9-01d0-0e57-f6bf-9aade00aafb8@linaro.org
Signed-off-by: Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog>
---
 drivers/hwspinlock/sun6i_hwspinlock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwspinlock/sun6i_hwspinlock.c b/drivers/hwspinlock/sun6i_hwspinlock.c
index c2d3145880..e7cc121310 100644
--- a/drivers/hwspinlock/sun6i_hwspinlock.c
+++ b/drivers/hwspinlock/sun6i_hwspinlock.c
@@ -104,14 +104,14 @@ static int sun6i_hwspinlock_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	priv->ahb_clk = devm_clk_get(&pdev->dev, "ahb");
+	priv->ahb_clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->ahb_clk)) {
 		err = PTR_ERR(priv->ahb_clk);
 		dev_err(&pdev->dev, "unable to get AHB clock (%d)\n", err);
 		return err;
 	}
 
-	priv->reset = devm_reset_control_get(&pdev->dev, "ahb");
+	priv->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
 	if (IS_ERR(priv->reset))
 		return dev_err_probe(&pdev->dev, PTR_ERR(priv->reset),
 				     "unable to get reset control\n");
-- 
2.34.1

[PATCH 2/3] dt-bindings: hwlock: sun6i: Add compatible for Allwinner D1

[Nguyen Minh Tien]

The D1 and T113 have a hardware spinlock with the register layout the
sun6i driver handles. Add a compatible for it, with the A31 one as
fallback.

Signed-off-by: Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog>
---
 .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml    | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
index 584cce3211..13f9a20ae1 100644
--- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
+++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
@@ -15,7 +15,12 @@ description:
 
 properties:
   compatible:
-    const: allwinner,sun6i-a31-hwspinlock
+    oneOf:
+      - const: allwinner,sun6i-a31-hwspinlock
+      - items:
+          - enum:
+              - allwinner,sun20i-d1-hwspinlock
+          - const: allwinner,sun6i-a31-hwspinlock
 
   reg:
     maxItems: 1
-- 
2.34.1

Re: [PATCH 2/3] dt-bindings: hwlock: sun6i: Add compatible for Allwinner D1

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Nguyen Minh Tien]

Add the hardware spinlock of the D1 and T113. It goes in
sunxi-d1-t113.dtsi rather than sunxi-d1s-t113.dtsi, as the D1s manual
has no spinlock in its memory map.

Signed-off-by: Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog>
---
 arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi
index 3b077dc086..228cc5c074 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi
@@ -11,5 +11,14 @@ dsp_wdt: watchdog@1700400 {
 			clock-names = "hosc", "losc";
 			status = "reserved";
 		};
+
+		hwlock: hwlock@3005000 {
+			compatible = "allwinner,sun20i-d1-hwspinlock",
+				     "allwinner,sun6i-a31-hwspinlock";
+			reg = <0x3005000 0x1000>;
+			clocks = <&ccu CLK_BUS_SPINLOCK>;
+			resets = <&ccu RST_BUS_SPINLOCK>;
+			#hwlock-cells = <1>;
+		};
 	};
 };
-- 
2.34.1

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Wilken Gottwalt]

On Sun, 27 Sep 2026 09:56:26 +0700
Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:

> Add the hardware spinlock of the D1 and T113. It goes in
> sunxi-d1-t113.dtsi rather than sunxi-d1s-t113.dtsi, as the D1s manual
> has no spinlock in its memory map.
> 
> Signed-off-by: Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog>
> ---
>  arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi
> b/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi index 3b077dc086..228cc5c074 100644
> --- a/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi
> +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1-t113.dtsi
> @@ -11,5 +11,14 @@ dsp_wdt: watchdog@1700400 {
>  			clock-names = "hosc", "losc";
>  			status = "reserved";
>  		};
> +
> +		hwlock: hwlock@3005000 {
> +			compatible = "allwinner,sun20i-d1-hwspinlock",
> +				     "allwinner,sun6i-a31-hwspinlock";
> +			reg = <0x3005000 0x1000>;
> +			clocks = <&ccu CLK_BUS_SPINLOCK>;
> +			resets = <&ccu RST_BUS_SPINLOCK>;
> +			#hwlock-cells = <1>;
> +		};
>  	};
>  };

Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
the driver in the sun6i_hwspinlock_ids struct, drop
"allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
file accordingly? Hmm, there are actually a lot more devices, which support that
spinlock (H2, H2+, H3, H5, H6...). A31 was the first one introducing that IP
core, but newer reference manuals removed the spinlock section completely.
There it is an unnamed 4k block in the memory map.

Bjorn, what do you think? Just stay with the "allwinner,sun6i-a31-hwspinlock"
string or add all the possible combinations like
"allwinner,sun8i-h2-plus-hwspinlock" or "allwinner,sun8i-a83t-hwspinlock".
I mean, it is just a naming game and there are 10+ SoCs supporting this
spinlock register file.

Oh, and I may be able to test it against the D1, I own a Sipeed Nezha. Though,
setting that one up for kernel + FreeRTOS testing is really, uhm, annoying.

greetings,
Wilken

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Nguyen Minh Tien]

Hi Wilken,

> Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> the driver in the sun6i_hwspinlock_ids struct, drop
> "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> file accordingly?

Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
more at the naming. I'd like to keep the A31 fallback: it's the usual
pattern, other blocks in this dtsi do the same (timer, I2S, LED
controller), and Conor already acked the binding in 2/3. If Bjorn
prefers a driver entry instead, I'm fine to change it.

> Oh, and I may be able to test it against the D1, I own a Sipeed Nezha.

That would be great. You don't need FreeRTOS for it: I tested with a
small Linux module that takes each lock and checks the status
register. I can clean it up for the single-core D1 and send it.

Thanks,
Tien

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Wilken Gottwalt]

On Wed, 30 Sep 2026 20:02:21 +0700
Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:

> Hi Wilken,
> 
> > Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> > the driver in the sun6i_hwspinlock_ids struct, drop
> > "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> > file accordingly?
> 
> Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
> more at the naming. I'd like to keep the A31 fallback: it's the usual
> pattern, other blocks in this dtsi do the same (timer, I2S, LED
> controller), and Conor already acked the binding in 2/3. If Bjorn
> prefers a driver entry instead, I'm fine to change it.

Yeah, Conor was a bit quick to act here, such things happen often with patchsets
made out of documentation/devicetrees and code. Though, the get clock and resets
patch is fine. The driver could use some modernization.

> > Oh, and I may be able to test it against the D1, I own a Sipeed Nezha.
> 
> That would be great. You don't need FreeRTOS for it: I tested with a
> small Linux module that takes each lock and checks the status
> register. I can clean it up for the single-core D1 and send it.

Uhm, the Linux-only test doesn't work as a hwspinlock test, it misses the entire
point of the primitive. A hwspinlock arbitrates between two independent agents,
in this case, Linux running on the C906 core and FreeRTOS running on the HiFi
DSP, both sharing the same memory bus and other hardware. If Linux is the only
one who ever takes the locks, it is simultaneously writer and reader of the status
register, so the test cannot fail even for a broken (or fake) implementation. That
would basically test nothing at all, well, maybe it would be some kind of bring-up
test, but overall quite useless.

greetings,
Wilken

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2661 bytes --]

On Wed, Sep 30, 2026 at 02:05:47PM +0000, Wilken Gottwalt wrote:
> On Wed, 30 Sep 2026 20:02:21 +0700
> Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:
> 
> > Hi Wilken,
> > 
> > > Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> > > the driver in the sun6i_hwspinlock_ids struct, drop
> > > "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> > > file accordingly?
> > 
> > Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
> > more at the naming. I'd like to keep the A31 fallback: it's the usual
> > pattern, other blocks in this dtsi do the same (timer, I2S, LED
> > controller), and Conor already acked the binding in 2/3. If Bjorn
> > prefers a driver entry instead, I'm fine to change it.
> 
> Yeah, Conor was a bit quick to act here, such things happen often with patchsets
> made out of documentation/devicetrees and code. Though, the get clock and resets
> patch is fine. The driver could use some modernization.

I dunno, was I too quick to act? The patched looked correct to me, since
it was using a fallback to a device that it appears to be compatible
with. Had the series done what you're suggesting, my review feedback
would have been to tell the Tien to add a fallback.

> > Bjorn, what do you think? Just stay with the "allwinner,sun6i-a31-hwspinlock"
> > string or add all the possible combinations like
> > "allwinner,sun8i-h2-plus-hwspinlock" or "allwinner,sun8i-a83t-hwspinlock".
> > I mean, it is just a naming game and there are 10+ SoCs supporting this
> > spinlock register file

All devices compatible with the a31 should use the a31 as a fallback.

> 
> > > Oh, and I may be able to test it against the D1, I own a Sipeed Nezha.
> > 
> > That would be great. You don't need FreeRTOS for it: I tested with a
> > small Linux module that takes each lock and checks the status
> > register. I can clean it up for the single-core D1 and send it.
> 
> Uhm, the Linux-only test doesn't work as a hwspinlock test, it misses the entire
> point of the primitive. A hwspinlock arbitrates between two independent agents,
> in this case, Linux running on the C906 core and FreeRTOS running on the HiFi
> DSP, both sharing the same memory bus and other hardware. If Linux is the only
> one who ever takes the locks, it is simultaneously writer and reader of the status
> register, so the test cannot fail even for a broken (or fake) implementation. That
> would basically test nothing at all, well, maybe it would be some kind of bring-up
> test, but overall quite useless.
> 
> greetings,
> Wilken

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Wilken Gottwalt]

On Fri, 2 Oct 2026 08:32:51 +0100
Conor Dooley <conor.dooley@microchip.com> wrote:

> On Wed, Sep 30, 2026 at 02:05:47PM +0000, Wilken Gottwalt wrote:
> > On Wed, 30 Sep 2026 20:02:21 +0700
> > Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:
> > 
> > > Hi Wilken,
> > > 
> > > > Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> > > > the driver in the sun6i_hwspinlock_ids struct, drop
> > > > "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> > > > file accordingly?
> > > 
> > > Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
> > > more at the naming. I'd like to keep the A31 fallback: it's the usual
> > > pattern, other blocks in this dtsi do the same (timer, I2S, LED
> > > controller), and Conor already acked the binding in 2/3. If Bjorn
> > > prefers a driver entry instead, I'm fine to change it.
> > 
> > Yeah, Conor was a bit quick to act here, such things happen often with patchsets
> > made out of documentation/devicetrees and code. Though, the get clock and resets
> > patch is fine. The driver could use some modernization.
> 
> I dunno, was I too quick to act? The patched looked correct to me, since
> it was using a fallback to a device that it appears to be compatible
> with. Had the series done what you're suggesting, my review feedback
> would have been to tell the Tien to add a fallback.

I'm just not sure how to actually do it right, because so many SoCs include that
feature. That is why I asked how you would do it having more insight as a
subsystem maintainer. It just looks incomplete to me. In the past I actually
verified it working with H2, H2+ and H3, none of them being
"allwinner,sun6i-a31-hwspinlock".

> > > Bjorn, what do you think? Just stay with the "allwinner,sun6i-a31-hwspinlock"
> > > string or add all the possible combinations like
> > > "allwinner,sun8i-h2-plus-hwspinlock" or "allwinner,sun8i-a83t-hwspinlock".
> > > I mean, it is just a naming game and there are 10+ SoCs supporting this
> > > spinlock register file
> 
> All devices compatible with the a31 should use the a31 as a fallback.
> 
> > 
> > > > Oh, and I may be able to test it against the D1, I own a Sipeed Nezha.
> > > 
> > > That would be great. You don't need FreeRTOS for it: I tested with a
> > > small Linux module that takes each lock and checks the status
> > > register. I can clean it up for the single-core D1 and send it.
> > 
> > Uhm, the Linux-only test doesn't work as a hwspinlock test, it misses the entire
> > point of the primitive. A hwspinlock arbitrates between two independent agents,
> > in this case, Linux running on the C906 core and FreeRTOS running on the HiFi
> > DSP, both sharing the same memory bus and other hardware. If Linux is the only
> > one who ever takes the locks, it is simultaneously writer and reader of the status
> > register, so the test cannot fail even for a broken (or fake) implementation. That
> > would basically test nothing at all, well, maybe it would be some kind of bring-up
> > test, but overall quite useless.

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Chen-Yu Tsai]

On Fri, Oct 2, 2026 at 3:48 PM Wilken Gottwalt
<wilken.gottwalt@posteo.net> wrote:
>
> On Fri, 2 Oct 2026 08:32:51 +0100
> Conor Dooley <conor.dooley@microchip.com> wrote:
>
> > On Wed, Sep 30, 2026 at 02:05:47PM +0000, Wilken Gottwalt wrote:
> > > On Wed, 30 Sep 2026 20:02:21 +0700
> > > Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:
> > >
> > > > Hi Wilken,
> > > >
> > > > > Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> > > > > the driver in the sun6i_hwspinlock_ids struct, drop
> > > > > "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> > > > > file accordingly?
> > > >
> > > > Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
> > > > more at the naming. I'd like to keep the A31 fallback: it's the usual
> > > > pattern, other blocks in this dtsi do the same (timer, I2S, LED
> > > > controller), and Conor already acked the binding in 2/3. If Bjorn
> > > > prefers a driver entry instead, I'm fine to change it.
> > >
> > > Yeah, Conor was a bit quick to act here, such things happen often with patchsets
> > > made out of documentation/devicetrees and code. Though, the get clock and resets
> > > patch is fine. The driver could use some modernization.
> >
> > I dunno, was I too quick to act? The patched looked correct to me, since
> > it was using a fallback to a device that it appears to be compatible
> > with. Had the series done what you're suggesting, my review feedback
> > would have been to tell the Tien to add a fallback.
>
> I'm just not sure how to actually do it right, because so many SoCs include that
> feature. That is why I asked how you would do it having more insight as a
> subsystem maintainer. It just looks incomplete to me. In the past I actually
> verified it working with H2, H2+ and H3, none of them being
> "allwinner,sun6i-a31-hwspinlock".

Even if all the SoCs had them, if no one enabled and tested them, then they
wouldn't have entries in the binding or dtsi files.

> > > > Bjorn, what do you think? Just stay with the "allwinner,sun6i-a31-hwspinlock"
> > > > string or add all the possible combinations like
> > > > "allwinner,sun8i-h2-plus-hwspinlock" or "allwinner,sun8i-a83t-hwspinlock".
> > > > I mean, it is just a naming game and there are 10+ SoCs supporting this
> > > > spinlock register file
> >
> > All devices compatible with the a31 should use the a31 as a fallback.
> >
> > >
> > > > > Oh, and I may be able to test it against the D1, I own a Sipeed Nezha.
> > > >
> > > > That would be great. You don't need FreeRTOS for it: I tested with a
> > > > small Linux module that takes each lock and checks the status
> > > > register. I can clean it up for the single-core D1 and send it.
> > >
> > > Uhm, the Linux-only test doesn't work as a hwspinlock test, it misses the entire
> > > point of the primitive. A hwspinlock arbitrates between two independent agents,
> > > in this case, Linux running on the C906 core and FreeRTOS running on the HiFi
> > > DSP, both sharing the same memory bus and other hardware. If Linux is the only
> > > one who ever takes the locks, it is simultaneously writer and reader of the status
> > > register, so the test cannot fail even for a broken (or fake) implementation. That
> > > would basically test nothing at all, well, maybe it would be some kind of bring-up
> > > test, but overall quite useless.

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

On Fri, Oct 02, 2026 at 07:48:21AM +0000, Wilken Gottwalt wrote:
> On Fri, 2 Oct 2026 08:32:51 +0100
> Conor Dooley <conor.dooley@microchip.com> wrote:
> 
> > On Wed, Sep 30, 2026 at 02:05:47PM +0000, Wilken Gottwalt wrote:
> > > On Wed, 30 Sep 2026 20:02:21 +0700
> > > Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:
> > > 
> > > > Hi Wilken,
> > > > 
> > > > > Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> > > > > the driver in the sun6i_hwspinlock_ids struct, drop
> > > > > "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> > > > > file accordingly?
> > > > 
> > > > Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
> > > > more at the naming. I'd like to keep the A31 fallback: it's the usual
> > > > pattern, other blocks in this dtsi do the same (timer, I2S, LED
> > > > controller), and Conor already acked the binding in 2/3. If Bjorn
> > > > prefers a driver entry instead, I'm fine to change it.
> > > 
> > > Yeah, Conor was a bit quick to act here, such things happen often with patchsets
> > > made out of documentation/devicetrees and code. Though, the get clock and resets
> > > patch is fine. The driver could use some modernization.
> > 
> > I dunno, was I too quick to act? The patched looked correct to me, since
> > it was using a fallback to a device that it appears to be compatible
> > with. Had the series done what you're suggesting, my review feedback
> > would have been to tell the Tien to add a fallback.
> 
> I'm just not sure how to actually do it right, because so many SoCs include that
> feature.

If devices share a programming model with existing devices or have a
programming model that's a functional superset of existing devices (so a
new optional feature etc) they should use the existing device as a
fallback compatible.

> That is why I asked how you would do it having more insight as a
> subsystem maintainer.


> It just looks incomplete to me. In the past I actually
> verified it working with H2, H2+ and H3, none of them being
> "allwinner,sun6i-a31-hwspinlock".

Then probably there should be a patch adding all of these to the enum
alongside the new d1 compatible.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] riscv: dts: allwinner: d1-t113: Add the hardware spinlock

[Chen-Yu Tsai]

On Fri, Oct 2, 2026 at 3:34 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Wed, Sep 30, 2026 at 02:05:47PM +0000, Wilken Gottwalt wrote:
> > On Wed, 30 Sep 2026 20:02:21 +0700
> > Nguyen Minh Tien <tien.nguyenminh@embeddedlinux.blog> wrote:
> >
> > > Hi Wilken,
> > >
> > > > Wouldn't it make more sense to add the "allwinner,sun20i-d1-hwspinlock" line to
> > > > the driver in the sun6i_hwspinlock_ids struct, drop
> > > > "allwinner,sun6i-a31-hwspinlock" here in the D1 device tree and update the yaml
> > > > file accordingly?
> > >
> > > Thanks for looking at it. Bjorn hasn't replied yet, so I looked a bit
> > > more at the naming. I'd like to keep the A31 fallback: it's the usual
> > > pattern, other blocks in this dtsi do the same (timer, I2S, LED
> > > controller), and Conor already acked the binding in 2/3. If Bjorn
> > > prefers a driver entry instead, I'm fine to change it.
> >
> > Yeah, Conor was a bit quick to act here, such things happen often with patchsets
> > made out of documentation/devicetrees and code. Though, the get clock and resets
> > patch is fine. The driver could use some modernization.
>
> I dunno, was I too quick to act? The patched looked correct to me, since
> it was using a fallback to a device that it appears to be compatible
> with. Had the series done what you're suggesting, my review feedback
> would have been to tell the Tien to add a fallback.
>
> > > Bjorn, what do you think? Just stay with the "allwinner,sun6i-a31-hwspinlock"
> > > string or add all the possible combinations like
> > > "allwinner,sun8i-h2-plus-hwspinlock" or "allwinner,sun8i-a83t-hwspinlock".
> > > I mean, it is just a naming game and there are 10+ SoCs supporting this
> > > spinlock register file
>
> All devices compatible with the a31 should use the a31 as a fallback.

+1.

If the hardware is backward compatible, then with the fallback compatible
everything works.

We ask folks to always add SoC-specific compatibles just in case.


ChenYu

> >
> > > > Oh, and I may be able to test it against the D1, I own a Sipeed Nezha.
> > >
> > > That would be great. You don't need FreeRTOS for it: I tested with a
> > > small Linux module that takes each lock and checks the status
> > > register. I can clean it up for the single-core D1 and send it.
> >
> > Uhm, the Linux-only test doesn't work as a hwspinlock test, it misses the entire
> > point of the primitive. A hwspinlock arbitrates between two independent agents,
> > in this case, Linux running on the C906 core and FreeRTOS running on the HiFi
> > DSP, both sharing the same memory bus and other hardware. If Linux is the only
> > one who ever takes the locks, it is simultaneously writer and reader of the status
> > register, so the test cannot fail even for a broken (or fake) implementation. That
> > would basically test nothing at all, well, maybe it would be some kind of bring-up
> > test, but overall quite useless.
> >
> > greetings,
> > Wilken