[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[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 |  260 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 269 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..fd9759f
--- /dev/null
+++ b/drivers/cpufreq/arm-cpufreq.c
@@ -0,0 +1,260 @@
+/*
+ * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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/err.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <asm/cpu.h>
+#include <mach/hardware.h>
+#include <mach/clock.h>
+
+static u32 *cpu_freqs;
+static u32 *cpu_volts;
+static u32 trans_latency;
+static int cpu_op_nr;
+
+static int cpu_freq_khz_min;
+static int cpu_freq_khz_max;
+
+static struct clk *cpu_clk;
+static struct cpufreq_frequency_table *arm_freq_table;
+
+static int set_cpu_freq(int freq)
+{
+	int ret = 0;
+	int org_cpu_rate;
+
+	org_cpu_rate = clk_get_rate(cpu_clk);
+	if (org_cpu_rate == freq)
+		return ret;
+
+	ret = clk_set_rate(cpu_clk, freq);
+	if (ret != 0) {
+		printk(KERN_DEBUG "cannot set CPU clock rate\n");
+		return ret;
+	}
+
+	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;
+	int freq_Hz, cpu;
+	int ret = 0;
+	unsigned int index;
+
+	cpufreq_frequency_table_target(policy, arm_freq_table,
+			target_freq, relation, &index);
+	freq_Hz = arm_freq_table[index].frequency * 1000;
+
+	freqs.old = clk_get_rate(cpu_clk) / 1000;
+	freqs.new = clk_round_rate(cpu_clk, freq_Hz);
+	freqs.new = (freqs.new ? 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);
+
+#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);
+	}
+
+	return ret;
+}
+
+static int arm_cpufreq_init(struct cpufreq_policy *policy)
+{
+	int ret;
+
+	printk(KERN_INFO "ARM CPU frequency driver\n");
+
+	if (policy->cpu >= num_possible_cpus())
+		return -EINVAL;
+
+	policy->cur = clk_get_rate(cpu_clk) / 1000;
+	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
+	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
+	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: failed to register i.MXC CPUfreq with error code %d\n",
+		       __func__, ret);
+		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);
+
+	set_cpu_freq(cpu_freq_khz_max * 1000);
+	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 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 == 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_freqs, 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;
+
+	cpu_freq_khz_min = cpu_freqs[0] / 1000;
+	cpu_freq_khz_max = cpu_freqs[0] / 1000;
+
+	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;
+		if ((cpu_freqs[i] / 1000) < cpu_freq_khz_min)
+			cpu_freq_khz_min = cpu_freqs[i] / 1000;
+		if ((cpu_freqs[i] / 1000) > cpu_freq_khz_max)
+			cpu_freq_khz_max = 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;
+	}
+
+	ret = cpufreq_register_driver(&arm_cpufreq_driver);
+	if (ret)
+		goto clock_put;
+
+	of_node_put(cpu0);
+
+	return 0;
+
+clock_put:
+	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");
-- 
1.7.5.4

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[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 |  260 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 269 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..fd9759f
--- /dev/null
+++ b/drivers/cpufreq/arm-cpufreq.c
@@ -0,0 +1,260 @@
+/*
+ * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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/err.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <asm/cpu.h>
+#include <mach/hardware.h>
+#include <mach/clock.h>
+
+static u32 *cpu_freqs;
+static u32 *cpu_volts;
+static u32 trans_latency;
+static int cpu_op_nr;
+
+static int cpu_freq_khz_min;
+static int cpu_freq_khz_max;
+
+static struct clk *cpu_clk;
+static struct cpufreq_frequency_table *arm_freq_table;
+
+static int set_cpu_freq(int freq)
+{
+	int ret = 0;
+	int org_cpu_rate;
+
+	org_cpu_rate = clk_get_rate(cpu_clk);
+	if (org_cpu_rate == freq)
+		return ret;
+
+	ret = clk_set_rate(cpu_clk, freq);
+	if (ret != 0) {
+		printk(KERN_DEBUG "cannot set CPU clock rate\n");
+		return ret;
+	}
+
+	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;
+	int freq_Hz, cpu;
+	int ret = 0;
+	unsigned int index;
+
+	cpufreq_frequency_table_target(policy, arm_freq_table,
+			target_freq, relation, &index);
+	freq_Hz = arm_freq_table[index].frequency * 1000;
+
+	freqs.old = clk_get_rate(cpu_clk) / 1000;
+	freqs.new = clk_round_rate(cpu_clk, freq_Hz);
+	freqs.new = (freqs.new ? 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);
+
+#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);
+	}
+
+	return ret;
+}
+
+static int arm_cpufreq_init(struct cpufreq_policy *policy)
+{
+	int ret;
+
+	printk(KERN_INFO "ARM CPU frequency driver\n");
+
+	if (policy->cpu >= num_possible_cpus())
+		return -EINVAL;
+
+	policy->cur = clk_get_rate(cpu_clk) / 1000;
+	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
+	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
+	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: failed to register i.MXC CPUfreq with error code %d\n",
+		       __func__, ret);
+		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);
+
+	set_cpu_freq(cpu_freq_khz_max * 1000);
+	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 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 == 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_freqs, 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;
+
+	cpu_freq_khz_min = cpu_freqs[0] / 1000;
+	cpu_freq_khz_max = cpu_freqs[0] / 1000;
+
+	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;
+		if ((cpu_freqs[i] / 1000) < cpu_freq_khz_min)
+			cpu_freq_khz_min = cpu_freqs[i] / 1000;
+		if ((cpu_freqs[i] / 1000) > cpu_freq_khz_max)
+			cpu_freq_khz_max = 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;
+	}
+
+	ret = cpufreq_register_driver(&arm_cpufreq_driver);
+	if (ret)
+		goto clock_put;
+
+	of_node_put(cpu0);
+
+	return 0;
+
+clock_put:
+	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");
-- 
1.7.5.4

[RFC V1 2/4] dts/imx6q: add cpufreq property

[Richard Zhao]

Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
---
 arch/arm/boot/dts/imx6q.dtsi |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 263e8f3..9e9943b 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -29,6 +29,8 @@
 			compatible = "arm,cortex-a9";
 			reg = <0>;
 			next-level-cache = <&L2>;
+			cpu_freqs = <996000000 792000000 396000000 198000000>;
+			cpu_volts = <1225000 1100000 950000 850000>;
 		};
 
 		cpu@1 {
-- 
1.7.5.4

[RFC V1 3/4] 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

[RFC V1 4/4] 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

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

TODO:
 - add voltage change.

Thanks
Richard 

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

TODO:
 - add voltage change.

Thanks
Richard 

Re: [RFC V1 2/4] dts/imx6q: add cpufreq property

[Shawn Guo]

Hi Richard,

Whenever we invent some new device tree binding support, we need to
Cc devicetree-discuss@lists.ozlabs.org (Cc-ed).

On Thu, Dec 15, 2011 at 07:16:36PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
>  arch/arm/boot/dts/imx6q.dtsi |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 263e8f3..9e9943b 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -29,6 +29,8 @@
>  			compatible = "arm,cortex-a9";
>  			reg = <0>;
>  			next-level-cache = <&L2>;
> +			cpu_freqs = <996000000 792000000 396000000 198000000>;
> +			cpu_volts = <1225000 1100000 950000 850000>;

For dt property name, we use '-' rather than '_'.

And I'm not sure this is correct, since these data obviously does not
belong to "arm,cortex-a9".

Regards,
Shawn

>  		};
>  
>  		cpu@1 {
> -- 
> 1.7.5.4

[RFC V1 2/4] dts/imx6q: add cpufreq property

[Shawn Guo]

Hi Richard,

Whenever we invent some new device tree binding support, we need to
Cc devicetree-discuss at lists.ozlabs.org (Cc-ed).

On Thu, Dec 15, 2011 at 07:16:36PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
>  arch/arm/boot/dts/imx6q.dtsi |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 263e8f3..9e9943b 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -29,6 +29,8 @@
>  			compatible = "arm,cortex-a9";
>  			reg = <0>;
>  			next-level-cache = <&L2>;
> +			cpu_freqs = <996000000 792000000 396000000 198000000>;
> +			cpu_volts = <1225000 1100000 950000 850000>;

For dt property name, we use '-' rather than '_'.

And I'm not sure this is correct, since these data obviously does not
belong to "arm,cortex-a9".

Regards,
Shawn

>  		};
>  
>  		cpu at 1 {
> -- 
> 1.7.5.4

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Mark Langsdorf]

Comments below. I tested this on the Calxeda Highbank SoC using
QEMU. I found one definite error and a few things I would change.

On 12/15/2011 05:16 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>
> ---

> diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c
> new file mode 100644
> index 0000000..fd9759f
> --- /dev/null
> +++ b/drivers/cpufreq/arm-cpufreq.c
> @@ -0,0 +1,260 @@
> +/*
> + * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * 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/err.h>
> +#include<linux/slab.h>
> +#include<linux/of.h>
> +#include<asm/cpu.h>
> +#include<mach/hardware.h>
> +#include<mach/clock.h>

These should probably be replaced by <linux/of_clk.h>. See
below for details.

> +
> +static u32 *cpu_freqs;
> +static u32 *cpu_volts;
> +static u32 trans_latency;
> +static int cpu_op_nr;
> +
> +static int cpu_freq_khz_min;
> +static int cpu_freq_khz_max;
> +
> +static struct clk *cpu_clk;
> +static struct cpufreq_frequency_table *arm_freq_table;
> +
> +static int set_cpu_freq(int freq)
> +{
> +	int ret = 0;
> +	int org_cpu_rate;
> +
> +	org_cpu_rate = clk_get_rate(cpu_clk);
> +	if (org_cpu_rate == freq)
> +		return ret;
> +
> +	ret = clk_set_rate(cpu_clk, freq);
> +	if (ret != 0) {
> +		printk(KERN_DEBUG "cannot set CPU clock rate\n");
> +		return ret;
> +	}
> +
> +	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;
> +	int freq_Hz, cpu;
> +	int ret = 0;
> +	unsigned int index;
> +
> +	cpufreq_frequency_table_target(policy, arm_freq_table,
> +			target_freq, relation,&index);
> +	freq_Hz = arm_freq_table[index].frequency * 1000;
> +
> +	freqs.old = clk_get_rate(cpu_clk) / 1000;
> +	freqs.new = clk_round_rate(cpu_clk, freq_Hz);
> +	freqs.new = (freqs.new ? 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);
> +
> +#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);
> +	}
> +
> +	return ret;
> +}
> +
> +static int arm_cpufreq_init(struct cpufreq_policy *policy)
> +{
> +	int ret;
> +
> +	printk(KERN_INFO "ARM CPU frequency driver\n");
> +
> +	if (policy->cpu>= num_possible_cpus())
> +		return -EINVAL;
> +
> +	policy->cur = clk_get_rate(cpu_clk) / 1000;
> +	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
> +	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
> +	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: failed to register i.MXC CPUfreq with error code %d\n",
> +		       __func__, ret);
> +		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);
> +
> +	set_cpu_freq(cpu_freq_khz_max * 1000);
> +	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 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 == 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_freqs, cpu_op_nr);

cpu_freqs should clearly be cpu_volts in this instance.

> +		} else
> +			printk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");
> +	}
> +
> +	if (of_property_read_u32(cpu0, "trans_latency",&trans_latency))
> +		trans_latency = CPUFREQ_ETERNAL;

I'm not sure this is an appropriate default value. I suspect it will do
very strange things on actual hardware that fails to define
trans_latency in the device tree.
> +
> +	cpu_freq_khz_min = cpu_freqs[0] / 1000;
> +	cpu_freq_khz_max = cpu_freqs[0] / 1000;
> +
> +	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;
> +		if ((cpu_freqs[i] / 1000)<  cpu_freq_khz_min)
> +			cpu_freq_khz_min = cpu_freqs[i] / 1000;
> +		if ((cpu_freqs[i] / 1000)>  cpu_freq_khz_max)
> +			cpu_freq_khz_max = 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;
> +	}

I'd prefer to see clk_get90 replaced with of_clk_get() and
get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
I resubmitted yesterday. The of_clk_get() doesn't require defining
an arm_clk structure, so it's slightly more portable for mach-
definitions. Also, the of_clk_get() method doesn't require
mach/clock.h and mach/hardware.h, which is good because most
of the mach- definitions don't include them.

--Mark Langsdorf
Calxeda, Inc.

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Mark Langsdorf]

Comments below. I tested this on the Calxeda Highbank SoC using
QEMU. I found one definite error and a few things I would change.

On 12/15/2011 05:16 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>
> ---

> diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c
> new file mode 100644
> index 0000000..fd9759f
> --- /dev/null
> +++ b/drivers/cpufreq/arm-cpufreq.c
> @@ -0,0 +1,260 @@
> +/*
> + * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * 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/err.h>
> +#include<linux/slab.h>
> +#include<linux/of.h>
> +#include<asm/cpu.h>
> +#include<mach/hardware.h>
> +#include<mach/clock.h>

These should probably be replaced by <linux/of_clk.h>. See
below for details.

> +
> +static u32 *cpu_freqs;
> +static u32 *cpu_volts;
> +static u32 trans_latency;
> +static int cpu_op_nr;
> +
> +static int cpu_freq_khz_min;
> +static int cpu_freq_khz_max;
> +
> +static struct clk *cpu_clk;
> +static struct cpufreq_frequency_table *arm_freq_table;
> +
> +static int set_cpu_freq(int freq)
> +{
> +	int ret = 0;
> +	int org_cpu_rate;
> +
> +	org_cpu_rate = clk_get_rate(cpu_clk);
> +	if (org_cpu_rate == freq)
> +		return ret;
> +
> +	ret = clk_set_rate(cpu_clk, freq);
> +	if (ret != 0) {
> +		printk(KERN_DEBUG "cannot set CPU clock rate\n");
> +		return ret;
> +	}
> +
> +	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;
> +	int freq_Hz, cpu;
> +	int ret = 0;
> +	unsigned int index;
> +
> +	cpufreq_frequency_table_target(policy, arm_freq_table,
> +			target_freq, relation,&index);
> +	freq_Hz = arm_freq_table[index].frequency * 1000;
> +
> +	freqs.old = clk_get_rate(cpu_clk) / 1000;
> +	freqs.new = clk_round_rate(cpu_clk, freq_Hz);
> +	freqs.new = (freqs.new ? 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);
> +
> +#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);
> +	}
> +
> +	return ret;
> +}
> +
> +static int arm_cpufreq_init(struct cpufreq_policy *policy)
> +{
> +	int ret;
> +
> +	printk(KERN_INFO "ARM CPU frequency driver\n");
> +
> +	if (policy->cpu>= num_possible_cpus())
> +		return -EINVAL;
> +
> +	policy->cur = clk_get_rate(cpu_clk) / 1000;
> +	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
> +	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
> +	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: failed to register i.MXC CPUfreq with error code %d\n",
> +		       __func__, ret);
> +		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);
> +
> +	set_cpu_freq(cpu_freq_khz_max * 1000);
> +	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 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 == 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_freqs, cpu_op_nr);

cpu_freqs should clearly be cpu_volts in this instance.

> +		} else
> +			printk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");
> +	}
> +
> +	if (of_property_read_u32(cpu0, "trans_latency",&trans_latency))
> +		trans_latency = CPUFREQ_ETERNAL;

I'm not sure this is an appropriate default value. I suspect it will do
very strange things on actual hardware that fails to define
trans_latency in the device tree.
> +
> +	cpu_freq_khz_min = cpu_freqs[0] / 1000;
> +	cpu_freq_khz_max = cpu_freqs[0] / 1000;
> +
> +	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;
> +		if ((cpu_freqs[i] / 1000)<  cpu_freq_khz_min)
> +			cpu_freq_khz_min = cpu_freqs[i] / 1000;
> +		if ((cpu_freqs[i] / 1000)>  cpu_freq_khz_max)
> +			cpu_freq_khz_max = 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;
> +	}

I'd prefer to see clk_get90 replaced with of_clk_get() and
get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
I resubmitted yesterday. The of_clk_get() doesn't require defining
an arm_clk structure, so it's slightly more portable for mach-
definitions. Also, the of_clk_get() method doesn't require
mach/clock.h and mach/hardware.h, which is good because most
of the mach- definitions don't include them.

--Mark Langsdorf
Calxeda, Inc.

[RFC V1 2/4] dts/imx6q: add cpufreq property

[Mark Langsdorf]

On 12/15/2011 05:16 AM, Richard Zhao wrote:
> Signed-off-by: Richard Zhao<richard.zhao@linaro.org>
> ---
>   arch/arm/boot/dts/imx6q.dtsi |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 263e8f3..9e9943b 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -29,6 +29,8 @@
>   			compatible = "arm,cortex-a9";
>   			reg =<0>;
>   			next-level-cache =<&L2>;
> +			cpu_freqs =<996000000 792000000 396000000 198000000>;
> +			cpu_volts =<1225000 1100000 950000 850000>;
+			trans_latency =<3000>;
>   		};
>
>   		cpu at 1 {

The trans_latency value definitely needs to be defined to avoid
strange behavior in the driver.

--Mark Langsdorf
Calxeda, Inc.

Re: [RFC V1 2/4] dts/imx6q: add cpufreq property

[Mark Langsdorf]

On 12/15/2011 05:16 AM, Richard Zhao wrote:
> Signed-off-by: Richard Zhao<richard.zhao@linaro.org>
> ---
>   arch/arm/boot/dts/imx6q.dtsi |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 263e8f3..9e9943b 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -29,6 +29,8 @@
>   			compatible = "arm,cortex-a9";
>   			reg =<0>;
>   			next-level-cache =<&L2>;
> +			cpu_freqs =<996000000 792000000 396000000 198000000>;
> +			cpu_volts =<1225000 1100000 950000 850000>;
+			trans_latency =<3000>;
>   		};
>
>   		cpu@1 {

The trans_latency value definitely needs to be defined to avoid
strange behavior in the driver.

--Mark Langsdorf
Calxeda, Inc.

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Russell King - ARM Linux]

On Thu, Dec 15, 2011 at 07:16:35PM +0800, Richard Zhao wrote:
> +#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);

NAK.  If you think this is a good solution, you're very wrong.  If this
is what's in the core cpufreq code, then it too is very broken.

I've seen this exact method result in the loops_per_jiffy being totally
buggered over time by the constant scaling up and down.  The only way
to do this _sensibly_ is to record the _initial_ loops_per_jiffy and
_inital_ frequency, and scale from that.

That way you get consistent results irrespective of the scalings you
do over time, rather than something which continually deteriorates with
every frequency change.

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Russell King - ARM Linux]

On Thu, Dec 15, 2011 at 07:16:35PM +0800, Richard Zhao wrote:
> +#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);

NAK.  If you think this is a good solution, you're very wrong.  If this
is what's in the core cpufreq code, then it too is very broken.

I've seen this exact method result in the loops_per_jiffy being totally
buggered over time by the constant scaling up and down.  The only way
to do this _sensibly_ is to record the _initial_ loops_per_jiffy and
_inital_ frequency, and scale from that.

That way you get consistent results irrespective of the scalings you
do over time, rather than something which continually deteriorates with
every frequency change.

Re: [RFC V1 2/4] dts/imx6q: add cpufreq property

[Richard Zhao]

On Thu, Dec 15, 2011 at 07:58:54PM +0800, Shawn Guo wrote:
> Hi Richard,
> 
> Whenever we invent some new device tree binding support, we need to
> Cc devicetree-discuss@lists.ozlabs.org (Cc-ed).
Thanks for your reminder.
> 
> On Thu, Dec 15, 2011 at 07:16:36PM +0800, Richard Zhao wrote:
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> >  arch/arm/boot/dts/imx6q.dtsi |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index 263e8f3..9e9943b 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -29,6 +29,8 @@
> >  			compatible = "arm,cortex-a9";
> >  			reg = <0>;
> >  			next-level-cache = <&L2>;
> > +			cpu_freqs = <996000000 792000000 396000000 198000000>;
> > +			cpu_volts = <1225000 1100000 950000 850000>;
> 
> For dt property name, we use '-' rather than '_'.
ok.
> 
> And I'm not sure this is correct, since these data obviously does not
> belong to "arm,cortex-a9".
That's why RFC.

Thanks
Richard
> 
> Regards,
> Shawn
> 
> >  		};
> >  
> >  		cpu@1 {
> > -- 
> > 1.7.5.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 2/4] dts/imx6q: add cpufreq property

[Richard Zhao]

On Thu, Dec 15, 2011 at 07:58:54PM +0800, Shawn Guo wrote:
> Hi Richard,
> 
> Whenever we invent some new device tree binding support, we need to
> Cc devicetree-discuss at lists.ozlabs.org (Cc-ed).
Thanks for your reminder.
> 
> On Thu, Dec 15, 2011 at 07:16:36PM +0800, Richard Zhao wrote:
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> >  arch/arm/boot/dts/imx6q.dtsi |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index 263e8f3..9e9943b 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -29,6 +29,8 @@
> >  			compatible = "arm,cortex-a9";
> >  			reg = <0>;
> >  			next-level-cache = <&L2>;
> > +			cpu_freqs = <996000000 792000000 396000000 198000000>;
> > +			cpu_volts = <1225000 1100000 950000 850000>;
> 
> For dt property name, we use '-' rather than '_'.
ok.
> 
> And I'm not sure this is correct, since these data obviously does not
> belong to "arm,cortex-a9".
That's why RFC.

Thanks
Richard
> 
> Regards,
> Shawn
> 
> >  		};
> >  
> >  		cpu at 1 {
> > -- 
> > 1.7.5.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

On Thu, Dec 15, 2011 at 12:50:07PM -0600, Mark Langsdorf wrote:
> Comments below. I tested this on the Calxeda Highbank SoC using
> QEMU. I found one definite error and a few things I would change.
Thanks for your test.
> 
> On 12/15/2011 05:16 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>
> >---
> 
> >diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c
> >new file mode 100644
> >index 0000000..fd9759f
> >--- /dev/null
> >+++ b/drivers/cpufreq/arm-cpufreq.c
> >@@ -0,0 +1,260 @@
> >+/*
> >+ * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> >+ */
> >+
> >+/*
> >+ * 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/err.h>
> >+#include<linux/slab.h>
> >+#include<linux/of.h>
> >+#include<asm/cpu.h>
> >+#include<mach/hardware.h>
> >+#include<mach/clock.h>
> 
> These should probably be replaced by <linux/of_clk.h>. See
> below for details.
I'll remove <mach/*>.
But are you sure clk DT binding has went to -next or -rc? If yes,
I'm glad to use it. If no, I don't want to pend on it.
> 
> >+
> >+static u32 *cpu_freqs;
> >+static u32 *cpu_volts;
> >+static u32 trans_latency;
> >+static int cpu_op_nr;
> >+
> >+static int cpu_freq_khz_min;
> >+static int cpu_freq_khz_max;
> >+
> >+static struct clk *cpu_clk;
> >+static struct cpufreq_frequency_table *arm_freq_table;
> >+
> >+static int set_cpu_freq(int freq)
> >+{
> >+	int ret = 0;
> >+	int org_cpu_rate;
> >+
> >+	org_cpu_rate = clk_get_rate(cpu_clk);
> >+	if (org_cpu_rate == freq)
> >+		return ret;
> >+
> >+	ret = clk_set_rate(cpu_clk, freq);
> >+	if (ret != 0) {
> >+		printk(KERN_DEBUG "cannot set CPU clock rate\n");
> >+		return ret;
> >+	}
> >+
> >+	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;
> >+	int freq_Hz, cpu;
> >+	int ret = 0;
> >+	unsigned int index;
> >+
> >+	cpufreq_frequency_table_target(policy, arm_freq_table,
> >+			target_freq, relation,&index);
> >+	freq_Hz = arm_freq_table[index].frequency * 1000;
> >+
> >+	freqs.old = clk_get_rate(cpu_clk) / 1000;
> >+	freqs.new = clk_round_rate(cpu_clk, freq_Hz);
> >+	freqs.new = (freqs.new ? 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);
> >+
> >+#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);
> >+	}
> >+
> >+	return ret;
> >+}
> >+
> >+static int arm_cpufreq_init(struct cpufreq_policy *policy)
> >+{
> >+	int ret;
> >+
> >+	printk(KERN_INFO "ARM CPU frequency driver\n");
> >+
> >+	if (policy->cpu>= num_possible_cpus())
> >+		return -EINVAL;
> >+
> >+	policy->cur = clk_get_rate(cpu_clk) / 1000;
> >+	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
> >+	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
> >+	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: failed to register i.MXC CPUfreq with error code %d\n",
> >+		       __func__, ret);
> >+		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);
> >+
> >+	set_cpu_freq(cpu_freq_khz_max * 1000);
> >+	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 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 == 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_freqs, cpu_op_nr);
> 
> cpu_freqs should clearly be cpu_volts in this instance.
Thanks.
> 
> >+		} else
> >+			printk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");
> >+	}
> >+
> >+	if (of_property_read_u32(cpu0, "trans_latency",&trans_latency))
> >+		trans_latency = CPUFREQ_ETERNAL;
> 
> I'm not sure this is an appropriate default value. I suspect it will do
> very strange things on actual hardware that fails to define
> trans_latency in the device tree.
CPUFREQ_ETERNAL breaks governor ondemand and conservative. But it's the
recommended default vaule in cpufreq doc.
> >+
> >+	cpu_freq_khz_min = cpu_freqs[0] / 1000;
> >+	cpu_freq_khz_max = cpu_freqs[0] / 1000;
> >+
> >+	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;
> >+		if ((cpu_freqs[i] / 1000)<  cpu_freq_khz_min)
> >+			cpu_freq_khz_min = cpu_freqs[i] / 1000;
> >+		if ((cpu_freqs[i] / 1000)>  cpu_freq_khz_max)
> >+			cpu_freq_khz_max = 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;
> >+	}
> 
> I'd prefer to see clk_get90 replaced with of_clk_get()
ditto
> and
> get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
> I resubmitted yesterday.
This driver only have one instance, because all cpu cores shares the same
clk and voltage. You can see cpumask_setall(policy->cpus).
And get_this_cpu_node() adds the dependency that cpufreq_driver.init must run
on the cpu the policy indicate, which is not correct.

Thanks for your comments
Richard
> The of_clk_get() doesn't require defining
> an arm_clk structure, so it's slightly more portable for mach-
> definitions. Also, the of_clk_get() method doesn't require
> mach/clock.h and mach/hardware.h, which is good because most
> of the mach- definitions don't include them.
> 
> --Mark Langsdorf
> Calxeda, Inc.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

On Thu, Dec 15, 2011 at 12:50:07PM -0600, Mark Langsdorf wrote:
> Comments below. I tested this on the Calxeda Highbank SoC using
> QEMU. I found one definite error and a few things I would change.
Thanks for your test.
> 
> On 12/15/2011 05:16 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>
> >---
> 
> >diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c
> >new file mode 100644
> >index 0000000..fd9759f
> >--- /dev/null
> >+++ b/drivers/cpufreq/arm-cpufreq.c
> >@@ -0,0 +1,260 @@
> >+/*
> >+ * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> >+ */
> >+
> >+/*
> >+ * 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/err.h>
> >+#include<linux/slab.h>
> >+#include<linux/of.h>
> >+#include<asm/cpu.h>
> >+#include<mach/hardware.h>
> >+#include<mach/clock.h>
> 
> These should probably be replaced by <linux/of_clk.h>. See
> below for details.
I'll remove <mach/*>.
But are you sure clk DT binding has went to -next or -rc? If yes,
I'm glad to use it. If no, I don't want to pend on it.
> 
> >+
> >+static u32 *cpu_freqs;
> >+static u32 *cpu_volts;
> >+static u32 trans_latency;
> >+static int cpu_op_nr;
> >+
> >+static int cpu_freq_khz_min;
> >+static int cpu_freq_khz_max;
> >+
> >+static struct clk *cpu_clk;
> >+static struct cpufreq_frequency_table *arm_freq_table;
> >+
> >+static int set_cpu_freq(int freq)
> >+{
> >+	int ret = 0;
> >+	int org_cpu_rate;
> >+
> >+	org_cpu_rate = clk_get_rate(cpu_clk);
> >+	if (org_cpu_rate == freq)
> >+		return ret;
> >+
> >+	ret = clk_set_rate(cpu_clk, freq);
> >+	if (ret != 0) {
> >+		printk(KERN_DEBUG "cannot set CPU clock rate\n");
> >+		return ret;
> >+	}
> >+
> >+	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;
> >+	int freq_Hz, cpu;
> >+	int ret = 0;
> >+	unsigned int index;
> >+
> >+	cpufreq_frequency_table_target(policy, arm_freq_table,
> >+			target_freq, relation,&index);
> >+	freq_Hz = arm_freq_table[index].frequency * 1000;
> >+
> >+	freqs.old = clk_get_rate(cpu_clk) / 1000;
> >+	freqs.new = clk_round_rate(cpu_clk, freq_Hz);
> >+	freqs.new = (freqs.new ? 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);
> >+
> >+#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);
> >+	}
> >+
> >+	return ret;
> >+}
> >+
> >+static int arm_cpufreq_init(struct cpufreq_policy *policy)
> >+{
> >+	int ret;
> >+
> >+	printk(KERN_INFO "ARM CPU frequency driver\n");
> >+
> >+	if (policy->cpu>= num_possible_cpus())
> >+		return -EINVAL;
> >+
> >+	policy->cur = clk_get_rate(cpu_clk) / 1000;
> >+	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
> >+	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
> >+	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: failed to register i.MXC CPUfreq with error code %d\n",
> >+		       __func__, ret);
> >+		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);
> >+
> >+	set_cpu_freq(cpu_freq_khz_max * 1000);
> >+	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 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 == 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_freqs, cpu_op_nr);
> 
> cpu_freqs should clearly be cpu_volts in this instance.
Thanks.
> 
> >+		} else
> >+			printk(KERN_WARNING "cpufreq: invalid cpu_volts!\n");
> >+	}
> >+
> >+	if (of_property_read_u32(cpu0, "trans_latency",&trans_latency))
> >+		trans_latency = CPUFREQ_ETERNAL;
> 
> I'm not sure this is an appropriate default value. I suspect it will do
> very strange things on actual hardware that fails to define
> trans_latency in the device tree.
CPUFREQ_ETERNAL breaks governor ondemand and conservative. But it's the
recommended default vaule in cpufreq doc.
> >+
> >+	cpu_freq_khz_min = cpu_freqs[0] / 1000;
> >+	cpu_freq_khz_max = cpu_freqs[0] / 1000;
> >+
> >+	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;
> >+		if ((cpu_freqs[i] / 1000)<  cpu_freq_khz_min)
> >+			cpu_freq_khz_min = cpu_freqs[i] / 1000;
> >+		if ((cpu_freqs[i] / 1000)>  cpu_freq_khz_max)
> >+			cpu_freq_khz_max = 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;
> >+	}
> 
> I'd prefer to see clk_get90 replaced with of_clk_get()
ditto
> and
> get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
> I resubmitted yesterday.
This driver only have one instance, because all cpu cores shares the same
clk and voltage. You can see cpumask_setall(policy->cpus).
And get_this_cpu_node() adds the dependency that cpufreq_driver.init must run
on the cpu the policy indicate, which is not correct.

Thanks for your comments
Richard
> The of_clk_get() doesn't require defining
> an arm_clk structure, so it's slightly more portable for mach-
> definitions. Also, the of_clk_get() method doesn't require
> mach/clock.h and mach/hardware.h, which is good because most
> of the mach- definitions don't include them.
> 
> --Mark Langsdorf
> Calxeda, Inc.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

On Thu, Dec 15, 2011 at 08:29:11PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2011 at 07:16:35PM +0800, Richard Zhao wrote:
> > +#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);
> 
> NAK.  If you think this is a good solution, you're very wrong.  If this
> is what's in the core cpufreq code, then it too is very broken.
> 
> I've seen this exact method result in the loops_per_jiffy being totally
> buggered over time by the constant scaling up and down.  The only way
> to do this _sensibly_ is to record the _initial_ loops_per_jiffy and
> _inital_ frequency, and scale from that.
> 
> That way you get consistent results irrespective of the scalings you
> do over time, rather than something which continually deteriorates with
> every frequency change.
Thanks for your comments. I'll recalculate loops_per_jiffy always based on boot
up values.

Thanks
Richard
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

On Thu, Dec 15, 2011 at 08:29:11PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2011 at 07:16:35PM +0800, Richard Zhao wrote:
> > +#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);
> 
> NAK.  If you think this is a good solution, you're very wrong.  If this
> is what's in the core cpufreq code, then it too is very broken.
> 
> I've seen this exact method result in the loops_per_jiffy being totally
> buggered over time by the constant scaling up and down.  The only way
> to do this _sensibly_ is to record the _initial_ loops_per_jiffy and
> _inital_ frequency, and scale from that.
> 
> That way you get consistent results irrespective of the scalings you
> do over time, rather than something which continually deteriorates with
> every frequency change.
Thanks for your comments. I'll recalculate loops_per_jiffy always based on boot
up values.

Thanks
Richard
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Russell King - ARM Linux]

On Thu, Dec 15, 2011 at 12:50:07PM -0600, Mark Langsdorf wrote:
> I'd prefer to see clk_get90 replaced with of_clk_get() and
> get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
> I resubmitted yesterday.

Why isn't of_clk_get() hidden inside clk_get() ?

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Russell King - ARM Linux]

On Thu, Dec 15, 2011 at 12:50:07PM -0600, Mark Langsdorf wrote:
> I'd prefer to see clk_get90 replaced with of_clk_get() and
> get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
> I resubmitted yesterday.

Why isn't of_clk_get() hidden inside clk_get() ?

[RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

On Fri, Dec 16, 2011 at 08:24:23AM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2011 at 12:50:07PM -0600, Mark Langsdorf wrote:
> > I'd prefer to see clk_get90 replaced with of_clk_get() and
> > get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
> > I resubmitted yesterday.
> 
> Why isn't of_clk_get() hidden inside clk_get() ?
Good point.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC V1 1/4] cpufreq: add arm soc generic cpufreq driver

[Richard Zhao]

On Fri, Dec 16, 2011 at 08:24:23AM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2011 at 12:50:07PM -0600, Mark Langsdorf wrote:
> > I'd prefer to see clk_get90 replaced with of_clk_get() and
> > get_this_cpu_node() from the clk-cpufreq driver by Jamie Iles that
> > I resubmitted yesterday.
> 
> Why isn't of_clk_get() hidden inside clk_get() ?
Good point.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>