cpufreq: Unable to set cpufreq to maximum

cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Hi,

i currently try to get cpufreq-cpu0 aka cpufreq-dt running on Freescale i.MX28
(ARM). First i ported the regulator driver and now i want to test it with
cpufreq-cpu0. Everything works fine except changing back to maximum frequency
454736 kHz. I get the following error:

root@duckbill:/sys/devices/system/cpu/cpu0/cpufreq# echo "454736" >
scaling_setspeed
[ 1111.226006] cpu cpu0: failed to find OPP for 454736842
[ 1111.231399] cpufreq: __target_index: Failed to change cpu frequency: -34

I'm using 3.17.0-rc7-00466-g6f28c5c-dirty #1 Thu Oct 2 12:07:49 UTC 2014
armv5tejl GNU/Linux and here is the relevant part of the dts file:

cpus {
	#address-cells = <1>;
	#size-cells = <0>;

	cpu@0 {
		compatible = "arm,arm926ej-s";
		device_type = "cpu";
		reg = <0x0>;
		operating-points = <
			/* kHz	uV */
			454736  1550000
			392727  1475000
			360000  1375000
			261818  1275000
			64000   1050000
		>;
		clocks = <&clks 4>;
		clock-latency = <61036>; /* two CLK32 periods */
		cpu0-supply = <&reg_vddd>;
	};
};

It looks to me like a rounding problem.

For the complete history look at:
http://www.spinics.net/lists/arm-kernel/msg365504.html

Any ideas?

Best regards

Stefan

Re: cpufreq: Unable to set cpufreq to maximum

[Lucas Stach]

Am Donnerstag, den 02.10.2014, 15:17 +0200 schrieb Stefan Wahren:
> Hi,
> 
> i currently try to get cpufreq-cpu0 aka cpufreq-dt running on Freescale i.MX28
> (ARM). First i ported the regulator driver and now i want to test it with
> cpufreq-cpu0. Everything works fine except changing back to maximum frequency
> 454736 kHz. I get the following error:
> 
> root@duckbill:/sys/devices/system/cpu/cpu0/cpufreq# echo "454736" >
> scaling_setspeed
> [ 1111.226006] cpu cpu0: failed to find OPP for 454736842
> [ 1111.231399] cpufreq: __target_index: Failed to change cpu frequency: -34
> 
> I'm using 3.17.0-rc7-00466-g6f28c5c-dirty #1 Thu Oct 2 12:07:49 UTC 2014
> armv5tejl GNU/Linux and here is the relevant part of the dts file:
> 
> cpus {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	cpu@0 {
> 		compatible = "arm,arm926ej-s";
> 		device_type = "cpu";
> 		reg = <0x0>;
> 		operating-points = <
> 			/* kHz	uV */
> 			454736  1550000
> 			392727  1475000
> 			360000  1375000
> 			261818  1275000
> 			64000   1050000
> 		>;
> 		clocks = <&clks 4>;
> 		clock-latency = <61036>; /* two CLK32 periods */
> 		cpu0-supply = <&reg_vddd>;
> 	};
> };
> 
> It looks to me like a rounding problem.
> 
> For the complete history look at:
> http://www.spinics.net/lists/arm-kernel/msg365504.html
> 
> Any ideas?

Isn't your highest OPP just wrong? Apparently the CPU clock (which is a
fixed PLL plus a divider on MX28 if I'm not mistaken) is only able to
provide a slightly higher frequency than your OPP. cpufreq handles that
as an error, which is valid behavior as going higher than the defined
OPP may not be qualified by the vendor.

Does it work if you just change this OPP to 454737 kHz?

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Hi Lucas,

> Lucas Stach <l.stach@pengutronix.de> hat am 2. Oktober 2014 um 15:59
> geschrieben:
>
>
> Am Donnerstag, den 02.10.2014, 15:17 +0200 schrieb Stefan Wahren:
> > Hi,
> >
> > i currently try to get cpufreq-cpu0 aka cpufreq-dt running on Freescale
> > i.MX28
> > (ARM). First i ported the regulator driver and now i want to test it with
> > cpufreq-cpu0. Everything works fine except changing back to maximum
> > frequency
> > 454736 kHz. I get the following error:
> >
>> [...]
> >
> > Any ideas?
>
> Isn't your highest OPP just wrong? Apparently the CPU clock (which is a
> fixed PLL plus a divider on MX28 if I'm not mistaken) is only able to
> provide a slightly higher frequency than your OPP. cpufreq handles that
> as an error, which is valid behavior as going higher than the defined
> OPP may not be qualified by the vendor.
 
I hope so.

>
> Does it work if you just change this OPP to 454737 kHz?
>
 
Yes after that i can change it with userspace governor. But i get this warning
during modprobe:
 
root@duckbill:~# modprobe cpufreq-cpu0
[   48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq:
454736 KHz
[   48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 454737 KHz

I'm not sure if it's okay.
 
>
> Regards,
> Lucas
> --
> Pengutronix e.K. | Lucas Stach |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
>

Stefan

Re: cpufreq: Unable to set cpufreq to maximum

[Lucas Stach]

Am Donnerstag, den 02.10.2014, 17:01 +0200 schrieb Stefan Wahren:
> Hi Lucas,
> 
> > Lucas Stach <l.stach@pengutronix.de> hat am 2. Oktober 2014 um 15:59
> > geschrieben:
> >
> >
> > Am Donnerstag, den 02.10.2014, 15:17 +0200 schrieb Stefan Wahren:
> > > Hi,
> > >
> > > i currently try to get cpufreq-cpu0 aka cpufreq-dt running on Freescale
> > > i.MX28
> > > (ARM). First i ported the regulator driver and now i want to test it with
> > > cpufreq-cpu0. Everything works fine except changing back to maximum
> > > frequency
> > > 454736 kHz. I get the following error:
> > >
> >> [...]
> > >
> > > Any ideas?
> >
> > Isn't your highest OPP just wrong? Apparently the CPU clock (which is a
> > fixed PLL plus a divider on MX28 if I'm not mistaken) is only able to
> > provide a slightly higher frequency than your OPP. cpufreq handles that
> > as an error, which is valid behavior as going higher than the defined
> > OPP may not be qualified by the vendor.
>  
> I hope so.
> 
> >
> > Does it work if you just change this OPP to 454737 kHz?
> >
>  
> Yes after that i can change it with userspace governor. But i get this warning
> during modprobe:
>  
> root@duckbill:~# modprobe cpufreq-cpu0
> [   48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq:
> 454736 KHz
> [   48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
> changed to: 454737 KHz
> 
> I'm not sure if it's okay.
>  
It's harmless in your case. But to get rid of it you might try the
attached patch which may be an acceptable solution.

------------------------------>8-----------------------------------
>From db9e708edf6451b619ee73f68c3fcde2eccd7b0c Mon Sep 17 00:00:00 2001
From: Lucas Stach <l.stach@pengutronix.de>
Date: Thu, 2 Oct 2014 17:20:09 +0200
Subject: [PATCH] cpufreq: generic: try preserve some accuracy while
converting
 from Hz to kHz

---
 drivers/cpufreq/cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d9fdeddcef96..459830c6576f 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -187,7 +187,7 @@ unsigned int cpufreq_generic_get(unsigned int cpu)
 		return 0;
 	}
 
-	return clk_get_rate(policy->clk) / 1000;
+	return DIV_ROUND_CLOSEST(clk_get_rate(policy->clk), 1000);
 }
 EXPORT_SYMBOL_GPL(cpufreq_generic_get);
 
-- 
2.1.0

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Hi Lucas,

> Lucas Stach <l.stach@pengutronix.de> hat am 2. Oktober 2014 um 17:22
> geschrieben:
>
> [...]
> >
> > >
> > > Does it work if you just change this OPP to 454737 kHz?
> > >
> >
> > Yes after that i can change it with userspace governor. But i get this
> > warning
> > during modprobe:
> >
> > root@duckbill:~# modprobe cpufreq-cpu0
> > [ 48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq:
> > 454736 KHz
> > [ 48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
> > changed to: 454737 KHz
> >
> > I'm not sure if it's okay.
> >
> It's harmless in your case. But to get rid of it you might try the
> attached patch which may be an acceptable solution.
>
 
thanks a lot. The warning disappear after apply this patch :)
 
Stefan

> ------------------------------>8-----------------------------------
> From db9e708edf6451b619ee73f68c3fcde2eccd7b0c Mon Sep 17 00:00:00 2001
> From: Lucas Stach <l.stach@pengutronix.de>
> Date: Thu, 2 Oct 2014 17:20:09 +0200
> Subject: [PATCH] cpufreq: generic: try preserve some accuracy while
> converting
> from Hz to kHz
>
> ---
> drivers/cpufreq/cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d9fdeddcef96..459830c6576f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -187,7 +187,7 @@ unsigned int cpufreq_generic_get(unsigned int cpu)
> return 0;
> }
>
> - return clk_get_rate(policy->clk) / 1000;
> + return DIV_ROUND_CLOSEST(clk_get_rate(policy->clk), 1000);
> }
> EXPORT_SYMBOL_GPL(cpufreq_generic_get);
>
> --
> 2.1.0
>
> --
> Pengutronix e.K. | Lucas Stach |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
>

Re: cpufreq: Unable to set cpufreq to maximum

[Viresh Kumar]

On 2 October 2014 20:52, Lucas Stach <l.stach@pengutronix.de> wrote:
> It's harmless in your case. But to get rid of it you might try the
> attached patch which may be an acceptable solution.
>
> ------------------------------>8-----------------------------------
> From db9e708edf6451b619ee73f68c3fcde2eccd7b0c Mon Sep 17 00:00:00 2001
> From: Lucas Stach <l.stach@pengutronix.de>
> Date: Thu, 2 Oct 2014 17:20:09 +0200
> Subject: [PATCH] cpufreq: generic: try preserve some accuracy while
> converting
>  from Hz to kHz
>
> ---
>  drivers/cpufreq/cpufreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d9fdeddcef96..459830c6576f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -187,7 +187,7 @@ unsigned int cpufreq_generic_get(unsigned int cpu)
>                 return 0;
>         }
>
> -       return clk_get_rate(policy->clk) / 1000;
> +       return DIV_ROUND_CLOSEST(clk_get_rate(policy->clk), 1000);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_generic_get);

Absolutely NOT..

This can't be an acceptable solution to the problem you have in your DT.
This routine is used for getting the current frequency of a CPU and we
can't play with that..

The problem that Stefan is getting is: The boot loader has programmed
cpu to a frequency which isn't present in your DT table. And so kernel
tries to adjust CPU frequency to one of the listed frequencies.

Solutions:
- Add another entry to your DT table to reflect the currently programmed
frequency.
- Just ignore the error, as that is just for information.

Re: cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Am 06.10.2014 um 06:21 schrieb Viresh Kumar:
> On 2 October 2014 20:52, Lucas Stach <l.stach@pengutronix.de> wrote:
>> It's harmless in your case. But to get rid of it you might try the
>> attached patch which may be an acceptable solution.
>>
>> ------------------------------>8-----------------------------------
>> From db9e708edf6451b619ee73f68c3fcde2eccd7b0c Mon Sep 17 00:00:00 2001
>> From: Lucas Stach <l.stach@pengutronix.de>
>> Date: Thu, 2 Oct 2014 17:20:09 +0200
>> Subject: [PATCH] cpufreq: generic: try preserve some accuracy while
>> converting
>>  from Hz to kHz
>>
>> ---
>>  drivers/cpufreq/cpufreq.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index d9fdeddcef96..459830c6576f 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -187,7 +187,7 @@ unsigned int cpufreq_generic_get(unsigned int cpu)
>>                 return 0;
>>         }
>>
>> -       return clk_get_rate(policy->clk) / 1000;
>> +       return DIV_ROUND_CLOSEST(clk_get_rate(policy->clk), 1000);
>>  }
>>  EXPORT_SYMBOL_GPL(cpufreq_generic_get);
> Absolutely NOT..
>
> This can't be an acceptable solution to the problem you have in your DT.
> This routine is used for getting the current frequency of a CPU and we
> can't play with that..

I thought about it and agree with you.

> The problem that Stefan is getting is: The boot loader has programmed
> cpu to a frequency which isn't present in your DT table. And so kernel
> tries to adjust CPU frequency to one of the listed frequencies.

Here comes the real problem, i'm not able to specify the exact frequency
of the cpu. The clock has a value in Hz, but in the DT we have kHz. So
there are always rounding issues. The question is how to handle it.

> Solutions:
> - Add another entry to your DT table to reflect the currently programmed
> frequency.
> - Just ignore the error, as that is just for information.

Ignoring errors isn't a solution. I have another idea. I've looked a
little bit at the code in the cpufreq-(cpu0/dt). Usually the frequencies
are rounded down, but in cpu0_set_target() with
dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz) they are rounded up. From
my point of view that's wrong.

I fixed the problem by decreasing frequency in DT and replacing
dev_pm_opp_find_freq_ceil() with dev_pm_opp_find_freq_floor() in
cpufreq-cpu0.c . Unfortunatelly this will break all other DTS files with
roundings in OPPs.

Stefan

Re: cpufreq: Unable to set cpufreq to maximum

[Viresh Kumar]

On 6 October 2014 12:11, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Ignoring errors isn't a solution. I have another idea. I've looked a
> little bit at the code in the cpufreq-(cpu0/dt). Usually the frequencies
> are rounded down, but in cpu0_set_target() with
> dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz) they are rounded up. From
> my point of view that's wrong.

@Shawn ??

> I fixed the problem by decreasing frequency in DT and replacing
> dev_pm_opp_find_freq_ceil() with dev_pm_opp_find_freq_floor() in
> cpufreq-cpu0.c . Unfortunatelly this will break all other DTS files with
> roundings in OPPs.

I am still not sure how this fixed your problem..

Re: cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Am 06.10.2014 um 10:50 schrieb Viresh Kumar:
> On 6 October 2014 12:11, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Ignoring errors isn't a solution. I have another idea. I've looked a
>> little bit at the code in the cpufreq-(cpu0/dt). Usually the frequencies
>> are rounded down, but in cpu0_set_target() with
>> dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz) they are rounded up. From
>> my point of view that's wrong.
> @Shawn ??
>
>> I fixed the problem by decreasing frequency in DT and replacing
>> dev_pm_opp_find_freq_ceil() with dev_pm_opp_find_freq_floor() in
>> cpufreq-cpu0.c . Unfortunatelly this will break all other DTS files with
>> roundings in OPPs.
> I am still not sure how this fixed your problem..

Sorry, about the confusion. The main problem about setting cpufreq to
maximum has been fixed by DT settings. Above i talked about an idea to
fix the confusing warning after fixing the main problem:

root@duckbill:~# modprobe cpufreq-cpu0
[   48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted
freq: 454736 KHz
[   48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial
frequency changed to: 454737 KHz

But the best solution would be a more intelligent function like
dev_pm_opp_find_freq_nearest() which find the OPP with the smallest
frequency distance. So nobody of the DTS users has to care about
frequency rounding.

Currently i'm trying to adapt the DTS file to the driver and that's not
the intention of devicetree.

Stefan

Re: cpufreq: Unable to set cpufreq to maximum

[Viresh Kumar]

On 6 October 2014 14:43, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Sorry, about the confusion. The main problem about setting cpufreq to
> maximum has been fixed by DT settings. Above i talked about an idea to
> fix the confusing warning after fixing the main problem:
>
> root@duckbill:~# modprobe cpufreq-cpu0
> [   48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted
> freq: 454736 KHz
> [   48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial
> frequency changed to: 454737 KHz
>
> But the best solution would be a more intelligent function like
> dev_pm_opp_find_freq_nearest() which find the OPP with the smallest
> frequency distance. So nobody of the DTS users has to care about
> frequency rounding.
>
> Currently i'm trying to adapt the DTS file to the driver and that's not
> the intention of devicetree.

It looks that one of us hasn't understood how things are working :)

 So here is my part of theory:
- Search for CPUFREQ_NEED_INITIAL_FREQ_CHECK in cpufreq.c
- When starting cpufreq core for every CPU we check if the current
frequency at which is running is mentioned in DT. If not, we throw above
warnings and fix the freq.

- Until this time cpufreq-driver's ->target_index() routine hasn't come
into picture and so NO opp routine will fix that..

Does this sound reasonable ?

Re: cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Am 06.10.2014 um 11:26 schrieb Viresh Kumar:
> On 6 October 2014 14:43, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Sorry, about the confusion. The main problem about setting cpufreq to
>> maximum has been fixed by DT settings. Above i talked about an idea to
>> fix the confusing warning after fixing the main problem:
>>
>> root@duckbill:~# modprobe cpufreq-cpu0
>> [   48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted
>> freq: 454736 KHz
>> [   48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial
>> frequency changed to: 454737 KHz
>>
>> But the best solution would be a more intelligent function like
>> dev_pm_opp_find_freq_nearest() which find the OPP with the smallest
>> frequency distance. So nobody of the DTS users has to care about
>> frequency rounding.
>>
>> Currently i'm trying to adapt the DTS file to the driver and that's not
>> the intention of devicetree.
> It looks that one of us hasn't understood how things are working :)

I think i know him ;-)

>
>  So here is my part of theory:
> - Search for CPUFREQ_NEED_INITIAL_FREQ_CHECK in cpufreq.c
> - When starting cpufreq core for every CPU we check if the current
> frequency at which is running is mentioned in DT. If not, we throw above
> warnings and fix the freq.

Okay i.MX28 cpu is running initial on 454736842 Hz. 454736 kHz in the DT
will make cpufreq core happy, but cpufreq-cpu0 will select next lower
frequency. 454737 kHz in the DT will make cpufreq sad, but it's
selectable by cpufreq-cpu0.

Is it possible cpufreq core round the clock frequency down?

> - Until this time cpufreq-driver's ->target_index() routine hasn't come
> into picture and so NO opp routine will fix that..
>
> Does this sound reasonable ?

Thanks for your explanation.

Stefan

Re: cpufreq: Unable to set cpufreq to maximum

[Viresh Kumar]

On 6 October 2014 15:35, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Okay i.MX28 cpu is running initial on 454736842 Hz. 454736 kHz in the DT
> will make cpufreq core happy,

Probably you know why cpufreq-core would be happy but let me still
write it down.
policy->cur = cpufreq_generic_get(), i.e. clk_get_rate(policy->clk) / 1000
So, policy->cur = 454736842/1000 = 454736

This is == to the DT value and so cpufreq-core is happy.

> but cpufreq-cpu0 will select next lower

Where did this come from in the above theory ?

> frequency. 454737 kHz in the DT will make cpufreq sad, but it's
> selectable by cpufreq-cpu0.

I couldn't understand why you mentioned this here. It has nothing to
do with the warning you got..

> Is it possible cpufreq core round the clock frequency down?

Even this, sorry :(

Re: cpufreq: Unable to set cpufreq to maximum

[Stefan Wahren]

Am 06.10.2014 um 12:16 schrieb Viresh Kumar:
> On 6 October 2014 15:35, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> but cpufreq-cpu0 will select next lower
> Where did this come from in the above theory ?
>

I was wrong. cpufreq-cpu0 will search for a higher OPP than 454736842
and fails:

[ 1111.226006] cpu cpu0: failed to find OPP for 454736842

May be Shawn has an idea or solution.

Sorry for that noise.

Stefan

Re: cpufreq: Unable to set cpufreq to maximum

[Viresh Kumar]

On 6 October 2014 16:42, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Am 06.10.2014 um 12:16 schrieb Viresh Kumar:
>> On 6 October 2014 15:35, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>> but cpufreq-cpu0 will select next lower
>> Where did this come from in the above theory ?
>>
>
> I was wrong. cpufreq-cpu0 will search for a higher OPP than 454736842
> and fails:
>
> [ 1111.226006] cpu cpu0: failed to find OPP for 454736842

I have been talking about this error all this time:

[   48.573808] cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq:
454736 KHz
[   48.604871] cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 454737 KHz

And this has nothing to do with the error you just pasted. For the above two
errors cpufreq-cpu0's ->target_index() isn't called at all and so it doesn't
make a difference..