[PATCH V5 0/7] add a generic cpufreq driver

[PATCH V5 0/7] add a generic cpufreq driver

[Richard Zhao]

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

[PATCH V5 1/7] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp

[Richard Zhao]

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
-- 
1.7.5.4

[PATCH V5 2/7] arm/imx: cpufreq: remove loops_per_jiffy recalculate for smp

[Richard Zhao]

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);
-- 
1.7.5.4

[PATCH V5 3/7] cpufreq: OMAP: remove loops_per_jiffy recalculate for smp

[Richard Zhao]

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;
-- 
1.7.5.4

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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 at 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 at 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 at 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");
-- 
1.7.5.4

[PATCH V5 5/7] dts/imx6q: add cpufreq property

[Richard Zhao]

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 at 1 {
-- 
1.7.5.4

[PATCH V5 6/7] arm/imx6q: register arm_clk as cpu to clkdev

[Richard Zhao]

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

[PATCH V5 7/7] arm/imx6q: select ARCH_HAS_CPUFREQ

[Richard Zhao]

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
-- 
1.7.5.4

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Shawn Guo]

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_01
###
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_05
###
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 :)

-- 
Regards,
Shawn

[1] git://git.linaro.org/people/dlezcano/pm-qa.git

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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_01
> ###
> 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_05
> ###
> 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Shawn Guo]

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_05
> > ###
> > 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>

-- 
Regards,
Shawn

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Shawn Guo]

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)

-- 
Regards,
Shawn

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Mark Brown]

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.

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Mark Brown]

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.

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Mark Brown]

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.

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Mark Brown]

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.

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Shawn Guo]

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.

-- 
Regards,
Shawn

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Shawn Guo]

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 :)

-- 
Regards,
Shawn

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Richard Zhao]

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 at 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 at ubuntu:~# 
root at ubuntu:~# 
root at ubuntu:~# 
root at 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

[PATCH V5 4/7] cpufreq: add clk-reg cpufreq driver

[Shawn Guo]

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?

-- 
Regards,
Shawn