On 13 August 2013 00:40, Hans-Christian Egtvedt egtvedt@samfundet.no wrote:
This patch adds a dynamically calculated frequency table to the at32ap driver. In short the architecture can scale in power of two between a maximum and minimum frequency. Min, max, and the steps in between are added to the table.
Signed-off-by: Hans-Christian Egtvedt egtvedt@samfundet.no
drivers/cpufreq/at32ap-cpufreq.c | 47 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c index 6544887..dbf3f3d 100644 --- a/drivers/cpufreq/at32ap-cpufreq.c +++ b/drivers/cpufreq/at32ap-cpufreq.c @@ -19,8 +19,10 @@ #include <linux/clk.h> #include <linux/err.h> #include <linux/export.h> +#include <linux/slab.h>
static struct clk *cpuclk; +static struct cpufreq_frequency_table *freq_table;
static int at32_verify_speed(struct cpufreq_policy *policy) { @@ -85,13 +87,19 @@ static int at32_set_target(struct cpufreq_policy *policy,
static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy) {
unsigned int frequency;
int retval;
int steps;
int i;
if (policy->cpu != 0) return -EINVAL; cpuclk = clk_get(NULL, "cpu"); if (IS_ERR(cpuclk)) { pr_debug("cpufreq: could not get CPU clk\n");
return PTR_ERR(cpuclk);
retval = PTR_ERR(cpuclk);
goto out_err; } policy->cpuinfo.min_freq = (clk_round_rate(cpuclk, 1) + 500) / 1000;
@@ -101,9 +109,46 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy) policy->min = policy->cpuinfo.min_freq; policy->max = policy->cpuinfo.max_freq;
/*
* AVR32 CPU frequency rate scales in power of two between maximum and
* minimum, also add space for the table end marker.
*
* Further validate that the frequency is usable, and append it to the
* frequency table.
*/
steps = fls(policy->cpuinfo.max_freq / policy->cpuinfo.min_freq) + 1;
freq_table = kzalloc(steps * sizeof(struct cpufreq_frequency_table),
GFP_KERNEL);
if (!freq_table) {
retval = -ENOMEM;
goto out_err_put_clk;
}
frequency = policy->cpuinfo.max_freq;
for (i = 0; i < (steps - 1); i++) {
unsigned int rate = clk_round_rate(cpuclk, frequency * 1000);
rate /= 1000;
if (rate != frequency)
freq_table[i].frequency = CPUFREQ_ENTRY_INVALID;
else
freq_table[i].frequency = frequency;
frequency /= 2;
}
freq_table[steps - 1].frequency = CPUFREQ_TABLE_END;
retval = cpufreq_frequency_table_cpuinfo(policy, freq_table);
if (retval)
goto out_err_kfree;
printk("cpufreq: AT32AP CPU frequency driver\n"); return 0;
+out_err_kfree:
kfree(freq_table);
+out_err_put_clk:
clk_put(cpuclk);
+out_err:
return retval;
}
static struct cpufreq_driver at32_driver = {
Thanks for your patch, I have folded below patch with your patch while applying..
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c index eaac7cb..c586d3e 100644 --- a/drivers/cpufreq/at32ap-cpufreq.c +++ b/drivers/cpufreq/at32ap-cpufreq.c @@ -87,10 +87,8 @@ static int at32_set_target(struct cpufreq_policy *policy,
static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy) { - unsigned int frequency; - int retval; - int steps; - int i; + unsigned int frequency, rate; + int retval, steps, i;
if (policy->cpu != 0) return -EINVAL; @@ -126,20 +124,23 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
frequency = policy->cpuinfo.max_freq; for (i = 0; i < (steps - 1); i++) { - unsigned int rate = clk_round_rate(cpuclk, frequency * 1000); - rate /= 1000; + rate = clk_round_rate(cpuclk, frequency * 1000) / 1000; + if (rate != frequency) freq_table[i].frequency = CPUFREQ_ENTRY_INVALID; else freq_table[i].frequency = frequency; + frequency /= 2; } + freq_table[steps - 1].frequency = CPUFREQ_TABLE_END;
retval = cpufreq_frequency_table_cpuinfo(policy, freq_table); if (retval) goto out_err_kfree;
+ cpufreq_frequency_table_get_attr(freq_table, policy->cpu); printk("cpufreq: AT32AP CPU frequency driver\n");
return 0;
On 13 August 2013 10:38, Viresh Kumar viresh.kumar@linaro.org wrote:
Thanks for your patch, I have folded below patch with your patch while applying..
And actually this too, otherwise I need to do this separately in another patch..
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c index c586d3e..1d19fa5 100644 --- a/drivers/cpufreq/at32ap-cpufreq.c +++ b/drivers/cpufreq/at32ap-cpufreq.c @@ -136,15 +136,12 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
freq_table[steps - 1].frequency = CPUFREQ_TABLE_END;
- retval = cpufreq_frequency_table_cpuinfo(policy, freq_table); - if (retval) - goto out_err_kfree; - - cpufreq_frequency_table_get_attr(freq_table, policy->cpu); - printk("cpufreq: AT32AP CPU frequency driver\n"); + retval = cpufreq_table_validate_and_show(policy, freq_table); + if (!retval) { + printk("cpufreq: AT32AP CPU frequency driver\n"); + return 0; + }
- return 0; -out_err_kfree: kfree(freq_table); out_err_put_clk: clk_put(cpuclk);
Around Tue 13 Aug 2013 10:41:39 +0530 or thereabout, Viresh Kumar wrote:
On 13 August 2013 10:38, Viresh Kumar viresh.kumar@linaro.org wrote:
Thanks for your patch, I have folded below patch with your patch while applying..
And actually this too, otherwise I need to do this separately in another patch..
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c index c586d3e..1d19fa5 100644 --- a/drivers/cpufreq/at32ap-cpufreq.c +++ b/drivers/cpufreq/at32ap-cpufreq.c @@ -136,15 +136,12 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
freq_table[steps - 1].frequency = CPUFREQ_TABLE_END;
retval = cpufreq_frequency_table_cpuinfo(policy, freq_table);
if (retval)
goto out_err_kfree;
cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
printk("cpufreq: AT32AP CPU frequency driver\n");
retval = cpufreq_table_validate_and_show(policy, freq_table);
if (!retval) {
printk("cpufreq: AT32AP CPU frequency driver\n");
return 0;
}
return 0;
-out_err_kfree: kfree(freq_table); out_err_put_clk: clk_put(cpuclk);
Acked-by: Hans-Christian Egtvedt egtvedt@samfundet.no
Thanks for cleaning up (-:
On 13 August 2013 12:22, Hans-Christian Egtvedt egtvedt@samfundet.no wrote:
Acked-by: Hans-Christian Egtvedt egtvedt@samfundet.no
Thanks for cleaning up (-:
No problem..
Actually there is more to it.. We have got a freq table but neither .verify() nor .target() uses it :) and so we need to fix those two also to fix it up..
.verify() looks to be pretty straight forward and so can be replaced by the generic routine I wrote for that..
But .target() does some calculations which aren't very straight forward to replace..
I believe I can remove all below code and simply get the frequency as suggested by cpufreq-core ??
/* Convert target_freq from kHz to Hz */ freq = clk_round_rate(cpuclk, target_freq * 1000);
/* Check if policy->min <= new_freq <= policy->max */ if(freq < (policy->min * 1000) || freq > (policy->max * 1000)) return -EINVAL;
pr_debug("cpufreq: requested frequency %u Hz\n", target_freq * 1000);
freqs.new = (freq + 500) / 1000; freqs.flags = 0;
Around Tue 13 Aug 2013 12:28:04 +0530 or thereabout, Viresh Kumar wrote:
On 13 August 2013 12:22, Hans-Christian Egtvedt egtvedt@samfundet.no wrote:
Acked-by: Hans-Christian Egtvedt egtvedt@samfundet.no
Thanks for cleaning up (-:
No problem..
Actually there is more to it.. We have got a freq table but neither .verify() nor .target() uses it :) and so we need to fix those two also to fix it up..
.verify() looks to be pretty straight forward and so can be replaced by the generic routine I wrote for that..
I though you already had a patch to swap verify, hence the need to cleanup the cpufreq driver first to provide a frequency table?
But .target() does some calculations which aren't very straight forward to replace..
I believe I can remove all below code and simply get the frequency as suggested by cpufreq-core ??
/* Convert target_freq from kHz to Hz */ freq = clk_round_rate(cpuclk, target_freq * 1000);
/* Check if policy->min <= new_freq <= policy->max */ if(freq < (policy->min * 1000) || freq > (policy->max * 1000)) return -EINVAL;
This sanity checks that the frequency the system clock can generate is within the policy limits. It is initially feed the target_freq frequency.
These values are already calculated during init and added to the table, so one shouldn't need to perform the clk_round_rate() call anymore.
pr_debug("cpufreq: requested frequency %u Hz\n", target_freq * 1000);
freqs.new = (freq + 500) / 1000; freqs.flags = 0;
I don't know the details of how the cpufreq-core suggests frequencies. If it looks up in the frequency table, then we don't need this sanity check in .target().
On 13 August 2013 14:36, Hans-Christian Egtvedt egtvedt@samfundet.no wrote:
Around Tue 13 Aug 2013 12:28:04 +0530 or thereabout, Viresh Kumar wrote:
.verify() looks to be pretty straight forward and so can be replaced by the generic routine I wrote for that..
I though you already had a patch to swap verify, hence the need to cleanup the cpufreq driver first to provide a frequency table?
Yes, that's what.. That generic routine can be used instead of your .verify()..
But .target() does some calculations which aren't very straight forward to replace..
I believe I can remove all below code and simply get the frequency as suggested by cpufreq-core ??
/* Convert target_freq from kHz to Hz */ freq = clk_round_rate(cpuclk, target_freq * 1000); /* Check if policy->min <= new_freq <= policy->max */ if(freq < (policy->min * 1000) || freq > (policy->max * 1000)) return -EINVAL;
This sanity checks that the frequency the system clock can generate is within the policy limits. It is initially feed the target_freq frequency.
This is already done in core.
These values are already calculated during init and added to the table, so one shouldn't need to perform the clk_round_rate() call anymore.
good.
pr_debug("cpufreq: requested frequency %u Hz\n", target_freq * 1000); freqs.new = (freq + 500) / 1000; freqs.flags = 0;
I don't know the details of how the cpufreq-core suggests frequencies. If it looks up in the frequency table, then we don't need this sanity check in .target().
Yes, it just picks the best match from freq table, though here is the actual implementation..
On 13 August 2013 14:39, Viresh Kumar viresh.kumar@linaro.org wrote:
Yes, that's what.. That generic routine can be used instead of your .verify()..
So here is a patch for that which I will include in my original series. It uses functions from this patch:
http://www.kernelhub.org/?msg=309940&p=2
-----------x-------------------x---------------------
commit e02892f73f8f669b265310e51a677c7253e32389 Author: Viresh Kumar viresh.kumar@linaro.org Date: Fri Aug 9 12:05:56 2013 +0530
cpufreq: at32ap: Use generic cpufreq routines
Most of the CPUFreq drivers do similar things in .exit() and .verify() routines and .attr. So its better if we have generic routines for them which can be used by cpufreq drivers then.
This patch uses these generic routines for this driver.
Cc: Hans-Christian Egtvedt egtvedt@samfundet.no Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/at32ap-cpufreq.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c index 1d19fa5..788f7e7 100644 --- a/drivers/cpufreq/at32ap-cpufreq.c +++ b/drivers/cpufreq/at32ap-cpufreq.c @@ -24,16 +24,6 @@ static struct clk *cpuclk; static struct cpufreq_frequency_table *freq_table;
-static int at32_verify_speed(struct cpufreq_policy *policy) -{ - if (policy->cpu != 0) - return -EINVAL; - - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq, - policy->cpuinfo.max_freq); - return 0; -} - static unsigned int at32_get_speed(unsigned int cpu) { /* No SMP support */ @@ -152,7 +142,7 @@ out_err: static struct cpufreq_driver at32_driver = { .name = "at32ap", .init = at32_cpufreq_driver_init, - .verify = at32_verify_speed, + .verify = cpufreq_generic_frequency_table_verify, .target = at32_set_target, .get = at32_get_speed, .flags = CPUFREQ_STICKY,
Around Tue 13 Aug 2013 19:00:22 +0530 or thereabout, Viresh Kumar wrote:
On 13 August 2013 14:39, Viresh Kumar viresh.kumar@linaro.org wrote:
Yes, that's what.. That generic routine can be used instead of your .verify()..
So here is a patch for that which I will include in my original series. It uses functions from this patch:
Great. Is there anything left for the at32ap cpufreq driver?
-----------x-------------------x---------------------
commit e02892f73f8f669b265310e51a677c7253e32389 Author: Viresh Kumar viresh.kumar@linaro.org Date: Fri Aug 9 12:05:56 2013 +0530
cpufreq: at32ap: Use generic cpufreq routines Most of the CPUFreq drivers do similar things in .exit() and
.verify() routines and .attr. So its better if we have generic routines for them which can be used by cpufreq drivers then.
This patch uses these generic routines for this driver. Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Hans-Christian Egtvedt egtvedt@samfundet.no
drivers/cpufreq/at32ap-cpufreq.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
<snipp diff>
On 14 August 2013 11:37, Hans-Christian Egtvedt egtvedt@samfundet.no wrote:
Around Tue 13 Aug 2013 19:00:22 +0530 or thereabout, Viresh Kumar wrote:
On 13 August 2013 14:39, Viresh Kumar viresh.kumar@linaro.org wrote:
Yes, that's what.. That generic routine can be used instead of your .verify()..
So here is a patch for that which I will include in my original series. It uses functions from this patch:
Great. Is there anything left for the at32ap cpufreq driver?
You are cc'd for one more patch which simplifies target()... Other than that nothing else for now.. But more will come soon :)
I am running a cpufreq cleanup drive :) ... That may end only once I have reduced code redundancy to the lowest possible level :)
Around Tue 13 Aug 2013 10:38:14 +0530 or thereabout, Viresh Kumar wrote:
On 13 August 2013 00:40, Hans-Christian Egtvedt egtvedt@samfundet.no wrote:
This patch adds a dynamically calculated frequency table to the at32ap driver. In short the architecture can scale in power of two between a maximum and minimum frequency. Min, max, and the steps in between are added to the table.
Signed-off-by: Hans-Christian Egtvedt egtvedt@samfundet.no
drivers/cpufreq/at32ap-cpufreq.c | 47 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-)
<snipp diff>
Thanks for your patch, I have folded below patch with your patch while applying..
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c index eaac7cb..c586d3e 100644 --- a/drivers/cpufreq/at32ap-cpufreq.c +++ b/drivers/cpufreq/at32ap-cpufreq.c @@ -87,10 +87,8 @@ static int at32_set_target(struct cpufreq_policy *policy,
static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy) {
unsigned int frequency;
int retval;
int steps;
int i;
unsigned int frequency, rate;
int retval, steps, i; if (policy->cpu != 0) return -EINVAL;
@@ -126,20 +124,23 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
frequency = policy->cpuinfo.max_freq; for (i = 0; i < (steps - 1); i++) {
unsigned int rate = clk_round_rate(cpuclk, frequency * 1000);
rate /= 1000;
rate = clk_round_rate(cpuclk, frequency * 1000) / 1000;
if (rate != frequency) freq_table[i].frequency = CPUFREQ_ENTRY_INVALID; else freq_table[i].frequency = frequency;
frequency /= 2; }
freq_table[steps - 1].frequency = CPUFREQ_TABLE_END; retval = cpufreq_frequency_table_cpuinfo(policy, freq_table); if (retval) goto out_err_kfree;
cpufreq_frequency_table_get_attr(freq_table, policy->cpu); printk("cpufreq: AT32AP CPU frequency driver\n"); return 0;
Acked-by: Hans-Christian Egtvedt egtvedt@samfundet.no
linaro-kernel@lists.linaro.org