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

Claudiu.Beznea
2023-05-05T12:51:38Z

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@microchip.com>
>> ---
>> .../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@microchip.com>
>> +
>> +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
>