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

[Marek Vasut <marex@denx.de>]

On 06/08/2016 06:12 AM, Sriram Dash wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Monday, June 06, 2016 6:21 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
>> <rajesh.bhagat@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@nxp.com>; u-boot at lists.denx.de
>>>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh
>>>> Bhagat <rajesh.bhagat@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@nxp.com>
>>>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@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.

I really dislike such function names, but OK, it's manged to spread
through the codebase too much.


-- 
Best regards,
Marek Vasut