* Re: [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz
2017-07-28 8:36 [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz Sébastien Szymanski
@ 2017-08-01 23:25 ` Rafael J. Wysocki
2017-08-02 3:49 ` Viresh Kumar
2017-08-03 2:03 ` Shawn Guo
2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2017-08-01 23:25 UTC (permalink / raw)
To: Sébastien Szymanski, linux-arm-kernel
Cc: linux-kernel, linux-pm, Viresh Kumar, Shawn Guo, Fabio Estevam,
Julien Boibessot
On Friday, July 28, 2017 10:36:33 AM Sébastien Szymanski wrote:
> Setting the frequency higher than 528Mhz actually sets the ARM
> clock to 528MHz. That's because PLL2 is used as the root clock when the
> frequency is higher than 396MHz.
>
> cpupower frequency-set -f 792000
>
> arm_clk_root on the CCM_CLKO2 signal is 528MHz instead of 792MHz.
>
> [ 61.606383] cpu cpu0: 396 MHz, 1025 mV --> 792 MHz, 1225 mV
>
> pll2 1 1 528000000 0 0
> pll2_bypass 1 1 528000000 0 0
> pll2_bus 3 3 528000000 0 0
> ca7_secondary_sel 1 1 528000000 0 0
> step 1 1 528000000 0 0
> pll1_sw 1 1 528000000 0 0
> arm 1 1 528000000 0 0
>
> Fixes this by using the PLL1 as the root clock when the frequency is
> higher than 528MHz.
>
> cpupower frequency-set -f 792000
>
> arm_clk_root on the CCM_CLKO2 signal is now 792MHz as expected.
>
> [ 69.717987] cpu cpu0: 198 MHz, 950 mV --> 792 MHz, 1225 mV
>
> pll1 1 1 792000000 0 0
> pll1_bypass 1 1 792000000 0 0
> pll1_sys 1 1 792000000 0 0
> pll1_sw 1 1 792000000 0 0
> arm 1 1 792000000 0 0
>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
> drivers/cpufreq/imx6q-cpufreq.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> index b6edd3c..e5fba50 100644
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -18,6 +18,7 @@
>
> #define PU_SOC_VOLTAGE_NORMAL 1250000
> #define PU_SOC_VOLTAGE_HIGH 1275000
> +#define FREQ_528_MHZ 528000000
> #define FREQ_1P2_GHZ 1200000000
>
> static struct regulator *arm_reg;
> @@ -110,14 +111,20 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
> * 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 ((old_freq * 1000) <= FREQ_528_MHZ) {
> + 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 > FREQ_528_MHZ) {
> + clk_set_rate(pll1_sys_clk, freq_hz);
> + clk_set_parent(pll1_sw_clk, pll1_sys_clk);
> + }
> } else {
> clk_set_parent(step_clk, pll2_pfd2_396m_clk);
> clk_set_parent(pll1_sw_clk, step_clk);
>
Any comments anyone? Good, bad, ugly?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz
2017-07-28 8:36 [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz Sébastien Szymanski
2017-08-01 23:25 ` Rafael J. Wysocki
@ 2017-08-02 3:49 ` Viresh Kumar
2017-08-03 2:03 ` Shawn Guo
2 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2017-08-02 3:49 UTC (permalink / raw)
To: Shawn Guo, Sébastien Szymanski
Cc: linux-kernel, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
Fabio Estevam, Julien Boibessot
On 28-07-17, 10:36, Sébastien Szymanski wrote:
> Setting the frequency higher than 528Mhz actually sets the ARM
> clock to 528MHz. That's because PLL2 is used as the root clock when the
> frequency is higher than 396MHz.
>
> cpupower frequency-set -f 792000
>
> arm_clk_root on the CCM_CLKO2 signal is 528MHz instead of 792MHz.
>
> [ 61.606383] cpu cpu0: 396 MHz, 1025 mV --> 792 MHz, 1225 mV
>
> pll2 1 1 528000000 0 0
> pll2_bypass 1 1 528000000 0 0
> pll2_bus 3 3 528000000 0 0
> ca7_secondary_sel 1 1 528000000 0 0
> step 1 1 528000000 0 0
> pll1_sw 1 1 528000000 0 0
> arm 1 1 528000000 0 0
>
> Fixes this by using the PLL1 as the root clock when the frequency is
> higher than 528MHz.
>
> cpupower frequency-set -f 792000
>
> arm_clk_root on the CCM_CLKO2 signal is now 792MHz as expected.
>
> [ 69.717987] cpu cpu0: 198 MHz, 950 mV --> 792 MHz, 1225 mV
>
> pll1 1 1 792000000 0 0
> pll1_bypass 1 1 792000000 0 0
> pll1_sys 1 1 792000000 0 0
> pll1_sw 1 1 792000000 0 0
> arm 1 1 792000000 0 0
>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
> drivers/cpufreq/imx6q-cpufreq.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
Looks fine to me:
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
@Shawn: Can you also confirm if this looks fine to you ?
--
viresh
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz
2017-07-28 8:36 [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz Sébastien Szymanski
2017-08-01 23:25 ` Rafael J. Wysocki
2017-08-02 3:49 ` Viresh Kumar
@ 2017-08-03 2:03 ` Shawn Guo
2017-08-03 7:32 ` Sébastien Szymanski
2 siblings, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2017-08-03 2:03 UTC (permalink / raw)
To: Sébastien Szymanski, Anson Huang, Leonard Crestez
Cc: linux-kernel, linux-pm, Viresh Kumar, Rafael J. Wysocki,
Julien Boibessot, Fabio Estevam, linux-arm-kernel
On Fri, Jul 28, 2017 at 10:36:33AM +0200, Sébastien Szymanski wrote:
> Setting the frequency higher than 528Mhz actually sets the ARM
> clock to 528MHz. That's because PLL2 is used as the root clock when the
> frequency is higher than 396MHz.
>
> cpupower frequency-set -f 792000
>
> arm_clk_root on the CCM_CLKO2 signal is 528MHz instead of 792MHz.
>
> [ 61.606383] cpu cpu0: 396 MHz, 1025 mV --> 792 MHz, 1225 mV
>
> pll2 1 1 528000000 0 0
> pll2_bypass 1 1 528000000 0 0
> pll2_bus 3 3 528000000 0 0
> ca7_secondary_sel 1 1 528000000 0 0
> step 1 1 528000000 0 0
> pll1_sw 1 1 528000000 0 0
> arm 1 1 528000000 0 0
>
> Fixes this by using the PLL1 as the root clock when the frequency is
> higher than 528MHz.
>
> cpupower frequency-set -f 792000
>
> arm_clk_root on the CCM_CLKO2 signal is now 792MHz as expected.
>
> [ 69.717987] cpu cpu0: 198 MHz, 950 mV --> 792 MHz, 1225 mV
>
> pll1 1 1 792000000 0 0
> pll1_bypass 1 1 792000000 0 0
> pll1_sys 1 1 792000000 0 0
> pll1_sw 1 1 792000000 0 0
> arm 1 1 792000000 0 0
>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
Can you please specify on which SoCs you are seeing this problem? And I
would like invite Anson and Leonard to review it.
Shawn
> ---
> drivers/cpufreq/imx6q-cpufreq.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> index b6edd3c..e5fba50 100644
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -18,6 +18,7 @@
>
> #define PU_SOC_VOLTAGE_NORMAL 1250000
> #define PU_SOC_VOLTAGE_HIGH 1275000
> +#define FREQ_528_MHZ 528000000
> #define FREQ_1P2_GHZ 1200000000
>
> static struct regulator *arm_reg;
> @@ -110,14 +111,20 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
> * 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 ((old_freq * 1000) <= FREQ_528_MHZ) {
> + 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 > FREQ_528_MHZ) {
> + clk_set_rate(pll1_sys_clk, freq_hz);
> + clk_set_parent(pll1_sw_clk, pll1_sys_clk);
> + }
> } else {
> clk_set_parent(step_clk, pll2_pfd2_396m_clk);
> clk_set_parent(pll1_sw_clk, step_clk);
> --
> 2.7.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz
2017-08-03 2:03 ` Shawn Guo
@ 2017-08-03 7:32 ` Sébastien Szymanski
2017-08-04 11:17 ` Leonard Crestez
0 siblings, 1 reply; 6+ messages in thread
From: Sébastien Szymanski @ 2017-08-03 7:32 UTC (permalink / raw)
To: Shawn Guo, Anson Huang, Leonard Crestez
Cc: linux-pm, Viresh Kumar, Rafael J. Wysocki, linux-kernel,
Julien Boibessot, Fabio Estevam, linux-arm-kernel
Hello,
On 08/03/2017 04:03 AM, Shawn Guo wrote:
> On Fri, Jul 28, 2017 at 10:36:33AM +0200, Sébastien Szymanski wrote:
>> Setting the frequency higher than 528Mhz actually sets the ARM
>> clock to 528MHz. That's because PLL2 is used as the root clock when the
>> frequency is higher than 396MHz.
>>
>> cpupower frequency-set -f 792000
>>
>> arm_clk_root on the CCM_CLKO2 signal is 528MHz instead of 792MHz.
>>
>> [ 61.606383] cpu cpu0: 396 MHz, 1025 mV --> 792 MHz, 1225 mV
>>
>> pll2 1 1 528000000 0 0
>> pll2_bypass 1 1 528000000 0 0
>> pll2_bus 3 3 528000000 0 0
>> ca7_secondary_sel 1 1 528000000 0 0
>> step 1 1 528000000 0 0
>> pll1_sw 1 1 528000000 0 0
>> arm 1 1 528000000 0 0
>>
>> Fixes this by using the PLL1 as the root clock when the frequency is
>> higher than 528MHz.
>>
>> cpupower frequency-set -f 792000
>>
>> arm_clk_root on the CCM_CLKO2 signal is now 792MHz as expected.
>>
>> [ 69.717987] cpu cpu0: 198 MHz, 950 mV --> 792 MHz, 1225 mV
>>
>> pll1 1 1 792000000 0 0
>> pll1_bypass 1 1 792000000 0 0
>> pll1_sys 1 1 792000000 0 0
>> pll1_sw 1 1 792000000 0 0
>> arm 1 1 792000000 0 0
>>
>> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
>
> Can you please specify on which SoCs you are seeing this problem? And I
> would like invite Anson and Leonard to review it.
My SoC is MCIMX6Y2CVM08AA which is a 792MHz i.MX6ULL. I forgot to
mention that I added the following operating points in my device tree:
operating-points: 792000 1225000
fsl,soc-operating-points: 792000 1175000
Best regards,
>
> Shawn
>
>> ---
>> drivers/cpufreq/imx6q-cpufreq.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
>> index b6edd3c..e5fba50 100644
>> --- a/drivers/cpufreq/imx6q-cpufreq.c
>> +++ b/drivers/cpufreq/imx6q-cpufreq.c
>> @@ -18,6 +18,7 @@
>>
>> #define PU_SOC_VOLTAGE_NORMAL 1250000
>> #define PU_SOC_VOLTAGE_HIGH 1275000
>> +#define FREQ_528_MHZ 528000000
>> #define FREQ_1P2_GHZ 1200000000
>>
>> static struct regulator *arm_reg;
>> @@ -110,14 +111,20 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
>> * 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 ((old_freq * 1000) <= FREQ_528_MHZ) {
>> + 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 > FREQ_528_MHZ) {
>> + clk_set_rate(pll1_sys_clk, freq_hz);
>> + clk_set_parent(pll1_sw_clk, pll1_sys_clk);
>> + }
>> } else {
>> clk_set_parent(step_clk, pll2_pfd2_396m_clk);
>> clk_set_parent(pll1_sw_clk, step_clk);
>> --
>> 2.7.3
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Sébastien Szymanski
Software engineer, Armadeus Systems
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] cpufreq: imx6q: imx6ull: use PLL1 for frequency higher than 528MHz
2017-08-03 7:32 ` Sébastien Szymanski
@ 2017-08-04 11:17 ` Leonard Crestez
0 siblings, 0 replies; 6+ messages in thread
From: Leonard Crestez @ 2017-08-04 11:17 UTC (permalink / raw)
To: Sébastien Szymanski, Shawn Guo, Anson Huang, Jacky Bai
Cc: linux-kernel, linux-pm, Viresh Kumar, Rafael J. Wysocki,
Julien Boibessot, Fabio Estevam, linux-arm-kernel
On Thu, 2017-08-03 at 09:32 +0200, Sébastien Szymanski wrote:
> On 08/03/2017 04:03 AM, Shawn Guo wrote:
> > On Fri, Jul 28, 2017 at 10:36:33AM +0200, Sébastien Szymanski wrote:
> > > Setting the frequency higher than 528Mhz actually sets the ARM
> > > clock to 528MHz. That's because PLL2 is used as the root clock when the
> > > frequency is higher than 396MHz.
> > >
> > > cpupower frequency-set -f 792000
> > >
> > > arm_clk_root on the CCM_CLKO2 signal is 528MHz instead of 792MHz.
> > >
> > > [ 61.606383] cpu cpu0: 396 MHz, 1025 mV --> 792 MHz, 1225 mV
> > >
> > > pll2 1 1 528000000 0 0
> > > pll2_bypass 1 1 528000000 0 0
> > > pll2_bus 3 3 528000000 0 0
> > > ca7_secondary_sel 1 1 528000000 0 0
> > > step 1 1 528000000 0 0
> > > pll1_sw 1 1 528000000 0 0
> > > arm 1 1 528000000 0 0
> > >
> > > Fixes this by using the PLL1 as the root clock when the frequency is
> > > higher than 528MHz.
> > >
> > > cpupower frequency-set -f 792000
> > >
> > > arm_clk_root on the CCM_CLKO2 signal is now 792MHz as expected.
> > >
> > > [ 69.717987] cpu cpu0: 198 MHz, 950 mV --> 792 MHz, 1225 mV
> > >
> > > pll1 1 1 792000000 0 0
> > > pll1_bypass 1 1 792000000 0 0
> > > pll1_sys 1 1 792000000 0 0
> > > pll1_sw 1 1 792000000 0 0
> > > arm 1 1 792000000 0 0
> > >
> > > Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
> >
> > Can you please specify on which SoCs you are seeing this problem? And I
> > would like invite Anson and Leonard to review it.
>
> My SoC is MCIMX6Y2CVM08AA which is a 792MHz i.MX6ULL. I forgot to
> mention that I added the following operating points in my device tree:
>
> operating-points: 792000 1225000
> fsl,soc-operating-points: 792000 1175000
Yeah, I was wondering about that.
Adding support for higher imx6ul/ull frequencies "properly" requires adding the extra OPPs to .dtsi (they are different between 6ul and 6ull) and reading the speed grading from OCOTP to check if they are supported.
This patch can only work if you hack OPPs for a specific board.
Reading the speed grading is essential because otherwise you risk causing random crashes while running at frequencies not supported by the hardware.
^ permalink raw reply [flat|nested] 6+ messages in thread