[U-Boot] [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV helper

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Monday, June 06, 2016 6:21 PM
>To: Sriram Dash <sriram.dash at nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun at nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat at nxp.com>
>Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and IS_SVR_REV
>helper
>
>On 06/06/2016 06:21 AM, Sriram Dash wrote:
>>> -----Original Message-----
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Thursday, June 02, 2016 6:18 PM
>>> To: Sriram Dash <sriram.dash at nxp.com>; u-boot at lists.denx.de
>>> Cc: york sun <york.sun at nxp.com>; albert.u.boot at aribaud.net; Rajesh
>>> Bhagat <rajesh.bhagat at nxp.com>
>>> Subject: Re: [PATCH v2 1/5] arm64: fsl-layerscape: add get_svr and
>>> IS_SVR_REV helper
>>>
>>> On 06/02/2016 08:54 AM, Sriram Dash wrote:
>>>> Signed-off-by: Sriram Dash <sriram.dash at nxp.com>
>>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat at nxp.com>
>>>> ---
>>>> Changes in v2:
>>>>   - No update
>>>>
>>>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c                | 7 +++++++
>>>>  arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 2 ++
>>>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 3 +++
>>>>  arch/arm/include/asm/arch-fsl-layerscape/soc.h         | 2 ++
>>>>  4 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> index 9a5a6b5..9c575c1 100644
>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>> @@ -528,6 +528,13 @@ u32 fsl_qoriq_core_to_type(unsigned int core)
>>>>  	return -1;      /* cannot identify the cluster */
>>>>  }
>>>>
>>>> +uint get_svr(void)
>>>
>>> Should be u32 . The function should have some more recognizable name,
>>> fsl_get_svr() or such.
>>>
>>
>> Hello Marek,
>>
>> As done in PPC for all the Socs for FSL, in arch/powerpc/cpu/mpc83xx/start.S ,
>and ARM v7 LS1021, in arch/arm/cpu/armv7/ls102xa/cpu.c, this uint get_svr(void)
>function finds the svr number for the SoC. Now, as i have to make the code generic
>for PPC and ARM (in patch 3/5 namely make errata function common for PPC and
>ARM), I am currently having the same name and return type used.
>
>You can rename it now.
>

Somewhat I agree to the point. It is good to have u32 instead of 
uint and the get_svr() function should have some more recognizable
name. But, this function is already being used in many files and this
would require for some uncalled 120 + changes for PPC and ARM v7,
which is not in the scope of this patchset.
Currently, I am focusing on the errata implementation, which I want
to make generic, by having the same names across PPC, ARMv7 and
ARMv8. So, I am planning to do this change at a later point of time.

>> Regards,
>> Sriram
>>
>>>> +{
>>>> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
>>>> +
>>>> +	return gur_in32(&gur->svr);
>>>> +}
>>>> +
>>>>  #ifdef CONFIG_DISPLAY_CPUINFO
>>>>  int print_cpuinfo(void)
>>>>  {
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> index 57b99d4..4151994 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
>>>> @@ -592,4 +592,6 @@ struct ccsr_cci400 {
>>>>  #define SCR0_CLIENTPD_MASK		0x00000001
>>>>  #define SCR0_USFCFG_MASK		0x00000400
>>>>
>>>> +uint get_svr(void);
>>>> +
>>>>  #endif	/* __ARCH_FSL_LSCH2_IMMAP_H__*/
>>>> 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 65b3357..e48bbaf 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> @@ -319,4 +319,7 @@ struct ccsr_reset {
>>>>  	u32 ip_rev1;			/* 0xbf8 */
>>>>  	u32 ip_rev2;			/* 0xbfc */
>>>>  };
>>>> +
>>>> +uint get_svr(void);
>>>> +
>>>>  #endif /* __ARCH_FSL_LSCH3_IMMAP_H_ */ diff --git
>>>> a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> index 831d817..3f1a0a8 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h
>>>> @@ -52,6 +52,8 @@ struct cpu_type {
>>>>  #define SVR_MIN(svr)		(((svr) >> 0) & 0xf)
>>>>  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
>>>>  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
>>>> +#define IS_SVR_REV(svr, maj, min) \
>>>> +		((SVR_MAJ(svr) == maj) && (SVR_MIN(svr) == min))
>>>>
>>>>  /* ahci port register default value */
>>>>  #define AHCI_PORT_PHY_1_CFG    0xa003fffe
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Marek Vasut
>
>
>--
>Best regards,
>Marek Vasut