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

[york sun]

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