Convert layerscape pcie bind document to yaml file.
Signed-off-by: Frank Li <[email protected]>
---
.../bindings/pci/fsl,layerscape-pcie-ep.yaml | 84 +++++++++
.../bindings/pci/fsl,layerscape-pcie.yaml | 163 ++++++++++++++++++
.../bindings/pci/layerscape-pci.txt | 79 ---------
3 files changed, 247 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..3b592c820eb4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
@@ -0,0 +1,84 @@
+# 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 controller
+
+maintainers:
+ - Frank Li <[email protected]>
+
+description: |+
+ This PCIe endpoint controller is based on the Synopsys DesignWare PCIe IP
+ and thus inherits all the common properties defined in snps,dw-pcie-ep.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,ls2088a-pcie-ep
+ - fsl,ls1088a-pcie-ep
+ - fsl,ls1046a-pcie-ep
+ - fsl,ls1028a-pcie-ep
+ - fsl,lx2160ar2-pcie-ep
+
+ reg:
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: regs
+ - const: addr_space
+
+ fsl,pcie-scfg:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: A phandle 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:
+ $ref: /schemas/types.yaml#/definitions/flag
+ 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.
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#address-cells"
+ - "#size-cells"
+ - device_type
+ - bus-range
+ - ranges
+
+allOf:
+ - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+
+ - if:
+ properties:
+ compatible:
+ enum:
+ - fsl,ls1028a-pcie-ep
+ - fsl,ls1046a-pcie-ep
+ - fsl,ls1088a-pcie-ep
+ then:
+ properties:
+ interrupt-names:
+ items:
+ - const: pme
+
+unevaluatedProperties: false
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..e3719da306f25
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
@@ -0,0 +1,163 @@
+# 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 controller
+
+maintainers:
+ - Frank Li <[email protected]>
+
+description: |+
+ 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.
+
+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
+
+ reg:
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: regs
+ - const: config
+
+ fsl,pcie-scfg:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: A phandle 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:
+ $ref: /schemas/types.yaml#/definitions/flag
+ 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.
+
+ msi-parent: true
+
+ iommu-map: true
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#address-cells"
+ - "#size-cells"
+ - device_type
+ - bus-range
+ - ranges
+ - interrupts
+ - interrupt-names
+ - "#interrupt-cells"
+ - interrupt-map-mask
+ - interrupt-map
+
+allOf:
+ - $ref: /schemas/pci/pci-bus.yaml#
+
+ - 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
+ 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
+
+unevaluatedProperties: false
+
+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
Hey Frank,
On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
> Convert layerscape pcie bind document to yaml file.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> .../bindings/pci/fsl,layerscape-pcie-ep.yaml | 84 +++++++++
> .../bindings/pci/fsl,layerscape-pcie.yaml | 163 ++++++++++++++++++
> .../bindings/pci/layerscape-pci.txt | 79 ---------
> 3 files changed, 247 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..3b592c820eb4c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> @@ -0,0 +1,84 @@
> +# 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 controller
> +
> +maintainers:
> + - Frank Li <[email protected]>
> +
> +description: |+
Are you sure that you need this chomping operator?
> + This PCIe endpoint controller is based on the Synopsys DesignWare PCIe IP
> + and thus inherits all the common properties defined in snps,dw-pcie-ep.yaml.
You shouldn't need this statement given you have the ref: below.
> +
> + 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,ls2088a-pcie-ep
> + - fsl,ls1088a-pcie-ep
> + - fsl,ls1046a-pcie-ep
> + - fsl,ls1028a-pcie-ep
> + - fsl,lx2160ar2-pcie-ep
Where did the fallback compatible go?
> +
> + reg:
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: regs
> + - const: addr_space
The example uses "regs" and "config". Where did addr_space come from?
> + fsl,pcie-scfg:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: A phandle 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:
dma-coherent: true
> + $ref: /schemas/types.yaml#/definitions/flag
> + 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.
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
This was not previously required, why is it required now?
> + - "#address-cells"
> + - "#size-cells"
> + - device_type
> + - bus-range
> + - ranges
> +
> +allOf:
> + - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> +
> + - if:
> + properties:
> + compatible:
> + enum:
> + - fsl,ls1028a-pcie-ep
> + - fsl,ls1046a-pcie-ep
> + - fsl,ls1088a-pcie-ep
> + then:
> + properties:
> + interrupt-names:
> + items:
> + - const: pme
Please define the interrupt properties at the top-level and constrain
them on a per-device basis.
> +
> +unevaluatedProperties: false
> 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..e3719da306f25
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> @@ -0,0 +1,163 @@
> +# 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#
Only brief comments here, as it most is the same comments as for the
> +
> +title: Freescale Layerscape PCIe controller
> +
> +maintainers:
> + - Frank Li <[email protected]>
> +
> +description: |+
> + 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.
Same two comments here as above.
> +
> + 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
> +
> + reg:
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: regs
> + - const: config
> +
> + fsl,pcie-scfg:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: A phandle 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:
> + $ref: /schemas/types.yaml#/definitions/flag
> + 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.
Same here.
> +
> + big-endian:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: If the PEX_LUT and PF register block is in big-endian, specify
> + this property.
> +
> + msi-parent: true
> +
> + iommu-map: true
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
Same here.
> + - "#address-cells"
> + - "#size-cells"
> + - device_type
> + - bus-range
> + - ranges
> + - interrupts
> + - interrupt-names
> + - "#interrupt-cells"
> + - interrupt-map-mask
> + - interrupt-map
> +
> +allOf:
> + - $ref: /schemas/pci/pci-bus.yaml#
> +
> + - 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
> + 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
And same here.
Thanks,
Conor.
On Wed, Feb 07, 2024 at 05:17:55PM +0000, Conor Dooley wrote:
> Hey Frank,
>
> On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
> > Convert layerscape pcie bind document to yaml file.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > .../bindings/pci/fsl,layerscape-pcie-ep.yaml | 84 +++++++++
> > .../bindings/pci/fsl,layerscape-pcie.yaml | 163 ++++++++++++++++++
> > .../bindings/pci/layerscape-pci.txt | 79 ---------
> > 3 files changed, 247 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..3b592c820eb4c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > @@ -0,0 +1,84 @@
> > +# 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 controller
> > +
> > +maintainers:
> > + - Frank Li <[email protected]>
> > +
> > +description: |+
>
> Are you sure that you need this chomping operator?
>
> > + This PCIe endpoint controller is based on the Synopsys DesignWare PCIe IP
>
> > + and thus inherits all the common properties defined in snps,dw-pcie-ep.yaml.
>
> You shouldn't need this statement given you have the ref: below.
>
> > +
> > + 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,ls2088a-pcie-ep
> > + - fsl,ls1088a-pcie-ep
> > + - fsl,ls1046a-pcie-ep
> > + - fsl,ls1028a-pcie-ep
> > + - fsl,lx2160ar2-pcie-ep
>
> Where did the fallback compatible go?
So far, no fallback compatible needed now. each devices already have its
compatible string.
>
> > +
> > + reg:
> > + maxItems: 2
> > +
> > + reg-names:
> > + items:
> > + - const: regs
> > + - const: addr_space
>
> The example uses "regs" and "config". Where did addr_space come from?
Example just show pcie-host part. Not show pcie-ep part.
pcie-ep part need 'addr_space'.
>
> > + fsl,pcie-scfg:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: A phandle 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:
>
> dma-coherent: true
>
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + 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.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
>
> This was not previously required, why is it required now?
Actually its needed.
>
> > + - "#address-cells"
> > + - "#size-cells"
> > + - device_type
> > + - bus-range
> > + - ranges
> > +
> > +allOf:
> > + - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + enum:
> > + - fsl,ls1028a-pcie-ep
> > + - fsl,ls1046a-pcie-ep
> > + - fsl,ls1088a-pcie-ep
> > + then:
> > + properties:
> > + interrupt-names:
> > + items:
> > + - const: pme
>
> Please define the interrupt properties at the top-level and constrain
> them on a per-device basis.
>
> > +
> > +unevaluatedProperties: false
> > 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..e3719da306f25
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> > @@ -0,0 +1,163 @@
> > +# 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#
>
> Only brief comments here, as it most is the same comments as for the
> > +
> > +title: Freescale Layerscape PCIe controller
> > +
> > +maintainers:
> > + - Frank Li <[email protected]>
> > +
> > +description: |+
> > + 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.
>
> Same two comments here as above.
>
> > +
> > + 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
> > +
> > + reg:
> > + maxItems: 2
> > +
> > + reg-names:
> > + items:
> > + - const: regs
> > + - const: config
> > +
> > + fsl,pcie-scfg:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: A phandle 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:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + 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.
>
> Same here.
>
> > +
> > + big-endian:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description: If the PEX_LUT and PF register block is in big-endian, specify
> > + this property.
> > +
> > + msi-parent: true
> > +
> > + iommu-map: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
>
> Same here.
>
> > + - "#address-cells"
> > + - "#size-cells"
> > + - device_type
> > + - bus-range
> > + - ranges
> > + - interrupts
> > + - interrupt-names
> > + - "#interrupt-cells"
> > + - interrupt-map-mask
> > + - interrupt-map
> > +
> > +allOf:
> > + - $ref: /schemas/pci/pci-bus.yaml#
> > +
>
> > + - 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
> > + 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
>
> And same here.
>
> Thanks,
> Conor.
On Wed, Feb 07, 2024 at 12:49:19PM -0500, Frank Li wrote:
> On Wed, Feb 07, 2024 at 05:17:55PM +0000, Conor Dooley wrote:
> > Hey Frank,
> >
> > On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
> > > Convert layerscape pcie bind document to yaml file.
> > >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > .../bindings/pci/fsl,layerscape-pcie-ep.yaml | 84 +++++++++
> > > .../bindings/pci/fsl,layerscape-pcie.yaml | 163 ++++++++++++++++++
> > > .../bindings/pci/layerscape-pci.txt | 79 ---------
> > > 3 files changed, 247 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..3b592c820eb4c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > @@ -0,0 +1,84 @@
> > > +# 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 controller
> > > +
> > > +maintainers:
> > > + - Frank Li <[email protected]>
> > > +
> > > +description: |+
> >
> > Are you sure that you need this chomping operator?
> >
> > > + This PCIe endpoint controller is based on the Synopsys DesignWare PCIe IP
> >
> > > + and thus inherits all the common properties defined in snps,dw-pcie-ep.yaml.
> >
> > You shouldn't need this statement given you have the ref: below.
> >
> > > +
> > > + 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,ls2088a-pcie-ep
> > > + - fsl,ls1088a-pcie-ep
> > > + - fsl,ls1046a-pcie-ep
> > > + - fsl,ls1028a-pcie-ep
> > > + - fsl,lx2160ar2-pcie-ep
> >
> > Where did the fallback compatible go?
>
> So far, no fallback compatible needed now. each devices already have its
> compatible string.
It used to exist though, have you checked that u-boot or *bsd etc do not
use the fallback compatible? You also need to mention your justification
for removing it in the commit message.
> > > +
> > > + reg:
> > > + maxItems: 2
> > > +
> > > + reg-names:
> > > + items:
> > > + - const: regs
> > > + - const: addr_space
> >
> > The example uses "regs" and "config". Where did addr_space come from?
>
> Example just show pcie-host part. Not show pcie-ep part.
> pcie-ep part need 'addr_space'.
Okay. Again, please mention where this is coming from.
>
> >
> > > + fsl,pcie-scfg:
> > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > + description: A phandle 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:
> >
> > dma-coherent: true
> >
> > > + $ref: /schemas/types.yaml#/definitions/flag
> > > + 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.
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - reg-names
> >
> > This was not previously required, why is it required now?
>
> Actually its needed.
Well, if it wasn't, I'd hope that you wouldn't be making it required.
But I asked /why/ and you've not given a reason. Please mention the why
in your commit message for v2.
Cheers,
Conor.
On Thu, Feb 08, 2024 at 02:34:47PM -0500, Frank Li wrote:
> On Thu, Feb 08, 2024 at 07:12:47PM +0000, Conor Dooley wrote:
> > On Wed, Feb 07, 2024 at 12:49:19PM -0500, Frank Li wrote:
> > > On Wed, Feb 07, 2024 at 05:17:55PM +0000, Conor Dooley wrote:
> > > > On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
> > > > > +
> > > > > + 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,ls2088a-pcie-ep
> > > > > + - fsl,ls1088a-pcie-ep
> > > > > + - fsl,ls1046a-pcie-ep
> > > > > + - fsl,ls1028a-pcie-ep
> > > > > + - fsl,lx2160ar2-pcie-ep
> > > >
> > > > Where did the fallback compatible go?
> > >
> > > So far, no fallback compatible needed now. each devices already have its
> > > compatible string.
> >
> > It used to exist though, have you checked that u-boot or *bsd etc do not
> > use the fallback compatible? You also need to mention your justification
> > for removing it in the commit message.
>
> This commit just convert binding doc from txt to yaml. I just make sure
> which equal to what descript in txt.
The text binding does have a fallback compatible though:
EP mode:
"fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep"
So this is a change compared to the text binding, without any
justification for it being okay to do.
> If there are someting wrong in "uboot"
> or "bsd", we can fixed it later.
If other bits of software are using the fallback, you cannot remove it.
> I checked driver code. exited dts tree
> under kernel, which use unexited fallback compatible string
> "fsl, lx-pcie-ep", which should be removed at dts file.
What do you mean by "unexisted"? It was in the text binding, so it is
perfectly fine to have it in the dts. Given it has users, I don't think
you should be removing the fallback without a very good justification.
> > > > > + reg:
> > > > > + maxItems: 2
> > > > > +
> > > > > + reg-names:
> > > > > + items:
> > > > > + - const: regs
> > > > > + - const: addr_space
> > > >
> > > > The example uses "regs" and "config". Where did addr_space come from?
> > >
> > > Example just show pcie-host part. Not show pcie-ep part.
> > > pcie-ep part need 'addr_space'.
> >
> > Okay. Again, please mention where this is coming from.
>
> Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
> of 'regs'. It needs extra effort to make driver code algin common
> snps,dwc-pcie-ep.yaml, and update exist all dts files.
>
> I think it will be deleted soon.
What I am looking for here is you to explain in the commit message that
the endpoint driver in linux and the dts have always used "addr_space".
Checking that there's not a u-boot or *bsd that uses "config" would also
be very helpful.
Thanks,
Conor.
On Thu, Feb 08, 2024 at 07:12:47PM +0000, Conor Dooley wrote:
> On Wed, Feb 07, 2024 at 12:49:19PM -0500, Frank Li wrote:
> > On Wed, Feb 07, 2024 at 05:17:55PM +0000, Conor Dooley wrote:
> > > Hey Frank,
> > >
> > > On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
> > > > Convert layerscape pcie bind document to yaml file.
> > > >
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > .../bindings/pci/fsl,layerscape-pcie-ep.yaml | 84 +++++++++
> > > > .../bindings/pci/fsl,layerscape-pcie.yaml | 163 ++++++++++++++++++
> > > > .../bindings/pci/layerscape-pci.txt | 79 ---------
> > > > 3 files changed, 247 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..3b592c820eb4c
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> > > > @@ -0,0 +1,84 @@
> > > > +# 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 controller
> > > > +
> > > > +maintainers:
> > > > + - Frank Li <[email protected]>
> > > > +
> > > > +description: |+
> > >
> > > Are you sure that you need this chomping operator?
> > >
> > > > + This PCIe endpoint controller is based on the Synopsys DesignWare PCIe IP
> > >
> > > > + and thus inherits all the common properties defined in snps,dw-pcie-ep.yaml.
> > >
> > > You shouldn't need this statement given you have the ref: below.
> > >
> > > > +
> > > > + 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,ls2088a-pcie-ep
> > > > + - fsl,ls1088a-pcie-ep
> > > > + - fsl,ls1046a-pcie-ep
> > > > + - fsl,ls1028a-pcie-ep
> > > > + - fsl,lx2160ar2-pcie-ep
> > >
> > > Where did the fallback compatible go?
> >
> > So far, no fallback compatible needed now. each devices already have its
> > compatible string.
>
> It used to exist though, have you checked that u-boot or *bsd etc do not
> use the fallback compatible? You also need to mention your justification
> for removing it in the commit message.
This commit just convert binding doc from txt to yaml. I just make sure
which equal to what descript in txt. If there are someting wrong in "uboot"
or "bsd", we can fixed it later. I checked driver code. exited dts tree
under kernel, which use unexited fallback compatible string
"fsl, lx-pcie-ep", which should be removed at dts file.
>
> > > > +
> > > > + reg:
> > > > + maxItems: 2
> > > > +
> > > > + reg-names:
> > > > + items:
> > > > + - const: regs
> > > > + - const: addr_space
> > >
> > > The example uses "regs" and "config". Where did addr_space come from?
> >
> > Example just show pcie-host part. Not show pcie-ep part.
> > pcie-ep part need 'addr_space'.
>
> Okay. Again, please mention where this is coming from.
Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
of 'regs'. It needs extra effort to make driver code algin common
snps,dwc-pcie-ep.yaml, and update exist all dts files.
I think it will be deleted soon.
>
> >
> > >
> > > > + fsl,pcie-scfg:
> > > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > > + description: A phandle 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:
> > >
> > > dma-coherent: true
> > >
> > > > + $ref: /schemas/types.yaml#/definitions/flag
> > > > + 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.
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - reg
> > > > + - reg-names
> > >
> > > This was not previously required, why is it required now?
> >
> > Actually its needed.
>
> Well, if it wasn't, I'd hope that you wouldn't be making it required.
> But I asked /why/ and you've not given a reason. Please mention the why
> in your commit message for v2.
Sorry, I just sent v2 before see this. According to driver code, it is
needed. Please check v2, I will update at v3.
>
> Cheers,
> Conor.
>
On Thu, Feb 08, 2024 at 07:44:59PM +0000, Conor Dooley wrote:
> On Thu, Feb 08, 2024 at 02:34:47PM -0500, Frank Li wrote:
> > On Thu, Feb 08, 2024 at 07:12:47PM +0000, Conor Dooley wrote:
> > > On Wed, Feb 07, 2024 at 12:49:19PM -0500, Frank Li wrote:
> > > > On Wed, Feb 07, 2024 at 05:17:55PM +0000, Conor Dooley wrote:
> > > > > On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
>
> > > > > > +
> > > > > > + 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,ls2088a-pcie-ep
> > > > > > + - fsl,ls1088a-pcie-ep
> > > > > > + - fsl,ls1046a-pcie-ep
> > > > > > + - fsl,ls1028a-pcie-ep
> > > > > > + - fsl,lx2160ar2-pcie-ep
> > > > >
> > > > > Where did the fallback compatible go?
> > > >
> > > > So far, no fallback compatible needed now. each devices already have its
> > > > compatible string.
> > >
> > > It used to exist though, have you checked that u-boot or *bsd etc do not
> > > use the fallback compatible? You also need to mention your justification
> > > for removing it in the commit message.
> >
> > This commit just convert binding doc from txt to yaml. I just make sure
> > which equal to what descript in txt.
>
> The text binding does have a fallback compatible though:
> EP mode:
> "fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep"
> So this is a change compared to the text binding, without any
> justification for it being okay to do.
Okay, I see what your concern. I think old txt is wrong. Or try to work
as it, but actually not.
>
> > If there are someting wrong in "uboot"
> > or "bsd", we can fixed it later.
>
> If other bits of software are using the fallback, you cannot remove it.
>
> > I checked driver code. exited dts tree
> > under kernel, which use unexited fallback compatible string
> > "fsl, lx-pcie-ep", which should be removed at dts file.
>
> What do you mean by "unexisted"? It was in the text binding, so it is
> perfectly fine to have it in the dts. Given it has users, I don't think
> you should be removing the fallback without a very good justification.
>
No driver parse "fsl,lx-pcie-ep". I can keep it as equal as old txt file
and remove later if need.
> > > > > > + reg:
> > > > > > + maxItems: 2
> > > > > > +
> > > > > > + reg-names:
> > > > > > + items:
> > > > > > + - const: regs
> > > > > > + - const: addr_space
> > > > >
> > > > > The example uses "regs" and "config". Where did addr_space come from?
> > > >
> > > > Example just show pcie-host part. Not show pcie-ep part.
> > > > pcie-ep part need 'addr_space'.
> > >
> > > Okay. Again, please mention where this is coming from.
> >
> > Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
> > of 'regs'. It needs extra effort to make driver code algin common
> > snps,dwc-pcie-ep.yaml, and update exist all dts files.
> >
> > I think it will be deleted soon.
>
> What I am looking for here is you to explain in the commit message that
> the endpoint driver in linux and the dts have always used "addr_space".
> Checking that there's not a u-boot or *bsd that uses "config" would also
> be very helpful.
I confused. Actually this two part PCIE-RC and PCIE-EP.
PCIE-RC using 'config'
PCIE-EP using 'addr_spcae'
I check old txt file, which have not mention it. I can remove it.
Frank
>
> Thanks,
> Conor.
>
On Thu, Feb 08, 2024 at 03:20:50PM -0500, Frank Li wrote:
> > > > > > > + reg:
> > > > > > > + maxItems: 2
> > > > > > > +
> > > > > > > + reg-names:
> > > > > > > + items:
> > > > > > > + - const: regs
> > > > > > > + - const: addr_space
> > > > > >
> > > > > > The example uses "regs" and "config". Where did addr_space come from?
> > > > >
> > > > > Example just show pcie-host part. Not show pcie-ep part.
> > > > > pcie-ep part need 'addr_space'.
> > > >
> > > > Okay. Again, please mention where this is coming from.
> > >
> > > Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
> > > of 'regs'. It needs extra effort to make driver code algin common
> > > snps,dwc-pcie-ep.yaml, and update exist all dts files.
> > >
> > > I think it will be deleted soon.
> >
> > What I am looking for here is you to explain in the commit message that
> > the endpoint driver in linux and the dts have always used "addr_space".
> > Checking that there's not a u-boot or *bsd that uses "config" would also
> > be very helpful.
>
> I confused. Actually this two part PCIE-RC and PCIE-EP.
> PCIE-RC using 'config'
> PCIE-EP using 'addr_spcae'
Yeah, I get this. The text binding makes it seem like "config" should be
used for both RC and EP, so I am just asking you to check that there are
no drivers in other kernels or bootloaders that use "config" for EP
mode.
> I check old txt file, which have not mention it. I can remove it.
if you drop "addr_space", you'll need to update the endpoint driver so
that it supports both "addr_space" and "config". If there are no
endpoint drivers using "config" in other operating systems, and all the
dts files use "addr_space", documenting "reg" and "addr_space" for
endpoint mode seems fair to me.
Thanks,
Conor.
On Thu, Feb 08, 2024 at 09:20:08PM +0000, Conor Dooley wrote:
> On Thu, Feb 08, 2024 at 03:20:50PM -0500, Frank Li wrote:
>
> > > > > > > > + reg:
> > > > > > > > + maxItems: 2
> > > > > > > > +
> > > > > > > > + reg-names:
> > > > > > > > + items:
> > > > > > > > + - const: regs
> > > > > > > > + - const: addr_space
> > > > > > >
> > > > > > > The example uses "regs" and "config". Where did addr_space come from?
> > > > > >
> > > > > > Example just show pcie-host part. Not show pcie-ep part.
> > > > > > pcie-ep part need 'addr_space'.
> > > > >
> > > > > Okay. Again, please mention where this is coming from.
> > > >
> > > > Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
> > > > of 'regs'. It needs extra effort to make driver code algin common
> > > > snps,dwc-pcie-ep.yaml, and update exist all dts files.
> > > >
> > > > I think it will be deleted soon.
> > >
> > > What I am looking for here is you to explain in the commit message that
> > > the endpoint driver in linux and the dts have always used "addr_space".
> > > Checking that there's not a u-boot or *bsd that uses "config" would also
> > > be very helpful.
> >
> > I confused. Actually this two part PCIE-RC and PCIE-EP.
> > PCIE-RC using 'config'
> > PCIE-EP using 'addr_spcae'
>
> Yeah, I get this. The text binding makes it seem like "config" should be
> used for both RC and EP, so I am just asking you to check that there are
> no drivers in other kernels or bootloaders that use "config" for EP
> mode.
There are not 'config' concept for EP mode. Only RC mode have 'config'
space concept to get PCIe device's config space. EP mode only have
"add_space" for outbound windows. If other place using "config" for EP, it
is totally wrong, they should fix it.
>
> > I check old txt file, which have not mention it. I can remove it.
>
> if you drop "addr_space", you'll need to update the endpoint driver so
> that it supports both "addr_space" and "config". If there are no
> endpoint drivers using "config" in other operating systems, and all the
> dts files use "addr_space", documenting "reg" and "addr_space" for
> endpoint mode seems fair to me.
It is up to how to create patches. "addr_space" needs. If you want me to
create one version, which 100% match original txt. I can do that. Then
create increment patch to fix the problem.
If want to create a basic work version like this, which included some minus
fixes.
The both method is fine for me. Second method just need more efforts.
Frank
>
> Thanks,
> Conor.
On Thu, Feb 08, 2024 at 04:34:19PM -0500, Frank Li wrote:
> On Thu, Feb 08, 2024 at 09:20:08PM +0000, Conor Dooley wrote:
> > On Thu, Feb 08, 2024 at 03:20:50PM -0500, Frank Li wrote:
> >
> > > > > > > > > + reg:
> > > > > > > > > + maxItems: 2
> > > > > > > > > +
> > > > > > > > > + reg-names:
> > > > > > > > > + items:
> > > > > > > > > + - const: regs
> > > > > > > > > + - const: addr_space
> > > > > > > >
> > > > > > > > The example uses "regs" and "config". Where did addr_space come from?
> > > > > > >
> > > > > > > Example just show pcie-host part. Not show pcie-ep part.
> > > > > > > pcie-ep part need 'addr_space'.
> > > > > >
> > > > > > Okay. Again, please mention where this is coming from.
> > > > >
> > > > > Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
> > > > > of 'regs'. It needs extra effort to make driver code algin common
> > > > > snps,dwc-pcie-ep.yaml, and update exist all dts files.
> > > > >
> > > > > I think it will be deleted soon.
> > > >
> > > > What I am looking for here is you to explain in the commit message that
> > > > the endpoint driver in linux and the dts have always used "addr_space".
> > > > Checking that there's not a u-boot or *bsd that uses "config" would also
> > > > be very helpful.
> > >
> > > I confused. Actually this two part PCIE-RC and PCIE-EP.
> > > PCIE-RC using 'config'
> > > PCIE-EP using 'addr_spcae'
> >
> > Yeah, I get this. The text binding makes it seem like "config" should be
> > used for both RC and EP, so I am just asking you to check that there are
> > no drivers in other kernels or bootloaders that use "config" for EP
> > mode.
>
> There are not 'config' concept for EP mode. Only RC mode have 'config'
> space concept to get PCIe device's config space. EP mode only have
> "add_space" for outbound windows. If other place using "config" for EP, it
> is totally wrong, they should fix it.
It might be a totally wrong concept, but it is what the binding said, so
they are within their rights to use that name.
>
> >
> > > I check old txt file, which have not mention it. I can remove it.
> >
> > if you drop "addr_space", you'll need to update the endpoint driver so
> > that it supports both "addr_space" and "config". If there are no
> > endpoint drivers using "config" in other operating systems, and all the
> > dts files use "addr_space", documenting "reg" and "addr_space" for
> > endpoint mode seems fair to me.
>
> It is up to how to create patches. "addr_space" needs. If you want me to
> create one version, which 100% match original txt. I can do that. Then
> create increment patch to fix the problem.
>
> If want to create a basic work version like this, which included some minus
> fixes.
>
> The both method is fine for me. Second method just need more efforts.
I don't have a problem with you making it "addr_space" as long as you
check to make sure that there are no users of "config" outside the
kernel and you mention in the commit message that this is a difference
from the text binding (and why).
Thanks,
Conor.