diff mbox series

[v9,03/15] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC

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

Commit Message

Brad Larson Jan. 19, 2023, 3:51 a.m. UTC 
  Document the cadence qspi controller compatible for AMD Pensando
Elba SoC boards.  The Elba qspi fifo size is 1024.

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

---

Changes since v6:
- Add 1024 to cdns,fifo-depth property to resolve dtbs_check error

---
 .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Jan. 19, 2023, 7:53 a.m. UTC | #1 
On 19/01/2023 04:51, Brad Larson wrote:
> Document the cadence qspi controller compatible for AMD Pensando
> Elba SoC boards.  The Elba qspi fifo size is 1024.
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> 
> ---
> 
> Changes since v6:
> - Add 1024 to cdns,fifo-depth property to resolve dtbs_check error
> 
> ---
>  .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index 4707294d8f59..a6556854234f 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -20,11 +20,23 @@ allOf:
>        required:
>          - power-domains
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - amd,pensando-elba-qspi
> +    then:
> +      properties:
> +        cdns,fifo-depth:
> +          enum: [ 128, 256, 1024 ]
> +          default: 1024

This won't work either... did you test it? Is 1024 really allowed?

> +
>  properties:
>    compatible:
>      oneOf:
>        - items:
>            - enum:
> +              - amd,pensando-elba-qspi
>                - ti,k2g-qspi
>                - ti,am654-ospi
>                - intel,lgm-qspi
> @@ -48,7 +60,7 @@ properties:
>      description:
>        Size of the data FIFO in words.
>      $ref: "/schemas/types.yaml#/definitions/uint32"
> -    enum: [ 128, 256 ]
> +    enum: [ 128, 256, 1024 ]

The answer is here - your change is meaningless... I mean, really think
about it, why do you allow 1024 only for your variant and then
immediately allow for all variants? This does not make sense.

If you tested with proper patch you would see:

elba-asic.dtb: spi@2400: cdns,fifo-depth:0:0: 1024 is not one of [128, 256]


>      default: 128
>  
>    cdns,fifo-width:

Best regards,
Krzysztof
Brad Larson Jan. 24, 2023, 1:16 a.m. UTC | #2 
On 19/01/2023 7:53 UTC, Krzysztof Kozlowski wrote:
>On 19/01/2023 04:51, Brad Larson wrote:
>> Document the cadence qspi controller compatible for AMD Pensando
>> Elba SoC boards.  The Elba qspi fifo size is 1024.
>> 
...
>> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> index 4707294d8f59..a6556854234f 100644
>> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> @@ -20,11 +20,23 @@ allOf:
>>        required:
>>          - power-domains
>>  
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          enum:
>> +            - amd,pensando-elba-qspi
>> +    then:
>> +      properties:
>> +        cdns,fifo-depth:
>> +          enum: [ 128, 256, 1024 ]
>> +          default: 1024
>
>This won't work either... did you test it? Is 1024 really allowed?

Removing enum/default from the cdns,fifo-depth property definition
and using if/then/else in the allOf block fixed the problem.

Yes the Cadence IP 1024 fifo-size is supported and is in production.
The below passes the dtbs_check/dt_binding_check and leaves all
other implemenations with max 256 fifo-depth.

Do you want to move the allOf block to the bottom as was requested
for cdns,sdhci.yaml?  This is the updated diff:

--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -20,11 +20,28 @@ allOf:
       required:
         - power-domains
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amd,pensando-elba-qspi
+    then:
+      properties:
+        cdns,fifo-depth:
+          enum: [ 128, 256, 1024 ]
+          default: 1024
+    else:
+      properties:
+        cdns,fifo-depth:
+          enum: [ 128, 256 ]
+          default: 128
+
 properties:
   compatible:
     oneOf:
       - items:
           - enum:
+              - amd,pensando-elba-qspi
               - ti,k2g-qspi
               - ti,am654-ospi
               - intel,lgm-qspi
@@ -48,8 +65,6 @@ properties:
     description:
       Size of the data FIFO in words.
     $ref: "/schemas/types.yaml#/definitions/uint32"
-    enum: [ 128, 256 ]
-    default: 128
 
   cdns,fifo-width:
     $ref: /schemas/types.yaml#/definitions/uint32

Regards,
Brad
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index 4707294d8f59..a6556854234f 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -20,11 +20,23 @@  allOf:
       required:
         - power-domains
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - amd,pensando-elba-qspi
+    then:
+      properties:
+        cdns,fifo-depth:
+          enum: [ 128, 256, 1024 ]
+          default: 1024
+
 properties:
   compatible:
     oneOf:
       - items:
           - enum:
+              - amd,pensando-elba-qspi
               - ti,k2g-qspi
               - ti,am654-ospi
               - intel,lgm-qspi
@@ -48,7 +60,7 @@  properties:
     description:
       Size of the data FIFO in words.
     $ref: "/schemas/types.yaml#/definitions/uint32"
-    enum: [ 128, 256 ]
+    enum: [ 128, 256, 1024 ]
     default: 128
 
   cdns,fifo-width: