| Message ID | 20240301162741.765524-2-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | dt-bindings: pci: layerscape-pci: Convert to yaml format | expand |
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 */ > + }; > + }; > +...
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 */ > > + }; > > + }; > > +...
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
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
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
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 */ - };
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