mbox series

[v2,0/2] Update sdhci-atmel dt-binding documentation

Message ID 20240830-atmel-sdhci-v2-0-b7f58973f3fc@microchip.com
Headers show
Series Update sdhci-atmel dt-binding documentation | expand

Message

Dharma Balasubiramani Aug. 30, 2024, 8:19 a.m. UTC
This patch series converts the sdhci-atmel dt-binding to yaml format and adds
the sama7d65,sama7g5 compatibles to the list.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Changes in v2:
- Add missing deleted file to the patch 
"Documentation/devicetree/bindings/mmc/sdhci-atmel.txt"
- Link to v1: https://lore.kernel.org/r/20240830-atmel-sdhci-v1-0-01e3ec8c9804@microchip.com

---
Dharma Balasubiramani (2):
      dt-bindings: mmc: sdhci-atmel: Convert to json schema
      dt-bindings: mmc: atmel,sama5d2-sdhci: Add sama7d65 compatible

 .../bindings/mmc/atmel,sama5d2-sdhci.yaml          | 103 +++++++++++++++++++++
 .../devicetree/bindings/mmc/sdhci-atmel.txt        |  35 -------
 2 files changed, 103 insertions(+), 35 deletions(-)
---
base-commit: 985bf40edf4343dcb04c33f58b40b4a85c1776d4
change-id: 20240830-atmel-sdhci-c9a92b710624

Best regards,

Comments

Dharma Balasubiramani Aug. 30, 2024, 2:12 p.m. UTC | #1
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>;
>> +    };
Dharma Balasubiramani Aug. 30, 2024, 2:18 p.m. UTC | #2
On 30/08/24 7:46 pm, Conor Dooley wrote:
>> Sure, I will update it like this below
>>
>> clocks:
>>     items:
>>       - description: hclock
>>       - description: multclk
>>       - description: baseclk
>>     minItems: 2
>>     maxItems: 3
> This shouldn't pass dt_binding_check, because the max in this snippet is
> redundant due to it matching the size of the items list.
Yes, hint: "maxItems" is not needed with an "items" list
I will remove it from both clocks and clock-names properties.
Thanks.