[PATCH v2 0/5] Enable hwlock on Allwinner A64

[PATCH v2 0/5] Enable hwlock on Allwinner A64

[Bastian Germann]

The allwinner,sun6i-a31-hwspinlock compatible driver can be used with
the Allwinner A64 chip. Add the wiring required to enable it.

The device tree binding needs some work to verify everything that is
needed by the sun6i hwlock driver.

The hwlock device was verified to be available with this series applied
on a Pinebook.

1/5 was already applied in linux-next.

Changelog:
v2:
   Fix the dt-binding example for the added properties.
   Add a compatible string for a64.
   Wording 3/5: driver -> Linux driver.

Bastian Germann (5):
  dt-bindings: hwlock: sun6i: Add missing #hwlock-cells
  dt-bindings: hwlock: sun6i: Add #hwlock-cells to example
  dt-bindings: hwlock: sun6i: Add missing names
  dt-bindings: hwlock: sun6i: Add a64 compatible string
  arm64: dts: allwinner: a64: Add hwspinlock node

 .../allwinner,sun6i-a31-hwspinlock.yaml       | 21 ++++++++++++++++++-
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 ++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

-- 
2.39.1

[PATCH v2 1/5] dt-bindings: hwlock: sun6i: Add missing #hwlock-cells

[Bastian Germann]

The allwinner,sun6i-a31-hwspinlock.yaml binding needs #hwlock-cells
which is required by the hwlock type.

Fixes: f9e784dcb63f ("dt-bindings: hwlock: add sun6i_hwspinlock")
Signed-off-by: Bastian Germann <bage@debian.org>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213231931.6546-2-bage@debian.org
---
 .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
index 10e5a53e447b..01b1bbb3061f 100644
--- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
+++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
@@ -26,11 +26,15 @@ properties:
   resets:
     maxItems: 1
 
+  '#hwlock-cells':
+    const: 1
+
 required:
   - compatible
   - reg
   - clocks
   - resets
+  - "#hwlock-cells"
 
 additionalProperties: false
 
-- 
2.39.1

[PATCH v2 2/5] dt-bindings: hwlock: sun6i: Add #hwlock-cells to example

[Bastian Germann]

The dt-bindings tools will compile the yaml dt examples
and this prevents an error about this node not existing.

Signed-off-by: Bastian Germann <bage@debian.org>
---
 .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
index 01b1bbb3061f..38478dad8b25 100644
--- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
+++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
@@ -48,5 +48,6 @@ examples:
         reg = <0x01c18000 0x1000>;
         clocks = <&ccu CLK_BUS_SPINLOCK>;
         resets = <&ccu RST_BUS_SPINLOCK>;
+        #hwlock-cells = <1>;
     };
 ...
-- 
2.39.1

[PATCH v2 3/5] dt-bindings: hwlock: sun6i: Add missing names

[Bastian Germann]

The allwinner,sun6i-a31-hwspinlock.yaml binding needs clock-names
and reset-names set to "ahb" as required by the Linux driver.

Fixes: f9e784dcb63f ("dt-bindings: hwlock: add sun6i_hwspinlock")
Signed-off-by: Bastian Germann <bage@debian.org>
---
 .../hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
index 38478dad8b25..6cdfe22deb3c 100644
--- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
+++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
@@ -23,9 +23,17 @@ properties:
   clocks:
     maxItems: 1
 
+  clock-names:
+    items:
+      - const: ahb
+
   resets:
     maxItems: 1
 
+  reset-names:
+    items:
+      - const: ahb
+
   '#hwlock-cells':
     const: 1
 
@@ -33,7 +41,9 @@ required:
   - compatible
   - reg
   - clocks
+  - clock-names
   - resets
+  - reset-names
   - "#hwlock-cells"
 
 additionalProperties: false
@@ -47,7 +57,9 @@ examples:
         compatible = "allwinner,sun6i-a31-hwspinlock";
         reg = <0x01c18000 0x1000>;
         clocks = <&ccu CLK_BUS_SPINLOCK>;
+        clock-names = "ahb";
         resets = <&ccu RST_BUS_SPINLOCK>;
+        reset-names = "ahb";
         #hwlock-cells = <1>;
     };
 ...
-- 
2.39.1

[PATCH v2 4/5] dt-bindings: hwlock: sun6i: Add a64 compatible string

[Bastian Germann]

Add an allwinner,sun50i-a64-hwspinlock compatible string to the device
tree binding. The A31 and A64 have compatible hwspinlocks.

Signed-off-by: Bastian Germann <bage@debian.org>
---
 .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 4 +++-
 1 file changed, 3 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 6cdfe22deb3c..281c285282a1 100644
--- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
+++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
@@ -15,7 +15,9 @@ description:
 
 properties:
   compatible:
-    const: allwinner,sun6i-a31-hwspinlock
+    items:
+      - const: allwinner,sun6i-a31-hwspinlock
+      - const: allwinner,sun50i-a64-hwspinlock
 
   reg:
     maxItems: 1
-- 
2.39.1

[PATCH v2 5/5] arm64: dts: allwinner: a64: Add hwspinlock node

[Bastian Germann]

Add the hwspinlock to A64 which is already implemented for A31.

Signed-off-by: Bastian Germann <bage@debian.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 77b5349f6087..f2ecc21f06ed 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -851,6 +851,17 @@ wdt0: watchdog@1c20ca0 {
 			clocks = <&osc24M>;
 		};
 
+		hwspinlock: hwlock@1c18000 {
+			compatible = "allwinner,sun50i-a64-hwspinlock",
+				     "allwinner,sun6i-a31-hwspinlock";
+			reg = <0x01c18000 0x1000>;
+			clocks = <&ccu CLK_BUS_SPINLOCK>;
+			clock-names = "ahb";
+			resets = <&ccu RST_BUS_SPINLOCK>;
+			reset-names = "ahb";
+			#hwlock-cells = <1>;
+		};
+
 		spdif: spdif@1c21000 {
 			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-a64-spdif",
-- 
2.39.1

Re: [PATCH v2 2/5] dt-bindings: hwlock: sun6i: Add #hwlock-cells to example

[Andre Przywara]

On Wed, 15 Feb 2023 21:37:07 +0100
Bastian Germann <bage@debian.org> wrote:

Hi,

> The dt-bindings tools will compile the yaml dt examples
> and this prevents an error about this node not existing.

This needs to be part of patch 1/5, otherwise it will break
bisecting. Just squash the two patches together.

Cheers,
Andre

> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml          | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> index 01b1bbb3061f..38478dad8b25 100644
> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> @@ -48,5 +48,6 @@ examples:
>          reg = <0x01c18000 0x1000>;
>          clocks = <&ccu CLK_BUS_SPINLOCK>;
>          resets = <&ccu RST_BUS_SPINLOCK>;
> +        #hwlock-cells = <1>;
>      };
>  ...

Re: [PATCH v2 2/5] dt-bindings: hwlock: sun6i: Add #hwlock-cells to example

[Bastian Germann]

Am 15.02.23 um 21:40 schrieb Andre Przywara:
> On Wed, 15 Feb 2023 21:37:07 +0100
> Bastian Germann <bage@debian.org> wrote:
> 
> Hi,
> 
>> The dt-bindings tools will compile the yaml dt examples
>> and this prevents an error about this node not existing.
> 
> This needs to be part of patch 1/5, otherwise it will break
> bisecting. Just squash the two patches together.

Is this also okay with 1/5 already being applied in linux-next?

> 
> Cheers,
> Andre
> 
>> Signed-off-by: Bastian Germann <bage@debian.org>
>> ---
>>   .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml          | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
>> index 01b1bbb3061f..38478dad8b25 100644
>> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
>> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
>> @@ -48,5 +48,6 @@ examples:
>>           reg = <0x01c18000 0x1000>;
>>           clocks = <&ccu CLK_BUS_SPINLOCK>;
>>           resets = <&ccu RST_BUS_SPINLOCK>;
>> +        #hwlock-cells = <1>;
>>       };
>>   ...
> 

Re: [PATCH v2 3/5] dt-bindings: hwlock: sun6i: Add missing names

[Andre Przywara]

On Wed, 15 Feb 2023 21:37:08 +0100
Bastian Germann <bage@debian.org> wrote:

> The allwinner,sun6i-a31-hwspinlock.yaml binding needs clock-names
> and reset-names set to "ahb" as required by the Linux driver.

