[PATCH v6 0/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[PATCH v6 0/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Frank Li]

Change from
    Change from v5 to v6
    - Rob require every commit pass dtb_check, so first commit included
nesssary change. and list change at commit message.
    - Reduce to 3 patches.

    Change from v4 to v5
    - split 5 patch.
    - create one base patch, which the same as original txt.
    - two patch fix for RC.
    - two patch fix for EP.

    Change from v3 to v4
    - remove ep label
    - remove status="disabled"
    - remove deprecated property
    - fixed irq max-numbers
    - because dts still use "reg" instead "dbi", to avoid dtb check warning,
    not referernece to snps,dwc-pcie yet.

    Change from v2 to v3
    - update commit message, show change compare txt file
    - add failback compatible fsl,ls-pcie-ep.
    - add commit message about 'addr_space' and 'config'.

    Change from v1 to v2
    - remove '|-'
    - dma-coherent: true
    - add interrupts and interrupt-names at before Allof
    - remove ref to snps,pcie*.yaml, some reg-names are not aligned with in
    drivers
    - Add an example for pcie-ep

Frank Li (3):
  dt-bindings: pci: layerscape-pci: Convert to yaml format
  dt-bindings: pci: layerscape-pci: Add snps,dw-pcie.yaml reference
  dt-bindings: pci: layerscape-pci-ep: Add snps,dw-pcie-ep.yaml
    reference

 .../bindings/pci/fsl,layerscape-pcie-ep.yaml  |  91 +++++++++
 .../bindings/pci/fsl,layerscape-pcie.yaml     | 173 ++++++++++++++++++
 .../bindings/pci/layerscape-pci.txt           |  79 --------
 3 files changed, 264 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/layerscape-pci.txt

-- 
2.34.1

[PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Frank Li]

Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
and fsl,layerscape-pcie.yaml.
yaml files contain the same content as the original txt file.

Do below changes to pass dtb_binding check:
- Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.
- Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../bindings/pci/fsl,layerscape-pcie-ep.yaml  |  87 +++++++++++++
 .../bindings/pci/fsl,layerscape-pcie.yaml     | 121 ++++++++++++++++++
 .../bindings/pci/layerscape-pci.txt           |  79 ------------
 3 files changed, 208 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/layerscape-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
new file mode 100644
index 0000000000000..cf517e4e46a33
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Layerscape PCIe Root Complex(RC) controller
+
+maintainers:
+  - Frank Li <Frank.Li@nxp.com>
+
+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.
+
+  Also as per the available Reference Manuals, there is no specific 'version'
+  register available in the Freescale PCIe controller register set,
+  which can allow determining the underlying DesignWare PCIe controller version
+  information.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - fsl,ls1028a-pcie-ep
+          - fsl,ls2046a-pcie-ep
+          - fsl,ls2088a-pcie-ep
+          - fsl,ls1046a-pcie-ep
+          - fsl,ls1043a-pcie-ep
+          - fsl,ls1012a-pcie-ep
+          - fsl,lx2160ar2-pcie-ep
+      - const: fsl,ls-pcie-ep
+
+  reg:
+    description: base addresses and lengths of the PCIe controller register blocks.
+
+  interrupts:
+    description: A list of interrupt outputs of the controller. Must contain an
+      entry for each entry in the interrupt-names property.
+
+  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
+    description: Must include two entries.
+      The first entry must be a link to the SCFG device node
+      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
+
+  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: false
+
+required:
+  - compatible
+  - reg
+  - interrupt-names
+
diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
new file mode 100644
index 0000000000000..3f2d058701d22
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
@@ -0,0 +1,121 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Layerscape PCIe Root Complex(RC) controller
+
+maintainers:
+  - Frank Li <Frank.Li@nxp.com>
+
+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.
+
+  Also as per the available Reference Manuals, there is no specific 'version'
+  register available in the Freescale PCIe controller register set,
+  which can allow determining the underlying DesignWare PCIe controller version
+  information.
+
+properties:
+  compatible:
+    enum:
+      - fsl,ls1021a-pcie
+      - fsl,ls2080a-pcie
+      - fsl,ls2085a-pcie
+      - fsl,ls2088a-pcie
+      - fsl,ls1088a-pcie
+      - fsl,ls1046a-pcie
+      - fsl,ls1043a-pcie
+      - fsl,ls1012a-pcie
+      - fsl,ls1028a-pcie
+      - fsl,lx2160a-pcie
+
+  reg:
+    description: base addresses and lengths of the PCIe controller register blocks.
+
+  interrupts:
+    description: A list of interrupt outputs of the controller. Must contain an
+      entry for each entry in the interrupt-names property.
+
+  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
+    description: Must include two entries.
+      The first entry must be a link to the SCFG device node
+      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
+
+  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
+
+required:
+  - compatible
+  - reg
+  - interrupt-names
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie@3400000 {
+        compatible = "fsl,ls1088a-pcie";
+        reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
+            <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
+        reg-names = "regs", "config";
+        interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
+        interrupt-names = "aer";
+        #address-cells = <3>;
+        #size-cells = <2>;
+        dma-coherent;
+        device_type = "pci";
+        bus-range = <0x0 0xff>;
+        ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
+                 0x82000000 0x0 0x40000000 0x20 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
+        msi-parent = <&its>;
+        #interrupt-cells = <1>;
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0000 0 0 1 &gic 0 0 0 109 IRQ_TYPE_LEVEL_HIGH>,
+                        <0000 0 0 2 &gic 0 0 0 110 IRQ_TYPE_LEVEL_HIGH>,
+                        <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 */
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
deleted file mode 100644
index ee8a4791a78b4..0000000000000
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ /dev/null
@@ -1,79 +0,0 @@
-Freescale Layerscape PCIe controller
-
-This PCIe host 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.
-
-Also as per the available Reference Manuals, there is no specific 'version'
-register available in the Freescale PCIe controller register set,
-which can allow determining the underlying DesignWare PCIe controller version
-information.
-
-Required properties:
-- compatible: should contain the platform identifier such as:
-  RC mode:
-        "fsl,ls1021a-pcie"
-        "fsl,ls2080a-pcie", "fsl,ls2085a-pcie"
-        "fsl,ls2088a-pcie"
-        "fsl,ls1088a-pcie"
-        "fsl,ls1046a-pcie"
-        "fsl,ls1043a-pcie"
-        "fsl,ls1012a-pcie"
-        "fsl,ls1028a-pcie"
-  EP mode:
-	"fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep"
-	"fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
-	"fsl,ls1088a-pcie-ep", "fsl,ls-pcie-ep"
-	"fsl,ls2088a-pcie-ep", "fsl,ls-pcie-ep"
-	"fsl,lx2160ar2-pcie-ep", "fsl,ls-pcie-ep"
-- reg: base addresses and lengths of the PCIe controller register blocks.
-- interrupts: A list of interrupt outputs of the controller. Must contain an
-  entry for each entry in the interrupt-names property.
-- interrupt-names: It could include the following entries:
-  "aer": Used for interrupt line which reports AER events when
-	 non MSI/MSI-X/INTx mode is used
-  "pme": Used for interrupt line which reports PME events when
-	 non MSI/MSI-X/INTx mode is used
-  "intr": 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).
-- fsl,pcie-scfg: Must include two entries.
-  The first entry must be a link to the SCFG device node
-  The second entry is the physical PCIe controller index starting from '0'.
-  This is used to get SCFG PEXN registers
-- dma-coherent: 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.
-
-Optional properties:
-- big-endian: If the PEX_LUT and PF register block is in big-endian, specify
-  this property.
-
-Example:
-
-        pcie@3400000 {
-                compatible = "fsl,ls1088a-pcie";
-                reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
-                      <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
-                reg-names = "regs", "config";
-                interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
-                interrupt-names = "aer";
-                #address-cells = <3>;
-                #size-cells = <2>;
-                device_type = "pci";
-                dma-coherent;
-                num-viewport = <256>;
-                bus-range = <0x0 0xff>;
-                ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
-                          0x82000000 0x0 0x40000000 0x20 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
-                msi-parent = <&its>;
-                #interrupt-cells = <1>;
-                interrupt-map-mask = <0 0 0 7>;
-                interrupt-map = <0000 0 0 1 &gic 0 0 0 109 IRQ_TYPE_LEVEL_HIGH>,
-                                <0000 0 0 2 &gic 0 0 0 110 IRQ_TYPE_LEVEL_HIGH>,
-                                <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 */
-        };
-- 
2.34.1

Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Rob Herring]

