Re: [PATCH 5/5] dt-bindings: clocks: at91sam9x5-sckc: convert to yaml

[Claudiu.Beznea]

On 04.05.2023 15:50, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 04/05/2023 08:07, Claudiu Beznea wrote:
>> Convert Atmel slow clock controller documentation to yaml.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
>> ---
>>  .../devicetree/bindings/clock/at91-clock.txt  | 30 -------
>>  .../bindings/clock/atmel,at91sam9x5-sckc.yaml | 84 +++++++++++++++++++
>>  2 files changed, 84 insertions(+), 30 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/clock/at91-clock.txt
>>  create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
>> deleted file mode 100644
>> index 57394785d3b0..000000000000
>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
>> +++ /dev/null
>> @@ -1,30 +0,0 @@
>> -Device Tree Clock bindings for arch-at91
>> -
>> -This binding uses the common clock binding[1].
>> -
>> -[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> -
>> -Slow Clock controller:
>> -
>> -Required properties:
>> -- compatible : shall be one of the following:
>> -     "atmel,at91sam9x5-sckc",
>> -     "atmel,sama5d3-sckc",
>> -     "atmel,sama5d4-sckc" or
>> -     "microchip,sam9x60-sckc":
>> -             at91 SCKC (Slow Clock Controller)
>> -- #clock-cells : shall be 1 for "microchip,sam9x60-sckc" otherwise shall be 0.
>> -- clocks : shall be the input parent clock phandle for the clock.
>> -
>> -Optional properties:
>> -- atmel,osc-bypass : boolean property. Set this when a clock signal is directly
>> -  provided on XIN.
>> -
>> -For example:
>> -     sckc@fffffe50 {
>> -             compatible = "atmel,at91sam9x5-sckc";
>> -             reg = <0xfffffe50 0x4>;
>> -             clocks = <&slow_xtal>;
>> -             #clock-cells = <0>;
>> -     };
>> -
>> diff --git a/Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml
>> new file mode 100644
>> index 000000000000..62660c823ea1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml
>> @@ -0,0 +1,84 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/clock/atmel,at91sam9x5-sckc.yaml#";
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> 
> Drop quotes.
> 
>> +
>> +title: Atmel Slow Clock Controller (SCKC)
>> +
>> +maintainers:
>> +  - Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
> 
> Drop items here.
> 
>> +          - enum:
>> +              - atmel,at91sam9x5-sckc
>> +              - atmel,sama5d4-sckc
>> +              - atmel,sama5d3-sckc
> 
> Keep order by name, so 5d3 and then 5d4.
> 
>> +              - microchip,sam9x60-sckc
>> +      - items:
>> +          - const: microchip,sama7g5-sckc
>> +          - const: microchip,sam9x60-sckc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  "#clock-cells":
>> +    enum: [0, 1]
>> +
>> +  atmel,osc-bypass:
>> +    type: boolean
>> +    description: set when a clock signal is directly provided on XIN
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
> 
> put it after allOf, just like previous patch.
> 
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - microchip,sam9x60-sckc
>> +              - microchip,sama7g5-sckc
> 
> Drop this one. It's not needed.
> 
>> +    then:
>> +      properties:
>> +        "#clock-cells":
>> +          const: 1
>> +      required:
>> +        - "#clock-cells"
>> +        - clocks
> 
> Move these to top-level.
> 
>> +    else:
>> +      properties:
>> +        "#clock-cells":
>> +          const: 0
>> +      if:
>> +        properties:
>> +          compatible:
>> +            contains:
>> +              enum:
>> +                - atmel,sama5d4-sckc
>> +                - atmel,sama5d3-sckc
>> +      then:
>> +        required:
>> +          - "#clock-cells"
>> +          - clocks
> 
> and drop these required... but this is if-within-else. Very confusing.
> Which case you want to handle that way? So other compatibles do not need
> clock cells or clocks?

Actually, I think I messed this up a bit. I'll double check.

Thank you for your review,
Claudiu

> 
> 
> Best regards,
> Krzysztof
>