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

Message ID 20190825154423.30781-2-Zhiqiang.Hou@nxp.com
State Superseded
Delegated to: Prabhakar Kushwaha
Headers show
Series
  • dm: pcie_fsl: Fix some issues
Related show

Commit Message

Z.q. Hou Aug. 25, 2019, 3:42 p.m. UTC
From: Hou Zhiqiang <Zhiqiang.Hou@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,
the following build error will come up:

drivers/pci/pcie_fsl.c: In function 'fsl_pcie_init_port':
./arch/powerpc/include/asm/immap_85xx.h:3000:21: error:
'CONFIG_SYS_FSL_CORENET_SERDES_OFFSET' undeclared (first use
in this function); did you mean 'CONFIG_SYS_FSL_CORENET_SERDES_ADDR'?
  (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_CORENET_SERDES_OFFSET)
	                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix this build error by replacing it with a new added macro for
SerDes address of P4080.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V2:
 - Replaced CONFIG_SYS_FSL_CORENET_SERDES_ADDR with the CCSR base + 
   P4080 SerDes offset.
 - Reworded the change log slightly.

 drivers/pci/pcie_fsl.c | 2 +-
 drivers/pci/pcie_fsl.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Bin Meng Aug. 26, 2019, 5:58 a.m. UTC | #1
Hi Zhiqiang,

On Sun, Aug 25, 2019 at 11:42 PM Z.q. Hou <zhiqiang.hou@nxp.com> wrote:
>
> From: Hou Zhiqiang <Zhiqiang.Hou@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 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. Replacing the macro name to P4080_SERDES_ADDR does not
help anything.

> drivers/pci/pcie_fsl.c: In function 'fsl_pcie_init_port':
> ./arch/powerpc/include/asm/immap_85xx.h:3000:21: error:
> 'CONFIG_SYS_FSL_CORENET_SERDES_OFFSET' undeclared (first use
> in this function); did you mean 'CONFIG_SYS_FSL_CORENET_SERDES_ADDR'?
>   (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_CORENET_SERDES_OFFSET)
>                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Fix this build error by replacing it with a new added macro for
> SerDes address of P4080.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> V2:
>  - Replaced CONFIG_SYS_FSL_CORENET_SERDES_ADDR with the CCSR base +
>    P4080 SerDes offset.
>  - Reworded the change log slightly.
>
>  drivers/pci/pcie_fsl.c | 2 +-
>  drivers/pci/pcie_fsl.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>

Regards,
Bin
Z.q. Hou Aug. 26, 2019, 8:34 a.m. UTC | #2
Hi Bin,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: 2019年8月26日 13:59
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> <prabhakar.kushwaha@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@nxp.com> wrote:
> >
> > From: Hou Zhiqiang <Zhiqiang.Hou@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.
http://patchwork.ozlabs.org/project/uboot/list/?series=120966

> 
> > 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.

Thanks,
Zhiqiang

> 
> > drivers/pci/pcie_fsl.c: In function 'fsl_pcie_init_port':
> > ./arch/powerpc/include/asm/immap_85xx.h:3000:21: error:
> > 'CONFIG_SYS_FSL_CORENET_SERDES_OFFSET' undeclared (first use in this
> > function); did you mean 'CONFIG_SYS_FSL_CORENET_SERDES_ADDR'?
> >   (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_CORENET_SERDES_OFFSET)
> >
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > Fix this build error by replacing it with a new added macro for SerDes
> > address of P4080.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> > V2:
> >  - Replaced CONFIG_SYS_FSL_CORENET_SERDES_ADDR with the CCSR
> base +
> >    P4080 SerDes offset.
> >  - Reworded the change log slightly.
> >
> >  drivers/pci/pcie_fsl.c | 2 +-
> >  drivers/pci/pcie_fsl.h | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> 
> Regards,
> Bin
Bin Meng Aug. 26, 2019, 8:50 a.m. UTC | #3
Hi Zhiqiang,

