All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver
@ 2012-08-10  5:37 ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel, Shawn Guo

Patch "cpufreq: Add a generic cpufreq-cpu0 driver" has a dependency
on following branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/tol

Changes since v2:
* Drop patch "regulator: add a new API regulator_set_voltage_tol()",
  which has been applied on regulator tree as above.
* Rewrite function of_init_opp_table to make it concise.
  
Regards,
Shawn

Richard Zhao (1):
  ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp

Shawn Guo (2):
  PM / OPP: Initialize OPP table from device tree
  cpufreq: Add a generic cpufreq-cpu0 driver

 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
 Documentation/devicetree/bindings/power/opp.txt    |   25 ++
 arch/arm/kernel/smp.c                              |   54 ++++
 drivers/base/power/opp.c                           |   47 ++++
 drivers/cpufreq/Kconfig                            |   11 +
 drivers/cpufreq/Makefile                           |    2 +
 drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
 include/linux/opp.h                                |    8 +
 8 files changed, 473 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
 create mode 100644 Documentation/devicetree/bindings/power/opp.txt
 create mode 100644 drivers/cpufreq/cpufreq-cpu0.c

-- 
1.7.5.4



^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver
@ 2012-08-10  5:37 ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: linux-arm-kernel

Patch "cpufreq: Add a generic cpufreq-cpu0 driver" has a dependency
on following branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/tol

Changes since v2:
* Drop patch "regulator: add a new API regulator_set_voltage_tol()",
  which has been applied on regulator tree as above.
* Rewrite function of_init_opp_table to make it concise.
  
Regards,
Shawn

Richard Zhao (1):
  ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp

Shawn Guo (2):
  PM / OPP: Initialize OPP table from device tree
  cpufreq: Add a generic cpufreq-cpu0 driver

 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
 Documentation/devicetree/bindings/power/opp.txt    |   25 ++
 arch/arm/kernel/smp.c                              |   54 ++++
 drivers/base/power/opp.c                           |   47 ++++
 drivers/cpufreq/Kconfig                            |   11 +
 drivers/cpufreq/Makefile                           |    2 +
 drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
 include/linux/opp.h                                |    8 +
 8 files changed, 473 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
 create mode 100644 Documentation/devicetree/bindings/power/opp.txt
 create mode 100644 drivers/cpufreq/cpufreq-cpu0.c

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-08-10  5:37 ` Shawn Guo
@ 2012-08-10  5:37   ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel, Richard Zhao, Shawn Guo

From: Richard Zhao <richard.zhao@linaro.org>

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>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Shawn Guo <shawn.guo@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 ebd8ad2..8e03567 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>
@@ -584,3 +585,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



^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-08-10  5:37   ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: linux-arm-kernel

From: Richard Zhao <richard.zhao@linaro.org>

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>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Shawn Guo <shawn.guo@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 ebd8ad2..8e03567 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>
@@ -584,3 +585,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

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
  2012-08-10  5:37 ` Shawn Guo
@ 2012-08-10  5:37   ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel, Shawn Guo

With a lot of devices booting from device tree nowadays, it requires
that OPP table can be initialized from device tree.  The patch adds
a helper function of_init_opp_table together with a binding doc for
that purpose.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 Documentation/devicetree/bindings/power/opp.txt |   25 ++++++++++++
 drivers/base/power/opp.c                        |   47 +++++++++++++++++++++++
 include/linux/opp.h                             |    8 ++++
 3 files changed, 80 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/opp.txt

diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
new file mode 100644
index 0000000..74499e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -0,0 +1,25 @@
+* Generic OPP Interface
+
+SoCs have a standard set of tuples consisting of frequency and
+voltage pairs that the device will support per voltage domain. These
+are called Operating Performance Points or OPPs.
+
+Properties:
+- operating-points: An array of 2-tuples items, and each item consists
+  of frequency and voltage like <freq-kHz vol-uV>.
+	freq: clock frequency in kHz
+	vol: voltage in microvolt
+
+Examples:
+
+cpu@0 {
+	compatible = "arm,cortex-a9";
+	reg = <0>;
+	next-level-cache = <&L2>;
+	operating-points = <
+		/* kHz    uV */
+		792000  1100000
+		396000  950000
+		198000  850000
+	>;
+};
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index ac993ea..d946864 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -22,6 +22,7 @@
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
 #include <linux/opp.h>
+#include <linux/of.h>
 
 /*
  * Internal data structure organization with the OPP layer library is as
@@ -674,3 +675,49 @@ struct srcu_notifier_head *opp_get_notifier(struct device *dev)
 
 	return &dev_opp->head;
 }
+
+#ifdef CONFIG_OF
+/**
+ * of_init_opp_table() - Initialize opp table from device tree
+ * @dev:	device pointer used to lookup device OPPs.
+ *
+ * Register the initial OPP table with the OPP library for given device.
+ */
+int of_init_opp_table(struct device *dev)
+{
+	const struct property *prop;
+	const __be32 *val;
+	int nr;
+
+	prop = of_find_property(dev->of_node, "operating-points", NULL);
+	if (!prop)
+		return -ENODEV;
+	if (!prop->value)
+		return -ENODATA;
+
+	/*
+	 * Each OPP is a set of tuples consisting of frequency and
+	 * voltage like <freq-kHz vol-uV>.
+	 */
+	nr = prop->length / sizeof(u32);
+	if (nr % 2) {
+		dev_err(dev, "%s: Invalid OPP list\n", __func__);
+		return -EINVAL;
+	}
+
+	val = prop->value;
+	while (nr) {
+		unsigned long freq = be32_to_cpup(val++) * 1000;
+		unsigned long volt = be32_to_cpup(val++);
+
+		if (opp_add(dev, freq, volt)) {
+			dev_warn(dev, "%s: Failed to add OPP %ld\n",
+				 __func__, freq);
+			continue;
+		}
+		nr -= 2;
+	}
+
+	return 0;
+}
+#endif
diff --git a/include/linux/opp.h b/include/linux/opp.h
index 2a4e5fa..214e0eb 100644
--- a/include/linux/opp.h
+++ b/include/linux/opp.h
@@ -48,6 +48,14 @@ int opp_disable(struct device *dev, unsigned long freq);
 
 struct srcu_notifier_head *opp_get_notifier(struct device *dev);
 
+#ifdef CONFIG_OF
+int of_init_opp_table(struct device *dev);
+#else
+static inline int of_init_opp_table(struct device *dev)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_OF */
 #else
 static inline unsigned long opp_get_voltage(struct opp *opp)
 {
-- 
1.7.5.4



^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
@ 2012-08-10  5:37   ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: linux-arm-kernel

With a lot of devices booting from device tree nowadays, it requires
that OPP table can be initialized from device tree.  The patch adds
a helper function of_init_opp_table together with a binding doc for
that purpose.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 Documentation/devicetree/bindings/power/opp.txt |   25 ++++++++++++
 drivers/base/power/opp.c                        |   47 +++++++++++++++++++++++
 include/linux/opp.h                             |    8 ++++
 3 files changed, 80 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/opp.txt

diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
new file mode 100644
index 0000000..74499e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -0,0 +1,25 @@
+* Generic OPP Interface
+
+SoCs have a standard set of tuples consisting of frequency and
+voltage pairs that the device will support per voltage domain. These
+are called Operating Performance Points or OPPs.
+
+Properties:
+- operating-points: An array of 2-tuples items, and each item consists
+  of frequency and voltage like <freq-kHz vol-uV>.
+	freq: clock frequency in kHz
+	vol: voltage in microvolt
+
+Examples:
+
+cpu at 0 {
+	compatible = "arm,cortex-a9";
+	reg = <0>;
+	next-level-cache = <&L2>;
+	operating-points = <
+		/* kHz    uV */
+		792000  1100000
+		396000  950000
+		198000  850000
+	>;
+};
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index ac993ea..d946864 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -22,6 +22,7 @@
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
 #include <linux/opp.h>
+#include <linux/of.h>
 
 /*
  * Internal data structure organization with the OPP layer library is as
@@ -674,3 +675,49 @@ struct srcu_notifier_head *opp_get_notifier(struct device *dev)
 
 	return &dev_opp->head;
 }
+
+#ifdef CONFIG_OF
+/**
+ * of_init_opp_table() - Initialize opp table from device tree
+ * @dev:	device pointer used to lookup device OPPs.
+ *
+ * Register the initial OPP table with the OPP library for given device.
+ */
+int of_init_opp_table(struct device *dev)
+{
+	const struct property *prop;
+	const __be32 *val;
+	int nr;
+
+	prop = of_find_property(dev->of_node, "operating-points", NULL);
+	if (!prop)
+		return -ENODEV;
+	if (!prop->value)
+		return -ENODATA;
+
+	/*
+	 * Each OPP is a set of tuples consisting of frequency and
+	 * voltage like <freq-kHz vol-uV>.
+	 */
+	nr = prop->length / sizeof(u32);
+	if (nr % 2) {
+		dev_err(dev, "%s: Invalid OPP list\n", __func__);
+		return -EINVAL;
+	}
+
+	val = prop->value;
+	while (nr) {
+		unsigned long freq = be32_to_cpup(val++) * 1000;
+		unsigned long volt = be32_to_cpup(val++);
+
+		if (opp_add(dev, freq, volt)) {
+			dev_warn(dev, "%s: Failed to add OPP %ld\n",
+				 __func__, freq);
+			continue;
+		}
+		nr -= 2;
+	}
+
+	return 0;
+}
+#endif
diff --git a/include/linux/opp.h b/include/linux/opp.h
index 2a4e5fa..214e0eb 100644
--- a/include/linux/opp.h
+++ b/include/linux/opp.h
@@ -48,6 +48,14 @@ int opp_disable(struct device *dev, unsigned long freq);
 
 struct srcu_notifier_head *opp_get_notifier(struct device *dev);
 
+#ifdef CONFIG_OF
+int of_init_opp_table(struct device *dev);
+#else
+static inline int of_init_opp_table(struct device *dev)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_OF */
 #else
 static inline unsigned long opp_get_voltage(struct opp *opp)
 {
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-08-10  5:37 ` Shawn Guo
@ 2012-08-10  5:37   ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel, Shawn Guo

It adds a generic cpufreq driver for CPU0 frequency management based on
clk, regulator, OPP and device tree support.  It can support both
uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
share clock and voltage across all CPUs.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
 drivers/cpufreq/Kconfig                            |   11 +
 drivers/cpufreq/Makefile                           |    2 +
 drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
 4 files changed, 339 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
 create mode 100644 drivers/cpufreq/cpufreq-cpu0.c

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
new file mode 100644
index 0000000..4416ccc
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -0,0 +1,55 @@
+Generic CPU0 cpufreq driver
+
+It is a generic cpufreq driver for CPU0 frequency management.  It
+supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
+systems which share clock and voltage across all CPUs.
+
+Both required and optional properties listed below must be defined
+under node /cpus/cpu@0.
+
+Required properties:
+- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
+  for details
+
+Optional properties:
+- clock-latency: Specify the possible maximum transition latency for clock,
+  in unit of nanoseconds.
+- voltage-tolerance: Specify the CPU voltage tolerance in percentage.
+
+Examples:
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu@0 {
+		compatible = "arm,cortex-a9";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		transition-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu@1 {
+		compatible = "arm,cortex-a9";
+		reg = <1>;
+		next-level-cache = <&L2>;
+	};
+
+	cpu@2 {
+		compatible = "arm,cortex-a9";
+		reg = <2>;
+		next-level-cache = <&L2>;
+	};
+
+	cpu@3 {
+		compatible = "arm,cortex-a9";
+		reg = <3>;
+		next-level-cache = <&L2>;
+	};
+};
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index e24a2a1..ea512f4 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -179,6 +179,17 @@ config CPU_FREQ_GOV_CONSERVATIVE
 
 	  If in doubt, say N.
 
+config GENERIC_CPUFREQ_CPU0
+	bool "Generic CPU0 cpufreq driver"
+	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
+	select CPU_FREQ_TABLE
+	help
+	  This adds a generic cpufreq driver for CPU0 frequency management.
+	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
+	  systems which share clock and voltage across all CPUs.
+
+	  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 9531fc2..a378ed2 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_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
+
 ##################################################################################
 # x86 drivers.
 # Link order matters. K8 is preferred to ACPI because of firmware bugs in early
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
new file mode 100644
index 0000000..fd3bac6
--- /dev/null
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -0,0 +1,271 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
+ *
+ * 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/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/opp.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+static unsigned int transition_latency;
+static unsigned int voltage_tolerance; /* in percentage */
+
+static struct device *cpu_dev;
+static struct clk *cpu_clk;
+static struct regulator *cpu_reg;
+static struct cpufreq_frequency_table *freq_table;
+
+static int cpu0_verify_speed(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, freq_table);
+}
+
+static unsigned int cpu0_get_speed(unsigned int cpu)
+{
+	return clk_get_rate(cpu_clk) / 1000;
+}
+
+static int cpu0_set_target(struct cpufreq_policy *policy,
+			   unsigned int target_freq, unsigned int relation)
+{
+	struct cpufreq_freqs freqs;
+	struct opp *opp;
+	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
+	unsigned int index, cpu;
+	int ret;
+
+	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
+					     relation, &index);
+	if (ret) {
+		pr_err("failed to match target freqency %d: %d\n",
+		       target_freq, ret);
+		return ret;
+	}
+
+	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
+	if (freq_Hz < 0)
+		freq_Hz = freq_table[index].frequency * 1000;
+	freqs.new = freq_Hz / 1000;
+	freqs.old = clk_get_rate(cpu_clk) / 1000;
+
+	if (freqs.old == freqs.new)
+		return 0;
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	}
+
+	if (cpu_reg) {
+		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
+		if (IS_ERR(opp)) {
+			pr_err("failed to find OPP for %ld\n", freq_Hz);
+			return PTR_ERR(opp);
+		}
+		volt = opp_get_voltage(opp);
+		tol = volt * voltage_tolerance / 100;
+		volt_old = regulator_get_voltage(cpu_reg);
+	}
+
+	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
+		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
+		 freqs.new / 1000, volt ? volt / 1000 : -1);
+
+	/* scaling up?  scale voltage before frequency */
+	if (cpu_reg && freqs.new > freqs.old) {
+		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage up: %d\n", ret);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
+	if (ret) {
+		pr_err("failed to set clock rate: %d\n", ret);
+		if (cpu_reg)
+			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
+		return ret;
+	}
+
+	/* scaling down?  scale voltage after frequency */
+	if (cpu_reg && freqs.new < freqs.old) {
+		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage down: %d\n", ret);
+			clk_set_rate(cpu_clk, freqs.old * 1000);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	}
+
+	return 0;
+}
+
+static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
+{
+	int ret;
+
+	if (policy->cpu != 0)
+		return -EINVAL;
+
+	ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+	if (ret) {
+		pr_err("invalid frequency table: %d\n", ret);
+		return ret;
+	}
+
+	policy->cpuinfo.transition_latency = transition_latency;
+	policy->cur = clk_get_rate(cpu_clk) / 1000;
+
+	/*
+	 * The driver only supports the SMP configuartion where all processors
+	 * share the clock and voltage and clock.  Use cpufreq affected_cpus
+	 * interface to have all CPUs scaled together.
+	 */
+	policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
+	cpumask_setall(policy->cpus);
+
+	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
+	return 0;
+}
+
+static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
+{
+	cpufreq_frequency_table_put_attr(policy->cpu);
+
+	return 0;
+}
+
+static struct freq_attr *cpu0_cpufreq_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+static struct cpufreq_driver cpu0_cpufreq_driver = {
+	.flags = CPUFREQ_STICKY,
+	.verify = cpu0_verify_speed,
+	.target = cpu0_set_target,
+	.get = cpu0_get_speed,
+	.init = cpu0_cpufreq_init,
+	.exit = cpu0_cpufreq_exit,
+	.name = "generic_cpu0",
+	.attr = cpu0_cpufreq_attr,
+};
+
+static int __devinit cpu0_cpufreq_driver_init(void)
+{
+	struct device_node *np;
+	int ret;
+
+	np = of_find_node_by_path("/cpus/cpu@0");
+	if (!np) {
+		pr_err("failed to find cpu0 node\n");
+		return -ENOENT;
+	}
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		pr_err("failed to get cpu0 device\n");
+		ret = -ENODEV;
+		goto out_put_node;
+	}
+
+	cpu_dev->of_node = np;
+
+	cpu_clk = clk_get(cpu_dev, NULL);
+	if (IS_ERR(cpu_clk)) {
+		ret = PTR_ERR(cpu_clk);
+		pr_err("failed to get cpu0 clock: %d\n", ret);
+		goto out_put_node;
+	}
+
+	cpu_reg = regulator_get(cpu_dev, "cpu0");
+	if (IS_ERR(cpu_reg)) {
+		pr_warn("failed to get cpu0 regulator\n");
+		cpu_reg = NULL;
+	}
+
+	ret = of_init_opp_table(cpu_dev);
+	if (ret) {
+		pr_err("failed to init OPP table: %d\n", ret);
+		goto out_put_node;
+	}
+
+	ret = opp_init_cpufreq_table(cpu_dev, &freq_table);
+	if (ret) {
+		pr_err("failed to init cpufreq table: %d\n", ret);
+		goto out_put_node;
+	}
+
+	of_property_read_u32(np, "voltage-tolerance", &voltage_tolerance);
+
+	if (of_property_read_u32(np, "clock-latency", &transition_latency))
+		transition_latency = CPUFREQ_ETERNAL;
+
+	if (cpu_reg) {
+		struct opp *opp;
+		unsigned long min_uV, max_uV;
+		int i;
+
+		/*
+		 * OPP is maintained in order of increasing frequency, and
+		 * freq_table initialised from OPP is therefore sorted in the
+		 * same order.
+		 */
+		for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
+			;
+		opp = opp_find_freq_exact(cpu_dev,
+				freq_table[0].frequency * 1000, true);
+		min_uV = opp_get_voltage(opp);
+		opp = opp_find_freq_exact(cpu_dev,
+				freq_table[i-1].frequency * 1000, true);
+		max_uV = opp_get_voltage(opp);
+		ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
+		if (ret > 0)
+			transition_latency += ret * 1000;
+	}
+
+	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
+	if (ret) {
+		pr_err("failed register driver: %d\n", ret);
+		goto out_free_table;
+	}
+
+	of_node_put(np);
+	return 0;
+
+out_free_table:
+	opp_free_cpufreq_table(cpu_dev, &freq_table);
+out_put_node:
+	of_node_put(np);
+	return ret;
+}
+late_initcall(cpu0_cpufreq_driver_init);
+
+MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
+MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
+MODULE_LICENSE("GPL");
-- 
1.7.5.4



^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-08-10  5:37   ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-10  5:37 UTC (permalink / raw)
  To: linux-arm-kernel

It adds a generic cpufreq driver for CPU0 frequency management based on
clk, regulator, OPP and device tree support.  It can support both
uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
share clock and voltage across all CPUs.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
 drivers/cpufreq/Kconfig                            |   11 +
 drivers/cpufreq/Makefile                           |    2 +
 drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
 4 files changed, 339 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
 create mode 100644 drivers/cpufreq/cpufreq-cpu0.c

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
new file mode 100644
index 0000000..4416ccc
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -0,0 +1,55 @@
+Generic CPU0 cpufreq driver
+
+It is a generic cpufreq driver for CPU0 frequency management.  It
+supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
+systems which share clock and voltage across all CPUs.
+
+Both required and optional properties listed below must be defined
+under node /cpus/cpu at 0.
+
+Required properties:
+- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
+  for details
+
+Optional properties:
+- clock-latency: Specify the possible maximum transition latency for clock,
+  in unit of nanoseconds.
+- voltage-tolerance: Specify the CPU voltage tolerance in percentage.
+
+Examples:
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu at 0 {
+		compatible = "arm,cortex-a9";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		transition-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu at 1 {
+		compatible = "arm,cortex-a9";
+		reg = <1>;
+		next-level-cache = <&L2>;
+	};
+
+	cpu at 2 {
+		compatible = "arm,cortex-a9";
+		reg = <2>;
+		next-level-cache = <&L2>;
+	};
+
+	cpu at 3 {
+		compatible = "arm,cortex-a9";
+		reg = <3>;
+		next-level-cache = <&L2>;
+	};
+};
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index e24a2a1..ea512f4 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -179,6 +179,17 @@ config CPU_FREQ_GOV_CONSERVATIVE
 
 	  If in doubt, say N.
 
+config GENERIC_CPUFREQ_CPU0
+	bool "Generic CPU0 cpufreq driver"
+	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
+	select CPU_FREQ_TABLE
+	help
+	  This adds a generic cpufreq driver for CPU0 frequency management.
+	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
+	  systems which share clock and voltage across all CPUs.
+
+	  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 9531fc2..a378ed2 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_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
+
 ##################################################################################
 # x86 drivers.
 # Link order matters. K8 is preferred to ACPI because of firmware bugs in early
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
new file mode 100644
index 0000000..fd3bac6
--- /dev/null
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -0,0 +1,271 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
+ *
+ * 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/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/opp.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+static unsigned int transition_latency;
+static unsigned int voltage_tolerance; /* in percentage */
+
+static struct device *cpu_dev;
+static struct clk *cpu_clk;
+static struct regulator *cpu_reg;
+static struct cpufreq_frequency_table *freq_table;
+
+static int cpu0_verify_speed(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, freq_table);
+}
+
+static unsigned int cpu0_get_speed(unsigned int cpu)
+{
+	return clk_get_rate(cpu_clk) / 1000;
+}
+
+static int cpu0_set_target(struct cpufreq_policy *policy,
+			   unsigned int target_freq, unsigned int relation)
+{
+	struct cpufreq_freqs freqs;
+	struct opp *opp;
+	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
+	unsigned int index, cpu;
+	int ret;
+
+	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
+					     relation, &index);
+	if (ret) {
+		pr_err("failed to match target freqency %d: %d\n",
+		       target_freq, ret);
+		return ret;
+	}
+
+	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
+	if (freq_Hz < 0)
+		freq_Hz = freq_table[index].frequency * 1000;
+	freqs.new = freq_Hz / 1000;
+	freqs.old = clk_get_rate(cpu_clk) / 1000;
+
+	if (freqs.old == freqs.new)
+		return 0;
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	}
+
+	if (cpu_reg) {
+		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
+		if (IS_ERR(opp)) {
+			pr_err("failed to find OPP for %ld\n", freq_Hz);
+			return PTR_ERR(opp);
+		}
+		volt = opp_get_voltage(opp);
+		tol = volt * voltage_tolerance / 100;
+		volt_old = regulator_get_voltage(cpu_reg);
+	}
+
+	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
+		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
+		 freqs.new / 1000, volt ? volt / 1000 : -1);
+
+	/* scaling up?  scale voltage before frequency */
+	if (cpu_reg && freqs.new > freqs.old) {
+		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage up: %d\n", ret);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
+	if (ret) {
+		pr_err("failed to set clock rate: %d\n", ret);
+		if (cpu_reg)
+			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
+		return ret;
+	}
+
+	/* scaling down?  scale voltage after frequency */
+	if (cpu_reg && freqs.new < freqs.old) {
+		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage down: %d\n", ret);
+			clk_set_rate(cpu_clk, freqs.old * 1000);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	}
+
+	return 0;
+}
+
+static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
+{
+	int ret;
+
+	if (policy->cpu != 0)
+		return -EINVAL;
+
+	ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+	if (ret) {
+		pr_err("invalid frequency table: %d\n", ret);
+		return ret;
+	}
+
+	policy->cpuinfo.transition_latency = transition_latency;
+	policy->cur = clk_get_rate(cpu_clk) / 1000;
+
+	/*
+	 * The driver only supports the SMP configuartion where all processors
+	 * share the clock and voltage and clock.  Use cpufreq affected_cpus
+	 * interface to have all CPUs scaled together.
+	 */
+	policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
+	cpumask_setall(policy->cpus);
+
+	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
+	return 0;
+}
+
+static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
+{
+	cpufreq_frequency_table_put_attr(policy->cpu);
+
+	return 0;
+}
+
+static struct freq_attr *cpu0_cpufreq_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+static struct cpufreq_driver cpu0_cpufreq_driver = {
+	.flags = CPUFREQ_STICKY,
+	.verify = cpu0_verify_speed,
+	.target = cpu0_set_target,
+	.get = cpu0_get_speed,
+	.init = cpu0_cpufreq_init,
+	.exit = cpu0_cpufreq_exit,
+	.name = "generic_cpu0",
+	.attr = cpu0_cpufreq_attr,
+};
+
+static int __devinit cpu0_cpufreq_driver_init(void)
+{
+	struct device_node *np;
+	int ret;
+
+	np = of_find_node_by_path("/cpus/cpu at 0");
+	if (!np) {
+		pr_err("failed to find cpu0 node\n");
+		return -ENOENT;
+	}
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		pr_err("failed to get cpu0 device\n");
+		ret = -ENODEV;
+		goto out_put_node;
+	}
+
+	cpu_dev->of_node = np;
+
+	cpu_clk = clk_get(cpu_dev, NULL);
+	if (IS_ERR(cpu_clk)) {
+		ret = PTR_ERR(cpu_clk);
+		pr_err("failed to get cpu0 clock: %d\n", ret);
+		goto out_put_node;
+	}
+
+	cpu_reg = regulator_get(cpu_dev, "cpu0");
+	if (IS_ERR(cpu_reg)) {
+		pr_warn("failed to get cpu0 regulator\n");
+		cpu_reg = NULL;
+	}
+
+	ret = of_init_opp_table(cpu_dev);
+	if (ret) {
+		pr_err("failed to init OPP table: %d\n", ret);
+		goto out_put_node;
+	}
+
+	ret = opp_init_cpufreq_table(cpu_dev, &freq_table);
+	if (ret) {
+		pr_err("failed to init cpufreq table: %d\n", ret);
+		goto out_put_node;
+	}
+
+	of_property_read_u32(np, "voltage-tolerance", &voltage_tolerance);
+
+	if (of_property_read_u32(np, "clock-latency", &transition_latency))
+		transition_latency = CPUFREQ_ETERNAL;
+
+	if (cpu_reg) {
+		struct opp *opp;
+		unsigned long min_uV, max_uV;
+		int i;
+
+		/*
+		 * OPP is maintained in order of increasing frequency, and
+		 * freq_table initialised from OPP is therefore sorted in the
+		 * same order.
+		 */
+		for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
+			;
+		opp = opp_find_freq_exact(cpu_dev,
+				freq_table[0].frequency * 1000, true);
+		min_uV = opp_get_voltage(opp);
+		opp = opp_find_freq_exact(cpu_dev,
+				freq_table[i-1].frequency * 1000, true);
+		max_uV = opp_get_voltage(opp);
+		ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
+		if (ret > 0)
+			transition_latency += ret * 1000;
+	}
+
+	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
+	if (ret) {
+		pr_err("failed register driver: %d\n", ret);
+		goto out_free_table;
+	}
+
+	of_node_put(np);
+	return 0;
+
+out_free_table:
+	opp_free_cpufreq_table(cpu_dev, &freq_table);
+out_put_node:
+	of_node_put(np);
+	return ret;
+}
+late_initcall(cpu0_cpufreq_driver_init);
+
+MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
+MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
+MODULE_LICENSE("GPL");
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
  2012-08-10  5:37   ` Shawn Guo
@ 2012-08-16 21:05     ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-08-16 21:05 UTC (permalink / raw)
  To: Shawn Guo
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel, Arnd Bergmann

On Friday, August 10, 2012, Shawn Guo wrote:
> With a lot of devices booting from device tree nowadays, it requires
> that OPP table can be initialized from device tree.  The patch adds
> a helper function of_init_opp_table together with a binding doc for
> that purpose.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I need an ACK from whoever maintains the DT bindings for that.

Thanks,
Rafael


> ---
>  Documentation/devicetree/bindings/power/opp.txt |   25 ++++++++++++
>  drivers/base/power/opp.c                        |   47 +++++++++++++++++++++++
>  include/linux/opp.h                             |    8 ++++
>  3 files changed, 80 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/opp.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> new file mode 100644
> index 0000000..74499e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -0,0 +1,25 @@
> +* Generic OPP Interface
> +
> +SoCs have a standard set of tuples consisting of frequency and
> +voltage pairs that the device will support per voltage domain. These
> +are called Operating Performance Points or OPPs.
> +
> +Properties:
> +- operating-points: An array of 2-tuples items, and each item consists
> +  of frequency and voltage like <freq-kHz vol-uV>.
> +	freq: clock frequency in kHz
> +	vol: voltage in microvolt
> +
> +Examples:
> +
> +cpu@0 {
> +	compatible = "arm,cortex-a9";
> +	reg = <0>;
> +	next-level-cache = <&L2>;
> +	operating-points = <
> +		/* kHz    uV */
> +		792000  1100000
> +		396000  950000
> +		198000  850000
> +	>;
> +};
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index ac993ea..d946864 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -22,6 +22,7 @@
>  #include <linux/rculist.h>
>  #include <linux/rcupdate.h>
>  #include <linux/opp.h>
> +#include <linux/of.h>
>  
>  /*
>   * Internal data structure organization with the OPP layer library is as
> @@ -674,3 +675,49 @@ struct srcu_notifier_head *opp_get_notifier(struct device *dev)
>  
>  	return &dev_opp->head;
>  }
> +
> +#ifdef CONFIG_OF
> +/**
> + * of_init_opp_table() - Initialize opp table from device tree
> + * @dev:	device pointer used to lookup device OPPs.
> + *
> + * Register the initial OPP table with the OPP library for given device.
> + */
> +int of_init_opp_table(struct device *dev)
> +{
> +	const struct property *prop;
> +	const __be32 *val;
> +	int nr;
> +
> +	prop = of_find_property(dev->of_node, "operating-points", NULL);
> +	if (!prop)
> +		return -ENODEV;
> +	if (!prop->value)
> +		return -ENODATA;
> +
> +	/*
> +	 * Each OPP is a set of tuples consisting of frequency and
> +	 * voltage like <freq-kHz vol-uV>.
> +	 */
> +	nr = prop->length / sizeof(u32);
> +	if (nr % 2) {
> +		dev_err(dev, "%s: Invalid OPP list\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	val = prop->value;
> +	while (nr) {
> +		unsigned long freq = be32_to_cpup(val++) * 1000;
> +		unsigned long volt = be32_to_cpup(val++);
> +
> +		if (opp_add(dev, freq, volt)) {
> +			dev_warn(dev, "%s: Failed to add OPP %ld\n",
> +				 __func__, freq);
> +			continue;
> +		}
> +		nr -= 2;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/include/linux/opp.h b/include/linux/opp.h
> index 2a4e5fa..214e0eb 100644
> --- a/include/linux/opp.h
> +++ b/include/linux/opp.h
> @@ -48,6 +48,14 @@ int opp_disable(struct device *dev, unsigned long freq);
>  
>  struct srcu_notifier_head *opp_get_notifier(struct device *dev);
>  
> +#ifdef CONFIG_OF
> +int of_init_opp_table(struct device *dev);
> +#else
> +static inline int of_init_opp_table(struct device *dev)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_OF */
>  #else
>  static inline unsigned long opp_get_voltage(struct opp *opp)
>  {
> 


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
@ 2012-08-16 21:05     ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-08-16 21:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, August 10, 2012, Shawn Guo wrote:
> With a lot of devices booting from device tree nowadays, it requires
> that OPP table can be initialized from device tree.  The patch adds
> a helper function of_init_opp_table together with a binding doc for
> that purpose.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I need an ACK from whoever maintains the DT bindings for that.

Thanks,
Rafael


> ---
>  Documentation/devicetree/bindings/power/opp.txt |   25 ++++++++++++
>  drivers/base/power/opp.c                        |   47 +++++++++++++++++++++++
>  include/linux/opp.h                             |    8 ++++
>  3 files changed, 80 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/opp.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> new file mode 100644
> index 0000000..74499e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -0,0 +1,25 @@
> +* Generic OPP Interface
> +
> +SoCs have a standard set of tuples consisting of frequency and
> +voltage pairs that the device will support per voltage domain. These
> +are called Operating Performance Points or OPPs.
> +
> +Properties:
> +- operating-points: An array of 2-tuples items, and each item consists
> +  of frequency and voltage like <freq-kHz vol-uV>.
> +	freq: clock frequency in kHz
> +	vol: voltage in microvolt
> +
> +Examples:
> +
> +cpu at 0 {
> +	compatible = "arm,cortex-a9";
> +	reg = <0>;
> +	next-level-cache = <&L2>;
> +	operating-points = <
> +		/* kHz    uV */
> +		792000  1100000
> +		396000  950000
> +		198000  850000
> +	>;
> +};
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index ac993ea..d946864 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -22,6 +22,7 @@
>  #include <linux/rculist.h>
>  #include <linux/rcupdate.h>
>  #include <linux/opp.h>
> +#include <linux/of.h>
>  
>  /*
>   * Internal data structure organization with the OPP layer library is as
> @@ -674,3 +675,49 @@ struct srcu_notifier_head *opp_get_notifier(struct device *dev)
>  
>  	return &dev_opp->head;
>  }
> +
> +#ifdef CONFIG_OF
> +/**
> + * of_init_opp_table() - Initialize opp table from device tree
> + * @dev:	device pointer used to lookup device OPPs.
> + *
> + * Register the initial OPP table with the OPP library for given device.
> + */
> +int of_init_opp_table(struct device *dev)
> +{
> +	const struct property *prop;
> +	const __be32 *val;
> +	int nr;
> +
> +	prop = of_find_property(dev->of_node, "operating-points", NULL);
> +	if (!prop)
> +		return -ENODEV;
> +	if (!prop->value)
> +		return -ENODATA;
> +
> +	/*
> +	 * Each OPP is a set of tuples consisting of frequency and
> +	 * voltage like <freq-kHz vol-uV>.
> +	 */
> +	nr = prop->length / sizeof(u32);
> +	if (nr % 2) {
> +		dev_err(dev, "%s: Invalid OPP list\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	val = prop->value;
> +	while (nr) {
> +		unsigned long freq = be32_to_cpup(val++) * 1000;
> +		unsigned long volt = be32_to_cpup(val++);
> +
> +		if (opp_add(dev, freq, volt)) {
> +			dev_warn(dev, "%s: Failed to add OPP %ld\n",
> +				 __func__, freq);
> +			continue;
> +		}
> +		nr -= 2;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/include/linux/opp.h b/include/linux/opp.h
> index 2a4e5fa..214e0eb 100644
> --- a/include/linux/opp.h
> +++ b/include/linux/opp.h
> @@ -48,6 +48,14 @@ int opp_disable(struct device *dev, unsigned long freq);
>  
>  struct srcu_notifier_head *opp_get_notifier(struct device *dev);
>  
> +#ifdef CONFIG_OF
> +int of_init_opp_table(struct device *dev);
> +#else
> +static inline int of_init_opp_table(struct device *dev)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_OF */
>  #else
>  static inline unsigned long opp_get_voltage(struct opp *opp)
>  {
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
  2012-08-16 21:05     ` Rafael J. Wysocki
@ 2012-08-17  0:52         ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-17  0:52 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Aug 16, 2012 at 11:05:54PM +0200, Rafael J. Wysocki wrote:
> On Friday, August 10, 2012, Shawn Guo wrote:
> > With a lot of devices booting from device tree nowadays, it requires
> > that OPP table can be initialized from device tree.  The patch adds
> > a helper function of_init_opp_table together with a binding doc for
> > that purpose.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> I need an ACK from whoever maintains the DT bindings for that.
> 
Rob, please?

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
@ 2012-08-17  0:52         ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-17  0:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 16, 2012 at 11:05:54PM +0200, Rafael J. Wysocki wrote:
> On Friday, August 10, 2012, Shawn Guo wrote:
> > With a lot of devices booting from device tree nowadays, it requires
> > that OPP table can be initialized from device tree.  The patch adds
> > a helper function of_init_opp_table together with a binding doc for
> > that purpose.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> I need an ACK from whoever maintains the DT bindings for that.
> 
Rob, please?

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
  2012-08-17  0:52         ` Shawn Guo
@ 2012-08-17  1:53           ` Rob Herring
  -1 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2012-08-17  1:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rafael J. Wysocki, cpufreq, linux-pm, Kevin Hilman,
	Nishanth Menon, Shilimkar Santosh, Richard Zhao,
	Russell King - ARM Linux, Mike Turquette, Mark Brown,
	devicetree-discuss, linux-arm-kernel, Arnd Bergmann

On 08/16/2012 07:52 PM, Shawn Guo wrote:
> On Thu, Aug 16, 2012 at 11:05:54PM +0200, Rafael J. Wysocki wrote:
>> On Friday, August 10, 2012, Shawn Guo wrote:
>>> With a lot of devices booting from device tree nowadays, it requires
>>> that OPP table can be initialized from device tree.  The patch adds
>>> a helper function of_init_opp_table together with a binding doc for
>>> that purpose.
>>>
>>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>
>> I need an ACK from whoever maintains the DT bindings for that.
>>
> Rob, please?

Acked-by: Rob Herring <rob.herring@calxeda.com>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
@ 2012-08-17  1:53           ` Rob Herring
  0 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2012-08-17  1:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/16/2012 07:52 PM, Shawn Guo wrote:
> On Thu, Aug 16, 2012 at 11:05:54PM +0200, Rafael J. Wysocki wrote:
>> On Friday, August 10, 2012, Shawn Guo wrote:
>>> With a lot of devices booting from device tree nowadays, it requires
>>> that OPP table can be initialized from device tree.  The patch adds
>>> a helper function of_init_opp_table together with a binding doc for
>>> that purpose.
>>>
>>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>
>> I need an ACK from whoever maintains the DT bindings for that.
>>
> Rob, please?

Acked-by: Rob Herring <rob.herring@calxeda.com>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver
  2012-08-10  5:37 ` Shawn Guo
@ 2012-08-31  6:42   ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-31  6:42 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel

Ping, Rafael.

Regards,
Shawn

On Fri, Aug 10, 2012 at 01:37:22PM +0800, Shawn Guo wrote:
> Patch "cpufreq: Add a generic cpufreq-cpu0 driver" has a dependency
> on following branch.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/tol
> 
> Changes since v2:
> * Drop patch "regulator: add a new API regulator_set_voltage_tol()",
>   which has been applied on regulator tree as above.
> * Rewrite function of_init_opp_table to make it concise.
>   
> Regards,
> Shawn
> 
> Richard Zhao (1):
>   ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
> 
> Shawn Guo (2):
>   PM / OPP: Initialize OPP table from device tree
>   cpufreq: Add a generic cpufreq-cpu0 driver
> 
>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
>  Documentation/devicetree/bindings/power/opp.txt    |   25 ++
>  arch/arm/kernel/smp.c                              |   54 ++++
>  drivers/base/power/opp.c                           |   47 ++++
>  drivers/cpufreq/Kconfig                            |   11 +
>  drivers/cpufreq/Makefile                           |    2 +
>  drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
>  include/linux/opp.h                                |    8 +
>  8 files changed, 473 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>  create mode 100644 Documentation/devicetree/bindings/power/opp.txt
>  create mode 100644 drivers/cpufreq/cpufreq-cpu0.c
> 
> -- 
> 1.7.5.4
> 
> 


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver
@ 2012-08-31  6:42   ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-08-31  6:42 UTC (permalink / raw)
  To: linux-arm-kernel

Ping, Rafael.

Regards,
Shawn

On Fri, Aug 10, 2012 at 01:37:22PM +0800, Shawn Guo wrote:
> Patch "cpufreq: Add a generic cpufreq-cpu0 driver" has a dependency
> on following branch.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/tol
> 
> Changes since v2:
> * Drop patch "regulator: add a new API regulator_set_voltage_tol()",
>   which has been applied on regulator tree as above.
> * Rewrite function of_init_opp_table to make it concise.
>   
> Regards,
> Shawn
> 
> Richard Zhao (1):
>   ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
> 
> Shawn Guo (2):
>   PM / OPP: Initialize OPP table from device tree
>   cpufreq: Add a generic cpufreq-cpu0 driver
> 
>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
>  Documentation/devicetree/bindings/power/opp.txt    |   25 ++
>  arch/arm/kernel/smp.c                              |   54 ++++
>  drivers/base/power/opp.c                           |   47 ++++
>  drivers/cpufreq/Kconfig                            |   11 +
>  drivers/cpufreq/Makefile                           |    2 +
>  drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
>  include/linux/opp.h                                |    8 +
>  8 files changed, 473 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>  create mode 100644 Documentation/devicetree/bindings/power/opp.txt
>  create mode 100644 drivers/cpufreq/cpufreq-cpu0.c
> 
> -- 
> 1.7.5.4
> 
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver
  2012-08-31  6:42   ` Shawn Guo
@ 2012-09-01  5:53     ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-01  5:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel

On Friday, August 31, 2012, Shawn Guo wrote:
> Ping, Rafael.

Yeah, sorry for the delay.

I'll get to it when I get back from LinuxCon/Plumbers Conf. (early next week).

Thanks,
Rafael


> On Fri, Aug 10, 2012 at 01:37:22PM +0800, Shawn Guo wrote:
> > Patch "cpufreq: Add a generic cpufreq-cpu0 driver" has a dependency
> > on following branch.
> > 
> >  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/tol
> > 
> > Changes since v2:
> > * Drop patch "regulator: add a new API regulator_set_voltage_tol()",
> >   which has been applied on regulator tree as above.
> > * Rewrite function of_init_opp_table to make it concise.
> >   
> > Regards,
> > Shawn
> > 
> > Richard Zhao (1):
> >   ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
> > 
> > Shawn Guo (2):
> >   PM / OPP: Initialize OPP table from device tree
> >   cpufreq: Add a generic cpufreq-cpu0 driver
> > 
> >  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
> >  Documentation/devicetree/bindings/power/opp.txt    |   25 ++
> >  arch/arm/kernel/smp.c                              |   54 ++++
> >  drivers/base/power/opp.c                           |   47 ++++
> >  drivers/cpufreq/Kconfig                            |   11 +
> >  drivers/cpufreq/Makefile                           |    2 +
> >  drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
> >  include/linux/opp.h                                |    8 +
> >  8 files changed, 473 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> >  create mode 100644 Documentation/devicetree/bindings/power/opp.txt
> >  create mode 100644 drivers/cpufreq/cpufreq-cpu0.c
> > 
> 
> 
> 


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver
@ 2012-09-01  5:53     ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-01  5:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, August 31, 2012, Shawn Guo wrote:
> Ping, Rafael.

Yeah, sorry for the delay.

I'll get to it when I get back from LinuxCon/Plumbers Conf. (early next week).

Thanks,
Rafael


> On Fri, Aug 10, 2012 at 01:37:22PM +0800, Shawn Guo wrote:
> > Patch "cpufreq: Add a generic cpufreq-cpu0 driver" has a dependency
> > on following branch.
> > 
> >  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/tol
> > 
> > Changes since v2:
> > * Drop patch "regulator: add a new API regulator_set_voltage_tol()",
> >   which has been applied on regulator tree as above.
> > * Rewrite function of_init_opp_table to make it concise.
> >   
> > Regards,
> > Shawn
> > 
> > Richard Zhao (1):
> >   ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
> > 
> > Shawn Guo (2):
> >   PM / OPP: Initialize OPP table from device tree
> >   cpufreq: Add a generic cpufreq-cpu0 driver
> > 
> >  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
> >  Documentation/devicetree/bindings/power/opp.txt    |   25 ++
> >  arch/arm/kernel/smp.c                              |   54 ++++
> >  drivers/base/power/opp.c                           |   47 ++++
> >  drivers/cpufreq/Kconfig                            |   11 +
> >  drivers/cpufreq/Makefile                           |    2 +
> >  drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
> >  include/linux/opp.h                                |    8 +
> >  8 files changed, 473 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> >  create mode 100644 Documentation/devicetree/bindings/power/opp.txt
> >  create mode 100644 drivers/cpufreq/cpufreq-cpu0.c
> > 
> 
> 
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-08-10  5:37   ` Shawn Guo
@ 2012-09-04 23:18     ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-04 23:18 UTC (permalink / raw)
  To: Shawn Guo
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel

On Friday, August 10, 2012, Shawn Guo wrote:
> It adds a generic cpufreq driver for CPU0 frequency management based on
> clk, regulator, OPP and device tree support.  It can support both
> uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> share clock and voltage across all CPUs.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I seem to recall that this has been discussed with several people and
undergone some changes as a result of comments.  Would it be possible
to get some ACKs from the people involved?

> ---
>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
>  drivers/cpufreq/Kconfig                            |   11 +
>  drivers/cpufreq/Makefile                           |    2 +
>  drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
>  4 files changed, 339 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>  create mode 100644 drivers/cpufreq/cpufreq-cpu0.c
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> new file mode 100644
> index 0000000..4416ccc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> @@ -0,0 +1,55 @@
> +Generic CPU0 cpufreq driver
> +
> +It is a generic cpufreq driver for CPU0 frequency management.  It
> +supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
> +systems which share clock and voltage across all CPUs.
> +
> +Both required and optional properties listed below must be defined
> +under node /cpus/cpu@0.
> +
> +Required properties:
> +- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
> +  for details
> +
> +Optional properties:
> +- clock-latency: Specify the possible maximum transition latency for clock,
> +  in unit of nanoseconds.
> +- voltage-tolerance: Specify the CPU voltage tolerance in percentage.
> +
> +Examples:
> +
> +cpus {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	cpu@0 {
> +		compatible = "arm,cortex-a9";
> +		reg = <0>;
> +		next-level-cache = <&L2>;
> +		operating-points = <
> +			/* kHz    uV */
> +			792000  1100000
> +			396000  950000
> +			198000  850000
> +		>;
> +		transition-latency = <61036>; /* two CLK32 periods */
> +	};
> +
> +	cpu@1 {
> +		compatible = "arm,cortex-a9";
> +		reg = <1>;
> +		next-level-cache = <&L2>;
> +	};
> +
> +	cpu@2 {
> +		compatible = "arm,cortex-a9";
> +		reg = <2>;
> +		next-level-cache = <&L2>;
> +	};
> +
> +	cpu@3 {
> +		compatible = "arm,cortex-a9";
> +		reg = <3>;
> +		next-level-cache = <&L2>;
> +	};
> +};
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index e24a2a1..ea512f4 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -179,6 +179,17 @@ config CPU_FREQ_GOV_CONSERVATIVE
>  
>  	  If in doubt, say N.
>  
> +config GENERIC_CPUFREQ_CPU0
> +	bool "Generic CPU0 cpufreq driver"
> +	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
> +	select CPU_FREQ_TABLE
> +	help
> +	  This adds a generic cpufreq driver for CPU0 frequency management.
> +	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
> +	  systems which share clock and voltage across all CPUs.
> +
> +	  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 9531fc2..a378ed2 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_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
> +
>  ##################################################################################
>  # x86 drivers.
>  # Link order matters. K8 is preferred to ACPI because of firmware bugs in early
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> new file mode 100644
> index 0000000..fd3bac6
> --- /dev/null
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -0,0 +1,271 @@
> +/*
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + *
> + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c

What do you mean by "reuse" here?

> + *
> + * 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:

The kernel is just GPLv2 (not later) and it will stay this way.  Please
update this comment accordingly.

> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
> +
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/opp.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +static unsigned int transition_latency;
> +static unsigned int voltage_tolerance; /* in percentage */
> +
> +static struct device *cpu_dev;
> +static struct clk *cpu_clk;
> +static struct regulator *cpu_reg;
> +static struct cpufreq_frequency_table *freq_table;
> +
> +static int cpu0_verify_speed(struct cpufreq_policy *policy)
> +{
> +	return cpufreq_frequency_table_verify(policy, freq_table);
> +}
> +
> +static unsigned int cpu0_get_speed(unsigned int cpu)
> +{
> +	return clk_get_rate(cpu_clk) / 1000;
> +}
> +
> +static int cpu0_set_target(struct cpufreq_policy *policy,
> +			   unsigned int target_freq, unsigned int relation)
> +{
> +	struct cpufreq_freqs freqs;
> +	struct opp *opp;
> +	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
> +	unsigned int index, cpu;
> +	int ret;
> +
> +	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> +					     relation, &index);
> +	if (ret) {
> +		pr_err("failed to match target freqency %d: %d\n",
> +		       target_freq, ret);
> +		return ret;
> +	}
> +
> +	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
> +	if (freq_Hz < 0)
> +		freq_Hz = freq_table[index].frequency * 1000;
> +	freqs.new = freq_Hz / 1000;
> +	freqs.old = clk_get_rate(cpu_clk) / 1000;
> +
> +	if (freqs.old == freqs.new)
> +		return 0;
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +	}
> +
> +	if (cpu_reg) {
> +		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
> +		if (IS_ERR(opp)) {
> +			pr_err("failed to find OPP for %ld\n", freq_Hz);
> +			return PTR_ERR(opp);
> +		}
> +		volt = opp_get_voltage(opp);
> +		tol = volt * voltage_tolerance / 100;
> +		volt_old = regulator_get_voltage(cpu_reg);
> +	}
> +
> +	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> +		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> +		 freqs.new / 1000, volt ? volt / 1000 : -1);
> +
> +	/* scaling up?  scale voltage before frequency */
> +	if (cpu_reg && freqs.new > freqs.old) {
> +		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage up: %d\n", ret);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
> +	if (ret) {
> +		pr_err("failed to set clock rate: %d\n", ret);
> +		if (cpu_reg)
> +			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
> +		return ret;
> +	}
> +
> +	/* scaling down?  scale voltage after frequency */
> +	if (cpu_reg && freqs.new < freqs.old) {
> +		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage down: %d\n", ret);
> +			clk_set_rate(cpu_clk, freqs.old * 1000);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> +	}
> +
> +	return 0;
> +}
> +
> +static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
> +{
> +	int ret;
> +
> +	if (policy->cpu != 0)
> +		return -EINVAL;
> +
> +	ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> +	if (ret) {
> +		pr_err("invalid frequency table: %d\n", ret);
> +		return ret;
> +	}
> +
> +	policy->cpuinfo.transition_latency = transition_latency;
> +	policy->cur = clk_get_rate(cpu_clk) / 1000;
> +
> +	/*
> +	 * The driver only supports the SMP configuartion where all processors
> +	 * share the clock and voltage and clock.  Use cpufreq affected_cpus
> +	 * interface to have all CPUs scaled together.
> +	 */
> +	policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
> +	cpumask_setall(policy->cpus);
> +
> +	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
> +
> +	return 0;
> +}
> +
> +static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
> +{
> +	cpufreq_frequency_table_put_attr(policy->cpu);
> +
> +	return 0;
> +}
> +
> +static struct freq_attr *cpu0_cpufreq_attr[] = {
> +	&cpufreq_freq_attr_scaling_available_freqs,
> +	NULL,
> +};
> +
> +static struct cpufreq_driver cpu0_cpufreq_driver = {
> +	.flags = CPUFREQ_STICKY,
> +	.verify = cpu0_verify_speed,
> +	.target = cpu0_set_target,
> +	.get = cpu0_get_speed,
> +	.init = cpu0_cpufreq_init,
> +	.exit = cpu0_cpufreq_exit,
> +	.name = "generic_cpu0",
> +	.attr = cpu0_cpufreq_attr,
> +};
> +
> +static int __devinit cpu0_cpufreq_driver_init(void)
> +{
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_node_by_path("/cpus/cpu@0");
> +	if (!np) {
> +		pr_err("failed to find cpu0 node\n");
> +		return -ENOENT;
> +	}
> +
> +	cpu_dev = get_cpu_device(0);
> +	if (!cpu_dev) {
> +		pr_err("failed to get cpu0 device\n");
> +		ret = -ENODEV;
> +		goto out_put_node;
> +	}
> +
> +	cpu_dev->of_node = np;
> +
> +	cpu_clk = clk_get(cpu_dev, NULL);
> +	if (IS_ERR(cpu_clk)) {
> +		ret = PTR_ERR(cpu_clk);
> +		pr_err("failed to get cpu0 clock: %d\n", ret);
> +		goto out_put_node;
> +	}
> +
> +	cpu_reg = regulator_get(cpu_dev, "cpu0");
> +	if (IS_ERR(cpu_reg)) {
> +		pr_warn("failed to get cpu0 regulator\n");
> +		cpu_reg = NULL;
> +	}
> +
> +	ret = of_init_opp_table(cpu_dev);
> +	if (ret) {
> +		pr_err("failed to init OPP table: %d\n", ret);
> +		goto out_put_node;
> +	}
> +
> +	ret = opp_init_cpufreq_table(cpu_dev, &freq_table);
> +	if (ret) {
> +		pr_err("failed to init cpufreq table: %d\n", ret);
> +		goto out_put_node;
> +	}
> +
> +	of_property_read_u32(np, "voltage-tolerance", &voltage_tolerance);
> +
> +	if (of_property_read_u32(np, "clock-latency", &transition_latency))
> +		transition_latency = CPUFREQ_ETERNAL;
> +
> +	if (cpu_reg) {
> +		struct opp *opp;
> +		unsigned long min_uV, max_uV;
> +		int i;
> +
> +		/*
> +		 * OPP is maintained in order of increasing frequency, and
> +		 * freq_table initialised from OPP is therefore sorted in the
> +		 * same order.
> +		 */
> +		for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
> +			;
> +		opp = opp_find_freq_exact(cpu_dev,
> +				freq_table[0].frequency * 1000, true);
> +		min_uV = opp_get_voltage(opp);
> +		opp = opp_find_freq_exact(cpu_dev,
> +				freq_table[i-1].frequency * 1000, true);
> +		max_uV = opp_get_voltage(opp);
> +		ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
> +		if (ret > 0)
> +			transition_latency += ret * 1000;
> +	}
> +
> +	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
> +	if (ret) {
> +		pr_err("failed register driver: %d\n", ret);
> +		goto out_free_table;
> +	}
> +
> +	of_node_put(np);
> +	return 0;
> +
> +out_free_table:
> +	opp_free_cpufreq_table(cpu_dev, &freq_table);
> +out_put_node:
> +	of_node_put(np);
> +	return ret;
> +}
> +late_initcall(cpu0_cpufreq_driver_init);
> +
> +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> +MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
> +MODULE_LICENSE("GPL");

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-04 23:18     ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-04 23:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, August 10, 2012, Shawn Guo wrote:
> It adds a generic cpufreq driver for CPU0 frequency management based on
> clk, regulator, OPP and device tree support.  It can support both
> uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> share clock and voltage across all CPUs.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I seem to recall that this has been discussed with several people and
undergone some changes as a result of comments.  Would it be possible
to get some ACKs from the people involved?

> ---
>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |   55 ++++
>  drivers/cpufreq/Kconfig                            |   11 +
>  drivers/cpufreq/Makefile                           |    2 +
>  drivers/cpufreq/cpufreq-cpu0.c                     |  271 ++++++++++++++++++++
>  4 files changed, 339 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>  create mode 100644 drivers/cpufreq/cpufreq-cpu0.c
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> new file mode 100644
> index 0000000..4416ccc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> @@ -0,0 +1,55 @@
> +Generic CPU0 cpufreq driver
> +
> +It is a generic cpufreq driver for CPU0 frequency management.  It
> +supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
> +systems which share clock and voltage across all CPUs.
> +
> +Both required and optional properties listed below must be defined
> +under node /cpus/cpu at 0.
> +
> +Required properties:
> +- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
> +  for details
> +
> +Optional properties:
> +- clock-latency: Specify the possible maximum transition latency for clock,
> +  in unit of nanoseconds.
> +- voltage-tolerance: Specify the CPU voltage tolerance in percentage.
> +
> +Examples:
> +
> +cpus {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	cpu at 0 {
> +		compatible = "arm,cortex-a9";
> +		reg = <0>;
> +		next-level-cache = <&L2>;
> +		operating-points = <
> +			/* kHz    uV */
> +			792000  1100000
> +			396000  950000
> +			198000  850000
> +		>;
> +		transition-latency = <61036>; /* two CLK32 periods */
> +	};
> +
> +	cpu at 1 {
> +		compatible = "arm,cortex-a9";
> +		reg = <1>;
> +		next-level-cache = <&L2>;
> +	};
> +
> +	cpu at 2 {
> +		compatible = "arm,cortex-a9";
> +		reg = <2>;
> +		next-level-cache = <&L2>;
> +	};
> +
> +	cpu at 3 {
> +		compatible = "arm,cortex-a9";
> +		reg = <3>;
> +		next-level-cache = <&L2>;
> +	};
> +};
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index e24a2a1..ea512f4 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -179,6 +179,17 @@ config CPU_FREQ_GOV_CONSERVATIVE
>  
>  	  If in doubt, say N.
>  
> +config GENERIC_CPUFREQ_CPU0
> +	bool "Generic CPU0 cpufreq driver"
> +	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
> +	select CPU_FREQ_TABLE
> +	help
> +	  This adds a generic cpufreq driver for CPU0 frequency management.
> +	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
> +	  systems which share clock and voltage across all CPUs.
> +
> +	  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 9531fc2..a378ed2 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_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
> +
>  ##################################################################################
>  # x86 drivers.
>  # Link order matters. K8 is preferred to ACPI because of firmware bugs in early
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> new file mode 100644
> index 0000000..fd3bac6
> --- /dev/null
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -0,0 +1,271 @@
> +/*
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + *
> + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c

What do you mean by "reuse" here?

> + *
> + * 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:

The kernel is just GPLv2 (not later) and it will stay this way.  Please
update this comment accordingly.

> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
> +
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/opp.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +static unsigned int transition_latency;
> +static unsigned int voltage_tolerance; /* in percentage */
> +
> +static struct device *cpu_dev;
> +static struct clk *cpu_clk;
> +static struct regulator *cpu_reg;
> +static struct cpufreq_frequency_table *freq_table;
> +
> +static int cpu0_verify_speed(struct cpufreq_policy *policy)
> +{
> +	return cpufreq_frequency_table_verify(policy, freq_table);
> +}
> +
> +static unsigned int cpu0_get_speed(unsigned int cpu)
> +{
> +	return clk_get_rate(cpu_clk) / 1000;
> +}
> +
> +static int cpu0_set_target(struct cpufreq_policy *policy,
> +			   unsigned int target_freq, unsigned int relation)
> +{
> +	struct cpufreq_freqs freqs;
> +	struct opp *opp;
> +	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
> +	unsigned int index, cpu;
> +	int ret;
> +
> +	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> +					     relation, &index);
> +	if (ret) {
> +		pr_err("failed to match target freqency %d: %d\n",
> +		       target_freq, ret);
> +		return ret;
> +	}
> +
> +	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
> +	if (freq_Hz < 0)
> +		freq_Hz = freq_table[index].frequency * 1000;
> +	freqs.new = freq_Hz / 1000;
> +	freqs.old = clk_get_rate(cpu_clk) / 1000;
> +
> +	if (freqs.old == freqs.new)
> +		return 0;
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +	}
> +
> +	if (cpu_reg) {
> +		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
> +		if (IS_ERR(opp)) {
> +			pr_err("failed to find OPP for %ld\n", freq_Hz);
> +			return PTR_ERR(opp);
> +		}
> +		volt = opp_get_voltage(opp);
> +		tol = volt * voltage_tolerance / 100;
> +		volt_old = regulator_get_voltage(cpu_reg);
> +	}
> +
> +	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> +		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> +		 freqs.new / 1000, volt ? volt / 1000 : -1);
> +
> +	/* scaling up?  scale voltage before frequency */
> +	if (cpu_reg && freqs.new > freqs.old) {
> +		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage up: %d\n", ret);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
> +	if (ret) {
> +		pr_err("failed to set clock rate: %d\n", ret);
> +		if (cpu_reg)
> +			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
> +		return ret;
> +	}
> +
> +	/* scaling down?  scale voltage after frequency */
> +	if (cpu_reg && freqs.new < freqs.old) {
> +		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage down: %d\n", ret);
> +			clk_set_rate(cpu_clk, freqs.old * 1000);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> +	}
> +
> +	return 0;
> +}
> +
> +static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
> +{
> +	int ret;
> +
> +	if (policy->cpu != 0)
> +		return -EINVAL;
> +
> +	ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> +	if (ret) {
> +		pr_err("invalid frequency table: %d\n", ret);
> +		return ret;
> +	}
> +
> +	policy->cpuinfo.transition_latency = transition_latency;
> +	policy->cur = clk_get_rate(cpu_clk) / 1000;
> +
> +	/*
> +	 * The driver only supports the SMP configuartion where all processors
> +	 * share the clock and voltage and clock.  Use cpufreq affected_cpus
> +	 * interface to have all CPUs scaled together.
> +	 */
> +	policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
> +	cpumask_setall(policy->cpus);
> +
> +	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
> +
> +	return 0;
> +}
> +
> +static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
> +{
> +	cpufreq_frequency_table_put_attr(policy->cpu);
> +
> +	return 0;
> +}
> +
> +static struct freq_attr *cpu0_cpufreq_attr[] = {
> +	&cpufreq_freq_attr_scaling_available_freqs,
> +	NULL,
> +};
> +
> +static struct cpufreq_driver cpu0_cpufreq_driver = {
> +	.flags = CPUFREQ_STICKY,
> +	.verify = cpu0_verify_speed,
> +	.target = cpu0_set_target,
> +	.get = cpu0_get_speed,
> +	.init = cpu0_cpufreq_init,
> +	.exit = cpu0_cpufreq_exit,
> +	.name = "generic_cpu0",
> +	.attr = cpu0_cpufreq_attr,
> +};
> +
> +static int __devinit cpu0_cpufreq_driver_init(void)
> +{
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_node_by_path("/cpus/cpu at 0");
> +	if (!np) {
> +		pr_err("failed to find cpu0 node\n");
> +		return -ENOENT;
> +	}
> +
> +	cpu_dev = get_cpu_device(0);
> +	if (!cpu_dev) {
> +		pr_err("failed to get cpu0 device\n");
> +		ret = -ENODEV;
> +		goto out_put_node;
> +	}
> +
> +	cpu_dev->of_node = np;
> +
> +	cpu_clk = clk_get(cpu_dev, NULL);
> +	if (IS_ERR(cpu_clk)) {
> +		ret = PTR_ERR(cpu_clk);
> +		pr_err("failed to get cpu0 clock: %d\n", ret);
> +		goto out_put_node;
> +	}
> +
> +	cpu_reg = regulator_get(cpu_dev, "cpu0");
> +	if (IS_ERR(cpu_reg)) {
> +		pr_warn("failed to get cpu0 regulator\n");
> +		cpu_reg = NULL;
> +	}
> +
> +	ret = of_init_opp_table(cpu_dev);
> +	if (ret) {
> +		pr_err("failed to init OPP table: %d\n", ret);
> +		goto out_put_node;
> +	}
> +
> +	ret = opp_init_cpufreq_table(cpu_dev, &freq_table);
> +	if (ret) {
> +		pr_err("failed to init cpufreq table: %d\n", ret);
> +		goto out_put_node;
> +	}
> +
> +	of_property_read_u32(np, "voltage-tolerance", &voltage_tolerance);
> +
> +	if (of_property_read_u32(np, "clock-latency", &transition_latency))
> +		transition_latency = CPUFREQ_ETERNAL;
> +
> +	if (cpu_reg) {
> +		struct opp *opp;
> +		unsigned long min_uV, max_uV;
> +		int i;
> +
> +		/*
> +		 * OPP is maintained in order of increasing frequency, and
> +		 * freq_table initialised from OPP is therefore sorted in the
> +		 * same order.
> +		 */
> +		for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
> +			;
> +		opp = opp_find_freq_exact(cpu_dev,
> +				freq_table[0].frequency * 1000, true);
> +		min_uV = opp_get_voltage(opp);
> +		opp = opp_find_freq_exact(cpu_dev,
> +				freq_table[i-1].frequency * 1000, true);
> +		max_uV = opp_get_voltage(opp);
> +		ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
> +		if (ret > 0)
> +			transition_latency += ret * 1000;
> +	}
> +
> +	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
> +	if (ret) {
> +		pr_err("failed register driver: %d\n", ret);
> +		goto out_free_table;
> +	}
> +
> +	of_node_put(np);
> +	return 0;
> +
> +out_free_table:
> +	opp_free_cpufreq_table(cpu_dev, &freq_table);
> +out_put_node:
> +	of_node_put(np);
> +	return ret;
> +}
> +late_initcall(cpu0_cpufreq_driver_init);
> +
> +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> +MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
> +MODULE_LICENSE("GPL");

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-08-10  5:37   ` Shawn Guo
@ 2012-09-04 23:27     ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-04 23:27 UTC (permalink / raw)
  To: Shawn Guo
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, devicetree-discuss,
	linux-arm-kernel, Richard Zhao

On Friday, August 10, 2012, Shawn Guo wrote:
> From: Richard Zhao <richard.zhao@linaro.org>
> 
> 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>
> Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Applied to the linux-next branch of the linux-pm.git tree as v3.7 material.

Thanks,
Rafael


> ---
>  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 ebd8ad2..8e03567 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>
> @@ -584,3 +585,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
> 


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-04 23:27     ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-04 23:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, August 10, 2012, Shawn Guo wrote:
> From: Richard Zhao <richard.zhao@linaro.org>
> 
> 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>
> Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Applied to the linux-next branch of the linux-pm.git tree as v3.7 material.

Thanks,
Rafael


> ---
>  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 ebd8ad2..8e03567 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>
> @@ -584,3 +585,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
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
  2012-08-17  1:53           ` Rob Herring
@ 2012-09-04 23:27             ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-04 23:27 UTC (permalink / raw)
  To: Rob Herring
  Cc: Shawn Guo, cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, devicetree-discuss, linux-arm-kernel,
	Arnd Bergmann

On Friday, August 17, 2012, Rob Herring wrote:
> On 08/16/2012 07:52 PM, Shawn Guo wrote:
> > On Thu, Aug 16, 2012 at 11:05:54PM +0200, Rafael J. Wysocki wrote:
> >> On Friday, August 10, 2012, Shawn Guo wrote:
> >>> With a lot of devices booting from device tree nowadays, it requires
> >>> that OPP table can be initialized from device tree.  The patch adds
> >>> a helper function of_init_opp_table together with a binding doc for
> >>> that purpose.
> >>>
> >>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> >>
> >> I need an ACK from whoever maintains the DT bindings for that.
> >>
> > Rob, please?
> 
> Acked-by: Rob Herring <rob.herring@calxeda.com>

Applied to the linux-next branch of the linux-pm.git tree as v3.7 material.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree
@ 2012-09-04 23:27             ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-04 23:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, August 17, 2012, Rob Herring wrote:
> On 08/16/2012 07:52 PM, Shawn Guo wrote:
> > On Thu, Aug 16, 2012 at 11:05:54PM +0200, Rafael J. Wysocki wrote:
> >> On Friday, August 10, 2012, Shawn Guo wrote:
> >>> With a lot of devices booting from device tree nowadays, it requires
> >>> that OPP table can be initialized from device tree.  The patch adds
> >>> a helper function of_init_opp_table together with a binding doc for
> >>> that purpose.
> >>>
> >>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> >>
> >> I need an ACK from whoever maintains the DT bindings for that.
> >>
> > Rob, please?
> 
> Acked-by: Rob Herring <rob.herring@calxeda.com>

Applied to the linux-next branch of the linux-pm.git tree as v3.7 material.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-04 23:18     ` Rafael J. Wysocki
@ 2012-09-05  1:12         ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-05  1:12 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
> On Friday, August 10, 2012, Shawn Guo wrote:
> > It adds a generic cpufreq driver for CPU0 frequency management based on
> > clk, regulator, OPP and device tree support.  It can support both
> > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> > share clock and voltage across all CPUs.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> I seem to recall that this has been discussed with several people and
> undergone some changes as a result of comments.  Would it be possible
> to get some ACKs from the people involved?
> 

Mark, Santosh,

Reviewed-by, please?

AnilKumar,

Tested-by, please?

<snip>

> > --- /dev/null
> > +++ b/drivers/cpufreq/cpufreq-cpu0.c
> > @@ -0,0 +1,271 @@
> > +/*
> > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > + *
> > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
> 
> What do you mean by "reuse" here?
> 
cpu0_set_target() function is almost a copy of omap_target() from
drivers/cpufreq/omap-cpufreq.c.  I should probably just say so?

> > + *
> > + * 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:
> 
> The kernel is just GPLv2 (not later) and it will stay this way.  Please
> update this comment accordingly.
> 
Okay.

> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-05  1:12         ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-05  1:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
> On Friday, August 10, 2012, Shawn Guo wrote:
> > It adds a generic cpufreq driver for CPU0 frequency management based on
> > clk, regulator, OPP and device tree support.  It can support both
> > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> > share clock and voltage across all CPUs.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> I seem to recall that this has been discussed with several people and
> undergone some changes as a result of comments.  Would it be possible
> to get some ACKs from the people involved?
> 

Mark, Santosh,

Reviewed-by, please?

AnilKumar,

Tested-by, please?

<snip>

> > --- /dev/null
> > +++ b/drivers/cpufreq/cpufreq-cpu0.c
> > @@ -0,0 +1,271 @@
> > +/*
> > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > + *
> > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
> 
> What do you mean by "reuse" here?
> 
cpu0_set_target() function is almost a copy of omap_target() from
drivers/cpufreq/omap-cpufreq.c.  I should probably just say so?

> > + *
> > + * 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:
> 
> The kernel is just GPLv2 (not later) and it will stay this way.  Please
> update this comment accordingly.
> 
Okay.

> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-05  1:12         ` Shawn Guo
@ 2012-09-05  4:53           ` AnilKumar, Chimata
  -1 siblings, 0 replies; 54+ messages in thread
From: AnilKumar, Chimata @ 2012-09-05  4:53 UTC (permalink / raw)
  To: Shawn Guo, Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Hilman, Kevin, Menon, Nishanth, Shilimkar,
	Santosh, Richard Zhao, Russell King - ARM Linux, Mike Turquette,
	Mark Brown, Rob Herring, devicetree-discuss, linux-arm-kernel

On Wed, Sep 05, 2012 at 06:42:21, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
> > On Friday, August 10, 2012, Shawn Guo wrote:
> > > It adds a generic cpufreq driver for CPU0 frequency management based on
> > > clk, regulator, OPP and device tree support.  It can support both
> > > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> > > share clock and voltage across all CPUs.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > I seem to recall that this has been discussed with several people and
> > undergone some changes as a result of comments.  Would it be possible
> > to get some ACKs from the people involved?
> > 
> 
> Mark, Santosh,
> 
> Reviewed-by, please?
> 
> AnilKumar,
> 
> Tested-by, please?

I have tested on AM335x-EVM and AM335x-Bone devices.

Tested-by: AnilKumar Ch <anilkumar@ti.com>

> 
> <snip>
> 
> > > --- /dev/null
> > > +++ b/drivers/cpufreq/cpufreq-cpu0.c
> > > @@ -0,0 +1,271 @@
> > > +/*
> > > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > > + *
> > > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
> > 
> > What do you mean by "reuse" here?
> > 
> cpu0_set_target() function is almost a copy of omap_target() from
> drivers/cpufreq/omap-cpufreq.c.  I should probably just say so?
> 
> > > + *
> > > + * 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:
> > 
> > The kernel is just GPLv2 (not later) and it will stay this way.  Please
> > update this comment accordingly.
> > 
> Okay.
> 
> > > + *
> > > + * http://www.opensource.org/licenses/gpl-license.html
> > > + * http://www.gnu.org/copyleft/gpl.html
> > > + */
> 
> -- 
> Regards,
> Shawn
> 


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-05  4:53           ` AnilKumar, Chimata
  0 siblings, 0 replies; 54+ messages in thread
From: AnilKumar, Chimata @ 2012-09-05  4:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 05, 2012 at 06:42:21, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
> > On Friday, August 10, 2012, Shawn Guo wrote:
> > > It adds a generic cpufreq driver for CPU0 frequency management based on
> > > clk, regulator, OPP and device tree support.  It can support both
> > > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> > > share clock and voltage across all CPUs.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > I seem to recall that this has been discussed with several people and
> > undergone some changes as a result of comments.  Would it be possible
> > to get some ACKs from the people involved?
> > 
> 
> Mark, Santosh,
> 
> Reviewed-by, please?
> 
> AnilKumar,
> 
> Tested-by, please?

I have tested on AM335x-EVM and AM335x-Bone devices.

Tested-by: AnilKumar Ch <anilkumar@ti.com>

> 
> <snip>
> 
> > > --- /dev/null
> > > +++ b/drivers/cpufreq/cpufreq-cpu0.c
> > > @@ -0,0 +1,271 @@
> > > +/*
> > > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > > + *
> > > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
> > 
> > What do you mean by "reuse" here?
> > 
> cpu0_set_target() function is almost a copy of omap_target() from
> drivers/cpufreq/omap-cpufreq.c.  I should probably just say so?
> 
> > > + *
> > > + * 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:
> > 
> > The kernel is just GPLv2 (not later) and it will stay this way.  Please
> > update this comment accordingly.
> > 
> Okay.
> 
> > > + *
> > > + * http://www.opensource.org/licenses/gpl-license.html
> > > + * http://www.gnu.org/copyleft/gpl.html
> > > + */
> 
> -- 
> Regards,
> Shawn
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-05  1:12         ` Shawn Guo
@ 2012-09-05  5:02           ` Shilimkar, Santosh
  -1 siblings, 0 replies; 54+ messages in thread
From: Shilimkar, Santosh @ 2012-09-05  5:02 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rafael J. Wysocki, cpufreq, linux-pm, Kevin Hilman,
	Nishanth Menon, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, AnilKumar Ch,
	devicetree-discuss, linux-arm-kernel

Shawn,

On Wed, Sep 5, 2012 at 6:42 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
>> On Friday, August 10, 2012, Shawn Guo wrote:
>> > It adds a generic cpufreq driver for CPU0 frequency management based on
>> > clk, regulator, OPP and device tree support.  It can support both
>> > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
>> > share clock and voltage across all CPUs.
>> >
>> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>
>> I seem to recall that this has been discussed with several people and
>> undergone some changes as a result of comments.  Would it be possible
>> to get some ACKs from the people involved?
>>
>
> Mark, Santosh,
>
> Reviewed-by, please?
>
As commented already, the name of file was bit odd to me
otherwise, patch itself is fine by me.

FWIW ,
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-05  5:02           ` Shilimkar, Santosh
  0 siblings, 0 replies; 54+ messages in thread
From: Shilimkar, Santosh @ 2012-09-05  5:02 UTC (permalink / raw)
  To: linux-arm-kernel

Shawn,

On Wed, Sep 5, 2012 at 6:42 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
>> On Friday, August 10, 2012, Shawn Guo wrote:
>> > It adds a generic cpufreq driver for CPU0 frequency management based on
>> > clk, regulator, OPP and device tree support.  It can support both
>> > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
>> > share clock and voltage across all CPUs.
>> >
>> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>
>> I seem to recall that this has been discussed with several people and
>> undergone some changes as a result of comments.  Would it be possible
>> to get some ACKs from the people involved?
>>
>
> Mark, Santosh,
>
> Reviewed-by, please?
>
As commented already, the name of file was bit odd to me
otherwise, patch itself is fine by me.

FWIW ,
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-05  1:12         ` Shawn Guo
@ 2012-09-05 13:38           ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-05 13:38 UTC (permalink / raw)
  To: Shawn Guo
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, AnilKumar Ch,
	devicetree-discuss, linux-arm-kernel

On Wednesday, September 05, 2012, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
> > On Friday, August 10, 2012, Shawn Guo wrote:
> > > It adds a generic cpufreq driver for CPU0 frequency management based on
> > > clk, regulator, OPP and device tree support.  It can support both
> > > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> > > share clock and voltage across all CPUs.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > I seem to recall that this has been discussed with several people and
> > undergone some changes as a result of comments.  Would it be possible
> > to get some ACKs from the people involved?
> > 
> 
> Mark, Santosh,
> 
> Reviewed-by, please?
> 
> AnilKumar,
> 
> Tested-by, please?
> 
> <snip>
> 
> > > --- /dev/null
> > > +++ b/drivers/cpufreq/cpufreq-cpu0.c
> > > @@ -0,0 +1,271 @@
> > > +/*
> > > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > > + *
> > > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
> > 
> > What do you mean by "reuse" here?
> > 
> cpu0_set_target() function is almost a copy of omap_target() from
> drivers/cpufreq/omap-cpufreq.c.  I should probably just say so?

Well, what about actually avoiding the code duplication?  That is,
can you make OMAP be a user of your new core function and drop the
OMAP-specific one, perhaps?

Rafael


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-05 13:38           ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-05 13:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, September 05, 2012, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote:
> > On Friday, August 10, 2012, Shawn Guo wrote:
> > > It adds a generic cpufreq driver for CPU0 frequency management based on
> > > clk, regulator, OPP and device tree support.  It can support both
> > > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which
> > > share clock and voltage across all CPUs.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > I seem to recall that this has been discussed with several people and
> > undergone some changes as a result of comments.  Would it be possible
> > to get some ACKs from the people involved?
> > 
> 
> Mark, Santosh,
> 
> Reviewed-by, please?
> 
> AnilKumar,
> 
> Tested-by, please?
> 
> <snip>
> 
> > > --- /dev/null
> > > +++ b/drivers/cpufreq/cpufreq-cpu0.c
> > > @@ -0,0 +1,271 @@
> > > +/*
> > > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > > + *
> > > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c
> > 
> > What do you mean by "reuse" here?
> > 
> cpu0_set_target() function is almost a copy of omap_target() from
> drivers/cpufreq/omap-cpufreq.c.  I should probably just say so?

Well, what about actually avoiding the code duplication?  That is,
can you make OMAP be a user of your new core function and drop the
OMAP-specific one, perhaps?

Rafael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-05 13:38           ` Rafael J. Wysocki
@ 2012-09-05 13:59             ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-05 13:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, AnilKumar Ch,
	devicetree-discuss, linux-arm-kernel

On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> Well, what about actually avoiding the code duplication?  That is,
> can you make OMAP be a user of your new core function and drop the
> OMAP-specific one, perhaps?
> 
I would expect that the driver will replace the omap-cpufreq driver
completely at some point, where omap becomes a DT only platform.

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-05 13:59             ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-05 13:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> Well, what about actually avoiding the code duplication?  That is,
> can you make OMAP be a user of your new core function and drop the
> OMAP-specific one, perhaps?
> 
I would expect that the driver will replace the omap-cpufreq driver
completely at some point, where omap becomes a DT only platform.

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-05 13:59             ` Shawn Guo
@ 2012-09-05 20:18               ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-05 20:18 UTC (permalink / raw)
  To: Shawn Guo
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, AnilKumar Ch,
	devicetree-discuss, linux-arm-kernel

On Wednesday, September 05, 2012, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> > Well, what about actually avoiding the code duplication?  That is,
> > can you make OMAP be a user of your new core function and drop the
> > OMAP-specific one, perhaps?
> > 
> I would expect that the driver will replace the omap-cpufreq driver
> completely at some point, where omap becomes a DT only platform.

That's fine, but why do we need two almost identical functions to start with?

Rafael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-05 20:18               ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-05 20:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, September 05, 2012, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> > Well, what about actually avoiding the code duplication?  That is,
> > can you make OMAP be a user of your new core function and drop the
> > OMAP-specific one, perhaps?
> > 
> I would expect that the driver will replace the omap-cpufreq driver
> completely at some point, where omap becomes a DT only platform.

That's fine, but why do we need two almost identical functions to start with?

Rafael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-05 20:18               ` Rafael J. Wysocki
@ 2012-09-06  7:29                 ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-06  7:29 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: cpufreq, linux-pm, Kevin Hilman, Nishanth Menon,
	Shilimkar Santosh, Richard Zhao, Russell King - ARM Linux,
	Mike Turquette, Mark Brown, Rob Herring, AnilKumar Ch,
	devicetree-discuss, linux-arm-kernel

On Wed, Sep 05, 2012 at 10:18:50PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 05, 2012, Shawn Guo wrote:
> > On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> > > Well, what about actually avoiding the code duplication?  That is,
> > > can you make OMAP be a user of your new core function and drop the
> > > OMAP-specific one, perhaps?
> > > 
> > I would expect that the driver will replace the omap-cpufreq driver
> > completely at some point, where omap becomes a DT only platform.
> 
> That's fine, but why do we need two almost identical functions to start with?
> 
Probably because it does not worth the churn considering that any
particular cpufreq driver having an identical .set_target as this
generic one should eventually be replaced by this driver completely,
IMO.

But anyway, it could be another patch which may need more discussion,
so I just submitted the v4 with leaving this particular comment to be
addressed in another patch.

I'm unsure this is what you are ordering.  But here is what I come up
with to address your comment.  Please let me know if this is exactly
what you are asking for, or you expect something different.

Regards,
Shawn

8<---
 drivers/cpufreq/Kconfig          |    4 ++
 drivers/cpufreq/Kconfig.arm      |    1 +
 drivers/cpufreq/Makefile         |    1 +
 drivers/cpufreq/cpufreq-cpu0.c   |   94 +++++-----------------------------
 drivers/cpufreq/cpufreq.h        |   31 +++++++++++
 drivers/cpufreq/cpufreq_target.c |   99 +++++++++++++++++++++++++++++++++++
 drivers/cpufreq/omap-cpufreq.c   |  105 +++++---------------------------------
 7 files changed, 163 insertions(+), 172 deletions(-)
 create mode 100644 drivers/cpufreq/cpufreq.h
 create mode 100644 drivers/cpufreq/cpufreq_target.c

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index ea512f4..206eec9 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -20,6 +20,9 @@ if CPU_FREQ
 config CPU_FREQ_TABLE
 	tristate
 
+config CPU_FREQ_TARGET
+	tristate
+
 config CPU_FREQ_STAT
 	tristate "CPU frequency translation statistics"
 	select CPU_FREQ_TABLE
@@ -183,6 +186,7 @@ config GENERIC_CPUFREQ_CPU0
 	bool "Generic CPU0 cpufreq driver"
 	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
 	select CPU_FREQ_TABLE
+	select CPU_FREQ_TARGET
 	help
 	  This adds a generic cpufreq driver for CPU0 frequency management.
 	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 5961e64..60d81d0 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -7,6 +7,7 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 	select CPU_FREQ_TABLE
+	select CPU_FREQ_TARGET
 
 config ARM_S3C2416_CPUFREQ
 	bool "S3C2416 CPU Frequency scaling support"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index a378ed2..f7d19d1 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
 
 # CPUfreq cross-arch helpers
 obj-$(CONFIG_CPU_FREQ_TABLE)		+= freq_table.o
+obj-$(CONFIG_CPU_FREQ_TARGET)		+= cpufreq_target.o
 
 obj-$(CONFIG_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
 
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index e915827..51096e8 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -1,9 +1,6 @@
 /*
  * Copyright (C) 2012 Freescale Semiconductor, Inc.
  *
- * The OPP code in function cpu0_set_target() is reused from
- * drivers/cpufreq/omap-cpufreq.c
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -20,6 +17,7 @@
 #include <linux/opp.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include "cpufreq.h"
 
 static unsigned int transition_latency;
 static unsigned int voltage_tolerance; /* in percentage */
@@ -42,84 +40,18 @@ static unsigned int cpu0_get_speed(unsigned int cpu)
 static int cpu0_set_target(struct cpufreq_policy *policy,
 			   unsigned int target_freq, unsigned int relation)
 {
-	struct cpufreq_freqs freqs;
-	struct opp *opp;
-	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
-	unsigned int index, cpu;
-	int ret;
-
-	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
-					     relation, &index);
-	if (ret) {
-		pr_err("failed to match target freqency %d: %d\n",
-		       target_freq, ret);
-		return ret;
-	}
-
-	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
-	if (freq_Hz < 0)
-		freq_Hz = freq_table[index].frequency * 1000;
-	freqs.new = freq_Hz / 1000;
-	freqs.old = clk_get_rate(cpu_clk) / 1000;
-
-	if (freqs.old == freqs.new)
-		return 0;
-
-	for_each_online_cpu(cpu) {
-		freqs.cpu = cpu;
-		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
-	}
-
-	if (cpu_reg) {
-		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
-		if (IS_ERR(opp)) {
-			pr_err("failed to find OPP for %ld\n", freq_Hz);
-			return PTR_ERR(opp);
-		}
-		volt = opp_get_voltage(opp);
-		tol = volt * voltage_tolerance / 100;
-		volt_old = regulator_get_voltage(cpu_reg);
-	}
-
-	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
-		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
-		 freqs.new / 1000, volt ? volt / 1000 : -1);
-
-	/* scaling up?  scale voltage before frequency */
-	if (cpu_reg && freqs.new > freqs.old) {
-		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
-		if (ret) {
-			pr_err("failed to scale voltage up: %d\n", ret);
-			freqs.new = freqs.old;
-			return ret;
-		}
-	}
-
-	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
-	if (ret) {
-		pr_err("failed to set clock rate: %d\n", ret);
-		if (cpu_reg)
-			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
-		return ret;
-	}
-
-	/* scaling down?  scale voltage after frequency */
-	if (cpu_reg && freqs.new < freqs.old) {
-		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
-		if (ret) {
-			pr_err("failed to scale voltage down: %d\n", ret);
-			clk_set_rate(cpu_clk, freqs.old * 1000);
-			freqs.new = freqs.old;
-			return ret;
-		}
-	}
-
-	for_each_online_cpu(cpu) {
-		freqs.cpu = cpu;
-		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
-	}
-
-	return 0;
+	struct cpufreq_target_data data;
+
+	data.dev = cpu_dev;
+	data.clk = cpu_clk;
+	data.reg = cpu_reg;
+	data.tol = voltage_tolerance;
+	data.freq_table = freq_table;
+	data.policy = policy;
+	data.target_freq = target_freq;
+	data.relation = relation;
+
+	return cpufreq_set_target(&data);
 }
 
 static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
diff --git a/drivers/cpufreq/cpufreq.h b/drivers/cpufreq/cpufreq.h
new file mode 100644
index 0000000..ae380b3
--- /dev/null
+++ b/drivers/cpufreq/cpufreq.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DRIVERS_CPUFREQ_H
+#define _DRIVERS_CPUFREQ_H
+
+struct device;
+struct clk;
+struct regulator;
+struct cpufreq_frequency_table;
+struct cpufreq_policy;
+
+struct cpufreq_target_data {
+	struct device *dev;
+	struct clk *clk;
+	struct regulator *reg;
+	unsigned int tol;
+	struct cpufreq_frequency_table *freq_table;
+	struct cpufreq_policy *policy;
+	unsigned int target_freq;
+	unsigned int relation;
+};
+
+int cpufreq_set_target(struct cpufreq_target_data *data);
+
+#endif /* _DRIVERS_CPUFREQ_H */
diff --git a/drivers/cpufreq/cpufreq_target.c b/drivers/cpufreq/cpufreq_target.c
new file mode 100644
index 0000000..02a5584
--- /dev/null
+++ b/drivers/cpufreq/cpufreq_target.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * Based on function omap_target() from drivers/cpufreq/omap-cpufreq.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/clk.h>
+#include <linux/cpufreq.h>
+#include <linux/module.h>
+#include <linux/opp.h>
+#include <linux/regulator/consumer.h>
+#include "cpufreq.h"
+
+int cpufreq_set_target(struct cpufreq_target_data *d)
+{
+	struct cpufreq_freqs freqs;
+	struct opp *opp;
+	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
+	unsigned int index, cpu;
+	int ret;
+
+	ret = cpufreq_frequency_table_target(d->policy, d->freq_table,
+					     d->target_freq, d->relation,
+					     &index);
+	if (ret) {
+		pr_err("failed to match target freqency %d: %d\n",
+		       d->target_freq, ret);
+		return ret;
+	}
+
+	freq_Hz = clk_round_rate(d->clk, d->freq_table[index].frequency * 1000);
+	if (freq_Hz < 0)
+		freq_Hz = d->freq_table[index].frequency * 1000;
+	freqs.new = freq_Hz / 1000;
+	freqs.old = clk_get_rate(d->clk) / 1000;
+
+	if (freqs.old == freqs.new)
+		return 0;
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	}
+
+	if (d->reg) {
+		opp = opp_find_freq_ceil(d->dev, &freq_Hz);
+		if (IS_ERR(opp)) {
+			pr_err("failed to find OPP for %ld\n", freq_Hz);
+			return PTR_ERR(opp);
+		}
+		volt = opp_get_voltage(opp);
+		tol = volt * d->tol / 100;
+		volt_old = regulator_get_voltage(d->reg);
+	}
+
+	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
+		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
+		 freqs.new / 1000, volt ? volt / 1000 : -1);
+
+	/* scaling up?  scale voltage before frequency */
+	if (d->reg && freqs.new > freqs.old) {
+		ret = regulator_set_voltage_tol(d->reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage up: %d\n", ret);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	ret = clk_set_rate(d->clk, freqs.new * 1000);
+	if (ret) {
+		pr_err("failed to set clock rate: %d\n", ret);
+		if (d->reg)
+			regulator_set_voltage_tol(d->reg, volt_old, tol);
+		return ret;
+	}
+
+	/* scaling down?  scale voltage after frequency */
+	if (d->reg && freqs.new < freqs.old) {
+		ret = regulator_set_voltage_tol(d->reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage down: %d\n", ret);
+			clk_set_rate(d->clk, freqs.old * 1000);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpufreq_set_target);
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 83a78ad..0772df5 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -37,6 +37,8 @@
 
 #include <mach/hardware.h>
 
+#include "cpufreq.h"
+
 /* OPP tolerance in percentage */
 #define	OPP_TOLERANCE	4
 
@@ -69,97 +71,18 @@ static int omap_target(struct cpufreq_policy *policy,
 		       unsigned int target_freq,
 		       unsigned int relation)
 {
-	unsigned int i;
-	int r, ret = 0;
-	struct cpufreq_freqs freqs;
-	struct opp *opp;
-	unsigned long freq, volt = 0, volt_old = 0, tol = 0;
-
-	if (!freq_table) {
-		dev_err(mpu_dev, "%s: cpu%d: no freq table!\n", __func__,
-				policy->cpu);
-		return -EINVAL;
-	}
-
-	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
-			relation, &i);
-	if (ret) {
-		dev_dbg(mpu_dev, "%s: cpu%d: no freq match for %d(ret=%d)\n",
-			__func__, policy->cpu, target_freq, ret);
-		return ret;
-	}
-	freqs.new = freq_table[i].frequency;
-	if (!freqs.new) {
-		dev_err(mpu_dev, "%s: cpu%d: no match for freq %d\n", __func__,
-			policy->cpu, target_freq);
-		return -EINVAL;
-	}
-
-	freqs.old = omap_getspeed(policy->cpu);
-	freqs.cpu = policy->cpu;
-
-	if (freqs.old == freqs.new && policy->cur == freqs.new)
-		return ret;
-
-	/* notifiers */
-	for_each_cpu(i, policy->cpus) {
-		freqs.cpu = i;
-		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
-	}
-
-	freq = freqs.new * 1000;
-
-	if (mpu_reg) {
-		opp = opp_find_freq_ceil(mpu_dev, &freq);
-		if (IS_ERR(opp)) {
-			dev_err(mpu_dev, "%s: unable to find MPU OPP for %d\n",
-				__func__, freqs.new);
-			return -EINVAL;
-		}
-		volt = opp_get_voltage(opp);
-		tol = volt * OPP_TOLERANCE / 100;
-		volt_old = regulator_get_voltage(mpu_reg);
-	}
-
-	dev_dbg(mpu_dev, "cpufreq-omap: %u MHz, %ld mV --> %u MHz, %ld mV\n", 
-		freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
-		freqs.new / 1000, volt ? volt / 1000 : -1);
-
-	/* scaling up?  scale voltage before frequency */
-	if (mpu_reg && (freqs.new > freqs.old)) {
-		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
-		if (r < 0) {
-			dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
-				 __func__);
-			freqs.new = freqs.old;
-			goto done;
-		}
-	}
-
-	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
-
-	/* scaling down?  scale voltage after frequency */
-	if (mpu_reg && (freqs.new < freqs.old)) {
-		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
-		if (r < 0) {
-			dev_warn(mpu_dev, "%s: unable to scale voltage down.\n",
-				 __func__);
-			ret = clk_set_rate(mpu_clk, freqs.old * 1000);
-			freqs.new = freqs.old;
-			goto done;
-		}
-	}
-
-	freqs.new = omap_getspeed(policy->cpu);
-
-done:
-	/* notifiers */
-	for_each_cpu(i, policy->cpus) {
-		freqs.cpu = i;
-		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
-	}
-
-	return ret;
+	struct cpufreq_target_data data;
+
+	data.dev = mpu_dev;
+	data.clk = mpu_clk;
+	data.reg = mpu_reg;
+	data.tol = OPP_TOLERANCE;
+	data.freq_table = freq_table;
+	data.policy = policy;
+	data.target_freq = target_freq;
+	data.relation = relation;
+
+	return cpufreq_set_target(&data);
 }
 
 static inline void freq_table_free(void)
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-06  7:29                 ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-06  7:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 05, 2012 at 10:18:50PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 05, 2012, Shawn Guo wrote:
> > On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> > > Well, what about actually avoiding the code duplication?  That is,
> > > can you make OMAP be a user of your new core function and drop the
> > > OMAP-specific one, perhaps?
> > > 
> > I would expect that the driver will replace the omap-cpufreq driver
> > completely at some point, where omap becomes a DT only platform.
> 
> That's fine, but why do we need two almost identical functions to start with?
> 
Probably because it does not worth the churn considering that any
particular cpufreq driver having an identical .set_target as this
generic one should eventually be replaced by this driver completely,
IMO.

But anyway, it could be another patch which may need more discussion,
so I just submitted the v4 with leaving this particular comment to be
addressed in another patch.

I'm unsure this is what you are ordering.  But here is what I come up
with to address your comment.  Please let me know if this is exactly
what you are asking for, or you expect something different.

Regards,
Shawn

8<---
 drivers/cpufreq/Kconfig          |    4 ++
 drivers/cpufreq/Kconfig.arm      |    1 +
 drivers/cpufreq/Makefile         |    1 +
 drivers/cpufreq/cpufreq-cpu0.c   |   94 +++++-----------------------------
 drivers/cpufreq/cpufreq.h        |   31 +++++++++++
 drivers/cpufreq/cpufreq_target.c |   99 +++++++++++++++++++++++++++++++++++
 drivers/cpufreq/omap-cpufreq.c   |  105 +++++---------------------------------
 7 files changed, 163 insertions(+), 172 deletions(-)
 create mode 100644 drivers/cpufreq/cpufreq.h
 create mode 100644 drivers/cpufreq/cpufreq_target.c

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index ea512f4..206eec9 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -20,6 +20,9 @@ if CPU_FREQ
 config CPU_FREQ_TABLE
 	tristate
 
+config CPU_FREQ_TARGET
+	tristate
+
 config CPU_FREQ_STAT
 	tristate "CPU frequency translation statistics"
 	select CPU_FREQ_TABLE
@@ -183,6 +186,7 @@ config GENERIC_CPUFREQ_CPU0
 	bool "Generic CPU0 cpufreq driver"
 	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
 	select CPU_FREQ_TABLE
+	select CPU_FREQ_TARGET
 	help
 	  This adds a generic cpufreq driver for CPU0 frequency management.
 	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 5961e64..60d81d0 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -7,6 +7,7 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 	select CPU_FREQ_TABLE
+	select CPU_FREQ_TARGET
 
 config ARM_S3C2416_CPUFREQ
 	bool "S3C2416 CPU Frequency scaling support"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index a378ed2..f7d19d1 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
 
 # CPUfreq cross-arch helpers
 obj-$(CONFIG_CPU_FREQ_TABLE)		+= freq_table.o
+obj-$(CONFIG_CPU_FREQ_TARGET)		+= cpufreq_target.o
 
 obj-$(CONFIG_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
 
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index e915827..51096e8 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -1,9 +1,6 @@
 /*
  * Copyright (C) 2012 Freescale Semiconductor, Inc.
  *
- * The OPP code in function cpu0_set_target() is reused from
- * drivers/cpufreq/omap-cpufreq.c
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -20,6 +17,7 @@
 #include <linux/opp.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include "cpufreq.h"
 
 static unsigned int transition_latency;
 static unsigned int voltage_tolerance; /* in percentage */
@@ -42,84 +40,18 @@ static unsigned int cpu0_get_speed(unsigned int cpu)
 static int cpu0_set_target(struct cpufreq_policy *policy,
 			   unsigned int target_freq, unsigned int relation)
 {
-	struct cpufreq_freqs freqs;
-	struct opp *opp;
-	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
-	unsigned int index, cpu;
-	int ret;
-
-	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
-					     relation, &index);
-	if (ret) {
-		pr_err("failed to match target freqency %d: %d\n",
-		       target_freq, ret);
-		return ret;
-	}
-
-	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
-	if (freq_Hz < 0)
-		freq_Hz = freq_table[index].frequency * 1000;
-	freqs.new = freq_Hz / 1000;
-	freqs.old = clk_get_rate(cpu_clk) / 1000;
-
-	if (freqs.old == freqs.new)
-		return 0;
-
-	for_each_online_cpu(cpu) {
-		freqs.cpu = cpu;
-		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
-	}
-
-	if (cpu_reg) {
-		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
-		if (IS_ERR(opp)) {
-			pr_err("failed to find OPP for %ld\n", freq_Hz);
-			return PTR_ERR(opp);
-		}
-		volt = opp_get_voltage(opp);
-		tol = volt * voltage_tolerance / 100;
-		volt_old = regulator_get_voltage(cpu_reg);
-	}
-
-	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
-		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
-		 freqs.new / 1000, volt ? volt / 1000 : -1);
-
-	/* scaling up?  scale voltage before frequency */
-	if (cpu_reg && freqs.new > freqs.old) {
-		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
-		if (ret) {
-			pr_err("failed to scale voltage up: %d\n", ret);
-			freqs.new = freqs.old;
-			return ret;
-		}
-	}
-
-	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
-	if (ret) {
-		pr_err("failed to set clock rate: %d\n", ret);
-		if (cpu_reg)
-			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
-		return ret;
-	}
-
-	/* scaling down?  scale voltage after frequency */
-	if (cpu_reg && freqs.new < freqs.old) {
-		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
-		if (ret) {
-			pr_err("failed to scale voltage down: %d\n", ret);
-			clk_set_rate(cpu_clk, freqs.old * 1000);
-			freqs.new = freqs.old;
-			return ret;
-		}
-	}
-
-	for_each_online_cpu(cpu) {
-		freqs.cpu = cpu;
-		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
-	}
-
-	return 0;
+	struct cpufreq_target_data data;
+
+	data.dev = cpu_dev;
+	data.clk = cpu_clk;
+	data.reg = cpu_reg;
+	data.tol = voltage_tolerance;
+	data.freq_table = freq_table;
+	data.policy = policy;
+	data.target_freq = target_freq;
+	data.relation = relation;
+
+	return cpufreq_set_target(&data);
 }
 
 static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
diff --git a/drivers/cpufreq/cpufreq.h b/drivers/cpufreq/cpufreq.h
new file mode 100644
index 0000000..ae380b3
--- /dev/null
+++ b/drivers/cpufreq/cpufreq.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DRIVERS_CPUFREQ_H
+#define _DRIVERS_CPUFREQ_H
+
+struct device;
+struct clk;
+struct regulator;
+struct cpufreq_frequency_table;
+struct cpufreq_policy;
+
+struct cpufreq_target_data {
+	struct device *dev;
+	struct clk *clk;
+	struct regulator *reg;
+	unsigned int tol;
+	struct cpufreq_frequency_table *freq_table;
+	struct cpufreq_policy *policy;
+	unsigned int target_freq;
+	unsigned int relation;
+};
+
+int cpufreq_set_target(struct cpufreq_target_data *data);
+
+#endif /* _DRIVERS_CPUFREQ_H */
diff --git a/drivers/cpufreq/cpufreq_target.c b/drivers/cpufreq/cpufreq_target.c
new file mode 100644
index 0000000..02a5584
--- /dev/null
+++ b/drivers/cpufreq/cpufreq_target.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * Based on function omap_target() from drivers/cpufreq/omap-cpufreq.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/clk.h>
+#include <linux/cpufreq.h>
+#include <linux/module.h>
+#include <linux/opp.h>
+#include <linux/regulator/consumer.h>
+#include "cpufreq.h"
+
+int cpufreq_set_target(struct cpufreq_target_data *d)
+{
+	struct cpufreq_freqs freqs;
+	struct opp *opp;
+	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
+	unsigned int index, cpu;
+	int ret;
+
+	ret = cpufreq_frequency_table_target(d->policy, d->freq_table,
+					     d->target_freq, d->relation,
+					     &index);
+	if (ret) {
+		pr_err("failed to match target freqency %d: %d\n",
+		       d->target_freq, ret);
+		return ret;
+	}
+
+	freq_Hz = clk_round_rate(d->clk, d->freq_table[index].frequency * 1000);
+	if (freq_Hz < 0)
+		freq_Hz = d->freq_table[index].frequency * 1000;
+	freqs.new = freq_Hz / 1000;
+	freqs.old = clk_get_rate(d->clk) / 1000;
+
+	if (freqs.old == freqs.new)
+		return 0;
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	}
+
+	if (d->reg) {
+		opp = opp_find_freq_ceil(d->dev, &freq_Hz);
+		if (IS_ERR(opp)) {
+			pr_err("failed to find OPP for %ld\n", freq_Hz);
+			return PTR_ERR(opp);
+		}
+		volt = opp_get_voltage(opp);
+		tol = volt * d->tol / 100;
+		volt_old = regulator_get_voltage(d->reg);
+	}
+
+	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
+		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
+		 freqs.new / 1000, volt ? volt / 1000 : -1);
+
+	/* scaling up?  scale voltage before frequency */
+	if (d->reg && freqs.new > freqs.old) {
+		ret = regulator_set_voltage_tol(d->reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage up: %d\n", ret);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	ret = clk_set_rate(d->clk, freqs.new * 1000);
+	if (ret) {
+		pr_err("failed to set clock rate: %d\n", ret);
+		if (d->reg)
+			regulator_set_voltage_tol(d->reg, volt_old, tol);
+		return ret;
+	}
+
+	/* scaling down?  scale voltage after frequency */
+	if (d->reg && freqs.new < freqs.old) {
+		ret = regulator_set_voltage_tol(d->reg, volt, tol);
+		if (ret) {
+			pr_err("failed to scale voltage down: %d\n", ret);
+			clk_set_rate(d->clk, freqs.old * 1000);
+			freqs.new = freqs.old;
+			return ret;
+		}
+	}
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpufreq_set_target);
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 83a78ad..0772df5 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -37,6 +37,8 @@
 
 #include <mach/hardware.h>
 
+#include "cpufreq.h"
+
 /* OPP tolerance in percentage */
 #define	OPP_TOLERANCE	4
 
@@ -69,97 +71,18 @@ static int omap_target(struct cpufreq_policy *policy,
 		       unsigned int target_freq,
 		       unsigned int relation)
 {
-	unsigned int i;
-	int r, ret = 0;
-	struct cpufreq_freqs freqs;
-	struct opp *opp;
-	unsigned long freq, volt = 0, volt_old = 0, tol = 0;
-
-	if (!freq_table) {
-		dev_err(mpu_dev, "%s: cpu%d: no freq table!\n", __func__,
-				policy->cpu);
-		return -EINVAL;
-	}
-
-	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
-			relation, &i);
-	if (ret) {
-		dev_dbg(mpu_dev, "%s: cpu%d: no freq match for %d(ret=%d)\n",
-			__func__, policy->cpu, target_freq, ret);
-		return ret;
-	}
-	freqs.new = freq_table[i].frequency;
-	if (!freqs.new) {
-		dev_err(mpu_dev, "%s: cpu%d: no match for freq %d\n", __func__,
-			policy->cpu, target_freq);
-		return -EINVAL;
-	}
-
-	freqs.old = omap_getspeed(policy->cpu);
-	freqs.cpu = policy->cpu;
-
-	if (freqs.old == freqs.new && policy->cur == freqs.new)
-		return ret;
-
-	/* notifiers */
-	for_each_cpu(i, policy->cpus) {
-		freqs.cpu = i;
-		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
-	}
-
-	freq = freqs.new * 1000;
-
-	if (mpu_reg) {
-		opp = opp_find_freq_ceil(mpu_dev, &freq);
-		if (IS_ERR(opp)) {
-			dev_err(mpu_dev, "%s: unable to find MPU OPP for %d\n",
-				__func__, freqs.new);
-			return -EINVAL;
-		}
-		volt = opp_get_voltage(opp);
-		tol = volt * OPP_TOLERANCE / 100;
-		volt_old = regulator_get_voltage(mpu_reg);
-	}
-
-	dev_dbg(mpu_dev, "cpufreq-omap: %u MHz, %ld mV --> %u MHz, %ld mV\n", 
-		freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
-		freqs.new / 1000, volt ? volt / 1000 : -1);
-
-	/* scaling up?  scale voltage before frequency */
-	if (mpu_reg && (freqs.new > freqs.old)) {
-		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
-		if (r < 0) {
-			dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
-				 __func__);
-			freqs.new = freqs.old;
-			goto done;
-		}
-	}
-
-	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
-
-	/* scaling down?  scale voltage after frequency */
-	if (mpu_reg && (freqs.new < freqs.old)) {
-		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
-		if (r < 0) {
-			dev_warn(mpu_dev, "%s: unable to scale voltage down.\n",
-				 __func__);
-			ret = clk_set_rate(mpu_clk, freqs.old * 1000);
-			freqs.new = freqs.old;
-			goto done;
-		}
-	}
-
-	freqs.new = omap_getspeed(policy->cpu);
-
-done:
-	/* notifiers */
-	for_each_cpu(i, policy->cpus) {
-		freqs.cpu = i;
-		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
-	}
-
-	return ret;
+	struct cpufreq_target_data data;
+
+	data.dev = mpu_dev;
+	data.clk = mpu_clk;
+	data.reg = mpu_reg;
+	data.tol = OPP_TOLERANCE;
+	data.freq_table = freq_table;
+	data.policy = policy;
+	data.target_freq = target_freq;
+	data.relation = relation;
+
+	return cpufreq_set_target(&data);
 }
 
 static inline void freq_table_free(void)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
  2012-09-06  7:29                 ` Shawn Guo
@ 2012-09-06 19:59                     ` Rafael J. Wysocki
  -1 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-06 19:59 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thursday, September 06, 2012, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 10:18:50PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, September 05, 2012, Shawn Guo wrote:
> > > On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> > > > Well, what about actually avoiding the code duplication?  That is,
> > > > can you make OMAP be a user of your new core function and drop the
> > > > OMAP-specific one, perhaps?
> > > > 
> > > I would expect that the driver will replace the omap-cpufreq driver
> > > completely at some point, where omap becomes a DT only platform.
> > 
> > That's fine, but why do we need two almost identical functions to start with?
> > 
> Probably because it does not worth the churn considering that any
> particular cpufreq driver having an identical .set_target as this
> generic one should eventually be replaced by this driver completely,
> IMO.
> 
> But anyway, it could be another patch which may need more discussion,
> so I just submitted the v4 with leaving this particular comment to be
> addressed in another patch.

It may be a different patch, I'm only concerned about the final outcome
(i.e. no added code duplication, please).

> I'm unsure this is what you are ordering.  But here is what I come up
> with to address your comment.  Please let me know if this is exactly
> what you are asking for, or you expect something different.

Yes, it is.

Thanks,
Rafael


> 8<---
>  drivers/cpufreq/Kconfig          |    4 ++
>  drivers/cpufreq/Kconfig.arm      |    1 +
>  drivers/cpufreq/Makefile         |    1 +
>  drivers/cpufreq/cpufreq-cpu0.c   |   94 +++++-----------------------------
>  drivers/cpufreq/cpufreq.h        |   31 +++++++++++
>  drivers/cpufreq/cpufreq_target.c |   99 +++++++++++++++++++++++++++++++++++
>  drivers/cpufreq/omap-cpufreq.c   |  105 +++++---------------------------------
>  7 files changed, 163 insertions(+), 172 deletions(-)
>  create mode 100644 drivers/cpufreq/cpufreq.h
>  create mode 100644 drivers/cpufreq/cpufreq_target.c
> 
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index ea512f4..206eec9 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -20,6 +20,9 @@ if CPU_FREQ
>  config CPU_FREQ_TABLE
>  	tristate
>  
> +config CPU_FREQ_TARGET
> +	tristate
> +
>  config CPU_FREQ_STAT
>  	tristate "CPU frequency translation statistics"
>  	select CPU_FREQ_TABLE
> @@ -183,6 +186,7 @@ config GENERIC_CPUFREQ_CPU0
>  	bool "Generic CPU0 cpufreq driver"
>  	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
>  	select CPU_FREQ_TABLE
> +	select CPU_FREQ_TARGET
>  	help
>  	  This adds a generic cpufreq driver for CPU0 frequency management.
>  	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 5961e64..60d81d0 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -7,6 +7,7 @@ config ARM_OMAP2PLUS_CPUFREQ
>  	depends on ARCH_OMAP2PLUS
>  	default ARCH_OMAP2PLUS
>  	select CPU_FREQ_TABLE
> +	select CPU_FREQ_TARGET
>  
>  config ARM_S3C2416_CPUFREQ
>  	bool "S3C2416 CPU Frequency scaling support"
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index a378ed2..f7d19d1 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
>  
>  # CPUfreq cross-arch helpers
>  obj-$(CONFIG_CPU_FREQ_TABLE)		+= freq_table.o
> +obj-$(CONFIG_CPU_FREQ_TARGET)		+= cpufreq_target.o
>  
>  obj-$(CONFIG_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
>  
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index e915827..51096e8 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -1,9 +1,6 @@
>  /*
>   * Copyright (C) 2012 Freescale Semiconductor, Inc.
>   *
> - * The OPP code in function cpu0_set_target() is reused from
> - * drivers/cpufreq/omap-cpufreq.c
> - *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
> @@ -20,6 +17,7 @@
>  #include <linux/opp.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> +#include "cpufreq.h"
>  
>  static unsigned int transition_latency;
>  static unsigned int voltage_tolerance; /* in percentage */
> @@ -42,84 +40,18 @@ static unsigned int cpu0_get_speed(unsigned int cpu)
>  static int cpu0_set_target(struct cpufreq_policy *policy,
>  			   unsigned int target_freq, unsigned int relation)
>  {
> -	struct cpufreq_freqs freqs;
> -	struct opp *opp;
> -	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
> -	unsigned int index, cpu;
> -	int ret;
> -
> -	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> -					     relation, &index);
> -	if (ret) {
> -		pr_err("failed to match target freqency %d: %d\n",
> -		       target_freq, ret);
> -		return ret;
> -	}
> -
> -	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
> -	if (freq_Hz < 0)
> -		freq_Hz = freq_table[index].frequency * 1000;
> -	freqs.new = freq_Hz / 1000;
> -	freqs.old = clk_get_rate(cpu_clk) / 1000;
> -
> -	if (freqs.old == freqs.new)
> -		return 0;
> -
> -	for_each_online_cpu(cpu) {
> -		freqs.cpu = cpu;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> -	}
> -
> -	if (cpu_reg) {
> -		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
> -		if (IS_ERR(opp)) {
> -			pr_err("failed to find OPP for %ld\n", freq_Hz);
> -			return PTR_ERR(opp);
> -		}
> -		volt = opp_get_voltage(opp);
> -		tol = volt * voltage_tolerance / 100;
> -		volt_old = regulator_get_voltage(cpu_reg);
> -	}
> -
> -	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> -		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> -		 freqs.new / 1000, volt ? volt / 1000 : -1);
> -
> -	/* scaling up?  scale voltage before frequency */
> -	if (cpu_reg && freqs.new > freqs.old) {
> -		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> -		if (ret) {
> -			pr_err("failed to scale voltage up: %d\n", ret);
> -			freqs.new = freqs.old;
> -			return ret;
> -		}
> -	}
> -
> -	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
> -	if (ret) {
> -		pr_err("failed to set clock rate: %d\n", ret);
> -		if (cpu_reg)
> -			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
> -		return ret;
> -	}
> -
> -	/* scaling down?  scale voltage after frequency */
> -	if (cpu_reg && freqs.new < freqs.old) {
> -		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> -		if (ret) {
> -			pr_err("failed to scale voltage down: %d\n", ret);
> -			clk_set_rate(cpu_clk, freqs.old * 1000);
> -			freqs.new = freqs.old;
> -			return ret;
> -		}
> -	}
> -
> -	for_each_online_cpu(cpu) {
> -		freqs.cpu = cpu;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> -	}
> -
> -	return 0;
> +	struct cpufreq_target_data data;
> +
> +	data.dev = cpu_dev;
> +	data.clk = cpu_clk;
> +	data.reg = cpu_reg;
> +	data.tol = voltage_tolerance;
> +	data.freq_table = freq_table;
> +	data.policy = policy;
> +	data.target_freq = target_freq;
> +	data.relation = relation;
> +
> +	return cpufreq_set_target(&data);
>  }
>  
>  static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
> diff --git a/drivers/cpufreq/cpufreq.h b/drivers/cpufreq/cpufreq.h
> new file mode 100644
> index 0000000..ae380b3
> --- /dev/null
> +++ b/drivers/cpufreq/cpufreq.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _DRIVERS_CPUFREQ_H
> +#define _DRIVERS_CPUFREQ_H
> +
> +struct device;
> +struct clk;
> +struct regulator;
> +struct cpufreq_frequency_table;
> +struct cpufreq_policy;
> +
> +struct cpufreq_target_data {
> +	struct device *dev;
> +	struct clk *clk;
> +	struct regulator *reg;
> +	unsigned int tol;
> +	struct cpufreq_frequency_table *freq_table;
> +	struct cpufreq_policy *policy;
> +	unsigned int target_freq;
> +	unsigned int relation;
> +};
> +
> +int cpufreq_set_target(struct cpufreq_target_data *data);
> +
> +#endif /* _DRIVERS_CPUFREQ_H */
> diff --git a/drivers/cpufreq/cpufreq_target.c b/drivers/cpufreq/cpufreq_target.c
> new file mode 100644
> index 0000000..02a5584
> --- /dev/null
> +++ b/drivers/cpufreq/cpufreq_target.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + *
> + * Based on function omap_target() from drivers/cpufreq/omap-cpufreq.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/clk.h>
> +#include <linux/cpufreq.h>
> +#include <linux/module.h>
> +#include <linux/opp.h>
> +#include <linux/regulator/consumer.h>
> +#include "cpufreq.h"
> +
> +int cpufreq_set_target(struct cpufreq_target_data *d)
> +{
> +	struct cpufreq_freqs freqs;
> +	struct opp *opp;
> +	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
> +	unsigned int index, cpu;
> +	int ret;
> +
> +	ret = cpufreq_frequency_table_target(d->policy, d->freq_table,
> +					     d->target_freq, d->relation,
> +					     &index);
> +	if (ret) {
> +		pr_err("failed to match target freqency %d: %d\n",
> +		       d->target_freq, ret);
> +		return ret;
> +	}
> +
> +	freq_Hz = clk_round_rate(d->clk, d->freq_table[index].frequency * 1000);
> +	if (freq_Hz < 0)
> +		freq_Hz = d->freq_table[index].frequency * 1000;
> +	freqs.new = freq_Hz / 1000;
> +	freqs.old = clk_get_rate(d->clk) / 1000;
> +
> +	if (freqs.old == freqs.new)
> +		return 0;
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +	}
> +
> +	if (d->reg) {
> +		opp = opp_find_freq_ceil(d->dev, &freq_Hz);
> +		if (IS_ERR(opp)) {
> +			pr_err("failed to find OPP for %ld\n", freq_Hz);
> +			return PTR_ERR(opp);
> +		}
> +		volt = opp_get_voltage(opp);
> +		tol = volt * d->tol / 100;
> +		volt_old = regulator_get_voltage(d->reg);
> +	}
> +
> +	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> +		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> +		 freqs.new / 1000, volt ? volt / 1000 : -1);
> +
> +	/* scaling up?  scale voltage before frequency */
> +	if (d->reg && freqs.new > freqs.old) {
> +		ret = regulator_set_voltage_tol(d->reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage up: %d\n", ret);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_set_rate(d->clk, freqs.new * 1000);
> +	if (ret) {
> +		pr_err("failed to set clock rate: %d\n", ret);
> +		if (d->reg)
> +			regulator_set_voltage_tol(d->reg, volt_old, tol);
> +		return ret;
> +	}
> +
> +	/* scaling down?  scale voltage after frequency */
> +	if (d->reg && freqs.new < freqs.old) {
> +		ret = regulator_set_voltage_tol(d->reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage down: %d\n", ret);
> +			clk_set_rate(d->clk, freqs.old * 1000);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_set_target);
> diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> index 83a78ad..0772df5 100644
> --- a/drivers/cpufreq/omap-cpufreq.c
> +++ b/drivers/cpufreq/omap-cpufreq.c
> @@ -37,6 +37,8 @@
>  
>  #include <mach/hardware.h>
>  
> +#include "cpufreq.h"
> +
>  /* OPP tolerance in percentage */
>  #define	OPP_TOLERANCE	4
>  
> @@ -69,97 +71,18 @@ static int omap_target(struct cpufreq_policy *policy,
>  		       unsigned int target_freq,
>  		       unsigned int relation)
>  {
> -	unsigned int i;
> -	int r, ret = 0;
> -	struct cpufreq_freqs freqs;
> -	struct opp *opp;
> -	unsigned long freq, volt = 0, volt_old = 0, tol = 0;
> -
> -	if (!freq_table) {
> -		dev_err(mpu_dev, "%s: cpu%d: no freq table!\n", __func__,
> -				policy->cpu);
> -		return -EINVAL;
> -	}
> -
> -	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> -			relation, &i);
> -	if (ret) {
> -		dev_dbg(mpu_dev, "%s: cpu%d: no freq match for %d(ret=%d)\n",
> -			__func__, policy->cpu, target_freq, ret);
> -		return ret;
> -	}
> -	freqs.new = freq_table[i].frequency;
> -	if (!freqs.new) {
> -		dev_err(mpu_dev, "%s: cpu%d: no match for freq %d\n", __func__,
> -			policy->cpu, target_freq);
> -		return -EINVAL;
> -	}
> -
> -	freqs.old = omap_getspeed(policy->cpu);
> -	freqs.cpu = policy->cpu;
> -
> -	if (freqs.old == freqs.new && policy->cur == freqs.new)
> -		return ret;
> -
> -	/* notifiers */
> -	for_each_cpu(i, policy->cpus) {
> -		freqs.cpu = i;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> -	}
> -
> -	freq = freqs.new * 1000;
> -
> -	if (mpu_reg) {
> -		opp = opp_find_freq_ceil(mpu_dev, &freq);
> -		if (IS_ERR(opp)) {
> -			dev_err(mpu_dev, "%s: unable to find MPU OPP for %d\n",
> -				__func__, freqs.new);
> -			return -EINVAL;
> -		}
> -		volt = opp_get_voltage(opp);
> -		tol = volt * OPP_TOLERANCE / 100;
> -		volt_old = regulator_get_voltage(mpu_reg);
> -	}
> -
> -	dev_dbg(mpu_dev, "cpufreq-omap: %u MHz, %ld mV --> %u MHz, %ld mV\n", 
> -		freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> -		freqs.new / 1000, volt ? volt / 1000 : -1);
> -
> -	/* scaling up?  scale voltage before frequency */
> -	if (mpu_reg && (freqs.new > freqs.old)) {
> -		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
> -		if (r < 0) {
> -			dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
> -				 __func__);
> -			freqs.new = freqs.old;
> -			goto done;
> -		}
> -	}
> -
> -	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> -
> -	/* scaling down?  scale voltage after frequency */
> -	if (mpu_reg && (freqs.new < freqs.old)) {
> -		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
> -		if (r < 0) {
> -			dev_warn(mpu_dev, "%s: unable to scale voltage down.\n",
> -				 __func__);
> -			ret = clk_set_rate(mpu_clk, freqs.old * 1000);
> -			freqs.new = freqs.old;
> -			goto done;
> -		}
> -	}
> -
> -	freqs.new = omap_getspeed(policy->cpu);
> -
> -done:
> -	/* notifiers */
> -	for_each_cpu(i, policy->cpus) {
> -		freqs.cpu = i;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> -	}
> -
> -	return ret;
> +	struct cpufreq_target_data data;
> +
> +	data.dev = mpu_dev;
> +	data.clk = mpu_clk;
> +	data.reg = mpu_reg;
> +	data.tol = OPP_TOLERANCE;
> +	data.freq_table = freq_table;
> +	data.policy = policy;
> +	data.target_freq = target_freq;
> +	data.relation = relation;
> +
> +	return cpufreq_set_target(&data);
>  }
>  
>  static inline void freq_table_free(void)
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
@ 2012-09-06 19:59                     ` Rafael J. Wysocki
  0 siblings, 0 replies; 54+ messages in thread
From: Rafael J. Wysocki @ 2012-09-06 19:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, September 06, 2012, Shawn Guo wrote:
> On Wed, Sep 05, 2012 at 10:18:50PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, September 05, 2012, Shawn Guo wrote:
> > > On Wed, Sep 05, 2012 at 03:38:12PM +0200, Rafael J. Wysocki wrote:
> > > > Well, what about actually avoiding the code duplication?  That is,
> > > > can you make OMAP be a user of your new core function and drop the
> > > > OMAP-specific one, perhaps?
> > > > 
> > > I would expect that the driver will replace the omap-cpufreq driver
> > > completely at some point, where omap becomes a DT only platform.
> > 
> > That's fine, but why do we need two almost identical functions to start with?
> > 
> Probably because it does not worth the churn considering that any
> particular cpufreq driver having an identical .set_target as this
> generic one should eventually be replaced by this driver completely,
> IMO.
> 
> But anyway, it could be another patch which may need more discussion,
> so I just submitted the v4 with leaving this particular comment to be
> addressed in another patch.

It may be a different patch, I'm only concerned about the final outcome
(i.e. no added code duplication, please).

> I'm unsure this is what you are ordering.  But here is what I come up
> with to address your comment.  Please let me know if this is exactly
> what you are asking for, or you expect something different.

Yes, it is.

Thanks,
Rafael


> 8<---
>  drivers/cpufreq/Kconfig          |    4 ++
>  drivers/cpufreq/Kconfig.arm      |    1 +
>  drivers/cpufreq/Makefile         |    1 +
>  drivers/cpufreq/cpufreq-cpu0.c   |   94 +++++-----------------------------
>  drivers/cpufreq/cpufreq.h        |   31 +++++++++++
>  drivers/cpufreq/cpufreq_target.c |   99 +++++++++++++++++++++++++++++++++++
>  drivers/cpufreq/omap-cpufreq.c   |  105 +++++---------------------------------
>  7 files changed, 163 insertions(+), 172 deletions(-)
>  create mode 100644 drivers/cpufreq/cpufreq.h
>  create mode 100644 drivers/cpufreq/cpufreq_target.c
> 
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index ea512f4..206eec9 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -20,6 +20,9 @@ if CPU_FREQ
>  config CPU_FREQ_TABLE
>  	tristate
>  
> +config CPU_FREQ_TARGET
> +	tristate
> +
>  config CPU_FREQ_STAT
>  	tristate "CPU frequency translation statistics"
>  	select CPU_FREQ_TABLE
> @@ -183,6 +186,7 @@ config GENERIC_CPUFREQ_CPU0
>  	bool "Generic CPU0 cpufreq driver"
>  	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
>  	select CPU_FREQ_TABLE
> +	select CPU_FREQ_TARGET
>  	help
>  	  This adds a generic cpufreq driver for CPU0 frequency management.
>  	  It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 5961e64..60d81d0 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -7,6 +7,7 @@ config ARM_OMAP2PLUS_CPUFREQ
>  	depends on ARCH_OMAP2PLUS
>  	default ARCH_OMAP2PLUS
>  	select CPU_FREQ_TABLE
> +	select CPU_FREQ_TARGET
>  
>  config ARM_S3C2416_CPUFREQ
>  	bool "S3C2416 CPU Frequency scaling support"
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index a378ed2..f7d19d1 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
>  
>  # CPUfreq cross-arch helpers
>  obj-$(CONFIG_CPU_FREQ_TABLE)		+= freq_table.o
> +obj-$(CONFIG_CPU_FREQ_TARGET)		+= cpufreq_target.o
>  
>  obj-$(CONFIG_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
>  
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index e915827..51096e8 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -1,9 +1,6 @@
>  /*
>   * Copyright (C) 2012 Freescale Semiconductor, Inc.
>   *
> - * The OPP code in function cpu0_set_target() is reused from
> - * drivers/cpufreq/omap-cpufreq.c
> - *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
> @@ -20,6 +17,7 @@
>  #include <linux/opp.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> +#include "cpufreq.h"
>  
>  static unsigned int transition_latency;
>  static unsigned int voltage_tolerance; /* in percentage */
> @@ -42,84 +40,18 @@ static unsigned int cpu0_get_speed(unsigned int cpu)
>  static int cpu0_set_target(struct cpufreq_policy *policy,
>  			   unsigned int target_freq, unsigned int relation)
>  {
> -	struct cpufreq_freqs freqs;
> -	struct opp *opp;
> -	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
> -	unsigned int index, cpu;
> -	int ret;
> -
> -	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> -					     relation, &index);
> -	if (ret) {
> -		pr_err("failed to match target freqency %d: %d\n",
> -		       target_freq, ret);
> -		return ret;
> -	}
> -
> -	freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
> -	if (freq_Hz < 0)
> -		freq_Hz = freq_table[index].frequency * 1000;
> -	freqs.new = freq_Hz / 1000;
> -	freqs.old = clk_get_rate(cpu_clk) / 1000;
> -
> -	if (freqs.old == freqs.new)
> -		return 0;
> -
> -	for_each_online_cpu(cpu) {
> -		freqs.cpu = cpu;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> -	}
> -
> -	if (cpu_reg) {
> -		opp = opp_find_freq_ceil(cpu_dev, &freq_Hz);
> -		if (IS_ERR(opp)) {
> -			pr_err("failed to find OPP for %ld\n", freq_Hz);
> -			return PTR_ERR(opp);
> -		}
> -		volt = opp_get_voltage(opp);
> -		tol = volt * voltage_tolerance / 100;
> -		volt_old = regulator_get_voltage(cpu_reg);
> -	}
> -
> -	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> -		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> -		 freqs.new / 1000, volt ? volt / 1000 : -1);
> -
> -	/* scaling up?  scale voltage before frequency */
> -	if (cpu_reg && freqs.new > freqs.old) {
> -		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> -		if (ret) {
> -			pr_err("failed to scale voltage up: %d\n", ret);
> -			freqs.new = freqs.old;
> -			return ret;
> -		}
> -	}
> -
> -	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
> -	if (ret) {
> -		pr_err("failed to set clock rate: %d\n", ret);
> -		if (cpu_reg)
> -			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
> -		return ret;
> -	}
> -
> -	/* scaling down?  scale voltage after frequency */
> -	if (cpu_reg && freqs.new < freqs.old) {
> -		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> -		if (ret) {
> -			pr_err("failed to scale voltage down: %d\n", ret);
> -			clk_set_rate(cpu_clk, freqs.old * 1000);
> -			freqs.new = freqs.old;
> -			return ret;
> -		}
> -	}
> -
> -	for_each_online_cpu(cpu) {
> -		freqs.cpu = cpu;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> -	}
> -
> -	return 0;
> +	struct cpufreq_target_data data;
> +
> +	data.dev = cpu_dev;
> +	data.clk = cpu_clk;
> +	data.reg = cpu_reg;
> +	data.tol = voltage_tolerance;
> +	data.freq_table = freq_table;
> +	data.policy = policy;
> +	data.target_freq = target_freq;
> +	data.relation = relation;
> +
> +	return cpufreq_set_target(&data);
>  }
>  
>  static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
> diff --git a/drivers/cpufreq/cpufreq.h b/drivers/cpufreq/cpufreq.h
> new file mode 100644
> index 0000000..ae380b3
> --- /dev/null
> +++ b/drivers/cpufreq/cpufreq.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _DRIVERS_CPUFREQ_H
> +#define _DRIVERS_CPUFREQ_H
> +
> +struct device;
> +struct clk;
> +struct regulator;
> +struct cpufreq_frequency_table;
> +struct cpufreq_policy;
> +
> +struct cpufreq_target_data {
> +	struct device *dev;
> +	struct clk *clk;
> +	struct regulator *reg;
> +	unsigned int tol;
> +	struct cpufreq_frequency_table *freq_table;
> +	struct cpufreq_policy *policy;
> +	unsigned int target_freq;
> +	unsigned int relation;
> +};
> +
> +int cpufreq_set_target(struct cpufreq_target_data *data);
> +
> +#endif /* _DRIVERS_CPUFREQ_H */
> diff --git a/drivers/cpufreq/cpufreq_target.c b/drivers/cpufreq/cpufreq_target.c
> new file mode 100644
> index 0000000..02a5584
> --- /dev/null
> +++ b/drivers/cpufreq/cpufreq_target.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + *
> + * Based on function omap_target() from drivers/cpufreq/omap-cpufreq.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/clk.h>
> +#include <linux/cpufreq.h>
> +#include <linux/module.h>
> +#include <linux/opp.h>
> +#include <linux/regulator/consumer.h>
> +#include "cpufreq.h"
> +
> +int cpufreq_set_target(struct cpufreq_target_data *d)
> +{
> +	struct cpufreq_freqs freqs;
> +	struct opp *opp;
> +	unsigned long freq_Hz, volt = 0, volt_old = 0, tol = 0;
> +	unsigned int index, cpu;
> +	int ret;
> +
> +	ret = cpufreq_frequency_table_target(d->policy, d->freq_table,
> +					     d->target_freq, d->relation,
> +					     &index);
> +	if (ret) {
> +		pr_err("failed to match target freqency %d: %d\n",
> +		       d->target_freq, ret);
> +		return ret;
> +	}
> +
> +	freq_Hz = clk_round_rate(d->clk, d->freq_table[index].frequency * 1000);
> +	if (freq_Hz < 0)
> +		freq_Hz = d->freq_table[index].frequency * 1000;
> +	freqs.new = freq_Hz / 1000;
> +	freqs.old = clk_get_rate(d->clk) / 1000;
> +
> +	if (freqs.old == freqs.new)
> +		return 0;
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +	}
> +
> +	if (d->reg) {
> +		opp = opp_find_freq_ceil(d->dev, &freq_Hz);
> +		if (IS_ERR(opp)) {
> +			pr_err("failed to find OPP for %ld\n", freq_Hz);
> +			return PTR_ERR(opp);
> +		}
> +		volt = opp_get_voltage(opp);
> +		tol = volt * d->tol / 100;
> +		volt_old = regulator_get_voltage(d->reg);
> +	}
> +
> +	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> +		 freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> +		 freqs.new / 1000, volt ? volt / 1000 : -1);
> +
> +	/* scaling up?  scale voltage before frequency */
> +	if (d->reg && freqs.new > freqs.old) {
> +		ret = regulator_set_voltage_tol(d->reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage up: %d\n", ret);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	ret = clk_set_rate(d->clk, freqs.new * 1000);
> +	if (ret) {
> +		pr_err("failed to set clock rate: %d\n", ret);
> +		if (d->reg)
> +			regulator_set_voltage_tol(d->reg, volt_old, tol);
> +		return ret;
> +	}
> +
> +	/* scaling down?  scale voltage after frequency */
> +	if (d->reg && freqs.new < freqs.old) {
> +		ret = regulator_set_voltage_tol(d->reg, volt, tol);
> +		if (ret) {
> +			pr_err("failed to scale voltage down: %d\n", ret);
> +			clk_set_rate(d->clk, freqs.old * 1000);
> +			freqs.new = freqs.old;
> +			return ret;
> +		}
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		freqs.cpu = cpu;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_set_target);
> diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> index 83a78ad..0772df5 100644
> --- a/drivers/cpufreq/omap-cpufreq.c
> +++ b/drivers/cpufreq/omap-cpufreq.c
> @@ -37,6 +37,8 @@
>  
>  #include <mach/hardware.h>
>  
> +#include "cpufreq.h"
> +
>  /* OPP tolerance in percentage */
>  #define	OPP_TOLERANCE	4
>  
> @@ -69,97 +71,18 @@ static int omap_target(struct cpufreq_policy *policy,
>  		       unsigned int target_freq,
>  		       unsigned int relation)
>  {
> -	unsigned int i;
> -	int r, ret = 0;
> -	struct cpufreq_freqs freqs;
> -	struct opp *opp;
> -	unsigned long freq, volt = 0, volt_old = 0, tol = 0;
> -
> -	if (!freq_table) {
> -		dev_err(mpu_dev, "%s: cpu%d: no freq table!\n", __func__,
> -				policy->cpu);
> -		return -EINVAL;
> -	}
> -
> -	ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> -			relation, &i);
> -	if (ret) {
> -		dev_dbg(mpu_dev, "%s: cpu%d: no freq match for %d(ret=%d)\n",
> -			__func__, policy->cpu, target_freq, ret);
> -		return ret;
> -	}
> -	freqs.new = freq_table[i].frequency;
> -	if (!freqs.new) {
> -		dev_err(mpu_dev, "%s: cpu%d: no match for freq %d\n", __func__,
> -			policy->cpu, target_freq);
> -		return -EINVAL;
> -	}
> -
> -	freqs.old = omap_getspeed(policy->cpu);
> -	freqs.cpu = policy->cpu;
> -
> -	if (freqs.old == freqs.new && policy->cur == freqs.new)
> -		return ret;
> -
> -	/* notifiers */
> -	for_each_cpu(i, policy->cpus) {
> -		freqs.cpu = i;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> -	}
> -
> -	freq = freqs.new * 1000;
> -
> -	if (mpu_reg) {
> -		opp = opp_find_freq_ceil(mpu_dev, &freq);
> -		if (IS_ERR(opp)) {
> -			dev_err(mpu_dev, "%s: unable to find MPU OPP for %d\n",
> -				__func__, freqs.new);
> -			return -EINVAL;
> -		}
> -		volt = opp_get_voltage(opp);
> -		tol = volt * OPP_TOLERANCE / 100;
> -		volt_old = regulator_get_voltage(mpu_reg);
> -	}
> -
> -	dev_dbg(mpu_dev, "cpufreq-omap: %u MHz, %ld mV --> %u MHz, %ld mV\n", 
> -		freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
> -		freqs.new / 1000, volt ? volt / 1000 : -1);
> -
> -	/* scaling up?  scale voltage before frequency */
> -	if (mpu_reg && (freqs.new > freqs.old)) {
> -		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
> -		if (r < 0) {
> -			dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
> -				 __func__);
> -			freqs.new = freqs.old;
> -			goto done;
> -		}
> -	}
> -
> -	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> -
> -	/* scaling down?  scale voltage after frequency */
> -	if (mpu_reg && (freqs.new < freqs.old)) {
> -		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
> -		if (r < 0) {
> -			dev_warn(mpu_dev, "%s: unable to scale voltage down.\n",
> -				 __func__);
> -			ret = clk_set_rate(mpu_clk, freqs.old * 1000);
> -			freqs.new = freqs.old;
> -			goto done;
> -		}
> -	}
> -
> -	freqs.new = omap_getspeed(policy->cpu);
> -
> -done:
> -	/* notifiers */
> -	for_each_cpu(i, policy->cpus) {
> -		freqs.cpu = i;
> -		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> -	}
> -
> -	return ret;
> +	struct cpufreq_target_data data;
> +
> +	data.dev = mpu_dev;
> +	data.clk = mpu_clk;
> +	data.reg = mpu_reg;
> +	data.tol = OPP_TOLERANCE;
> +	data.freq_table = freq_table;
> +	data.policy = policy;
> +	data.target_freq = target_freq;
> +	data.relation = relation;
> +
> +	return cpufreq_set_target(&data);
>  }
>  
>  static inline void freq_table_free(void)
> 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-08-10  5:37   ` Shawn Guo
@ 2012-09-06 22:35       ` Stephen Warren
  -1 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-06 22:35 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Kevin Hilman, Nishanth Menon, Mike Turquette, Richard Zhao,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Rafael J. Wysocki,
	Shilimkar Santosh, Rob Herring,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux,
	Richard Zhao, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 08/09/2012 11:37 PM, Shawn Guo wrote:
> From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Acked-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

I believe this patch is causing issues initializing PCI-Express on Tegra.

In next-20120906, I cold-booted 10 times. 3 times, PCIe initialized OK,
and 7 times, the driver timed out in arch/arm/mach-tegra/pcie.c function
tegra_pcie_check_link(). With this patch reverted, another 10 cold boot
attempts all succeeded just fine. Similarly, the regression appeared in
next-20120905, and I isolated it to arch/arm/kernel/, and this is the
only patch in that directory between next-20120904 and next-20120905.

Do you have any idea what the problem might be?

Looking at the timestamps in dmesg in the failing case, the driver is
waiting the expected (per pcie.c driver code) 1.2 seconds before giving
up on the port, although I suppose if the kernel's idea of real-time is
off, then the dmesg log timestamps might be off too.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-06 22:35       ` Stephen Warren
  0 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-06 22:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/09/2012 11:37 PM, Shawn Guo wrote:
> From: Richard Zhao <richard.zhao@linaro.org>
> 
> 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>
> Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I believe this patch is causing issues initializing PCI-Express on Tegra.

In next-20120906, I cold-booted 10 times. 3 times, PCIe initialized OK,
and 7 times, the driver timed out in arch/arm/mach-tegra/pcie.c function
tegra_pcie_check_link(). With this patch reverted, another 10 cold boot
attempts all succeeded just fine. Similarly, the regression appeared in
next-20120905, and I isolated it to arch/arm/kernel/, and this is the
only patch in that directory between next-20120904 and next-20120905.

Do you have any idea what the problem might be?

Looking at the timestamps in dmesg in the failing case, the driver is
waiting the expected (per pcie.c driver code) 1.2 seconds before giving
up on the port, although I suppose if the kernel's idea of real-time is
off, then the dmesg log timestamps might be off too.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-09-06 22:35       ` Stephen Warren
@ 2012-09-07  2:58           ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-07  2:58 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Rafael J. Wysocki, Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, Richard Zhao,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On Thu, Sep 06, 2012 at 04:35:25PM -0600, Stephen Warren wrote:
> I believe this patch is causing issues initializing PCI-Express on Tegra.
> 
> In next-20120906, I cold-booted 10 times. 3 times, PCIe initialized OK,
> and 7 times, the driver timed out in arch/arm/mach-tegra/pcie.c function
> tegra_pcie_check_link(). With this patch reverted, another 10 cold boot
> attempts all succeeded just fine. Similarly, the regression appeared in
> next-20120905, and I isolated it to arch/arm/kernel/, and this is the
> only patch in that directory between next-20120904 and next-20120905.
> 
> Do you have any idea what the problem might be?
> 
> Looking at the timestamps in dmesg in the failing case, the driver is
> waiting the expected (per pcie.c driver code) 1.2 seconds before giving
> up on the port, although I suppose if the kernel's idea of real-time is
> off, then the dmesg log timestamps might be off too.

Just for identifying the problem, can you test the following change to
see if it fixes the failure.

Regards,
Shawn

diff --git a/arch/arm/mach-tegra/cpu-tegra.c b/arch/arm/mach-tegra/cpu-tegra.c
index ceb52db..a3bbdc9 100644
--- a/arch/arm/mach-tegra/cpu-tegra.c
+++ b/arch/arm/mach-tegra/cpu-tegra.c
@@ -247,5 +247,5 @@ static void __exit tegra_cpufreq_exit(void)
 MODULE_AUTHOR("Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>");
 MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
 MODULE_LICENSE("GPL");
-module_init(tegra_cpufreq_init);
+late_init(tegra_cpufreq_init);
 module_exit(tegra_cpufreq_exit);

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-07  2:58           ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-07  2:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 06, 2012 at 04:35:25PM -0600, Stephen Warren wrote:
> I believe this patch is causing issues initializing PCI-Express on Tegra.
> 
> In next-20120906, I cold-booted 10 times. 3 times, PCIe initialized OK,
> and 7 times, the driver timed out in arch/arm/mach-tegra/pcie.c function
> tegra_pcie_check_link(). With this patch reverted, another 10 cold boot
> attempts all succeeded just fine. Similarly, the regression appeared in
> next-20120905, and I isolated it to arch/arm/kernel/, and this is the
> only patch in that directory between next-20120904 and next-20120905.
> 
> Do you have any idea what the problem might be?
> 
> Looking at the timestamps in dmesg in the failing case, the driver is
> waiting the expected (per pcie.c driver code) 1.2 seconds before giving
> up on the port, although I suppose if the kernel's idea of real-time is
> off, then the dmesg log timestamps might be off too.

Just for identifying the problem, can you test the following change to
see if it fixes the failure.

Regards,
Shawn

diff --git a/arch/arm/mach-tegra/cpu-tegra.c b/arch/arm/mach-tegra/cpu-tegra.c
index ceb52db..a3bbdc9 100644
--- a/arch/arm/mach-tegra/cpu-tegra.c
+++ b/arch/arm/mach-tegra/cpu-tegra.c
@@ -247,5 +247,5 @@ static void __exit tegra_cpufreq_exit(void)
 MODULE_AUTHOR("Colin Cross <ccross@android.com>");
 MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
 MODULE_LICENSE("GPL");
-module_init(tegra_cpufreq_init);
+late_init(tegra_cpufreq_init);
 module_exit(tegra_cpufreq_exit);

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-09-07  2:58           ` Shawn Guo
@ 2012-09-07 16:55               ` Stephen Warren
  -1 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-07 16:55 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rafael J. Wysocki, Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, Richard Zhao,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 09/06/2012 08:58 PM, Shawn Guo wrote:
> On Thu, Sep 06, 2012 at 04:35:25PM -0600, Stephen Warren wrote:
>> I believe this patch is causing issues initializing PCI-Express on Tegra.
>>
>> In next-20120906, I cold-booted 10 times. 3 times, PCIe initialized OK,
>> and 7 times, the driver timed out in arch/arm/mach-tegra/pcie.c function
>> tegra_pcie_check_link(). With this patch reverted, another 10 cold boot
>> attempts all succeeded just fine. Similarly, the regression appeared in
>> next-20120905, and I isolated it to arch/arm/kernel/, and this is the
>> only patch in that directory between next-20120904 and next-20120905.
>>
>> Do you have any idea what the problem might be?
>>
>> Looking at the timestamps in dmesg in the failing case, the driver is
>> waiting the expected (per pcie.c driver code) 1.2 seconds before giving
>> up on the port, although I suppose if the kernel's idea of real-time is
>> off, then the dmesg log timestamps might be off too.
> 
> Just for identifying the problem, can you test the following change to
> see if it fixes the failure.

Yes, that does solve the problem (well, with s/late_init/late_initcall/).

As you imply, that change wouldn't help if cpu-tegra.c was built as a
module. So, is this something you can work around in your patch?

Thanks for the quick response.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-07 16:55               ` Stephen Warren
  0 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-07 16:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/06/2012 08:58 PM, Shawn Guo wrote:
> On Thu, Sep 06, 2012 at 04:35:25PM -0600, Stephen Warren wrote:
>> I believe this patch is causing issues initializing PCI-Express on Tegra.
>>
>> In next-20120906, I cold-booted 10 times. 3 times, PCIe initialized OK,
>> and 7 times, the driver timed out in arch/arm/mach-tegra/pcie.c function
>> tegra_pcie_check_link(). With this patch reverted, another 10 cold boot
>> attempts all succeeded just fine. Similarly, the regression appeared in
>> next-20120905, and I isolated it to arch/arm/kernel/, and this is the
>> only patch in that directory between next-20120904 and next-20120905.
>>
>> Do you have any idea what the problem might be?
>>
>> Looking at the timestamps in dmesg in the failing case, the driver is
>> waiting the expected (per pcie.c driver code) 1.2 seconds before giving
>> up on the port, although I suppose if the kernel's idea of real-time is
>> off, then the dmesg log timestamps might be off too.
> 
> Just for identifying the problem, can you test the following change to
> see if it fixes the failure.

Yes, that does solve the problem (well, with s/late_init/late_initcall/).

As you imply, that change wouldn't help if cpu-tegra.c was built as a
module. So, is this something you can work around in your patch?

Thanks for the quick response.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-09-07 16:55               ` Stephen Warren
@ 2012-09-10  1:17                 ` Shawn Guo
  -1 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-10  1:17 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Rafael J. Wysocki, Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm, devicetree-discuss,
	Mark Brown, cpufreq, Shilimkar Santosh, Rob Herring,
	Richard Zhao, Richard Zhao, linux-arm-kernel, linux-tegra

On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
> Yes, that does solve the problem (well, with s/late_init/late_initcall/).
> 
> As you imply, that change wouldn't help if cpu-tegra.c was built as a
> module.

I doubt that.  Have you confirmed it with testing?  When you install
module cpu-tegra, the pcie initialization has been done, right?

> So, is this something you can work around in your patch?
> 
Before we head to a solution, we need to identify the cause.

One thing I note is there is no loops_per_jiffy updating in cpu-tegra
driver.  But if I understand correctly, the driver is running on
MP system.  It should update loops_per_jiffy when cpu frequency
changes, right?  I'm worrying about that the timeout in tegra pcie
driver just happens to work with a wrong loops_per_jiffy, but becomes
broken when loops_per_jiffy gets correct.

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-10  1:17                 ` Shawn Guo
  0 siblings, 0 replies; 54+ messages in thread
From: Shawn Guo @ 2012-09-10  1:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
> Yes, that does solve the problem (well, with s/late_init/late_initcall/).
> 
> As you imply, that change wouldn't help if cpu-tegra.c was built as a
> module.

I doubt that.  Have you confirmed it with testing?  When you install
module cpu-tegra, the pcie initialization has been done, right?

> So, is this something you can work around in your patch?
> 
Before we head to a solution, we need to identify the cause.

One thing I note is there is no loops_per_jiffy updating in cpu-tegra
driver.  But if I understand correctly, the driver is running on
MP system.  It should update loops_per_jiffy when cpu frequency
changes, right?  I'm worrying about that the timeout in tegra pcie
driver just happens to work with a wrong loops_per_jiffy, but becomes
broken when loops_per_jiffy gets correct.

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-09-10  1:17                 ` Shawn Guo
@ 2012-09-10 17:17                     ` Stephen Warren
  -1 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-10 17:17 UTC (permalink / raw)
  To: Shawn Guo, Prashant Gaikwad
  Cc: Rafael J. Wysocki, Kevin Hilman, Nishanth Menon, Mike Turquette,
	Russell King - ARM Linux, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, Richard Zhao,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 09/09/2012 07:17 PM, Shawn Guo wrote:
> On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
>> Yes, that does solve the problem (well, with s/late_init/late_initcall/).
>>
>> As you imply, that change wouldn't help if cpu-tegra.c was built as a
>> module.
> 
> I doubt that.  Have you confirmed it with testing?  When you install
> module cpu-tegra, the pcie initialization has been done, right?

Never mind, the code can't be built as a module anyway.

Aside from that, I misinterpreted your test patch, and thought that it
was moving the Tegra cpufreq driver initialization earlier, but it's
actually moving it later, and in fact by chance after PCIe
initialization, which explains why it solves the issue.

I think the root of the problem is that cpufreq is lowering the CPU
frequency, yet the patch which converted Tegra to the common clock
framework removed the code that actually changes the CPU clock rate. So,
cpufreq thinks the CPU is running at e.g. 216MHz, but it's actually
still running at 1.0GHz, and hence re-calculating the delay loops breaks
things, since delays aren't as long as the system thinks they are. The
variability is due to whether lowering the CPU frequency just happens to
occur before or after the PCIe controller is initialized.

Prashant, are you able to fix the clock driver deficiency within the
next 2-3 days or so (so I can include the fix in the pull requests I
send for 3.7, which need to be sent before the end of the week)? Or, do
we need to disable cpufreq for Tegra because of this?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-10 17:17                     ` Stephen Warren
  0 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-10 17:17 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/09/2012 07:17 PM, Shawn Guo wrote:
> On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
>> Yes, that does solve the problem (well, with s/late_init/late_initcall/).
>>
>> As you imply, that change wouldn't help if cpu-tegra.c was built as a
>> module.
> 
> I doubt that.  Have you confirmed it with testing?  When you install
> module cpu-tegra, the pcie initialization has been done, right?

Never mind, the code can't be built as a module anyway.

Aside from that, I misinterpreted your test patch, and thought that it
was moving the Tegra cpufreq driver initialization earlier, but it's
actually moving it later, and in fact by chance after PCIe
initialization, which explains why it solves the issue.

I think the root of the problem is that cpufreq is lowering the CPU
frequency, yet the patch which converted Tegra to the common clock
framework removed the code that actually changes the CPU clock rate. So,
cpufreq thinks the CPU is running at e.g. 216MHz, but it's actually
still running at 1.0GHz, and hence re-calculating the delay loops breaks
things, since delays aren't as long as the system thinks they are. The
variability is due to whether lowering the CPU frequency just happens to
occur before or after the PCIe controller is initialized.

Prashant, are you able to fix the clock driver deficiency within the
next 2-3 days or so (so I can include the fix in the pull requests I
send for 3.7, which need to be sent before the end of the week)? Or, do
we need to disable cpufreq for Tegra because of this?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-09-10 17:17                     ` Stephen Warren
@ 2012-09-11 11:59                         ` Prashant Gaikwad
  -1 siblings, 0 replies; 54+ messages in thread
From: Prashant Gaikwad @ 2012-09-11 11:59 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Shawn Guo, Rafael J. Wysocki, Kevin Hilman, Nishanth Menon,
	Mike Turquette, Russell King - ARM Linux,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, Richard Zhao,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On Monday 10 September 2012 10:47 PM, Stephen Warren wrote:
> On 09/09/2012 07:17 PM, Shawn Guo wrote:
>> On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
>>> Yes, that does solve the problem (well, with s/late_init/late_initcall/).
>>>
>>> As you imply, that change wouldn't help if cpu-tegra.c was built as a
>>> module.
>> I doubt that.  Have you confirmed it with testing?  When you install
>> module cpu-tegra, the pcie initialization has been done, right?
> Never mind, the code can't be built as a module anyway.
>
> Aside from that, I misinterpreted your test patch, and thought that it
> was moving the Tegra cpufreq driver initialization earlier, but it's
> actually moving it later, and in fact by chance after PCIe
> initialization, which explains why it solves the issue.
>
> I think the root of the problem is that cpufreq is lowering the CPU
> frequency, yet the patch which converted Tegra to the common clock
> framework removed the code that actually changes the CPU clock rate. So,
> cpufreq thinks the CPU is running at e.g. 216MHz, but it's actually
> still running at 1.0GHz, and hence re-calculating the delay loops breaks
> things, since delays aren't as long as the system thinks they are. The
> variability is due to whether lowering the CPU frequency just happens to
> occur before or after the PCIe controller is initialized.
>
> Prashant, are you able to fix the clock driver deficiency within the
> next 2-3 days or so (so I can include the fix in the pull requests I
> send for 3.7, which need to be sent before the end of the week)? Or, do
> we need to disable cpufreq for Tegra because of this?

Your fix looks good to me except the concern I have mentioned in reply to that patch.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-11 11:59                         ` Prashant Gaikwad
  0 siblings, 0 replies; 54+ messages in thread
From: Prashant Gaikwad @ 2012-09-11 11:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 10 September 2012 10:47 PM, Stephen Warren wrote:
> On 09/09/2012 07:17 PM, Shawn Guo wrote:
>> On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
>>> Yes, that does solve the problem (well, with s/late_init/late_initcall/).
>>>
>>> As you imply, that change wouldn't help if cpu-tegra.c was built as a
>>> module.
>> I doubt that.  Have you confirmed it with testing?  When you install
>> module cpu-tegra, the pcie initialization has been done, right?
> Never mind, the code can't be built as a module anyway.
>
> Aside from that, I misinterpreted your test patch, and thought that it
> was moving the Tegra cpufreq driver initialization earlier, but it's
> actually moving it later, and in fact by chance after PCIe
> initialization, which explains why it solves the issue.
>
> I think the root of the problem is that cpufreq is lowering the CPU
> frequency, yet the patch which converted Tegra to the common clock
> framework removed the code that actually changes the CPU clock rate. So,
> cpufreq thinks the CPU is running at e.g. 216MHz, but it's actually
> still running at 1.0GHz, and hence re-calculating the delay loops breaks
> things, since delays aren't as long as the system thinks they are. The
> variability is due to whether lowering the CPU frequency just happens to
> occur before or after the PCIe controller is initialized.
>
> Prashant, are you able to fix the clock driver deficiency within the
> next 2-3 days or so (so I can include the fix in the pull requests I
> send for 3.7, which need to be sent before the end of the week)? Or, do
> we need to disable cpufreq for Tegra because of this?

Your fix looks good to me except the concern I have mentioned in reply to that patch.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
  2012-09-11 11:59                         ` Prashant Gaikwad
@ 2012-09-11 15:16                             ` Stephen Warren
  -1 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-11 15:16 UTC (permalink / raw)
  To: Prashant Gaikwad
  Cc: Shawn Guo, Rafael J. Wysocki, Kevin Hilman, Nishanth Menon,
	Mike Turquette, Russell King - ARM Linux,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Brown,
	cpufreq-u79uwXL29TY76Z2rM5mHXA, Shilimkar Santosh, Rob Herring,
	Richard Zhao, Richard Zhao,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 09/11/2012 05:59 AM, Prashant Gaikwad wrote:
> On Monday 10 September 2012 10:47 PM, Stephen Warren wrote:
>> On 09/09/2012 07:17 PM, Shawn Guo wrote:
>>> On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
>>>> Yes, that does solve the problem (well, with
>>>> s/late_init/late_initcall/).
>>>>
>>>> As you imply, that change wouldn't help if cpu-tegra.c was built as a
>>>> module.
>>> I doubt that.  Have you confirmed it with testing?  When you install
>>> module cpu-tegra, the pcie initialization has been done, right?
>> Never mind, the code can't be built as a module anyway.
>>
>> Aside from that, I misinterpreted your test patch, and thought that it
>> was moving the Tegra cpufreq driver initialization earlier, but it's
>> actually moving it later, and in fact by chance after PCIe
>> initialization, which explains why it solves the issue.
>>
>> I think the root of the problem is that cpufreq is lowering the CPU
>> frequency, yet the patch which converted Tegra to the common clock
>> framework removed the code that actually changes the CPU clock rate. So,
>> cpufreq thinks the CPU is running at e.g. 216MHz, but it's actually
>> still running at 1.0GHz, and hence re-calculating the delay loops breaks
>> things, since delays aren't as long as the system thinks they are. The
>> variability is due to whether lowering the CPU frequency just happens to
>> occur before or after the PCIe controller is initialized.
>>
>> Prashant, are you able to fix the clock driver deficiency within the
>> next 2-3 days or so (so I can include the fix in the pull requests I
>> send for 3.7, which need to be sent before the end of the week)? Or, do
>> we need to disable cpufreq for Tegra because of this?
> 
> Your fix looks good to me except the concern I have mentioned in reply
> to that patch.

Well, the cpufreq driver will need explicit knowledge of
Tega20-vs-Tegra30 anyway, due to e.g. differing CPU clock names,
probable different latencies, etc.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp
@ 2012-09-11 15:16                             ` Stephen Warren
  0 siblings, 0 replies; 54+ messages in thread
From: Stephen Warren @ 2012-09-11 15:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/11/2012 05:59 AM, Prashant Gaikwad wrote:
> On Monday 10 September 2012 10:47 PM, Stephen Warren wrote:
>> On 09/09/2012 07:17 PM, Shawn Guo wrote:
>>> On Fri, Sep 07, 2012 at 10:55:00AM -0600, Stephen Warren wrote:
>>>> Yes, that does solve the problem (well, with
>>>> s/late_init/late_initcall/).
>>>>
>>>> As you imply, that change wouldn't help if cpu-tegra.c was built as a
>>>> module.
>>> I doubt that.  Have you confirmed it with testing?  When you install
>>> module cpu-tegra, the pcie initialization has been done, right?
>> Never mind, the code can't be built as a module anyway.
>>
>> Aside from that, I misinterpreted your test patch, and thought that it
>> was moving the Tegra cpufreq driver initialization earlier, but it's
>> actually moving it later, and in fact by chance after PCIe
>> initialization, which explains why it solves the issue.
>>
>> I think the root of the problem is that cpufreq is lowering the CPU
>> frequency, yet the patch which converted Tegra to the common clock
>> framework removed the code that actually changes the CPU clock rate. So,
>> cpufreq thinks the CPU is running at e.g. 216MHz, but it's actually
>> still running at 1.0GHz, and hence re-calculating the delay loops breaks
>> things, since delays aren't as long as the system thinks they are. The
>> variability is due to whether lowering the CPU frequency just happens to
>> occur before or after the PCIe controller is initialized.
>>
>> Prashant, are you able to fix the clock driver deficiency within the
>> next 2-3 days or so (so I can include the fix in the pull requests I
>> send for 3.7, which need to be sent before the end of the week)? Or, do
>> we need to disable cpufreq for Tegra because of this?
> 
> Your fix looks good to me except the concern I have mentioned in reply
> to that patch.

Well, the cpufreq driver will need explicit knowledge of
Tega20-vs-Tegra30 anyway, due to e.g. differing CPU clock names,
probable different latencies, etc.

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2012-09-11 15:16 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-10  5:37 [PATCH v3 0/3] Add a generic cpufreq-cpu0 driver Shawn Guo
2012-08-10  5:37 ` Shawn Guo
2012-08-10  5:37 ` [PATCH v3 1/3] ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp Shawn Guo
2012-08-10  5:37   ` Shawn Guo
2012-09-04 23:27   ` Rafael J. Wysocki
2012-09-04 23:27     ` Rafael J. Wysocki
     [not found]   ` <1344577046-14847-2-git-send-email-shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2012-09-06 22:35     ` Stephen Warren
2012-09-06 22:35       ` Stephen Warren
     [not found]       ` <5049252D.9000802-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-09-07  2:58         ` Shawn Guo
2012-09-07  2:58           ` Shawn Guo
     [not found]           ` <20120907025847.GI26709-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2012-09-07 16:55             ` Stephen Warren
2012-09-07 16:55               ` Stephen Warren
2012-09-10  1:17               ` Shawn Guo
2012-09-10  1:17                 ` Shawn Guo
     [not found]                 ` <20120910011741.GP26709-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2012-09-10 17:17                   ` Stephen Warren
2012-09-10 17:17                     ` Stephen Warren
     [not found]                     ` <504E20BE.8050107-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-09-11 11:59                       ` Prashant Gaikwad
2012-09-11 11:59                         ` Prashant Gaikwad
     [not found]                         ` <504F2789.1070006-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-09-11 15:16                           ` Stephen Warren
2012-09-11 15:16                             ` Stephen Warren
2012-08-10  5:37 ` [PATCH v3 2/3] PM / OPP: Initialize OPP table from device tree Shawn Guo
2012-08-10  5:37   ` Shawn Guo
2012-08-16 21:05   ` Rafael J. Wysocki
2012-08-16 21:05     ` Rafael J. Wysocki
     [not found]     ` <201208162305.55114.rjw-KKrjLPT3xs0@public.gmane.org>
2012-08-17  0:52       ` Shawn Guo
2012-08-17  0:52         ` Shawn Guo
2012-08-17  1:53         ` Rob Herring
2012-08-17  1:53           ` Rob Herring
2012-09-04 23:27           ` Rafael J. Wysocki
2012-09-04 23:27             ` Rafael J. Wysocki
2012-08-10  5:37 ` [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver Shawn Guo
2012-08-10  5:37   ` Shawn Guo
2012-09-04 23:18   ` Rafael J. Wysocki
2012-09-04 23:18     ` Rafael J. Wysocki
     [not found]     ` <201209050118.39045.rjw-KKrjLPT3xs0@public.gmane.org>
2012-09-05  1:12       ` Shawn Guo
2012-09-05  1:12         ` Shawn Guo
2012-09-05  4:53         ` AnilKumar, Chimata
2012-09-05  4:53           ` AnilKumar, Chimata
2012-09-05  5:02         ` Shilimkar, Santosh
2012-09-05  5:02           ` Shilimkar, Santosh
2012-09-05 13:38         ` Rafael J. Wysocki
2012-09-05 13:38           ` Rafael J. Wysocki
2012-09-05 13:59           ` Shawn Guo
2012-09-05 13:59             ` Shawn Guo
2012-09-05 20:18             ` Rafael J. Wysocki
2012-09-05 20:18               ` Rafael J. Wysocki
2012-09-06  7:29               ` Shawn Guo
2012-09-06  7:29                 ` Shawn Guo
     [not found]                 ` <20120906072935.GB2362-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2012-09-06 19:59                   ` Rafael J. Wysocki
2012-09-06 19:59                     ` Rafael J. Wysocki
2012-08-31  6:42 ` [PATCH v3 0/3] " Shawn Guo
2012-08-31  6:42   ` Shawn Guo
2012-09-01  5:53   ` Rafael J. Wysocki
2012-09-01  5:53     ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.