On 06/08/2016 06:12 AM, Sriram Dash wrote: >> -----Original Message----- >> From: Marek Vasut [mailto:ma...@denx.de] >> Sent: Monday, June 06, 2016 6:21 PM >> To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de >> Cc: york sun <york....@nxp.com>; albert.u.b...@aribaud.net; Rajesh Bhagat >> <rajesh.bha...@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:ma...@denx.de] >>>> Sent: Thursday, June 02, 2016 6:18 PM >>>> To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de >>>> Cc: york sun <york....@nxp.com>; albert.u.b...@aribaud.net; Rajesh >>>> Bhagat <rajesh.bha...@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.d...@nxp.com> >>>>> Signed-off-by: Rajesh Bhagat <rajesh.bha...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot