<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> Ok, I misunderstood the patch set (I thought the PciHostBridgeLib itself would eventually move to DEN0115).</div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> 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...</div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> Reviewed-by: Andrei Warkentin <awarkentin@vmware.com></div> <div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div id="Signature"> <div> <div></div> <div name="divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> --</div> <div name="divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> Andrei Warkentin,</div> <div name="divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> Arm Enablement Architect,</div> <div name="divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> Cloud Platform Business Unit, VMware</div> </div> </div> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Andrei Warkentin <awarkentin@vmware.com><br> <b>Sent:</b> Friday, August 6, 2021 7:02 PM<br> <b>To:</b> devel@edk2.groups.io <devel@edk2.groups.io>; jeremy.linton@arm.com <jeremy.linton@arm.com><br> <b>Cc:</b> pete@akeo.ie <pete@akeo.ie>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Sunny.Wang@arm.com <Sunny.Wang@arm.com>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; René Treffer <treffer+groups.io@measite.de><br> <b>Subject:</b> Re: [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for CM4</font> <div> </div> </div> <style type="text/css" style="display:none">  </style> <div dir="ltr"> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> Hi Jeremy,</div> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> Is any of this still conceptually necessary if we adopt the SMCCC interface within UEFI?</div> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> Instead of assuming the first downstream bus is bus 1, could you read the secondary BN from the RP?</div> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> </div> <div> <div style="font-family:Consolas,Courier,monospace; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div id="x_Signature"> <div> <div></div> <div name="x_divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> --</div> <div name="x_divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> Andrei Warkentin,</div> <div name="x_divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> Arm Enablement Architect,</div> <div name="x_divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0"> Cloud Platform Business Unit, VMware</div> </div> </div> </div> <div id="x_appendonsend"></div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Jeremy Linton via groups.io <jeremy.linton=arm.com@groups.io><br> <b>Sent:</b> Thursday, August 5, 2021 7:35 PM<br> <b>To:</b> devel@edk2.groups.io <devel@edk2.groups.io><br> <b>Cc:</b> 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>; René Treffer <treffer+groups.io@measite.de><br> <b>Subject:</b> [edk2-devel] [PATCH 4/5] Silicon/Broadcom/Bcm27xx: Tweak PCIe for CM4</font> <div> </div> </div> <div class="x_BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="x_PlainText">The CM4 has an actual pcie slot, so we need to move the linkup<br> check to the configuration probe logic. Further the device<br> restriction logic needs to be relaxed to support downstream<br> PCIe switches.<br> <br> Suggested-by: René Treffer <treffer+groups.io@measite.de><br> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com><br> ---<br> .../Bcm2711PciHostBridgeLibConstructor.c | 5 -----<br> .../Library/Bcm2711PciSegmentLib/PciSegmentLib.c | 24 +++++++++++++++-------<br> 2 files changed, 17 insertions(+), 12 deletions(-)<br> <br> diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c<br> index 8587d2d36d..4d4c584726 100644<br> --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c<br> +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c<br> @@ -204,11 +204,6 @@ Bcm2711PciHostBridgeLibConstructor (<br> } while (((Data & 0x30) != 0x030) && (Timeout));<br> DEBUG ((DEBUG_VERBOSE, "PCIe link ready (status=%x) Timeout=%d\n", Data, Timeout));<br> <br> - if ((Data & 0x30) != 0x30) {<br> - DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=%x)\n", Data));<br> - return EFI_DEVICE_ERROR;<br> - }<br> -<br> if ((Data & 0x80) != 0x80) {<br> DEBUG ((DEBUG_ERROR, "PCIe link not in RC mode (status=%x)\n", Data));<br> return EFI_UNSUPPORTED;<br> diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c<br> index 44ce3b4b99..3ccc131eab 100644<br> --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c<br> +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c<br> @@ -78,6 +78,8 @@ PciSegmentLibGetConfigBase (<br> UINT64 Base;<br> UINT64 Offset;<br> UINT32 Dev;<br> + UINT32 Bus;<br> + UINT32 Data;<br> <br> Base = PCIE_REG_BASE;<br> Offset = Address & 0xFFF; /* Pick off the 4k register offset */<br> @@ -89,17 +91,25 @@ PciSegmentLibGetConfigBase (<br> Base += PCIE_EXT_CFG_DATA;<br> if (mPciSegmentLastAccess != Address) {<br> Dev = EFI_PCI_ADDR_DEV (Address);<br> + Bus = EFI_PCI_ADDR_BUS (Address);<br> + <br> /*<br> - * Scan things out directly rather than translating the "bus" to a device, etc..<br> - * only we need to limit each bus to a single device.<br> + * There can only be a single device on bus 1 (downstream of root).<br> + * Subsequent busses (behind a PCIe switch) can have more.<br> */<br> - if (Dev < 1) {<br> - MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address);<br> - mPciSegmentLastAccess = Address;<br> - } else {<br> - mPciSegmentLastAccess = 0;<br> + if (Dev > 0 && (Bus < 2)) {<br> return 0xFFFFFFFF;<br> }<br> +<br> + /* Don't probe slots if the link is down */<br> + Data = MmioRead32 (PCIE_REG_BASE + PCIE_MISC_PCIE_STATUS);<br> + if ((Data & 0x30) != 0x30) {<br> + DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=%x)\n", Data));<br> + return 0xFFFFFFFF;<br> + }<br> +<br> + MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address);<br> + mPciSegmentLastAccess = Address;<br> }<br> }<br> return Base + Offset;<br> -- <br> 2.13.7<br> <br> <br> <br> <br> <br> <br> </div> </span></font></div> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links:<p> You receive all messages sent to this group. <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/78815">View/Reply Online (#78815)</a> | | <a target="_blank" href="https://groups.io/mt/84688700/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>