diff mbox series

[v6,2/3] dt-bindings: pci: layerscape-pci: Add snps,dw-pcie.yaml reference

Message ID 20240301162741.765524-3-Frank.Li@nxp.com
State New
Headers show
Series dt-bindings: pci: layerscape-pci: Convert to yaml format | expand

Commit Message

Frank Li March 1, 2024, 4:27 p.m. UTC
Add snps,dw-pcie.yaml reference. Clean up all context that already exist in
snps,dw-pcie.yaml. Update interrupt-names requirement for difference
compatible string.

Set 'unevaluatedProperties' back to 'false'.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../bindings/pci/fsl,layerscape-pcie.yaml     | 104 +++++++++++++-----
 1 file changed, 78 insertions(+), 26 deletions(-)

Comments

Rob Herring (Arm) March 4, 2024, 6:17 p.m. UTC | #1
On Fri, Mar 01, 2024 at 11:27:40AM -0500, Frank Li wrote:
> Add snps,dw-pcie.yaml reference. Clean up all context that already exist in
> snps,dw-pcie.yaml. Update interrupt-names requirement for difference
> compatible string.
> 
> Set 'unevaluatedProperties' back to 'false'.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../bindings/pci/fsl,layerscape-pcie.yaml     | 104 +++++++++++++-----
>  1 file changed, 78 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> index 3f2d058701d22..137cc17933a4b 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> @@ -11,7 +11,6 @@ maintainers:
>  
>  description:
>    This PCIe RC controller is based on the Synopsys DesignWare PCIe IP
> -  and thus inherits all the common properties defined in snps,dw-pcie.yaml.
>  
>    This controller derives its clocks from the Reset Configuration Word (RCW)
>    which is used to describe the PLL settings at the time of chip-reset.
> @@ -36,31 +35,18 @@ properties:
>        - fsl,lx2160a-pcie
>  
>    reg:
> -    description: base addresses and lengths of the PCIe controller register blocks.
> +    maxItems: 2
> +
> +  reg-names:
> +    maxItems: 2

Need to define what the entries are. You change 'regs' to 'dbi' in the 
example. Was that an error in the example or are you planning on 
changing it in dts files? Besides the latter being an ABI change, I 
don't think you want to change dts files for platforms which are pretty 
stable.

>    interrupts:
> -    description: A list of interrupt outputs of the controller. Must contain an
> -      entry for each entry in the interrupt-names property.
> +    minItems: 1
> +    maxItems: 3
>  
>    interrupt-names:
>      minItems: 1
>      maxItems: 3
> -    description: It could include the following entries.
> -    items:
> -      oneOf:
> -        - description:
> -            Used for interrupt line which reports AER events when
> -            non MSI/MSI-X/INTx mode is used.
> -          const: aer
> -        - description:
> -            Used for interrupt line which reports PME events when
> -            non MSI/MSI-X/INTx mode is used.
> -          const: pme
> -        - description:
> -            Used for SoCs(like ls2080a, lx2160a, ls2080a, ls2088a, ls1088a)
> -            which has a single interrupt line for miscellaneous controller
> -            events(could include AER and PME events).
> -          const: intr
>  
>    fsl,pcie-scfg:
>      $ref: /schemas/types.yaml#/definitions/phandle
> @@ -69,23 +55,88 @@ properties:
>        The second entry is the physical PCIe controller index starting from '0'.
>        This is used to get SCFG PEXN registers
>  
> -  dma-coherent:
> -    description: Indicates that the hardware IP block can ensure the coherency
> -      of the data transferred from/to the IP block. This can avoid the software
> -      cache flush/invalid actions, and improve the performance significantly
> +  dma-coherent: true

No need to list.

> +
> +  msi-parent: true
> +
> +  iommu-map: true
>  
>    big-endian:
>      $ref: /schemas/types.yaml#/definitions/flag
>      description: If the PEX_LUT and PF register block is in big-endian, specify
>        this property.
>  
> -unevaluatedProperties: true
> +unevaluatedProperties: false
>  
>  required:
>    - compatible
>    - reg
>    - interrupt-names
>  
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#

