[edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for CM4

[Andrei Warkentin]

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 <awarkentin at vmware.com>

--
Andrei Warkentin,
Arm Enablement Architect,
Cloud Platform Business Unit, VMware
________________________________
From: Andrei Warkentin <awarkentin at vmware.com>
Sent: Friday, August 6, 2021 7:02 PM
To: devel at edk2.groups.io <devel at edk2.groups.io>; jeremy.linton at arm.com <jeremy.linton at arm.com>
Cc: pete at akeo.ie <pete at akeo.ie>; ardb+tianocore at kernel.org <ardb+tianocore at kernel.org>; Sunny.Wang at arm.com <Sunny.Wang at arm.com>; samer.el-haj-mahmoud at arm.com <samer.el-haj-mahmoud at arm.com>; René Treffer <treffer+groups.io at 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 at edk2.groups.io <devel at edk2.groups.io> on behalf of Jeremy Linton via groups.io <jeremy.linton=arm.com at groups.io>
Sent: Thursday, August 5, 2021 7:35 PM
To: devel at edk2.groups.io <devel at edk2.groups.io>
Cc: pete at akeo.ie <pete at akeo.ie>; ardb+tianocore at kernel.org <ardb+tianocore at kernel.org>; Andrei Warkentin <awarkentin at vmware.com>; Sunny.Wang at arm.com <Sunny.Wang at arm.com>; samer.el-haj-mahmoud at arm.com <samer.el-haj-mahmoud at arm.com>; Jeremy Linton <jeremy.linton at arm.com>; René Treffer <treffer+groups.io at 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.io at measite.de>
Signed-off-by: Jeremy Linton <jeremy.linton at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210806/6c858b2f/attachment.htm>