| Message ID | 20221114140916.200395-2-jonathanh@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add ECAM aperture for Tegra234 | expand |
On 14/11/2022 15:09, Jon Hunter wrote: > From: Vidya Sagar <vidyas@nvidia.com> > > Add support for ECAM aperture that is only supported for Tegra234 > devices. > > Co-developed-by: Vidya Sagar <vidyas@nvidia.com> > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > Co-developed-by: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > Changes since V1: > - Restricted the ECAM aperture to only Tegra234 devices that support it. > > .../bindings/pci/nvidia,tegra194-pcie.yaml | 76 +++++++++++++++---- > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- > 2 files changed, 62 insertions(+), 16 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml > index 75da3e8eecb9..7ae0f37f5364 100644 > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml > @@ -27,21 +27,12 @@ properties: > - nvidia,tegra234-pcie > > reg: > - items: > - - description: controller's application logic registers > - - description: configuration registers > - - description: iATU and DMA registers. This is where the iATU (internal > - Address Translation Unit) registers of the PCIe core are made > - available for software access. > - - description: aperture where the Root Port's own configuration > - registers are available. > + minItems: 4 > + maxItems: 5 > > reg-names: > - items: > - - const: appl > - - const: config > - - const: atu_dma > - - const: dbi > + minItems: 4 > + maxItems: 5 > > interrupts: > items: > @@ -202,6 +193,60 @@ properties: > > allOf: > - $ref: /schemas/pci/snps,dw-pcie.yaml# > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra194-pcie > + then: > + properties: > + reg: > + minItems: 4 > + maxItems: 4 How you wrote it, you do not need min/maxItems here, because you have items below. However see further comment. > + items: > + - description: controller's application logic registers > + - description: configuration registers > + - description: iATU and DMA registers. This is where the iATU (internal > + Address Translation Unit) registers of the PCIe core are made > + available for software access. > + - description: aperture where the Root Port's own configuration > + registers are available. > + reg-names: > + items: > + - const: appl > + - const: config > + - const: atu_dma > + - const: dbi > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra234-pcie > + then: > + properties: > + reg: > + minItems: 5 > + maxItems: 5 Similar issue. > + items: > + - description: controller's application logic registers > + - description: configuration registers > + - description: iATU and DMA registers. This is where the iATU (internal > + Address Translation Unit) registers of the PCIe core are made > + available for software access. > + - description: aperture where the Root Port's own configuration > + registers are available. > + - description: aperture to access the configuration space through ECAM. This is unnecessarily duplicated. You can keep the descriptions of items and reg-names items in top level (with min 4 and max 5) and restrict maxItems for 194 and minItems for 234 here. > + reg-names: > + items: > + - const: appl > + - const: config > + - const: atu_dma > + - const: dbi > + - const: ecam > + No need for blank line. > > unevaluatedProperties: false > Best regards, Krzysztof
On 14/11/2022 14:23, Krzysztof Kozlowski wrote: > On 14/11/2022 15:09, Jon Hunter wrote: >> From: Vidya Sagar <vidyas@nvidia.com> >> >> Add support for ECAM aperture that is only supported for Tegra234 >> devices. >> >> Co-developed-by: Vidya Sagar <vidyas@nvidia.com> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> Co-developed-by: Jon Hunter <jonathanh@nvidia.com> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> Changes since V1: >> - Restricted the ECAM aperture to only Tegra234 devices that support it. >> >> .../bindings/pci/nvidia,tegra194-pcie.yaml | 76 +++++++++++++++---- >> .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- >> 2 files changed, 62 insertions(+), 16 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >> index 75da3e8eecb9..7ae0f37f5364 100644 >> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >> @@ -27,21 +27,12 @@ properties: >> - nvidia,tegra234-pcie >> >> reg: >> - items: >> - - description: controller's application logic registers >> - - description: configuration registers >> - - description: iATU and DMA registers. This is where the iATU (internal >> - Address Translation Unit) registers of the PCIe core are made >> - available for software access. >> - - description: aperture where the Root Port's own configuration >> - registers are available. >> + minItems: 4 >> + maxItems: 5 >> >> reg-names: >> - items: >> - - const: appl >> - - const: config >> - - const: atu_dma >> - - const: dbi >> + minItems: 4 >> + maxItems: 5 >> >> interrupts: >> items: >> @@ -202,6 +193,60 @@ properties: >> >> allOf: >> - $ref: /schemas/pci/snps,dw-pcie.yaml# >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - nvidia,tegra194-pcie >> + then: >> + properties: >> + reg: >> + minItems: 4 >> + maxItems: 4 > > How you wrote it, you do not need min/maxItems here, because you have > items below. However see further comment. > >> + items: >> + - description: controller's application logic registers >> + - description: configuration registers >> + - description: iATU and DMA registers. This is where the iATU (internal >> + Address Translation Unit) registers of the PCIe core are made >> + available for software access. >> + - description: aperture where the Root Port's own configuration >> + registers are available. >> + reg-names: >> + items: >> + - const: appl >> + - const: config >> + - const: atu_dma >> + - const: dbi >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - nvidia,tegra234-pcie >> + then: >> + properties: >> + reg: >> + minItems: 5 >> + maxItems: 5 > > Similar issue. > >> + items: >> + - description: controller's application logic registers >> + - description: configuration registers >> + - description: iATU and DMA registers. This is where the iATU (internal >> + Address Translation Unit) registers of the PCIe core are made >> + available for software access. >> + - description: aperture where the Root Port's own configuration >> + registers are available. >> + - description: aperture to access the configuration space through ECAM. > > This is unnecessarily duplicated. You can keep the descriptions of items > and reg-names items in top level (with min 4 and max 5) and restrict > maxItems for 194 and minItems for 234 here. Yes I wondered if there was a good way to avoid duplication. It looks like I cannot have 'maxItems' and 'items' at the top-level, but obviously I can set 'maxItems' appropriately for each device. Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml: properties:reg: {'minItems': 4, 'maxItems': 5, 'items': [{'description': "controller's application logic registers"}, {'description': 'configuration registers'}, {'description': 'iATU and DMA registers. This is where the iATU (internal Address Translation Unit) registers of the PCIe core are made available for software access.'}, {'description': "aperture where the Root Port's own configuration registers are available."}, {'description': 'aperture to access the configuration space through ECAM.'}]} should not be valid under {'required': ['maxItems']} hint: "maxItems" is not needed with an "items" list Thanks Jon
On 14/11/2022 15:36, Jon Hunter wrote: > > On 14/11/2022 14:23, Krzysztof Kozlowski wrote: >> On 14/11/2022 15:09, Jon Hunter wrote: >>> From: Vidya Sagar <vidyas@nvidia.com> >>> >>> Add support for ECAM aperture that is only supported for Tegra234 >>> devices. >>> >>> Co-developed-by: Vidya Sagar <vidyas@nvidia.com> >>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >>> Co-developed-by: Jon Hunter <jonathanh@nvidia.com> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>> --- >>> Changes since V1: >>> - Restricted the ECAM aperture to only Tegra234 devices that support it. >>> >>> .../bindings/pci/nvidia,tegra194-pcie.yaml | 76 +++++++++++++++---- >>> .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- >>> 2 files changed, 62 insertions(+), 16 deletions(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >>> b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >>> index 75da3e8eecb9..7ae0f37f5364 100644 >>> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >>> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml >>> @@ -27,21 +27,12 @@ properties: >>> - nvidia,tegra234-pcie >>> reg: >>> - items: >>> - - description: controller's application logic registers >>> - - description: configuration registers >>> - - description: iATU and DMA registers. This is where the iATU >>> (internal >>> - Address Translation Unit) registers of the PCIe core are made >>> - available for software access. >>> - - description: aperture where the Root Port's own configuration >>> - registers are available. >>> + minItems: 4 >>> + maxItems: 5 >>> reg-names: >>> - items: >>> - - const: appl >>> - - const: config >>> - - const: atu_dma >>> - - const: dbi >>> + minItems: 4 >>> + maxItems: 5 >>> interrupts: >>> items: >>> @@ -202,6 +193,60 @@ properties: >>> allOf: >>> - $ref: /schemas/pci/snps,dw-pcie.yaml# >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - nvidia,tegra194-pcie >>> + then: >>> + properties: >>> + reg: >>> + minItems: 4 >>> + maxItems: 4 >> >> How you wrote it, you do not need min/maxItems here, because you have >> items below. However see further comment. >> >>> + items: >>> + - description: controller's application logic registers >>> + - description: configuration registers >>> + - description: iATU and DMA registers. This is where the >>> iATU (internal >>> + Address Translation Unit) registers of the PCIe core >>> are made >>> + available for software access. >>> + - description: aperture where the Root Port's own >>> configuration >>> + registers are available. >>> + reg-names: >>> + items: >>> + - const: appl >>> + - const: config >>> + - const: atu_dma >>> + - const: dbi >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - nvidia,tegra234-pcie >>> + then: >>> + properties: >>> + reg: >>> + minItems: 5 >>> + maxItems: 5 >> >> Similar issue. >> >>> + items: >>> + - description: controller's application logic registers >>> + - description: configuration registers >>> + - description: iATU and DMA registers. This is where the >>> iATU (internal >>> + Address Translation Unit) registers of the PCIe core >>> are made >>> + available for software access. >>> + - description: aperture where the Root Port's own >>> configuration >>> + registers are available. >>> + - description: aperture to access the configuration >>> space through ECAM. >> >> This is unnecessarily duplicated. You can keep the descriptions of items >> and reg-names items in top level (with min 4 and max 5) and restrict >> maxItems for 194 and minItems for 234 here. > > > Yes I wondered if there was a good way to avoid duplication. It looks > like I cannot have 'maxItems' and 'items' at the top-level, but > obviously I can set 'maxItems' appropriately for each device. Ah I get it. The 'items' list is defining the max. I have fixed up and sent out a new revision. Jon
diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml index 75da3e8eecb9..7ae0f37f5364 100644 --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml @@ -27,21 +27,12 @@ properties: - nvidia,tegra234-pcie reg: - items: - - description: controller's application logic registers - - description: configuration registers - - description: iATU and DMA registers. This is where the iATU (internal - Address Translation Unit) registers of the PCIe core are made - available for software access. - - description: aperture where the Root Port's own configuration - registers are available. + minItems: 4 + maxItems: 5 reg-names: - items: - - const: appl - - const: config - - const: atu_dma - - const: dbi + minItems: 4 + maxItems: 5 interrupts: items: @@ -202,6 +193,60 @@ properties: allOf: - $ref: /schemas/pci/snps,dw-pcie.yaml# + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra194-pcie + then: + properties: + reg: + minItems: 4 + maxItems: 4 + items: + - description: controller's application logic registers + - description: configuration registers + - description: iATU and DMA registers. This is where the iATU (internal + Address Translation Unit) registers of the PCIe core are made + available for software access. + - description: aperture where the Root Port's own configuration + registers are available. + reg-names: + items: + - const: appl + - const: config + - const: atu_dma + - const: dbi + + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra234-pcie + then: + properties: + reg: + minItems: 5 + maxItems: 5 + items: + - description: controller's application logic registers + - description: configuration registers + - description: iATU and DMA registers. This is where the iATU (internal + Address Translation Unit) registers of the PCIe core are made + available for software access. + - description: aperture where the Root Port's own configuration + registers are available. + - description: aperture to access the configuration space through ECAM. + reg-names: + items: + - const: appl + - const: config + - const: atu_dma + - const: dbi + - const: ecam + unevaluatedProperties: false @@ -305,8 +350,9 @@ examples: reg = <0x00 0x14160000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x36000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x36040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x36080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x36080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x24 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>; diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml index 7287d395e1b6..7e0b015f1414 100644 --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml @@ -35,7 +35,7 @@ properties: maxItems: 5 items: enum: [ dbi, dbi2, config, atu, atu_dma, app, appl, elbi, mgmt, ctrl, - parf, cfg, link, ulreg, smu, mpu, apb, phy ] + parf, cfg, link, ulreg, smu, mpu, apb, phy, ecam ] num-lanes: description: |