The Juno PIO mapping is 8M, so it should be using 32-bit PIO address translation. Futher, PIO addresses should start at 0 and be translated to/from the ARM MMIO region.
Signed-off-by: Jeremy Linton jeremy.linton@arm.com --- .../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 06de6d5..1774e7d 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 @@ -82,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;
@@ -107,9 +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
On Mon, Jul 11, 2016 at 11:17:09AM -0500, Jeremy Linton wrote:
The Juno PIO mapping is 8M, so it should be using 32-bit PIO address translation. Futher, PIO addresses should start at 0 and be translated to/from the ARM MMIO region.
Needs: Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeremy Linton jeremy.linton@arm.com
.../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 06de6d5..1774e7d 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 @@ -82,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
So, this looks like a change unrelated to the overall fix. It also looks like a sufficiantly spectacular fix (if only of a comment) that it merits its own commit message. Could you break it out into a separate patch please?
// TranslationTable = VEXPRESS_ATR_PCIE_WIN0; @@ -107,9 +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 -- 2.4.11
Looks like a clear fix to me - but I'd appreciate Ryan's review as well. Apart from that - with the standalone comment change dropped from this one, and the contributed-under added: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
On 11 July 2016 at 17:27, Leif Lindholm leif.lindholm@linaro.org wrote:
On Mon, Jul 11, 2016 at 11:17:09AM -0500, Jeremy Linton wrote:
The Juno PIO mapping is 8M, so it should be using 32-bit PIO address translation. Futher,
"Futher" -> "Further"?
PIO addresses should start at 0 and be translated to/from the ARM MMIO region.
Needs: Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeremy Linton jeremy.linton@arm.com
.../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 06de6d5..1774e7d 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);
So it should be IO32 *and* MEM64 at the same time?
// Setup the PCI Configuration Registers // Offset 0a: SubClass 04 PCI-PCI Bridge @@ -82,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
So, this looks like a change unrelated to the overall fix. It also looks like a sufficiantly spectacular fix (if only of a comment) that it merits its own commit message. Could you break it out into a separate patch please?
// TranslationTable = VEXPRESS_ATR_PCIE_WIN0;
@@ -107,9 +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
-- 2.4.11
Looks like a clear fix to me - but I'd appreciate Ryan's review as well.
I have no clue if the change is correct or not, but it looks like it might do what Jeremy says it does, if that helps??
Apart from that - with the standalone comment change dropped from this one, and the contributed-under added: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
On 07/11/2016 11:39 AM, Ryan Harkin wrote:
On 11 July 2016 at 17:27, Leif Lindholm leif.lindholm@linaro.org wrote:
On Mon, Jul 11, 2016 at 11:17:09AM -0500, Jeremy Linton wrote:
The Juno PIO mapping is 8M, so it should be using 32-bit PIO address translation. Futher,
"Futher" -> "Further"?
That's texas spelling of Boston english...
<chuckle>
PIO addresses should start at 0 and be translated to/from the ARM MMIO region.
Needs: Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeremy Linton jeremy.linton@arm.com
.../ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/XPressRich3.c index 06de6d5..1774e7d 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);So it should be IO32 *and* MEM64 at the same time?
// Setup the PCI Configuration Registers // Offset 0a: SubClass 04 PCI-PCI Bridge@@ -82,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
So, this looks like a change unrelated to the overall fix. It also looks like a sufficiantly spectacular fix (if only of a comment) that it merits its own commit message. Could you break it out into a separate patch please?
// TranslationTable = VEXPRESS_ATR_PCIE_WIN0;@@ -107,9 +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
-- 2.4.11
Looks like a clear fix to me - but I'd appreciate Ryan's review as well.
I have no clue if the change is correct or not, but it looks like it might do what Jeremy says it does, if that helps??
Apart from that - with the standalone comment change dropped from this one, and the contributed-under added: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
-
Jeremy Linton -
Leif Lindholm -
Ryan Harkin