The driver support single core and multi core ARM SoCs. For multi core, it assume all cores share the same clock and voltage.
Changes in v2: - add volatage change support - change '_' in property name to '-' - use initial value to calculate loops_per_jiffy - fix reading cpu_volts property bug - let cpufreq_frequency_table_cpuinfo routines handle cpu_freq_khz_max/min - don't change freq in arm_cpufreq_exit, because every core share the same freq. - use unsigned long describe frequency as much as possible. Because clk use unsigned long, but cpufreq use unsigned int.
[PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver [PATCH V2 2/4] dts/imx6q: add cpufreq property [PATCH V2 3/4] arm/imx6q: register arm_clk as cpu to clkdev [PATCH V2 4/4] arm/imx6q: select ARCH_HAS_CPUFREQ
Thanks Richard
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 72a0044..a0f7d35 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -2,6 +2,14 @@ # ARM CPU Frequency scaling drivers #
+config ARM_GENERIC_CPUFREQ + bool "ARM generic" + help + This adds ARM generic CPUFreq driver. It assumes all + cores of the SoC share the same clock and voltage. + + If in doubt, say N. + config ARM_S3C64XX_CPUFREQ bool "Samsung S3C64XX" depends on CPU_S3C6410 diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index ce75fcb..6ccf02d 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_X86_CPUFREQ_NFORCE2) += cpufreq-nforce2.o
################################################################################## # ARM SoC drivers +obj-$(CONFIG_ARM_GENERIC_CPUFREQ) += arm-cpufreq.o obj-$(CONFIG_UX500_SOC_DB8500) += db8500-cpufreq.o obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o obj-$(CONFIG_ARM_S5PV210_CPUFREQ) += s5pv210-cpufreq.o diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c new file mode 100644 index 0000000..e4d20da --- /dev/null +++ b/drivers/cpufreq/arm-cpufreq.c @@ -0,0 +1,269 @@ +/* + * Copyright (C) 2011 Freescale Semiconductor, Inc. + */ + +/* + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/module.h> +#include <linux/cpufreq.h> +#include <linux/clk.h> +#include <linux/regulator/consumer.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <asm/cpu.h> + +static u32 *cpu_freqs; /* HZ */ +static u32 *cpu_volts; /* uV */ +static u32 trans_latency; /* ns */ +static int cpu_op_nr; + +static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq; + +static struct clk *cpu_clk; +static struct regulator *cpu_reg; +static struct cpufreq_frequency_table *arm_freq_table; + +static int set_cpu_freq(unsigned long freq, int index, int higher) +{ + int ret = 0; + + if (higher && cpu_reg) + regulator_set_voltage(cpu_reg, + cpu_volts[index], cpu_volts[index]); + + ret = clk_set_rate(cpu_clk, freq); + if (ret != 0) { + printk(KERN_DEBUG "cannot set CPU clock rate\n"); + return ret; + } + + if (!higher && cpu_reg) + regulator_set_voltage(cpu_reg, + cpu_volts[index], cpu_volts[index]); + + return ret; +} + +static int arm_verify_speed(struct cpufreq_policy *policy) +{ + return cpufreq_frequency_table_verify(policy, arm_freq_table); +} + +static unsigned int arm_get_speed(unsigned int cpu) +{ + return clk_get_rate(cpu_clk) / 1000; +} + +static int arm_set_target(struct cpufreq_policy *policy, + unsigned int target_freq, unsigned int relation) +{ + struct cpufreq_freqs freqs; + unsigned long freq_Hz; + int cpu; + int ret = 0; + unsigned int index; + + cpufreq_frequency_table_target(policy, arm_freq_table, + target_freq, relation, &index); + freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]); + freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index]; + freqs.old = clk_get_rate(cpu_clk) / 1000; + freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]); + freqs.new = freq_Hz / 1000; + freqs.flags = 0; + + if (freqs.old == freqs.new) + return 0; + + for_each_possible_cpu(cpu) { + freqs.cpu = cpu; + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE); + } + + ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old)); + +#ifdef CONFIG_SMP + /* loops_per_jiffy is not updated by the cpufreq core for SMP systems. + * So update it for all CPUs. + */ + for_each_possible_cpu(cpu) + per_cpu(cpu_data, cpu).loops_per_jiffy = + cpufreq_scale(per_cpu(l_p_j_ref, cpu), l_p_j_ref_freq, + freqs.new); +#endif + + for_each_possible_cpu(cpu) { + freqs.cpu = cpu; + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE); + } + + return ret; +} + +static int arm_cpufreq_init(struct cpufreq_policy *policy) +{ + int ret; + + if (policy->cpu >= num_possible_cpus()) + return -EINVAL; + + policy->cur = clk_get_rate(cpu_clk) / 1000; + policy->shared_type = CPUFREQ_SHARED_TYPE_ANY; + cpumask_setall(policy->cpus); + /* Manual states, that PLL stabilizes in two CLK32 periods */ + policy->cpuinfo.transition_latency = trans_latency; + + ret = cpufreq_frequency_table_cpuinfo(policy, arm_freq_table); + + if (ret < 0) { + printk(KERN_ERR "%s: invalid frequency table for cpu %d\n", + __func__, policy->cpu); + return ret; + } + + cpufreq_frequency_table_get_attr(arm_freq_table, policy->cpu); + return 0; +} + +static int arm_cpufreq_exit(struct cpufreq_policy *policy) +{ + cpufreq_frequency_table_put_attr(policy->cpu); + return 0; +} + +static struct cpufreq_driver arm_cpufreq_driver = { + .flags = CPUFREQ_STICKY, + .verify = arm_verify_speed, + .target = arm_set_target, + .get = arm_get_speed, + .init = arm_cpufreq_init, + .exit = arm_cpufreq_exit, + .name = "arm", +}; + +static int __devinit arm_cpufreq_driver_init(void) +{ + struct device_node *cpu0; + const struct property *pp; + int cpu, i, ret; + + printk(KERN_INFO "ARM SoC generic CPU frequency driver\n"); + + cpu0 = of_find_node_by_path("/cpus/cpu@0"); + if (!cpu0) + return -ENODEV; + + pp = of_find_property(cpu0, "cpu-freqs", NULL); + if (!pp) { + ret = -ENODEV; + goto put_node; + } + cpu_op_nr = pp->length / sizeof(u32); + if (!cpu_op_nr) { + ret = -ENODEV; + goto put_node; + } + ret = -ENOMEM; + cpu_freqs = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr, GFP_KERNEL); + if (!cpu_freqs) + goto put_node; + of_property_read_u32_array(cpu0, "cpu-freqs", cpu_freqs, cpu_op_nr); + + pp = of_find_property(cpu0, "cpu-volts", NULL); + if (pp) { + if (cpu_op_nr == pp->length / sizeof(u32)) { + cpu_volts = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr, + GFP_KERNEL); + if (!cpu_volts) + goto free_cpu_freqs; + of_property_read_u32_array(cpu0, "cpu-volts", + cpu_volts, cpu_op_nr); + } else + printk(KERN_WARNING "cpufreq: invalid cpu_volts!\n"); + } + + if (of_property_read_u32(cpu0, "trans-latency", &trans_latency)) + trans_latency = CPUFREQ_ETERNAL; + + arm_freq_table = kmalloc(sizeof(struct cpufreq_frequency_table) + * (cpu_op_nr + 1), GFP_KERNEL); + if (!arm_freq_table) + goto free_cpu_volts; + + for (i = 0; i < cpu_op_nr; i++) { + arm_freq_table[i].index = i; + arm_freq_table[i].frequency = cpu_freqs[i] / 1000; + } + + arm_freq_table[i].index = i; + arm_freq_table[i].frequency = CPUFREQ_TABLE_END; + + cpu_clk = clk_get(NULL, "cpu"); + if (IS_ERR(cpu_clk)) { + printk(KERN_ERR "%s: failed to get cpu clock\n", __func__); + ret = PTR_ERR(cpu_clk); + goto free_freq_table; + } + + if (cpu_volts) { + cpu_reg = regulator_get(NULL, "cpu"); + if (IS_ERR(cpu_reg)) { + printk(KERN_WARNING + "cpufreq: regulator cpu get failed.\n"); + cpu_reg = NULL; + } + } + + l_p_j_ref_freq = clk_get_rate(cpu_clk); + for_each_possible_cpu(cpu) + per_cpu(l_p_j_ref, cpu) = + per_cpu(cpu_data, cpu).loops_per_jiffy; + + ret = cpufreq_register_driver(&arm_cpufreq_driver); + if (ret) + goto reg_put; + + of_node_put(cpu0); + + return 0; + +reg_put: + if (cpu_reg) + regulator_put(cpu_reg); + clk_put(cpu_clk); +free_freq_table: + kfree(arm_freq_table); +free_cpu_volts: + kfree(cpu_volts); +free_cpu_freqs: + kfree(cpu_freqs); +put_node: + of_node_put(cpu0); + + return ret; +} + +static void arm_cpufreq_driver_exit(void) +{ + cpufreq_unregister_driver(&arm_cpufreq_driver); + kfree(cpu_freqs); + kfree(cpu_volts); + kfree(arm_freq_table); + clk_put(cpu_clk); +} + +module_init(arm_cpufreq_driver_init); +module_exit(arm_cpufreq_driver_exit); + +MODULE_AUTHOR("Freescale Semiconductor Inc. Richard Zhao richard.zhao@freescale.com"); +MODULE_DESCRIPTION("ARM SoC generic CPUFreq driver"); +MODULE_LICENSE("GPL");
Hi Richard,
A couple of questions inline, but otherwise looks nice!
Jamie
On Fri, Dec 16, 2011 at 06:30:59PM +0800, Richard Zhao wrote:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
[...]
diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c new file mode 100644 index 0000000..e4d20da --- /dev/null +++ b/drivers/cpufreq/arm-cpufreq.c @@ -0,0 +1,269 @@ +/*
- Copyright (C) 2011 Freescale Semiconductor, Inc.
- */
+/*
- The code contained herein is licensed under the GNU General Public
- License. You may obtain a copy of the GNU General Public License
- Version 2 or later at the following locations:
- */
+#include <linux/module.h> +#include <linux/cpufreq.h> +#include <linux/clk.h> +#include <linux/regulator/consumer.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <asm/cpu.h>
+static u32 *cpu_freqs; /* HZ */ +static u32 *cpu_volts; /* uV */ +static u32 trans_latency; /* ns */ +static int cpu_op_nr;
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk;
This assumes that all CPU's share the same clk and run at the same rate. Is that a fair/safe assumption? I honestly don't know the answer to this so it's just a question!!!
+static struct regulator *cpu_reg; +static struct cpufreq_frequency_table *arm_freq_table;
+static int set_cpu_freq(unsigned long freq, int index, int higher) +{
- int ret = 0;
- if (higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- ret = clk_set_rate(cpu_clk, freq);
- if (ret != 0) {
printk(KERN_DEBUG "cannot set CPU clock rate\n");
Perhaps use pr_debug() and friends throughout this driver?
return ret;- }
- if (!higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- return ret;
+}
+static int arm_verify_speed(struct cpufreq_policy *policy) +{
- return cpufreq_frequency_table_verify(policy, arm_freq_table);
+}
+static unsigned int arm_get_speed(unsigned int cpu) +{
- return clk_get_rate(cpu_clk) / 1000;
+}
+static int arm_set_target(struct cpufreq_policy *policy,
unsigned int target_freq, unsigned int relation)+{
- struct cpufreq_freqs freqs;
- unsigned long freq_Hz;
- int cpu;
- int ret = 0;
- unsigned int index;
- cpufreq_frequency_table_target(policy, arm_freq_table,
target_freq, relation, &index);- freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index];
- freqs.old = clk_get_rate(cpu_clk) / 1000;
- freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freqs.new = freq_Hz / 1000;
Why round the rate then overwrite it? Should this be freqs.new /= 1000?
- freqs.flags = 0;
- if (freqs.old == freqs.new)
return 0;- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);- }
- ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old));
+#ifdef CONFIG_SMP
- /* loops_per_jiffy is not updated by the cpufreq core for SMP systems.
* So update it for all CPUs.*/- for_each_possible_cpu(cpu)
per_cpu(cpu_data, cpu).loops_per_jiffy =cpufreq_scale(per_cpu(l_p_j_ref, cpu), l_p_j_ref_freq,freqs.new);+#endif
- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);- }
- return ret;
+}
+static int arm_cpufreq_init(struct cpufreq_policy *policy) +{
- int ret;
- if (policy->cpu >= num_possible_cpus())
return -EINVAL;- policy->cur = clk_get_rate(cpu_clk) / 1000;
- policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
- cpumask_setall(policy->cpus);
- /* Manual states, that PLL stabilizes in two CLK32 periods */
- policy->cpuinfo.transition_latency = trans_latency;
- ret = cpufreq_frequency_table_cpuinfo(policy, arm_freq_table);
- if (ret < 0) {
printk(KERN_ERR "%s: invalid frequency table for cpu %d\n",__func__, policy->cpu);return ret;- }
- cpufreq_frequency_table_get_attr(arm_freq_table, policy->cpu);
- return 0;
+}
+static int arm_cpufreq_exit(struct cpufreq_policy *policy) +{
- cpufreq_frequency_table_put_attr(policy->cpu);
- return 0;
+}
+static struct cpufreq_driver arm_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = arm_verify_speed,
- .target = arm_set_target,
- .get = arm_get_speed,
- .init = arm_cpufreq_init,
- .exit = arm_cpufreq_exit,
- .name = "arm",
Is this really just for ARM or can it be a generic-clk driver? I can't see any ARM specifics here.
+};
+static int __devinit arm_cpufreq_driver_init(void) +{
- struct device_node *cpu0;
- const struct property *pp;
- int cpu, i, ret;
- printk(KERN_INFO "ARM SoC generic CPU frequency driver\n");
- cpu0 = of_find_node_by_path("/cpus/cpu@0");
- if (!cpu0)
return -ENODEV;- pp = of_find_property(cpu0, "cpu-freqs", NULL);
- if (!pp) {
ret = -ENODEV;goto put_node;- }
- cpu_op_nr = pp->length / sizeof(u32);
- if (!cpu_op_nr) {
ret = -ENODEV;goto put_node;- }
- ret = -ENOMEM;
- cpu_freqs = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr, GFP_KERNEL);
- if (!cpu_freqs)
goto put_node;- of_property_read_u32_array(cpu0, "cpu-freqs", cpu_freqs, cpu_op_nr);
- pp = of_find_property(cpu0, "cpu-volts", NULL);
- if (pp) {
if (cpu_op_nr == pp->length / sizeof(u32)) {cpu_volts = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr,GFP_KERNEL);if (!cpu_volts)goto free_cpu_freqs;of_property_read_u32_array(cpu0, "cpu-volts",cpu_volts, cpu_op_nr);} elseprintk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");- }
- if (of_property_read_u32(cpu0, "trans-latency", &trans_latency))
trans_latency = CPUFREQ_ETERNAL;- arm_freq_table = kmalloc(sizeof(struct cpufreq_frequency_table)
* (cpu_op_nr + 1), GFP_KERNEL);- if (!arm_freq_table)
goto free_cpu_volts;- for (i = 0; i < cpu_op_nr; i++) {
arm_freq_table[i].index = i;arm_freq_table[i].frequency = cpu_freqs[i] / 1000;- }
- arm_freq_table[i].index = i;
- arm_freq_table[i].frequency = CPUFREQ_TABLE_END;
- cpu_clk = clk_get(NULL, "cpu");
- if (IS_ERR(cpu_clk)) {
printk(KERN_ERR "%s: failed to get cpu clock\n", __func__);ret = PTR_ERR(cpu_clk);goto free_freq_table;- }
Should there be a clk_prepare() + clk_enable() pair here? I can't see it would really be needed but maybe for completeness? Again, just a question!
- if (cpu_volts) {
cpu_reg = regulator_get(NULL, "cpu");if (IS_ERR(cpu_reg)) {printk(KERN_WARNING"cpufreq: regulator cpu get failed.\n");cpu_reg = NULL;}- }
- l_p_j_ref_freq = clk_get_rate(cpu_clk);
- for_each_possible_cpu(cpu)
per_cpu(l_p_j_ref, cpu) =per_cpu(cpu_data, cpu).loops_per_jiffy;- ret = cpufreq_register_driver(&arm_cpufreq_driver);
- if (ret)
goto reg_put;- of_node_put(cpu0);
- return 0;
+reg_put:
- if (cpu_reg)
regulator_put(cpu_reg);- clk_put(cpu_clk);
+free_freq_table:
- kfree(arm_freq_table);
+free_cpu_volts:
- kfree(cpu_volts);
+free_cpu_freqs:
- kfree(cpu_freqs);
+put_node:
- of_node_put(cpu0);
- return ret;
+}
+static void arm_cpufreq_driver_exit(void) +{
- cpufreq_unregister_driver(&arm_cpufreq_driver);
- kfree(cpu_freqs);
- kfree(cpu_volts);
- kfree(arm_freq_table);
- clk_put(cpu_clk);
+}
+module_init(arm_cpufreq_driver_init); +module_exit(arm_cpufreq_driver_exit);
Are there any ARM platforms that wouldn't be able to use this driver? If there are then should platforms "opt-in" by calling a register function rather than having it auto registering as when we have multiple platforms in a single zImage the probe errors might not be too nice.
+MODULE_AUTHOR("Freescale Semiconductor Inc. Richard Zhao richard.zhao@freescale.com"); +MODULE_DESCRIPTION("ARM SoC generic CPUFreq driver");
+MODULE_LICENSE("GPL");
1.7.5.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 12/16/2011 02:52 AM, Jamie Iles wrote:
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk;
This assumes that all CPU's share the same clk and run at the same rate. Is that a fair/safe assumption? I honestly don't know the answer to this so it's just a question!!!
On MSM, cpus independently scale both frequency and voltage. Our clock driver isn't upstream yet. David Brown has a preliminary version here:
https://www.codeaurora.org/gitweb/quic/kernel/?p=davidb/linux-msm.git%3Ba=sh...
Once we get our driver upstream, MSM will be an exception and not select ARM_GENERIC_CPUFREQ. We'll probably have a separate msm-cpufreq.c driver under drivers/cpufreq/.
- Bryan
On Fri, Dec 16, 2011 at 11:59:02AM -0800, Bryan Huntsman wrote:
On 12/16/2011 02:52 AM, Jamie Iles wrote:
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk;
This assumes that all CPU's share the same clk and run at the same rate. Is that a fair/safe assumption? I honestly don't know the answer to this so it's just a question!!!
On MSM, cpus independently scale both frequency and voltage. Our clock driver isn't upstream yet. David Brown has a preliminary version here:
If - cpu_clk are per_cpu, and get from cpu0 clk, cpu1 clk, etc. The same to regulators. - set CPUFREQ_SHARED_TYPE_NONE - don't set affect cpus.
Does that help you?
Thanks Richard
https://www.codeaurora.org/gitweb/quic/kernel/?p=davidb/linux-msm.git%3Ba=sh...
Once we get our driver upstream, MSM will be an exception and not select ARM_GENERIC_CPUFREQ. We'll probably have a separate msm-cpufreq.c driver under drivers/cpufreq/.
- Bryan
-- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Hi Bryan,
On Fri, Dec 16, 2011 at 11:59:02AM -0800, Bryan Huntsman wrote:
On 12/16/2011 02:52 AM, Jamie Iles wrote:
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk;
This assumes that all CPU's share the same clk and run at the same rate. Is that a fair/safe assumption? I honestly don't know the answer to this so it's just a question!!!
On MSM, cpus independently scale both frequency and voltage. Our clock driver isn't upstream yet. David Brown has a preliminary version here:
https://www.codeaurora.org/gitweb/quic/kernel/?p=davidb/linux-msm.git%3Ba=sh...
Once we get our driver upstream, MSM will be an exception and not select ARM_GENERIC_CPUFREQ. We'll probably have a separate msm-cpufreq.c driver under drivers/cpufreq/.
Do you have to patch to implement per-cpu udelay? In current code, udelay uses global loops_per_jiffy.
Thanks Richard
- Bryan
-- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 12/18/11 17:03, Richard Zhao wrote:
Do you have to patch to implement per-cpu udelay? In current code, udelay uses global loops_per_jiffy.
We've been carrying forward the timer based udelay patches. They're in the patch tracker as 6873/1, 6874/1, and 6875/1.
On Fri, Dec 16, 2011 at 10:52:29AM +0000, Jamie Iles wrote:
Hi Richard,
A couple of questions inline, but otherwise looks nice!
Thanks for your review.
Jamie
On Fri, Dec 16, 2011 at 06:30:59PM +0800, Richard Zhao wrote:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
[...]
diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c new file mode 100644 index 0000000..e4d20da --- /dev/null +++ b/drivers/cpufreq/arm-cpufreq.c @@ -0,0 +1,269 @@ +/*
- Copyright (C) 2011 Freescale Semiconductor, Inc.
- */
+/*
- The code contained herein is licensed under the GNU General Public
- License. You may obtain a copy of the GNU General Public License
- Version 2 or later at the following locations:
- */
+#include <linux/module.h> +#include <linux/cpufreq.h> +#include <linux/clk.h> +#include <linux/regulator/consumer.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <asm/cpu.h>
+static u32 *cpu_freqs; /* HZ */ +static u32 *cpu_volts; /* uV */ +static u32 trans_latency; /* ns */ +static int cpu_op_nr;
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk;
This assumes that all CPU's share the same clk and run at the same rate. Is that a fair/safe assumption? I honestly don't know the answer to this so it's just a question!!!
As I know, most share the same clk/volt. From the code: IMX6: yes Tegra: Yes, but strange it sets CPUFREQ_SHARED_TYPE_ALL.
MSM is an exception. I can support the case, but I have to make sure it's handy to use.
+static struct regulator *cpu_reg; +static struct cpufreq_frequency_table *arm_freq_table;
+static int set_cpu_freq(unsigned long freq, int index, int higher) +{
- int ret = 0;
- if (higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- ret = clk_set_rate(cpu_clk, freq);
- if (ret != 0) {
printk(KERN_DEBUG "cannot set CPU clock rate\n");Perhaps use pr_debug() and friends throughout this driver?
ok. Thanks.
return ret;- }
- if (!higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- return ret;
+}
+static int arm_verify_speed(struct cpufreq_policy *policy) +{
- return cpufreq_frequency_table_verify(policy, arm_freq_table);
+}
+static unsigned int arm_get_speed(unsigned int cpu) +{
- return clk_get_rate(cpu_clk) / 1000;
+}
+static int arm_set_target(struct cpufreq_policy *policy,
unsigned int target_freq, unsigned int relation)+{
- struct cpufreq_freqs freqs;
- unsigned long freq_Hz;
- int cpu;
- int ret = 0;
- unsigned int index;
- cpufreq_frequency_table_target(policy, arm_freq_table,
target_freq, relation, &index);- freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index];
- freqs.old = clk_get_rate(cpu_clk) / 1000;
- freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]);
I forgot to delete this line.
- freqs.new = freq_Hz / 1000;
Why round the rate then overwrite it? Should this be freqs.new /= 1000?
- freqs.flags = 0;
- if (freqs.old == freqs.new)
return 0;- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);- }
- ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old));
+#ifdef CONFIG_SMP
- /* loops_per_jiffy is not updated by the cpufreq core for SMP systems.
* So update it for all CPUs.*/- for_each_possible_cpu(cpu)
per_cpu(cpu_data, cpu).loops_per_jiffy =cpufreq_scale(per_cpu(l_p_j_ref, cpu), l_p_j_ref_freq,freqs.new);+#endif
- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);- }
- return ret;
+}
+static int arm_cpufreq_init(struct cpufreq_policy *policy) +{
- int ret;
- if (policy->cpu >= num_possible_cpus())
return -EINVAL;- policy->cur = clk_get_rate(cpu_clk) / 1000;
- policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
- cpumask_setall(policy->cpus);
- /* Manual states, that PLL stabilizes in two CLK32 periods */
- policy->cpuinfo.transition_latency = trans_latency;
- ret = cpufreq_frequency_table_cpuinfo(policy, arm_freq_table);
- if (ret < 0) {
printk(KERN_ERR "%s: invalid frequency table for cpu %d\n",__func__, policy->cpu);return ret;- }
- cpufreq_frequency_table_get_attr(arm_freq_table, policy->cpu);
- return 0;
+}
+static int arm_cpufreq_exit(struct cpufreq_policy *policy) +{
- cpufreq_frequency_table_put_attr(policy->cpu);
- return 0;
+}
+static struct cpufreq_driver arm_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = arm_verify_speed,
- .target = arm_set_target,
- .get = arm_get_speed,
- .init = arm_cpufreq_init,
- .exit = arm_cpufreq_exit,
- .name = "arm",
Is this really just for ARM or can it be a generic-clk driver? I can't see any ARM specifics here.
If we make recalculating smp loops_per_jiffy portable, this driver will be portable too.
+};
+static int __devinit arm_cpufreq_driver_init(void) +{
- struct device_node *cpu0;
- const struct property *pp;
- int cpu, i, ret;
- printk(KERN_INFO "ARM SoC generic CPU frequency driver\n");
- cpu0 = of_find_node_by_path("/cpus/cpu@0");
- if (!cpu0)
return -ENODEV;- pp = of_find_property(cpu0, "cpu-freqs", NULL);
- if (!pp) {
ret = -ENODEV;goto put_node;- }
- cpu_op_nr = pp->length / sizeof(u32);
- if (!cpu_op_nr) {
ret = -ENODEV;goto put_node;- }
- ret = -ENOMEM;
- cpu_freqs = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr, GFP_KERNEL);
- if (!cpu_freqs)
goto put_node;- of_property_read_u32_array(cpu0, "cpu-freqs", cpu_freqs, cpu_op_nr);
- pp = of_find_property(cpu0, "cpu-volts", NULL);
- if (pp) {
if (cpu_op_nr == pp->length / sizeof(u32)) {cpu_volts = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr,GFP_KERNEL);if (!cpu_volts)goto free_cpu_freqs;of_property_read_u32_array(cpu0, "cpu-volts",cpu_volts, cpu_op_nr);} elseprintk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");- }
- if (of_property_read_u32(cpu0, "trans-latency", &trans_latency))
trans_latency = CPUFREQ_ETERNAL;- arm_freq_table = kmalloc(sizeof(struct cpufreq_frequency_table)
* (cpu_op_nr + 1), GFP_KERNEL);- if (!arm_freq_table)
goto free_cpu_volts;- for (i = 0; i < cpu_op_nr; i++) {
arm_freq_table[i].index = i;arm_freq_table[i].frequency = cpu_freqs[i] / 1000;- }
- arm_freq_table[i].index = i;
- arm_freq_table[i].frequency = CPUFREQ_TABLE_END;
- cpu_clk = clk_get(NULL, "cpu");
- if (IS_ERR(cpu_clk)) {
printk(KERN_ERR "%s: failed to get cpu clock\n", __func__);ret = PTR_ERR(cpu_clk);goto free_freq_table;- }
Should there be a clk_prepare() + clk_enable() pair here? I can't see it would really be needed but maybe for completeness? Again, just a question!
The cpu clock should already be prepared/enabled. The same to the regulator. IMHO, cpufreq is not used to handle cpu clk/power gating. The gating should be handled by cpu hotplug, idle and suspend/resume. Please correct me if I'm wrong.
- if (cpu_volts) {
cpu_reg = regulator_get(NULL, "cpu");if (IS_ERR(cpu_reg)) {printk(KERN_WARNING"cpufreq: regulator cpu get failed.\n");cpu_reg = NULL;}- }
- l_p_j_ref_freq = clk_get_rate(cpu_clk);
- for_each_possible_cpu(cpu)
per_cpu(l_p_j_ref, cpu) =per_cpu(cpu_data, cpu).loops_per_jiffy;- ret = cpufreq_register_driver(&arm_cpufreq_driver);
- if (ret)
goto reg_put;- of_node_put(cpu0);
- return 0;
+reg_put:
- if (cpu_reg)
regulator_put(cpu_reg);- clk_put(cpu_clk);
+free_freq_table:
- kfree(arm_freq_table);
+free_cpu_volts:
- kfree(cpu_volts);
+free_cpu_freqs:
- kfree(cpu_freqs);
+put_node:
- of_node_put(cpu0);
- return ret;
+}
+static void arm_cpufreq_driver_exit(void) +{
- cpufreq_unregister_driver(&arm_cpufreq_driver);
- kfree(cpu_freqs);
- kfree(cpu_volts);
- kfree(arm_freq_table);
- clk_put(cpu_clk);
+}
+module_init(arm_cpufreq_driver_init); +module_exit(arm_cpufreq_driver_exit);
Are there any ARM platforms that wouldn't be able to use this driver? If there are then should platforms "opt-in" by calling a register function rather than having it auto registering as when we have multiple platforms in a single zImage the probe errors might not be too nice.
Good point. But would it be better to check cpu node compatible property?
Thanks Richard
+MODULE_AUTHOR("Freescale Semiconductor Inc. Richard Zhao richard.zhao@freescale.com"); +MODULE_DESCRIPTION("ARM SoC generic CPUFreq driver");
+MODULE_LICENSE("GPL");
1.7.5.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Am Freitag, 16. Dezember 2011, 11:30:59 schrieb Richard Zhao:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
looks quite cool, but should also provide a description of the devicetree bindings (i.e. like the rest in Documentation/devicetree/bindings/...) for future reference.
Heiko
On Fri, Dec 16, 2011 at 12:26:03PM +0100, Heiko Stübner wrote:
Am Freitag, 16. Dezember 2011, 11:30:59 schrieb Richard Zhao:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
looks quite cool, but should also provide a description of the devicetree bindings (i.e. like the rest in Documentation/devicetree/bindings/...) for future reference.
Yes, Thanks for your remind.
Heiko
On 12/16/2011 04:30 AM, Richard Zhao wrote:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
What makes this specific to ARM and not a generic DT + clk api + regulator api driver?
Also, you need documentation of the DT bindings.
Rob
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 72a0044..a0f7d35 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -2,6 +2,14 @@ # ARM CPU Frequency scaling drivers # +config ARM_GENERIC_CPUFREQ
- bool "ARM generic"
- help
This adds ARM generic CPUFreq driver. It assumes allcores of the SoC share the same clock and voltage.If in doubt, say N.config ARM_S3C64XX_CPUFREQ bool "Samsung S3C64XX" depends on CPU_S3C6410 diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index ce75fcb..6ccf02d 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_X86_CPUFREQ_NFORCE2) += cpufreq-nforce2.o ################################################################################## # ARM SoC drivers +obj-$(CONFIG_ARM_GENERIC_CPUFREQ) += arm-cpufreq.o obj-$(CONFIG_UX500_SOC_DB8500) += db8500-cpufreq.o obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o obj-$(CONFIG_ARM_S5PV210_CPUFREQ) += s5pv210-cpufreq.o diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c new file mode 100644 index 0000000..e4d20da --- /dev/null +++ b/drivers/cpufreq/arm-cpufreq.c @@ -0,0 +1,269 @@ +/*
- Copyright (C) 2011 Freescale Semiconductor, Inc.
- */
+/*
- The code contained herein is licensed under the GNU General Public
- License. You may obtain a copy of the GNU General Public License
- Version 2 or later at the following locations:
- */
+#include <linux/module.h> +#include <linux/cpufreq.h> +#include <linux/clk.h> +#include <linux/regulator/consumer.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <asm/cpu.h>
+static u32 *cpu_freqs; /* HZ */ +static u32 *cpu_volts; /* uV */ +static u32 trans_latency; /* ns */ +static int cpu_op_nr;
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk; +static struct regulator *cpu_reg; +static struct cpufreq_frequency_table *arm_freq_table;
+static int set_cpu_freq(unsigned long freq, int index, int higher) +{
- int ret = 0;
- if (higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- ret = clk_set_rate(cpu_clk, freq);
- if (ret != 0) {
printk(KERN_DEBUG "cannot set CPU clock rate\n");return ret;- }
- if (!higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- return ret;
+}
+static int arm_verify_speed(struct cpufreq_policy *policy) +{
- return cpufreq_frequency_table_verify(policy, arm_freq_table);
+}
+static unsigned int arm_get_speed(unsigned int cpu) +{
- return clk_get_rate(cpu_clk) / 1000;
+}
+static int arm_set_target(struct cpufreq_policy *policy,
unsigned int target_freq, unsigned int relation)+{
- struct cpufreq_freqs freqs;
- unsigned long freq_Hz;
- int cpu;
- int ret = 0;
- unsigned int index;
- cpufreq_frequency_table_target(policy, arm_freq_table,
target_freq, relation, &index);- freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index];
- freqs.old = clk_get_rate(cpu_clk) / 1000;
- freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freqs.new = freq_Hz / 1000;
- freqs.flags = 0;
- if (freqs.old == freqs.new)
return 0;- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);- }
- ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old));
+#ifdef CONFIG_SMP
- /* loops_per_jiffy is not updated by the cpufreq core for SMP systems.
* So update it for all CPUs.*/- for_each_possible_cpu(cpu)
per_cpu(cpu_data, cpu).loops_per_jiffy =cpufreq_scale(per_cpu(l_p_j_ref, cpu), l_p_j_ref_freq,freqs.new);+#endif
- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);- }
- return ret;
+}
+static int arm_cpufreq_init(struct cpufreq_policy *policy) +{
- int ret;
- if (policy->cpu >= num_possible_cpus())
return -EINVAL;- policy->cur = clk_get_rate(cpu_clk) / 1000;
- policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
- cpumask_setall(policy->cpus);
- /* Manual states, that PLL stabilizes in two CLK32 periods */
- policy->cpuinfo.transition_latency = trans_latency;
- ret = cpufreq_frequency_table_cpuinfo(policy, arm_freq_table);
- if (ret < 0) {
printk(KERN_ERR "%s: invalid frequency table for cpu %d\n",__func__, policy->cpu);return ret;- }
- cpufreq_frequency_table_get_attr(arm_freq_table, policy->cpu);
- return 0;
+}
+static int arm_cpufreq_exit(struct cpufreq_policy *policy) +{
- cpufreq_frequency_table_put_attr(policy->cpu);
- return 0;
+}
+static struct cpufreq_driver arm_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = arm_verify_speed,
- .target = arm_set_target,
- .get = arm_get_speed,
- .init = arm_cpufreq_init,
- .exit = arm_cpufreq_exit,
- .name = "arm",
+};
+static int __devinit arm_cpufreq_driver_init(void) +{
- struct device_node *cpu0;
- const struct property *pp;
- int cpu, i, ret;
- printk(KERN_INFO "ARM SoC generic CPU frequency driver\n");
- cpu0 = of_find_node_by_path("/cpus/cpu@0");
- if (!cpu0)
return -ENODEV;- pp = of_find_property(cpu0, "cpu-freqs", NULL);
- if (!pp) {
ret = -ENODEV;goto put_node;- }
- cpu_op_nr = pp->length / sizeof(u32);
- if (!cpu_op_nr) {
ret = -ENODEV;goto put_node;- }
- ret = -ENOMEM;
- cpu_freqs = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr, GFP_KERNEL);
- if (!cpu_freqs)
goto put_node;- of_property_read_u32_array(cpu0, "cpu-freqs", cpu_freqs, cpu_op_nr);
- pp = of_find_property(cpu0, "cpu-volts", NULL);
- if (pp) {
if (cpu_op_nr == pp->length / sizeof(u32)) {cpu_volts = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr,GFP_KERNEL);if (!cpu_volts)goto free_cpu_freqs;of_property_read_u32_array(cpu0, "cpu-volts",cpu_volts, cpu_op_nr);} elseprintk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");- }
- if (of_property_read_u32(cpu0, "trans-latency", &trans_latency))
trans_latency = CPUFREQ_ETERNAL;- arm_freq_table = kmalloc(sizeof(struct cpufreq_frequency_table)
* (cpu_op_nr + 1), GFP_KERNEL);- if (!arm_freq_table)
goto free_cpu_volts;- for (i = 0; i < cpu_op_nr; i++) {
arm_freq_table[i].index = i;arm_freq_table[i].frequency = cpu_freqs[i] / 1000;- }
- arm_freq_table[i].index = i;
- arm_freq_table[i].frequency = CPUFREQ_TABLE_END;
- cpu_clk = clk_get(NULL, "cpu");
- if (IS_ERR(cpu_clk)) {
printk(KERN_ERR "%s: failed to get cpu clock\n", __func__);ret = PTR_ERR(cpu_clk);goto free_freq_table;- }
- if (cpu_volts) {
cpu_reg = regulator_get(NULL, "cpu");if (IS_ERR(cpu_reg)) {printk(KERN_WARNING"cpufreq: regulator cpu get failed.\n");cpu_reg = NULL;}- }
- l_p_j_ref_freq = clk_get_rate(cpu_clk);
- for_each_possible_cpu(cpu)
per_cpu(l_p_j_ref, cpu) =per_cpu(cpu_data, cpu).loops_per_jiffy;- ret = cpufreq_register_driver(&arm_cpufreq_driver);
- if (ret)
goto reg_put;- of_node_put(cpu0);
- return 0;
+reg_put:
- if (cpu_reg)
regulator_put(cpu_reg);- clk_put(cpu_clk);
+free_freq_table:
- kfree(arm_freq_table);
+free_cpu_volts:
- kfree(cpu_volts);
+free_cpu_freqs:
- kfree(cpu_freqs);
+put_node:
- of_node_put(cpu0);
- return ret;
+}
+static void arm_cpufreq_driver_exit(void) +{
- cpufreq_unregister_driver(&arm_cpufreq_driver);
- kfree(cpu_freqs);
- kfree(cpu_volts);
- kfree(arm_freq_table);
- clk_put(cpu_clk);
+}
+module_init(arm_cpufreq_driver_init); +module_exit(arm_cpufreq_driver_exit);
+MODULE_AUTHOR("Freescale Semiconductor Inc. Richard Zhao richard.zhao@freescale.com"); +MODULE_DESCRIPTION("ARM SoC generic CPUFreq driver"); +MODULE_LICENSE("GPL");
On Fri, Dec 16, 2011 at 08:32:35AM -0600, Rob Herring wrote:
On 12/16/2011 04:30 AM, Richard Zhao wrote:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
What makes this specific to ARM and not a generic DT + clk api + regulator api driver?
smp loops_per_jiffy update needs arm header <asm/cpu.h>.
Also, you need documentation of the DT bindings.
Yes, Thanks.
Richard
Rob
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 72a0044..a0f7d35 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -2,6 +2,14 @@ # ARM CPU Frequency scaling drivers # +config ARM_GENERIC_CPUFREQ
- bool "ARM generic"
- help
This adds ARM generic CPUFreq driver. It assumes allcores of the SoC share the same clock and voltage.If in doubt, say N.config ARM_S3C64XX_CPUFREQ bool "Samsung S3C64XX" depends on CPU_S3C6410 diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index ce75fcb..6ccf02d 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_X86_CPUFREQ_NFORCE2) += cpufreq-nforce2.o ################################################################################## # ARM SoC drivers +obj-$(CONFIG_ARM_GENERIC_CPUFREQ) += arm-cpufreq.o obj-$(CONFIG_UX500_SOC_DB8500) += db8500-cpufreq.o obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o obj-$(CONFIG_ARM_S5PV210_CPUFREQ) += s5pv210-cpufreq.o diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c new file mode 100644 index 0000000..e4d20da --- /dev/null +++ b/drivers/cpufreq/arm-cpufreq.c @@ -0,0 +1,269 @@ +/*
- Copyright (C) 2011 Freescale Semiconductor, Inc.
- */
+/*
- The code contained herein is licensed under the GNU General Public
- License. You may obtain a copy of the GNU General Public License
- Version 2 or later at the following locations:
- */
+#include <linux/module.h> +#include <linux/cpufreq.h> +#include <linux/clk.h> +#include <linux/regulator/consumer.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <asm/cpu.h>
+static u32 *cpu_freqs; /* HZ */ +static u32 *cpu_volts; /* uV */ +static u32 trans_latency; /* ns */ +static int cpu_op_nr;
+static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static unsigned long l_p_j_ref_freq;
+static struct clk *cpu_clk; +static struct regulator *cpu_reg; +static struct cpufreq_frequency_table *arm_freq_table;
+static int set_cpu_freq(unsigned long freq, int index, int higher) +{
- int ret = 0;
- if (higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- ret = clk_set_rate(cpu_clk, freq);
- if (ret != 0) {
printk(KERN_DEBUG "cannot set CPU clock rate\n");return ret;- }
- if (!higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- return ret;
+}
+static int arm_verify_speed(struct cpufreq_policy *policy) +{
- return cpufreq_frequency_table_verify(policy, arm_freq_table);
+}
+static unsigned int arm_get_speed(unsigned int cpu) +{
- return clk_get_rate(cpu_clk) / 1000;
+}
+static int arm_set_target(struct cpufreq_policy *policy,
unsigned int target_freq, unsigned int relation)+{
- struct cpufreq_freqs freqs;
- unsigned long freq_Hz;
- int cpu;
- int ret = 0;
- unsigned int index;
- cpufreq_frequency_table_target(policy, arm_freq_table,
target_freq, relation, &index);- freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index];
- freqs.old = clk_get_rate(cpu_clk) / 1000;
- freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]);
- freqs.new = freq_Hz / 1000;
- freqs.flags = 0;
- if (freqs.old == freqs.new)
return 0;- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);- }
- ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old));
+#ifdef CONFIG_SMP
- /* loops_per_jiffy is not updated by the cpufreq core for SMP systems.
* So update it for all CPUs.*/- for_each_possible_cpu(cpu)
per_cpu(cpu_data, cpu).loops_per_jiffy =cpufreq_scale(per_cpu(l_p_j_ref, cpu), l_p_j_ref_freq,freqs.new);+#endif
- for_each_possible_cpu(cpu) {
freqs.cpu = cpu;cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);- }
- return ret;
+}
+static int arm_cpufreq_init(struct cpufreq_policy *policy) +{
- int ret;
- if (policy->cpu >= num_possible_cpus())
return -EINVAL;- policy->cur = clk_get_rate(cpu_clk) / 1000;
- policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
- cpumask_setall(policy->cpus);
- /* Manual states, that PLL stabilizes in two CLK32 periods */
- policy->cpuinfo.transition_latency = trans_latency;
- ret = cpufreq_frequency_table_cpuinfo(policy, arm_freq_table);
- if (ret < 0) {
printk(KERN_ERR "%s: invalid frequency table for cpu %d\n",__func__, policy->cpu);return ret;- }
- cpufreq_frequency_table_get_attr(arm_freq_table, policy->cpu);
- return 0;
+}
+static int arm_cpufreq_exit(struct cpufreq_policy *policy) +{
- cpufreq_frequency_table_put_attr(policy->cpu);
- return 0;
+}
+static struct cpufreq_driver arm_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = arm_verify_speed,
- .target = arm_set_target,
- .get = arm_get_speed,
- .init = arm_cpufreq_init,
- .exit = arm_cpufreq_exit,
- .name = "arm",
+};
+static int __devinit arm_cpufreq_driver_init(void) +{
- struct device_node *cpu0;
- const struct property *pp;
- int cpu, i, ret;
- printk(KERN_INFO "ARM SoC generic CPU frequency driver\n");
- cpu0 = of_find_node_by_path("/cpus/cpu@0");
- if (!cpu0)
return -ENODEV;- pp = of_find_property(cpu0, "cpu-freqs", NULL);
- if (!pp) {
ret = -ENODEV;goto put_node;- }
- cpu_op_nr = pp->length / sizeof(u32);
- if (!cpu_op_nr) {
ret = -ENODEV;goto put_node;- }
- ret = -ENOMEM;
- cpu_freqs = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr, GFP_KERNEL);
- if (!cpu_freqs)
goto put_node;- of_property_read_u32_array(cpu0, "cpu-freqs", cpu_freqs, cpu_op_nr);
- pp = of_find_property(cpu0, "cpu-volts", NULL);
- if (pp) {
if (cpu_op_nr == pp->length / sizeof(u32)) {cpu_volts = kzalloc(sizeof(*cpu_freqs) * cpu_op_nr,GFP_KERNEL);if (!cpu_volts)goto free_cpu_freqs;of_property_read_u32_array(cpu0, "cpu-volts",cpu_volts, cpu_op_nr);} elseprintk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");- }
- if (of_property_read_u32(cpu0, "trans-latency", &trans_latency))
trans_latency = CPUFREQ_ETERNAL;- arm_freq_table = kmalloc(sizeof(struct cpufreq_frequency_table)
* (cpu_op_nr + 1), GFP_KERNEL);- if (!arm_freq_table)
goto free_cpu_volts;- for (i = 0; i < cpu_op_nr; i++) {
arm_freq_table[i].index = i;arm_freq_table[i].frequency = cpu_freqs[i] / 1000;- }
- arm_freq_table[i].index = i;
- arm_freq_table[i].frequency = CPUFREQ_TABLE_END;
- cpu_clk = clk_get(NULL, "cpu");
- if (IS_ERR(cpu_clk)) {
printk(KERN_ERR "%s: failed to get cpu clock\n", __func__);ret = PTR_ERR(cpu_clk);goto free_freq_table;- }
- if (cpu_volts) {
cpu_reg = regulator_get(NULL, "cpu");if (IS_ERR(cpu_reg)) {printk(KERN_WARNING"cpufreq: regulator cpu get failed.\n");cpu_reg = NULL;}- }
- l_p_j_ref_freq = clk_get_rate(cpu_clk);
- for_each_possible_cpu(cpu)
per_cpu(l_p_j_ref, cpu) =per_cpu(cpu_data, cpu).loops_per_jiffy;- ret = cpufreq_register_driver(&arm_cpufreq_driver);
- if (ret)
goto reg_put;- of_node_put(cpu0);
- return 0;
+reg_put:
- if (cpu_reg)
regulator_put(cpu_reg);- clk_put(cpu_clk);
+free_freq_table:
- kfree(arm_freq_table);
+free_cpu_volts:
- kfree(cpu_volts);
+free_cpu_freqs:
- kfree(cpu_freqs);
+put_node:
- of_node_put(cpu0);
- return ret;
+}
+static void arm_cpufreq_driver_exit(void) +{
- cpufreq_unregister_driver(&arm_cpufreq_driver);
- kfree(cpu_freqs);
- kfree(cpu_volts);
- kfree(arm_freq_table);
- clk_put(cpu_clk);
+}
+module_init(arm_cpufreq_driver_init); +module_exit(arm_cpufreq_driver_exit);
+MODULE_AUTHOR("Freescale Semiconductor Inc. Richard Zhao richard.zhao@freescale.com"); +MODULE_DESCRIPTION("ARM SoC generic CPUFreq driver"); +MODULE_LICENSE("GPL");
On Saturday 17 December 2011 16:00:03 Richard Zhao wrote:
On Fri, Dec 16, 2011 at 08:32:35AM -0600, Rob Herring wrote:
On 12/16/2011 04:30 AM, Richard Zhao wrote:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
What makes this specific to ARM and not a generic DT + clk api + regulator api driver?
smp loops_per_jiffy update needs arm header <asm/cpu.h>.
I would suggest to instead change the definition of adjust_jiffies in the core so it can be overridden by the architecture, like this
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 987a165..174584d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -189,6 +189,7 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_put); * systems as each CPU might be scaled differently. So, use the arch * per-CPU loops_per_jiffy value wherever possible. */ +#ifndef adjust_jiffies #ifndef CONFIG_SMP static unsigned long l_p_j_ref; static unsigned int l_p_j_ref_freq; @@ -218,7 +219,8 @@ static inline void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) { return; } -#endif +#endif /* CONFIG_SMP */ +#endif /* adjust_jiffies */
/**
Then ARM (and any others that want the driver) can provide their own implementation and set
#define adjust_jiffies(val, ci) adjust_jiffies((val), (ci))
to let the core use that instead of the generic UP version.
While we're there, we should probably try to fix drivers that use loops_per_jiffy, because that is not what they think it is on SMP.
Arnd
On Sat, Dec 17, 2011 at 10:29:29AM +0100, Arnd Bergmann wrote:
On Saturday 17 December 2011 16:00:03 Richard Zhao wrote:
On Fri, Dec 16, 2011 at 08:32:35AM -0600, Rob Herring wrote:
On 12/16/2011 04:30 AM, Richard Zhao wrote:
It support single core and multi-core ARM SoCs. But it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
drivers/cpufreq/Kconfig.arm | 8 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/arm-cpufreq.c | 269 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 0 deletions(-) create mode 100644 drivers/cpufreq/arm-cpufreq.c
What makes this specific to ARM and not a generic DT + clk api + regulator api driver?
smp loops_per_jiffy update needs arm header <asm/cpu.h>.
I would suggest to instead change the definition of adjust_jiffies in the core so it can be overridden by the architecture, like this
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 987a165..174584d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -189,6 +189,7 @@ EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
- systems as each CPU might be scaled differently. So, use the arch
- per-CPU loops_per_jiffy value wherever possible.
*/ +#ifndef adjust_jiffies #ifndef CONFIG_SMP static unsigned long l_p_j_ref; static unsigned int l_p_j_ref_freq; @@ -218,7 +219,8 @@ static inline void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci) { return; } -#endif +#endif /* CONFIG_SMP */ +#endif /* adjust_jiffies */ /**
Then ARM (and any others that want the driver) can provide their own implementation and set
#define adjust_jiffies(val, ci) adjust_jiffies((val), (ci))
to let the core use that instead of the generic UP version.
While we're there, we should probably try to fix drivers that use loops_per_jiffy, because that is not what they think it is on SMP.
Or let different arch register its different CPUFREQ_TRANSITION_NOTIFIER ?
Thanks Richard
Arnd
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, Dec 16, 2011 at 06:30:59PM +0800, Richard Zhao wrote:
- if (higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);
This is really bad, you're only supporting the configuration of a specific voltage which doesn't reflect what hardware does (things will be specified as a range of voltages) and will needlessly cause interoperability problems between chips and regulators if the regulator can't hit the *exact* voltage requested. There's a good solid reason why the regulator API specifies everything in terms of voltage ranges.
- ret = clk_set_rate(cpu_clk, freq);
- if (ret != 0) {
printk(KERN_DEBUG "cannot set CPU clock rate\n");return ret;- }
This error checking is really random - you're ignoring some errors and logging the errors you do detect as debug messages which seems odd. Similar issues apply throughough.
- if (cpu_volts) {
cpu_reg = regulator_get(NULL, "cpu");if (IS_ERR(cpu_reg)) {printk(KERN_WARNING"cpufreq: regulator cpu get failed.\n");cpu_reg = NULL;}- }
You should log what the error was. You're also not doing anything to check that the voltage ranges required by the frequencies are actually supported on this system, the driver should double check this so that governors don't sit there trying to set voltages that are impossible on a given board.
On Fri, Dec 16, 2011 at 06:30:59PM +0800, Richard Zhao wrote:
- if (higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);- ret = clk_set_rate(cpu_clk, freq);
- if (ret != 0) {
printk(KERN_DEBUG "cannot set CPU clock rate\n");return ret;- }
- if (!higher && cpu_reg)
regulator_set_voltage(cpu_reg,cpu_volts[index], cpu_volts[index]);
This appears to reintroduce the setting of an exact voltage which I'm sure was fixed in previous versions of the patch.
+static struct cpufreq_driver arm_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = arm_verify_speed,
- .target = arm_set_target,
- .get = arm_get_speed,
- .init = arm_cpufreq_init,
- .exit = arm_cpufreq_exit,
- .name = "arm",
+};
This code doesn't actually look terribly ARM specific...
- printk(KERN_INFO "ARM SoC generic CPU frequency driver\n");
Do we need this?
On Wed, Jan 18, 2012 at 11:39:50AM +0000, Mark Brown wrote:
This appears to reintroduce the setting of an exact voltage which I'm sure was fixed in previous versions of the patch.
Erk, sorry - it looks like the device tree list has quite a bit of lag in moderation and sent out some old patches which I didn't notice. All this has been fixed in subsequent verisons.
On Wed, Jan 18, 2012 at 11:42:28AM +0000, Mark Brown wrote:
On Wed, Jan 18, 2012 at 11:39:50AM +0000, Mark Brown wrote:
This appears to reintroduce the setting of an exact voltage which I'm sure was fixed in previous versions of the patch.
Erk, sorry - it looks like the device tree list has quite a bit of lag in moderation and sent out some old patches which I didn't notice. All this has been fixed in subsequent verisons.
Yeah, that's my fault. I'm rather a bursty moderator at times.
g.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/boot/dts/imx6q.dtsi | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index 263e8f3..f2e3eaf 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -29,6 +29,9 @@ compatible = "arm,cortex-a9"; reg = <0>; next-level-cache = <&L2>; + cpu-freqs = <996000000 792000000 396000000 198000000>; + cpu-volts = <1225000 1100000 950000 850000>; + trans-latency = <61036>; /* two CLK32 periods */ };
cpu@1 {
cpufreq needs cpu clock to change frequency.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/mach-imx/clock-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clock-imx6q.c b/arch/arm/mach-imx/clock-imx6q.c index 039a7ab..72acbc2 100644 --- a/arch/arm/mach-imx/clock-imx6q.c +++ b/arch/arm/mach-imx/clock-imx6q.c @@ -1911,6 +1911,7 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK(NULL, "gpmi_io_clk", gpmi_io_clk), _REGISTER_CLOCK(NULL, "usboh3_clk", usboh3_clk), _REGISTER_CLOCK(NULL, "sata_clk", sata_clk), + _REGISTER_CLOCK(NULL, "cpu", arm_clk), };
int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
Is there a portable/generic approach for other drivers that may want to use arm-cpufreq.c? arm_clk is not normally defined for my SoC and I don't see an easy way to pull it in.
--Mark Langsdorf Calxeda, Inc.
________________________________________ From: Richard Zhao [richard.zhao@linaro.org] Sent: Friday, December 16, 2011 4:31 AM To: linux-arm-kernel@lists.infradead.org; cpufreq@vger.kernel.org; devicetree-discuss@lists.ozlabs.org Cc: linux@arm.linux.org.uk; davej@redhat.com; kernel@pengutronix.de; shawn.guo@linaro.org; eric.miao@linaro.org; Mark Langsdorf; linaro-dev@lists.linaro.org; patches@linaro.org; Richard Zhao Subject: [PATCH V2 3/4] arm/imx6q: register arm_clk as cpu to clkdev
cpufreq needs cpu clock to change frequency.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/mach-imx/clock-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clock-imx6q.c b/arch/arm/mach-imx/clock-imx6q.c index 039a7ab..72acbc2 100644 --- a/arch/arm/mach-imx/clock-imx6q.c +++ b/arch/arm/mach-imx/clock-imx6q.c @@ -1911,6 +1911,7 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK(NULL, "gpmi_io_clk", gpmi_io_clk), _REGISTER_CLOCK(NULL, "usboh3_clk", usboh3_clk), _REGISTER_CLOCK(NULL, "sata_clk", sata_clk), + _REGISTER_CLOCK(NULL, "cpu", arm_clk), };
int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode) -- 1.7.5.4
On Fri, Dec 16, 2011 at 11:35:39AM -0500, Mark Langsdorf wrote:
Is there a portable/generic approach for other drivers that may want to use arm-cpufreq.c? arm_clk is not normally defined for my SoC and I don't see an easy way to pull it in.
Could you tell me the details? Is your board arch/arm/mach-highbank/ Rob maintained? clk API is the most generic way for arm as far as I find out.
Thanks Richard
--Mark Langsdorf Calxeda, Inc.
From: Richard Zhao [richard.zhao@linaro.org] Sent: Friday, December 16, 2011 4:31 AM To: linux-arm-kernel@lists.infradead.org; cpufreq@vger.kernel.org; devicetree-discuss@lists.ozlabs.org Cc: linux@arm.linux.org.uk; davej@redhat.com; kernel@pengutronix.de; shawn.guo@linaro.org; eric.miao@linaro.org; Mark Langsdorf; linaro-dev@lists.linaro.org; patches@linaro.org; Richard Zhao Subject: [PATCH V2 3/4] arm/imx6q: register arm_clk as cpu to clkdev
cpufreq needs cpu clock to change frequency.
Signed-off-by: Richard Zhao richard.zhao@linaro.org
arch/arm/mach-imx/clock-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clock-imx6q.c b/arch/arm/mach-imx/clock-imx6q.c index 039a7ab..72acbc2 100644 --- a/arch/arm/mach-imx/clock-imx6q.c +++ b/arch/arm/mach-imx/clock-imx6q.c @@ -1911,6 +1911,7 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK(NULL, "gpmi_io_clk", gpmi_io_clk), _REGISTER_CLOCK(NULL, "usboh3_clk", usboh3_clk), _REGISTER_CLOCK(NULL, "sata_clk", sata_clk),
_REGISTER_CLOCK(NULL, "cpu", arm_clk),};
int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
1.7.5.4
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/mach-imx/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index c44aa97..39cf00a 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -595,6 +595,7 @@ comment "i.MX6 family:"
config SOC_IMX6Q bool "i.MX6 Quad support" + select ARCH_HAS_CPUFREQ select ARM_GIC select CACHE_L2X0 select CPU_V7
-
Arnd Bergmann -
Bryan Huntsman -
Grant Likely -
Heiko Stübner -
Jamie Iles -
Mark Brown -
Mark Langsdorf -
Richard Zhao -
Richard Zhao -
Rob Herring -
Stephen Boyd