From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web09.7.1628286782071291384 for ; Fri, 06 Aug 2021 14:53:02 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jeremy.linton@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A0DAB31B; Fri, 6 Aug 2021 14:53:01 -0700 (PDT) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5B49D3F66F; Fri, 6 Aug 2021 14:53:01 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for CM4 To: Andrei Warkentin , "devel@edk2.groups.io" Cc: "pete@akeo.ie" , "ardb+tianocore@kernel.org" , "Sunny.Wang@arm.com" , "samer.el-haj-mahmoud@arm.com" , =?UTF-8?Q?Ren=c3=a9_Treffer?= References: <20210805163551.488035-1-jeremy.linton@arm.com> <20210805163551.488035-5-jeremy.linton@arm.com> From: "Jeremy Linton" Message-ID: <5ba43be0-be84-e2fa-f7c7-353e870435c7@arm.com> Date: Fri, 6 Aug 2021 16:52:56 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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). >=20 > I still think that (in general) would be a good idea - if not for the ben= efit 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()=20 code to be replaced by the SMC call. Which I will do, but I think its=20 better to fix to this one and make that a separate patch-set ideally=20 with another platform in parallel. >=20 > Reviewed-by: Andrei Warkentin >=20 > -- > Andrei Warkentin, > Arm Enablement Architect, > Cloud Platform Business Unit, VMware > ________________________________ > From: Andrei Warkentin > Sent: Friday, August 6, 2021 7:02 PM > To: devel@edk2.groups.io ; jeremy.linton@arm.com > Cc: pete@akeo.ie ; ardb+tianocore@kernel.org ; Sunny.Wang@arm.com ; samer.el-haj-mahmou= d@arm.com ; Ren=E9 Treffer > Subject: Re: [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCI= e for CM4 >=20 > Hi Jeremy, >=20 > Is any of this still conceptually necessary if we adopt the SMCCC interfa= ce within UEFI? >=20 > Instead of assuming the first downstream bus is bus 1, could you read the= secondary BN from the RP? >=20 > -- > Andrei Warkentin, > Arm Enablement Architect, > Cloud Platform Business Unit, VMware > ________________________________ > From: devel@edk2.groups.io on behalf of Jeremy Lin= ton via groups.io > Sent: Thursday, August 5, 2021 7:35 PM > To: devel@edk2.groups.io > Cc: pete@akeo.ie ; ardb+tianocore@kernel.org ; Andrei Warkentin ; Sunny.Wang@arm.com= ; samer.el-haj-mahmoud@arm.com ; Jeremy Linton ; Ren=E9 Treffer > Subject: [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe fo= r CM4 >=20 > 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. >=20 > Suggested-by: Ren=E9 Treffer > Signed-off-by: Jeremy Linton > --- > .../Bcm2711PciHostBridgeLibConstructor.c | 5 ----- > .../Library/Bcm2711PciSegmentLib/PciSegmentLib.c | 24 +++++++++++++++= ------- > 2 files changed, 17 insertions(+), 12 deletions(-) >=20 > diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm= 2711PciHostBridgeLibConstructor.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm271= 1PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c > index 8587d2d36d..4d4c584726 100644 > --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711Pci= HostBridgeLibConstructor.c > +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711Pci= HostBridgeLibConstructor.c > @@ -204,11 +204,6 @@ Bcm2711PciHostBridgeLibConstructor ( > } while (((Data & 0x30) !=3D 0x030) && (Timeout)); > DEBUG ((DEBUG_VERBOSE, "PCIe link ready (status=3D%x) Timeout=3D%d\n"= , Data, Timeout)); >=20 > - if ((Data & 0x30) !=3D 0x30) { > - DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=3D%x)\n", Data)); > - return EFI_DEVICE_ERROR; > - } > - > if ((Data & 0x80) !=3D 0x80) { > DEBUG ((DEBUG_ERROR, "PCIe link not in RC mode (status=3D%x)\n", Da= ta)); > return EFI_UNSUPPORTED; > diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSeg= mentLib.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmen= tLib.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; >=20 > Base =3D PCIE_REG_BASE; > Offset =3D Address & 0xFFF; /* Pick off the 4k register offse= t */ > @@ -89,17 +91,25 @@ PciSegmentLibGetConfigBase ( > Base +=3D PCIE_EXT_CFG_DATA; > if (mPciSegmentLastAccess !=3D Address) { > Dev =3D EFI_PCI_ADDR_DEV (Address); > + Bus =3D 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 =3D Address; > - } else { > - mPciSegmentLastAccess =3D 0; > + if (Dev > 0 && (Bus < 2)) { > return 0xFFFFFFFF; > } > + > + /* Don't probe slots if the link is down */ > + Data =3D MmioRead32 (PCIE_REG_BASE + PCIE_MISC_PCIE_STATUS); > + if ((Data & 0x30) !=3D 0x30) { > + DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=3D%x)\n", Da= ta)); > + return 0xFFFFFFFF; > + } > + > + MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address); > + mPciSegmentLastAccess =3D Address; > } > } > return Base + Offset; > -- > 2.13.7 >=20 >=20 >=20 >=20 >=20 >=20 >=20