On Mon, Aug 26, 2019 at 4:34 PM Z.q. Hou <zhiqiang.hou@nxp.com> wrote:
>
> Hi Bin,
>
> Thanks a lot for your comments!
>
> > -----Original Message-----
> > From: Bin Meng <bmeng.cn@gmail.com>
> > Sent: 2019年8月26日 13:59
> > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > <prabhakar.kushwaha@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@nxp.com> wrote:
> > >
> > > From: Hou Zhiqiang <Zhiqiang.Hou@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.
> http://patchwork.ozlabs.org/project/uboot/list/?series=120966

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?

Regards,
Bin
Prabhakar Kushwaha Aug. 26, 2019, 9:10 a.m. UTC | #4
Dear Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Monday, August 26, 2019 2:21 PM
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> <prabhakar.kushwaha@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@nxp.com> wrote:
> >
> > Hi Bin,
> >
> > Thanks a lot for your comments!
> >
> > > -----Original Message-----
> > > From: Bin Meng <bmeng.cn@gmail.com>
> > > Sent: 2019年8月26日 13:59
> > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > <prabhakar.kushwaha@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@nxp.com> wrote:
> > > >
> > > > From: Hou Zhiqiang <Zhiqiang.Hou@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;dat
> a
> >
> =02%7C01%7Cprabhakar.kushwaha%40nxp.com%7C927d704c60734c63fb7708
> d72a02
> >
> 7cd0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63702406248371
> 8776&a
> >
> mp;sdata=HLHA1%2FXmSxPPz%2FOF%2BWu30kQDD0xGsWOhxyHBEuMs8hw%
> 3D&amp;rese
> > rved=0
> 
> Is this patch series merged? Or still in the review queue. I would like to have a
> look.
> 

This patch series has not been merged. I am in process of integrating it. 

powerpc: Enable PCIe DM drvier for some platforms: http://patchwork.ozlabs.org/project/uboot/list/?series=120966

If you have feedback. Please do share. 

I can wait to send in in rc4 or rc5. 

--pk
Z.q. Hou Aug. 26, 2019, 10:17 a.m. UTC | #5
Hi Bin,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: 2019年8月26日 16:51
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> <prabhakar.kushwaha@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@nxp.com> wrote:
> >
> > Hi Bin,
> >
> > Thanks a lot for your comments!
> >
> > > -----Original Message-----
> > > From: Bin Meng <bmeng.cn@gmail.com>
> > > Sent: 2019年8月26日 13:59
> > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > <prabhakar.kushwaha@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@nxp.com>
> wrote:
> > > >
> > > > From: Hou Zhiqiang <Zhiqiang.Hou@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&amp;sda
> >
> ta=6M7wj0KNNxL6TNaP9gQABcFJUb8gvV%2BfOhzY9sOswck%3D&amp;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
Bin Meng Aug. 26, 2019, 12:27 p.m. UTC | #6
Hi Zhiqiang,

On Mon, Aug 26, 2019 at 6:17 PM Z.q. Hou <zhiqiang.hou@nxp.com> wrote:
>
> Hi Bin,
>
> Thanks a lot for your comments!
>
> > -----Original Message-----
> > From: Bin Meng <bmeng.cn@gmail.com>
> > Sent: 2019年8月26日 16:51
> > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > <prabhakar.kushwaha@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@nxp.com> wrote:
> > >
> > > Hi Bin,
> > >
> > > Thanks a lot for your comments!
> > >
> > > > -----Original Message-----
> > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > Sent: 2019年8月26日 13:59
> > > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > > <prabhakar.kushwaha@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@nxp.com>
> > wrote:
> > > > >
> > > > > From: Hou Zhiqiang <Zhiqiang.Hou@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&amp;sda
> > >
> > ta=6M7wj0KNNxL6TNaP9gQABcFJUb8gvV%2BfOhzY9sOswck%3D&amp;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.

Ah, I see. Thanks for the info.

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

Looks the IS_ENABLED() really does no better than #ifdef, so let's
switch all IS_ENABLED in this routine to #ifdef.

Regards,
Bin
Bin Meng Aug. 26, 2019, 1 p.m. UTC | #7
Hi Prabhakar,

