diff mbox series

[v10,02/15] dt-bindings: mmc: cdns: Add AMD Pensando Elba SoC

Message ID 20230306040739.51488-3-blarson@amd.com
State New
Headers
Series Support AMD Pensando Elba SoC |

Commit Message

Brad Larson March 6, 2023, 4:07 a.m. UTC 
  AMD Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables.

Signed-off-by: Brad Larson <blarson@amd.com>
---

v10 changes:
- Move reset-names property definition next to existing resets prop
- Move allOf to the bottom and set resets/reset-names required only for pensando
- Fix reg maxItems for existing, must be 1

v9 changes:
- Add reset-names and resets properties
- Add if/then on property amd,pensando-elba-sd4hc to set reg property
  values for minItems and maxItems

---
 .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 33 ++++++++++++++++---
 1 file changed, 29 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski March 6, 2023, 8:28 a.m. UTC | #1 
On 06/03/2023 05:07, Brad Larson wrote:
> AMD Pensando Elba ARM 64-bit SoC is integrated with this IP and
> explicitly controls byte-lane enables.
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
> 
> v10 changes:
> - Move reset-names property definition next to existing resets prop
> - Move allOf to the bottom and set resets/reset-names required only for pensando
> - Fix reg maxItems for existing, must be 1
> 
> v9 changes:
> - Add reset-names and resets properties
> - Add if/then on property amd,pensando-elba-sd4hc to set reg property
>   values for minItems and maxItems
> 
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 33 ++++++++++++++++---
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index adacd0535c14..0c4d6d4b2b58 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -9,19 +9,18 @@ title: Cadence SD/SDIO/eMMC Host Controller (SD4HC)
>  maintainers:
>    - Masahiro Yamada <yamada.masahiro@socionext.com>
>  
> -allOf:
> -  - $ref: mmc-controller.yaml
> -
>  properties:
>    compatible:
>      items:
>        - enum:
> +          - amd,pensando-elba-sd4hc
>            - microchip,mpfs-sd4hc
>            - socionext,uniphier-sd4hc
>        - const: cdns,sd4hc
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    interrupts:
>      maxItems: 1
> @@ -30,8 +29,13 @@ properties:
>      maxItems: 1
>  
>    resets:
> +    description: physical line number to hardware reset the mmc

This part seems to be not needed anymore. Resets field was already added.

>      maxItems: 1
>  
> +  reset-names:
> +    items:
> +      - const: hw

Why did you add reset-names for one item? There is no v8 of this patch,
so I cannot find previous discussion about it.


>    # PHY DLL input delays:
>    # They are used to delay the data valid window, and align the window to
>    # sampling clock. The delay starts from 5ns (for delay parameter equal to 0)
> @@ -120,6 +124,27 @@ required:
>    - interrupts
>    - clocks
>  
> +allOf:
> +  - $ref: mmc-controller.yaml
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amd,pensando-elba-sd4hc
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2

Hm, we missed to mention it before, but what is the second reg for? It's
not obvious from the binding so probably you need to describe it instead
minItems:
  items:
    - description: foo
    - description: bar

> +      required:
> +        - reset-names
> +        - resets
> +    else:
> +      properties:
> +        reset-names: false
> +        resets: false
> +        reg:
> +          maxItems: 1
> +
>  unevaluatedProperties: false
>  
>  examples:

