| Message ID | 1476869802-30528-2-git-send-email-priyanka.jain@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
Hi York > -----Original Message----- > From: Priyanka Jain [mailto:priyanka.jain@nxp.com] > Sent: Wednesday, October 19, 2016 3:07 PM > To: u-boot@lists.denx.de > Cc: Priyanka Jain <priyanka.jain@nxp.com>; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com> > Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection > > Timer base address has been changed from LS2080A SoC to > new SoCs like LS2088A, LS1088A. > > Use SVR based timer base address detection to avoid compile time #ifdef. > > Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> > --- > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 14 +++++++++++++- > .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 ++- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > index b7a2e0c..ce04e48 100644 > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > @@ -424,8 +424,10 @@ int arch_early_init_r(void) > > int timer_init(void) > { > - u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; > + u32 __iomem *cntcr; > #ifdef CONFIG_FSL_LSCH3 > + struct ccsr_gur __iomem *gur = (void > *)(CONFIG_SYS_FSL_GUTS_ADDR); > + u32 svr, ver; > u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR; > #endif > #ifdef CONFIG_LS2080A > @@ -439,6 +441,16 @@ int timer_init(void) > #endif > > #ifdef CONFIG_FSL_LSCH3 > + svr = gur_in32(&gur->svr); > + ver = SVR_SOC_VER(svr); > + if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) || > + (ver == SVR_LS2085A) || (ver == SVR_LS2045A)) > + cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR; > + else > +#endif > + cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; > + > +#ifdef CONFIG_FSL_LSCH3 > /* Enable timebase for all clusters. > * It is safe to do so even some clusters are not enabled. > */ > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > index 7acba27..e6cdfcb 100644 > --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > @@ -23,7 +23,8 @@ > #define CONFIG_SYS_IFC_ADDR (CONFIG_SYS_IMMR + > 0x01240000) > #define CONFIG_SYS_NS16550_COM1 (CONFIG_SYS_IMMR + > 0x011C0500) > #define CONFIG_SYS_NS16550_COM2 (CONFIG_SYS_IMMR + > 0x011C0600) > -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d0000 > +#define LS2080A_LS2085A_TIMER_ADDR 0x023d0000 Is this ok to define new constant without CONFIG_? Only problem, it is not consistent with existing defines. -prabhakar
On 10/19/2016 10:28 PM, Prabhakar Kushwaha wrote: > Hi York > >> -----Original Message----- >> From: Priyanka Jain [mailto:priyanka.jain@nxp.com] >> Sent: Wednesday, October 19, 2016 3:07 PM >> To: u-boot@lists.denx.de >> Cc: Priyanka Jain <priyanka.jain@nxp.com>; Prabhakar Kushwaha >> <prabhakar.kushwaha@nxp.com> >> Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection >> >> Timer base address has been changed from LS2080A SoC to >> new SoCs like LS2088A, LS1088A. >> >> Use SVR based timer base address detection to avoid compile time #ifdef. >> >> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com> >> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> >> --- >> arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 14 +++++++++++++- >> .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 ++- >> 2 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >> index b7a2e0c..ce04e48 100644 >> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >> @@ -424,8 +424,10 @@ int arch_early_init_r(void) >> >> int timer_init(void) >> { >> - u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; >> + u32 __iomem *cntcr; >> #ifdef CONFIG_FSL_LSCH3 >> + struct ccsr_gur __iomem *gur = (void >> *)(CONFIG_SYS_FSL_GUTS_ADDR); >> + u32 svr, ver; >> u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR; >> #endif >> #ifdef CONFIG_LS2080A >> @@ -439,6 +441,16 @@ int timer_init(void) >> #endif >> >> #ifdef CONFIG_FSL_LSCH3 >> + svr = gur_in32(&gur->svr); >> + ver = SVR_SOC_VER(svr); >> + if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) || >> + (ver == SVR_LS2085A) || (ver == SVR_LS2045A)) >> + cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR; >> + else >> +#endif >> + cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; >> + >> +#ifdef CONFIG_FSL_LSCH3 >> /* Enable timebase for all clusters. >> * It is safe to do so even some clusters are not enabled. >> */ >> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >> index 7acba27..e6cdfcb 100644 >> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >> @@ -23,7 +23,8 @@ >> #define CONFIG_SYS_IFC_ADDR (CONFIG_SYS_IMMR + >> 0x01240000) >> #define CONFIG_SYS_NS16550_COM1 (CONFIG_SYS_IMMR + >> 0x011C0500) >> #define CONFIG_SYS_NS16550_COM2 (CONFIG_SYS_IMMR + >> 0x011C0600) >> -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d0000 >> +#define LS2080A_LS2085A_TIMER_ADDR 0x023d0000 > > Is this ok to define new constant without CONFIG_? > Only problem, it is not consistent with existing defines. > Prabhakar, I opinion on this is 1) Let's check if a Kconfig option is better for it If yes, let's convert it to Kconfig. If not, go to 2) 2) Let's use a different name space for the macros. I suggest to use SYS_FSL_* for fixed value for our hardware. York
> -----Original Message----- > From: york sun > Sent: Thursday, October 20, 2016 10:46 AM > To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; u- > boot@lists.denx.de > Cc: Priyanka Jain <priyanka.jain@nxp.com> > Subject: Re: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address > detection > > On 10/19/2016 10:28 PM, Prabhakar Kushwaha wrote: > > Hi York > > > >> -----Original Message----- > >> From: Priyanka Jain [mailto:priyanka.jain@nxp.com] > >> Sent: Wednesday, October 19, 2016 3:07 PM > >> To: u-boot@lists.denx.de > >> Cc: Priyanka Jain <priyanka.jain@nxp.com>; Prabhakar Kushwaha > >> <prabhakar.kushwaha@nxp.com> > >> Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address > >> detection > >> > >> Timer base address has been changed from LS2080A SoC to new SoCs like > >> LS2088A, LS1088A. > >> > >> Use SVR based timer base address detection to avoid compile time #ifdef. > >> > >> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com> > >> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> > >> --- > >> arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 14 +++++++++++++- > >> .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 ++- > >> 2 files changed, 15 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > >> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > >> index b7a2e0c..ce04e48 100644 > >> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > >> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > >> @@ -424,8 +424,10 @@ int arch_early_init_r(void) > >> > >> int timer_init(void) > >> { > >> - u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; > >> + u32 __iomem *cntcr; > >> #ifdef CONFIG_FSL_LSCH3 > >> + struct ccsr_gur __iomem *gur = (void > >> *)(CONFIG_SYS_FSL_GUTS_ADDR); > >> + u32 svr, ver; > >> u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR; > #endif > >> #ifdef CONFIG_LS2080A @@ -439,6 +441,16 @@ int timer_init(void) > >> #endif > >> > >> #ifdef CONFIG_FSL_LSCH3 > >> + svr = gur_in32(&gur->svr); > >> + ver = SVR_SOC_VER(svr); > >> + if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) || > >> + (ver == SVR_LS2085A) || (ver == SVR_LS2045A)) > >> + cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR; > >> + else > >> +#endif > >> + cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; > >> + > >> +#ifdef CONFIG_FSL_LSCH3 > >> /* Enable timebase for all clusters. > >> * It is safe to do so even some clusters are not enabled. > >> */ > >> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >> index 7acba27..e6cdfcb 100644 > >> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >> @@ -23,7 +23,8 @@ > >> #define CONFIG_SYS_IFC_ADDR (CONFIG_SYS_IMMR > + > >> 0x01240000) > >> #define CONFIG_SYS_NS16550_COM1 > (CONFIG_SYS_IMMR + > >> 0x011C0500) > >> #define CONFIG_SYS_NS16550_COM2 > (CONFIG_SYS_IMMR + > >> 0x011C0600) > >> -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d0000 > >> +#define LS2080A_LS2085A_TIMER_ADDR 0x023d0000 > > > > Is this ok to define new constant without CONFIG_? > > Only problem, it is not consistent with existing defines. > > > > Prabhakar, > > I opinion on this is > > 1) Let's check if a Kconfig option is better for it If yes, let's convert it to > Kconfig. If not, go to 2) > > 2) Let's use a different name space for the macros. > I suggest to use SYS_FSL_* for fixed value for our hardware. > > York LS2080A_LS2085A_TIMER_ADDR is defining the timer controller (register) offset. Kconfig option does not look to be very helpful here. I am thinking of renaming this to "SYS_FSL_LS2080A_LS2085A_TIMER_ADDR" Please confirm if this is fine. Priyanka
On 10/23/2016 11:29 PM, Priyanka Jain wrote: > > >> -----Original Message----- >> From: york sun >> Sent: Thursday, October 20, 2016 10:46 AM >> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; u- >> boot@lists.denx.de >> Cc: Priyanka Jain <priyanka.jain@nxp.com> >> Subject: Re: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address >> detection >> >> On 10/19/2016 10:28 PM, Prabhakar Kushwaha wrote: >>> Hi York >>> >>>> -----Original Message----- >>>> From: Priyanka Jain [mailto:priyanka.jain@nxp.com] >>>> Sent: Wednesday, October 19, 2016 3:07 PM >>>> To: u-boot@lists.denx.de >>>> Cc: Priyanka Jain <priyanka.jain@nxp.com>; Prabhakar Kushwaha >>>> <prabhakar.kushwaha@nxp.com> >>>> Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address >>>> detection >>>> >>>> Timer base address has been changed from LS2080A SoC to new SoCs like >>>> LS2088A, LS1088A. >>>> >>>> Use SVR based timer base address detection to avoid compile time #ifdef. >>>> >>>> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com> >>>> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> >>>> --- >>>> arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 14 +++++++++++++- >>>> .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 ++- >>>> 2 files changed, 15 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >>>> index b7a2e0c..ce04e48 100644 >>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c >>>> @@ -424,8 +424,10 @@ int arch_early_init_r(void) >>>> >>>> int timer_init(void) >>>> { >>>> - u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; >>>> + u32 __iomem *cntcr; >>>> #ifdef CONFIG_FSL_LSCH3 >>>> + struct ccsr_gur __iomem *gur = (void >>>> *)(CONFIG_SYS_FSL_GUTS_ADDR); >>>> + u32 svr, ver; >>>> u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR; >> #endif >>>> #ifdef CONFIG_LS2080A @@ -439,6 +441,16 @@ int timer_init(void) >>>> #endif >>>> >>>> #ifdef CONFIG_FSL_LSCH3 >>>> + svr = gur_in32(&gur->svr); >>>> + ver = SVR_SOC_VER(svr); >>>> + if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) || >>>> + (ver == SVR_LS2085A) || (ver == SVR_LS2045A)) >>>> + cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR; >>>> + else >>>> +#endif >>>> + cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; >>>> + >>>> +#ifdef CONFIG_FSL_LSCH3 >>>> /* Enable timebase for all clusters. >>>> * It is safe to do so even some clusters are not enabled. >>>> */ >>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >>>> index 7acba27..e6cdfcb 100644 >>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h >>>> @@ -23,7 +23,8 @@ >>>> #define CONFIG_SYS_IFC_ADDR (CONFIG_SYS_IMMR >> + >>>> 0x01240000) >>>> #define CONFIG_SYS_NS16550_COM1 >> (CONFIG_SYS_IMMR + >>>> 0x011C0500) >>>> #define CONFIG_SYS_NS16550_COM2 >> (CONFIG_SYS_IMMR + >>>> 0x011C0600) >>>> -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d0000 >>>> +#define LS2080A_LS2085A_TIMER_ADDR 0x023d0000 >>> >>> Is this ok to define new constant without CONFIG_? >>> Only problem, it is not consistent with existing defines. >>> >> >> Prabhakar, >> >> I opinion on this is >> >> 1) Let's check if a Kconfig option is better for it If yes, let's convert it to >> Kconfig. If not, go to 2) >> >> 2) Let's use a different name space for the macros. >> I suggest to use SYS_FSL_* for fixed value for our hardware. >> >> York > > LS2080A_LS2085A_TIMER_ADDR is defining the timer controller (register) offset. > Kconfig option does not look to be very helpful here. > I am thinking of renaming this to "SYS_FSL_LS2080A_LS2085A_TIMER_ADDR" > > Please confirm if this is fine. > Sorry for late reply. The new name is OK. Please consider to change other existing CONFIG_* macros to SYS_FSL_* space, and remove them from the white list. York
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index b7a2e0c..ce04e48 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -424,8 +424,10 @@ int arch_early_init_r(void) int timer_init(void) { - u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; + u32 __iomem *cntcr; #ifdef CONFIG_FSL_LSCH3 + struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR); + u32 svr, ver; u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR; #endif #ifdef CONFIG_LS2080A @@ -439,6 +441,16 @@ int timer_init(void) #endif #ifdef CONFIG_FSL_LSCH3 + svr = gur_in32(&gur->svr); + ver = SVR_SOC_VER(svr); + if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) || + (ver == SVR_LS2085A) || (ver == SVR_LS2045A)) + cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR; + else +#endif + cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR; + +#ifdef CONFIG_FSL_LSCH3 /* Enable timebase for all clusters. * It is safe to do so even some clusters are not enabled. */ diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h index 7acba27..e6cdfcb 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h @@ -23,7 +23,8 @@ #define CONFIG_SYS_IFC_ADDR (CONFIG_SYS_IMMR + 0x01240000) #define CONFIG_SYS_NS16550_COM1 (CONFIG_SYS_IMMR + 0x011C0500) #define CONFIG_SYS_NS16550_COM2 (CONFIG_SYS_IMMR + 0x011C0600) -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d0000 +#define LS2080A_LS2085A_TIMER_ADDR 0x023d0000 +#define CONFIG_SYS_FSL_TIMER_ADDR 0x023e0000 #define CONFIG_SYS_FSL_PMU_CLTBENR (CONFIG_SYS_FSL_PMU_ADDR + \ 0x18A0) #define FSL_PMU_PCTBENR_OFFSET (CONFIG_SYS_FSL_PMU_ADDR + 0x8A0)