Ok, I misunderstood the patch set (I thought the PciHostBridgeLib itself would
eventually move to DEN0115).
I still think that (in general) would be a good idea - if not for the benefit
of the Pi, then for the next upstreamed platform where you could avoid
implementing custom config access code...
Reviewed-by: Andrei Warkentin <awarken...@vmware.com>
--
Andrei Warkentin,
Arm Enablement Architect,
Cloud Platform Business Unit, VMware
________________________________
From: Andrei Warkentin <awarken...@vmware.com>
Sent: Friday, August 6, 2021 7:02 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; jeremy.lin...@arm.com
<jeremy.lin...@arm.com>
Cc: p...@akeo.ie <p...@akeo.ie>; ardb+tianoc...@kernel.org
<ardb+tianoc...@kernel.org>; sunny.w...@arm.com <sunny.w...@arm.com>;
samer.el-haj-mahm...@arm.com <samer.el-haj-mahm...@arm.com>; René Treffer
<treffer+groups...@measite.de>
Subject: Re: [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for
CM4
Hi Jeremy,
Is any of this still conceptually necessary if we adopt the SMCCC interface
within UEFI?
Instead of assuming the first downstream bus is bus 1, could you read the
secondary BN from the RP?
--
Andrei Warkentin,
Arm Enablement Architect,
Cloud Platform Business Unit, VMware
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Jeremy Linton
via groups.io <jeremy.linton=arm....@groups.io>
Sent: Thursday, August 5, 2021 7:35 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: p...@akeo.ie <p...@akeo.ie>; ardb+tianoc...@kernel.org
<ardb+tianoc...@kernel.org>; Andrei Warkentin <awarken...@vmware.com>;
sunny.w...@arm.com <sunny.w...@arm.com>; samer.el-haj-mahm...@arm.com
<samer.el-haj-mahm...@arm.com>; Jeremy Linton <jeremy.lin...@arm.com>; René
Treffer <treffer+groups...@measite.de>
Subject: [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for CM4
The CM4 has an actual pcie slot, so we need to move the linkup
check to the configuration probe logic. Further the device
restriction logic needs to be relaxed to support downstream
PCIe switches.
Suggested-by: René Treffer <treffer+groups...@measite.de>
Signed-off-by: Jeremy Linton <jeremy.lin...@arm.com>
---
.../Bcm2711PciHostBridgeLibConstructor.c | 5 -----
.../Library/Bcm2711PciSegmentLib/PciSegmentLib.c | 24 +++++++++++++++-------
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git
a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c
b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c
index 8587d2d36d..4d4c584726 100644
---
a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c
+++
b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c
@@ -204,11 +204,6 @@ Bcm2711PciHostBridgeLibConstructor (
} while (((Data & 0x30) != 0x030) && (Timeout));
DEBUG ((DEBUG_VERBOSE, "PCIe link ready (status=%x) Timeout=%d\n", Data,
Timeout));
- if ((Data & 0x30) != 0x30) {
- DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=%x)\n", Data));
- return EFI_DEVICE_ERROR;
- }
-
if ((Data & 0x80) != 0x80) {
DEBUG ((DEBUG_ERROR, "PCIe link not in RC mode (status=%x)\n", Data));
return EFI_UNSUPPORTED;
diff --git
a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
index 44ce3b4b99..3ccc131eab 100644
--- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
+++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
@@ -78,6 +78,8 @@ PciSegmentLibGetConfigBase (
UINT64 Base;
UINT64 Offset;
UINT32 Dev;
+ UINT32 Bus;
+ UINT32 Data;
Base = PCIE_REG_BASE;
Offset = Address & 0xFFF; /* Pick off the 4k register offset */
@@ -89,17 +91,25 @@ PciSegmentLibGetConfigBase (
Base += PCIE_EXT_CFG_DATA;
if (mPciSegmentLastAccess != Address) {
Dev = EFI_PCI_ADDR_DEV (Address);
+ Bus = EFI_PCI_ADDR_BUS (Address);
+
/*
- * Scan things out directly rather than translating the "bus" to a
device, etc..
- * only we need to limit each bus to a single device.
+ * There can only be a single device on bus 1 (downstream of root).
+ * Subsequent busses (behind a PCIe switch) can have more.
*/
- if (Dev < 1) {
- MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address);
- mPciSegmentLastAccess = Address;
- } else {
- mPciSegmentLastAccess = 0;
+ if (Dev > 0 && (Bus < 2)) {
return 0xFFFFFFFF;
}
+
+ /* Don't probe slots if the link is down */
+ Data = MmioRead32 (PCIE_REG_BASE + PCIE_MISC_PCIE_STATUS);
+ if ((Data & 0x30) != 0x30) {
+ DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=%x)\n", Data));
+ return 0xFFFFFFFF;
+ }
+
+ MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address);
+ mPciSegmentLastAccess = Address;
}
}
return Base + Offset;
--
2.13.7
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78815): https://edk2.groups.io/g/devel/message/78815
Mute This Topic: https://groups.io/mt/84688700/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
