[PATCH 0/6 v6] cpufreq: add support for Calxeda ECX-1000 (highbank)

STALE4934d

Revision v6 of 12 in this series.

18 messages, 3 authors, 2012-11-29 · open the first message on its own page

[PATCH 0/6 v6] cpufreq: add support for Calxeda ECX-1000 (highbank)

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:04:37

This patch series adds cpufreq support for the Calxeda
ECX-1000 (highbank) SoCs. The driver is based on the 
cpufreq-cpu0 driver. Because of the unique way that 
highbank uses the EnergyCore Management Engine to manage
voltages, it was not possible to use the cpufreq-cpu0 driver.

--Mark Langsdorf

[PATCH 1/6 v6] arm: use devicetree to get smp_twd clock

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:04:38

From: Rob Herring <redacted>

Signed-off-by: Rob Herring <redacted>
Signed-off-by: Mark Langsdorf <redacted>
---
Changes from v4, v5
        None.
Changes from v3
        No longer setting *clk to NULL in twd_get_clock().
Changes from v2
        Turned the check for the node pointer into an if-then-else statement.
        Removed the second, redundant clk_get_rate.
Changes from v1
        None.

 arch/arm/kernel/smp_twd.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index b22d700..af46b80 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -237,12 +237,15 @@ static irqreturn_t twd_handler(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static struct clk *twd_get_clock(void)
+static struct clk *twd_get_clock(struct device_node *np)
 {
 	struct clk *clk;
 	int err;
 
-	clk = clk_get_sys("smp_twd", NULL);
+	if (np)
+		clk = of_clk_get(np, 0);
+	else
+		clk = clk_get_sys("smp_twd", NULL);
 	if (IS_ERR(clk)) {
 		pr_err("smp_twd: clock not found: %d\n", (int)PTR_ERR(clk));
 		return clk;
@@ -263,6 +266,7 @@ static struct clk *twd_get_clock(void)
 		return ERR_PTR(err);
 	}
 
+	twd_timer_rate = clk_get_rate(clk);
 	return clk;
 }
 
@@ -273,12 +277,7 @@ static int __cpuinit twd_timer_setup(struct clock_event_device *clk)
 {
 	struct clock_event_device **this_cpu_clk;
 
-	if (!twd_clk)
-		twd_clk = twd_get_clock();
-
-	if (!IS_ERR_OR_NULL(twd_clk))
-		twd_timer_rate = clk_get_rate(twd_clk);
-	else
+	if (IS_ERR_OR_NULL(twd_clk))
 		twd_calibrate_rate();
 
 	__raw_writel(0, twd_base + TWD_TIMER_CONTROL);
@@ -349,6 +348,8 @@ int __init twd_local_timer_register(struct twd_local_timer *tlt)
 	if (!twd_base)
 		return -ENOMEM;
 
+	twd_clk = twd_get_clock(NULL);
+
 	return twd_local_timer_common_register();
 }
 
@@ -383,6 +384,8 @@ void __init twd_local_timer_of_register(void)
 		goto out;
 	}
 
+	twd_clk = twd_get_clock(np);
+
 	err = twd_local_timer_common_register();
 
 out:
-- 
1.7.11.7

[PATCH 2/6 v6] clk, highbank: Prevent glitches in non-bypass reset mode

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:04:39

The highbank clock will glitch with the current code if the
clock rate is reset without relocking the PLL. Program the PLL
correctly to prevent glitches.

Signed-off-by: Mark Langsdorf <redacted>
Signed-off-by: Rob Herring <redacted>
Acked-by: Mike Turquette <redacted>
---
Changes from v5
	Added Mike Turquette's ack.
Changes from v4
        None.
Changes from v3
        Changelog text and patch name now correspond to the actual patch.
        was clk, highbank: remove non-bypass reset mode.
Changes from v2
        None.
Changes from v1:
        Removed erroneous reformating.

 drivers/clk/clk-highbank.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
index 52fecad..3a0b723 100644
--- a/drivers/clk/clk-highbank.c
+++ b/drivers/clk/clk-highbank.c
@@ -182,8 +182,10 @@ static int clk_pll_set_rate(struct clk_hw *hwclk, unsigned long rate,
 		reg |= HB_PLL_EXT_ENA;
 		reg &= ~HB_PLL_EXT_BYPASS;
 	} else {
+		writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
 		reg &= ~HB_PLL_DIVQ_MASK;
 		reg |= divq << HB_PLL_DIVQ_SHIFT;
+		writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
 	}
 	writel(reg, hbclk->reg);
 