That's already referenced in the common schema.

> +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,lx2160a-pcie
> +    then:
> +      properties:
> +        interrupts:
> +          maxItems: 3

max is already 3.

minItems: 3

> +        interrupt-names:
> +          items:
> +            - const: pme
> +            - const: aer
> +            - const: intr

I guess since you figured out the ordering here, you should keep them 
despite what I said in the first patch.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,ls1028a-pcie
> +            - fsl,ls1046a-pcie
> +            - fsl,ls1043a-pcie
> +            - fsl,ls1012a-pcie
> +    then:
> +      properties:
> +        interrupts:
> +          maxItems: 2

minItems: 2
maxItems: 2

> +        interrupt-names:
> +          items:
> +            - const: pme
> +            - const: aer
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,ls2080a-pcie
> +            - fsl,ls2085a-pcie
> +            - fsl,ls2088a-pcie
> +            - fsl,ls1021a-pcie
> +    then:
> +      properties:
> +        interrupts:
> +          maxItems: 1
> +        interrupt-names:
> +          items:
> +            - const: intr
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,ls1088a-pcie
> +    then:
> +      properties:
> +        interrupts:
> +          maxItems: 1
> +        interrupt-names:
> +          items:
> +            - const: aer
> +
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -98,7 +149,7 @@ examples:
>          compatible = "fsl,ls1088a-pcie";
>          reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
>              <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
> -        reg-names = "regs", "config";
> +        reg-names = "dbi", "config";
>          interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
>          interrupt-names = "aer";
>          #address-cells = <3>;
> @@ -116,6 +167,7 @@ examples:
>                          <0000 0 0 3 &gic 0 0 0 111 IRQ_TYPE_LEVEL_HIGH>,
>                          <0000 0 0 4 &gic 0 0 0 112 IRQ_TYPE_LEVEL_HIGH>;
>          iommu-map = <0 &smmu 0 1>; /* Fixed-up by bootloader */
> +        msi-map = <0 &its 0 1>; /* Fixed-up by bootloader */
>        };
>      };
>  ...
> -- 
> 2.34.1
>
Frank Li March 4, 2024, 7:10 p.m. UTC | #2
On Mon, Mar 04, 2024 at 12:17:58PM -0600, Rob Herring wrote:
> On Fri, Mar 01, 2024 at 11:27:40AM -0500, Frank Li wrote:
> > Add snps,dw-pcie.yaml reference. Clean up all context that already exist in
> > snps,dw-pcie.yaml. Update interrupt-names requirement for difference
> > compatible string.
> > 
> > Set 'unevaluatedProperties' back to 'false'.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../bindings/pci/fsl,layerscape-pcie.yaml     | 104 +++++++++++++-----
> >  1 file changed, 78 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> > index 3f2d058701d22..137cc17933a4b 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> > @@ -11,7 +11,6 @@ maintainers:
> >  
> >  description:
> >    This PCIe RC controller is based on the Synopsys DesignWare PCIe IP
> > -  and thus inherits all the common properties defined in snps,dw-pcie.yaml.
> >  
> >    This controller derives its clocks from the Reset Configuration Word (RCW)
> >    which is used to describe the PLL settings at the time of chip-reset.
> > @@ -36,31 +35,18 @@ properties:
> >        - fsl,lx2160a-pcie
> >  
> >    reg:
> > -    description: base addresses and lengths of the PCIe controller register blocks.
> > +    maxItems: 2
> > +
> > +  reg-names:
> > +    maxItems: 2
> 
> Need to define what the entries are. You change 'regs' to 'dbi' in the 
> example. Was that an error in the example or are you planning on 
> changing it in dts files? Besides the latter being an ABI change, I 
> don't think you want to change dts files for platforms which are pretty 
> stable.

It is on my plan.
https://lore.kernel.org/linux-pci/20240229194559.709182-1-Frank.Li@nxp.com/

Need change driver first, then change dts. 

It still need maintainance, even it is stable.

Frank 

