diff mbox series

[v3,2/4] dt-bindings: clock: sun55i-a523-ccu: Add A523 CPU CCU clock controller

Message ID 20260925225112.14081-3-andre.przywara@arm.com (mailing list archive)
State New
Headers
Series arm64: allwinner: a523: Enable CPU clocks |

Commit Message

Andre Przywara Sept. 25, 2026, 10:51 p.m. UTC 
There are four clock controllers in the A523 SoC, but only three are
described in the DT binding so far.

Add a description for the CPU CCU, which provides separate clocks for
the two CPU clusters and the DSU interconnect.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../clock/allwinner,sun55i-a523-ccu.yaml      | 38 ++++++++++++++++++-
 .../dt-bindings/clock/sun55i-a523-cpu-ccu.h   | 13 +++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clock/sun55i-a523-cpu-ccu.h

Comments

Krzysztof Kozlowski Sept. 29, 2026, 8:42 a.m. UTC | #1 
On Sat, Sep 26, 2026 at 12:51:10AM +0200, Andre Przywara wrote:
> There are four clock controllers in the A523 SoC, but only three are
> described in the DT binding so far.
> 
> Add a description for the CPU CCU, which provides separate clocks for
> the two CPU clusters and the DSU interconnect.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../clock/allwinner,sun55i-a523-ccu.yaml      | 38 ++++++++++++++++++-
>  .../dt-bindings/clock/sun55i-a523-cpu-ccu.h   | 13 +++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
> index 58be701a720ea..8d5481fb63ae8 100644
> --- a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
> +++ b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
> @@ -19,6 +19,7 @@ properties:
>    compatible:
>      enum:
>        - allwinner,sun55i-a523-ccu
> +      - allwinner,sun55i-a523-cpu-ccu
>        - allwinner,sun55i-a523-mcu-ccu
>        - allwinner,sun55i-a523-r-ccu
>  
> @@ -35,12 +36,23 @@ properties:
>  
>  required:
>    - "#clock-cells"
> -  - "#reset-cells"
>    - compatible
>    - reg
>    - clocks
>    - clock-names
>  
> +if:

This goes to the allOf block.

> +  properties:
> +    compatible:
> +      contains:
> +        const: allwinner,sun55i-a523-cpu-ccu
> +then:
> +  properties:
> +    "#reset-cells": false
> +else:
> +  required:
> +    - "#reset-cells"
> +
>  allOf:
>    - if:
>        properties:
> @@ -64,6 +76,30 @@ allOf:
>              - const: iosc
>              - const: losc-fanout
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun55i-a523-cpu-ccu
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: High Frequency Oscillator (usually at 24MHz)
> +            - description: Low Frequency Oscillator (usually at 32kHz)
> +            - description: Internal Oscillator
> +            - description: Peripherals PLL 0 (1200 MHz output)
> +            - description: Peripherals PLL 0 (600 MHz output)
> +
> +        clock-names:
> +          items:
> +            - const: hosc
> +            - const: losc
> +            - const: iosc
> +            - const: pll-periph0-2x
> +            - const: pll-periph0-600m
> +
>    - if:
>        properties:
>          compatible:
> diff --git a/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
> new file mode 100644
> index 0000000000000..042f2310f64de
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h

Filename should match compatible.

> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright 2025 Arm Ltd.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
> +#define _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
> +
> +#define CLK_CPU_L		7

Indices start from 0 or 1

Best regards,
Krzysztof
Andre Przywara Oct. 4, 2026, 10:50 p.m. UTC | #2 
On Tue, 29 Sep 2026 10:42:29 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

Hi Krzysztof,

thanks for the review!

> On Sat, Sep 26, 2026 at 12:51:10AM +0200, Andre Przywara wrote:
> > There are four clock controllers in the A523 SoC, but only three are
> > described in the DT binding so far.
> > 
> > Add a description for the CPU CCU, which provides separate clocks for
> > the two CPU clusters and the DSU interconnect.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../clock/allwinner,sun55i-a523-ccu.yaml      | 38 ++++++++++++++++++-
> >  .../dt-bindings/clock/sun55i-a523-cpu-ccu.h   | 13 +++++++
> >  2 files changed, 50 insertions(+), 1 deletion(-)
> >  create mode 100644 include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
> > index 58be701a720ea..8d5481fb63ae8 100644
> > --- a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
> > +++ b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
> > @@ -19,6 +19,7 @@ properties:
> >    compatible:
> >      enum:
> >        - allwinner,sun55i-a523-ccu
> > +      - allwinner,sun55i-a523-cpu-ccu
> >        - allwinner,sun55i-a523-mcu-ccu
> >        - allwinner,sun55i-a523-r-ccu
> >  
> > @@ -35,12 +36,23 @@ properties:
> >  
> >  required:
> >    - "#clock-cells"
> > -  - "#reset-cells"
> >    - compatible
> >    - reg
> >    - clocks
> >    - clock-names
> >  
> > +if:
> 
> This goes to the allOf block.

