Add ECAM aperture for Tegra234
Subsystems (6)
Jon Hunter
maintainerv3 cover Add ECAM aperture for Tegra234
This series adds ECAM aperture information in both device-tree and documentation files for Tegra234.
Vidya Sagar (2):
dt-bindings: PCI: tegra234: Add ECAM support
arm64: tegra: Add ECAM aperture info for all the PCIe controllers
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 55 +++++++++++-------- 3 files changed, 66 insertions(+), 25 deletions(-)
Signature · 1 line
2.25.1
Jon Hunter
maintainerPATCH v3 1/2 dt-bindings: PCI: tegra234: Add ECAM support
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +32 -2
@@ -27,6 +27,7 @@ properties: - nvidia,tegra234-pcie reg: + minItems: 4 items: - description: controller's application logic registers - description: configuration registers
@@ -35,13 +36,16 @@ properties: 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: + minItems: 4 items: - const: appl - const: config - const: atu_dma - const: dbi + - const: ecam interrupts: items:
@@ -202,6 +206,31 @@ properties: allOf: - $ref: /schemas/pci/snps,dw-pcie.yaml# + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra194-pcie + then: + properties: + reg: + maxItems: 4 + reg-names: + maxItems: 4 + + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra234-pcie + then: + properties: + reg: + minItems: 5 + reg-names: + minItems: 5 unevaluatedProperties: false
@@ -305,8 +334,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>;
Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml +1 -1
@@ -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: |
Signature · 1 line
2.25.1
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 8 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>
Best regards,
Krzysztof
Rob Herring
maintainerRe: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
In reply to Jon HunterQuoted 34 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it..../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +1
-0@@ -27,6 +27,7 @@ properties: - nvidia,tegra234-pcie reg: + minItems: 4 items: - description: controller's application logic registers - description: configuration registers@@ -35,13 +36,16 @@ properties: available for software access. - description: aperture where the Root Port's own configuration registers are available.
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml properties: · line 48
Quoted 10 lines
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +3
-0+ - description: aperture to access the configuration space through ECAM. reg-names: + minItems: 4 items: - const: appl - const: config - const: atu_dma - const: dbi + - const: ecam
Wouldn't this be mutually exclusive with 'config'? 'config' is not really h/w, but an just an iATU window typically.
Where's the driver change to use this?
Rob
Jon Hunter
maintainerRe: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 37 lines · Rob Herring
Quoted 36 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it..../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +2
-0@@ -27,6 +27,7 @@ properties: - nvidia,tegra234-pcie reg: + minItems: 4 items: - description: controller's application logic registers - description: configuration registers@@ -35,13 +36,16 @@ properties: 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.
Quoted 11 lines
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml properties: · line 48
Quoted 8 lines
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +2
-0reg-names: + minItems: 4 items: - const: appl - const: config - const: atu_dma - const: dbi + - const: ecamWouldn't this be mutually exclusive with 'config'? 'config' is not really h/w, but an just an iATU window typically.
Yes that is true, however, I was chatting with Sagar and we really need both ranges to be defined.
Quoted 1 line
Where's the driver change to use this?
For Linux there is not one. However, we need this for our port of the EDK2 bootloader [0] that parses device-tree and can support booting Linux with either device-tree or ACPI. We wanted to have a common device-tree we can use for EDK2 and Linux.
Jon
[0] https://github.com/NVIDIA/edk2-nvidia/wiki
Signature · 1 line
nvpublic
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 55 lines · Jon Hunter
Quoted 37 lines · Rob Herring
Quoted 36 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it..../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +2
-0@@ -27,6 +27,7 @@ properties: - nvidia,tegra234-pcie reg: + minItems: 4 items: - description: controller's application logic registers - description: configuration registers@@ -35,13 +36,16 @@ properties: 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.Quoted 11 lines
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml properties: · line 48
Quoted 8 lines
Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml +2
-0reg-names: + minItems: 4 items: - const: appl - const: config - const: atu_dma - const: dbi + - const: ecamWouldn't this be mutually exclusive with 'config'? 'config' is not really h/w, but an just an iATU window typically.
Yes that is true, however, I was chatting with Sagar and we really need both ranges to be defined.
Quoted 6 lines
Quoted 1 line
Where's the driver change to use this?
For Linux there is not one. However, we need this for our port of the EDK2 bootloader [0] that parses device-tree and can support booting Linux with either device-tree or ACPI. We wanted to have a common device-tree we can use for EDK2 and Linux.
Ok, makes sense.
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
In reply to Jon HunterQuoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)
Both patches applied now.
Thanks,
Thierry
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.
Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl']
'ecam' is not one of ['atu_dma']
'ecam' is not one of ['smu', 'mpu']
From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 39 lines · Rob Herring
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
Quoted 5 lines
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl'] 'ecam' is not one of ['atu_dma'] 'ecam' is not one of ['smu', 'mpu'] From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Stephen reported the other day that he wasn't able to resolve this conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has now propagated to ARM SoC so it can't be easily backed out, but I guess we could revert on that tree and instead apply the patch to the DT tree and resolve the conflict there.
I guess the better alternative would be to try and resolve the merge properly and let Stephen (and Linus) know.
Thierry
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 47 lines · Thierry Reding
Quoted 39 lines · Rob Herring
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:Quoted 5 lines
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl'] 'ecam' is not one of ['atu_dma'] 'ecam' is not one of ['smu', 'mpu'] From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Quoted 8 lines
Stephen reported the other day that he wasn't able to resolve this conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has now propagated to ARM SoC so it can't be easily backed out, but I guess we could revert on that tree and instead apply the patch to the DT tree and resolve the conflict there.
I guess the better alternative would be to try and resolve the merge properly and let Stephen (and Linus) know.
Instead, can you prepare a patch on top of Sergey's adding a 'oneOf' entry with 'ecam'. As this is a new thing, it should have its own entry. Then when merging, we just throw out the change from your side.
I'd really prefer that bindings don't go thru the soc tree unless there's some strong reason. The default is to go via the subsystem trees. Beyond 'we are running the dtschema checks on all our dts files and can't have the warnings', I don't know what that would be. I wish everyone was doing that, but I'm pretty sure most are not.
Rob
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 61 lines · Rob Herring
Quoted 47 lines · Thierry Reding
Quoted 39 lines · Rob Herring
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:Quoted 5 lines
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl'] 'ecam' is not one of ['atu_dma'] 'ecam' is not one of ['smu', 'mpu'] From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Quoted 8 lines
Stephen reported the other day that he wasn't able to resolve this conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has now propagated to ARM SoC so it can't be easily backed out, but I guess we could revert on that tree and instead apply the patch to the DT tree and resolve the conflict there.
I guess the better alternative would be to try and resolve the merge properly and let Stephen (and Linus) know.
Instead, can you prepare a patch on top of Sergey's adding a 'oneOf' entry with 'ecam'. As this is a new thing, it should have its own entry. Then when merging, we just throw out the change from your side.
Quoted 5 lines
I'd really prefer that bindings don't go thru the soc tree unless there's some strong reason. The default is to go via the subsystem trees. Beyond 'we are running the dtschema checks on all our dts files and can't have the warnings', I don't know what that would be. I wish everyone was doing that, but I'm pretty sure most are not.
That's actually the reason why I wanted this to go through ARM SoC, so that the validation tools wouldn't accumulate even more errors. For some time now I've been working on getting at least Tegra to validate properly and on 64-bit ARM I have a local tree on top of linux-next that has no warnings left, though I do need to flush out quite a few bindings conversions.
Until those are all upstream, that argument is a little moot and don't feel strongly about where the DT bindings ultimately end up.
Thierry
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
In reply to Rob HerringQuoted 54 lines · Rob Herring
Quoted 53 lines · Thierry Reding
Quoted 39 lines · Rob Herring
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:Quoted 5 lines
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl'] 'ecam' is not one of ['atu_dma'] 'ecam' is not one of ['smu', 'mpu'] From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Stephen reported the other day that he wasn't able to resolve this conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has now propagated to ARM SoC so it can't be easily backed out, but I guess we could revert on that tree and instead apply the patch to the DT tree and resolve the conflict there.
Quoted 7 lines
Quoted 2 lines
I guess the better alternative would be to try and resolve the merge properly and let Stephen (and Linus) know.
Instead, can you prepare a patch on top of Sergey's adding a 'oneOf' entry with 'ecam'. As this is a new thing, it should have its own entry. Then when merging, we just throw out the change from your side.
Right, the only change that is required here is to extend the reg-names oneOf list of the DT-bindings:
< Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
with the 'ecam' entry. If it's a vendor-specific part then add to the last the last entry defines the vendor-specific duplicates of the generic CSR spaces.
On the other hand I don't really see a reason in adding the ECAM CSRs space to the generic DW PCIe device since basically the ECAM memory is just a pre-configured outbound iATU window. So if it's a ECAM-based device then it should have been already configured by the system bootloader upon the kernel boot up. Thus there is no point in having the generic DW PCIe resources and it should be just a generic ECAM-based device with a single ECAM CSR space as the "snps,dw-pcie-ecam"/"pci-host-ecam-generic" DT-bindings require especially seeing the Nvidia low-level driver doesn't use the ECAM registers at all. Moreover the DW PCIe core driver doesn't differentiate between the already configured iATU windows and the one available for the ranges-based mapping. Instead the DW PCIe core just disables all the detected in- and outbound iATUs by means of the dw_pcie_iatu_setup() method. So the pre-configured ECAM space will be reset by the driver core anyway.
What I would suggest here is to split up the devices:
1. If it's a ECAM-based device, then it should be identified as "snps,dw-pcie-ecam"/"pci-host-ecam-generic", then it will have a single ECAM CSR space with no need in other resources.
2. If it's a DW PCIe device with Nvidia Tegra194-specific settings, then it should be identified as "nvidia,tegra194-pcie" with no ECAM registers.
Thus we wouldn't need to have any modifications applied to the bindings and to the DT-files. What do you think?
-Serge(y)
Quoted 9 lines
I'd really prefer that bindings don't go thru the soc tree unless there's some strong reason. The default is to go via the subsystem trees. Beyond 'we are running the dtschema checks on all our dts files and can't have the warnings', I don't know what that would be. I wish everyone was doing that, but I'm pretty sure most are not.
Rob
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 71 lines · Serge Semin
Quoted 54 lines · Rob Herring
Quoted 53 lines · Thierry Reding
Quoted 39 lines · Rob Herring
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:Quoted 5 lines
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl'] 'ecam' is not one of ['atu_dma'] 'ecam' is not one of ['smu', 'mpu'] From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Stephen reported the other day that he wasn't able to resolve this conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has now propagated to ARM SoC so it can't be easily backed out, but I guess we could revert on that tree and instead apply the patch to the DT tree and resolve the conflict there.
Quoted 7 lines
Quoted 2 lines
I guess the better alternative would be to try and resolve the merge properly and let Stephen (and Linus) know.
Instead, can you prepare a patch on top of Sergey's adding a 'oneOf' entry with 'ecam'. As this is a new thing, it should have its own entry. Then when merging, we just throw out the change from your side.
Right, the only change that is required here is to extend the reg-names oneOf list of the DT-bindings:
<Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
with the 'ecam' entry. If it's a vendor-specific part then add to the last the last entry defines the vendor-specific duplicates of the generic CSR spaces.
Quoted 15 lines
On the other hand I don't really see a reason in adding the ECAM CSRs space to the generic DW PCIe device since basically the ECAM memory is just a pre-configured outbound iATU window. So if it's a ECAM-based device then it should have been already configured by the system bootloader upon the kernel boot up. Thus there is no point in having the generic DW PCIe resources and it should be just a generic ECAM-based device with a single ECAM CSR space as the "snps,dw-pcie-ecam"/"pci-host-ecam-generic" DT-bindings require especially seeing the Nvidia low-level driver doesn't use the ECAM registers at all. Moreover the DW PCIe core driver doesn't differentiate between the already configured iATU windows and the one available for the ranges-based mapping. Instead the DW PCIe core just disables all the detected in- and outbound iATUs by means of the
dw_pcie_iatu_setup()method. So the pre-configured ECAM space will be reset by the driver core anyway.
This was discussed some before. This is for the firmware/bootloader to setup ECAM mode. Then the kernel will see generic (ACPI) ECAM.
Yes, it is iATU config, but so is 'config'. If we were starting over, I'd say 'reg' should just have the entire address space for iATU and the driver could figure out how to configure it (beyond what ranges says). But that ship has sailed. Also, note that the address range here is disjoint from 'config', so it looks like we'd need 2 entries anyways.
Rob
Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support
Quoted 93 lines · Rob Herring
Quoted 71 lines · Serge Semin
Quoted 54 lines · Rob Herring
Quoted 53 lines · Thierry Reding
Quoted 39 lines · Rob Herring
Quoted 16 lines · Thierry Reding
Quoted 14 lines · Jon Hunter
From: Vidya Sagar <[email protected]>
Add support for ECAM aperture that is only supported for Tegra234 devices.
Signed-off-by Vidya Sagar <[email protected]>Co-developed-by Jon Hunter <[email protected]>Signed-off-by Jon Hunter <[email protected]>---
Changes since V2:
- Avoid duplication of reg items and reg-names
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.Quoted 5 lines
Quoted 3 lines
.../bindings/pci/nvidia,tegra194-pcie.yaml | 34 +++++++++++++++++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-)Both patches applied now.
linux-next now fails with this. I suspect it is due to Sergey's changes to the DWC schema.
/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:
'dbi' was expected
'dbi2' was expected
'ecam' is not one of ['elbi', 'app']
'atu' was expected
'dma' was expected
'phy' was expected
'config' was expected /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
fixed:Quoted 5 lines
'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl'] 'ecam' is not one of ['atu_dma'] 'ecam' is not one of ['smu', 'mpu'] From schema:/builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
Stephen reported the other day that he wasn't able to resolve this conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has now propagated to ARM SoC so it can't be easily backed out, but I guess we could revert on that tree and instead apply the patch to the DT tree and resolve the conflict there.
Quoted 7 lines
Quoted 2 lines
I guess the better alternative would be to try and resolve the merge properly and let Stephen (and Linus) know.
Instead, can you prepare a patch on top of Sergey's adding a 'oneOf' entry with 'ecam'. As this is a new thing, it should have its own entry. Then when merging, we just throw out the change from your side.
Right, the only change that is required here is to extend the reg-names oneOf list of the DT-bindings:
<Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
with the 'ecam' entry. If it's a vendor-specific part then add to the last the last entry defines the vendor-specific duplicates of the generic CSR spaces.Quoted 15 lines
On the other hand I don't really see a reason in adding the ECAM CSRs space to the generic DW PCIe device since basically the ECAM memory is just a pre-configured outbound iATU window. So if it's a ECAM-based device then it should have been already configured by the system bootloader upon the kernel boot up. Thus there is no point in having the generic DW PCIe resources and it should be just a generic ECAM-based device with a single ECAM CSR space as the "snps,dw-pcie-ecam"/"pci-host-ecam-generic" DT-bindings require especially seeing the Nvidia low-level driver doesn't use the ECAM registers at all. Moreover the DW PCIe core driver doesn't differentiate between the already configured iATU windows and the one available for the ranges-based mapping. Instead the DW PCIe core just disables all the detected in- and outbound iATUs by means of the
dw_pcie_iatu_setup()method. So the pre-configured ECAM space will be reset by the driver core anyway.This was discussed some before. This is for the
firmware/bootloaderto setup ECAM mode. Then the kernel will see generic (ACPI) ECAM.
Quoted 6 lines
Yes, it is iATU config, but so is 'config'. If we were starting over, I'd say 'reg' should just have the entire address space for iATU and the driver could figure out how to configure it (beyond what ranges says). But that ship has sailed. Also, note that the address range here is disjoint from 'config', so it looks like we'd need 2 entries anyways.
Thanks for the explanation. I've got another suggestion them. The semantics of the 'config' reg-space and the ECAM reg-space is very similar. The only difference is the way the corresponding outbound iATU window is configured. The DW PCIe driver just maps it in accordance with the requested peripheral device BDF value so the very first 4KB is translated to the CFG TLPs sent out to that device. The same space could be mapped with the IATU_REGION_CTRL_2_OFF_OUTBOUND_i.CFG_SHIFT_MODE flag set. So the entire PCIe BDFs space will be available via the same MMIO region. Thus the corresponding peripheral device will be available at the BDF-based offset with respect to the 'config' reg-space base address. Of course in the later case the 'config' MMIO range must be much greater (256MB) than in the former case (4KB), but still the difference in just the way the window is configured. Moreover the DW PCIe driver could be easier fixed to using the later approach if the 'config' reg-space is specified wide enough to map the entire PCIe bus CFG-space (256MB).
Based on that and on the fact you said that the ECAM reg-space will be used by the bootloader/firmware only anyway, I'd suggest to just extend the 'config' reg-space semantics. So the bootloader/firmware will be able to use it to create a generic ECAM device. The kernel DW PCIe driver will by default use it to remap each peripheral device CFG-space on request, but at some point the driver could be converted to map the entire PCIe BDFs similarly to the ECAM space. Thus we won't need to add a redundant ECAM reg-space. What do you think?
* hopefully it isn't too late for the suggested approach.
-Serge(y)
Quoted 2 lines
Rob
Jon Hunter
maintainerPATCH v3 2/2 arm64: tegra: Add ECAM aperture info for all the PCIe controllers
In reply to Jon HunterFrom: Vidya Sagar <[email protected]>
Add the ECAM aperture information for all the PCIe controllers of Tegra234.
---
Changes since V2:
- None
Changes since V1:
- None
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 55 ++++++++++++++---------- 1 file changed, 33 insertions(+), 22 deletions(-)
arch/arm64/boot/dts/nvidia/tegra234.dtsi +33 -22
@@ -2235,8 +2235,9 @@ pcie@140a0000 { reg = <0x00 0x140a0000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x2a000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x2a040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x2a080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x2a080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x35 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2288,8 +2289,9 @@ pcie@140c0000 { reg = <0x00 0x140c0000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x2c000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x2c040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x2c080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x2c080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x38 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2341,8 +2343,9 @@ pcie@140e0000 { reg = <0x00 0x140e0000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x2e000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x2e040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x2e080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x2e080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x3b 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2394,8 +2397,9 @@ pcie@14100000 { reg = <0x00 0x14100000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x30000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x30040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x30080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x30080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x20 0xb0000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2447,8 +2451,9 @@ pcie@14120000 { reg = <0x00 0x14120000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x32000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x32040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x32080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x32080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x20 0xf0000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2500,8 +2505,9 @@ pcie@14140000 { reg = <0x00 0x14140000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x34000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x34040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x34080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x34080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x21 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2553,8 +2559,9 @@ pcie@14160000 { 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>;
@@ -2606,8 +2613,9 @@ pcie@14180000 { reg = <0x00 0x14180000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x38000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x38040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x38080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x38080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x27 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2659,8 +2667,9 @@ pcie@141a0000 { reg = <0x00 0x141a0000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x3a000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x3a040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x3a080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x3a080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x2b 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2712,8 +2721,9 @@ pcie@141c0000 { reg = <0x00 0x141c0000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x3c000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x3c040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x3c080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x3c080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x2e 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
@@ -2765,8 +2775,9 @@ pcie@141e0000 { reg = <0x00 0x141e0000 0x0 0x00020000>, /* appl registers (128K) */ <0x00 0x3e000000 0x0 0x00040000>, /* configuration space (256K) */ <0x00 0x3e040000 0x0 0x00040000>, /* iATU_DMA reg space (256K) */ - <0x00 0x3e080000 0x0 0x00040000>; /* DBI reg space (256K) */ - reg-names = "appl", "config", "atu_dma", "dbi"; + <0x00 0x3e080000 0x0 0x00040000>, /* DBI reg space (256K) */ + <0x32 0x30000000 0x0 0x10000000>; /* ECAM (256MB) */ + reg-names = "appl", "config", "atu_dma", "dbi", "ecam"; #address-cells = <3>; #size-cells = <2>;
Signature · 1 line
2.25.1