From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932074AbeBKQRh (ORCPT ); Sun, 11 Feb 2018 11:17:37 -0500 Received: from mail.kmu-office.ch ([178.209.48.109]:54928 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751077AbeBKQRf (ORCPT ); Sun, 11 Feb 2018 11:17:35 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Sun, 11 Feb 2018 17:17:32 +0100 From: Stefan Agner To: Anson Huang Cc: Fabio Estevam , rjw@rjwysocki.net, viresh kumar , linux-pm@vger.kernel.org, Marcel Ziswiler , max.oss.09@gmail.com, linux-kernel , Octavian Purdila , Fabio Estevam , Shawn Guo , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , dl-linux-imx Subject: Re: [PATCH] cpufreq: imx6q: support frequencies >528MHz for i.MX6UL/ULL In-Reply-To: References: <20180118235836.17393-1-stefan@agner.ch> Message-ID: <79baed40abbcc72920fd133bc43e378a@agner.ch> User-Agent: Roundcube Webmail/1.3.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11.02.2018 02:42, Anson Huang wrote: > Anson Huang > Best Regards! > > >> -----Original Message----- >> From: Fabio Estevam [mailto:festevam@gmail.com] >> Sent: Sunday, February 11, 2018 12:26 AM >> To: Stefan Agner ; Anson Huang >> Cc: rjw@rjwysocki.net; viresh kumar ; >> linux-pm@vger.kernel.org; Marcel Ziswiler ; >> max.oss.09@gmail.com; linux-kernel ; Octavian >> Purdila ; Fabio Estevam >> ; Shawn Guo ; moderated >> list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE >> ; dl-linux-imx >> 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 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 >> > --- >> > 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 > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: stefan@agner.ch (Stefan Agner) Date: Sun, 11 Feb 2018 17:17:32 +0100 Subject: [PATCH] cpufreq: imx6q: support frequencies >528MHz for i.MX6UL/ULL In-Reply-To: References: <20180118235836.17393-1-stefan@agner.ch> Message-ID: <79baed40abbcc72920fd133bc43e378a@agner.ch> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 11.02.2018 02:42, Anson Huang wrote: > Anson Huang > Best Regards! > > >> -----Original Message----- >> From: Fabio Estevam [mailto:festevam at gmail.com] >> Sent: Sunday, February 11, 2018 12:26 AM >> To: Stefan Agner ; Anson Huang >> Cc: rjw at rjwysocki.net; viresh kumar ; >> linux-pm at vger.kernel.org; Marcel Ziswiler ; >> max.oss.09 at gmail.com; linux-kernel ; Octavian >> Purdila ; Fabio Estevam >> ; Shawn Guo ; moderated >> list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE >> ; dl-linux-imx >> 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 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 >> > --- >> > 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 > > 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