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.web11.2503.1615612137116009782 for ; Fri, 12 Mar 2021 21:08:57 -0800 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 D73B0ED1; Fri, 12 Mar 2021 21:08:55 -0800 (PST) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8DE2D3F7D7; Fri, 12 Mar 2021 21:08:55 -0800 (PST) Subject: Re: [edk2-platforms][PATCH 1/1] Silicon/Broadcom/Bcm27xx: Allow more than one device on pcie busses >1 To: =?UTF-8?Q?Ren=c3=a9_Treffer?= , devel@edk2.groups.io Cc: Pete Batard , Leif Lindholm , Ard Biesheuvel , "Andrei Warkentin (awarkentin@vmware.com)" , Samer El-Haj-Mahmoud References: From: "Jeremy Linton" Message-ID: <74065f91-c57c-522f-055e-ff0a09463a17@arm.com> Date: Fri, 12 Mar 2021 23:08:55 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi, First, thanks for the patches, this really helps! On 3/12/21 12:32 PM, Ren=C3=A9 Treffer wrote: > There is only a single pcie port on the bcm2711 so limiting the number = of > devices to 1 worked as long as there is no way to add a pcie switch. >=20 > On the compute module 4 it is possible to add a pcie switch (tested wit= h > asm1184e) which adds 5 new pcie busses. >=20 > In the current state the pci enumeration fails for the pcie switch > internal bus (bus 2, device 1,3,5,7). The root port gets configured wit= h > subordniate=3D0x2, blocking e.g. a booting linux from discovering devic= es > behind the switch. >=20 > Devices behind the switch work after lifting the device limit on busses > other than 0 and 1. >=20 > Signed-off-by: Ren=C3=A9 Treffer > --- > =C2=A0.../Library/Bcm2711PciSegmentLib/PciSegmentLib.c=C2=A0=C2=A0 | 1= 4 +++++++------- > =C2=A01 file changed, 7 insertions(+), 7 deletions(-) >=20 > diff --git > a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c > b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c > index 44ce3b4b99..4af9374d23 100644 > --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentL= ib.c > +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentL= ib.c > @@ -78,6 +78,7 @@ PciSegmentLibGetConfigBase ( > =C2=A0=C2=A0 UINT64=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Base; > =C2=A0=C2=A0 UINT64=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Offset; > =C2=A0=C2=A0 UINT32=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Dev; > +=C2=A0 UINT32=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Bus; > =20 > =C2=A0=C2=A0 Base =3D PCIE_REG_BASE; > =C2=A0=C2=A0 Offset =3D Address & 0xFFF;=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 /* Pick off the 4k register offset */ > @@ -89,17 +90,16 @@ PciSegmentLibGetConfigBase ( > =C2=A0=C2=A0=C2=A0=C2=A0 Base +=3D PCIE_EXT_CFG_DATA; > =C2=A0=C2=A0=C2=A0=C2=A0 if (mPciSegmentLastAccess !=3D Address) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Dev =3D EFI_PCI_ADDR_DEV (Address= ); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Bus =3D EFI_PCI_ADDR_BUS (Address); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * Scan things out directly rather= than translating the "bus" to > a device, etc.. > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * only we need to limit each bus = to a single device. > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * There can only be a single devi= ce on bus 1 (downstream of root). > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * Subsequent busses (behind a PCI= e switch) could have more. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 */ > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (Dev < 1) { > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 MmioWrite32 (PC= IE_REG_BASE + PCIE_EXT_CFG_INDEX, Address); > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mPciSegmentLast= Access =3D Address; > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } else { > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mPciSegmentLast= Access =3D 0; > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (Dev > 0 && (Bus =3D=3D 1 || Bus =3D= =3D 0)) { Did you try this with another switch plugged in downstream the first one? This looks right, because presumably the first new switch is sending the=20 UR's properly for its subordinate buses. Dumping the segment last access reset is fine too. Although, this set has unicode whitespace characters, which presumably=20 Ard fixed up in the previous patch since I see it there too. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return 0x= FFFFFFFF; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_C= FG_INDEX, Address); > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mPciSegmentLastAccess =3D Address; > =C2=A0=C2=A0=C2=A0=C2=A0 } > =C2=A0=C2=A0 } > =C2=A0=C2=A0 return Base + Offset; >=20 So, Reviewed-by: Jeremy Linton Thanks again!