> 
> >    interrupts:
> > -    description: A list of interrupt outputs of the controller. Must contain an
> > -      entry for each entry in the interrupt-names property.
> > +    minItems: 1
> > +    maxItems: 3
> >  
> >    interrupt-names:
> >      minItems: 1
> >      maxItems: 3
> > -    description: It could include the following entries.
> > -    items:
> > -      oneOf:
> > -        - description:
> > -            Used for interrupt line which reports AER events when
> > -            non MSI/MSI-X/INTx mode is used.
> > -          const: aer
> > -        - description:
> > -            Used for interrupt line which reports PME events when
> > -            non MSI/MSI-X/INTx mode is used.
> > -          const: pme
> > -        - description:
> > -            Used for SoCs(like ls2080a, lx2160a, ls2080a, ls2088a, ls1088a)
> > -            which has a single interrupt line for miscellaneous controller
> > -            events(could include AER and PME events).
> > -          const: intr
> >  
> >    fsl,pcie-scfg:
> >      $ref: /schemas/types.yaml#/definitions/phandle
> > @@ -69,23 +55,88 @@ properties:
> >        The second entry is the physical PCIe controller index starting from '0'.
> >        This is used to get SCFG PEXN registers
> >  
> > -  dma-coherent:
> > -    description: Indicates that the hardware IP block can ensure the coherency
> > -      of the data transferred from/to the IP block. This can avoid the software
> > -      cache flush/invalid actions, and improve the performance significantly
> > +  dma-coherent: true
> 
> No need to list.
> 
> > +
> > +  msi-parent: true
> > +
> > +  iommu-map: true
> >  
> >    big-endian:
> >      $ref: /schemas/types.yaml#/definitions/flag
> >      description: If the PEX_LUT and PF register block is in big-endian, specify
> >        this property.
> >  
> > -unevaluatedProperties: true
> > +unevaluatedProperties: false
> >  
> >  required:
> >    - compatible
> >    - reg
> >    - interrupt-names
> >  
> > +allOf:
> > +  - $ref: /schemas/pci/pci-bus.yaml#
> 
> That's already referenced in the common schema.
> 
> > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,lx2160a-pcie
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 3
> 
> max is already 3.
> 
> minItems: 3
> 
> > +        interrupt-names:
> > +          items:
> > +            - const: pme
> > +            - const: aer
> > +            - const: intr
> 
> I guess since you figured out the ordering here, you should keep them 
> despite what I said in the first patch.
> 
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,ls1028a-pcie
> > +            - fsl,ls1046a-pcie
> > +            - fsl,ls1043a-pcie
> > +            - fsl,ls1012a-pcie
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 2
> 
> minItems: 2
> maxItems: 2
> 
> > +        interrupt-names:
> > +          items:
> > +            - const: pme
> > +            - const: aer
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,ls2080a-pcie
> > +            - fsl,ls2085a-pcie
> > +            - fsl,ls2088a-pcie
> > +            - fsl,ls1021a-pcie
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 1
> > +        interrupt-names:
> > +          items:
> > +            - const: intr
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,ls1088a-pcie
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 1
> > +        interrupt-names:
> > +          items:
> > +            - const: aer
> > +
> >  examples:
> >    - |
> >      #include <dt-bindings/interrupt-controller/arm-gic.h>
> > @@ -98,7 +149,7 @@ examples:
> >          compatible = "fsl,ls1088a-pcie";
> >          reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
> >              <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
> > -        reg-names = "regs", "config";
> > +        reg-names = "dbi", "config";
> >          interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> >          interrupt-names = "aer";
> >          #address-cells = <3>;
> > @@ -116,6 +167,7 @@ examples:
> >                          <0000 0 0 3 &gic 0 0 0 111 IRQ_TYPE_LEVEL_HIGH>,
> >                          <0000 0 0 4 &gic 0 0 0 112 IRQ_TYPE_LEVEL_HIGH>;
> >          iommu-map = <0 &smmu 0 1>; /* Fixed-up by bootloader */
> > +        msi-map = <0 &its 0 1>; /* Fixed-up by bootloader */
> >        };
> >      };
> >  ...
> > -- 
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
index 3f2d058701d22..137cc17933a4b 100644
--- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
@@ -11,7 +11,6 @@  maintainers:
 
 description:
   This PCIe RC controller is based on the Synopsys DesignWare PCIe IP
