The PCIe PIO translation is incorrect on the Juno, correct that. While we are updating that module correct the comments to more accurately reflect the code and what is actually happening.
Jeremy Linton (2): ArmJuno: fix Juno PIO host bridge mapping ArmJuno: Correct AXI->PCIe translation comments
.../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
The Juno PIO mapping is 8M, so it should be using a 32-bit PIO address translation. Further, PIO addresses should start at 0 and be translated to/from the ARM MMIO region.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeremy Linton jeremy.linton@arm.com Reviewed-by: Leif Lindholm leif.lindholm@linaro.org --- ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 06de6d5..0c7881d 100644 --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c @@ -72,7 +72,9 @@ HWPciRbInit (
PCI_TRACE ("PCIe Setting up Address Translation");
- PCIE_ROOTPORT_WRITE32 (PCIE_BAR_WIN, PCIE_BAR_WIN_SUPPORT_IO | PCIE_BAR_WIN_SUPPORT_MEM | PCIE_BAR_WIN_SUPPORT_MEM64); + // The Juno PIO window is 8M, so we need full 32-bit PIO decoding. + PCIE_ROOTPORT_WRITE32 (PCIE_BAR_WIN, PCIE_BAR_WIN_SUPPORT_IO | PCIE_BAR_WIN_SUPPORT_IO32 | + PCIE_BAR_WIN_SUPPORT_MEM | PCIE_BAR_WIN_SUPPORT_MEM64);
// Setup the PCI Configuration Registers // Offset 0a: SubClass 04 PCI-PCI Bridge @@ -107,8 +109,9 @@ HWPciRbInit ( SetTranslationAddressEntry (CpuIo, TranslationTable, PCI_ECAM_BASE, PCI_ECAM_BASE, PCI_ECAM_SIZE, PCI_ATR_TRSLID_PCIE_CONF); TranslationTable += PCI_ATR_ENTRY_SIZE;
- // PCI IO Support - SetTranslationAddressEntry (CpuIo, TranslationTable, PCI_IO_BASE, PCI_IO_BASE, PCI_IO_SIZE, PCI_ATR_TRSLID_PCIE_IO); + // PCI IO Support, the PIO space is translated from the arm MMIO PCI_IO_BASE address to the PIO base address of 0 + // AKA, PIO addresses used by endpoints are generally in the range of 0-64K. + SetTranslationAddressEntry (CpuIo, TranslationTable, PCI_IO_BASE, 0, PCI_IO_SIZE, PCI_ATR_TRSLID_PCIE_IO); TranslationTable += PCI_ATR_ENTRY_SIZE;
// PCI MEM32 Support
The AXI<->PCIe translation comments are out of date with respect to the code. In the first case the AXI master port is incorrectly called a slave. In the second case the the translation direction indicated for the slave port is the wrong direction.
Correct both of these comments to reflect what the code is doing.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeremy Linton jeremy.linton@arm.com --- ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 0c7881d..d134cc4 100644 --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c @@ -84,7 +84,7 @@ HWPciRbInit ( PCIE_ROOTPORT_WRITE32 (PCIE_PCI_IDS + PCIE_PCI_IDS_CLASSCODE_OFFSET, ((PLDA_BRIDGE_CCR << 8) | PCI_BRIDGE_REVISION_ID));
// - // PCIE Window 0 -> AXI4 Slave 0 Address Translations + // PCIE Window 0 -> AXI4 Master 0 Address Translations // TranslationTable = VEXPRESS_ATR_PCIE_WIN0;
@@ -101,7 +101,7 @@ HWPciRbInit ( ARM_JUNO_EXTRA_SYSTEM_MEMORY_SZ, PCI_ATR_TRSLID_AXIMEMORY);
// - // PCIE Window 0 -> AXI4 Slave 0 Address Translations + // AXI4 Slave 0 -> PCIE Window 0 Address Translations // TranslationTable = VEXPRESS_ATR_AXI4_SLV1;
On 14 July 2016 at 15:58, Jeremy Linton jeremy.linton@arm.com wrote:
The AXI<->PCIe translation comments are out of date with respect to the code. In the first case the AXI master port is incorrectly called a slave. In the second case the the translation direction indicated for the slave port is the wrong direction.
Correct both of these comments to reflect what the code is doing.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeremy Linton jeremy.linton@arm.com
ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 0c7881d..d134cc4 100644 --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c @@ -84,7 +84,7 @@ HWPciRbInit ( PCIE_ROOTPORT_WRITE32 (PCIE_PCI_IDS + PCIE_PCI_IDS_CLASSCODE_OFFSET, ((PLDA_BRIDGE_CCR << 8) | PCI_BRIDGE_REVISION_ID));
//
- // PCIE Window 0 -> AXI4 Slave 0 Address Translations
- // PCIE Window 0 -> AXI4 Master 0 Address Translations // TranslationTable = VEXPRESS_ATR_PCIE_WIN0;
@@ -101,7 +101,7 @@ HWPciRbInit ( ARM_JUNO_EXTRA_SYSTEM_MEMORY_SZ, PCI_ATR_TRSLID_AXIMEMORY);
//
- // PCIE Window 0 -> AXI4 Slave 0 Address Translations
- // AXI4 Slave 0 -> PCIE Window 0 Address Translations // TranslationTable = VEXPRESS_ATR_AXI4_SLV1;
Slave 0 or slave 1?
On 07/14/2016 09:16 AM, Ard Biesheuvel wrote:
On 14 July 2016 at 15:58, Jeremy Linton jeremy.linton@arm.com wrote:
The AXI<->PCIe translation comments are out of date with respect to the code. In the first case the AXI master port is incorrectly called a slave. In the second case the the translation direction indicated for the slave port is the wrong direction.
Correct both of these comments to reflect what the code is doing.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeremy Linton jeremy.linton@arm.com
ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 0c7881d..d134cc4 100644 --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c @@ -84,7 +84,7 @@ HWPciRbInit ( PCIE_ROOTPORT_WRITE32 (PCIE_PCI_IDS + PCIE_PCI_IDS_CLASSCODE_OFFSET, ((PLDA_BRIDGE_CCR << 8) | PCI_BRIDGE_REVISION_ID));
//
- // PCIE Window 0 -> AXI4 Slave 0 Address Translations
- // PCIE Window 0 -> AXI4 Master 0 Address Translations // TranslationTable = VEXPRESS_ATR_PCIE_WIN0;
@@ -101,7 +101,7 @@ HWPciRbInit ( ARM_JUNO_EXTRA_SYSTEM_MEMORY_SZ, PCI_ATR_TRSLID_AXIMEMORY);
//
- // PCIE Window 0 -> AXI4 Slave 0 Address Translations
- // AXI4 Slave 0 -> PCIE Window 0 Address Translations // TranslationTable = VEXPRESS_ATR_AXI4_SLV1;
Slave 0 or slave 1?
Slave 1, I actually was just talking to Leif about tweaking it before committing it.
On Thu, Jul 14, 2016 at 09:19:09AM -0500, Jeremy Linton wrote:
On 07/14/2016 09:16 AM, Ard Biesheuvel wrote:
On 14 July 2016 at 15:58, Jeremy Linton jeremy.linton@arm.com wrote:
The AXI<->PCIe translation comments are out of date with respect to the code. In the first case the AXI master port is incorrectly called a slave. In the second case the the translation direction indicated for the slave port is the wrong direction.
Correct both of these comments to reflect what the code is doing.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeremy Linton jeremy.linton@arm.com
ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 0c7881d..d134cc4 100644 --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c @@ -84,7 +84,7 @@ HWPciRbInit ( PCIE_ROOTPORT_WRITE32 (PCIE_PCI_IDS + PCIE_PCI_IDS_CLASSCODE_OFFSET, ((PLDA_BRIDGE_CCR << 8) | PCI_BRIDGE_REVISION_ID));
//
- // PCIE Window 0 -> AXI4 Slave 0 Address Translations
- // PCIE Window 0 -> AXI4 Master 0 Address Translations // TranslationTable = VEXPRESS_ATR_PCIE_WIN0;
@@ -101,7 +101,7 @@ HWPciRbInit ( ARM_JUNO_EXTRA_SYSTEM_MEMORY_SZ, PCI_ATR_TRSLID_AXIMEMORY);
//
- // PCIE Window 0 -> AXI4 Slave 0 Address Translations
- // AXI4 Slave 0 -> PCIE Window 0 Address Translations // TranslationTable = VEXPRESS_ATR_AXI4_SLV1;
Slave 0 or slave 1?
Slave 1, I actually was just talking to Leif about tweaking it before committing it.
Fixed and series pushed, thanks.
/ Leif
Thanks Jeremy.
If Ryan's happy with this change, I'm happy to push it.
Regards,
Leif
On 14 July 2016 at 14:58, Jeremy Linton jeremy.linton@arm.com wrote:
The PCIe PIO translation is incorrect on the Juno, correct that. While we are updating that module correct the comments to more accurately reflect the code and what is actually happening.
Jeremy Linton (2): ArmJuno: fix Juno PIO host bridge mapping ArmJuno: Correct AXI->PCIe translation comments
.../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
-- 2.4.11
On 14 July 2016 at 15:09, Leif Lindholm leif.lindholm@linaro.org wrote:
Thanks Jeremy.
If Ryan's happy with this change, I'm happy to push it.
Yep, fine. I tested v3 (I think) and it worked fine.
Regards,
Leif
On 14 July 2016 at 14:58, Jeremy Linton jeremy.linton@arm.com wrote:
The PCIe PIO translation is incorrect on the Juno, correct that. While we are updating that module correct the comments to more accurately reflect the code and what is actually happening.
Jeremy Linton (2): ArmJuno: fix Juno PIO host bridge mapping ArmJuno: Correct AXI->PCIe translation comments
.../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
-- 2.4.11
-
Ard Biesheuvel -
Jeremy Linton -
Leif Lindholm -
Ryan Harkin