[edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for CM4
Jeremy Linton
jeremy.linton at arm.com
Fri Aug 6 21:52:56 UTC 2021
Hi,
On 8/6/21 11:04 AM, Andrei Warkentin wrote:
> 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...
Right, the only bit that goes away is the PciSegmentLibGetConfigBase()
code to be replaced by the SMC call. Which I will do, but I think its
better to fix to this one and make that a separate patch-set ideally
with another platform in parallel.
>
> 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 (#78834): https://edk2.groups.io/g/devel/message/78834
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list