On Fri, Mar 01, 2024 at 11:27:39AM -0500, Frank Li wrote:
> Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
> and fsl,layerscape-pcie.yaml.
> yaml files contain the same content as the original txt file.
> 
> Do below changes to pass dtb_binding check:
> - Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.

You mean 'remove from required' right? Because they are still here.

> - Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.

Sorry, but that's not acceptable either. You need the $ref's in this 
patch.

> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  |  87 +++++++++++++
>  .../bindings/pci/fsl,layerscape-pcie.yaml     | 121 ++++++++++++++++++
>  .../bindings/pci/layerscape-pci.txt           |  79 ------------
>  3 files changed, 208 insertions(+), 79 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
>  create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
>  delete mode 100644 Documentation/devicetree/bindings/pci/layerscape-pci.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> new file mode 100644
> index 0000000000000..cf517e4e46a33
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Layerscape PCIe Root Complex(RC) controller
> +
> +maintainers:
> +  - Frank Li <Frank.Li@nxp.com>
> +
> +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.
> +
> +  Also as per the available Reference Manuals, there is no specific 'version'
> +  register available in the Freescale PCIe controller register set,
> +  which can allow determining the underlying DesignWare PCIe controller version
> +  information.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - fsl,ls1028a-pcie-ep
> +          - fsl,ls2046a-pcie-ep
> +          - fsl,ls2088a-pcie-ep
> +          - fsl,ls1046a-pcie-ep
> +          - fsl,ls1043a-pcie-ep
> +          - fsl,ls1012a-pcie-ep
> +          - fsl,lx2160ar2-pcie-ep
> +      - const: fsl,ls-pcie-ep
> +
> +  reg:
> +    description: base addresses and lengths of the PCIe controller register blocks.

You need to define how many entries and what they are.

Missing 'reg-names'?

> +
> +  interrupts:
> +    description: A list of interrupt outputs of the controller. Must contain an
> +      entry for each entry in the interrupt-names property.

You need to define how many entries and what they are.

> +
> +  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

The way this works is the common schema defines all possible names. This 
schema needs to define how many entries, which names you use, and what 
is the order. So you need to add 'pme' and 'aer' to 
snps,dw-pcie-ep.yaml. 

I imagine the order of entries is a mess here, and I don't expect 
there's any new Layerscape platforms coming. So this binding can just 
say:

minItems: 1
maxItems: 3
items:
  enum: [ aer, pme, intr ]

> +
> +  fsl,pcie-scfg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Must include two entries.
> +      The first entry must be a link to the SCFG device node
> +      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

Already listed in the common schema, so you can drop.

> +
> +  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: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-names
> +
> diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> new file mode 100644
> index 0000000000000..3f2d058701d22
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> @@ -0,0 +1,121 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Layerscape PCIe Root Complex(RC) controller
> +
> +maintainers:
> +  - Frank Li <Frank.Li@nxp.com>
> +
> +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.
> +
> +  Also as per the available Reference Manuals, there is no specific 'version'
> +  register available in the Freescale PCIe controller register set,
> +  which can allow determining the underlying DesignWare PCIe controller version
> +  information.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,ls1021a-pcie
> +      - fsl,ls2080a-pcie
> +      - fsl,ls2085a-pcie
> +      - fsl,ls2088a-pcie
> +      - fsl,ls1088a-pcie
> +      - fsl,ls1046a-pcie
> +      - fsl,ls1043a-pcie
> +      - fsl,ls1012a-pcie
> +      - fsl,ls1028a-pcie
> +      - fsl,lx2160a-pcie
> +
> +  reg:
> +    description: base addresses and lengths of the PCIe controller register blocks.

You need to define how many entries and what they are.


> +
> +  interrupts:
> +    description: A list of interrupt outputs of the controller. Must contain an
> +      entry for each entry in the interrupt-names property.

You need to define how many entries and what they are.

> +
> +  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

Similar comment here, but the names are already defined in 
snps,dw-pcie.yaml.

> +
> +  fsl,pcie-scfg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Must include two entries.
> +      The first entry must be a link to the SCFG device node
> +      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

Drop

> +
> +  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
> +
> +required:

> +  - compatible
> +  - reg

Both required in common schema. Drop.

> +  - interrupt-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pcie@3400000 {
> +        compatible = "fsl,ls1088a-pcie";
> +        reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
> +            <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
> +        reg-names = "regs", "config";
> +        interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> +        interrupt-names = "aer";
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        dma-coherent;
> +        device_type = "pci";
> +        bus-range = <0x0 0xff>;
> +        ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
> +                 0x82000000 0x0 0x40000000 0x20 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> +        msi-parent = <&its>;
> +        #interrupt-cells = <1>;
> +        interrupt-map-mask = <0 0 0 7>;
> +        interrupt-map = <0000 0 0 1 &gic 0 0 0 109 IRQ_TYPE_LEVEL_HIGH>,
> +                        <0000 0 0 2 &gic 0 0 0 110 IRQ_TYPE_LEVEL_HIGH>,
> +                        <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 */
> +      };
> +    };
> +...

Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Frank Li]

On Mon, Mar 04, 2024 at 11:32:04AM -0600, Rob Herring wrote:
> On Fri, Mar 01, 2024 at 11:27:39AM -0500, Frank Li wrote:
> > Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
> > and fsl,layerscape-pcie.yaml.
> > yaml files contain the same content as the original txt file.
> > 
> > Do below changes to pass dtb_binding check:
> > - Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.
> 
> You mean 'remove from required' right? Because they are still here.
> 
> > - Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.
> 
> Sorry, but that's not acceptable either. You need the $ref's in this 
> patch.


Rob:

If I squash this 3 patches, it will match most your comments. And will back
to my previous v3's patches, 
https://lore.kernel.org/imx/20240214194145.2669744-1-Frank.Li@nxp.com/

Bjorn Helgaas suggest split to patches:
https://lore.kernel.org/imx/20240226210842.GA211190@bhelgaas/

First one will be equal to origial txt, then add change base on that.

I need a clear direction before I continue on this.

Frank

> 
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  |  87 +++++++++++++
> >  .../bindings/pci/fsl,layerscape-pcie.yaml     | 121 ++++++++++++++++++
> >  .../bindings/pci/layerscape-pci.txt           |  79 ------------
> >  3 files changed, 208 insertions(+), 79 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> >  create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > new file mode 100644
> > index 0000000000000..cf517e4e46a33
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > @@ -0,0 +1,87 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie-ep.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale Layerscape PCIe Root Complex(RC) controller
> > +
> > +maintainers:
> > +  - Frank Li <Frank.Li@nxp.com>
> > +
> > +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.
> > +
> > +  Also as per the available Reference Manuals, there is no specific 'version'
> > +  register available in the Freescale PCIe controller register set,
> > +  which can allow determining the underlying DesignWare PCIe controller version
> > +  information.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - fsl,ls1028a-pcie-ep
> > +          - fsl,ls2046a-pcie-ep
> > +          - fsl,ls2088a-pcie-ep
> > +          - fsl,ls1046a-pcie-ep
> > +          - fsl,ls1043a-pcie-ep
> > +          - fsl,ls1012a-pcie-ep
> > +          - fsl,lx2160ar2-pcie-ep
> > +      - const: fsl,ls-pcie-ep
> > +
> > +  reg:
> > +    description: base addresses and lengths of the PCIe controller register blocks.
> 
> You need to define how many entries and what they are.
> 
> Missing 'reg-names'?
> 
> > +
> > +  interrupts:
> > +    description: A list of interrupt outputs of the controller. Must contain an
> > +      entry for each entry in the interrupt-names property.
> 
> You need to define how many entries and what they are.
> 
> > +
> > +  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
> 
> The way this works is the common schema defines all possible names. This 
> schema needs to define how many entries, which names you use, and what 
> is the order. So you need to add 'pme' and 'aer' to 
> snps,dw-pcie-ep.yaml. 
> 
> I imagine the order of entries is a mess here, and I don't expect 
> there's any new Layerscape platforms coming. So this binding can just 
> say:
> 
> minItems: 1
> maxItems: 3
> items:
>   enum: [ aer, pme, intr ]
> 
> > +
> > +  fsl,pcie-scfg:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: Must include two entries.
> > +      The first entry must be a link to the SCFG device node
> > +      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
> 
> Already listed in the common schema, so you can drop.
> 
> > +
> > +  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: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupt-names
> > +
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> > new file mode 100644
> > index 0000000000000..3f2d058701d22
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> > @@ -0,0 +1,121 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale Layerscape PCIe Root Complex(RC) controller
> > +
> > +maintainers:
> > +  - Frank Li <Frank.Li@nxp.com>
> > +
> > +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.
> > +
> > +  Also as per the available Reference Manuals, there is no specific 'version'
> > +  register available in the Freescale PCIe controller register set,
> > +  which can allow determining the underlying DesignWare PCIe controller version
> > +  information.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,ls1021a-pcie
> > +      - fsl,ls2080a-pcie
> > +      - fsl,ls2085a-pcie
> > +      - fsl,ls2088a-pcie
> > +      - fsl,ls1088a-pcie
> > +      - fsl,ls1046a-pcie
> > +      - fsl,ls1043a-pcie
> > +      - fsl,ls1012a-pcie
> > +      - fsl,ls1028a-pcie
> > +      - fsl,lx2160a-pcie
> > +
> > +  reg:
> > +    description: base addresses and lengths of the PCIe controller register blocks.
> 
> You need to define how many entries and what they are.
> 
> 
> > +
> > +  interrupts:
> > +    description: A list of interrupt outputs of the controller. Must contain an
> > +      entry for each entry in the interrupt-names property.
> 
> You need to define how many entries and what they are.
> 
> > +
> > +  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
> 
> Similar comment here, but the names are already defined in 
> snps,dw-pcie.yaml.
> 
> > +
> > +  fsl,pcie-scfg:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: Must include two entries.
> > +      The first entry must be a link to the SCFG device node
> > +      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
> 
> Drop
> 
> > +
> > +  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
> > +
> > +required:
> 
> > +  - compatible
> > +  - reg
> 
> Both required in common schema. Drop.
> 
> > +  - interrupt-names
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      pcie@3400000 {
> > +        compatible = "fsl,ls1088a-pcie";
> > +        reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
> > +            <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
> > +        reg-names = "regs", "config";
> > +        interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> > +        interrupt-names = "aer";
> > +        #address-cells = <3>;
> > +        #size-cells = <2>;
> > +        dma-coherent;
> > +        device_type = "pci";
> > +        bus-range = <0x0 0xff>;
> > +        ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
> > +                 0x82000000 0x0 0x40000000 0x20 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> > +        msi-parent = <&its>;
> > +        #interrupt-cells = <1>;
> > +        interrupt-map-mask = <0 0 0 7>;
> > +        interrupt-map = <0000 0 0 1 &gic 0 0 0 109 IRQ_TYPE_LEVEL_HIGH>,
> > +                        <0000 0 0 2 &gic 0 0 0 110 IRQ_TYPE_LEVEL_HIGH>,
> > +                        <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 */
> > +      };
> > +    };
> > +...

Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Rob Herring]

On Mon, Mar 04, 2024 at 12:47:08PM -0500, Frank Li wrote:
> On Mon, Mar 04, 2024 at 11:32:04AM -0600, Rob Herring wrote:
> > On Fri, Mar 01, 2024 at 11:27:39AM -0500, Frank Li wrote:
> > > Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
> > > and fsl,layerscape-pcie.yaml.
> > > yaml files contain the same content as the original txt file.
> > > 
> > > Do below changes to pass dtb_binding check:
> > > - Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.
> > 
> > You mean 'remove from required' right? Because they are still here.
> > 
> > > - Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.
> > 
> > Sorry, but that's not acceptable either. You need the $ref's in this 
> > patch.
> 
> 
> Rob:
> 
> If I squash this 3 patches, it will match most your comments. And will back
> to my previous v3's patches, 
> https://lore.kernel.org/imx/20240214194145.2669744-1-Frank.Li@nxp.com/

Plus our review comments I hope...

> 
> Bjorn Helgaas suggest split to patches:
> https://lore.kernel.org/imx/20240226210842.GA211190@bhelgaas/
> 
> First one will be equal to origial txt, then add change base on that.
> 
> I need a clear direction before I continue on this.

Bjorn's suggestion doesn't work. I think it was confused in that you 
said you were 'adding' things. You aren't adding things, just fixing 
things to make the validation pass.

If you want to split things, you could add reg and/or interrupt names to 
the common schema first. And then add Layerscape schemas. But don't add 
things you turn around and remove in the very next patch. I think 
Krzysztof told you the very same thing.

Rob

Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Frank Li]

On Tue, Mar 05, 2024 at 08:46:36AM -0600, Rob Herring wrote:
> On Mon, Mar 04, 2024 at 12:47:08PM -0500, Frank Li wrote:
> > On Mon, Mar 04, 2024 at 11:32:04AM -0600, Rob Herring wrote:
> > > On Fri, Mar 01, 2024 at 11:27:39AM -0500, Frank Li wrote:
> > > > Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
> > > > and fsl,layerscape-pcie.yaml.
> > > > yaml files contain the same content as the original txt file.
> > > > 
> > > > Do below changes to pass dtb_binding check:
> > > > - Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.
> > > 
> > > You mean 'remove from required' right? Because they are still here.
> > > 
> > > > - Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.
> > > 
> > > Sorry, but that's not acceptable either. You need the $ref's in this 
> > > patch.
> > 
> > 
> > Rob:
> > 
> > If I squash this 3 patches, it will match most your comments. And will back
> > to my previous v3's patches, 
> > https://lore.kernel.org/imx/20240214194145.2669744-1-Frank.Li@nxp.com/
> 
> Plus our review comments I hope...
> 
> > 
> > Bjorn Helgaas suggest split to patches:
> > https://lore.kernel.org/imx/20240226210842.GA211190@bhelgaas/
> > 
> > First one will be equal to origial txt, then add change base on that.
> > 
> > I need a clear direction before I continue on this.
> 
> Bjorn's suggestion doesn't work. I think it was confused in that you 
> said you were 'adding' things. You aren't adding things, just fixing 
> things to make the validation pass.
> 
> If you want to split things, you could add reg and/or interrupt names to 
> the common schema first. And then add Layerscape schemas. But don't add 
> things you turn around and remove in the very next patch. I think 
> Krzysztof told you the very same thing.

@Bjorn:
	Do you agree squash these 3 patches?  I don't want to split again.
I prefer focus on the real value things. It is not difficult to review at
one patches. Origianl txt actaully relately simple. 

Frank

> 
> Rob

Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format

[Frank Li]

On Tue, Mar 05, 2024 at 08:46:36AM -0600, Rob Herring wrote:
> On Mon, Mar 04, 2024 at 12:47:08PM -0500, Frank Li wrote:
> > On Mon, Mar 04, 2024 at 11:32:04AM -0600, Rob Herring wrote:
> > > On Fri, Mar 01, 2024 at 11:27:39AM -0500, Frank Li wrote:
> > > > Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
> > > > and fsl,layerscape-pcie.yaml.
> > > > yaml files contain the same content as the original txt file.
> > > > 
> > > > Do below changes to pass dtb_binding check:
> > > > - Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.
> > > 
> > > You mean 'remove from required' right? Because they are still here.
> > > 
> > > > - Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.
> > > 
> > > Sorry, but that's not acceptable either. You need the $ref's in this 
> > > patch.
> > 
> > 
> > Rob:
> > 
> > If I squash this 3 patches, it will match most your comments. And will back
> > to my previous v3's patches, 
> > https://lore.kernel.org/imx/20240214194145.2669744-1-Frank.Li@nxp.com/
> 
> Plus our review comments I hope...
> 
> > 
> > Bjorn Helgaas suggest split to patches:
> > https://lore.kernel.org/imx/20240226210842.GA211190@bhelgaas/
> > 
> > First one will be equal to origial txt, then add change base on that.
> > 
> > I need a clear direction before I continue on this.
> 
> Bjorn's suggestion doesn't work. I think it was confused in that you 
> said you were 'adding' things. You aren't adding things, just fixing 
> things to make the validation pass.
> 
> If you want to split things, you could add reg and/or interrupt names to 
> the common schema first. And then add Layerscape schemas. But don't add 
> things you turn around and remove in the very next patch. I think 
> Krzysztof told you the very same thing.

@Bjorn:

   Do you agree on square 3 patch to one or others? All dt-binding scheme
have to pass dt_check. Rob and conor already take many time to review this
patches. 

Frank 


> 
> Rob

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

[Frank Li]

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
 
   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 */
       };
     };
 ...
-- 
2.34.1

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

[Rob Herring]

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
> 

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

[Frank Li]

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
> > 

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

[Frank Li]

Add snps,dw-pcie-ep.yaml.

Remove context that exist in snps,dw-pcie-ep.yaml.

Add an example for pcie-ep.

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

diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
index cf517e4e46a33..07965683beece 100644
--- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
@@ -10,8 +10,7 @@ maintainers:
   - Frank Li <Frank.Li@nxp.com>
 
 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 PCIe RC controller is based on the Synopsys DesignWare PCIe IP.
 
   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.
@@ -35,31 +34,18 @@ properties:
       - const: fsl,ls-pcie-ep
 
   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
@@ -68,10 +54,7 @@ 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
 
   big-endian:
     $ref: /schemas/types.yaml#/definitions/flag
@@ -85,3 +68,24 @@ required:
   - reg
   - interrupt-names
 
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie-ep@3400000 {
+        compatible = "fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep";
+        reg = <0x00 0x03400000 0x0 0x00100000
+              0x80 0x00000000 0x8 0x00000000>;
+        reg-names = "dbi", "addr_space";
+        interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; /* PME interrupt */
+        interrupt-names = "app";
+      };
+    };
+...
-- 
2.34.1

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

[Rob Herring]

On Fri, Mar 01, 2024 at 11:27:41AM -0500, Frank Li wrote:
> Add snps,dw-pcie-ep.yaml.
> 
> Remove context that exist in snps,dw-pcie-ep.yaml.
> 
> Add an example for pcie-ep.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  | 54 ++++++++++---------
>  1 file changed, 29 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> index cf517e4e46a33..07965683beece 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> @@ -10,8 +10,7 @@ maintainers:
>    - Frank Li <Frank.Li@nxp.com>
>  
>  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 PCIe RC controller is based on the Synopsys DesignWare PCIe IP.
>  
>    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.
> @@ -35,31 +34,18 @@ properties:
>        - const: fsl,ls-pcie-ep
>  
>    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
> @@ -68,10 +54,7 @@ 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
>  
>    big-endian:
>      $ref: /schemas/types.yaml#/definitions/flag
> @@ -85,3 +68,24 @@ required:
>    - reg
>    - interrupt-names
>  
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pcie-ep@3400000 {
> +        compatible = "fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep";
> +        reg = <0x00 0x03400000 0x0 0x00100000
> +              0x80 0x00000000 0x8 0x00000000>;
> +        reg-names = "dbi", "addr_space";
> +        interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; /* PME interrupt */

PME or...

> +        interrupt-names = "app";

app? You seem to just be changing the names to make the example happy. 
What do the dts files have? You need to make those pass.

Rob

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

[Frank Li]

On Mon, Mar 04, 2024 at 12:20:49PM -0600, Rob Herring wrote:
> On Fri, Mar 01, 2024 at 11:27:41AM -0500, Frank Li wrote:
> > Add snps,dw-pcie-ep.yaml.
> > 
> > Remove context that exist in snps,dw-pcie-ep.yaml.
> > 
> > Add an example for pcie-ep.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  | 54 ++++++++++---------
> >  1 file changed, 29 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > index cf517e4e46a33..07965683beece 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > @@ -10,8 +10,7 @@ maintainers:
> >    - Frank Li <Frank.Li@nxp.com>
> >  
> >  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 PCIe RC controller is based on the Synopsys DesignWare PCIe IP.
> >  
> >    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.
> > @@ -35,31 +34,18 @@ properties:
> >        - const: fsl,ls-pcie-ep
> >  
> >    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
> > @@ -68,10 +54,7 @@ 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
> >  
> >    big-endian:
> >      $ref: /schemas/types.yaml#/definitions/flag
> > @@ -85,3 +68,24 @@ required:
> >    - reg
> >    - interrupt-names
> >  
> > +allOf:
> > +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      pcie-ep@3400000 {
> > +        compatible = "fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep";
> > +        reg = <0x00 0x03400000 0x0 0x00100000
> > +              0x80 0x00000000 0x8 0x00000000>;
> > +        reg-names = "dbi", "addr_space";
> > +        interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; /* PME interrupt */
> 
> PME or...
> 
> > +        interrupt-names = "app";
> 
> app? You seem to just be changing the names to make the example happy. 
> What do the dts files have? You need to make those pass.

It's on my plan.

First need change 'regs' to 'dbi'. 

https://lore.kernel.org/linux-pci/20240229194559.709182-1-Frank.Li@nxp.com/

After that, I can update all dts.

The second step:

Change EP side interrupt-names. 

Frank

> 
> Rob

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

[Rob Herring]

On Mon, Mar 04, 2024 at 02:08:41PM -0500, Frank Li wrote:
> On Mon, Mar 04, 2024 at 12:20:49PM -0600, Rob Herring wrote:
> > On Fri, Mar 01, 2024 at 11:27:41AM -0500, Frank Li wrote:
> > > Add snps,dw-pcie-ep.yaml.
> > > 
> > > Remove context that exist in snps,dw-pcie-ep.yaml.
> > > 
> > > Add an example for pcie-ep.
> > > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  | 54 ++++++++++---------
> > >  1 file changed, 29 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > index cf517e4e46a33..07965683beece 100644
> > > --- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > @@ -10,8 +10,7 @@ maintainers:
> > >    - Frank Li <Frank.Li@nxp.com>
> > >  
> > >  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 PCIe RC controller is based on the Synopsys DesignWare PCIe IP.
> > >  
> > >    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.
> > > @@ -35,31 +34,18 @@ properties:
> > >        - const: fsl,ls-pcie-ep
> > >  
> > >    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
> > > @@ -68,10 +54,7 @@ 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
> > >  
> > >    big-endian:
> > >      $ref: /schemas/types.yaml#/definitions/flag
> > > @@ -85,3 +68,24 @@ required:
> > >    - reg
> > >    - interrupt-names
> > >  
> > > +allOf:
> > > +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +
> > > +    soc {
> > > +      #address-cells = <2>;
> > > +      #size-cells = <2>;
> > > +
> > > +      pcie-ep@3400000 {
> > > +        compatible = "fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep";
> > > +        reg = <0x00 0x03400000 0x0 0x00100000
> > > +              0x80 0x00000000 0x8 0x00000000>;
> > > +        reg-names = "dbi", "addr_space";
> > > +        interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; /* PME interrupt */
> > 
> > PME or...
> > 
> > > +        interrupt-names = "app";
> > 
> > app? You seem to just be changing the names to make the example happy. 
> > What do the dts files have? You need to make those pass.
> 
> It's on my plan.
> 
> First need change 'regs' to 'dbi'. 
> 
> https://lore.kernel.org/linux-pci/20240229194559.709182-1-Frank.Li@nxp.com/
> 
> After that, I can update all dts.

No! 

I'm saying you shouldn't be changing the dts files. That's an ABI 
which you are likely breaking. You should adjust the binding so the dts 
files pass. The exception is if you know the change is safe and not 
going to break the ABI or the dts is just been wrong all along (e.g. 
missing a compatible the OS doesn't rely on).

Rob

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

[Frank Li]

On Tue, Mar 05, 2024 at 08:37:19AM -0600, Rob Herring wrote:
> On Mon, Mar 04, 2024 at 02:08:41PM -0500, Frank Li wrote:
> > On Mon, Mar 04, 2024 at 12:20:49PM -0600, Rob Herring wrote:
> > > On Fri, Mar 01, 2024 at 11:27:41AM -0500, Frank Li wrote:
> > > > Add snps,dw-pcie-ep.yaml.
> > > > 
> > > > Remove context that exist in snps,dw-pcie-ep.yaml.
> > > > 
> > > > Add an example for pcie-ep.
> > > > 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  | 54 ++++++++++---------
> > > >  1 file changed, 29 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > > index cf517e4e46a33..07965683beece 100644
> > > > --- a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > > @@ -10,8 +10,7 @@ maintainers:
> > > >    - Frank Li <Frank.Li@nxp.com>
> > > >  
> > > >  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 PCIe RC controller is based on the Synopsys DesignWare PCIe IP.
> > > >  
> > > >    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.
> > > > @@ -35,31 +34,18 @@ properties:
> > > >        - const: fsl,ls-pcie-ep
> > > >  
> > > >    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
> > > > @@ -68,10 +54,7 @@ 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
> > > >  
> > > >    big-endian:
> > > >      $ref: /schemas/types.yaml#/definitions/flag
> > > > @@ -85,3 +68,24 @@ required:
> > > >    - reg
> > > >    - interrupt-names
> > > >  
> > > > +allOf:
> > > > +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +
> > > > +    soc {
> > > > +      #address-cells = <2>;
> > > > +      #size-cells = <2>;
> > > > +
> > > > +      pcie-ep@3400000 {
> > > > +        compatible = "fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep";
> > > > +        reg = <0x00 0x03400000 0x0 0x00100000
> > > > +              0x80 0x00000000 0x8 0x00000000>;
> > > > +        reg-names = "dbi", "addr_space";
> > > > +        interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; /* PME interrupt */
> > > 
> > > PME or...
> > > 
> > > > +        interrupt-names = "app";
> > > 
> > > app? You seem to just be changing the names to make the example happy. 
> > > What do the dts files have? You need to make those pass.
> > 
> > It's on my plan.
> > 
> > First need change 'regs' to 'dbi'. 
> > 
> > https://lore.kernel.org/linux-pci/20240229194559.709182-1-Frank.Li@nxp.com/
> > 
> > After that, I can update all dts.
> 
> No! 
> 
> I'm saying you shouldn't be changing the dts files. That's an ABI 
> which you are likely breaking. You should adjust the binding so the dts 
> files pass. The exception is if you know the change is safe and not 
> going to break the ABI or the dts is just been wrong all along (e.g. 
> missing a compatible the OS doesn't rely on).

The problem is that I have not method to make dts check without warning.

If using snps,dw-pcie-ep.yaml/snps,dw-pcie.yaml, it always complain 'regs'
and 'pme'.

If update snps,dw-pcie-ep-.yaml/snps,dw-pcie.yaml, there are two name
'regs' and 'dbi' will be the same thing. it should be bad.

If don't included snps,dw-pcie-ep-.yaml/snps,dw-pcie.yaml, I have to
duplicate most parts, which already in these files.

If you have good method, let me know.

Frank

> 
> Rob