-- 
1.7.11.7

[PATCH 3/6 v6] cpufreq: tolerate inexact values when collecting stats

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:04:40

This patch is withdrawn due to a need for severe rework.

Changes from v4
        Withdrawn.      
Changes from v3, v2
        None.
Changes from v1
        Implemented a simple round-up algorithm instead of the over/under
method that could cause errors on Intel processors with boost mode.

[PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:05:29

Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.

Signed-off-by: Mark Langsdorf <redacted>
Cc: devicetree-discuss at lists.ozlabs.org
---
Changes from v5
	Changed ipc_transmit() to pl320_ipc_transmit().
Changes from v4
        Removed erroneous changes to arch/arm/Kconfig.
        Removed unnecessary changes to drivers/cpufreq/Kconfig.arm
        Alphabetized additions to arch/arm/mach-highbank/Kconfig
        Changed ipc call and header to match new ipc location in 
        drivers/mailbox.
Changes from v3
        None.
Changes from v2
        Changed transition latency binding in code to match documentation.
Changes from v1
        Added highbank specific Kconfig changes.

 .../bindings/cpufreq/highbank-cpufreq.txt          |  53 +++++
 arch/arm/boot/dts/highbank.dts                     |  10 +
 arch/arm/mach-highbank/Kconfig                     |   2 +
 drivers/cpufreq/Kconfig.arm                        |  13 ++
 drivers/cpufreq/Makefile                           |   1 +
 drivers/cpufreq/highbank-cpufreq.c                 | 229 +++++++++++++++++++++
 6 files changed, 308 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
 create mode 100644 drivers/cpufreq/highbank-cpufreq.c
diff --git a/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
new file mode 100644
index 0000000..1d5a836
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
@@ -0,0 +1,53 @@
+Highbank cpufreq driver
+
+This is cpufreq driver for Calxeda ECX-1000 (highbank) processor. It is based
+on the generic cpu0 driver and uses a similar format for bindings. Since
+the EnergyCore Management Engine maintains the voltage based on the
+frequency, the voltage component of the operating points can be set to any
+arbitrary values.
+
+Both required 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
+- transition-latency: Specify the possible maximum transition latency for clock,
+  in unit of nanoseconds.
+
+Examples:
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu at 0 {
+		compatible = "arm,cortex-a9";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz  ignored */
+			790000  1000000
+			396000  1000000
+			198000  1000000
+		>;
+		transition-latency = <200000>;
+	};
+
+	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/arch/arm/boot/dts/highbank.dts b/arch/arm/boot/dts/highbank.dts
index 0c6fc34..8624c94 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts
@@ -36,6 +36,16 @@
 			next-level-cache = <&L2>;
 			clocks = <&a9pll>;
 			clock-names = "cpu";
+			operating-points = <
+				/* kHz    ignored */
+				 1300000  1000000
+				 1200000  1000000
+				 1100000  1000000
+				  800000  1000000
+				  400000  1000000
+				  200000  1000000
+			>;
+			transition-latency = <100000>;
 		};
 
 		cpu at 1 {
diff --git a/arch/arm/mach-highbank/Kconfig b/arch/arm/mach-highbank/Kconfig
index 2896881..b7862da 100644
--- a/arch/arm/mach-highbank/Kconfig
+++ b/arch/arm/mach-highbank/Kconfig
@@ -1,5 +1,7 @@
 config ARCH_HIGHBANK
 	bool "Calxeda ECX-1000 (Highbank)" if ARCH_MULTI_V7
+	select ARCH_HAS_CPUFREQ
+	select ARCH_HAS_OPP
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_AMBA
 	select ARM_GIC
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 5961e64..7a8bcdc 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -76,3 +76,16 @@ config ARM_EXYNOS5250_CPUFREQ
 	help
 	  This adds the CPUFreq driver for Samsung EXYNOS5250
 	  SoC.
+
+config ARM_HIGHBANK_CPUFREQ
+       tristate "Calxeda Highbank-based"
+       depends on ARCH_HIGHBANK
+       select CPU_FREQ_TABLE
+       select PM_OPP
+       default m
+       help
+         This adds the CPUFreq driver for Calxeda Highbank SoC
+         based boards.
+
+         If in doubt, say N.
+
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 1bc90e1..9e8f12a 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ)	+= exynos4210-cpufreq.o
 obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ)	+= exynos4x12-cpufreq.o
 obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ)	+= exynos5250-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)     += omap-cpufreq.o
+obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ)	+= highbank-cpufreq.o
 
 ##################################################################################
 # PowerPC platform drivers
diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
new file mode 100644
index 0000000..878d3ff
--- /dev/null
+++ b/drivers/cpufreq/highbank-cpufreq.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2012 Calxeda, Inc.
+ *
+ * derived from cpufreq-cpu0 by Freescale Semiconductor
+ *
+ * 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.
+ */
+
+#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/slab.h>
+#include <linux/mailbox.h>
+
+#define HB_CPUFREQ_CHANGE_NOTE 0x80000001
+
+static unsigned int transition_latency;
+
+static struct device *cpu_dev;
+static struct clk *cpu_clk;
+static struct cpufreq_frequency_table *freq_table;
+
+static int hb_verify_speed(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, freq_table);
+}
+
+static unsigned int hb_get_speed(unsigned int cpu)
+{
+	return clk_get_rate(cpu_clk) / 1000;
+}
+
+static int hb_voltage_change(unsigned int freq)
+{
+	int i;
+	u32 msg[7];
+
+	msg[0] = HB_CPUFREQ_CHANGE_NOTE;
+	msg[1] = freq / 1000;
+	for (i = 2; i < 7; i++)
+		msg[i] = 0;
+
+	return pl320_ipc_transmit(msg);
+}
+
+static int hb_set_target(struct cpufreq_policy *policy,
+			   unsigned int target_freq, unsigned int relation)
+{
+	struct cpufreq_freqs freqs;
+	unsigned long freq_Hz;
+	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);
+	}
+
+	pr_debug("%u MHz --> %u MHz\n", freqs.old / 1000, freqs.new / 1000);
+
+	/* scaling up?  scale voltage before frequency */
+	if (freqs.new > freqs.old) {
+		ret = hb_voltage_change(freqs.new);
+		if (ret) {
+			freqs.new = freqs.old;
+			return -EAGAIN;
+		}
+	}
+
+	ret = clk_set_rate(cpu_clk, freqs.new * 1000);
+	if (ret) {
+		pr_err("failed to set clock rate: %d\n", ret);
+		hb_voltage_change(freqs.old);
+		return ret;
+	}
+
+	/* scaling down?  scale voltage after frequency */
+	if (freqs.new < freqs.old) {
+		ret = hb_voltage_change(freqs.new);
+		if (ret) {
+			if (clk_set_rate(cpu_clk, freqs.old * 1000))
+				pr_err("also failed to reset freq\n");
+			freqs.new = freqs.old;
+			return -EAGAIN;
+		}
+	}
+
+	for_each_online_cpu(cpu) {
+		freqs.cpu = cpu;
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	}
+
+	return 0;
+}
+
+static int hb_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;
+
+	policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
+	cpumask_setall(policy->cpus);
+
+	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
+	return 0;
+}
+
+static int hb_cpufreq_exit(struct cpufreq_policy *policy)
+{
+	cpufreq_frequency_table_put_attr(policy->cpu);
+
+	return 0;
+}
+
+static struct freq_attr *hb_cpufreq_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+static struct cpufreq_driver hb_cpufreq_driver = {
+	.flags = CPUFREQ_STICKY,
+	.verify = hb_verify_speed,
+	.target = hb_set_target,
+	.get = hb_get_speed,
+	.init = hb_cpufreq_init,
+	.exit = hb_cpufreq_exit,
+	.name = "highbank-cpufreq",
+	.attr = hb_cpufreq_attr,
+};
+
+static int __devinit hb_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 highbank cpufreq node\n");
+		return -ENOENT;
+	}
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		pr_err("failed to get highbank cpufreq 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;
+	}
+
+	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;
+	}
+
+	if (of_property_read_u32(np, "transition-latency", &transition_latency))
+		transition_latency = CPUFREQ_ETERNAL;
+
+	ret = cpufreq_register_driver(&hb_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(hb_cpufreq_driver_init);
+
+MODULE_AUTHOR("Mark Langsdorf <mark.langsdorf@calxeda.com>");
+MODULE_DESCRIPTION("Calxeda Highbank cpufreq driver");
+MODULE_LICENSE("GPL");
-- 
1.7.11.7

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Shawn Guo <hidden>
Date: 2012-11-28 02:21:51

On Tue, Nov 27, 2012 at 02:04:32PM -0600, Mark Langsdorf wrote:
Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.
...
+static int hb_voltage_change(unsigned int freq)
+{
+	int i;
+	u32 msg[7];
+
+	msg[0] = HB_CPUFREQ_CHANGE_NOTE;
+	msg[1] = freq / 1000;
+	for (i = 2; i < 7; i++)
+		msg[i] = 0;
+
+	return pl320_ipc_transmit(msg);
Is it possible to have this handled inside clk_set_rate() call of cpu
clock?

Shawn
+}

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mark Langsdorf <hidden>
Date: 2012-11-28 13:16:11

On 11/27/2012 08:32 PM, Shawn Guo wrote:
On Tue, Nov 27, 2012 at 02:04:32PM -0600, Mark Langsdorf wrote:
quoted
Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.
...
quoted
+static int hb_voltage_change(unsigned int freq)
+{
+	int i;
+	u32 msg[7];
+
+	msg[0] = HB_CPUFREQ_CHANGE_NOTE;
+	msg[1] = freq / 1000;
+	for (i = 2; i < 7; i++)
+		msg[i] = 0;
+
+	return pl320_ipc_transmit(msg);
Is it possible to have this handled inside clk_set_rate() call of cpu
clock?
Standard practice is to have cpufreq_set_target() handle voltage
transitions and leave clk_set_rate() handle the frequency changes. I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded. I don't think the clk maintainers would
take that patch, either.

So no.

--Mark Langsdorf
Calxeda, Inc.

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Shawn Guo <hidden>
Date: 2012-11-28 14:32:17

On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
Standard practice is to have cpufreq_set_target() handle voltage
transitions and leave clk_set_rate() handle the frequency changes.
The standard practice is to have cpufreq_set_target() handle both
voltage and frequency transitions, while voltage is handled by regulator
and frequency by clk API.
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
I don't think the clk maintainers would
take that patch, either.
This is all handled platform clock specific .set_rate() hook.  I doubt
it will concern clk maintainers at all, especially when doing so we
will avoid another cpufreq driver by just using cpufreq-cpu0 driver.

Shawn

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Shawn Guo <hidden>
Date: 2012-11-28 14:52:58

On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
quoted
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
Need to also check whether frequency or voltage should be changed first
in .set_rate() though.

Shawn

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mark Langsdorf <hidden>
Date: 2012-11-28 15:01:15

On 11/28/2012 09:17 AM, Shawn Guo wrote:
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
quoted
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
quoted
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
Need to also check whether frequency or voltage should be changed first
in .set_rate() though.
Yes, that's entirely what I meant when I said that I would need to move
most of the hb_set_target() logic into .set_rate(). I would also need to
account for retries if the voltage set operation fails, which it
sometimes does.

The ECME handles changing the voltage but doesn't look like a voltage
regulator. Amongst other things, by design it doesn't export meaningful
voltage information to Linux. I tried to get cpufreq-clk0 to work with
the Highbank design and it ended up being much easier and more sane to
create a separate driver.

--Mark Langsdorf
Calxeda, Inc.

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mike Turquette <hidden>
Date: 2012-11-28 16:01:37

Quoting Shawn Guo (2012-11-28 07:17:44)
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
quoted
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
quoted
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
Need to also check whether frequency or voltage should be changed first
in .set_rate() though.

Shawn
The notifiers in the clk framework might be a better place for this than
just simply hacking the logic into the .set_rate callback.

I haven't looked at the definition of hb_voltage_change but does the
call graph make any clk api calls?  Are you talking over i2c to a
regulator?  If so then you'll probably hit the same reentrancy problem I
hit when trying to make a general solution.

Regards,
Mike
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mark Langsdorf <hidden>
Date: 2012-11-28 16:18:34

On 11/28/2012 10:01 AM, Mike Turquette wrote:
Quoting Shawn Guo (2012-11-28 07:17:44)
quoted
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
quoted
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
quoted
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
Need to also check whether frequency or voltage should be changed first
in .set_rate() though.

Shawn
The notifiers in the clk framework might be a better place for this than
just simply hacking the logic into the .set_rate callback.
Unless the clk notifiers are different than the cpufreq notifiers, they
don't handle returning error conditions very well. And given that the
voltage change operation can fail (though it almost always succeeds on a
retry) I need to be able to handle and detect that error condition.
I haven't looked at the definition of hb_voltage_change but does the
call graph make any clk api calls?  Are you talking over i2c to a
regulator?  If so then you'll probably hit the same reentrancy problem I
hit when trying to make a general solution.
I'm talking over a pl320 Interprocessor Communication Mailbox to a
separate core running it's own RTOS. The RTOS might speak i2c to a
regulator but it's a black box to me.

hb_voltage_change() doesn't make any clk api calls. It changes the
voltages, and then hb_set_target() makes clk api calls to change the
frequency.

--Mark Langsdorf
Calxeda, Inc.

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mike Turquette <hidden>
Date: 2012-11-28 21:06:16

Quoting Mark Langsdorf (2012-11-28 08:18:35)
On 11/28/2012 10:01 AM, Mike Turquette wrote:
quoted
Quoting Shawn Guo (2012-11-28 07:17:44)
quoted
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
quoted
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
quoted
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
Need to also check whether frequency or voltage should be changed first
in .set_rate() though.

Shawn
The notifiers in the clk framework might be a better place for this than
just simply hacking the logic into the .set_rate callback.
Unless the clk notifiers are different than the cpufreq notifiers, they
don't handle returning error conditions very well. And given that the
voltage change operation can fail (though it almost always succeeds on a
retry) I need to be able to handle and detect that error condition.
The notifier handler can handle the case where the transition fails (and
needs to be retried).

Also you should check out the clk notifiers.  I think they handle
failure decently.  If a notifer returns an error code then everything
unrolls and the clk_set_rate operation aborts.

Regards,
Mike
quoted
I haven't looked at the definition of hb_voltage_change but does the
call graph make any clk api calls?  Are you talking over i2c to a
regulator?  If so then you'll probably hit the same reentrancy problem I
hit when trying to make a general solution.
I'm talking over a pl320 Interprocessor Communication Mailbox to a
separate core running it's own RTOS. The RTOS might speak i2c to a
regulator but it's a black box to me.

hb_voltage_change() doesn't make any clk api calls. It changes the
voltages, and then hb_set_target() makes clk api calls to change the
frequency.

--Mark Langsdorf
Calxeda, Inc.

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mark Langsdorf <hidden>
Date: 2012-11-29 00:24:12

On 11/28/2012 03:05 PM, Mike Turquette wrote:
Quoting Mark Langsdorf (2012-11-28 08:18:35)
quoted
On 11/28/2012 10:01 AM, Mike Turquette wrote:
quoted
Quoting Shawn Guo (2012-11-28 07:17:44)
quoted
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
quoted
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
quoted
I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded.
You only need to move hb_voltage_change() into cpu clock's .set_rate()
hook with no need of checking if cpufreq is enabled or not.
Need to also check whether frequency or voltage should be changed first
in .set_rate() though.

Shawn
The notifiers in the clk framework might be a better place for this than
just simply hacking the logic into the .set_rate callback.
Unless the clk notifiers are different than the cpufreq notifiers, they
don't handle returning error conditions very well. And given that the
voltage change operation can fail (though it almost always succeeds on a
retry) I need to be able to handle and detect that error condition.
The notifier handler can handle the case where the transition fails (and
needs to be retried).

Also you should check out the clk notifiers.  I think they handle
failure decently.  If a notifer returns an error code then everything
unrolls and the clk_set_rate operation aborts.
Thanks for the pointer. The clk notifier calls seem to be working with
cpufreq-cpu0.

I did enough surgery on the code that I want to run a lot of stress
tests before I resubmit. I'll try to have something for Tuesday.

--Mark Langsdorf
Calxeda, Inc.

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Shawn Guo <hidden>
Date: 2012-11-29 01:26:22

The notifiers in the clk framework might be a better place for this than
just simply hacking the logic into the .set_rate callback.
Ah, right.  How did I forget about that nice piece?
I haven't looked at the definition of hb_voltage_change but does the
call graph make any clk api calls?  Are you talking over i2c to a
regulator?  If so then you'll probably hit the same reentrancy problem I
hit when trying to make a general solution.
So, how is your "reentrancy in the common clk framework" series[1]
going on?  Haven't seen any update since August.

Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/182198

Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

From: Mike Turquette <hidden>
Date: 2012-11-29 04:35:01

Quoting Shawn Guo (2012-11-28 17:51:36)
quoted
The notifiers in the clk framework might be a better place for this than
just simply hacking the logic into the .set_rate callback.
Ah, right.  How did I forget about that nice piece?
quoted
I haven't looked at the definition of hb_voltage_change but does the
call graph make any clk api calls?  Are you talking over i2c to a
regulator?  If so then you'll probably hit the same reentrancy problem I
hit when trying to make a general solution.
So, how is your "reentrancy in the common clk framework" series[1]
going on?  Haven't seen any update since August.
I've begun to look at a dvfs api that builds on top of the clock
framework, as opposed to using clk_set_rate as the dvfs api itself.
This eliminates the need for reentrancy, at least for the dvfs case.

I'll post more when I have it.  Honestly the reentrancy stuff was just
too ugly.  I might try again some day but for now I'm thinking a less
radical approach deserves consideration.

Thanks,
Mike
Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/182198

[PATCH 5/6 v6] power: export opp cpufreq functions

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:06:07

These functions are needed to make the cpufreq-core0 and highbank-cpufreq
drivers loadable as modules.

Signed-off-by: Mark Langsdorf <redacted>
Acked-by: Nishanth Menon <nm@ti.com>
---
Changes from v4, v5
        None.
Changes from v3
        includes linux/export.h instead of module.h.
Changes from v2
        None.
Changes from v1
        Added Nishanth Menon's ack.
        Clarified the purpose of the change in the commit message.

 drivers/base/power/opp.c | 4 ++++
 1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index d946864..4062ec3 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -23,6 +23,7 @@
 #include <linux/rcupdate.h>
 #include <linux/opp.h>
 #include <linux/of.h>
+#include <linux/export.h>
 
 /*
  * Internal data structure organization with the OPP layer library is as
@@ -643,6 +644,7 @@ int opp_init_cpufreq_table(struct device *dev,
 
 	return 0;
 }
+EXPORT_SYMBOL(opp_init_cpufreq_table);
 
 /**
  * opp_free_cpufreq_table() - free the cpufreq table
@@ -660,6 +662,7 @@ void opp_free_cpufreq_table(struct device *dev,
 	kfree(*table);
 	*table = NULL;
 }
+EXPORT_SYMBOL(opp_free_cpufreq_table);
 #endif		/* CONFIG_CPU_FREQ */
 
 /**
@@ -720,4 +723,5 @@ int of_init_opp_table(struct device *dev)
 
 	return 0;
 }
+EXPORT_SYMBOL(of_init_opp_table);
 #endif
-- 
1.7.11.7

[PATCH 4/6 v6] arm highbank: add support for pl320 IPC

From: Mark Langsdorf <hidden>
Date: 2012-11-27 20:06:29

From: Rob Herring <redacted>

The pl320 IPC allows for interprocessor communication between the highbank A9
and the EnergyCore Management Engine. The pl320 implements a straightforward
mailbox protocol.

This patch depends on Omar Ramirez Luna's [off-list ref]
mailbox driver patch series.

Signed-off-by: Mark Langsdorf <redacted>
Signed-off-by: Rob Herring <redacted>
Cc: Omar Ramirez Luna <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
---
Changes from v5
	Renamed ipc_transmit() to pl320_ipc_transmit().
	Properly exported pl320_ipc_{un}register_notifier().
Changes from v4
        Moved pl320-ipc.c from arch/arm/mach-highbank to drivers/mailbox.
        Moved header information to include/linux/mailbox.h.
        Added Kconfig options to reflect the new code location.
        Change drivers/mailbox/Makefile to build the omap mailboxes only 
        when they are configured.
        Removed ipc_call_fast and renamed ipc_call_slow ipc_transmit.
Changes from v3, v2
        None.
Changes from v1
        Removed erroneous changes for cpufreq Kconfig.


 arch/arm/mach-highbank/Kconfig |   2 +
 drivers/mailbox/Kconfig        |   9 ++
 drivers/mailbox/Makefile       |   4 +
 drivers/mailbox/pl320-ipc.c    | 199 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mailbox.h        |  19 +++-
 5 files changed, 232 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mailbox/Makefile
 create mode 100644 drivers/mailbox/pl320-ipc.c
diff --git a/arch/arm/mach-highbank/Kconfig b/arch/arm/mach-highbank/Kconfig
index 0e1d0a4..2896881 100644
--- a/arch/arm/mach-highbank/Kconfig
+++ b/arch/arm/mach-highbank/Kconfig
@@ -11,5 +11,7 @@ config ARCH_HIGHBANK
 	select GENERIC_CLOCKEVENTS
 	select HAVE_ARM_SCU
 	select HAVE_SMP
+	select MAILBOX
+	select PL320_MBOX
 	select SPARSE_IRQ
 	select USE_OF
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index be8cac0..e89fdb4 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -34,4 +34,13 @@ config OMAP_MBOX_KFIFO_SIZE
 	  This can also be changed at runtime (via the mbox_kfifo_size
 	  module parameter).
 
+config PL320_MBOX
+	bool "ARM PL320 Mailbox"
+	help
+	  An implementation of the ARM PL320 Interprocessor Communication
+	  Mailbox (IPCM), tailored for the Calxeda Highbank. It is used to
+	  send short messages between Highbank's A9 cores and the EnergyCore
+	  Management Engine, primarily for cpufreq. Say Y here if you want
+	  to use the PL320 IPCM support.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
new file mode 100644
index 0000000..c9f14c3
--- /dev/null
+++ b/drivers/mailbox/Makefile
@@ -0,0 +1,4 @@
+obj-$(CONFIG_OMAP1_MBOX)	+= mailbox.o mailbox-omap1.o
+obj-$(CONFIG_OMAP2PLUS_MBOX)	+= mailbox.o mailbox-omap2.o
+obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
+
diff --git a/drivers/mailbox/pl320-ipc.c b/drivers/mailbox/pl320-ipc.c
new file mode 100644
index 0000000..1a9d8e4
--- /dev/null
+++ b/drivers/mailbox/pl320-ipc.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright 2012 Calxeda, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/types.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/spinlock.h>
+#include <linux/device.h>
+#include <linux/amba/bus.h>
+
+#include <linux/mailbox.h>
+
+#define IPCMxSOURCE(m)		((m) * 0x40)
+#define IPCMxDSET(m)		(((m) * 0x40) + 0x004)
+#define IPCMxDCLEAR(m)		(((m) * 0x40) + 0x008)
+#define IPCMxDSTATUS(m)		(((m) * 0x40) + 0x00C)
+#define IPCMxMODE(m)		(((m) * 0x40) + 0x010)
+#define IPCMxMSET(m)		(((m) * 0x40) + 0x014)
+#define IPCMxMCLEAR(m)		(((m) * 0x40) + 0x018)
+#define IPCMxMSTATUS(m)		(((m) * 0x40) + 0x01C)
+#define IPCMxSEND(m)		(((m) * 0x40) + 0x020)
+#define IPCMxDR(m, dr)		(((m) * 0x40) + ((dr) * 4) + 0x024)
+
+#define IPCMMIS(irq)		(((irq) * 8) + 0x800)
+#define IPCMRIS(irq)		(((irq) * 8) + 0x804)
+
+#define MBOX_MASK(n)		(1 << (n))
+#define IPC_TX_MBOX		1
+#define IPC_RX_MBOX		2
+
+#define CHAN_MASK(n)		(1 << (n))
+#define A9_SOURCE		1
+#define M3_SOURCE		0
+
+static void __iomem *ipc_base;
+static int ipc_irq;
+static DEFINE_MUTEX(ipc_m1_lock);
+static DECLARE_COMPLETION(ipc_completion);
+static ATOMIC_NOTIFIER_HEAD(ipc_notifier);
+
+static inline void set_destination(int source, int mbox)
+{
+	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxDSET(mbox));
+	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxMSET(mbox));
+}
+
+static inline void clear_destination(int source, int mbox)
+{
+	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxDCLEAR(mbox));
+	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxMCLEAR(mbox));
+}
+
+static void __ipc_send(int mbox, u32 *data)
+{
+	int i;
+	for (i = 0; i < 7; i++)
+		__raw_writel(data[i], ipc_base + IPCMxDR(mbox, i));
+	__raw_writel(0x1, ipc_base + IPCMxSEND(mbox));
+}
+
+static u32 __ipc_rcv(int mbox, u32 *data)
+{
+	int i;
+	for (i = 0; i < 7; i++)
+		data[i] = __raw_readl(ipc_base + IPCMxDR(mbox, i));
+	return data[1];
+}
+
+/* blocking implmentation from the A9 side, not usuable in interrupts! */
+int pl320_ipc_transmit(u32 *data)
+{
+	int ret;
+
+	mutex_lock(&ipc_m1_lock);
+
+	init_completion(&ipc_completion);
+	__ipc_send(IPC_TX_MBOX, data);
+	ret = wait_for_completion_timeout(&ipc_completion,
+					  msecs_to_jiffies(1000));
+	if (ret == 0) {
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	ret = __ipc_rcv(IPC_TX_MBOX, data);
+out:
+	mutex_unlock(&ipc_m1_lock);
+	return ret;
+}
+EXPORT_SYMBOL(pl320_ipc_transmit);
+
+irqreturn_t ipc_handler(int irq, void *dev)
+{
+	u32 irq_stat;
+	u32 data[7];
+
+	irq_stat = __raw_readl(ipc_base + IPCMMIS(1));
+	if (irq_stat & MBOX_MASK(IPC_TX_MBOX)) {
+		__raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
+		complete(&ipc_completion);
+	}
+	if (irq_stat & MBOX_MASK(IPC_RX_MBOX)) {
+		__ipc_rcv(IPC_RX_MBOX, data);
+		atomic_notifier_call_chain(&ipc_notifier, data[0], data + 1);
+		__raw_writel(2, ipc_base + IPCMxSEND(IPC_RX_MBOX));
+	}
+
+	return IRQ_HANDLED;
+}
+
+int pl320_ipc_register_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&ipc_notifier, nb);
+}
+EXPORT_SYMBOL(pl320_ipc_register_notifier);
+
+int pl320_ipc_unregister_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&ipc_notifier, nb);
+}
+EXPORT_SYMBOL(pl320_ipc_unregister_notifier);
+
+static int __devinit pl320_probe(struct amba_device *adev,
+				const struct amba_id *id)
+{
+	int ret;
+
+	ipc_base = ioremap(adev->res.start, resource_size(&adev->res));
+	if (ipc_base == NULL)
+		return -ENOMEM;
+
+	__raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
+
+	ipc_irq = adev->irq[0];
+	ret = request_irq(ipc_irq, ipc_handler, 0, dev_name(&adev->dev), NULL);
+	if (ret < 0)
+		goto err;
+
+	/* Init slow mailbox */
+	__raw_writel(CHAN_MASK(A9_SOURCE),
+			ipc_base + IPCMxSOURCE(IPC_TX_MBOX));
+	__raw_writel(CHAN_MASK(M3_SOURCE),
+			ipc_base + IPCMxDSET(IPC_TX_MBOX));
+	__raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
+		     ipc_base + IPCMxMSET(IPC_TX_MBOX));
+
+	/* Init receive mailbox */
+	__raw_writel(CHAN_MASK(M3_SOURCE),
+			ipc_base + IPCMxSOURCE(IPC_RX_MBOX));
+	__raw_writel(CHAN_MASK(A9_SOURCE),
+			ipc_base + IPCMxDSET(IPC_RX_MBOX));
+	__raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
+		     ipc_base + IPCMxMSET(IPC_RX_MBOX));
+
+	return 0;
+err:
+	iounmap(ipc_base);
+	return ret;
+}
+
+static struct amba_id pl320_ids[] = {
+	{
+		.id	= 0x00041320,
+		.mask	= 0x000fffff,
+	},
+	{ 0, 0 },
+};
+
+static struct amba_driver pl320_driver = {
+	.drv = {
+		.name	= "pl320",
+	},
+	.id_table	= pl320_ids,
+	.probe		= pl320_probe,
+};
+
+static int __init ipc_init(void)
+{
+	return amba_driver_register(&pl320_driver);
+}
+module_init(ipc_init);
diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
index e8e4131..e7829e5 100644
--- a/include/linux/mailbox.h
+++ b/include/linux/mailbox.h
@@ -1,4 +1,16 @@
-/* mailbox.h */
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
 
 typedef u32 mbox_msg_t;
 struct omap_mbox;
@@ -20,3 +32,8 @@ void omap_mbox_save_ctx(struct omap_mbox *mbox);
 void omap_mbox_restore_ctx(struct omap_mbox *mbox);
 void omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq);
 void omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+
+int pl320_ipc_transmit(u32 *data);
+int pl320_ipc_register_notifier(struct notifier_block *nb);
+int pl320_ipc_unregister_notifier(struct notifier_block *nb);
+
-- 
1.7.11.7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help