Best regards,
Krzysztof
Brad Larson March 7, 2023, 2:11 a.m. UTC | #2 
On 06/03/2023 8:28, Krzysztof Kozlowski wrote: 
> On 06/03/2023 05:07, Brad Larson wrote:
>> AMD Pensando Elba ARM 64-bit SoC is integrated with this IP and
>> explicitly controls byte-lane enables.
>> 
>> Signed-off-by: Brad Larson <blarson@amd.com>
>> ---
>> 
>> v10 changes:
>> - Move reset-names property definition next to existing resets prop
>> - Move allOf to the bottom and set resets/reset-names required only for pensando
>> - Fix reg maxItems for existing, must be 1
>> 
>> v9 changes:
>> - Add reset-names and resets properties
>> - Add if/then on property amd,pensando-elba-sd4hc to set reg property
>>   values for minItems and maxItems
>> 
>> ---
>>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 33 ++++++++++++++++---
>>  1 file changed, 29 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
>> index adacd0535c14..0c4d6d4b2b58 100644
>> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
>> @@ -9,19 +9,18 @@ title: Cadence SD/SDIO/eMMC Host Controller (SD4HC)
>>  maintainers:
>>    - Masahiro Yamada <yamada.masahiro@socionext.com>
>>  
>> -allOf:
>> -  - $ref: mmc-controller.yaml
>> -
>>  properties:
>>    compatible:
>>      items:
>>        - enum:
>> +          - amd,pensando-elba-sd4hc
>>            - microchip,mpfs-sd4hc
>>            - socionext,uniphier-sd4hc
>>        - const: cdns,sd4hc
>>  
>>    reg:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    interrupts:
>>      maxItems: 1
>> @@ -30,8 +29,13 @@ properties:
>>      maxItems: 1
>>  
>>    resets:
>> +    description: physical line number to hardware reset the mmc
>
> This part seems to be not needed anymore. Resets field was already added.

Yes, see below.

>
>>      maxItems: 1
>>  
>> +  reset-names:
>> +    items:
>> +      - const: hw
>
> Why did you add reset-names for one item? There is no v8 of this patch,
> so I cannot find previous discussion about it.

I found resets property was added recently when I rebased.

cb7f090171393 (Kunihiko Hayashi    2023-02-13 13:52:33 +0900  32)   resets:
cb7f090171393 (Kunihiko Hayashi    2023-02-13 13:52:33 +0900  33)     maxItems: 1

I've deleted reset-names and dropped description for resets.

>
>>    # PHY DLL input delays:
>>    # They are used to delay the data valid window, and align the window to
>>    # sampling clock. The delay starts from 5ns (for delay parameter equal to 0)
>> @@ -120,6 +124,27 @@ required:
>>    - interrupts
>>    - clocks
>>  
>> +allOf:
>> +  - $ref: mmc-controller.yaml
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: amd,pensando-elba-sd4hc
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 2
>
> Hm, we missed to mention it before, but what is the second reg for? It's
> not obvious from the binding so probably you need to describe it instead
> minItems:
>   items:
>     - description: foo
>     - description: bar
>

The second reg is byte lane enable for writes.  The following passed dtbs_check
after getting: hint: "minItems" is only needed if less than the "items" list length

    then:
      properties:
        reg:
          items:
            - description: Host controller registers
            - description: Elba byte-lane enable register for writes
      required:
        - resets
    else:
      properties:
        resets: false
        reg:
          maxItems: 1

>> +      required:
>> +        - reset-names
>> +        - resets
>> +    else:
>> +      properties:
>> +        reset-names: false
>> +        resets: false
>> +        reg:
>> +          maxItems: 1
>> +
>>  unevaluatedProperties: false

Regards,
Brad
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
index adacd0535c14..0c4d6d4b2b58 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -9,19 +9,18 @@  title: Cadence SD/SDIO/eMMC Host Controller (SD4HC)
 maintainers:
   - Masahiro Yamada <yamada.masahiro@socionext.com>
 
-allOf:
-  - $ref: mmc-controller.yaml
-
 properties:
   compatible:
     items:
       - enum:
+          - amd,pensando-elba-sd4hc
           - microchip,mpfs-sd4hc
           - socionext,uniphier-sd4hc
       - const: cdns,sd4hc
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   interrupts:
     maxItems: 1
@@ -30,8 +29,13 @@  properties:
     maxItems: 1
 
   resets:
+    description: physical line number to hardware reset the mmc
     maxItems: 1
 
+  reset-names:
+    items:
+      - const: hw
+
   # PHY DLL input delays:
   # They are used to delay the data valid window, and align the window to
   # sampling clock. The delay starts from 5ns (for delay parameter equal to 0)
@@ -120,6 +124,27 @@  required:
   - interrupts
   - clocks
 
+allOf:
+  - $ref: mmc-controller.yaml
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amd,pensando-elba-sd4hc
+    then:
+      properties:
+        reg:
+          minItems: 2
+      required:
+        - reset-names
+        - resets
+    else:
+      properties:
+        reset-names: false
+        resets: false
+        reg:
+          maxItems: 1
+
 unevaluatedProperties: false
 
 examples: