On Thursday 21 November 2013 12:39 PM, Viresh Kumar wrote:
Sometimes boot loaders set CPU frequency to a value outside of frequency table present with cpufreq core. In such cases CPU might be unstable if it has to run on that frequency for long duration of time and so its better to set it to a frequency which is specified in freq-table. This also makes cpufreq stats inconsistent as cpufreq-stats would fail to register because current frequency of CPU isn't found in freq-table.
Because we don't want this change to effect boot process badly, we go for the next freq which is >= policy->cur ('cur' must be set by now, otherwise we will end up setting freq to lowest of the table as 'cur' is initialized to zero).
In case where CPU is already running on one of the frequencies present in freq-table, this would turn into a dummy call as __cpufreq_driver_target() would return early.
Reported-by: Carlos Hernandez ceh@ti.com Reported-by: Nishanth Menon nm@ti.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Copying another mail from Nishant here to get my cc'list back..
On Friday 22 November 2013 05:12 AM, Nishanth Menon wrote:
I gave this a quick run on my 3.12 kernel: http://pastebin.mozilla.org/3649730 is what I applied and output http://pastebin.mozilla.org/3649729
I need to see what I might have mucked up.. or see if I can test this on 3.13 master on a different board (since OMAP5 wont boot in master without clock dts nodes :()..
This happened because of common sense, which was missing in my patch :)
Try this over existing patch:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 6e8b226..e403388 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1126,13 +1126,19 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, * In case where CPU is already running on one of the frequencies * present in freq-table, this would turn into a dummy call as * __cpufreq_driver_target() would return early. + * + * We are passing target-freq as "policy->cur - 1" otherwise + * __cpufreq_driver_target() would simply fail, as policy->cur will be + * equal to target-freq. */ if (has_target()) { - ret = __cpufreq_driver_target(policy, policy->cur, + ret = __cpufreq_driver_target(policy, policy->cur - 1, CPUFREQ_RELATION_L); - if (ret) + if (ret) { pr_err("%s: Unable to set frequency from table: %d\n", __func__, ret); + goto err_out_unregister; + } }
/* related cpus should atleast have policy->cpus */