Mmmh, but I thought that Krzysztof pretty clearly NAKed this?
So we have to either reach consensus on this or find another solution,
like teaching the driver to comply with the existing binding.
We could for instance get the first clock, should the devm_clk_get()
call fail.

Cheers,
Andre


> 
> Fixes: f9e784dcb63f ("dt-bindings: hwlock: add sun6i_hwspinlock")
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  .../hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> index 38478dad8b25..6cdfe22deb3c 100644
> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> @@ -23,9 +23,17 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  clock-names:
> +    items:
> +      - const: ahb
> +
>    resets:
>      maxItems: 1
>  
> +  reset-names:
> +    items:
> +      - const: ahb
> +
>    '#hwlock-cells':
>      const: 1
>  
> @@ -33,7 +41,9 @@ required:
>    - compatible
>    - reg
>    - clocks
> +  - clock-names
>    - resets
> +  - reset-names
>    - "#hwlock-cells"
>  
>  additionalProperties: false
> @@ -47,7 +57,9 @@ examples:
>          compatible = "allwinner,sun6i-a31-hwspinlock";
>          reg = <0x01c18000 0x1000>;
>          clocks = <&ccu CLK_BUS_SPINLOCK>;
> +        clock-names = "ahb";
>          resets = <&ccu RST_BUS_SPINLOCK>;
> +        reset-names = "ahb";
>          #hwlock-cells = <1>;
>      };
>  ...

Re: [PATCH v2 4/5] dt-bindings: hwlock: sun6i: Add a64 compatible string

[Andre Przywara]

On Wed, 15 Feb 2023 21:37:09 +0100
Bastian Germann <bage@debian.org> wrote:

> Add an allwinner,sun50i-a64-hwspinlock compatible string to the device
> tree binding. The A31 and A64 have compatible hwspinlocks.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 4 +++-
>  1 file changed, 3 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 6cdfe22deb3c..281c285282a1 100644
> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> @@ -15,7 +15,9 @@ description:
>  
>  properties:
>    compatible:
> -    const: allwinner,sun6i-a31-hwspinlock
> +    items:
> +      - const: allwinner,sun6i-a31-hwspinlock
> +      - const: allwinner,sun50i-a64-hwspinlock

So this would not allow the previous single compatible string, which I
think we should preserve?
You can check out other bindings (like allwinner,sun6i-a31-spi.yaml) to
get an idea of how this could be written.

Cheers,
Andre


>  
>    reg:
>      maxItems: 1

Re: [PATCH v2 3/5] dt-bindings: hwlock: sun6i: Add missing names

[Bastian Germann]

Am 15.02.23 um 21:45 schrieb Andre Przywara:
> On Wed, 15 Feb 2023 21:37:08 +0100
> Bastian Germann <bage@debian.org> wrote:
> 
>> The allwinner,sun6i-a31-hwspinlock.yaml binding needs clock-names
>> and reset-names set to "ahb" as required by the Linux driver.
> 
> Mmmh, but I thought that Krzysztof pretty clearly NAKed this?
> So we have to either reach consensus on this or find another solution,
> like teaching the driver to comply with the existing binding.
> We could for instance get the first clock, should the devm_clk_get()
> call fail.

Either way - I wanted to send a fix for the dt-binding example as Rob requested.
This is not to say that I want to ignore the NAK.

> Cheers,
> Andre
> 
> 
>>
>> Fixes: f9e784dcb63f ("dt-bindings: hwlock: add sun6i_hwspinlock")
>> Signed-off-by: Bastian Germann <bage@debian.org>
>> ---
>>   .../hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
>> index 38478dad8b25..6cdfe22deb3c 100644
>> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
>> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
>> @@ -23,9 +23,17 @@ properties:
>>     clocks:
>>       maxItems: 1
>>   
>> +  clock-names:
>> +    items:
>> +      - const: ahb
>> +
>>     resets:
>>       maxItems: 1
>>   
>> +  reset-names:
>> +    items:
>> +      - const: ahb
>> +
>>     '#hwlock-cells':
>>       const: 1
>>   
>> @@ -33,7 +41,9 @@ required:
>>     - compatible
>>     - reg
>>     - clocks
>> +  - clock-names
>>     - resets
>> +  - reset-names
>>     - "#hwlock-cells"
>>   
>>   additionalProperties: false
>> @@ -47,7 +57,9 @@ examples:
>>           compatible = "allwinner,sun6i-a31-hwspinlock";
>>           reg = <0x01c18000 0x1000>;
>>           clocks = <&ccu CLK_BUS_SPINLOCK>;
>> +        clock-names = "ahb";
>>           resets = <&ccu RST_BUS_SPINLOCK>;
>> +        reset-names = "ahb";
>>           #hwlock-cells = <1>;
>>       };
>>   ...
> 

Re: [PATCH v2 5/5] arm64: dts: allwinner: a64: Add hwspinlock node

[Andre Przywara]

On Wed, 15 Feb 2023 21:37:10 +0100
Bastian Germann <bage@debian.org> wrote:

Hi,

> Add the hwspinlock to A64 which is already implemented for A31.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>

This looks alright to me, but there is still the debate what to do with
the names properties. So holding any tags back until this is resolved.

Cheers,
Andre

> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 77b5349f6087..f2ecc21f06ed 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -851,6 +851,17 @@ wdt0: watchdog@1c20ca0 {
>  			clocks = <&osc24M>;
>  		};
>  
> +		hwspinlock: hwlock@1c18000 {
> +			compatible = "allwinner,sun50i-a64-hwspinlock",
> +				     "allwinner,sun6i-a31-hwspinlock";
> +			reg = <0x01c18000 0x1000>;
> +			clocks = <&ccu CLK_BUS_SPINLOCK>;
> +			clock-names = "ahb";
> +			resets = <&ccu RST_BUS_SPINLOCK>;
> +			reset-names = "ahb";
> +			#hwlock-cells = <1>;
> +		};
> +
>  		spdif: spdif@1c21000 {
>  			#sound-dai-cells = <0>;
>  			compatible = "allwinner,sun50i-a64-spdif",

Re: [PATCH v2 4/5] dt-bindings: hwlock: sun6i: Add a64 compatible string

[Rob Herring]

On Wed, 15 Feb 2023 21:37:09 +0100, Bastian Germann wrote:
> Add an allwinner,sun50i-a64-hwspinlock compatible string to the device
> tree binding. The A31 and A64 have compatible hwspinlocks.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.example.dtb: hwlock@1c18000: compatible: ['allwinner,sun6i-a31-hwspinlock'] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230215203711.6293-5-bage@debian.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 2/5] dt-bindings: hwlock: sun6i: Add #hwlock-cells to example

[Andre Przywara]

On Wed, 15 Feb 2023 21:43:50 +0100
Bastian Germann <bage@debian.org> wrote:

> Am 15.02.23 um 21:40 schrieb Andre Przywara:
> > On Wed, 15 Feb 2023 21:37:07 +0100
> > Bastian Germann <bage@debian.org> wrote:
> > 
> > Hi,
> >   
> >> The dt-bindings tools will compile the yaml dt examples
> >> and this prevents an error about this node not existing.  
> > 
> > This needs to be part of patch 1/5, otherwise it will break
> > bisecting. Just squash the two patches together.  
> 
> Is this also okay with 1/5 already being applied in linux-next?

The idea of -next is to spot those things before they cause problems. I
don't know if it's already too late, let's see what Bjorn says (see
the other mail).

You could either send a totally separate fix patch (outside of a
series), or a new series with the correct patch. But for now let's wait
how this ends up.

Cheers,
Andre

> 
> > 
> > Cheers,
> > Andre
> >   
> >> Signed-off-by: Bastian Germann <bage@debian.org>
> >> ---
> >>   .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml          | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> >> index 01b1bbb3061f..38478dad8b25 100644
> >> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> >> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> >> @@ -48,5 +48,6 @@ examples:
> >>           reg = <0x01c18000 0x1000>;
> >>           clocks = <&ccu CLK_BUS_SPINLOCK>;
> >>           resets = <&ccu RST_BUS_SPINLOCK>;
> >> +        #hwlock-cells = <1>;
> >>       };
> >>   ...  
> >   
> 

Re: [PATCH v2 3/5] dt-bindings: hwlock: sun6i: Add missing names

[Krzysztof Kozlowski]

On 15/02/2023 21:37, Bastian Germann wrote:
> The allwinner,sun6i-a31-hwspinlock.yaml binding needs clock-names and
> reset-names set to "ahb" as required by the Linux driver.
> 
> Fixes: f9e784dcb63f ("dt-bindings: hwlock: add sun6i_hwspinlock") 
> Signed-off-by: Bastian Germann <bage@debian.org>

With new data, I changed my opinion and NAKed this. Still NAK, sorry.
Please drop the clock/reset-names from the driver (use indices) and DTS.

NAK means Not-acknowledge. Usually you should not send the same patch
after getting NAK, because it looks like you ignore the comment.

Best regards,
Krzysztof

Re: [PATCH v2 5/5] arm64: dts: allwinner: a64: Add hwspinlock node

[Krzysztof Kozlowski]

On 15/02/2023 21:37, Bastian Germann wrote:
> Add the hwspinlock to A64 which is already implemented for A31.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 77b5349f6087..f2ecc21f06ed 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -851,6 +851,17 @@ wdt0: watchdog@1c20ca0 {
>  			clocks = <&osc24M>;
>  		};
>  
> +		hwspinlock: hwlock@1c18000 {
> +			compatible = "allwinner,sun50i-a64-hwspinlock",
> +				     "allwinner,sun6i-a31-hwspinlock";
> +			reg = <0x01c18000 0x1000>;
> +			clocks = <&ccu CLK_BUS_SPINLOCK>;
> +			clock-names = "ahb";

Please drop.

> +			resets = <&ccu RST_BUS_SPINLOCK>;
> +			reset-names = "ahb";

Please drop.

Fix the driver instead.

Best regards,
Krzysztof

Re: [PATCH v2 4/5] dt-bindings: hwlock: sun6i: Add a64 compatible string

[Krzysztof Kozlowski]

On 15/02/2023 21:37, Bastian Germann wrote:
> Add an allwinner,sun50i-a64-hwspinlock compatible string to the device
> tree binding. The A31 and A64 have compatible hwspinlocks.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml       | 4 +++-
>  1 file changed, 3 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 6cdfe22deb3c..281c285282a1 100644
> --- a/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> +++ b/Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
> @@ -15,7 +15,9 @@ description:
>  
>  properties:
>    compatible:
> -    const: allwinner,sun6i-a31-hwspinlock
> +    items:
> +      - const: allwinner,sun6i-a31-hwspinlock
> +      - const: allwinner,sun50i-a64-hwspinlock

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).

That's my template answer. More detailed:

Please test that your DTS changes always pass your binding changes. The
above looks reversed with your DTS, thus if you tested DTS, you would
see errors. Expecting that existing in-kernel DTS passes dtbs_check is a
bit too much (although Samsung Exynos is almost there!), but at least
new code could not introduce new errors.

Best regards,
Krzysztof

Re: [PATCH v2 2/5] dt-bindings: hwlock: sun6i: Add #hwlock-cells to example

[Krzysztof Kozlowski]

On 15/02/2023 21:37, Bastian Germann wrote:
> The dt-bindings tools will compile the yaml dt examples
> and this prevents an error about this node not existing.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  .../bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml          | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

so Bjorn can fix his tree. Can be also squashed with original one.

Best regards,
Krzysztof

Re: [PATCH v2 3/5] dt-bindings: hwlock: sun6i: Add missing names

[Wilken Gottwalt]

On Thu, 16 Feb 2023 09:36:08 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 15/02/2023 21:37, Bastian Germann wrote:
> > The allwinner,sun6i-a31-hwspinlock.yaml binding needs clock-names and
> > reset-names set to "ahb" as required by the Linux driver.
> > 
> > Fixes: f9e784dcb63f ("dt-bindings: hwlock: add sun6i_hwspinlock") 
> > Signed-off-by: Bastian Germann <bage@debian.org>
> 
> With new data, I changed my opinion and NAKed this. Still NAK, sorry.
> Please drop the clock/reset-names from the driver (use indices) and DTS.

I won't be able to fix this in the next time. I'm currently in the state of
moving and can't set up my hardware to test the changes. And I'm not willing
to submit changes without testing. And with testing I really mean testing it
against a running Crust firmware which touches the hwspinlock unit.

If someone wants to change that, I'm happy to see it working out, but please
do it properly. Just testing the locks in Linux only is not sufficient. If
some directions are required, I still have my working repo up at Github:
https://github.com/wgottwalt/sunxi_hwspinlock
It may be a bit dated, but should be a good start.

Greetings,
Will