diff mbox series

[v4,2/5] dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for Cadence XSPI

Message ID 20240509010523.3152264-3-wsadowski@marvell.com
State New
Headers show
Series Marvell HW overlay support for Cadence xSPI | expand

Commit Message

Witold Sadowski May 9, 2024, 1:05 a.m. UTC
Add new bindings for v2 Marvell xSPI overlay:
mrvl,xspi-nor  compatible string
New compatible string to distinguish between orginal and modified xSPI
block

PHY configuration registers
Allow to change orginal xSPI PHY configuration values. If not set, and
Marvell overlay is enabled, safe defaults will be written into xSPI PHY

Optional base for xfer register set
Additional reg field to allocate xSPI Marvell overlay XFER block

Signed-off-by: Witold Sadowski <wsadowski@marvell.com>
---
 .../devicetree/bindings/spi/cdns,xspi.yaml    | 78 +++++++++++++++----
 1 file changed, 65 insertions(+), 13 deletions(-)

Comments

Conor Dooley May 9, 2024, 5:22 p.m. UTC | #1
Hey Witold,

On Wed, May 08, 2024 at 06:05:20PM -0700, Witold Sadowski wrote:

>  allOf:
>    - $ref: spi-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: marvell,cn10-xspi-nor
> +    then:
> +      properties:
> +        reg-names:
> +          items:
> +            - const: io
> +            - const: sdma
> +            - const: aux
> +            - const: xferbase
> +        reg:
> +          items:
> +            - description: address and length of the controller register set
> +            - description: address and length of the Slave DMA data port
> +            - description: address and length of the auxiliary registers
> +            - description: address and length of the xfer registers
> +    else:
> +      properties:
> +        reg-names:
> +          items:
> +            - const: io
> +            - const: sdma
> +            - const: aux
> +        reg:
> +          items:
> +            - description: address and length of the controller register set
> +            - description: address and length of the Slave DMA data port
> +            - description: address and length of the auxiliary registers

The usual approach here is to define the loosest possible constraints
at the top level, so unconditionally define the xfer register region,
and then constrain things based on compatible. In this case, you can set
minItems to 3 unconditionally and then do (in psuedocode):
if:
  marvell
then:
  reg:
    minitems: 4
else
  reg:
    maxItems: 3

Additionally, when the allOf: is more then just references to other
documents, it should be moved below the required list.

Thanks,
Conor.
Witold Sadowski May 27, 2024, 8:26 a.m. UTC | #2
> ----------------------------------------------------------------------
> Hey Witold,
> 
> On Wed, May 08, 2024 at 06:05:20PM -0700, Witold Sadowski wrote:
> 
> >  allOf:
> >    - $ref: spi-controller.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: marvell,cn10-xspi-nor
> > +    then:
> > +      properties:
> > +        reg-names:
> > +          items:
> > +            - const: io
> > +            - const: sdma
> > +            - const: aux
> > +            - const: xferbase
> > +        reg:
> > +          items:
> > +            - description: address and length of the controller
> register set
> > +            - description: address and length of the Slave DMA data
> port
> > +            - description: address and length of the auxiliary
> registers
> > +            - description: address and length of the xfer registers
> > +    else:
> > +      properties:
> > +        reg-names:
> > +          items:
> > +            - const: io
> > +            - const: sdma
> > +            - const: aux
> > +        reg:
> > +          items:
> > +            - description: address and length of the controller
> register set
> > +            - description: address and length of the Slave DMA data
> port
> > +            - description: address and length of the auxiliary
> > + registers
> 
> The usual approach here is to define the loosest possible constraints at
> the top level, so unconditionally define the xfer register region, and
> then constrain things based on compatible. In this case, you can set
> minItems to 3 unconditionally and then do (in psuedocode):
> if:
>   marvell
> then:
>   reg:
>     minitems: 4
> else
>   reg:
>     maxItems: 3
> 
> Additionally, when the allOf: is more then just references to other
> documents, it should be moved below the required list.

Thanks for hints.
Yaml file will be reworked to match guideliness

> 
> Thanks,
> Conor.

Regards
Witek
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index eb0f92468185..094f8b7ffc49 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -17,22 +17,43 @@  description: |
 
 allOf:
   - $ref: spi-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: marvell,cn10-xspi-nor
+    then:
+      properties:
+        reg-names:
+          items:
+            - const: io
+            - const: sdma
+            - const: aux
+            - const: xferbase
+        reg:
+          items:
+            - description: address and length of the controller register set
+            - description: address and length of the Slave DMA data port
+            - description: address and length of the auxiliary registers
+            - description: address and length of the xfer registers
+    else:
+      properties:
+        reg-names:
+          items:
+            - const: io
+            - const: sdma
+            - const: aux
+        reg:
+          items:
+            - description: address and length of the controller register set
+            - description: address and length of the Slave DMA data port
+            - description: address and length of the auxiliary registers
 
 properties:
   compatible:
-    const: cdns,xspi-nor
-
-  reg:
-    items:
-      - description: address and length of the controller register set
-      - description: address and length of the Slave DMA data port
-      - description: address and length of the auxiliary registers
-
-  reg-names:
-    items:
-      - const: io
-      - const: sdma
-      - const: aux
+    enum:
+      - cdns,xspi-nor
+      - marvell,cn10-xspi-nor
 
   interrupts:
     maxItems: 1
@@ -68,6 +89,37 @@  examples:
                 reg = <0>;
             };
 
+            flash@1 {
+                compatible = "jedec,spi-nor";
+                spi-max-frequency = <75000000>;
+                reg = <1>;
+            };
+        };
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        spi@d0010000 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "marvell,cn10-xspi-nor";
+            reg = <0x0 0xa0010000 0x0 0x1040>,
+                  <0x0 0xb0000000 0x0 0x1000>,
+                  <0x0 0xa0020000 0x0 0x100>,
+                  <0x0 0xa0090000 0x0 0x100>;
+            reg-names = "io", "sdma", "aux", "xferbase";
+            interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-parent = <&gic>;
+
+            flash@0 {
+                compatible = "jedec,spi-nor";
+                spi-max-frequency = <75000000>;
+                reg = <0>;
+            };
+
             flash@1 {
                 compatible = "jedec,spi-nor";
                 spi-max-frequency = <75000000>;