Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml format
From: Rob Herring
Date: Mon Mar 04 2024 - 12:32:21 EST
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@xxxxxxx>
> ---
> .../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@xxxxxxx>
> +
> +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@xxxxxxx>
> +
> +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 */
> + };
> + };
> +...