[U-Boot] [u-boot-release] [PATCH 06/10][v3] armv8: fsl-layerscape: Add support of QorIQ LS1012A SoC

[York Sun]

On 05/17/2016 12:47 PM, Edward L Swarthout wrote:
> From: York Sun [mailto:york.sun at nxp.com] 
>> On 05/17/2016 09:35 AM, Edward L Swarthout wrote:
>>> From: Prabhakar Kushwaha:
>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
>>>> +	unsigned int svr, ver;
>>> ...
>>>> +	svr = gur_in32(&gur->svr);
>>>> +	ver = SVR_SOC_VER(svr);
>>>> +	if (ver == SVR_LS1012) {
>>>> +		sys_info->freq_ddrbus *= (gur_in32(&gur->rcwsr[0]) >>
>>>
>>> Why introduce a run-time check for every board when this could be handled at compile time?
>>>
>> Do you mean the platform PLL is fixed and not controlled by RCW?
> 
> I mean use "#ifdef CONFIG_LS1012A" instead of reading and comparing SVR at run-time.
> (Note: the platform ratio is controlled by the RCW - it's just that only 4:1 is currently documented)
> 
> LS1012 is unique in that the core frequency is not based on sysclk but its own CGA_CLK_FREQ.
> Instead of twisting the meaning of CONFIG_SYS_CLK_FREQ and CONFIG_DDR_CLK_FREQ
> as the code does here, how about introducing CONFIG_CGA_CLK_FREQ for the freq_processor calculation?
> 

If this is unique to LS1012A, I agree we can use #ifdef to gate the code and use
CONFIG_CGA_CLK_FREQ. Please make sure to document this macro.

York