From mboxrd@z Thu Jan 1 00:00:00 1970 From: York Sun Date: Tue, 17 May 2016 13:33:15 -0700 Subject: [U-Boot] [u-boot-release] [PATCH 06/10][v3] armv8: fsl-layerscape: Add support of QorIQ LS1012A SoC In-Reply-To: References: <1463214680-11037-1-git-send-email-prabhakar.kushwaha@nxp.com> <1463214680-11037-7-git-send-email-prabhakar.kushwaha@nxp.com> <573B4C39.2080609@nxp.com> Message-ID: <573B800B.8060009@nxp.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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