The driver is based on clock and regulator APIs and support single core and multi core ARM SoCs. For multi core, it assume all cores share the same clock and voltage.
Thanks Arnd, Mark, Jamie, Rob, for your review.
Changes in V5: - add more comments - rename trans-latency to clk-trans-latency, and it only describe clk latency. Regulator latency is got from regulator_set_voltage_time.
Changes in v4: - add depends on HAVE_CLK && OF && REGULATOR - add set_cpu_freq fail check - regulator_put wehn module exit - add pr_fmt and convert all printk to pr_xxx - use voltage range - comment and doc fix - add cpu_volts value pre-check in module init - add helpfull module parameter max_freq - remove compatible string check on Arnd's comment. - remove generic-cpufreq to clk-reg-cpufreq
Changes in v3: - move adjusting smp loops_per_jiffy to arm common code, and also adjust global loops_per_jiffy. - remove adjusting loops_per_jiffy in imx and omap cpufreq drivers. - check compatible "generic-cpufreq" when module_init - change printk to pr_xxx - add generic-cpufreq DT binding doc
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 V5 1/7] ARM: add cpufreq transiton notifier to adjust [PATCH V5 2/7] arm/imx: cpufreq: remove loops_per_jiffy recalculate [PATCH V5 3/7] cpufreq: OMAP: remove loops_per_jiffy recalculate for [PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver [PATCH V5 5/7] dts/imx6q: add cpufreq property [PATCH V5 6/7] arm/imx6q: register arm_clk as cpu to clkdev [PATCH V5 7/7] arm/imx6q: select ARCH_HAS_CPUFREQ
Thanks Richard
If CONFIG_SMP, cpufreq skips loops_per_jiffy update, because different arch has different per-cpu loops_per_jiffy definition.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/kernel/smp.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index ef5640b..ac9cadc 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -25,6 +25,7 @@ #include <linux/percpu.h> #include <linux/clockchips.h> #include <linux/completion.h> +#include <linux/cpufreq.h>
#include <linux/atomic.h> #include <asm/cacheflush.h> @@ -631,3 +632,56 @@ int setup_profiling_timer(unsigned int multiplier) { return -EINVAL; } + +#ifdef CONFIG_CPU_FREQ + +static DEFINE_PER_CPU(unsigned long, l_p_j_ref); +static DEFINE_PER_CPU(unsigned long, l_p_j_ref_freq); +static unsigned long global_l_p_j_ref; +static unsigned long global_l_p_j_ref_freq; + +static int cpufreq_callback(struct notifier_block *nb, + unsigned long val, void *data) +{ + struct cpufreq_freqs *freq = data; + int cpu = freq->cpu; + + if (freq->flags & CPUFREQ_CONST_LOOPS) + return NOTIFY_OK; + + if (!per_cpu(l_p_j_ref, cpu)) { + per_cpu(l_p_j_ref, cpu) = + per_cpu(cpu_data, cpu).loops_per_jiffy; + per_cpu(l_p_j_ref_freq, cpu) = freq->old; + if (!global_l_p_j_ref) { + global_l_p_j_ref = loops_per_jiffy; + global_l_p_j_ref_freq = freq->old; + } + } + + if ((val == CPUFREQ_PRECHANGE && freq->old < freq->new) || + (val == CPUFREQ_POSTCHANGE && freq->old > freq->new) || + (val == CPUFREQ_RESUMECHANGE || val == CPUFREQ_SUSPENDCHANGE)) { + loops_per_jiffy = cpufreq_scale(global_l_p_j_ref, + global_l_p_j_ref_freq, + freq->new); + per_cpu(cpu_data, cpu).loops_per_jiffy = + cpufreq_scale(per_cpu(l_p_j_ref, cpu), + per_cpu(l_p_j_ref_freq, cpu), + freq->new); + } + return NOTIFY_OK; +} + +static struct notifier_block cpufreq_notifier = { + .notifier_call = cpufreq_callback, +}; + +static int __init register_cpufreq_notifier(void) +{ + return cpufreq_register_notifier(&cpufreq_notifier, + CPUFREQ_TRANSITION_NOTIFIER); +} +core_initcall(register_cpufreq_notifier); + +#endif
arm registered cpufreq transition notifier to recalculate it.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/plat-mxc/cpufreq.c | 10 ---------- 1 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/arch/arm/plat-mxc/cpufreq.c b/arch/arm/plat-mxc/cpufreq.c index c937e75..364793a 100644 --- a/arch/arm/plat-mxc/cpufreq.c +++ b/arch/arm/plat-mxc/cpufreq.c @@ -99,16 +99,6 @@ static int mxc_set_target(struct cpufreq_policy *policy,
ret = set_cpu_freq(freq_Hz);
-#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(cpu_data, cpu).loops_per_jiffy, - freqs.old, freqs.new); -#endif - for_each_possible_cpu(cpu) { freqs.cpu = cpu; cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
arm registered cpufreq transition notifier to recalculate it.
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- drivers/cpufreq/omap-cpufreq.c | 36 ------------------------------------ 1 files changed, 0 insertions(+), 36 deletions(-)
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c index 5d04c57..17da4c4 100644 --- a/drivers/cpufreq/omap-cpufreq.c +++ b/drivers/cpufreq/omap-cpufreq.c @@ -37,16 +37,6 @@
#include <mach/hardware.h>
-#ifdef CONFIG_SMP -struct lpj_info { - unsigned long ref; - unsigned int freq; -}; - -static DEFINE_PER_CPU(struct lpj_info, lpj_ref); -static struct lpj_info global_lpj_ref; -#endif - static struct cpufreq_frequency_table *freq_table; static atomic_t freq_table_users = ATOMIC_INIT(0); static struct clk *mpu_clk; @@ -118,32 +108,6 @@ static int omap_target(struct cpufreq_policy *policy, ret = clk_set_rate(mpu_clk, freqs.new * 1000); freqs.new = omap_getspeed(policy->cpu);
-#ifdef CONFIG_SMP - /* - * Note that loops_per_jiffy is not updated on SMP systems in - * cpufreq driver. So, update the per-CPU loops_per_jiffy value - * on frequency transition. We need to update all dependent CPUs. - */ - for_each_cpu(i, policy->cpus) { - struct lpj_info *lpj = &per_cpu(lpj_ref, i); - if (!lpj->freq) { - lpj->ref = per_cpu(cpu_data, i).loops_per_jiffy; - lpj->freq = freqs.old; - } - - per_cpu(cpu_data, i).loops_per_jiffy = - cpufreq_scale(lpj->ref, lpj->freq, freqs.new); - } - - /* And don't forget to adjust the global one */ - if (!global_lpj_ref.freq) { - global_lpj_ref.ref = loops_per_jiffy; - global_lpj_ref.freq = freqs.old; - } - loops_per_jiffy = cpufreq_scale(global_lpj_ref.ref, global_lpj_ref.freq, - freqs.new); -#endif - /* notifiers */ for_each_cpu(i, policy->cpus) { freqs.cpu = i;
The driver get cpu operation point table from device tree cpu0 node, and adjusts operating points using clk and regulator APIs.
It support single core and multi-core ARM SoCs. But currently it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org Reviewed-by: Jamie Iles jamie@jamieiles.com Reviewed-by: Mark Brown broonie@opensource.wolfsonmicro.com --- .../devicetree/bindings/cpufreq/clk-reg-cpufreq | 21 ++ drivers/cpufreq/Kconfig | 10 + drivers/cpufreq/Makefile | 2 + drivers/cpufreq/clk-reg-cpufreq.c | 302 ++++++++++++++++++++ 4 files changed, 335 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/cpufreq/clk-reg-cpufreq create mode 100644 drivers/cpufreq/clk-reg-cpufreq.c
diff --git a/Documentation/devicetree/bindings/cpufreq/clk-reg-cpufreq b/Documentation/devicetree/bindings/cpufreq/clk-reg-cpufreq new file mode 100644 index 0000000..e8dc763 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/clk-reg-cpufreq @@ -0,0 +1,21 @@ +Generic cpufreq driver based on clk and regulator APIs + +Required properties in /cpus/cpu@0: +- cpu-freqs : cpu frequency points it supports, in unit of Hz. + Each point takes on cell. +- cpu-volts : cpu voltage ranges required by the frequency points + at the same index, in unit of uV. + Each range takes two cells, one for min, the other for max. +- clk-trans-latency : cpu clk transition latency, in unit of ns. + It takes one cell. + +An example: +cpu@0 { + cpu-freqs = <996000000 792000000 396000000 198000000>; + cpu-volts = < /* min max */ + 1225000 1450000 /* 996M */ + 1100000 1450000 /* 792M */ + 950000 1450000 /* 396M */ + 850000 1450000>; /* 198M */ + clk-trans-latency = <61036>; /* two CLK32 periods */ +}; diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index e24a2a1..95470f1 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -179,6 +179,16 @@ config CPU_FREQ_GOV_CONSERVATIVE
If in doubt, say N.
+config CLK_REG_CPUFREQ_DRIVER + tristate "Generic cpufreq driver using clk and regulator APIs" + depends on HAVE_CLK && OF && REGULATOR + select CPU_FREQ_TABLE + help + This adds generic CPUFreq driver based on clk and regulator APIs. + It assumes all cores of the CPU share the same clock and voltage. + + If in doubt, say N. + menu "x86 CPU frequency scaling drivers" depends on X86 source "drivers/cpufreq/Kconfig.x86" diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index ce75fcb..2c4eb33 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -13,6 +13,8 @@ obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE) += cpufreq_conservative.o # CPUfreq cross-arch helpers obj-$(CONFIG_CPU_FREQ_TABLE) += freq_table.o
+obj-$(CONFIG_CLK_REG_CPUFREQ_DRIVER) += clk-reg-cpufreq.o + ################################################################################## # x86 drivers. # Link order matters. K8 is preferred to ACPI because of firmware bugs in early diff --git a/drivers/cpufreq/clk-reg-cpufreq.c b/drivers/cpufreq/clk-reg-cpufreq.c new file mode 100644 index 0000000..4b28fdd --- /dev/null +++ b/drivers/cpufreq/clk-reg-cpufreq.c @@ -0,0 +1,302 @@ +/* + * 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 + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#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> + +static u32 *cpu_freqs; /* Hz */ +static u32 *cpu_volts; /* uV */ +static u32 trans_latency; /* ns */ +static int cpu_op_nr; +static unsigned int cur_index; + +static struct clk *cpu_clk; +static struct regulator *cpu_reg; +static struct cpufreq_frequency_table *freq_table; + +static int set_cpu_freq(unsigned long freq, int index, int higher) +{ + int ret = 0; + + if (higher && cpu_reg) { + ret = regulator_set_voltage(cpu_reg, + cpu_volts[index * 2], cpu_volts[index * 2 + 1]); + if (ret) { + pr_err("set cpu voltage failed!\n"); + return ret; + } + } + + ret = clk_set_rate(cpu_clk, freq); + if (ret) { + if (cpu_reg) + regulator_set_voltage(cpu_reg, cpu_volts[cur_index * 2], + cpu_volts[cur_index * 2 + 1]); + pr_err("cannot set CPU clock rate\n"); + return ret; + } + + if (!higher && cpu_reg) { + ret = regulator_set_voltage(cpu_reg, + cpu_volts[index * 2], cpu_volts[index * 2 + 1]); + if (ret) + pr_warn("set cpu voltage failed, might run on" + " higher voltage!\n"); + ret = 0; + } + + return ret; +} + +static int clk_reg_verify_speed(struct cpufreq_policy *policy) +{ + return cpufreq_frequency_table_verify(policy, freq_table); +} + +static unsigned int clk_reg_get_speed(unsigned int cpu) +{ + return clk_get_rate(cpu_clk) / 1000; +} + +static int clk_reg_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, 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 = 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)); + if (ret) + freqs.new = clk_get_rate(cpu_clk) / 1000; + else + cur_index = index; + + for_each_possible_cpu(cpu) { + freqs.cpu = cpu; + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE); + } + + return ret; +} + +static int clk_reg_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); + policy->cpuinfo.transition_latency = trans_latency; + + ret = cpufreq_frequency_table_cpuinfo(policy, freq_table); + + if (ret < 0) { + pr_err("invalid frequency table for cpu %d\n", + policy->cpu); + return ret; + } + + cpufreq_frequency_table_get_attr(freq_table, policy->cpu); + cpufreq_frequency_table_target(policy, freq_table, policy->cur, + CPUFREQ_RELATION_H, &cur_index); + return 0; +} + +static int clk_reg_cpufreq_exit(struct cpufreq_policy *policy) +{ + cpufreq_frequency_table_put_attr(policy->cpu); + return 0; +} + +static struct cpufreq_driver clk_reg_cpufreq_driver = { + .flags = CPUFREQ_STICKY, + .verify = clk_reg_verify_speed, + .target = clk_reg_set_target, + .get = clk_reg_get_speed, + .init = clk_reg_cpufreq_init, + .exit = clk_reg_cpufreq_exit, + .name = "clk-reg", +}; + +static u32 max_freq = UINT_MAX / 1000; /* kHz */ +module_param(max_freq, uint, 0); +MODULE_PARM_DESC(max_freq, "max cpu frequency in unit of kHz"); + +static int __devinit clk_reg_cpufreq_driver_init(void) +{ + struct device_node *cpu0; + const struct property *pp; + int max_idx = 0, min_idx = 0; + int i, ret; + + 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 * 2 == pp->length / sizeof(u32)) { + cpu_volts = kzalloc(sizeof(*cpu_volts) * cpu_op_nr * 2, + GFP_KERNEL); + if (!cpu_volts) + goto free_cpu_freqs; + of_property_read_u32_array(cpu0, "cpu-volts", + cpu_volts, cpu_op_nr * 2); + } else + pr_warn("invalid cpu_volts!\n"); + } + + if (of_property_read_u32(cpu0, "clk-trans-latency", &trans_latency)) + trans_latency = CPUFREQ_ETERNAL; + + cpu_clk = clk_get(NULL, "cpu"); + if (IS_ERR(cpu_clk)) { + pr_err("failed to get cpu clock\n"); + ret = PTR_ERR(cpu_clk); + goto free_cpu_volts; + } + + if (cpu_volts) { + cpu_reg = regulator_get(NULL, "cpu"); + if (IS_ERR(cpu_reg)) { + pr_warn("regulator cpu get failed.\n"); + cpu_reg = NULL; + } + } + + freq_table = kmalloc(sizeof(struct cpufreq_frequency_table) + * (cpu_op_nr + 1), GFP_KERNEL); + if (!freq_table) { + ret = -ENOMEM; + goto reg_put; + } + + for (i = 0; i < cpu_op_nr; i++) { + freq_table[i].index = i; + if (cpu_freqs[i] > max_freq * 1000) { + freq_table[i].frequency = CPUFREQ_ENTRY_INVALID; + continue; + } + + if (cpu_reg) { + ret = regulator_is_supported_voltage(cpu_reg, + cpu_volts[i * 2], cpu_volts[i * 2 + 1]); + if (ret <= 0) { + freq_table[i].frequency = CPUFREQ_ENTRY_INVALID; + continue; + } + } + freq_table[i].frequency = cpu_freqs[i] / 1000; + max_idx = cpu_freqs[i] > cpu_freqs[max_idx] ? i : max_idx; + min_idx = cpu_freqs[i] < cpu_freqs[min_idx] ? i : min_idx; + } + + freq_table[i].index = i; + freq_table[i].frequency = CPUFREQ_TABLE_END; + + if (cpu_reg && trans_latency != CPUFREQ_ETERNAL) { + ret = regulator_set_voltage_time(cpu_reg, cpu_volts[min_idx], + cpu_volts[max_idx]); + if (ret < 0) { + pr_warn("regulator_set_voltage_time failed. " + "Default use 1us\n"); + ret = 1; + } + trans_latency += ret * 1000; + } + + ret = cpufreq_register_driver(&clk_reg_cpufreq_driver); + if (ret) + goto free_freq_table; + + of_node_put(cpu0); + + return 0; + +free_freq_table: + kfree(freq_table); +reg_put: + if (cpu_reg) + regulator_put(cpu_reg); + clk_put(cpu_clk); +free_cpu_volts: + kfree(cpu_volts); +free_cpu_freqs: + kfree(cpu_freqs); +put_node: + of_node_put(cpu0); + + return ret; +} + +static void clk_reg_cpufreq_driver_exit(void) +{ + cpufreq_unregister_driver(&clk_reg_cpufreq_driver); + kfree(cpu_freqs); + kfree(cpu_volts); + clk_put(cpu_clk); + if (cpu_reg) + regulator_put(cpu_reg); + kfree(freq_table); +} + +module_init(clk_reg_cpufreq_driver_init); +module_exit(clk_reg_cpufreq_driver_exit); + +MODULE_AUTHOR("Freescale Semiconductor Inc. Richard Zhao richard.zhao@freescale.com"); +MODULE_DESCRIPTION("Generic CPUFreq driver based on clk and regulator APIs"); +MODULE_LICENSE("GPL");
Hi Richard,
On Tue, Dec 27, 2011 at 04:24:19PM +0800, Richard Zhao wrote:
The driver get cpu operation point table from device tree cpu0 node, and adjusts operating points using clk and regulator APIs.
It support single core and multi-core ARM SoCs. But currently it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org Reviewed-by: Jamie Iles jamie@jamieiles.com Reviewed-by: Mark Brown broonie@opensource.wolfsonmicro.com
.../devicetree/bindings/cpufreq/clk-reg-cpufreq | 21 ++ drivers/cpufreq/Kconfig | 10 + drivers/cpufreq/Makefile | 2 + drivers/cpufreq/clk-reg-cpufreq.c | 302 ++++++++++++++++++++ 4 files changed, 335 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/cpufreq/clk-reg-cpufreq create mode 100644 drivers/cpufreq/clk-reg-cpufreq.c
[...]
+static struct cpufreq_driver clk_reg_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = clk_reg_verify_speed,
- .target = clk_reg_set_target,
- .get = clk_reg_get_speed,
- .init = clk_reg_cpufreq_init,
- .exit = clk_reg_cpufreq_exit,
- .name = "clk-reg",
+};
+static u32 max_freq = UINT_MAX / 1000; /* kHz */ +module_param(max_freq, uint, 0); +MODULE_PARM_DESC(max_freq, "max cpu frequency in unit of kHz");
Have you tried to pass this param from kernel cmdline? What's the syntax if we want to pass a 800 MHz max_freq?
And I played this driver on imx6q with pm-qa [1] cpufreq test suit from Linaro PMWG.
### cpufreq_01: ### test the cpufreq framework is available for frequency ### https://wiki.linaro.org/WorkingGroups/PowerManagement/Doc/QA/Scripts#cpufreq... ### cpufreq_01.0/cpu0: checking 'scaling_available_frequencies' exists... fail cpufreq_01.0/cpu1: checking 'scaling_available_frequencies' exists... fail cpufreq_01.0/cpu2: checking 'scaling_available_frequencies' exists... fail cpufreq_01.0/cpu3: checking 'scaling_available_frequencies' exists... fail
### cpufreq_05: ### test 'ondemand' and 'conservative' trigger correctly the configuration directory ### https://wiki.linaro.org/WorkingGroups/PowerManagement/Doc/QA/Scripts#cpufreq... ### cpufreq_05.0: checking 'ondemand' directory exists... pass cpufreq_05.1: checking 'conservative' directory exists... pass cpufreq_05.2: checking 'ondemand' directory is not there... pass cpufreq_05.3: checking 'conservative' directory is not there... pass cpufreq_05.4: checking 'ondemand' directory exists... fail cpufreq_05.5: checking 'conservative' directory exists... pass
The cpufreq_01 can be easily fixed with the following change.
8<----- @@ -146,6 +150,11 @@ static int clk_reg_cpufreq_exit(struct cpufreq_policy *policy) return 0; }
+static struct freq_attr *clk_reg_cpufreq_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + NULL, +}; + static struct cpufreq_driver clk_reg_cpufreq_driver = { .flags = CPUFREQ_STICKY, .verify = clk_reg_verify_speed, @@ -153,10 +162,15 @@ static struct cpufreq_driver clk_reg_cpufreq_driver = { .get = clk_reg_get_speed, .init = clk_reg_cpufreq_init, .exit = clk_reg_cpufreq_exit, + .attr = clk_reg_cpufreq_attr, .name = "clk-reg", }; ----->8
And I have not looked into the second one deeply, but maybe you want to :)
On Tue, Dec 27, 2011 at 11:05:41PM +0800, Shawn Guo wrote:
Hi Richard,
On Tue, Dec 27, 2011 at 04:24:19PM +0800, Richard Zhao wrote:
The driver get cpu operation point table from device tree cpu0 node, and adjusts operating points using clk and regulator APIs.
It support single core and multi-core ARM SoCs. But currently it assume all cores share the same frequency and voltage.
Signed-off-by: Richard Zhao richard.zhao@linaro.org Reviewed-by: Jamie Iles jamie@jamieiles.com Reviewed-by: Mark Brown broonie@opensource.wolfsonmicro.com
.../devicetree/bindings/cpufreq/clk-reg-cpufreq | 21 ++ drivers/cpufreq/Kconfig | 10 + drivers/cpufreq/Makefile | 2 + drivers/cpufreq/clk-reg-cpufreq.c | 302 ++++++++++++++++++++ 4 files changed, 335 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/cpufreq/clk-reg-cpufreq create mode 100644 drivers/cpufreq/clk-reg-cpufreq.c
[...]
+static struct cpufreq_driver clk_reg_cpufreq_driver = {
- .flags = CPUFREQ_STICKY,
- .verify = clk_reg_verify_speed,
- .target = clk_reg_set_target,
- .get = clk_reg_get_speed,
- .init = clk_reg_cpufreq_init,
- .exit = clk_reg_cpufreq_exit,
- .name = "clk-reg",
+};
+static u32 max_freq = UINT_MAX / 1000; /* kHz */ +module_param(max_freq, uint, 0); +MODULE_PARM_DESC(max_freq, "max cpu frequency in unit of kHz");
Have you tried to pass this param from kernel cmdline? What's the syntax if we want to pass a 800 MHz max_freq?
clk-reg-cpufreq.max_freq=800000
And I played this driver on imx6q with pm-qa [1] cpufreq test suit from Linaro PMWG.
### cpufreq_01: ### test the cpufreq framework is available for frequency ### https://wiki.linaro.org/WorkingGroups/PowerManagement/Doc/QA/Scripts#cpufreq... ### cpufreq_01.0/cpu0: checking 'scaling_available_frequencies' exists... fail cpufreq_01.0/cpu1: checking 'scaling_available_frequencies' exists... fail cpufreq_01.0/cpu2: checking 'scaling_available_frequencies' exists... fail cpufreq_01.0/cpu3: checking 'scaling_available_frequencies' exists... fail
hmm, scaling_available_frequencies is nice-to-have feature. I'm glad to add it.
### cpufreq_05: ### test 'ondemand' and 'conservative' trigger correctly the configuration directory ### https://wiki.linaro.org/WorkingGroups/PowerManagement/Doc/QA/Scripts#cpufreq... ### cpufreq_05.0: checking 'ondemand' directory exists... pass cpufreq_05.1: checking 'conservative' directory exists... pass cpufreq_05.2: checking 'ondemand' directory is not there... pass cpufreq_05.3: checking 'conservative' directory is not there... pass cpufreq_05.4: checking 'ondemand' directory exists... fail cpufreq_05.5: checking 'conservative' directory exists... pass
I past fail part script here: switch_ondemand cpu0 switch_conservative cpu1 check "'ondemand' directory exists" "test -d $CPU_PATH/cpufreq/ondemand" check "'conservative' directory exists" "test -d $CPU_PATH/cpufreq/conservative" This driver assume all cpu cores to share the same freq and voltage. The affected cpu is all other cpus. They also share one single governor. The test case does not suit this driver and not for most arm multi-core cpus I guess.
The cpufreq_01 can be easily fixed with the following change.
8<----- @@ -146,6 +150,11 @@ static int clk_reg_cpufreq_exit(struct cpufreq_policy *policy) return 0; }
+static struct freq_attr *clk_reg_cpufreq_attr[] = {
&cpufreq_freq_attr_scaling_available_freqs,NULL,+};
static struct cpufreq_driver clk_reg_cpufreq_driver = { .flags = CPUFREQ_STICKY, .verify = clk_reg_verify_speed, @@ -153,10 +162,15 @@ static struct cpufreq_driver clk_reg_cpufreq_driver = { .get = clk_reg_get_speed, .init = clk_reg_cpufreq_init, .exit = clk_reg_cpufreq_exit,
.attr = clk_reg_cpufreq_attr, .name = "clk-reg",}; ----->8
quite right.
Thanks Richard
And I have not looked into the second one deeply, but maybe you want to :)
-- Regards, Shawn
[1] git://git.linaro.org/people/dlezcano/pm-qa.git
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Dec 28, 2011 at 09:24:05AM +0800, Richard Zhao wrote:
Have you tried to pass this param from kernel cmdline? What's the syntax if we want to pass a 800 MHz max_freq?
clk-reg-cpufreq.max_freq=800000
Thanks. I was mistaken on the module name.
### cpufreq_05: ### test 'ondemand' and 'conservative' trigger correctly the configuration directory ### https://wiki.linaro.org/WorkingGroups/PowerManagement/Doc/QA/Scripts#cpufreq... ### cpufreq_05.0: checking 'ondemand' directory exists... pass cpufreq_05.1: checking 'conservative' directory exists... pass cpufreq_05.2: checking 'ondemand' directory is not there... pass cpufreq_05.3: checking 'conservative' directory is not there... pass cpufreq_05.4: checking 'ondemand' directory exists... fail cpufreq_05.5: checking 'conservative' directory exists... pass
I past fail part script here: switch_ondemand cpu0 switch_conservative cpu1 check "'ondemand' directory exists" "test -d $CPU_PATH/cpufreq/ondemand" check "'conservative' directory exists" "test -d $CPU_PATH/cpufreq/conservative" This driver assume all cpu cores to share the same freq and voltage. The affected cpu is all other cpus. They also share one single governor. The test case does not suit this driver and not for most arm multi-core cpus I guess.
Then this is the feedback that Linaro PMWG wants to have, I guess.
Here is my tag on this patch.
Acked-by: Shawn Guo shawn.guo@linaro.org
On Wed, Dec 28, 2011 at 10:01:13AM +0800, Shawn Guo wrote:
Here is my tag on this patch.
Acked-by: Shawn Guo shawn.guo@linaro.org
For record, this tag is only valid with the following conditions.
* Fix the failure of pm-qa case cpufreq_01 * Fix the failure of module build * Remove the dependency on REGULATOR since a set of empty functions have already been provided for !REGULATOR build. (regulator_is_supported_voltage and regulator_set_voltage_time to be added)
Hi Mark,
[...]
if (cpu_reg) {ret = regulator_is_supported_voltage(cpu_reg,cpu_volts[i * 2], cpu_volts[i * 2 + 1]);
Is there any reason you didn't export symbol regulator_is_supported_voltage? and also it don't have !REGULATOR dummy implementation.
Thanks Richard
On Wed, Dec 28, 2011 at 11:14:10AM +0800, Richard Zhao wrote:
Hi Mark,
[...]
if (cpu_reg) {ret = regulator_is_supported_voltage(cpu_reg,cpu_volts[i * 2], cpu_volts[i * 2 + 1]);Is there any reason you didn't export symbol regulator_is_supported_voltage? and also it don't have !REGULATOR dummy implementation.
regulator_set_voltage_time and some other functions don't have dummy one either.
Thanks Richard
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Dec 28, 2011 at 11:31:29AM +0800, Richard Zhao wrote:
On Wed, Dec 28, 2011 at 11:14:10AM +0800, Richard Zhao wrote:
if (cpu_reg) {ret = regulator_is_supported_voltage(cpu_reg,cpu_volts[i * 2], cpu_volts[i * 2 + 1]);
Is there any reason you didn't export symbol regulator_is_supported_voltage? and also it don't have !REGULATOR dummy implementation.
regulator_set_voltage_time and some other functions don't have dummy one either.
You can't usefully work with voltages without knowing what the actual voltages are - the only sensible stubs we could provide would return errors but then any driver using the stubs would probably fail to do whatever it was doing. With enable and disable we can sensibly stub things out with an always on regulator.
On Wed, Dec 28, 2011 at 11:42:37AM +0000, Mark Brown wrote:
On Wed, Dec 28, 2011 at 11:31:29AM +0800, Richard Zhao wrote:
On Wed, Dec 28, 2011 at 11:14:10AM +0800, Richard Zhao wrote:
if (cpu_reg) {ret = regulator_is_supported_voltage(cpu_reg,cpu_volts[i * 2], cpu_volts[i * 2 + 1]);Is there any reason you didn't export symbol regulator_is_supported_voltage? and also it don't have !REGULATOR dummy implementation.
regulator_set_voltage_time and some other functions don't have dummy one either.
You can't usefully work with voltages without knowing what the actual voltages are - the only sensible stubs we could provide would return errors but then any driver using the stubs would probably fail to do whatever it was doing. With enable and disable we can sensibly stub things out with an always on regulator.
Sorry, I can not get your point here. Let me describe the problem I met: - regulator_is_supported_voltage is not exported. when I build clk-reg-cpufreq as kernel module, there's a link error. - I saw linux/regulator/consumer.h has some dummy functions if !REGULATOR. I tried to make clk-reg-cpufreq driver work even !REGULATOR. I think that's why the dummy functions are there. If regulator_get return NULL, it'll avoid calling other regulator functions. But regulator_is_supported_voltage and regulator_set_voltage_time don't have such dummy ones. Undefined functions.
Thanks Richard
On Wed, Dec 28, 2011 at 08:05:20PM +0800, Richard Zhao wrote:
Looks like the problem with your mail client is that it's wrapping at exactly 80 characters which is too little - you need to leave space for being quoted.
On Wed, Dec 28, 2011 at 11:42:37AM +0000, Mark Brown wrote:
You can't usefully work with voltages without knowing what the actual voltages are - the only sensible stubs we could provide would return errors but then any driver using the stubs would probably fail to do whatever it was doing. With enable and disable we can sensibly stub things out with an always on regulator.
Sorry, I can not get your point here. Let me describe the problem I met:
- regulator_is_supported_voltage is not exported. when I build clk-reg-cpufreq as kernel module, there's a link error.
This is an oversight, I've just fixed it.
- I saw linux/regulator/consumer.h has some dummy functions if !REGULATOR. I tried to make clk-reg-cpufreq driver work even !REGULATOR. I think that's why the dummy functions are there. If regulator_get return NULL, it'll avoid calling other regulator functions. But regulator_is_supported_voltage and regulator_set_voltage_time don't have such dummy ones. Undefined functions.
I can only repeat what I wrote above explaining why no stubs are provided.
On Wed, Dec 28, 2011 at 12:14:04PM +0000, Mark Brown wrote:
On Wed, Dec 28, 2011 at 08:05:20PM +0800, Richard Zhao wrote:
Looks like the problem with your mail client is that it's wrapping at exactly 80 characters which is too little - you need to leave space for being quoted.
I'm using vim. any suggestion for auto-wrapping?
On Wed, Dec 28, 2011 at 11:42:37AM +0000, Mark Brown wrote:
You can't usefully work with voltages without knowing what the actual voltages are
if people don't enable REGULATOR, the stubs (if you mean the dummy func) are only be used to pass build. The code is optimized out by compiler.
- the only sensible stubs we could provide would return
errors but then any driver using the stubs would probably fail to do whatever it was doing.
In this case, If regulator_get return NULL, I won't call other regulator functions at runtime.
With enable and disable we can sensibly stub
things out with an always on regulator.
Sorry, I can not get your point here. Let me describe the problem I met:
- regulator_is_supported_voltage is not exported. when I build clk-reg-cpufreq as kernel module, there's a link error.
This is an oversight, I've just fixed it.
Thanks.
- I saw linux/regulator/consumer.h has some dummy functions if !REGULATOR. I tried to make clk-reg-cpufreq driver work even !REGULATOR. I think that's why the dummy functions are there. If regulator_get return NULL, it'll avoid calling other regulator functions. But regulator_is_supported_voltage and regulator_set_voltage_time don't have such dummy ones. Undefined functions.
I can only repeat what I wrote above explaining why no stubs are provided.
One word. You mean I have to always depends on REGULATOR config, right?
Thanks Richard
On Wed, Dec 28, 2011 at 08:40:56PM +0800, Richard Zhao wrote:
On Wed, Dec 28, 2011 at 12:14:04PM +0000, Mark Brown wrote:
On Wed, Dec 28, 2011 at 08:05:20PM +0800, Richard Zhao wrote:
Looks like the problem with your mail client is that it's wrapping at exactly 80 characters which is too little - you need to leave space for being quoted.
I'm using vim. any suggestion for auto-wrapping?
:set tw=72
It'd also be useful to leave blank lines
You can't usefully work with voltages without knowing what the actual voltages are
if people don't enable REGULATOR, the stubs (if you mean the dummy func) are only be used to pass build. The code is optimized out by compiler.
No, this is not the case. All the functions have return values which still need to be handled gracefully by drivers using those functions.
- the only sensible stubs we could provide would return
errors but then any driver using the stubs would probably fail to do whatever it was doing.
In this case, If regulator_get return NULL, I won't call other regulator functions at runtime.
That should be OK for your use case but it might not be sensible for other cases. Any stubs for this provided by the core need to work well for any user.
- I saw linux/regulator/consumer.h has some dummy functions if !REGULATOR. I tried to make clk-reg-cpufreq driver work even !REGULATOR. I think that's why the dummy functions are there. If regulator_get return NULL, it'll avoid calling other regulator functions. But regulator_is_supported_voltage and regulator_set_voltage_time don't have such dummy ones. Undefined functions.
I can only repeat what I wrote above explaining why no stubs are provided.
One word. You mean I have to always depends on REGULATOR config, right?
Yes.
On Wed, Dec 28, 2011 at 12:47:40PM +0000, Mark Brown wrote:
One word. You mean I have to always depends on REGULATOR config, right?
Yes.
I do not care too much. But it puts the driver on an interesting position, that is it can work without a regulator driver backing the cpu voltage but it has to enable CONFIG_REGULATOR.
On Wed, Dec 28, 2011 at 09:06:20PM +0800, Shawn Guo wrote:
On Wed, Dec 28, 2011 at 12:47:40PM +0000, Mark Brown wrote:
One word. You mean I have to always depends on REGULATOR config, right?
Yes.
I do not care too much. But it puts the driver on an interesting position, that is it can work without a regulator driver backing the cpu voltage but it has to enable CONFIG_REGULATOR.
Well, the other option is ifdefs in your driver.
On Wed, Dec 28, 2011 at 12:54:21PM +0000, Mark Brown wrote:
On Wed, Dec 28, 2011 at 09:06:20PM +0800, Shawn Guo wrote:
On Wed, Dec 28, 2011 at 12:47:40PM +0000, Mark Brown wrote:
One word. You mean I have to always depends on REGULATOR config, right?
Yes.
I do not care too much. But it puts the driver on an interesting position, that is it can work without a regulator driver backing the cpu voltage but it has to enable CONFIG_REGULATOR.
Well, the other option is ifdefs in your driver.
I'm much more comfortable with this oddness than ifdefs :)
There's still a bug that, after rmmod module, cpu0 still has cpufreq sysfs entry.
cpufreq_unregister_driver can not clean up everything.
unfortunately, I don't have much time to debug cpufreq core.
Log: root@ubuntu:~# insmod /clk-reg-cpufreq.ko clk_reg_cpufreq: regulator cpu get failed. trying to register driver clk-reg adding CPU 0 CPU 1 already managed, adding link CPU 2 already managed, adding link CPU 3 already managed, adding link setting new policy for CPU 0: 198000 - 996000 kHz new min and max freqs are 198000 - 996000 kHz governor switch __cpufreq_governor for CPU 0, event 1 governor: change or update limits __cpufreq_governor for CPU 0, event 3 target for CPU 0: 792000 kHz, relation 0 initialization complete adding CPU 1 adding CPU 2 adding CPU 3 driver clk-reg up and running root@ubuntu:~# root@ubuntu:~# root@ubuntu:~# root@ubuntu:~# rmmod clk-reg-cpufreq unregistering driver clk-reg unregistering CPU 0 removing link for cpu 1 removing link for cpu 2 removing link for cpu 3 __cpufreq_governor for CPU 0, event 2 last reference is dropped waiting for dropping of refcount wait complete adding CPU 1 Restoring governor userspace for cpu 1 CPU 0 already managed, adding link CPU 2 already managed, adding link CPU 3 already managed, adding link setting new policy for CPU 1: 198000 - 996000 kHz new min and max freqs are 198000 - 996000 kHz governor switch __cpufreq_governor for CPU 1, event 1 governor: change or update limits __cpufreq_governor for CPU 1, event 3 target for CPU 1: 792000 kHz, relation 0 initialization complete unregistering CPU 0 removing link unregistering CPU 1 removing link for cpu 2 removing link for cpu 3 __cpufreq_governor for CPU 1, event 2 last reference is dropped waiting for dropping of refcount wait complete adding CPU 2 Restoring governor userspace for cpu 2 CPU 0 already managed, adding link CPU 1 already managed, adding link CPU 3 already managed, adding link setting new policy for CPU 2: 198000 - 996000 kHz new min and max freqs are 198000 - 996000 kHz governor switch __cpufreq_governor for CPU 2, event 1 governor: change or update limits __cpufreq_governor for CPU 2, event 3 target for CPU 2: 792000 kHz, relation 0 initialization complete unregistering CPU 1 removing link unregistering CPU 2 removing link for cpu 0 removing link for cpu 3 __cpufreq_governor for CPU 2, event 2 last reference is dropped waiting for dropping of refcount wait complete adding CPU 0 Restoring governor userspace for cpu 0 CPU 1 already managed, adding link CPU 2 already managed, adding link CPU 3 already managed, adding link setting new policy for CPU 0: 198000 - 996000 kHz new min and max freqs are 198000 - 996000 kHz governor switch __cpufreq_governor for CPU 0, event 1 governor: change or update limits __cpufreq_governor for CPU 0, event 3 target for CPU 0: 792000 kHz, relation 0 initialization complete unregistering CPU 2 removing link unregistering CPU 3 removing link
Thanks Richard
Hi Dave,
On Thu, Dec 29, 2011 at 02:21:03PM +0800, Richard Zhao wrote:
There's still a bug that, after rmmod module, cpu0 still has cpufreq sysfs entry.
cpufreq_unregister_driver can not clean up everything.
Is this an known issue to cpufreq core?
Signed-off-by: Richard Zhao richard.zhao@linaro.org --- arch/arm/boot/dts/imx6q.dtsi | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index 263e8f3..d89b42d 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -29,6 +29,13 @@ compatible = "arm,cortex-a9"; reg = <0>; next-level-cache = <&L2>; + cpu-freqs = <996000000 792000000 396000000 198000000>; + cpu-volts = < /* min max */ + 1225000 1450000 /* 996M */ + 1100000 1450000 /* 792M */ + 950000 1450000 /* 396M */ + 850000 1450000>; /* 198M */ + clk-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)
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
-
Mark Brown -
Richard Zhao -
Richard Zhao -
Shawn Guo