On Mon, Aug 26, 2019 at 5:10 PM Prabhakar Kushwaha
<prabhakar.kushwaha@nxp.com> wrote:
>
> Dear Bin,
>
> > -----Original Message-----
> > From: Bin Meng <bmeng.cn@gmail.com>
> > Sent: Monday, August 26, 2019 2:21 PM
> > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > <prabhakar.kushwaha@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@nxp.com> wrote:
> > >
> > > Hi Bin,
> > >
> > > Thanks a lot for your comments!
> > >
> > > > -----Original Message-----
> > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > Sent: 2019年8月26日 13:59
> > > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > > <prabhakar.kushwaha@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@nxp.com> wrote:
> > > > >
> > > > > From: Hou Zhiqiang <Zhiqiang.Hou@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;dat
> > a
> > >
> > =02%7C01%7Cprabhakar.kushwaha%40nxp.com%7C927d704c60734c63fb7708
> > d72a02
> > >
> > 7cd0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63702406248371
> > 8776&a
> > >
> > mp;sdata=HLHA1%2FXmSxPPz%2FOF%2BWu30kQDD0xGsWOhxyHBEuMs8hw%
> > 3D&amp;rese
> > > rved=0
> >
> > Is this patch series merged? Or still in the review queue. I would like to have a
> > look.
> >
>
> This patch series has not been merged. I am in process of integrating it.
>
> powerpc: Enable PCIe DM drvier for some platforms: http://patchwork.ozlabs.org/project/uboot/list/?series=120966
>
> If you have feedback. Please do share.
>
> I can wait to send in in rc4 or rc5.

Thanks for letting me know the patch status. I will take a look soon.

Regards,
Bin
Bin Meng Aug. 27, 2019, 5:01 a.m. UTC | #8
Hi Prabhakar,

On Mon, Aug 26, 2019 at 9:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Prabhakar,
>
> On Mon, Aug 26, 2019 at 5:10 PM Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com> wrote:
> >
> > Dear Bin,
> >
> > > -----Original Message-----
> > > From: Bin Meng <bmeng.cn@gmail.com>
> > > Sent: Monday, August 26, 2019 2:21 PM
> > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > <prabhakar.kushwaha@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@nxp.com> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > Thanks a lot for your comments!
> > > >
> > > > > -----Original Message-----
> > > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > > Sent: 2019年8月26日 13:59
> > > > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > > > <prabhakar.kushwaha@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@nxp.com> wrote:
> > > > > >
> > > > > > From: Hou Zhiqiang <Zhiqiang.Hou@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;dat
> > > a
> > > >
> > > =02%7C01%7Cprabhakar.kushwaha%40nxp.com%7C927d704c60734c63fb7708
> > > d72a02
> > > >
> > > 7cd0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63702406248371
> > > 8776&a
> > > >
> > > mp;sdata=HLHA1%2FXmSxPPz%2FOF%2BWu30kQDD0xGsWOhxyHBEuMs8hw%
> > > 3D&amp;rese
> > > > rved=0
> > >
> > > Is this patch series merged? Or still in the review queue. I would like to have a
> > > look.
> > >
> >
> > This patch series has not been merged. I am in process of integrating it.
> >
> > powerpc: Enable PCIe DM drvier for some platforms: http://patchwork.ozlabs.org/project/uboot/list/?series=120966
> >
> > If you have feedback. Please do share.
> >
> > I can wait to send in in rc4 or rc5.
>
> Thanks for letting me know the patch status. I will take a look soon.
>

Thanks to Zhiqiang's quick response to my review comments, now I have
finished the review for patch series
http://patchwork.ozlabs.org/project/uboot/list/?series=120966.

Regards,
Bin
Prabhakar Kushwaha Aug. 27, 2019, 6:54 a.m. UTC | #9
Thanks Bin

Let me integrate his patch-set.

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Tuesday, August 27, 2019 10:32 AM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> Cc: Z.q. Hou <zhiqiang.hou@nxp.com>; u-boot@lists.denx.de
> Subject: Re: [PATCHv2 1/3] dm: pcie_fsl: Fix workaround of P4080 erratum A003
> 
> Hi Prabhakar,
> 
> On Mon, Aug 26, 2019 at 9:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Prabhakar,
> >
> > On Mon, Aug 26, 2019 at 5:10 PM Prabhakar Kushwaha
> > <prabhakar.kushwaha@nxp.com> wrote:
> > >
> > > Dear Bin,
> > >
> > > > -----Original Message-----
> > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > Sent: Monday, August 26, 2019 2:21 PM
> > > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > > <prabhakar.kushwaha@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@nxp.com>
> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > Thanks a lot for your comments!
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > > > Sent: 2019年8月26日 13:59
> > > > > > To: Z.q. Hou <zhiqiang.hou@nxp.com>
> > > > > > Cc: u-boot@lists.denx.de; Prabhakar Kushwaha
> > > > > > <prabhakar.kushwaha@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@nxp.com>
> wrote:
> > > > > > >
> > > > > > > From: Hou Zhiqiang <Zhiqiang.Hou@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.
> > > > > http://patch
> > > > >
> > > >
> work.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D120966&amp;
> > > > dat
> > > > a
> > > > >
> > > >
> =02%7C01%7Cprabhakar.kushwaha%40nxp.com%7C927d704c60734c63fb7708
> > > > d72a02
> > > > >
> > > >
> 7cd0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63702406248371
> > > > 8776&a
> > > > >
> > > >
> mp;sdata=HLHA1%2FXmSxPPz%2FOF%2BWu30kQDD0xGsWOhxyHBEuMs8hw%
> > > > 3D&amp;rese
> > > > > rved=0
> > > >
> > > > Is this patch series merged? Or still in the review queue. I would
> > > > like to have a look.
> > > >
> > >
> > > This patch series has not been merged. I am in process of integrating it.
> > >
> > > powerpc: Enable PCIe DM drvier for some platforms:
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpat
> > >
> chwork.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D120966&amp;
> > >
> data=02%7C01%7Cprabhakar.kushwaha%40nxp.com%7C13a11a268f744379648
> 508
> > >
> d72aabb175%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63702478
> 9212
> > >
> 237371&amp;sdata=eC0rQReEX7PtVDTlHXmTl2TyHVh70gRYVy4dsBrl7fk%3D&a
> mp;
> > > reserved=0
> > >
> > > If you have feedback. Please do share.
> > >
> > > I can wait to send in in rc4 or rc5.
> >
> > Thanks for letting me know the patch status. I will take a look soon.
> >
> 
> Thanks to Zhiqiang's quick response to my review comments, now I have
> finished the review for patch series
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatchwor
> k.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D120966&amp;data=0
> 2%7C01%7Cprabhakar.kushwaha%40nxp.com%7C13a11a268f744379648508d7
> 2aabb175%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370247892
> 12237371&amp;sdata=eC0rQReEX7PtVDTlHXmTl2TyHVh70gRYVy4dsBrl7fk%3D
> &amp;reserved=0.
> 
> Regards,
> Bin

Patch
diff mbox series

diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c
index 4d61a46cef..29b50f2376 100644
--- a/drivers/pci/pcie_fsl.c
+++ b/drivers/pci/pcie_fsl.c
@@ -444,7 +444,7 @@  static int fsl_pcie_init_port(struct fsl_pcie *pcie)
 	    !fsl_pcie_link_up(pcie)) {
 		serdes_corenet_t *srds_regs;
 
-		srds_regs = (void *)CONFIG_SYS_FSL_CORENET_SERDES_ADDR;
+		srds_regs = (void *)P4080_SERDES_ADDR;
 		val_32 = in_be32(&srds_regs->srdspccr0);
 
 		if ((val_32 >> 28) == 3) {
diff --git a/drivers/pci/pcie_fsl.h b/drivers/pci/pcie_fsl.h
index 5eefc31fa9..35a740241e 100644
--- a/drivers/pci/pcie_fsl.h
+++ b/drivers/pci/pcie_fsl.h
@@ -40,6 +40,8 @@ 
 #define LTSSM_L0_REV3			0x11
 #define LTSSM_L0			0x16
 
+#define P4080_SERDES_ADDR		(CONFIG_SYS_IMMR + 0xEA000)
+
 struct fsl_pcie {
 	int idx;
 	struct udevice *bus;