[U-Boot] [PATCH 1/2] armv8/fsl-lsch2: refactor the clock system initialization
Z.Q. Hou
zhiqiang.hou at nxp.com
Tue Sep 13 10:52:00 CEST 2016
Hi Prabhakar,
Thanks for your comments.
> -----Original Message-----
> From: Prabhakar Kushwaha
> Sent: 2016年9月13日 15:18
> To: Z.Q. Hou <zhiqiang.hou at nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; york sun <york.sun at nxp.com>; Vincent Hu
> <mingkai.hu at nxp.com>; Calvin Johnson <calvin.johnson at nxp.com>
> Cc: Z.Q. Hou <zhiqiang.hou at nxp.com>
> Subject: RE: [PATCH 1/2] armv8/fsl-lsch2: refactor the clock system
> initialization
>
>
> > -----Original Message-----
> > From: Zhiqiang Hou [mailto:Zhiqiang.Hou at nxp.com]
> > Sent: Monday, September 12, 2016 9:39 AM
> > To: u-boot at lists.denx.de; albert.u.boot at aribaud.net; york sun
> > <york.sun at nxp.com>; Vincent Hu <mingkai.hu at nxp.com>; Prabhakar
> > Kushwaha <prabhakar.kushwaha at nxp.com>; Calvin Johnson
> > <calvin.johnson at nxp.com>
> > Cc: Z.Q. Hou <zhiqiang.hou at nxp.com>
> > Subject: [PATCH 1/2] armv8/fsl-lsch2: refactor the clock system
> > initialization
> >
> > From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> >
> > Up to now, there are 3 kinds of SoC under Layerscape Chassis 2, such
> > as LS1043A, LS1046A and LS1012A. But the clocks tree has a lot of
> > difference, for instance the IP modules have different divisors to get
> > clock from Platform PLL. And the core cluster and platform PLL maybe
> > have different reference clock, such as LS1012A. Another problem is
> > which clock/PLL should be described by sys_info->freq_systembus, it is
> > confused in Chissis 2.
> >
> > This patch is to map the sys_info->freq_systembus to the Platform PLL,
> > and handle the different divisor of IP modules separately between
> > different SoCs. And separate cluster and platform PLL reference clock.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > ---
> > .../arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c | 96
> > ++++++++++++++++++----
> > .../include/asm/arch-fsl-layerscape/immap_lsch2.h | 1 +
> > include/configs/ls1012a_common.h | 6 +-
> > include/configs/ls1043a_common.h | 2 +-
> > 4 files changed, 86 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> > index 8922197..4fb736d 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> > @@ -52,22 +52,28 @@ void get_sys_info(struct sys_info *sys_info)
> > uint freq_c_pll[CONFIG_SYS_FSL_NUM_CC_PLLS];
> > uint ratio[CONFIG_SYS_FSL_NUM_CC_PLLS];
> > unsigned long sysclk = CONFIG_SYS_CLK_FREQ;
> > + unsigned long cluster_clk;
> >
> > sys_info->freq_systembus = sysclk;
> > +#ifdef CONFIG_CLUSTER_CLK_FREQ
> > + cluster_clk = CONFIG_CLUSTER_CLK_FREQ;
>
> Just thinking, why to have CONFIG_CLUSTER_CLK_FREQ specially for LS1012A.
>
The CONFIG_CLUSTER_CLK_FREQ is not special for LS1012A, actually all SoCs should define it. As the other SoCs have the same core cluster PLL and platform PLL reference clock, doesn't like LS1012A have 100MHz for core cluster PLL and 125MHz for platform PLL, so add this macro only in LS1012A and others reuse CONFIG_SYS_CLK_FREQ.
> Why cannot below be defined for all SoCs #ifndef
> CONFIG_CLUSTER_CLK_FREQ #define CONFIG_CLUSTER_CLK_FREQ
> CONFIG_SYS_CLK_FREQ #endif
>
> This will avoid special #else part mentioned just below.
>
Yes, it could be arranged, although have no functional change, seems more brief.
> > +#else
> > + cluster_clk = sysclk;
> > +#endif
> > #ifdef CONFIG_DDR_CLK_FREQ
> > sys_info->freq_ddrbus = CONFIG_DDR_CLK_FREQ; #else
> > sys_info->freq_ddrbus = sysclk;
>
> Is this case is only for LS1012A. then it is not required as below code under
> CONFIG_LS1012A doing same.
>
No, it works for all SoCs that have DDR PLL and DDR reference clock. Actually the reason to introduce the macro CONFIG_CLUSTER_CLK_FREQ is to avoid using CONFIG_DDR_CLK_FREQ on LS1012A.
The LS1012A has no DDR PLL and DDR clock is provided by platform PLL, so it isn't reasonable to use CONFIG_DDR_CLK_FREQ on LS1012A.
> > #endif
> >
> > -#ifdef CONFIG_LS1012A
> > - sys_info->freq_ddrbus *= (gur_in32(&gur->rcwsr[0]) >>
> > - FSL_CHASSIS2_RCWSR0_SYS_PLL_RAT_SHIFT) &
> > - FSL_CHASSIS2_RCWSR0_SYS_PLL_RAT_MASK;
> > -#else
> > + /* The freq_systembus is used to record frequency of platform PLL */
> > sys_info->freq_systembus *= (gur_in32(&gur->rcwsr[0]) >>
> > FSL_CHASSIS2_RCWSR0_SYS_PLL_RAT_SHIFT) &
> > FSL_CHASSIS2_RCWSR0_SYS_PLL_RAT_MASK;
> > +
> > +#ifdef CONFIG_LS1012A
> > + sys_info->freq_ddrbus = 2 * sys_info->freq_systembus; #else
> > sys_info->freq_ddrbus *= (gur_in32(&gur->rcwsr[0]) >>
> > FSL_CHASSIS2_RCWSR0_MEM_PLL_RAT_SHIFT) &
> > FSL_CHASSIS2_RCWSR0_MEM_PLL_RAT_MASK;
> > @@ -76,7 +82,7 @@ void get_sys_info(struct sys_info *sys_info)
> > for (i = 0; i < CONFIG_SYS_FSL_NUM_CC_PLLS; i++) {
> > ratio[i] = (in_be32(&clk->pllcgsr[i].pllcngsr) >> 1) & 0xff;
> > if (ratio[i] > 4)
> > - freq_c_pll[i] = sysclk * ratio[i];
> > + freq_c_pll[i] = cluster_clk * ratio[i];
> > else
> > freq_c_pll[i] = sys_info->freq_systembus * ratio[i];
> > }
> > @@ -91,11 +97,6 @@ void get_sys_info(struct sys_info *sys_info)
> > freq_c_pll[cplx_pll] / core_cplx_pll_div[c_pll_sel];
> > }
> >
> > -#ifdef CONFIG_LS1012A
> > - sys_info->freq_systembus = sys_info->freq_ddrbus / 2;
> > - sys_info->freq_ddrbus *= 2;
> > -#endif
> > -
> > #define HWA_CGA_M1_CLK_SEL 0xe0000000
> > #define HWA_CGA_M1_CLK_SHIFT 29
> > #ifdef CONFIG_SYS_DPAA_FMAN
> > @@ -148,7 +149,11 @@ void get_sys_info(struct sys_info *sys_info)
> > break;
> > }
> > #else
> > +#ifdef CONFIG_LS1043A
> > sys_info->freq_sdhc = sys_info->freq_systembus;
> > +#elif defined(CONFIG_LS1046A) || defined(CONFIG_LS1012A)
> > + sys_info->freq_sdhc = sys_info->freq_systembus / 2; #endif
> > #endif
> > #endif
> >
> > @@ -156,7 +161,11 @@ void get_sys_info(struct sys_info *sys_info)
> > ccr = ifc_in32(&ifc_regs.gregs->ifc_ccr);
> > ccr = ((ccr & IFC_CCR_CLK_DIV_MASK) >> IFC_CCR_CLK_DIV_SHIFT) + 1;
> >
> > +#ifdef CONFIG_LS1043A
> > sys_info->freq_localbus = sys_info->freq_systembus / ccr;
> > +#elif defined(CONFIG_LS1046A)
> > + sys_info->freq_localbus = sys_info->freq_systembus / 2 / ccr; #endif
> > #endif
> > }
> >
> > @@ -179,41 +188,98 @@ int get_clocks(void)
> > return 1;
> > }
> >
> > +/********************************************
> > + * get_bus_freq
> > + * return platform PLL freq in Hz
> > + *********************************************/
> > ulong get_bus_freq(ulong dummy)
> > {
> > + if (!gd->bus_clk)
> > + get_clocks();
> > +
> > return gd->bus_clk;
> > }
> >
> > ulong get_ddr_freq(ulong dummy)
> > {
> > + if (!gd->mem_clk)
> > + get_clocks();
> > +
> > return gd->mem_clk;
> > }
> >
> > #ifdef CONFIG_FSL_ESDHC
> > int get_sdhc_freq(ulong dummy)
> > {
> > + if (!gd->arch.sdhc_clk)
> > + get_clocks();
> > +
> > return gd->arch.sdhc_clk;
> > }
> > #endif
> >
> > int get_serial_clock(void)
> > {
> > - return gd->bus_clk;
> > +#ifdef CONFIG_LS1043A
> > + return get_bus_freq(0);
> > +#elif defined(CONFIG_LS1046A)
> > + return get_bus_freq(0) / 2;
> > +#elif defined(CONFIG_LS1012A)
> > + return get_bus_freq(0) / 4;
> > +#else
> > + return 0;
> > +#endif
> > +}
> > +
> > +int get_i2c_freq(ulong dummy)
> > +{
> > +#ifdef CONFIG_LS1043A
> > + return get_bus_freq(0);
>
> I will suggest to avoid SoC specific define from this file.
> Define CONFIG_SYS_FSL_I2C_CLK_DIV.
> #ifndef CONFIG_SYS_FSL_I2C_CLK_DIV
> #define CONFIG_SYS_FSL_I2C_CLK_DIV 2
> #endif
>
> Its value will be 1 and 4 only for LS1042 and LS1012A.
> Similarly you can do for other clocks like serial, dspi etc.
>
Yes, this is a good idea, will add in next version.
Thanks,
Zhiqiang
More information about the U-Boot
mailing list