diff mbox series

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

Message ID 20230215203711.6293-4-bage@debian.org
State New
Headers show
Series Enable hwlock on Allwinner A64 | expand

Commit Message

Bastian Germann Feb. 15, 2023, 8:37 p.m. UTC
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(+)

Comments

Krzysztof Kozlowski Feb. 16, 2023, 8:36 a.m. UTC | #1
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
Wilken Gottwalt Feb. 16, 2023, 8:54 a.m. UTC | #2
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
diff mbox series

Patch

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>;
     };
 ...