-  and thus inherits all the common properties defined in snps,dw-pcie.yaml.
 
   This controller derives its clocks from the Reset Configuration Word (RCW)
   which is used to describe the PLL settings at the time of chip-reset.
@@ -36,31 +35,18 @@  properties:
       - fsl,lx2160a-pcie
 
   reg:
-    description: base addresses and lengths of the PCIe controller register blocks.
+    maxItems: 2
+
+  reg-names:
+    maxItems: 2
 
   interrupts:
-    description: A list of interrupt outputs of the controller. Must contain an
-      entry for each entry in the interrupt-names property.
+    minItems: 1
+    maxItems: 3
 
   interrupt-names:
     minItems: 1
     maxItems: 3
-    description: It could include the following entries.
-    items:
-      oneOf:
-        - description:
-            Used for interrupt line which reports AER events when
-            non MSI/MSI-X/INTx mode is used.
-          const: aer
-        - description:
-            Used for interrupt line which reports PME events when
-            non MSI/MSI-X/INTx mode is used.
-          const: pme
-        - description:
-            Used for SoCs(like ls2080a, lx2160a, ls2080a, ls2088a, ls1088a)
-            which has a single interrupt line for miscellaneous controller
-            events(could include AER and PME events).
-          const: intr
 
   fsl,pcie-scfg:
     $ref: /schemas/types.yaml#/definitions/phandle
@@ -69,23 +55,88 @@  properties:
       The second entry is the physical PCIe controller index starting from '0'.
       This is used to get SCFG PEXN registers
 
-  dma-coherent:
-    description: Indicates that the hardware IP block can ensure the coherency
-      of the data transferred from/to the IP block. This can avoid the software
-      cache flush/invalid actions, and improve the performance significantly
+  dma-coherent: true
+
+  msi-parent: true
+
+  iommu-map: true
 
   big-endian:
     $ref: /schemas/types.yaml#/definitions/flag
     description: If the PEX_LUT and PF register block is in big-endian, specify
       this property.
 
-unevaluatedProperties: true
+unevaluatedProperties: false
 
 required:
   - compatible
   - reg
   - interrupt-names
 
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,lx2160a-pcie
+    then:
+      properties:
+        interrupts:
+          maxItems: 3
+        interrupt-names:
+          items:
+            - const: pme
+            - const: aer
+            - const: intr
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,ls1028a-pcie
+            - fsl,ls1046a-pcie
+            - fsl,ls1043a-pcie
+            - fsl,ls1012a-pcie
+    then:
+      properties:
+        interrupts:
+          maxItems: 2
+        interrupt-names:
+          items:
+            - const: pme
+            - const: aer
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,ls2080a-pcie
+            - fsl,ls2085a-pcie
+            - fsl,ls2088a-pcie
+            - fsl,ls1021a-pcie
+    then:
+      properties:
+        interrupts:
+          maxItems: 1
+        interrupt-names:
+          items:
+            - const: intr
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,ls1088a-pcie
+    then:
+      properties:
+        interrupts:
+          maxItems: 1
+        interrupt-names:
+          items:
+            - const: aer
+
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -98,7 +149,7 @@  examples:
         compatible = "fsl,ls1088a-pcie";
         reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
             <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
-        reg-names = "regs", "config";
+        reg-names = "dbi", "config";
         interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
         interrupt-names = "aer";
         #address-cells = <3>;
@@ -116,6 +167,7 @@  examples:
                         <0000 0 0 3 &gic 0 0 0 111 IRQ_TYPE_LEVEL_HIGH>,
                         <0000 0 0 4 &gic 0 0 0 112 IRQ_TYPE_LEVEL_HIGH>;
         iommu-map = <0 &smmu 0 1>; /* Fixed-up by bootloader */
+        msi-map = <0 &its 0 1>; /* Fixed-up by bootloader */
       };
     };
 ...