Anson Huang Best Regards!
> -----Original Message----- > From: Stefan Agner [mailto:[email protected]] > Sent: Monday, February 12, 2018 12:18 AM > To: Anson Huang <[email protected]> > Cc: Fabio Estevam <[email protected]>; [email protected]; viresh kumar > <[email protected]>; [email protected]; Marcel Ziswiler > <[email protected]>; [email protected]; linux-kernel > <[email protected]>; Octavian Purdila <[email protected]>; > Fabio Estevam <[email protected]>; Shawn Guo > <[email protected]>; moderated list:ARM/FREESCALE IMX / MXC ARM > ARCHITECTURE <[email protected]>; dl-linux-imx > <[email protected]> > Subject: Re: [PATCH] cpufreq: imx6q: support frequencies >528MHz for > i.MX6UL/ULL > > On 11.02.2018 02:42, Anson Huang wrote: > > Anson Huang > > Best Regards! > > > > > >> -----Original Message----- > >> From: Fabio Estevam [mailto:[email protected]] > >> Sent: Sunday, February 11, 2018 12:26 AM > >> To: Stefan Agner <[email protected]>; Anson Huang > <[email protected]> > >> Cc: [email protected]; viresh kumar <[email protected]>; > >> [email protected]; Marcel Ziswiler > >> <[email protected]>; [email protected]; linux-kernel > >> <[email protected]>; Octavian Purdila > >> <[email protected]>; Fabio Estevam <[email protected]>; > >> Shawn Guo <[email protected]>; moderated list:ARM/FREESCALE IMX / > >> MXC ARM ARCHITECTURE <[email protected]>; > >> dl-linux-imx <[email protected]> > >> Subject: Re: [PATCH] cpufreq: imx6q: support frequencies >528MHz for > >> i.MX6UL/ULL > >> > >> Hi Anson, > >> > >> On Thu, Jan 18, 2018 at 9:58 PM, Stefan Agner <[email protected]> wrote: > >> > Depending on SKU i.MX6UL/i.MX6ULL support frequencies up to 900MHz. > >> > Use PLL1 sys clock for all operating points higher than 528MHz. > >> > > >> > Note: For higher operating points VDD_SOC_IN needs to be 125mV > >> > higher than the ARM set-point (see datasheet). Specifically, the > >> > i.MX6UL/ULL EVK boards have an external DC regulator which needs > >> > adjustment. The regulator adjustment is not covered with this change. > >> > > >> > Signed-off-by: Stefan Agner <[email protected]> > >> > --- > >> > drivers/cpufreq/imx6q-cpufreq.c | 14 ++++++++------ > >> > 1 file changed, 8 insertions(+), 6 deletions(-) > >> > > >> > diff --git a/drivers/cpufreq/imx6q-cpufreq.c > >> > b/drivers/cpufreq/imx6q-cpufreq.c index 628fe899cb48..840f6386c780 > >> > 100644 > >> > --- a/drivers/cpufreq/imx6q-cpufreq.c > >> > +++ b/drivers/cpufreq/imx6q-cpufreq.c > >> > @@ -114,12 +114,14 @@ static int imx6q_set_target(struct > >> > cpufreq_policy > >> *policy, unsigned int index) > >> > */ > >> > clk_set_rate(arm_clk, (old_freq >> 1) * 1000); > >> > clk_set_parent(pll1_sw_clk, pll1_sys_clk); > >> > - if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) > >> > - clk_set_parent(secondary_sel_clk, > pll2_bus_clk); > >> > - else > >> > - clk_set_parent(secondary_sel_clk, > >> pll2_pfd2_396m_clk); > >> > - clk_set_parent(step_clk, secondary_sel_clk); > >> > - clk_set_parent(pll1_sw_clk, step_clk); > >> > + if (freq_hz <= clk_get_rate(pll2_bus_clk)) { > >> > + if (freq_hz > > clk_get_rate(pll2_pfd2_396m_clk)) > >> > + clk_set_parent(secondary_sel_clk, > >> pll2_bus_clk); > >> > + else > >> > + clk_set_parent(secondary_sel_clk, > >> pll2_pfd2_396m_clk); > >> > + clk_set_parent(step_clk, secondary_sel_clk); > >> > + clk_set_parent(pll1_sw_clk, step_clk); > >> > + } > > > > For cpufreq > 528MHz, ARM PLL needs to be set_rate, I did NOT see > > where sets ARM PLL rate? > > This is done unconditionally after the if statement: > > if (of_machine_is_compatible("fsl,imx6ul") || > of_machine_is_compatible("fsl,imx6ull")) { > /* > * When changing pll1_sw_clk's parent to pll1_sys_clk, > * CPU may run at higher than 528MHz, this will lead to > * the system unstable if the voltage is lower than the > * voltage of 528MHz, so lower the CPU frequency to one > * half before changing CPU frequency. > */ > clk_set_rate(arm_clk, (old_freq >> 1) * 1000); > clk_set_parent(pll1_sw_clk, pll1_sys_clk); > if (freq_hz <= clk_get_rate(pll2_bus_clk)) { > if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) > clk_set_parent(secondary_sel_clk, pll2_bus_clk); > else > clk_set_parent(secondary_sel_clk, > pll2_pfd2_396m_clk); > clk_set_parent(step_clk, secondary_sel_clk); > clk_set_parent(pll1_sw_clk, step_clk); > } > } else { > clk_set_parent(step_clk, pll2_pfd2_396m_clk); > clk_set_parent(pll1_sw_clk, step_clk); > if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) { > clk_set_rate(pll1_sys_clk, new_freq * 1000); > clk_set_parent(pll1_sw_clk, pll1_sys_clk); > } else { > /* pll1_sys needs to be enabled for divider rate change > to work. > */ > pll1_sys_temp_enabled = true; > clk_prepare_enable(pll1_sys_clk); > } > } > > /* Ensure the arm clock divider is what we expect */ > ret = clk_set_rate(arm_clk, new_freq * 1000); > > > -- > Stefan Thanks, I see the CLK_SET_RATE_PARENT flag is set for arm clk. Reviewed-by: Anson Huang <[email protected]> Anson. > > > > > > > Anson. > > > >> > } else { > >> > clk_set_parent(step_clk, pll2_pfd2_396m_clk); > >> > clk_set_parent(pll1_sw_clk, step_clk); > >> > >> Could you please help reviewing this patch? > >> > >> Thanks
