| Message ID | 20190723124247.47262-2-Zhiqiang.Hou@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Prabhakar Kushwaha |
| Headers | show |
| Series | dm: pcie_fsl: Fix some issues | expand |
On Tue, Jul 23, 2019 at 8:53 PM Hou Zhiqiang <Zhiqiang.Hou@nxp.com> wrote: > > 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: > > #define CONFIG_SYS_FSL_CORENET_SERDES_ADDR \ > (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_CORENET_SERDES_OFFSET) > > This is valid on P4080, but on non-corenet platforms, such > as MPC8548, there is not definition of > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET, then on these 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) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > drivers/pci/pcie_fsl.c | 2 +- > drivers/pci/pcie_fsl.h | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c > index bfb207e..999e9c9 100644 > --- a/drivers/pci/pcie_fsl.c > +++ b/drivers/pci/pcie_fsl.c > @@ -445,7 +445,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 ce2b1af..e09099b 100644 > --- a/drivers/pci/pcie_fsl.h > +++ b/drivers/pci/pcie_fsl.h > @@ -41,6 +41,12 @@ > #define LTSSM_L0_REV3 0x11 > #define LTSSM_L0 0x16 > > +#ifdef ARCH_P4080 > +#define P4080_SERDES_ADDR CONFIG_SYS_FSL_CORENET_SERDES_ADDR > +#else > +#define P4080_SERDES_ADDR 0 So for non-P4080 platform, we are accessing address at zero? Regards, Bin
Hi Bin, Sorry for my delay respond, and thanks for your comments! > -----Original Message----- > From: Bin Meng [mailto:bmeng.cn@gmail.com] > Sent: 2019年7月23日 21:04 > To: Z.q. Hou <zhiqiang.hou@nxp.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com> > Subject: Re: [U-Boot] [PATCH 1/3] dm: pcie_fsl: Fix workaround of P4080 > erratum A003 > > On Tue, Jul 23, 2019 at 8:53 PM Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > wrote: > > > > 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: > > > > #define CONFIG_SYS_FSL_CORENET_SERDES_ADDR \ > > (CONFIG_SYS_IMMR + > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET) > > > > This is valid on P4080, but on non-corenet platforms, such as MPC8548, > > there is not definition of CONFIG_SYS_FSL_CORENET_SERDES_OFFSET, then > > on these 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) > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > --- > > drivers/pci/pcie_fsl.c | 2 +- > > drivers/pci/pcie_fsl.h | 6 ++++++ > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c index > > bfb207e..999e9c9 100644 > > --- a/drivers/pci/pcie_fsl.c > > +++ b/drivers/pci/pcie_fsl.c > > @@ -445,7 +445,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 > > ce2b1af..e09099b 100644 > > --- a/drivers/pci/pcie_fsl.h > > +++ b/drivers/pci/pcie_fsl.h > > @@ -41,6 +41,12 @@ > > #define LTSSM_L0_REV3 0x11 > > #define LTSSM_L0 0x16 > > > > +#ifdef ARCH_P4080 > > +#define P4080_SERDES_ADDR > CONFIG_SYS_FSL_CORENET_SERDES_ADDR > > +#else > > +#define P4080_SERDES_ADDR 0 > > So for non-P4080 platform, we are accessing address at zero? This macro is only used in the workaround of P4080 Erratum A003, so for non-P4080, it won't run into this workaround. Thanks, Zhiqiang > > Regards, > Bin
On Fri, Aug 23, 2019 at 10:43 AM Z.q. Hou <zhiqiang.hou@nxp.com> wrote: > > Hi Bin, > > Sorry for my delay respond, and thanks for your comments! > > > -----Original Message----- > > From: Bin Meng [mailto:bmeng.cn@gmail.com] > > Sent: 2019年7月23日 21:04 > > To: Z.q. Hou <zhiqiang.hou@nxp.com> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Prabhakar Kushwaha > > <prabhakar.kushwaha@nxp.com> > > Subject: Re: [U-Boot] [PATCH 1/3] dm: pcie_fsl: Fix workaround of P4080 > > erratum A003 > > > > On Tue, Jul 23, 2019 at 8:53 PM Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > wrote: > > > > > > 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: > > > > > > #define CONFIG_SYS_FSL_CORENET_SERDES_ADDR \ > > > (CONFIG_SYS_IMMR + > > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET) > > > > > > This is valid on P4080, but on non-corenet platforms, such as MPC8548, > > > there is not definition of CONFIG_SYS_FSL_CORENET_SERDES_OFFSET, then > > > on these 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) > > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > --- > > > drivers/pci/pcie_fsl.c | 2 +- > > > drivers/pci/pcie_fsl.h | 6 ++++++ > > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c index > > > bfb207e..999e9c9 100644 > > > --- a/drivers/pci/pcie_fsl.c > > > +++ b/drivers/pci/pcie_fsl.c > > > @@ -445,7 +445,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 > > > ce2b1af..e09099b 100644 > > > --- a/drivers/pci/pcie_fsl.h > > > +++ b/drivers/pci/pcie_fsl.h > > > @@ -41,6 +41,12 @@ > > > #define LTSSM_L0_REV3 0x11 > > > #define LTSSM_L0 0x16 > > > > > > +#ifdef ARCH_P4080 > > > +#define P4080_SERDES_ADDR > > CONFIG_SYS_FSL_CORENET_SERDES_ADDR > > > +#else > > > +#define P4080_SERDES_ADDR 0 > > > > So for non-P4080 platform, we are accessing address at zero? > > This macro is only used in the workaround of P4080 Erratum A003, so for non-P4080, it won't run into this workaround. > Still don't get it. Then why not use #ifdef P4080_Erratum_A003 #endif ? Regards, Bin
Hi Bin, Thanks a lot for your comments! > -----Original Message----- > From: Bin Meng [mailto:bmeng.cn@gmail.com] > Sent: 2019年8月23日 11:15 > To: Z.q. Hou <zhiqiang.hou@nxp.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com> > Subject: Re: [U-Boot] [PATCH 1/3] dm: pcie_fsl: Fix workaround of P4080 > erratum A003 > > On Fri, Aug 23, 2019 at 10:43 AM Z.q. Hou <zhiqiang.hou@nxp.com> wrote: > > > > Hi Bin, > > > > Sorry for my delay respond, and thanks for your comments! > > > > > -----Original Message----- > > > From: Bin Meng [mailto:bmeng.cn@gmail.com] > > > Sent: 2019年7月23日 21:04 > > > To: Z.q. Hou <zhiqiang.hou@nxp.com> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Prabhakar Kushwaha > > > <prabhakar.kushwaha@nxp.com> > > > Subject: Re: [U-Boot] [PATCH 1/3] dm: pcie_fsl: Fix workaround of > > > P4080 erratum A003 > > > > > > On Tue, Jul 23, 2019 at 8:53 PM Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > wrote: > > > > > > > > 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: > > > > > > > > #define CONFIG_SYS_FSL_CORENET_SERDES_ADDR \ > > > > (CONFIG_SYS_IMMR + > > > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET) > > > > > > > > This is valid on P4080, but on non-corenet platforms, such as > > > > MPC8548, there is not definition of > > > > CONFIG_SYS_FSL_CORENET_SERDES_OFFSET, then on these 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) > > > > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > --- > > > > drivers/pci/pcie_fsl.c | 2 +- > > > > drivers/pci/pcie_fsl.h | 6 ++++++ > > > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c index > > > > bfb207e..999e9c9 100644 > > > > --- a/drivers/pci/pcie_fsl.c > > > > +++ b/drivers/pci/pcie_fsl.c > > > > @@ -445,7 +445,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 > > > > ce2b1af..e09099b 100644 > > > > --- a/drivers/pci/pcie_fsl.h > > > > +++ b/drivers/pci/pcie_fsl.h > > > > @@ -41,6 +41,12 @@ > > > > #define LTSSM_L0_REV3 0x11 > > > > #define LTSSM_L0 0x16 > > > > > > > > +#ifdef ARCH_P4080 > > > > +#define P4080_SERDES_ADDR > > > CONFIG_SYS_FSL_CORENET_SERDES_ADDR > > > > +#else > > > > +#define P4080_SERDES_ADDR 0 > > > > > > So for non-P4080 platform, we are accessing address at zero? > > > > This macro is only used in the workaround of P4080 Erratum A003, so for > non-P4080, it won't run into this workaround. > > > > Still don't get it. Then why not use #ifdef P4080_Erratum_A003 #endif ? Make sense, will change in v2. Thanks, Zhiqiang > > Regards, > Bin
diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c index bfb207e..999e9c9 100644 --- a/drivers/pci/pcie_fsl.c +++ b/drivers/pci/pcie_fsl.c @@ -445,7 +445,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 ce2b1af..e09099b 100644 --- a/drivers/pci/pcie_fsl.h +++ b/drivers/pci/pcie_fsl.h @@ -41,6 +41,12 @@ #define LTSSM_L0_REV3 0x11 #define LTSSM_L0 0x16 +#ifdef ARCH_P4080 +#define P4080_SERDES_ADDR CONFIG_SYS_FSL_CORENET_SERDES_ADDR +#else +#define P4080_SERDES_ADDR 0 +#endif + struct fsl_pcie { int idx; struct udevice *bus;
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: #define CONFIG_SYS_FSL_CORENET_SERDES_ADDR \ (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_CORENET_SERDES_OFFSET) This is valid on P4080, but on non-corenet platforms, such as MPC8548, there is not definition of CONFIG_SYS_FSL_CORENET_SERDES_OFFSET, then on these 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) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> --- drivers/pci/pcie_fsl.c | 2 +- drivers/pci/pcie_fsl.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)