Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe related PCDs

["Leif Lindholm" <leif@nuviainc.com>]

On Thu, Jun 18, 2020 at 10:33:05 +0000, Wasim Khan (OSS) wrote:
> 
> 
> > -----Original Message-----
> > From: Leif Lindholm <leif@nuviainc.com>
> > Sent: Thursday, June 18, 2020 3:38 PM
> > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > Cc: devel@edk2.groups.io; Meenakshi Aggarwal
> > <meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> > ard.biesheuvel@arm.com; Wasim Khan <wasim.khan@nxp.com>
> > Subject: Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe
> > related PCDs
> > 
> > On Wed, Jun 10, 2020 at 03:47:25 +0530, Wasim Khan wrote:
> > > From: Wasim Khan <wasim.khan@nxp.com>
> > >
> > > Define PCIe related PCDs for LX2160A.
> > >
> > > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > > ---
> > >  Silicon/NXP/LX2160A/LX2160A.dsc.inc | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > index fe8ed402fc4e..43e361464c8e 100644
> > > --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > @@ -38,6 +38,11 @@ [PcdsFixedAtBuild.common]
> > >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C0000
> > >
> > > +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x8000000000
> > 
> > This is already provided by LX2160A_PCI1_PHYS_ADDRESS in
> > Silicon/NXP/LX2160A/Include/Soc.h, and PCI_SEG0_MMIO_MEMBASE would be
> > better described as an alias of that. Unless the NXP
> > PciHostBridgeLib/PciSegmentLib is intended to be shared with SoCs where these
> > base addresses can be different in different platforms.
> 
> Yes, PciHostBridgeLib/PciSegmentLib are shared with different SoC
> with different base address and number of PCIe controllers.

Yes. What I meant was whether the base address could change for the
SoC depending on which platform it is integrated in?

> > If so, the PHYS_ADDRESSES would be better defined as derivatives of
> > PcdPciExp1BaseAddr.
> 
> OK, I will use PcdPciExp1BaseAddr and PcdNumPciController for PHYS_ADDRESSES
> 
> > 
> > > +  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|6
> > 
> > Is this number possible to be different for different platforms based on the same
> > SoC?
> 
> This is the total number of PCIe controller on a SoC (for LS1043 its
> 3, for LX2160 its 6)
> A platform may use few of them based on the RCW (For Ex:
> LX2160aRdbPkg has only PEX3 and PEX5 . LX2160aQdsPkg may have all 6
> controller enabled).

OK, then the use of the Pcd is definitely the right way to go, but
yes, please do as you suggest above to avoid multiple (and potentially
conflicting) definitions for the same address.

Regards,

Leif