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

[Andre Przywara <andre.przywara@arm.com>]

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
> 
>