Hi Prabhakar, Thanks a lot for your comments!
> -----Original Message----- > From: Prabhakar Kushwaha > Sent: 2016年12月28日 12:06 > To: Z.Q. Hou <zhiqiang....@nxp.com>; u-boot@lists.denx.de; > albert.u.b...@aribaud.net; york sun <york....@nxp.com>; Mingkai Hu > <mingkai...@nxp.com>; Calvin Johnson <calvin.john...@nxp.com> > Subject: RE: [PATCHv3 1/2] armv8/fsl-lsch2: refactor the clock system > initialization > > > > -----Original Message----- > > From: Z.Q. Hou > > Sent: Tuesday, December 27, 2016 3:58 PM > > To: Prabhakar Kushwaha <prabhakar.kushw...@nxp.com>; u- > > b...@lists.denx.de; albert.u.b...@aribaud.net; york sun > > <york....@nxp.com>; Mingkai Hu <mingkai...@nxp.com>; Calvin Johnson > > <calvin.john...@nxp.com> > > Subject: RE: [PATCHv3 1/2] armv8/fsl-lsch2: refactor the clock system > > initialization > > > > Hi Prabhakar, > > > > Thanks a lot for your comments! > > > > > -----Original Message----- > > > From: Prabhakar Kushwaha > > > Sent: 2016年12月27日 16:59 > > > To: Z.Q. Hou <zhiqiang....@nxp.com>; u-boot@lists.denx.de; > > > albert.u.b...@aribaud.net; york sun <york....@nxp.com>; Mingkai Hu > > > <mingkai...@nxp.com>; Calvin Johnson <calvin.john...@nxp.com> > > > Cc: Z.Q. Hou <zhiqiang....@nxp.com> > > > Subject: RE: [PATCHv3 1/2] armv8/fsl-lsch2: refactor the clock > > > system initialization > > > > > > > > > > -----Original Message----- > > > > From: Zhiqiang Hou [mailto:zhiqiang....@nxp.com] > > > > Sent: Tuesday, December 27, 2016 1:06 PM > > > > To: u-boot@lists.denx.de; albert.u.b...@aribaud.net; york sun > > > > <york....@nxp.com>; Mingkai Hu <mingkai...@nxp.com>; Prabhakar > > > > Kushwaha <prabhakar.kushw...@nxp.com>; Calvin Johnson > > > > <calvin.john...@nxp.com> > > > > Cc: Z.Q. Hou <zhiqiang....@nxp.com> > > > > Subject: [PATCHv3 1/2] armv8/fsl-lsch2: refactor the clock system > > > > initialization > > > > > > > > From: Hou Zhiqiang <zhiqiang....@nxp.com> > > > > > > > > Up to now, there are 3 kind of SoCs under Layerscape Chassis 2, > > > > like LS1043A, LS1046A and LS1012A. But the clocks tree has a lot > > > > of differences, for instance, the IP modules have different > > > > dividers to derive its clock from Platform PLL. And the core > > > > cluster PLL and platform PLL maybe have different reference clocks, such > as LS1012A. > > > > Another problem is which clock/PLL should be described by > > > > sys_info->freq_systembus, it is confused in Layerscape Chissis 2. > > > > > > > > This patch is to bind the sys_info->freq_systembus to the Platform > > > > PLL, and handle the different divider of IP modules separately > > > > between different SoCs, and separate reference clocks of core > > > > cluster PLL and platform PLL. > > > > > > > > Signed-off-by: Hou Zhiqiang <zhiqiang....@nxp.com> > > > > --- > > > > V3: > > > > - Generate the patch set base on the latest > > > > git://git.denx.de/u-boot-fsl- qoriq.git. > > > > - Use the Kconfig instead of header file to add CONFIG_* > > > > > > > > arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 76 > > > > ++++++++++++++++++++++ > > > > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 3 +- > > > > .../arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c | 68 > > > > ++++++++++++++----- > > > > ++++++++++++++.../include/asm/arch-fsl-layerscape/immap_lsch2.h > > > | 1 + > > > > include/configs/ls1012a_common.h | 6 +- > > > > include/configs/ls1043a_common.h | 3 +- > > > > include/configs/ls1046a_common.h | 3 +- > > > > include/configs/ls2080aqds.h | 2 - > > > > include/configs/ls2080ardb.h | 1 - > > > > 9 files changed, 134 insertions(+), 29 deletions(-) > > > > > > > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > > > > b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > > > > index cc0dc88..de1e5a4 100644 > > > > --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > > > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > > > > @@ -87,6 +87,82 @@ config MAX_CPUS > > > > cores, count the reserved ports. This will allocate enough > > > > memory > > > > in spin table to properly handle all cores. > > > > > > > > +menu "Layerscape clock tree configuration" > > > > + depends on FSL_LSCH2 || FSL_LSCH3 > > > > + > > > > +config SYS_FSL_CLK > > > > + bool "Enable clock tree initialization" > > > > + default y > > > > + > > > > +config CLUSTER_CLK_FREQ > > > > + int "Reference clock of core cluster" > > > > + depends on ARCH_LS1012A > > > > + default 100000000 > > > > + help > > > > + This number is the reference clock frequency of core PLL. > > > > + For most platforms, the core PLL and Platform PLL have the > > > > same > > > > + reference clock, but for some platforms, LS1012A for instance, > > > > + they are provided sepatately. > > > > + > > > > +config SYS_FSL_PCLK_DIV > > > > + int "Platform clock divider" > > > > + default 1 if ARCH_LS1043A > > > > + default 1 if ARCH_LS1046A > > > > + default 2 > > > > > > Only LS2080A and LS2088A requires Platform_PLL_freq / 2. So make 1 > > > as default and exception for LS2080A and LS2088A > > > > > The LS1012A also requires Platform_PLL_freq / 2 to derive Platform > > clock, referring to LS1012A Reference Manual, Rev. C. > > Isn't it better to make 2 the default value? > > > > > > > > > + help > > > > + This is the divider that is used to derive Platform clock from > > > > + Platform PLL, in another word: > > > > + Platform_clk = Platform_PLL_freq / this_divider > > > > + > > > > > > <snips> > > > > > > > +#endif > > > > + cluster_clk = CONFIG_CLUSTER_CLK_FREQ; > > > > + > > > > #ifdef CONFIG_DDR_CLK_FREQ > > > > sys_info->freq_ddrbus = CONFIG_DDR_CLK_FREQ; #else > > > > sys_info->freq_ddrbus = sysclk; > > > > #endif > > > > > > > > -#ifdef CONFIG_ARCH_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_ARCH_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]; > > > > } > > > > > > > > > > Above new logic of calculating sys_clk ddr_clk for LS1012A. It is > > > not matching with previous logic. > > > > > > Let me try to explain for LS1012A > > > > > > #define CONFIG_SYS_CLK_FREQ 100000000 > > > #define CONFIG_DDR_CLK_FREQ 125000000 > > > > > > Previous logic > > > sys_info->freq_ddrbus = 125MHz > > > sys_info->freq_ddrbus = 125 * 4 = 500 MHz here 4 is PLL ratio i.e. 4:1 > > > sys_info->freq_systembus = sys_info->freq_ddrbus/2 = 250MHz > > > sys_info->freq_ddrbus *2 = 1000MT/s > > > > > > New Logic > > > sys_info->freq_systembus = 100MHz > > > sys_info->freq_systembus = 100 * 4 = 400MHz here 4 is PLL ratio > > > sys_info->freq_ddrbus = 2 * sys_info->freq_systembus ==> 800MHz > > > > #define CONFIG_SYS_CLK_FREQ 125000000 > > sys_info->freq_systembus = 125MHz > > sys_info->freq_systembus = 125 * 4 = 500MHz here 4 is PLL ratio > > sys_info->freq_ddrbus = 2 * sys_info->freq_systembus ==> 1000MT/s > > > > The new logic has been discussed with York and Mingkai in patch v2. > > York and Mingkai recommended the new logic. Because the new logic > > match the clock tree generation process more. > > The CONFIG_SYS_CLK_FREQ is the Platform PLL reference clock, and > > actually it is 125MHz instead of 100MHz on ls1012a. > > And there isn't a DDR PLL to generate DDR clock but use the Platform > > PLL directly, so the CONFIG_DDR_CLK_FREQ should not be defined. > > > > Following defines are present in ls1012a_common.h in u-boot.git top commit > 47895838a4fbe867ba9a170c6d1ea6a794095025 > #define CONFIG_SYS_CLK_FREQ 100000000 > #define CONFIG_DDR_CLK_FREQ 125000000 > > Am I missing any pending patch updating CONFIG_DDR_CLK_FREQ > The CONFIG_SYS_CLK_FREQ has been changed to 125MHz and the definition of CONFIG_DDR_CLK_FREQ has been removed in this patch. I paste it here: diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h index 20f0c61..910835e 100644 --- a/include/configs/ls1012a_common.h +++ b/include/configs/ls1012a_common.h @@ -19,9 +19,7 @@ #define CONFIG_SYS_TEXT_BASE 0x40100000 -#define CONFIG_SYS_FSL_CLK -#define CONFIG_SYS_CLK_FREQ 100000000 -#define CONFIG_DDR_CLK_FREQ 125000000 +#define CONFIG_SYS_CLK_FREQ 125000000 Thanks, Zhiqiang _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot