Re: [RFC PATCH 1/7] dt-bindings: rtc: sun6i: Add H616 and R329 compatibles

[Rob Herring]

On Wed, Sep 01, 2021 at 12:39:45AM -0500, Samuel Holland wrote:
> For these new SoCs, start requiring a complete list of input clocks.
> 
> For H616, this means bus, hosc, and pll-32k. For R329, this means ahb,
> bus, and hosc; and optionally ext-osc32k.
> 
> I'm not sure how to best represent this in the binding...
> 
> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> ---
>  .../bindings/rtc/allwinner,sun6i-a31-rtc.yaml | 55 +++++++++++++++++--
>  include/dt-bindings/clock/sun50i-rtc.h        | 12 ++++
>  2 files changed, 61 insertions(+), 6 deletions(-)
>  create mode 100644 include/dt-bindings/clock/sun50i-rtc.h
> 
> diff --git a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
> index beeb90e55727..3e085db1294f 100644
> --- a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
> @@ -26,6 +26,8 @@ properties:
>            - const: allwinner,sun50i-a64-rtc
>            - const: allwinner,sun8i-h3-rtc
>        - const: allwinner,sun50i-h6-rtc
> +      - const: allwinner,sun50i-h616-rtc
> +      - const: allwinner,sun50i-r329-rtc

Can you please make all the single entry cases a single 'enum'.

>  
>    reg:
>      maxItems: 1
> @@ -37,7 +39,24 @@ properties:
>        - description: RTC Alarm 1
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 4
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      - anyOf:

This says the first entry is any of these. What about the rest of them?

> +          - const: ahb
> +            description: AHB parent for SPI bus clock

The description should go in 'clocks'. The order should be defined as 
well with the first clock being the one that existed previously.

> +          - const: bus
> +            description: AHB/APB bus clock for register access
> +          - const: ext-osc32k
> +            description: External 32768 Hz oscillator input
> +          - const: hosc
> +            description: 24 MHz oscillator input
> +          - const: pll-32k
> +            description: 32 kHz clock divided from a PLL
>  
>    clock-output-names:
>      minItems: 1
> @@ -85,6 +104,9 @@ allOf:
>              enum:
>                - allwinner,sun8i-h3-rtc
>                - allwinner,sun50i-h5-rtc
> +              - allwinner,sun50i-h6-rtc
> +              - allwinner,sun50i-h616-rtc
> +              - allwinner,sun50i-r329-rtc
>  
>      then:
>        properties:
> @@ -96,13 +118,35 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: allwinner,sun50i-h6-rtc
> +            enum:
> +              - allwinner,sun50i-h616-rtc
> +              - allwinner,sun50i-r329-rtc
>  
>      then:
> +      clocks:
> +        minItems: 3 # bus, hosc, and (pll-32k [H616] or ahb [R329])
> +
> +      clock-names:
> +        minItems: 3
> +
> +      required:
> +        - clock-names
> +
> +    else:
> +      required:
> +        - clock-output-names
> +
> +  - if:
> +      properties: clock-names
> +
> +    then:
> +      required:
> +        - clocks # hosc is required
> +
> +    else:
>        properties:
> -        clock-output-names:
> -          minItems: 3
> -          maxItems: 3
> +        clocks:
> +          maxItems: 1 # only ext-osc32k is allowed
>  
>    - if:
>        properties:
> @@ -127,7 +171,6 @@ required:
>    - compatible
>    - reg
>    - interrupts
> -  - clock-output-names
>  
>  additionalProperties: false
>  
> diff --git a/include/dt-bindings/clock/sun50i-rtc.h b/include/dt-bindings/clock/sun50i-rtc.h
> new file mode 100644
> index 000000000000..d45e3ff4e105
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun50i-rtc.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license please.

> +
> +#ifndef _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_
> +#define _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_
> +
> +#define CLK_OSC32K		0
> +#define CLK_OSC32K_FANOUT	1
> +#define CLK_IOSC		2
> +
> +#define CLK_RTC_SPI		8
> +
> +#endif /* _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ */
> -- 
> 2.31.1
> 
>