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

[york sun]

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

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