1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v3 0/7] RPi4: Enable ACPI PCIe conduit
  4. View patch

[edk2-devel] [PATCH v3 5/7] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor

Jeremy Linton posted 7 patches 4 years, 5 months ago
[edk2-devel] [PATCH v3 5/7] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor
Posted by Jeremy Linton 4 years, 5 months ago 
The existing code fails to create/finish configuring the
pcie subsystem if it fails to get a linkup. This is reasonable
on the RPi4 because it generally won't happen, and the OS
could not see the root port. Now that the OS can see the
root port, its a bit odd if it only shows up when
something is plugged into the first slot. Lets move the
link up check into the config accessor where it will be used
to restrict sending CFG TLP's out the port when nothing is
plugged in. Thus avoiding a SERROR during probe.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 .../Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c   | 5 -----
 .../Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c  | 7 +++++++
 2 files changed, 7 insertions(+), 5 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 6d15e82fa2..b627e5730b 100644
--- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
+++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
@@ -105,6 +105,13 @@ PciSegmentLibGetConfigBase (
           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;
     }
-- 
2.13.7



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79611): https://edk2.groups.io/g/devel/message/79611
Mute This Topic: https://groups.io/mt/85014310/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 5/7] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor
Posted by Andrei Warkentin 4 years, 5 months ago 
Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
________________________________
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, August 19, 2021 11:16 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: pete@akeo.ie <pete@akeo.ie>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Andrei Warkentin <awarkentin@vmware.com>; Sunny.Wang@arm.com <Sunny.Wang@arm.com>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; Jeremy Linton <jeremy.linton@arm.com>
Subject: [PATCH v3 5/7] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor

The existing code fails to create/finish configuring the
pcie subsystem if it fails to get a linkup. This is reasonable
on the RPi4 because it generally won't happen, and the OS
could not see the root port. Now that the OS can see the
root port, its a bit odd if it only shows up when
something is plugged into the first slot. Lets move the
link up check into the config accessor where it will be used
to restrict sending CFG TLP's out the port when nothing is
plugged in. Thus avoiding a SERROR during probe.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 .../Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c   | 5 -----
 .../Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c  | 7 +++++++
 2 files changed, 7 insertions(+), 5 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 6d15e82fa2..b627e5730b 100644
--- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
+++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
@@ -105,6 +105,13 @@ PciSegmentLibGetConfigBase (
           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;
     }
--
2.13.7



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79651): https://edk2.groups.io/g/devel/message/79651
Mute This Topic: https://groups.io/mt/85014310/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 5/7] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor
Posted by Ard Biesheuvel 4 years, 5 months ago 
On Fri, 20 Aug 2021 at 06:16, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> The existing code fails to create/finish configuring the
> pcie subsystem if it fails to get a linkup. This is reasonable
> on the RPi4 because it generally won't happen, and the OS
> could not see the root port. Now that the OS can see the
> root port, its a bit odd if it only shows up when
> something is plugged into the first slot. Lets move the
> link up check into the config accessor where it will be used
> to restrict sending CFG TLP's out the port when nothing is
> plugged in. Thus avoiding a SERROR during probe.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

How will this work when the PCIE/XHCI switch is in 'platform device' mode?

> ---
>  .../Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c   | 5 -----
>  .../Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c  | 7 +++++++
>  2 files changed, 7 insertions(+), 5 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 6d15e82fa2..b627e5730b 100644
> --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
> +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
> @@ -105,6 +105,13 @@ PciSegmentLibGetConfigBase (
>            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;
>      }
> --
> 2.13.7
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79683): https://edk2.groups.io/g/devel/message/79683
Mute This Topic: https://groups.io/mt/85014310/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 5/7] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor
Posted by Ard Biesheuvel 4 years, 5 months ago 
On Sun, 22 Aug 2021 at 15:37, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 20 Aug 2021 at 06:16, Jeremy Linton <jeremy.linton@arm.com> wrote:
> >
> > The existing code fails to create/finish configuring the
> > pcie subsystem if it fails to get a linkup. This is reasonable
> > on the RPi4 because it generally won't happen, and the OS
> > could not see the root port. Now that the OS can see the
> > root port, its a bit odd if it only shows up when
> > something is plugged into the first slot. Lets move the
> > link up check into the config accessor where it will be used
> > to restrict sending CFG TLP's out the port when nothing is
> > plugged in. Thus avoiding a SERROR during probe.
> >
> > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>
> How will this work when the PCIE/XHCI switch is in 'platform device' mode?
>

Never mind, that only affects the OS whereas this affects UEFI itself only.

> > ---
> >  .../Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c   | 5 -----
> >  .../Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c  | 7 +++++++
> >  2 files changed, 7 insertions(+), 5 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 6d15e82fa2..b627e5730b 100644
> > --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
> > +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
> > @@ -105,6 +105,13 @@ PciSegmentLibGetConfigBase (
> >            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;
> >      }
> > --
> > 2.13.7
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79684): https://edk2.groups.io/g/devel/message/79684
Mute This Topic: https://groups.io/mt/85014310/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.