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

[Prabhakar Kushwaha]

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: Wednesday, May 18, 2016 2:03 AM
> To: Edward L Swarthout <ed.swarthout at nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>
> Cc: u-boot at lists.denx.de
> Subject: Re: [u-boot-release] [PATCH 06/10][v3] armv8: fsl-layerscape: Add
> support of QorIQ LS1012A SoC
> 
> 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.
> 

Thanks ED, York,

Let me work on it. I will take care of this while sending v4.

--prabhakar