OK.

> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: allwinner,sun55i-a523-cpu-ccu
> > +then:
> > +  properties:
> > +    "#reset-cells": false
> > +else:
> > +  required:
> > +    - "#reset-cells"
> > +
> >  allOf:
> >    - if:
> >        properties:
> > @@ -64,6 +76,30 @@ allOf:
> >              - const: iosc
> >              - const: losc-fanout
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - allwinner,sun55i-a523-cpu-ccu
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: High Frequency Oscillator (usually at 24MHz)
> > +            - description: Low Frequency Oscillator (usually at 32kHz)
> > +            - description: Internal Oscillator
> > +            - description: Peripherals PLL 0 (1200 MHz output)
> > +            - description: Peripherals PLL 0 (600 MHz output)
> > +
> > +        clock-names:
> > +          items:
> > +            - const: hosc
> > +            - const: losc
> > +            - const: iosc
> > +            - const: pll-periph0-2x
> > +            - const: pll-periph0-600m
> > +
> >    - if:
> >        properties:
> >          compatible:
> > diff --git a/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
> > new file mode 100644
> > index 0000000000000..042f2310f64de
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
> 
> Filename should match compatible.

Is that a new requirement? We never did this for the Allwinner clock or
reset bindings, all headers are named like sun<x>i-<soc>-<name>-ccu.h.
And I think especially since we have sun55i-a523-mcu-ccu.h and
sun55i-a523-r-ccu.h, I think we should stay consistent, at least for
the existing SoCs.

> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +/*
> > + * Copyright 2025 Arm Ltd.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
> > +#define _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
> > +
> > +#define CLK_CPU_L		7
> 
> Indices start from 0 or 1

This is also something very common for Allwinner clocks: the CCU has
clocks that are not exposed to the DT, because they are internal
clocks we need to model the clock tree. We number them logically,
and often the basic clocks are not exposed. See sun50i-a64-ccu.h,
sun50i-h616-ccu.h, sun8i-h3-ccu.h, to just name a few (there are many
more). The other clocks are named in the internal headers, in the
clock driver directory.

Cheers,
Andre

 
> 
> Best regards,
> Krzysztof
> 
>
Krzysztof Kozlowski Oct. 6, 2026, 4:03 p.m. UTC | #3 
On 05/10/2026 00:50, Andre Przywara wrote:
> On Tue, 29 Sep 2026 10:42:29 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
> Hi Krzysztof,
> 
> thanks for the review!
> 
>> On Sat, Sep 26, 2026 at 12:51:10AM +0200, Andre Przywara wrote:
>>> There are four clock controllers in the A523 SoC, but only three are
>>> described in the DT binding so far.
>>>
>>> Add a description for the CPU CCU, which provides separate clocks for
>>> the two CPU clusters and the DSU interconnect.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  .../clock/allwinner,sun55i-a523-ccu.yaml      | 38 ++++++++++++++++++-
>>>  .../dt-bindings/clock/sun55i-a523-cpu-ccu.h   | 13 +++++++
>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>  create mode 100644 include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
>>> index 58be701a720ea..8d5481fb63ae8 100644
>>> --- a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
>>> @@ -19,6 +19,7 @@ properties:
>>>    compatible:
>>>      enum:
>>>        - allwinner,sun55i-a523-ccu
>>> +      - allwinner,sun55i-a523-cpu-ccu
>>>        - allwinner,sun55i-a523-mcu-ccu
>>>        - allwinner,sun55i-a523-r-ccu
>>>  
>>> @@ -35,12 +36,23 @@ properties:
>>>  
>>>  required:
>>>    - "#clock-cells"
>>> -  - "#reset-cells"
>>>    - compatible
>>>    - reg
>>>    - clocks
>>>    - clock-names
>>>  
>>> +if:
>>
>> This goes to the allOf block.
> 
> OK.
> 
>>> +  properties:
>>> +    compatible:
>>> +      contains:
>>> +        const: allwinner,sun55i-a523-cpu-ccu
>>> +then:
>>> +  properties:
>>> +    "#reset-cells": false
>>> +else:
>>> +  required:
>>> +    - "#reset-cells"
>>> +
>>>  allOf:
>>>    - if:
>>>        properties:
>>> @@ -64,6 +76,30 @@ allOf:
>>>              - const: iosc
>>>              - const: losc-fanout
>>>  
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          enum:
>>> +            - allwinner,sun55i-a523-cpu-ccu
>>> +
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          items:
>>> +            - description: High Frequency Oscillator (usually at 24MHz)
>>> +            - description: Low Frequency Oscillator (usually at 32kHz)
>>> +            - description: Internal Oscillator
>>> +            - description: Peripherals PLL 0 (1200 MHz output)
>>> +            - description: Peripherals PLL 0 (600 MHz output)
>>> +
>>> +        clock-names:
>>> +          items:
>>> +            - const: hosc
>>> +            - const: losc
>>> +            - const: iosc
>>> +            - const: pll-periph0-2x
>>> +            - const: pll-periph0-600m
>>> +
>>>    - if:
>>>        properties:
>>>          compatible:
>>> diff --git a/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
>>> new file mode 100644
>>> index 0000000000000..042f2310f64de
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
>>
>> Filename should match compatible.
> 
> Is that a new requirement? We never did this for the Allwinner clock or
> reset bindings, all headers are named like sun<x>i-<soc>-<name>-ccu.h.

I say EXACTLY the same thing for ~4 years, so yes. New requirement since
4 years. Allwinner for sure received same feedback already.

> And I think especially since we have sun55i-a523-mcu-ccu.h and
> sun55i-a523-r-ccu.h, I think we should stay consistent, at least for
> the existing SoCs.
> 
>>> @@ -0,0 +1,13 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>>> +/*
>>> + * Copyright 2025 Arm Ltd.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
>>> +#define _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
>>> +
>>> +#define CLK_CPU_L		7
>>
>> Indices start from 0 or 1
> 
> This is also something very common for Allwinner clocks: the CCU has
> clocks that are not exposed to the DT, because they are internal
> clocks we need to model the clock tree. We number them logically,

They are not ABI thus do not create impression they should be and you
prepare some placeholder for them.

These are not IDs of your clocks in the hardware. This is only the ABI.

Again, same feedback I repeat for 4 years.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
index 58be701a720ea..8d5481fb63ae8 100644
--- a/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
+++ b/Documentation/devicetree/bindings/clock/allwinner,sun55i-a523-ccu.yaml
@@ -19,6 +19,7 @@  properties:
   compatible:
     enum:
       - allwinner,sun55i-a523-ccu
+      - allwinner,sun55i-a523-cpu-ccu
       - allwinner,sun55i-a523-mcu-ccu
       - allwinner,sun55i-a523-r-ccu
 
@@ -35,12 +36,23 @@  properties:
 
 required:
   - "#clock-cells"
-  - "#reset-cells"
   - compatible
   - reg
   - clocks
   - clock-names
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: allwinner,sun55i-a523-cpu-ccu
+then:
+  properties:
+    "#reset-cells": false
+else:
+  required:
+    - "#reset-cells"
+
 allOf:
   - if:
       properties:
@@ -64,6 +76,30 @@  allOf:
             - const: iosc
             - const: losc-fanout
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun55i-a523-cpu-ccu
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: High Frequency Oscillator (usually at 24MHz)
+            - description: Low Frequency Oscillator (usually at 32kHz)
+            - description: Internal Oscillator
+            - description: Peripherals PLL 0 (1200 MHz output)
+            - description: Peripherals PLL 0 (600 MHz output)
+
+        clock-names:
+          items:
+            - const: hosc
+            - const: losc
+            - const: iosc
+            - const: pll-periph0-2x
+            - const: pll-periph0-600m
+
   - if:
       properties:
         compatible:
diff --git a/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
new file mode 100644
index 0000000000000..042f2310f64de
--- /dev/null
+++ b/include/dt-bindings/clock/sun55i-a523-cpu-ccu.h
@@ -0,0 +1,13 @@ 
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Copyright 2025 Arm Ltd.
+ */
+
+#ifndef _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
+#define _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_
+
+#define CLK_CPU_L		7
+#define CLK_CPU_DSU		8
+#define CLK_CPU_B		9
+
+#endif /* _DT_BINDINGS_CLK_SUN55I_A523_CPU_CCU_H_ */