[U-Boot] [PATCHv2 1/3] dm: pcie_fsl: Fix workaround of P4080 erratum A003

[Z.q. Hou]

Hi Bin,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bin Meng <bmeng.cn at gmail.com>
> Sent: 2019年8月26日 16:51
> To: Z.q. Hou <zhiqiang.hou at nxp.com>
> Cc: u-boot at lists.denx.de; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>
> Subject: Re: [PATCHv2 1/3] dm: pcie_fsl: Fix workaround of P4080 erratum
> A003
> 
> Hi Zhiqiang,
> 
> On Mon, Aug 26, 2019 at 4:34 PM Z.q. Hou <zhiqiang.hou at nxp.com> wrote:
> >
> > Hi Bin,
> >
> > Thanks a lot for your comments!
> >
> > > -----Original Message-----
> > > From: Bin Meng <bmeng.cn at gmail.com>
> > > Sent: 2019年8月26日 13:59
> > > To: Z.q. Hou <zhiqiang.hou at nxp.com>
> > > Cc: u-boot at lists.denx.de; Prabhakar Kushwaha
> > > <prabhakar.kushwaha at nxp.com>
> > > Subject: Re: [PATCHv2 1/3] dm: pcie_fsl: Fix workaround of P4080
> > > erratum
> > > A003
> > >
> > > Hi Zhiqiang,
> > >
> > > On Sun, Aug 25, 2019 at 11:42 PM Z.q. Hou <zhiqiang.hou at nxp.com>
> wrote:
> > > >
> > > > From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > > >
> > > > In the workaround of P4080 erratum A003, it uses the macro
> > > > CONFIG_SYS_FSL_CORENET_SERDES_ADDR to get the SerDes block
> > > register
> > > > address, the CONFIG_SYS_FSL_CORENET_SERDES_ADDR is defined as
> > > > following:
> > > >
> > > >         (CONFIG_SYS_IMMR +
> > > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET)
> > > >
> > > > The problem is the macro CONFIG_SYS_FSL_CORENET_SERDES_ADDR
> is
> > > defined
> > > > on both corenet and non-corenet platforms (though it should be
> > > > defined only on corenet platforms), but the macro
> > > > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET is only defined on
> corenet
> > > > platforms, so when enabled this driver on non-corenet platforms,
> > >
> > > so when enabling
> >
> > The following series will enable DM PCIe on some PowerPC platforms
> > including MPC8548CDS, which isn't a CORENET platform.
> >
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> work.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D120966&amp
> ;data
> >
> =02%7C01%7Czhiqiang.hou%40nxp.com%7C927d704c60734c63fb7708d72a
> 027cd0%7
> >
> C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63702406248358884
> 7&sda
> >
> ta=6M7wj0KNNxL6TNaP9gQABcFJUb8gvV%2BfOhzY9sOswck%3D&rese
> rved=0
> 
> Is this patch series merged? Or still in the review queue. I would like to have
> a look.
> 
> >
> > >
> > > > the following build error will come up:
> > > >
> > >
> > > This patch still does not look correct to me.
> > >
> > > So far only ARCH_P4080 selects SYS_P4080_ERRATUM_PCIE_A003, so
> the
> > > CONFIG_SYS_FSL_CORENET_SERDES_ADDR needs to be only defined in
> the
> > > P4080 codes.
> >
> > The CONFIG_SYS_FSL_CORENET_SERDES_ADDR is a macro for SerDes
> registers
> > Address, it is not dedicated for workarounds, and the SerDes registers
> > address macro was defined on both CORENET and non-CORENET
> platforms.
> >
> > > Replacing the macro name to P4080_SERDES_ADDR does not help
> anything.
> >
> > As the macro CONFIG_SYS_IMMR is always defined on CORENET and
> > non-CORENET platforms, so replacing the macro
> > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET,
> > which is only defined on CORENET platforms, with the P4080 SerDes
> > registers address in constant number can resolve the build error on
> non-CORENET.
> 
> I don't understand. Unless SYS_P4080_ERRATUM_PCIE_A003 is turned on by
> other non-CORENET platforms.
> Could you please point to me which patch does this?

In function fsl_pcie_init_port() in drivers/pci/pcie_fsl.c, it checks this Errata in
run-time by "IS_ENABLED(CONFIG_SYS_P4080_ERRATUM_PCIE_A003)", so
it always precompile the lines of this workaround no matter whether the
SYS_P4080_ERRATUM_PCIE_A003 has been selected or not.

Do you think it is better to use "#ifdef ... #endif"?

Thanks,
Zhiqiang

> 
> Regards,
> Bin