[U-Boot] [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection

[Priyanka Jain]

> -----Original Message-----
> From: york sun
> Sent: Thursday, October 20, 2016 10:46 AM
> To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; u-
> boot at lists.denx.de
> Cc: Priyanka Jain <priyanka.jain at 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 at nxp.com]
> >> Sent: Wednesday, October 19, 2016 3:07 PM
> >> To: u-boot at lists.denx.de
> >> Cc: Priyanka Jain <priyanka.jain at nxp.com>; Prabhakar Kushwaha
> >> <prabhakar.kushwaha at 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 at nxp.com>
> >> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha at 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