Re: [PATCH v2 1/2] dt-bindings: mmc: sdhci-atmel: Convert to json schema

[<Dharma.B@microchip.com>]

Hi Conor,

On 30/08/24 7:18 pm, Conor Dooley wrote:
> On Fri, Aug 30, 2024 at 01:49:42PM +0530, Dharma Balasubiramani wrote:
>> Convert sdhci-atmel documentation to yaml format. The new file will inherit
>> from sdhci-common.yaml.
>>
>> Note: Add microchip,sama7g5-sdhci to compatible list as we already use it
>> in the DT.
>>
>> Signed-off-by: Dharma Balasubiramani<dharma.b@microchip.com>
>> ---
>>   .../bindings/mmc/atmel,sama5d2-sdhci.yaml          | 98 ++++++++++++++++++++++
>>   .../devicetree/bindings/mmc/sdhci-atmel.txt        | 35 --------
>>   2 files changed, 98 insertions(+), 35 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/atmel,sama5d2-sdhci.yaml b/Documentation/devicetree/bindings/mmc/atmel,sama5d2-sdhci.yaml
>> new file mode 100644
>> index 000000000000..91d18b2545e1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/atmel,sama5d2-sdhci.yaml
>> @@ -0,0 +1,98 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id:http://devicetree.org/schemas/mmc/atmel,sama5d2-sdhci.yaml#
>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel SDHCI controller
>> +
>> +maintainers:
>> +  - Aubin Constans<aubin.constans@microchip.com>
>> +  - Nicolas Ferre<nicolas.ferre@microchip.com>
>> +
>> +description:
>> +  Bindings for the SDHCI controller found in Atmel/Microchip SoCs.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - enum:
>> +          - atmel,sama5d2-sdhci
>> +          - atmel,sama5d3-sdhci
>> +          - atmel,sama5d4-sdhci
>> +          - microchip,sam9x60-sdhci
>> +      - items:
>> +          - enum:
>> +              - microchip,sama7g5-sdhci
>> +          - const: microchip,sam9x60-sdhci
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    minItems: 2
>> +    maxItems: 3
>> +    description: |
>> +      The sama5d2 family includes three clocks: `hclock`, `multclk`, and `baseclk`.
>> +      For other families, including sam9x60 and sam9x7, only `hclock` and `multclk` are used.
> This should instead be an items list, rather than a text based
> description.
Sure, I will update it like this below

clocks:
   items:
     - description: hclock
     - description: multclk
     - description: baseclk
   minItems: 2
   maxItems: 3

> 
>> +
>> +  clock-names:
>> +    minItems: 2
>> +    maxItems: 3
> We prefer that you describe the entries at this level, and constrain
> them in the conditional bits below. IOW, move the items list here,
> and only use minItems/maxItems below.

Sure, I will move the items list here

clock-names:
   items:
     - const: hclock
     - const: multclk
     - const: baseclk
   minItems: 2
   maxItems: 3

> 
>> +
>> +  microchip,sdcal-inverted:
>> +    type: boolean
>> +    description:
>> +      When present, polarity on the SDCAL SoC pin is inverted.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +
>> +allOf:
>> +  - $ref: sdhci-common.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - atmel,sama5d2-sdhci
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 3
> maxItems: 3 or minItems: 3? Your answer will depend on whether or not
> baseclk is optional on sama5d2.

baseclk is must, hence I will modify it as below

allOf:
   - $ref: sdhci-common.yaml#
   - if:
       properties:
         compatible:
           contains:
             enum:
               - atmel,sama5d2-sdhci
     then:
       properties:
         clocks:
           minItems: 3
         clock-names:
           minItems: 3


> 
> Cheers,
> Conor.
> 
>> +        clock-names:
>> +          items:
>> +            - const: hclock
>> +            - const: multclk
>> +            - const: baseclk
>> +    else:
>> +      properties:
>> +        clocks:
>> +          maxItems: 2
>> +        clock-names:
>> +          items:
>> +            - const: hclock
>> +            - const: multclk
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/clock/at91.h>
>> +    mmc@a0000000 {
>> +        compatible = "atmel,sama5d2-sdhci";
>> +        reg = <0xa0000000 0x300>;
>> +        interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
>> +        clocks = <&sdmmc0_hclk>, <&sdmmc0_gclk>, <&main>;
>> +        clock-names = "hclock", "multclk", "baseclk";
>> +        assigned-clocks = <&sdmmc0_gclk>;
>> +        assigned-clock-rates = <480000000>;
>> +    };


-- 
With Best Regards,
Dharma B.