linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 00/10] Remove ARM local timer API
@ 2013-03-13 18:17 Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 01/10] clocksource: add generic dummy timer driver Stephen Boyd
                   ` (9 more replies)
  0 siblings, 10 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-arm-msm, Mark Rutland, Marc Zyngier,
	Kukjin Kim, Barry Song, Daniel Walker, Bryan Huntsman,
	Gregory CLEMENT, Santosh Shilimkar, Tony Lindgren, John Stultz,
	Thomas Gleixner

In light of Mark Rutland's recent work on divorcing the ARM architected
timers from the ARM local timer API and introducing a generic arch hook for
broadcast it seems that we should remove the local timer API entirely.
Doing so will reduce the architecture dependencies of our timer drivers,
reduce code in ARM core, and simplify timer drivers because they no longer
go through an architecture layer that is essentially a hotplug notifier.

Previous attempts have been made[1] unsuccessfully. I'm hoping this can
be accepted now so that we can clean up the timer drivers that are
used in both UP and SMP situations. Right now these drivers have to ignore
the timer setup callback on the boot CPU to avoid registering clockevents
twice. This is not very symmetric and causes convuluted code that does
the same thing in two places.

Patches based on v3.9-rc2. I'm still looking for Acks/Tested-by
on EXYNOS and PRIMA2.

[1] http://article.gmane.org/gmane.linux.ports.arm.kernel/145705

Note: A hotplug notifier is used by both x86 for the apb_timer (see 
apbt_cpuhp_notify) and by metag (see arch_timer_cpu_notify in
metag_generic.c) so this is not new.

Changes since v2:
 * Bug fixes in smp_twd from Tony Lindgren's testing
 * Move smp_twd to use late_time_init hook
 * Collected Acks

Changes since v1:
 * Picked up Mark's generic dummy timer driver
 * Split out omap changes into new patch

Mark Rutland (1):
  clocksource: add generic dummy timer driver

Stephen Boyd (9):
  ARM: smp: Remove duplicate dummy timer implementation
  ARM: smp_twd: Divorce smp_twd from local timer API
  ARM: OMAP2+: Divorce from local timer API
  ARM: EXYNOS4: Divorce mct from local timer API
  ARM: PRIMA2: Divorce timer-marco from local timer API
  ARM: msm: Divorce msm_timer from local timer API
  clocksource: time-armada-370-xp: Fix sparse warning
  clocksource: time-armada-370-xp: Divorce from local timer API
  ARM: smp: Remove local timer API

 arch/arm/Kconfig                         |  12 +--
 arch/arm/include/asm/localtimer.h        |  34 ---------
 arch/arm/kernel/smp.c                    |  87 ---------------------
 arch/arm/kernel/smp_twd.c                |  64 ++++++++++------
 arch/arm/mach-exynos/mct.c               |  53 +++++++++----
 arch/arm/mach-msm/timer.c                | 125 +++++++++++++++++--------------
 arch/arm/mach-omap2/Kconfig              |   1 -
 arch/arm/mach-omap2/timer.c              |   7 --
 arch/arm/mach-prima2/timer-marco.c       |  98 ++++++++++++------------
 drivers/clocksource/Makefile             |   1 +
 drivers/clocksource/dummy_timer.c        |  67 +++++++++++++++++
 drivers/clocksource/time-armada-370-xp.c |  88 ++++++++++------------
 include/linux/time-armada-370-xp.h       |   4 +-
 13 files changed, 309 insertions(+), 332 deletions(-)
 delete mode 100644 arch/arm/include/asm/localtimer.h
 create mode 100644 drivers/clocksource/dummy_timer.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-21 18:09   ` Mark Rutland
  2013-03-13 18:17 ` [PATCHv3 02/10] ARM: smp: Remove duplicate dummy timer implementation Stephen Boyd
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mark Rutland, linux-arm-msm, Thomas Gleixner, John Stultz, linux-kernel

From: Mark Rutland <mark.rutland@arm.com>

Several architectures have a dummy timer driver tightly coupled with
their broadcast code to support machines without cpu-local timers (or
where there is a lack of driver support).

Since 12ad100046: "clockevents: Add generic timer broadcast function"
it's been possible to write broadcast-capable timer drivers decoupled
from the broadcast mechanism. We can use this functionality to implement
a generic dummy timer driver that can be shared by all architectures
with generic tick broadcast (ARCH_HAS_TICK_BROADCAST).

This patch implements a generic dummy timer using this facility.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
[sboyd: Make percpu data static and use __this_cpu_ptr()]
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clocksource/Makefile      |  1 +
 drivers/clocksource/dummy_timer.c | 67 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 drivers/clocksource/dummy_timer.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 4d8283a..655d718 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_VT8500_TIMER)	+= vt8500_timer.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
+obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
diff --git a/drivers/clocksource/dummy_timer.c b/drivers/clocksource/dummy_timer.c
new file mode 100644
index 0000000..7e1ce45
--- /dev/null
+++ b/drivers/clocksource/dummy_timer.c
@@ -0,0 +1,67 @@
+/*
+ *  linux/drivers/clocksource/dummy_timer.c
+ *
+ *  Copyright (C) 2013 ARM Ltd.
+ *  All Rights Reserved
+ *
+ * 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/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/init.h>
+#include <linux/percpu.h>
+
+static DEFINE_PER_CPU(struct clock_event_device, dummy_timer_evt);
+
+static void dummy_timer_set_mode(enum clock_event_mode mode,
+			   struct clock_event_device *evt)
+{
+	/*
+	 * Core clockevents code will call this when exchanging timer devices.
+	 * We don't need to do anything here.
+	 */
+}
+
+static void __cpuinit dummy_timer_setup(void)
+{
+	int cpu = smp_processor_id();
+	struct clock_event_device *evt = __this_cpu_ptr(&dummy_timer_evt);
+
+	evt->name	= "dummy_timer";
+	evt->features	= CLOCK_EVT_FEAT_PERIODIC |
+			  CLOCK_EVT_FEAT_ONESHOT |
+			  CLOCK_EVT_FEAT_DUMMY;
+	evt->rating	= 100;
+	evt->set_mode	= dummy_timer_set_mode;
+	evt->cpumask	= cpumask_of(cpu);
+
+	clockevents_register_device(evt);
+}
+
+static int __cpuinit dummy_timer_cpu_notify(struct notifier_block *self,
+				      unsigned long action, void *hcpu)
+{
+	if ((action & ~CPU_TASKS_FROZEN) == CPU_STARTING)
+		dummy_timer_setup();
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block dummy_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = dummy_timer_cpu_notify,
+};
+
+static int __init dummy_timer_register(void)
+{
+	int err = register_cpu_notifier(&dummy_timer_cpu_nb);
+	if (err)
+		return err;
+
+	/* We won't get a call on the boot CPU, so register immediately */
+	dummy_timer_setup();
+
+	return 0;
+}
+device_initcall(dummy_timer_register);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 02/10] ARM: smp: Remove duplicate dummy timer implementation
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 01/10] clocksource: add generic dummy timer driver Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API Stephen Boyd
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-arm-msm, Russell King, linux-kernel

Drop ARM's version of the dummy timer now that we have a generic
implementation in drivers/clocksource.

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/smp.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 31644f1..4de4d14 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -469,24 +469,6 @@ void tick_broadcast(const struct cpumask *mask)
 }
 #endif
 
-static void broadcast_timer_set_mode(enum clock_event_mode mode,
-	struct clock_event_device *evt)
-{
-}
-
-static void __cpuinit broadcast_timer_setup(struct clock_event_device *evt)
-{
-	evt->name	= "dummy_timer";
-	evt->features	= CLOCK_EVT_FEAT_ONESHOT |
-			  CLOCK_EVT_FEAT_PERIODIC |
-			  CLOCK_EVT_FEAT_DUMMY;
-	evt->rating	= 400;
-	evt->mult	= 1;
-	evt->set_mode	= broadcast_timer_set_mode;
-
-	clockevents_register_device(evt);
-}
-
 static struct local_timer_ops *lt_ops;
 
 #ifdef CONFIG_LOCAL_TIMERS
@@ -510,8 +492,8 @@ static void __cpuinit percpu_timer_setup(void)
 
 	evt->cpumask = cpumask_of(cpu);
 
-	if (!lt_ops || lt_ops->setup(evt))
-		broadcast_timer_setup(evt);
+	if (lt_ops)
+		lt_ops->setup(evt);
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 01/10] clocksource: add generic dummy timer driver Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 02/10] ARM: smp: Remove duplicate dummy timer implementation Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-28 15:22   ` Mark Rutland
  2013-03-13 18:17 ` [PATCHv3 04/10] ARM: OMAP2+: Divorce " Stephen Boyd
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Tony Lindgren, linux-arm-msm, Russell King, linux-kernel

Separate the smp_twd timers from the local timer API. This will
allow us to remove ARM local timer support in the near future and
gets us closer to moving this driver to drivers/clocksource.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

Changes since v2:
 * Fix booting on qemu and omap

 arch/arm/Kconfig          |  2 +-
 arch/arm/kernel/smp_twd.c | 64 +++++++++++++++++++++++++++++++----------------
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5b71469..5ad2ccf 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1527,6 +1527,7 @@ config SMP
 	depends on HAVE_SMP
 	depends on MMU
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
+	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
 	select USE_GENERIC_SMP_HELPERS
 	help
 	  This enables support for systems with more than one CPU. If you have
@@ -1650,7 +1651,6 @@ config LOCAL_TIMERS
 	bool "Use local timer interrupts"
 	depends on SMP
 	default y
-	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
 	help
 	  Enable support for local timers on SMP platforms, rather then the
 	  legacy IPI broadcast method.  Local timers allows the system
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 3f25650..a79476d 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -24,7 +25,6 @@
 
 #include <asm/smp_plat.h>
 #include <asm/smp_twd.h>
-#include <asm/localtimer.h>
 
 /* set up by the platform code */
 static void __iomem *twd_base;
@@ -33,7 +33,7 @@ static struct clk *twd_clk;
 static unsigned long twd_timer_rate;
 static DEFINE_PER_CPU(bool, percpu_setup_called);
 
-static struct clock_event_device __percpu **twd_evt;
+static struct clock_event_device __percpu *twd_evt;
 static int twd_ppi;
 
 static void twd_set_mode(enum clock_event_mode mode,
@@ -90,8 +90,10 @@ static int twd_timer_ack(void)
 	return 0;
 }
 
-static void twd_timer_stop(struct clock_event_device *clk)
+static void twd_timer_stop(void)
 {
+	struct clock_event_device *clk = __this_cpu_ptr(twd_evt);
+
 	twd_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
 	disable_percpu_irq(clk->irq);
 }
@@ -106,7 +108,7 @@ static void twd_update_frequency(void *new_rate)
 {
 	twd_timer_rate = *((unsigned long *) new_rate);
 
-	clockevents_update_freq(*__this_cpu_ptr(twd_evt), twd_timer_rate);
+	clockevents_update_freq(__this_cpu_ptr(twd_evt), twd_timer_rate);
 }
 
 static int twd_rate_change(struct notifier_block *nb,
@@ -132,7 +134,7 @@ static struct notifier_block twd_clk_nb = {
 
 static int twd_clk_init(void)
 {
-	if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
+	if (twd_evt && __this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
 		return clk_notifier_register(twd_clk, &twd_clk_nb);
 
 	return 0;
@@ -151,7 +153,7 @@ static void twd_update_frequency(void *data)
 {
 	twd_timer_rate = clk_get_rate(twd_clk);
 
-	clockevents_update_freq(*__this_cpu_ptr(twd_evt), twd_timer_rate);
+	clockevents_update_freq(__this_cpu_ptr(twd_evt), twd_timer_rate);
 }
 
 static int twd_cpufreq_transition(struct notifier_block *nb,
@@ -177,7 +179,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
+	if (twd_evt && __this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);
 
@@ -228,7 +230,7 @@ static void __cpuinit twd_calibrate_rate(void)
 
 static irqreturn_t twd_handler(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 
 	if (twd_timer_ack()) {
 		evt->event_handler(evt);
@@ -265,9 +267,9 @@ static void twd_get_clock(struct device_node *np)
 /*
  * Setup the local clock events for a CPU.
  */
-static int __cpuinit twd_timer_setup(struct clock_event_device *clk)
+static void __cpuinit twd_timer_setup(void)
 {
-	struct clock_event_device **this_cpu_clk;
+	struct clock_event_device *clk = __this_cpu_ptr(twd_evt);
 	int cpu = smp_processor_id();
 
 	/*
@@ -276,9 +278,9 @@ static int __cpuinit twd_timer_setup(struct clock_event_device *clk)
 	 */
 	if (per_cpu(percpu_setup_called, cpu)) {
 		__raw_writel(0, twd_base + TWD_TIMER_CONTROL);
-		clockevents_register_device(*__this_cpu_ptr(twd_evt));
+		clockevents_register_device(clk);
 		enable_percpu_irq(clk->irq, 0);
-		return 0;
+		return;
 	}
 	per_cpu(percpu_setup_called, cpu) = true;
 
@@ -297,27 +299,37 @@ static int __cpuinit twd_timer_setup(struct clock_event_device *clk)
 	clk->set_mode = twd_set_mode;
 	clk->set_next_event = twd_set_next_event;
 	clk->irq = twd_ppi;
-
-	this_cpu_clk = __this_cpu_ptr(twd_evt);
-	*this_cpu_clk = clk;
+	clk->cpumask = cpumask_of(cpu);
 
 	clockevents_config_and_register(clk, twd_timer_rate,
 					0xf, 0xffffffff);
 	enable_percpu_irq(clk->irq, 0);
+}
 
-	return 0;
+static int __cpuinit twd_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		twd_timer_setup();
+		break;
+	case CPU_DYING:
+		twd_timer_stop();
+		break;
+	}
+
+	return NOTIFY_OK;
 }
 
-static struct local_timer_ops twd_lt_ops __cpuinitdata = {
-	.setup	= twd_timer_setup,
-	.stop	= twd_timer_stop,
+static struct notifier_block twd_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = twd_timer_cpu_notify,
 };
 
 static int __init twd_local_timer_common_register(struct device_node *np)
 {
 	int err;
 
-	twd_evt = alloc_percpu(struct clock_event_device *);
+	twd_evt = alloc_percpu(struct clock_event_device);
 	if (!twd_evt) {
 		err = -ENOMEM;
 		goto out_free;
@@ -329,12 +341,22 @@ static int __init twd_local_timer_common_register(struct device_node *np)
 		goto out_free;
 	}
 
-	err = local_timer_register(&twd_lt_ops);
+	err = register_cpu_notifier(&twd_timer_cpu_nb);
 	if (err)
 		goto out_irq;
 
 	twd_get_clock(np);
 
+	/*
+	 * Immediately configure the timer on the boot CPU, unless we need
+	 * jiffies to be incrementing to calibrate the rate in which case
+	 * setup the timer in late_time_init.
+	 */
+	if (twd_timer_rate)
+		twd_timer_setup();
+	else
+		late_time_init = twd_timer_setup;
+
 	return 0;
 
 out_irq:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 04/10] ARM: OMAP2+: Divorce from local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (2 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 05/10] ARM: EXYNOS4: Divorce mct " Stephen Boyd
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-kernel, linux-arm-msm

Now that the TWD doesn't rely on the local timer API, OMAP can
stop selecting it in Kconfig and relying on the config option to
decide if it should call smp_twd functions.

Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-omap2/Kconfig | 1 -
 arch/arm/mach-omap2/timer.c | 7 -------
 2 files changed, 8 deletions(-)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 49ac3df..6e1f871 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -88,7 +88,6 @@ config ARCH_OMAP4
 	select CACHE_L2X0
 	select CPU_V7
 	select HAVE_SMP
-	select LOCAL_TIMERS if SMP
 	select OMAP_INTERCONNECT
 	select PL310_ERRATA_588369
 	select PL310_ERRATA_727915
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 2bdd4cf..c00a8f8 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -587,7 +587,6 @@ OMAP_SYS_GP_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, "ti,timer-alwon",
 #ifdef CONFIG_ARCH_OMAP4
 OMAP_SYS_32K_TIMER_INIT(4, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
 			2, OMAP4_MPU_SOURCE);
-#ifdef CONFIG_LOCAL_TIMERS
 static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
 void __init omap4_local_timer_init(void)
 {
@@ -606,12 +605,6 @@ void __init omap4_local_timer_init(void)
 			pr_err("twd_local_timer_register failed %d\n", err);
 	}
 }
-#else /* CONFIG_LOCAL_TIMERS */
-void __init omap4_local_timer_init(void)
-{
-	omap4_sync32k_timer_init();
-}
-#endif /* CONFIG_LOCAL_TIMERS */
 #endif /* CONFIG_ARCH_OMAP4 */
 
 #ifdef CONFIG_SOC_OMAP5
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 05/10] ARM: EXYNOS4: Divorce mct from local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (3 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 04/10] ARM: OMAP2+: Divorce " Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 06/10] ARM: PRIMA2: Divorce timer-marco " Stephen Boyd
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-kernel, linux-arm-msm, Kukjin Kim

Separate the mct local timers from the local timer API. This will
allow us to remove ARM local timer support in the near future and
gets us closer to moving this driver to drivers/clocksource.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-exynos/mct.c | 53 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
index c9d6650..5a9a73f 100644
--- a/arch/arm/mach-exynos/mct.c
+++ b/arch/arm/mach-exynos/mct.c
@@ -16,13 +16,13 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/clockchips.h>
+#include <linux/cpu.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/percpu.h>
 #include <linux/of.h>
 
 #include <asm/arch_timer.h>
-#include <asm/localtimer.h>
 
 #include <plat/cpu.h>
 
@@ -42,7 +42,7 @@ static unsigned long clk_rate;
 static unsigned int mct_int_type;
 
 struct mct_clock_event_device {
-	struct clock_event_device *evt;
+	struct clock_event_device evt;
 	void __iomem *base;
 	char name[10];
 };
@@ -264,8 +264,6 @@ static void exynos4_clockevent_init(void)
 		setup_irq(EXYNOS4_IRQ_MCT_G0, &mct_comp_event_irq);
 }
 
-#ifdef CONFIG_LOCAL_TIMERS
-
 static DEFINE_PER_CPU(struct mct_clock_event_device, percpu_mct_tick);
 
 /* Clock event handling */
@@ -338,7 +336,7 @@ static inline void exynos4_tick_set_mode(enum clock_event_mode mode,
 
 static int exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
 {
-	struct clock_event_device *evt = mevt->evt;
+	struct clock_event_device *evt = &mevt->evt;
 
 	/*
 	 * This is for supporting oneshot mode.
@@ -360,7 +358,7 @@ static int exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
 static irqreturn_t exynos4_mct_tick_isr(int irq, void *dev_id)
 {
 	struct mct_clock_event_device *mevt = dev_id;
-	struct clock_event_device *evt = mevt->evt;
+	struct clock_event_device *evt = &mevt->evt;
 
 	exynos4_mct_tick_clear(mevt);
 
@@ -388,7 +386,6 @@ static int __cpuinit exynos4_local_timer_setup(struct clock_event_device *evt)
 	int mct_lx_irq;
 
 	mevt = this_cpu_ptr(&percpu_mct_tick);
-	mevt->evt = evt;
 
 	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
 	sprintf(mevt->name, "mct_tick%d", cpu);
@@ -426,7 +423,7 @@ static int __cpuinit exynos4_local_timer_setup(struct clock_event_device *evt)
 	return 0;
 }
 
-static void exynos4_local_timer_stop(struct clock_event_device *evt)
+static void __cpuinit exynos4_local_timer_stop(struct clock_event_device *evt)
 {
 	unsigned int cpu = smp_processor_id();
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
@@ -439,22 +436,38 @@ static void exynos4_local_timer_stop(struct clock_event_device *evt)
 		disable_percpu_irq(EXYNOS_IRQ_MCT_LOCALTIMER);
 }
 
-static struct local_timer_ops exynos4_mct_tick_ops __cpuinitdata = {
-	.setup	= exynos4_local_timer_setup,
-	.stop	= exynos4_local_timer_stop,
+static int __cpuinit exynos4_mct_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
+	struct clock_event_device *evt = &mevt->evt;
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		exynos4_local_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		exynos4_local_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block exynos4_mct_cpu_nb __cpuinitdata = {
+	.notifier_call = exynos4_mct_cpu_notify,
 };
-#endif /* CONFIG_LOCAL_TIMERS */
 
 static void __init exynos4_timer_resources(void)
 {
+	int err;
+	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
 	struct clk *mct_clk;
 	mct_clk = clk_get(NULL, "xtal");
 
 	clk_rate = clk_get_rate(mct_clk);
 
-#ifdef CONFIG_LOCAL_TIMERS
 	if (mct_int_type == MCT_INT_PPI) {
-		int err;
 
 		err = request_percpu_irq(EXYNOS_IRQ_MCT_LOCALTIMER,
 					 exynos4_mct_tick_isr, "MCT",
@@ -463,8 +476,16 @@ static void __init exynos4_timer_resources(void)
 		     EXYNOS_IRQ_MCT_LOCALTIMER, err);
 	}
 
-	local_timer_register(&exynos4_mct_tick_ops);
-#endif /* CONFIG_LOCAL_TIMERS */
+	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
+	if (err)
+		goto out_irq;
+
+	/* Immediately configure the timer on the boot CPU */
+	exynos4_local_timer_setup(&mevt->evt);
+	return;
+
+out_irq:
+	free_percpu_irq(EXYNOS_IRQ_MCT_LOCALTIMER, &percpu_mct_tick);
 }
 
 void __init exynos4_timer_init(void)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 06/10] ARM: PRIMA2: Divorce timer-marco from local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (4 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 05/10] ARM: EXYNOS4: Divorce mct " Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 07/10] ARM: msm: Divorce msm_timer " Stephen Boyd
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-kernel, linux-arm-msm, Barry Song

Separate the marco local timers from the local timer API. This
will allow us to remove ARM local timer support in the near future
and gets us closer to moving this driver to drivers/clocksource.

Cc: Barry Song <baohua.song@csr.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-prima2/timer-marco.c | 98 ++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 46 deletions(-)

diff --git a/arch/arm/mach-prima2/timer-marco.c b/arch/arm/mach-prima2/timer-marco.c
index f4eea2e..d54aac2 100644
--- a/arch/arm/mach-prima2/timer-marco.c
+++ b/arch/arm/mach-prima2/timer-marco.c
@@ -10,6 +10,7 @@
 #include <linux/interrupt.h>
 #include <linux/clockchips.h>
 #include <linux/clocksource.h>
+#include <linux/cpu.h>
 #include <linux/bitops.h>
 #include <linux/irq.h>
 #include <linux/clk.h>
@@ -18,7 +19,6 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <asm/sched_clock.h>
-#include <asm/localtimer.h>
 #include <asm/mach/time.h>
 
 #include "common.h"
@@ -154,13 +154,7 @@ static void sirfsoc_clocksource_resume(struct clocksource *cs)
 		BIT(1) | BIT(0), sirfsoc_timer_base + SIRFSOC_TIMER_64COUNTER_CTRL);
 }
 
-static struct clock_event_device sirfsoc_clockevent = {
-	.name = "sirfsoc_clockevent",
-	.rating = 200,
-	.features = CLOCK_EVT_FEAT_ONESHOT,
-	.set_mode = sirfsoc_timer_set_mode,
-	.set_next_event = sirfsoc_timer_set_next_event,
-};
+static struct clock_event_device __percpu *sirfsoc_clockevent;
 
 static struct clocksource sirfsoc_clocksource = {
 	.name = "sirfsoc_clocksource",
@@ -176,11 +170,8 @@ static struct irqaction sirfsoc_timer_irq = {
 	.name = "sirfsoc_timer0",
 	.flags = IRQF_TIMER | IRQF_NOBALANCING,
 	.handler = sirfsoc_timer_interrupt,
-	.dev_id = &sirfsoc_clockevent,
 };
 
-#ifdef CONFIG_LOCAL_TIMERS
-
 static struct irqaction sirfsoc_timer1_irq = {
 	.name = "sirfsoc_timer1",
 	.flags = IRQF_TIMER | IRQF_NOBALANCING,
@@ -189,56 +180,75 @@ static struct irqaction sirfsoc_timer1_irq = {
 
 static int __cpuinit sirfsoc_local_timer_setup(struct clock_event_device *ce)
 {
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
+	int cpu = smp_processor_id();
+	struct irqaction *action;
+
+	if (cpu == 0)
+		action = &sirfsoc_timer_irq;
+	else
+		action = &sirfsoc_timer1_irq;
 
-	ce->irq = sirfsoc_timer1_irq.irq;
+	ce->irq = action->irq;
 	ce->name = "local_timer";
-	ce->features = sirfsoc_clockevent.features;
-	ce->rating = sirfsoc_clockevent.rating;
+	ce->features = CLOCK_EVT_FEAT_ONESHOT;
+	ce->rating = 200;
 	ce->set_mode = sirfsoc_timer_set_mode;
 	ce->set_next_event = sirfsoc_timer_set_next_event;
-	ce->shift = sirfsoc_clockevent.shift;
-	ce->mult = sirfsoc_clockevent.mult;
-	ce->max_delta_ns = sirfsoc_clockevent.max_delta_ns;
-	ce->min_delta_ns = sirfsoc_clockevent.min_delta_ns;
+	clockevents_calc_mult_shift(ce, CLOCK_TICK_RATE, 60);
+	ce->max_delta_ns = clockevent_delta2ns(-2, ce);
+	ce->min_delta_ns = clockevent_delta2ns(2, ce);
+	ce->cpumask = cpumask_of(cpu);
 
-	sirfsoc_timer1_irq.dev_id = ce;
-	BUG_ON(setup_irq(ce->irq, &sirfsoc_timer1_irq));
-	irq_set_affinity(sirfsoc_timer1_irq.irq, cpumask_of(1));
+	action->dev_id = ce;
+	BUG_ON(setup_irq(ce->irq, action));
+	irq_set_affinity(action->irq, cpumask_of(cpu));
 
 	clockevents_register_device(ce);
 	return 0;
 }
 
-static void sirfsoc_local_timer_stop(struct clock_event_device *ce)
+static void __cpuinit sirfsoc_local_timer_stop(struct clock_event_device *ce)
 {
+	int cpu = smp_processor_id();
+
 	sirfsoc_timer_count_disable(1);
 
-	remove_irq(sirfsoc_timer1_irq.irq, &sirfsoc_timer1_irq);
+	if (cpu == 0)
+		remove_irq(sirfsoc_timer_irq.irq, &sirfsoc_timer_irq);
+	else
+		remove_irq(sirfsoc_timer1_irq.irq, &sirfsoc_timer1_irq);
+}
+
+static int __cpuinit sirfsoc_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(sirfsoc_clockevent);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		sirfsoc_local_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		sirfsoc_local_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
 }
 
-static struct local_timer_ops sirfsoc_local_timer_ops __cpuinitdata = {
-	.setup	= sirfsoc_local_timer_setup,
-	.stop	= sirfsoc_local_timer_stop,
+static struct notifier_block sirfsoc_cpu_nb __cpuinitdata = {
+	.notifier_call = sirfsoc_cpu_notify,
 };
-#endif /* CONFIG_LOCAL_TIMERS */
 
 static void __init sirfsoc_clockevent_init(void)
 {
-	clockevents_calc_mult_shift(&sirfsoc_clockevent, CLOCK_TICK_RATE, 60);
-
-	sirfsoc_clockevent.max_delta_ns =
-		clockevent_delta2ns(-2, &sirfsoc_clockevent);
-	sirfsoc_clockevent.min_delta_ns =
-		clockevent_delta2ns(2, &sirfsoc_clockevent);
-
-	sirfsoc_clockevent.cpumask = cpumask_of(0);
-	clockevents_register_device(&sirfsoc_clockevent);
-#ifdef CONFIG_LOCAL_TIMERS
-	local_timer_register(&sirfsoc_local_timer_ops);
-#endif
+	sirfsoc_clockevent = alloc_percpu(struct clock_event_device);
+	BUG_ON(!sirfsoc_clockevent);
+
+	BUG_ON(register_cpu_notifier(&sirfsoc_cpu_nb));
+
+	/* Immediately configure the timer on the boot CPU */
+	sirfsoc_local_timer_setup(this_cpu_ptr(sirfsoc_clockevent));
 }
 
 /* initialize the kernel jiffy timer source */
@@ -281,8 +291,6 @@ void __init sirfsoc_marco_timer_init(void)
 
 	BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
 
-	BUG_ON(setup_irq(sirfsoc_timer_irq.irq, &sirfsoc_timer_irq));
-
 	sirfsoc_clockevent_init();
 }
 
@@ -306,11 +314,9 @@ static void __init sirfsoc_of_timer_map(void)
 	if (!sirfsoc_timer_irq.irq)
 		panic("No irq passed for timer0 via DT\n");
 
-#ifdef CONFIG_LOCAL_TIMERS
 	sirfsoc_timer1_irq.irq = irq_of_parse_and_map(np, 1);
 	if (!sirfsoc_timer1_irq.irq)
 		panic("No irq passed for timer1 via DT\n");
-#endif
 
 	of_node_put(np);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 07/10] ARM: msm: Divorce msm_timer from local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (5 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 06/10] ARM: PRIMA2: Divorce timer-marco " Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning Stephen Boyd
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-arm-msm, Daniel Walker, Bryan Huntsman

Separate the msm_timer from the local timer API. This will allow
us to remove ARM local timer support in the near future and gets
us closer to moving this driver to drivers/clocksource.

Acked-by: David Brown <davidb@codeaurora.org>
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Bryan Huntsman <bryanh@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-msm/timer.c | 125 +++++++++++++++++++++++++---------------------
 1 file changed, 67 insertions(+), 58 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 2969027..4675c5e 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -16,6 +16,7 @@
 
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
+#include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -25,7 +26,6 @@
 #include <linux/of_irq.h>
 
 #include <asm/mach/time.h>
-#include <asm/localtimer.h>
 #include <asm/sched_clock.h>
 
 #include "common.h"
@@ -46,7 +46,7 @@ static void __iomem *event_base;
 
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 	/* Stop the timer tick */
 	if (evt->mode == CLOCK_EVT_MODE_ONESHOT) {
 		u32 ctrl = readl_relaxed(event_base + TIMER_ENABLE);
@@ -90,18 +90,7 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
 	writel_relaxed(ctrl, event_base + TIMER_ENABLE);
 }
 
-static struct clock_event_device msm_clockevent = {
-	.name		= "gp_timer",
-	.features	= CLOCK_EVT_FEAT_ONESHOT,
-	.rating		= 200,
-	.set_next_event	= msm_timer_set_next_event,
-	.set_mode	= msm_timer_set_mode,
-};
-
-static union {
-	struct clock_event_device *evt;
-	struct clock_event_device * __percpu *percpu_evt;
-} msm_evt;
+static struct clock_event_device __percpu *msm_evt;
 
 static void __iomem *source_base;
 
@@ -127,40 +116,66 @@ static struct clocksource msm_clocksource = {
 	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
-#ifdef CONFIG_LOCAL_TIMERS
+static int msm_timer_irq;
+static int msm_timer_has_ppi;
+
 static int __cpuinit msm_local_timer_setup(struct clock_event_device *evt)
 {
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
+	int cpu = smp_processor_id();
+	int err;
 
 	writel_relaxed(0, event_base + TIMER_ENABLE);
 	writel_relaxed(0, event_base + TIMER_CLEAR);
 	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
-	evt->irq = msm_clockevent.irq;
+	evt->irq = msm_timer_irq;
 	evt->name = "local_timer";
-	evt->features = msm_clockevent.features;
-	evt->rating = msm_clockevent.rating;
+	evt->features = CLOCK_EVT_FEAT_ONESHOT;
+	evt->rating = 200;
 	evt->set_mode = msm_timer_set_mode;
 	evt->set_next_event = msm_timer_set_next_event;
+	evt->cpumask = cpumask_of(cpu);
+
+	clockevents_config_and_register(evt, GPT_HZ, 4, 0xffffffff);
+
+	if (msm_timer_has_ppi) {
+		enable_percpu_irq(evt->irq, IRQ_TYPE_EDGE_RISING);
+	} else {
+		err = request_irq(evt->irq, msm_timer_interrupt,
+				IRQF_TIMER | IRQF_NOBALANCING |
+				IRQF_TRIGGER_RISING, "gp_timer", evt);
+		if (err)
+			pr_err("request_irq failed\n");
+	}
 
-	*__this_cpu_ptr(msm_evt.percpu_evt) = evt;
-	clockevents_config_and_register(evt, GPT_HZ, 4, 0xf0000000);
-	enable_percpu_irq(evt->irq, IRQ_TYPE_EDGE_RISING);
 	return 0;
 }
 
-static void msm_local_timer_stop(struct clock_event_device *evt)
+static void __cpuinit msm_local_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
 
-static struct local_timer_ops msm_local_timer_ops __cpuinitdata = {
-	.setup	= msm_local_timer_setup,
-	.stop	= msm_local_timer_stop,
+static int __cpuinit msm_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(msm_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		msm_local_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		msm_local_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block msm_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = msm_timer_cpu_notify,
 };
-#endif /* CONFIG_LOCAL_TIMERS */
 
 static notrace u32 msm_sched_clock_read(void)
 {
@@ -170,41 +185,35 @@ static notrace u32 msm_sched_clock_read(void)
 static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
 				  bool percpu)
 {
-	struct clock_event_device *ce = &msm_clockevent;
 	struct clocksource *cs = &msm_clocksource;
-	int res;
+	int res = 0;
 
-	writel_relaxed(0, event_base + TIMER_ENABLE);
-	writel_relaxed(0, event_base + TIMER_CLEAR);
-	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
-	ce->cpumask = cpumask_of(0);
-	ce->irq = irq;
-
-	clockevents_config_and_register(ce, GPT_HZ, 4, 0xffffffff);
-	if (percpu) {
-		msm_evt.percpu_evt = alloc_percpu(struct clock_event_device *);
-		if (!msm_evt.percpu_evt) {
-			pr_err("memory allocation failed for %s\n", ce->name);
+	msm_timer_irq = irq;
+	msm_timer_has_ppi = percpu;
+
+	msm_evt = alloc_percpu(struct clock_event_device);
+	if (!msm_evt) {
+		pr_err("memory allocation failed for clockevents\n");
+		goto err;
+	}
+
+	if (percpu)
+		res = request_percpu_irq(irq, msm_timer_interrupt,
+					 "gp_timer", msm_evt);
+
+	if (res) {
+		pr_err("request_percpu_irq failed\n");
+	} else {
+		res = register_cpu_notifier(&msm_timer_cpu_nb);
+		if (res) {
+			free_percpu_irq(irq, msm_evt);
 			goto err;
 		}
-		*__this_cpu_ptr(msm_evt.percpu_evt) = ce;
-		res = request_percpu_irq(ce->irq, msm_timer_interrupt,
-					 ce->name, msm_evt.percpu_evt);
-		if (!res) {
-			enable_percpu_irq(ce->irq, IRQ_TYPE_EDGE_RISING);
-#ifdef CONFIG_LOCAL_TIMERS
-			local_timer_register(&msm_local_timer_ops);
-#endif
-		}
-	} else {
-		msm_evt.evt = ce;
-		res = request_irq(ce->irq, msm_timer_interrupt,
-				  IRQF_TIMER | IRQF_NOBALANCING |
-				  IRQF_TRIGGER_RISING, ce->name, &msm_evt.evt);
+
+		/* Immediately configure the timer on the boot CPU */
+		msm_local_timer_setup(this_cpu_ptr(msm_evt));
 	}
 
-	if (res)
-		pr_err("request_irq failed for %s\n", ce->name);
 err:
 	writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
 	res = clocksource_register_hz(cs, dgt_hz);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (6 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 07/10] ARM: msm: Divorce msm_timer " Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-20 17:06   ` Gregory CLEMENT
  2013-03-13 18:17 ` [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API Stephen Boyd
  2013-03-13 18:17 ` [PATCHv3 10/10] ARM: smp: Remove " Stephen Boyd
  9 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-kernel, linux-arm-msm, Gregory CLEMENT

drivers/clocksource/time-armada-370-xp.c:217:13: warning: symbol
'armada_370_xp_timer_init' was not declared. Should it be static?

Also remove the __init marking in the prototype as it's
unnecessary and drop the init.h file.

Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clocksource/time-armada-370-xp.c | 3 ++-
 include/linux/time-armada-370-xp.h       | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 47a6730..efe4aef 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -27,10 +27,11 @@
 #include <linux/of_address.h>
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
 #include <asm/localtimer.h>
-#include <linux/percpu.h>
 /*
  * Timer block registers.
  */
diff --git a/include/linux/time-armada-370-xp.h b/include/linux/time-armada-370-xp.h
index dfdfdc0..6fb0856 100644
--- a/include/linux/time-armada-370-xp.h
+++ b/include/linux/time-armada-370-xp.h
@@ -11,8 +11,6 @@
 #ifndef __TIME_ARMADA_370_XPPRCMU_H
 #define __TIME_ARMADA_370_XPPRCMU_H
 
-#include <linux/init.h>
-
-void __init armada_370_xp_timer_init(void);
+void armada_370_xp_timer_init(void);
 
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (7 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  2013-03-20 17:09   ` Gregory CLEMENT
  2013-03-13 18:17 ` [PATCHv3 10/10] ARM: smp: Remove " Stephen Boyd
  9 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-kernel, linux-arm-msm, Gregory CLEMENT

Separate the armada 370xp local timers from the local timer API.
This will allow us to remove ARM local timer support in the near
future and makes this driver multi-architecture friendly.

Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clocksource/time-armada-370-xp.c | 85 ++++++++++++++------------------
 1 file changed, 38 insertions(+), 47 deletions(-)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index efe4aef..ee2e50c5 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpu.h>
 #include <linux/timer.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
@@ -31,7 +32,6 @@
 #include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
-#include <asm/localtimer.h>
 /*
  * Timer block registers.
  */
@@ -70,7 +70,7 @@ static bool timer25Mhz = true;
  */
 static u32 ticks_per_jiffy;
 
-static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
+static struct clock_event_device __percpu *armada_370_xp_evt;
 
 static u32 notrace armada_370_xp_read_sched_clock(void)
 {
@@ -143,14 +143,7 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode,
 	}
 }
 
-static struct clock_event_device armada_370_xp_clkevt = {
-	.name		= "armada_370_xp_per_cpu_tick",
-	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
-	.shift		= 32,
-	.rating		= 300,
-	.set_next_event	= armada_370_xp_clkevt_next_event,
-	.set_mode	= armada_370_xp_clkevt_mode,
-};
+static int armada_370_xp_clkevt_irq;
 
 static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
 {
@@ -173,42 +166,53 @@ static int __cpuinit armada_370_xp_timer_setup(struct clock_event_device *evt)
 	u32 u;
 	int cpu = smp_processor_id();
 
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
-
 	u = readl(local_base + TIMER_CTRL_OFF);
 	if (timer25Mhz)
 		writel(u | TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 	else
 		writel(u & ~TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 
-	evt->name		= armada_370_xp_clkevt.name;
-	evt->irq		= armada_370_xp_clkevt.irq;
-	evt->features		= armada_370_xp_clkevt.features;
-	evt->shift		= armada_370_xp_clkevt.shift;
-	evt->rating		= armada_370_xp_clkevt.rating,
+	evt->name		= "armada_370_xp_per_cpu_tick",
+	evt->features		= CLOCK_EVT_FEAT_ONESHOT |
+				  CLOCK_EVT_FEAT_PERIODIC;
+	evt->shift		= 32,
+	evt->rating		= 300,
 	evt->set_next_event	= armada_370_xp_clkevt_next_event,
 	evt->set_mode		= armada_370_xp_clkevt_mode,
+	evt->irq		= armada_370_xp_clkevt_irq;
 	evt->cpumask		= cpumask_of(cpu);
 
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = evt;
-
 	clockevents_config_and_register(evt, timer_clk, 1, 0xfffffffe);
 	enable_percpu_irq(evt->irq, 0);
 
 	return 0;
 }
 
-static void  armada_370_xp_timer_stop(struct clock_event_device *evt)
+static void __cpuinit armada_370_xp_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
 
-static struct local_timer_ops armada_370_xp_local_timer_ops __cpuinitdata = {
-	.setup	= armada_370_xp_timer_setup,
-	.stop	=  armada_370_xp_timer_stop,
+static int __cpuinit armada_370_xp_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(armada_370_xp_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		armada_370_xp_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		armada_370_xp_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block armada_370_xp_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = armada_370_xp_timer_cpu_notify,
 };
 
 void __init armada_370_xp_timer_init(void)
@@ -224,9 +228,6 @@ void __init armada_370_xp_timer_init(void)
 
 	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
 		/* The fixed 25MHz timer is available so let's use it */
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u | TIMER0_25MHZ,
-		       local_base + TIMER_CTRL_OFF);
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u | TIMER0_25MHZ,
 		       timer_base + TIMER_CTRL_OFF);
@@ -236,9 +237,6 @@ void __init armada_370_xp_timer_init(void)
 		struct clk *clk = of_clk_get(np, 0);
 		WARN_ON(IS_ERR(clk));
 		rate =  clk_get_rate(clk);
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u & ~(TIMER0_25MHZ),
-		       local_base + TIMER_CTRL_OFF);
 
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u & ~(TIMER0_25MHZ),
@@ -252,7 +250,7 @@ void __init armada_370_xp_timer_init(void)
 	 * We use timer 0 as clocksource, and private(local) timer 0
 	 * for clockevents
 	 */
-	armada_370_xp_clkevt.irq = irq_of_parse_and_map(np, 4);
+	armada_370_xp_clkevt_irq = irq_of_parse_and_map(np, 4);
 
 	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
 
@@ -277,26 +275,19 @@ void __init armada_370_xp_timer_init(void)
 			      "armada_370_xp_clocksource",
 			      timer_clk, 300, 32, clocksource_mmio_readl_down);
 
-	/* Register the clockevent on the private timer of CPU 0 */
-	armada_370_xp_clkevt.cpumask = cpumask_of(0);
-	clockevents_config_and_register(&armada_370_xp_clkevt,
-					timer_clk, 1, 0xfffffffe);
+	register_cpu_notifier(&armada_370_xp_timer_cpu_nb);
 
-	percpu_armada_370_xp_evt = alloc_percpu(struct clock_event_device *);
+	armada_370_xp_evt = alloc_percpu(struct clock_event_device);
 
 
 	/*
 	 * Setup clockevent timer (interrupt-driven).
 	 */
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = &armada_370_xp_clkevt;
-	res = request_percpu_irq(armada_370_xp_clkevt.irq,
+	res = request_percpu_irq(armada_370_xp_clkevt_irq,
 				armada_370_xp_timer_interrupt,
-				armada_370_xp_clkevt.name,
-				percpu_armada_370_xp_evt);
-	if (!res) {
-		enable_percpu_irq(armada_370_xp_clkevt.irq, 0);
-#ifdef CONFIG_LOCAL_TIMERS
-		local_timer_register(&armada_370_xp_local_timer_ops);
-#endif
-	}
+				"armada_370_xp_per_cpu_tick",
+				armada_370_xp_evt);
+	/* Immediately configure the timer on the boot CPU */
+	if (!res)
+		armada_370_xp_timer_setup(this_cpu_ptr(armada_370_xp_evt));
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCHv3 10/10] ARM: smp: Remove local timer API
  2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
                   ` (8 preceding siblings ...)
  2013-03-13 18:17 ` [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API Stephen Boyd
@ 2013-03-13 18:17 ` Stephen Boyd
  9 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-13 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-kernel, linux-arm-msm, Russell King

There are no more users of this API, remove it.

Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/Kconfig                  | 10 ------
 arch/arm/include/asm/localtimer.h | 34 -------------------
 arch/arm/kernel/smp.c             | 69 ---------------------------------------
 3 files changed, 113 deletions(-)
 delete mode 100644 arch/arm/include/asm/localtimer.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5ad2ccf..f1a4c57 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1647,16 +1647,6 @@ config ARM_PSCI
 	  0022A ("Power State Coordination Interface System Software on
 	  ARM processors").
 
-config LOCAL_TIMERS
-	bool "Use local timer interrupts"
-	depends on SMP
-	default y
-	help
-	  Enable support for local timers on SMP platforms, rather then the
-	  legacy IPI broadcast method.  Local timers allows the system
-	  accounting to be spread across the timer interval, preventing a
-	  "thundering herd" at every timer tick.
-
 config ARCH_NR_GPIO
 	int
 	default 1024 if ARCH_SHMOBILE || ARCH_TEGRA
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
deleted file mode 100644
index f77ffc1..0000000
--- a/arch/arm/include/asm/localtimer.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- *  arch/arm/include/asm/localtimer.h
- *
- *  Copyright (C) 2004-2005 ARM Ltd.
- *
- * 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 __ASM_ARM_LOCALTIMER_H
-#define __ASM_ARM_LOCALTIMER_H
-
-#include <linux/errno.h>
-
-struct clock_event_device;
-
-struct local_timer_ops {
-	int  (*setup)(struct clock_event_device *);
-	void (*stop)(struct clock_event_device *);
-};
-
-#ifdef CONFIG_LOCAL_TIMERS
-/*
- * Register a local timer driver
- */
-int local_timer_register(struct local_timer_ops *);
-#else
-static inline int local_timer_register(struct local_timer_ops *ops)
-{
-	return -ENXIO;
-}
-#endif
-
-#endif
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 4de4d14..4c49138 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -41,7 +41,6 @@
 #include <asm/sections.h>
 #include <asm/tlbflush.h>
 #include <asm/ptrace.h>
-#include <asm/localtimer.h>
 #include <asm/smp_plat.h>
 #include <asm/virt.h>
 #include <asm/mach/arch.h>
@@ -133,8 +132,6 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-static void percpu_timer_stop(void);
-
 static int platform_cpu_kill(unsigned int cpu)
 {
 	if (smp_ops.cpu_kill)
@@ -178,11 +175,6 @@ int __cpuinit __cpu_disable(void)
 	migrate_irqs();
 
 	/*
-	 * Stop the local timer for this CPU.
-	 */
-	percpu_timer_stop();
-
-	/*
 	 * Flush user cache and TLB mappings, and then remove this CPU
 	 * from the vm mask set of all processes.
 	 *
@@ -269,8 +261,6 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid)
 	store_cpu_topology(cpuid);
 }
 
-static void percpu_timer_setup(void);
-
 /*
  * This is the secondary CPU boot entry.  We're using this CPUs
  * idle thread stack, but a set of temporary page tables.
@@ -325,11 +315,6 @@ asmlinkage void __cpuinit secondary_start_kernel(void)
 	set_cpu_online(cpu, true);
 	complete(&cpu_running);
 
-	/*
-	 * Setup the percpu timer for this CPU.
-	 */
-	percpu_timer_setup();
-
 	local_irq_enable();
 	local_fiq_enable();
 
@@ -376,12 +361,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 		max_cpus = ncores;
 	if (ncores > 1 && max_cpus) {
 		/*
-		 * Enable the local timer or broadcast device for the
-		 * boot CPU, but only if we have more than one CPU.
-		 */
-		percpu_timer_setup();
-
-		/*
 		 * Initialise the present map, which describes the set of CPUs
 		 * actually populated at the present time. A platform should
 		 * re-initialize the map in the platforms smp_prepare_cpus()
@@ -457,11 +436,6 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
 	return sum;
 }
 
-/*
- * Timer (local or broadcast) support
- */
-static DEFINE_PER_CPU(struct clock_event_device, percpu_clockevent);
-
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 void tick_broadcast(const struct cpumask *mask)
 {
@@ -469,49 +443,6 @@ void tick_broadcast(const struct cpumask *mask)
 }
 #endif
 
-static struct local_timer_ops *lt_ops;
-
-#ifdef CONFIG_LOCAL_TIMERS
-int local_timer_register(struct local_timer_ops *ops)
-{
-	if (!is_smp() || !setup_max_cpus)
-		return -ENXIO;
-
-	if (lt_ops)
-		return -EBUSY;
-
-	lt_ops = ops;
-	return 0;
-}
-#endif
-
-static void __cpuinit percpu_timer_setup(void)
-{
-	unsigned int cpu = smp_processor_id();
-	struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
-
-	evt->cpumask = cpumask_of(cpu);
-
-	if (lt_ops)
-		lt_ops->setup(evt);
-}
-
-#ifdef CONFIG_HOTPLUG_CPU
-/*
- * The generic clock events code purposely does not stop the local timer
- * on CPU_DEAD/CPU_DEAD_FROZEN hotplug events, so we have to do it
- * manually here.
- */
-static void percpu_timer_stop(void)
-{
-	unsigned int cpu = smp_processor_id();
-	struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
-
-	if (lt_ops)
-		lt_ops->stop(evt);
-}
-#endif
-
 static DEFINE_RAW_SPINLOCK(stop_lock);
 
 /*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning
  2013-03-13 18:17 ` [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning Stephen Boyd
@ 2013-03-20 17:06   ` Gregory CLEMENT
  0 siblings, 0 replies; 31+ messages in thread
From: Gregory CLEMENT @ 2013-03-20 17:06 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, Jason Cooper

On 03/13/2013 07:17 PM, Stephen Boyd wrote:
> drivers/clocksource/time-armada-370-xp.c:217:13: warning: symbol
> 'armada_370_xp_timer_init' was not declared. Should it be static?
> 
> Also remove the __init marking in the prototype as it's
> unnecessary and drop the init.h file.
> 
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/clocksource/time-armada-370-xp.c | 3 ++-
>  include/linux/time-armada-370-xp.h       | 4 +---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index 47a6730..efe4aef 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -27,10 +27,11 @@
>  #include <linux/of_address.h>
>  #include <linux/irq.h>
>  #include <linux/module.h>
> +#include <linux/percpu.h>
> +#include <linux/time-armada-370-xp.h>
>  
>  #include <asm/sched_clock.h>
>  #include <asm/localtimer.h>
> -#include <linux/percpu.h>
>  /*
>   * Timer block registers.
>   */
> diff --git a/include/linux/time-armada-370-xp.h b/include/linux/time-armada-370-xp.h
> index dfdfdc0..6fb0856 100644
> --- a/include/linux/time-armada-370-xp.h
> +++ b/include/linux/time-armada-370-xp.h
> @@ -11,8 +11,6 @@
>  #ifndef __TIME_ARMADA_370_XPPRCMU_H
>  #define __TIME_ARMADA_370_XPPRCMU_H
>  
> -#include <linux/init.h>
> -
> -void __init armada_370_xp_timer_init(void);
> +void armada_370_xp_timer_init(void);
>  
>  #endif
> 
Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-13 18:17 ` [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API Stephen Boyd
@ 2013-03-20 17:09   ` Gregory CLEMENT
  2013-03-20 17:20     ` Stephen Boyd
  2013-03-20 17:21     ` Gregory CLEMENT
  0 siblings, 2 replies; 31+ messages in thread
From: Gregory CLEMENT @ 2013-03-20 17:09 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-arm-kernel, linux-kernel, linux-arm-msm

On 03/13/2013 07:17 PM, Stephen Boyd wrote:
> Separate the armada 370xp local timers from the local timer API.
> This will allow us to remove ARM local timer support in the near
> future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on
linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores),
it crashed:

Booting Linux on physical CPU 0x0
Linux version 3.9.0-rc3-next-20130319-00010-g728b448 (gclement@FE-greg-laptop) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #153 SMP Wed Mar 20 17:58:57 CET 2013
CPU: ARMv7 Processor [562f5842] revision 2 (ARMv7), cr=10c53c7d
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
Machine: Marvell Armada 370/XP (Device Tree), model: PlatHome OpenBlocks AX3-4 board
bootconsole [earlycon0] enabled
Memory policy: ECC disabled, Data cache writealloc
PERCPU: Embedded 7 pages/cpu @c22b0000 s7104 r8192 d13376 u32768
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 784912
Kernel command line: console=ttyS0,115200 earlyprintk
PID hash table entries: 4096 (order: 2, 16384 bytes)
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
__ex_table already sorted, skipping sort
Memory: 3072MB = 3072MB total
Memory: 3113520k/3113520k available, 32208k reserved, 2367488K highmem
Virtual kernel memory layout:
    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    vmalloc : 0xf0000000 - 0xff000000   ( 240 MB)
    lowmem  : 0xc0000000 - 0xef800000   ( 760 MB)
    pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
    modules : 0xbf000000 - 0xbfe00000   (  14 MB)
      .text : 0xc0008000 - 0xc041647c   (4154 kB)
      .init : 0xc0417000 - 0xc0633bc0   (2163 kB)
      .data : 0xc0634000 - 0xc06605c0   ( 178 kB)
       .bss : 0xc06605c0 - 0xc0686a2c   ( 154 kB)
Hierarchical RCU implementation.
        RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
NR_IRQS:16 nr_irqs:16 16
Aurora cache controller enabled
l2x0: 16 ways, CACHE_ID 0x00000100, AUX_CTRL 0x1a696b12, Cache size: 1048576 B
sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 171798ms
Console: colour dummy device 80x30
Calibrating delay loop...
Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
sp : c0635eb8  ip : 00000000  fp : c063c3f0
r10: 000003ff  r9 : 00000000  r8 : 00000010
r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c53c7d  Table: 0000406a  DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)
Stack: (0xc0635eb8 to 0xc0636000)
5ea0:                                                       ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
[<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
[<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
[<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
[<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
Exception stack(0xc0635f18 to 0xc0635f60)
5f00:                                                       0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
[<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528)
[<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc)
[<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)
Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
---[ end trace 1b75b31a2719ed1c ]---
Kernel panic - not syncing: Fatal exception in interrupt
Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
Modules linked in:
CPU: 0    Tainted: G      D       (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
sp : c0635cc0  ip : 00000000  fp : c063c3f0
r10: 000003ff  r9 : 00000000  r8 : 00000010
r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c53c7d  Table: 0000406a  DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)
Stack: (0xc0635cc0 to 0xc0636000)
5cc0: ef004c80 c0063224 00000010 00000010 c0635f18 c0660ac0 c0635d20 c005fcb8
5ce0: c0632b90 c000ed94 c0316d54 60000153 00000001 c00085a8 c0316ca4 c0316d54
5d00: 60000153 ffffffff c0635d54 c03a53b0 00000000 c0660b70 600001d3 c000db60
5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0
5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54
5d60: 60000153 ffffffff 600001d3 c0635d94 c06608c0 c0634000 0000000b c06400e0
5d80: c03a53b0 00000000 c063f180 c0011624 c03a5310 200001d3 c0642f80 00010000
5da0: c0634238 0000000b 00100100 c0635e70 e92d45f0 7ed5a5f7 00000001 00000500
5dc0: c000dbcc c0634000 c063c3f0 c00082a0 00000006 c22b0768 00000004 00000000
5de0: 00030001 e92d45f0 ffffffff 00000001 c22b076c 00000000 c065fa1c c065f9c0
5e00: 00100100 00000002 c065f9c0 00000000 c063cca0 00000000 00000002 c065f9c0
5e20: c0661ca4 c06618e6 00000041 00000002 0000000a 00000002 ffffffff 00000000
5e40: 00000000 00000000 00000000 00000000 c06602cc 00000000 c00146d4 e92d45f4
5e60: 600001d3 c0634050 00000001 c000dbcc c0053e30 c22b3c40 fffffffe e92d45f0
5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0
5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
[<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
[<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
[<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
[<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
Exception stack(0xc0635d20 to 0xc0635d68)
5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0
5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54
5d60: 60000153 ffffffff
[<c000db60>] (__irq_svc+0x40/0x50) from [<c0316d54>] (panic+0x144/0x1b0)
[<c0316d54>] (panic+0x144/0x1b0) from [<c0011624>] (die+0x278/0x2a8)
[<c0011624>] (die+0x278/0x2a8) from [<c00082a0>] (do_undefinstr+0x9c/0x1c4)
[<c00082a0>] (do_undefinstr+0x9c/0x1c4) from [<c000dbcc>] (__und_svc_finish+0x0/0x14)
Exception stack(0xc0635e70 to 0xc0635eb8)
5e60:                                     c0053e30 c22b3c40 fffffffe e92d45f0
5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0
5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff
[<c000dbcc>] (__und_svc_finish+0x0/0x14) from [<e92d45f0>] (0xe92d45f0)
Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
---[ end trace 1b75b31a2719ed1d ]---


I am trying to figure out what happened.


> 
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/clocksource/time-armada-370-xp.c | 85 ++++++++++++++------------------
>  1 file changed, 38 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index efe4aef..ee2e50c5 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -19,6 +19,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/kernel.h>
>  #include <linux/clk.h>
> +#include <linux/cpu.h>
>  #include <linux/timer.h>
>  #include <linux/clockchips.h>
>  #include <linux/interrupt.h>
> @@ -31,7 +32,6 @@
>  #include <linux/time-armada-370-xp.h>
>  
>  #include <asm/sched_clock.h>
> -#include <asm/localtimer.h>
>  /*
>   * Timer block registers.
>   */
> @@ -70,7 +70,7 @@ static bool timer25Mhz = true;
>   */
>  static u32 ticks_per_jiffy;
>  
> -static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
> +static struct clock_event_device __percpu *armada_370_xp_evt;
>  
>  static u32 notrace armada_370_xp_read_sched_clock(void)
>  {
> @@ -143,14 +143,7 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode,
>  	}
>  }
>  
> -static struct clock_event_device armada_370_xp_clkevt = {
> -	.name		= "armada_370_xp_per_cpu_tick",
> -	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
> -	.shift		= 32,
> -	.rating		= 300,
> -	.set_next_event	= armada_370_xp_clkevt_next_event,
> -	.set_mode	= armada_370_xp_clkevt_mode,
> -};
> +static int armada_370_xp_clkevt_irq;
>  
>  static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
>  {
> @@ -173,42 +166,53 @@ static int __cpuinit armada_370_xp_timer_setup(struct clock_event_device *evt)
>  	u32 u;
>  	int cpu = smp_processor_id();
>  
> -	/* Use existing clock_event for cpu 0 */
> -	if (!smp_processor_id())
> -		return 0;
> -
>  	u = readl(local_base + TIMER_CTRL_OFF);
>  	if (timer25Mhz)
>  		writel(u | TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
>  	else
>  		writel(u & ~TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
>  
> -	evt->name		= armada_370_xp_clkevt.name;
> -	evt->irq		= armada_370_xp_clkevt.irq;
> -	evt->features		= armada_370_xp_clkevt.features;
> -	evt->shift		= armada_370_xp_clkevt.shift;
> -	evt->rating		= armada_370_xp_clkevt.rating,
> +	evt->name		= "armada_370_xp_per_cpu_tick",
> +	evt->features		= CLOCK_EVT_FEAT_ONESHOT |
> +				  CLOCK_EVT_FEAT_PERIODIC;
> +	evt->shift		= 32,
> +	evt->rating		= 300,
>  	evt->set_next_event	= armada_370_xp_clkevt_next_event,
>  	evt->set_mode		= armada_370_xp_clkevt_mode,
> +	evt->irq		= armada_370_xp_clkevt_irq;
>  	evt->cpumask		= cpumask_of(cpu);
>  
> -	*__this_cpu_ptr(percpu_armada_370_xp_evt) = evt;
> -
>  	clockevents_config_and_register(evt, timer_clk, 1, 0xfffffffe);
>  	enable_percpu_irq(evt->irq, 0);
>  
>  	return 0;
>  }
>  
> -static void  armada_370_xp_timer_stop(struct clock_event_device *evt)
> +static void __cpuinit armada_370_xp_timer_stop(struct clock_event_device *evt)
>  {
>  	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
>  	disable_percpu_irq(evt->irq);
>  }
>  
> -static struct local_timer_ops armada_370_xp_local_timer_ops __cpuinitdata = {
> -	.setup	= armada_370_xp_timer_setup,
> -	.stop	=  armada_370_xp_timer_stop,
> +static int __cpuinit armada_370_xp_timer_cpu_notify(struct notifier_block *self,
> +					   unsigned long action, void *hcpu)
> +{
> +	struct clock_event_device *evt = this_cpu_ptr(armada_370_xp_evt);
> +
> +	switch (action & ~CPU_TASKS_FROZEN) {
> +	case CPU_STARTING:
> +		armada_370_xp_timer_setup(evt);
> +		break;
> +	case CPU_DYING:
> +		armada_370_xp_timer_stop(evt);
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block armada_370_xp_timer_cpu_nb __cpuinitdata = {
> +	.notifier_call = armada_370_xp_timer_cpu_notify,
>  };
>  
>  void __init armada_370_xp_timer_init(void)
> @@ -224,9 +228,6 @@ void __init armada_370_xp_timer_init(void)
>  
>  	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
>  		/* The fixed 25MHz timer is available so let's use it */
> -		u = readl(local_base + TIMER_CTRL_OFF);
> -		writel(u | TIMER0_25MHZ,
> -		       local_base + TIMER_CTRL_OFF);
>  		u = readl(timer_base + TIMER_CTRL_OFF);
>  		writel(u | TIMER0_25MHZ,
>  		       timer_base + TIMER_CTRL_OFF);
> @@ -236,9 +237,6 @@ void __init armada_370_xp_timer_init(void)
>  		struct clk *clk = of_clk_get(np, 0);
>  		WARN_ON(IS_ERR(clk));
>  		rate =  clk_get_rate(clk);
> -		u = readl(local_base + TIMER_CTRL_OFF);
> -		writel(u & ~(TIMER0_25MHZ),
> -		       local_base + TIMER_CTRL_OFF);
>  
>  		u = readl(timer_base + TIMER_CTRL_OFF);
>  		writel(u & ~(TIMER0_25MHZ),
> @@ -252,7 +250,7 @@ void __init armada_370_xp_timer_init(void)
>  	 * We use timer 0 as clocksource, and private(local) timer 0
>  	 * for clockevents
>  	 */
> -	armada_370_xp_clkevt.irq = irq_of_parse_and_map(np, 4);
> +	armada_370_xp_clkevt_irq = irq_of_parse_and_map(np, 4);
>  
>  	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
>  
> @@ -277,26 +275,19 @@ void __init armada_370_xp_timer_init(void)
>  			      "armada_370_xp_clocksource",
>  			      timer_clk, 300, 32, clocksource_mmio_readl_down);
>  
> -	/* Register the clockevent on the private timer of CPU 0 */
> -	armada_370_xp_clkevt.cpumask = cpumask_of(0);
> -	clockevents_config_and_register(&armada_370_xp_clkevt,
> -					timer_clk, 1, 0xfffffffe);
> +	register_cpu_notifier(&armada_370_xp_timer_cpu_nb);
>  
> -	percpu_armada_370_xp_evt = alloc_percpu(struct clock_event_device *);
> +	armada_370_xp_evt = alloc_percpu(struct clock_event_device);
>  
>  
>  	/*
>  	 * Setup clockevent timer (interrupt-driven).
>  	 */
> -	*__this_cpu_ptr(percpu_armada_370_xp_evt) = &armada_370_xp_clkevt;
> -	res = request_percpu_irq(armada_370_xp_clkevt.irq,
> +	res = request_percpu_irq(armada_370_xp_clkevt_irq,
>  				armada_370_xp_timer_interrupt,
> -				armada_370_xp_clkevt.name,
> -				percpu_armada_370_xp_evt);
> -	if (!res) {
> -		enable_percpu_irq(armada_370_xp_clkevt.irq, 0);
> -#ifdef CONFIG_LOCAL_TIMERS
> -		local_timer_register(&armada_370_xp_local_timer_ops);
> -#endif
> -	}
> +				"armada_370_xp_per_cpu_tick",
> +				armada_370_xp_evt);
> +	/* Immediately configure the timer on the boot CPU */
> +	if (!res)
> +		armada_370_xp_timer_setup(this_cpu_ptr(armada_370_xp_evt));
>  }
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-20 17:09   ` Gregory CLEMENT
@ 2013-03-20 17:20     ` Stephen Boyd
  2013-03-20 17:26       ` Gregory CLEMENT
  2013-03-20 17:21     ` Gregory CLEMENT
  1 sibling, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-20 17:20 UTC (permalink / raw)
  To: Gregory CLEMENT; +Cc: linux-arm-kernel, linux-kernel, linux-arm-msm

On 03/20/13 10:09, Gregory CLEMENT wrote:
> On 03/13/2013 07:17 PM, Stephen Boyd wrote:
>> Separate the armada 370xp local timers from the local timer API.
>> This will allow us to remove ARM local timer support in the near
>> future and makes this driver multi-architecture friendly.
> At first view the code looks good, but when I applied your patch set on
> linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores),
> it crashed:
[...]
> Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> Modules linked in:
> CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
> PC is at 0xe92d45f0
> LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
> pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
> sp : c0635eb8  ip : 00000000  fp : c063c3f0
> r10: 000003ff  r9 : 00000000  r8 : 00000010
> r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
> r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> Control: 10c53c7d  Table: 0000406a  DAC: 00000015
> Process swapper/0 (pid: 0, stack limit = 0xc0634238)
> Stack: (0xc0635eb8 to 0xc0636000)
> 5ea0:                                                       ef004c80 c0063224
> 5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
> 5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
> 5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
> 5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
> 5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
> 5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
> 5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
> 5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
> [<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
> [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
> [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
> [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
> [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
> Exception stack(0xc0635f18 to 0xc0635f60)
> 5f00:                                                       0000001a ffff8ad0
> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
> [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528)
> [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc)
> [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)
> Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
>
>
>
> I am trying to figure out what happened.

Argh. Stupid casting again. Can you try this?

---8<----

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index ee2e50c5..a9bf37a 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
        /*
         * ACK timer interrupt and call event handler.
         */
-       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+       struct clock_event_device *evt = dev_id;
 
        writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
        evt->event_handler(evt);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-20 17:09   ` Gregory CLEMENT
  2013-03-20 17:20     ` Stephen Boyd
@ 2013-03-20 17:21     ` Gregory CLEMENT
  1 sibling, 0 replies; 31+ messages in thread
From: Gregory CLEMENT @ 2013-03-20 17:21 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-arm-msm, linux-kernel, linux-arm-kernel

On 03/20/2013 06:09 PM, Gregory CLEMENT wrote:
> On 03/13/2013 07:17 PM, Stephen Boyd wrote:
>> Separate the armada 370xp local timers from the local timer API.
>> This will allow us to remove ARM local timer support in the near
>> future and makes this driver multi-architecture friendly.
> 
> At first view the code looks good, but when I applied your patch set on
> linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores),
> it crashed:
> 
> Booting Linux on physical CPU 0x0
> Linux version 3.9.0-rc3-next-20130319-00010-g728b448 (gclement@FE-greg-laptop) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #153 SMP Wed Mar 20 17:58:57 CET 2013
> CPU: ARMv7 Processor [562f5842] revision 2 (ARMv7), cr=10c53c7d
> CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
> Machine: Marvell Armada 370/XP (Device Tree), model: PlatHome OpenBlocks AX3-4 board
> bootconsole [earlycon0] enabled
> Memory policy: ECC disabled, Data cache writealloc
> PERCPU: Embedded 7 pages/cpu @c22b0000 s7104 r8192 d13376 u32768
> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 784912
> Kernel command line: console=ttyS0,115200 earlyprintk
> PID hash table entries: 4096 (order: 2, 16384 bytes)
> Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
> Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
> __ex_table already sorted, skipping sort
> Memory: 3072MB = 3072MB total
> Memory: 3113520k/3113520k available, 32208k reserved, 2367488K highmem
> Virtual kernel memory layout:
>     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
>     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
>     vmalloc : 0xf0000000 - 0xff000000   ( 240 MB)
>     lowmem  : 0xc0000000 - 0xef800000   ( 760 MB)
>     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
>     modules : 0xbf000000 - 0xbfe00000   (  14 MB)
>       .text : 0xc0008000 - 0xc041647c   (4154 kB)
>       .init : 0xc0417000 - 0xc0633bc0   (2163 kB)
>       .data : 0xc0634000 - 0xc06605c0   ( 178 kB)
>        .bss : 0xc06605c0 - 0xc0686a2c   ( 154 kB)
> Hierarchical RCU implementation.
>         RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
> NR_IRQS:16 nr_irqs:16 16
> Aurora cache controller enabled
> l2x0: 16 ways, CACHE_ID 0x00000100, AUX_CTRL 0x1a696b12, Cache size: 1048576 B
> sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 171798ms
> Console: colour dummy device 80x30
> Calibrating delay loop...
> Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> Modules linked in:
> CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
> PC is at 0xe92d45f0
> LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
> pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
> sp : c0635eb8  ip : 00000000  fp : c063c3f0
> r10: 000003ff  r9 : 00000000  r8 : 00000010
> r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
> r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> Control: 10c53c7d  Table: 0000406a  DAC: 00000015
> Process swapper/0 (pid: 0, stack limit = 0xc0634238)
> Stack: (0xc0635eb8 to 0xc0636000)
> 5ea0:                                                       ef004c80 c0063224
> 5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
> 5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
> 5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
> 5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
> 5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
> 5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
> 5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
> 5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
> [<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
> [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
> [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
> [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
> [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
> Exception stack(0xc0635f18 to 0xc0635f60)
> 5f00:                                                       0000001a ffff8ad0
> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
> [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528)
> [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc)
> [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)
> Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
> ---[ end trace 1b75b31a2719ed1c ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
> Modules linked in:
> CPU: 0    Tainted: G      D       (3.9.0-rc3-next-20130319-00010-g728b448 #153)
> PC is at 0xe92d45f0
> LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
> pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
> sp : c0635cc0  ip : 00000000  fp : c063c3f0
> r10: 000003ff  r9 : 00000000  r8 : 00000010
> r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
> r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> Control: 10c53c7d  Table: 0000406a  DAC: 00000015
> Process swapper/0 (pid: 0, stack limit = 0xc0634238)
> Stack: (0xc0635cc0 to 0xc0636000)
> 5cc0: ef004c80 c0063224 00000010 00000010 c0635f18 c0660ac0 c0635d20 c005fcb8
> 5ce0: c0632b90 c000ed94 c0316d54 60000153 00000001 c00085a8 c0316ca4 c0316d54
> 5d00: 60000153 ffffffff c0635d54 c03a53b0 00000000 c0660b70 600001d3 c000db60
> 5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0
> 5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54
> 5d60: 60000153 ffffffff 600001d3 c0635d94 c06608c0 c0634000 0000000b c06400e0
> 5d80: c03a53b0 00000000 c063f180 c0011624 c03a5310 200001d3 c0642f80 00010000
> 5da0: c0634238 0000000b 00100100 c0635e70 e92d45f0 7ed5a5f7 00000001 00000500
> 5dc0: c000dbcc c0634000 c063c3f0 c00082a0 00000006 c22b0768 00000004 00000000
> 5de0: 00030001 e92d45f0 ffffffff 00000001 c22b076c 00000000 c065fa1c c065f9c0
> 5e00: 00100100 00000002 c065f9c0 00000000 c063cca0 00000000 00000002 c065f9c0
> 5e20: c0661ca4 c06618e6 00000041 00000002 0000000a 00000002 ffffffff 00000000
> 5e40: 00000000 00000000 00000000 00000000 c06602cc 00000000 c00146d4 e92d45f4
> 5e60: 600001d3 c0634050 00000001 c000dbcc c0053e30 c22b3c40 fffffffe e92d45f0
> 5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0
> 5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff ef004c80 c0063224
> 5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
> 5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
> 5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
> 5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
> 5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
> 5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
> 5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
> 5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
> [<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
> [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
> [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
> [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
> [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
> Exception stack(0xc0635d20 to 0xc0635d68)
> 5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0
> 5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54
> 5d60: 60000153 ffffffff
> [<c000db60>] (__irq_svc+0x40/0x50) from [<c0316d54>] (panic+0x144/0x1b0)
> [<c0316d54>] (panic+0x144/0x1b0) from [<c0011624>] (die+0x278/0x2a8)
> [<c0011624>] (die+0x278/0x2a8) from [<c00082a0>] (do_undefinstr+0x9c/0x1c4)
> [<c00082a0>] (do_undefinstr+0x9c/0x1c4) from [<c000dbcc>] (__und_svc_finish+0x0/0x14)
> Exception stack(0xc0635e70 to 0xc0635eb8)
> 5e60:                                     c0053e30 c22b3c40 fffffffe e92d45f0
> 5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0
> 5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff
> [<c000dbcc>] (__und_svc_finish+0x0/0x14) from [<e92d45f0>] (0xe92d45f0)
> Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
> ---[ end trace 1b75b31a2719ed1d ]---
> 
> 
> I am trying to figure out what happened.

Ok I found it, you forgot to also change the indirection in interrupt handler,
if you add the following chunk to your patch, it fixes the issue:


--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
        /*
         * ACK timer interrupt and call event handler.
         */
-       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+       struct clock_event_device *evt = (struct clock_event_device *)dev_id;

        writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
        evt->event_handler(evt);


> 
> 
>>
>> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  drivers/clocksource/time-armada-370-xp.c | 85 ++++++++++++++------------------
>>  1 file changed, 38 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
>> index efe4aef..ee2e50c5 100644
>> --- a/drivers/clocksource/time-armada-370-xp.c
>> +++ b/drivers/clocksource/time-armada-370-xp.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/kernel.h>
>>  #include <linux/clk.h>
>> +#include <linux/cpu.h>
>>  #include <linux/timer.h>
>>  #include <linux/clockchips.h>
>>  #include <linux/interrupt.h>
>> @@ -31,7 +32,6 @@
>>  #include <linux/time-armada-370-xp.h>
>>  
>>  #include <asm/sched_clock.h>
>> -#include <asm/localtimer.h>
>>  /*
>>   * Timer block registers.
>>   */
>> @@ -70,7 +70,7 @@ static bool timer25Mhz = true;
>>   */
>>  static u32 ticks_per_jiffy;
>>  
>> -static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
>> +static struct clock_event_device __percpu *armada_370_xp_evt;
>>  
>>  static u32 notrace armada_370_xp_read_sched_clock(void)
>>  {
>> @@ -143,14 +143,7 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode,
>>  	}
>>  }
>>  
>> -static struct clock_event_device armada_370_xp_clkevt = {
>> -	.name		= "armada_370_xp_per_cpu_tick",
>> -	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
>> -	.shift		= 32,
>> -	.rating		= 300,
>> -	.set_next_event	= armada_370_xp_clkevt_next_event,
>> -	.set_mode	= armada_370_xp_clkevt_mode,
>> -};
>> +static int armada_370_xp_clkevt_irq;
>>  
>>  static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
>>  {
>> @@ -173,42 +166,53 @@ static int __cpuinit armada_370_xp_timer_setup(struct clock_event_device *evt)
>>  	u32 u;
>>  	int cpu = smp_processor_id();
>>  
>> -	/* Use existing clock_event for cpu 0 */
>> -	if (!smp_processor_id())
>> -		return 0;
>> -
>>  	u = readl(local_base + TIMER_CTRL_OFF);
>>  	if (timer25Mhz)
>>  		writel(u | TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
>>  	else
>>  		writel(u & ~TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
>>  
>> -	evt->name		= armada_370_xp_clkevt.name;
>> -	evt->irq		= armada_370_xp_clkevt.irq;
>> -	evt->features		= armada_370_xp_clkevt.features;
>> -	evt->shift		= armada_370_xp_clkevt.shift;
>> -	evt->rating		= armada_370_xp_clkevt.rating,
>> +	evt->name		= "armada_370_xp_per_cpu_tick",
>> +	evt->features		= CLOCK_EVT_FEAT_ONESHOT |
>> +				  CLOCK_EVT_FEAT_PERIODIC;
>> +	evt->shift		= 32,
>> +	evt->rating		= 300,
>>  	evt->set_next_event	= armada_370_xp_clkevt_next_event,
>>  	evt->set_mode		= armada_370_xp_clkevt_mode,
>> +	evt->irq		= armada_370_xp_clkevt_irq;
>>  	evt->cpumask		= cpumask_of(cpu);
>>  
>> -	*__this_cpu_ptr(percpu_armada_370_xp_evt) = evt;
>> -
>>  	clockevents_config_and_register(evt, timer_clk, 1, 0xfffffffe);
>>  	enable_percpu_irq(evt->irq, 0);
>>  
>>  	return 0;
>>  }
>>  
>> -static void  armada_370_xp_timer_stop(struct clock_event_device *evt)
>> +static void __cpuinit armada_370_xp_timer_stop(struct clock_event_device *evt)
>>  {
>>  	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
>>  	disable_percpu_irq(evt->irq);
>>  }
>>  
>> -static struct local_timer_ops armada_370_xp_local_timer_ops __cpuinitdata = {
>> -	.setup	= armada_370_xp_timer_setup,
>> -	.stop	=  armada_370_xp_timer_stop,
>> +static int __cpuinit armada_370_xp_timer_cpu_notify(struct notifier_block *self,
>> +					   unsigned long action, void *hcpu)
>> +{
>> +	struct clock_event_device *evt = this_cpu_ptr(armada_370_xp_evt);
>> +
>> +	switch (action & ~CPU_TASKS_FROZEN) {
>> +	case CPU_STARTING:
>> +		armada_370_xp_timer_setup(evt);
>> +		break;
>> +	case CPU_DYING:
>> +		armada_370_xp_timer_stop(evt);
>> +		break;
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static struct notifier_block armada_370_xp_timer_cpu_nb __cpuinitdata = {
>> +	.notifier_call = armada_370_xp_timer_cpu_notify,
>>  };
>>  
>>  void __init armada_370_xp_timer_init(void)
>> @@ -224,9 +228,6 @@ void __init armada_370_xp_timer_init(void)
>>  
>>  	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
>>  		/* The fixed 25MHz timer is available so let's use it */
>> -		u = readl(local_base + TIMER_CTRL_OFF);
>> -		writel(u | TIMER0_25MHZ,
>> -		       local_base + TIMER_CTRL_OFF);
>>  		u = readl(timer_base + TIMER_CTRL_OFF);
>>  		writel(u | TIMER0_25MHZ,
>>  		       timer_base + TIMER_CTRL_OFF);
>> @@ -236,9 +237,6 @@ void __init armada_370_xp_timer_init(void)
>>  		struct clk *clk = of_clk_get(np, 0);
>>  		WARN_ON(IS_ERR(clk));
>>  		rate =  clk_get_rate(clk);
>> -		u = readl(local_base + TIMER_CTRL_OFF);
>> -		writel(u & ~(TIMER0_25MHZ),
>> -		       local_base + TIMER_CTRL_OFF);
>>  
>>  		u = readl(timer_base + TIMER_CTRL_OFF);
>>  		writel(u & ~(TIMER0_25MHZ),
>> @@ -252,7 +250,7 @@ void __init armada_370_xp_timer_init(void)
>>  	 * We use timer 0 as clocksource, and private(local) timer 0
>>  	 * for clockevents
>>  	 */
>> -	armada_370_xp_clkevt.irq = irq_of_parse_and_map(np, 4);
>> +	armada_370_xp_clkevt_irq = irq_of_parse_and_map(np, 4);
>>  
>>  	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
>>  
>> @@ -277,26 +275,19 @@ void __init armada_370_xp_timer_init(void)
>>  			      "armada_370_xp_clocksource",
>>  			      timer_clk, 300, 32, clocksource_mmio_readl_down);
>>  
>> -	/* Register the clockevent on the private timer of CPU 0 */
>> -	armada_370_xp_clkevt.cpumask = cpumask_of(0);
>> -	clockevents_config_and_register(&armada_370_xp_clkevt,
>> -					timer_clk, 1, 0xfffffffe);
>> +	register_cpu_notifier(&armada_370_xp_timer_cpu_nb);
>>  
>> -	percpu_armada_370_xp_evt = alloc_percpu(struct clock_event_device *);
>> +	armada_370_xp_evt = alloc_percpu(struct clock_event_device);
>>  
>>  
>>  	/*
>>  	 * Setup clockevent timer (interrupt-driven).
>>  	 */
>> -	*__this_cpu_ptr(percpu_armada_370_xp_evt) = &armada_370_xp_clkevt;
>> -	res = request_percpu_irq(armada_370_xp_clkevt.irq,
>> +	res = request_percpu_irq(armada_370_xp_clkevt_irq,
>>  				armada_370_xp_timer_interrupt,
>> -				armada_370_xp_clkevt.name,
>> -				percpu_armada_370_xp_evt);
>> -	if (!res) {
>> -		enable_percpu_irq(armada_370_xp_clkevt.irq, 0);
>> -#ifdef CONFIG_LOCAL_TIMERS
>> -		local_timer_register(&armada_370_xp_local_timer_ops);
>> -#endif
>> -	}
>> +				"armada_370_xp_per_cpu_tick",
>> +				armada_370_xp_evt);
>> +	/* Immediately configure the timer on the boot CPU */
>> +	if (!res)
>> +		armada_370_xp_timer_setup(this_cpu_ptr(armada_370_xp_evt));
>>  }
>>
> 
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-20 17:20     ` Stephen Boyd
@ 2013-03-20 17:26       ` Gregory CLEMENT
  2013-03-20 17:44         ` Gregory CLEMENT
  0 siblings, 1 reply; 31+ messages in thread
From: Gregory CLEMENT @ 2013-03-20 17:26 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-arm-kernel, linux-kernel, linux-arm-msm

On 03/20/2013 06:20 PM, Stephen Boyd wrote:
> On 03/20/13 10:09, Gregory CLEMENT wrote:
>> On 03/13/2013 07:17 PM, Stephen Boyd wrote:
>>> Separate the armada 370xp local timers from the local timer API.
>>> This will allow us to remove ARM local timer support in the near
>>> future and makes this driver multi-architecture friendly.
>> At first view the code looks good, but when I applied your patch set on
>> linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores),
>> it crashed:
> [...]
>> Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
>> Modules linked in:
>> CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
>> PC is at 0xe92d45f0
>> LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
>> pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
>> sp : c0635eb8  ip : 00000000  fp : c063c3f0
>> r10: 000003ff  r9 : 00000000  r8 : 00000010
>> r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
>> r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
>> Control: 10c53c7d  Table: 0000406a  DAC: 00000015
>> Process swapper/0 (pid: 0, stack limit = 0xc0634238)
>> Stack: (0xc0635eb8 to 0xc0636000)
>> 5ea0:                                                       ef004c80 c0063224
>> 5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
>> 5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
>> 5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
>> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
>> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
>> 5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
>> 5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
>> 5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
>> 5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
>> 5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
>> [<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
>> [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
>> [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
>> [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
>> [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
>> Exception stack(0xc0635f18 to 0xc0635f60)
>> 5f00:                                                       0000001a ffff8ad0
>> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
>> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
>> [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528)
>> [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc)
>> [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)
>> Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
>>
>>
>>
>> I am trying to figure out what happened.
> 
> Argh. Stupid casting again. Can you try this?

Our emails  must have crossed, your fix is also fine :)


> 
> ---8<----
> 
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index ee2e50c5..a9bf37a 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
>         /*
>          * ACK timer interrupt and call event handler.
>          */
> -       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
> +       struct clock_event_device *evt = dev_id;
>  
>         writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
>         evt->event_handler(evt);
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-20 17:26       ` Gregory CLEMENT
@ 2013-03-20 17:44         ` Gregory CLEMENT
  2013-03-20 18:00           ` Stephen Boyd
  0 siblings, 1 reply; 31+ messages in thread
From: Gregory CLEMENT @ 2013-03-20 17:44 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, Jason Cooper

On 03/20/2013 06:26 PM, Gregory CLEMENT wrote:
> On 03/20/2013 06:20 PM, Stephen Boyd wrote:
>> On 03/20/13 10:09, Gregory CLEMENT wrote:
>>> On 03/13/2013 07:17 PM, Stephen Boyd wrote:
>>>> Separate the armada 370xp local timers from the local timer API.
>>>> This will allow us to remove ARM local timer support in the near
>>>> future and makes this driver multi-architecture friendly.
>>> At first view the code looks good, but when I applied your patch set on
>>> linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores),
>>> it crashed:
>> [...]
>>> Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
>>> Modules linked in:
>>> CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
>>> PC is at 0xe92d45f0
>>> LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
>>> pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
>>> sp : c0635eb8  ip : 00000000  fp : c063c3f0
>>> r10: 000003ff  r9 : 00000000  r8 : 00000010
>>> r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
>>> r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
>>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
>>> Control: 10c53c7d  Table: 0000406a  DAC: 00000015
>>> Process swapper/0 (pid: 0, stack limit = 0xc0634238)
>>> Stack: (0xc0635eb8 to 0xc0636000)
>>> 5ea0:                                                       ef004c80 c0063224
>>> 5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
>>> 5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
>>> 5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
>>> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
>>> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
>>> 5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
>>> 5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
>>> 5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
>>> 5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
>>> 5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
>>> [<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
>>> [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
>>> [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
>>> [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
>>> [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
>>> Exception stack(0xc0635f18 to 0xc0635f60)
>>> 5f00:                                                       0000001a ffff8ad0
>>> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
>>> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
>>> [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528)
>>> [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc)
>>> [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)
>>> Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
>>>
>>>
>>>
>>> I am trying to figure out what happened.
>>
>> Argh. Stupid casting again. Can you try this?
> 
> Our emails  must have crossed, your fix is also fine :)
> 

I have also tested the patch set on a Armada 370 based board (Armada 370 DB),
and it works well.
So with this last chunk you can add my:
Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
and also my
tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


> 
>>
>> ---8<----
>>
>> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
>> index ee2e50c5..a9bf37a 100644
>> --- a/drivers/clocksource/time-armada-370-xp.c
>> +++ b/drivers/clocksource/time-armada-370-xp.c
>> @@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
>>         /*
>>          * ACK timer interrupt and call event handler.
>>          */
>> -       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
>> +       struct clock_event_device *evt = dev_id;
>>  
>>         writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
>>         evt->event_handler(evt);
>>
> 
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API
  2013-03-20 17:44         ` Gregory CLEMENT
@ 2013-03-20 18:00           ` Stephen Boyd
  0 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-03-20 18:00 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, Jason Cooper

On 03/20/13 10:44, Gregory CLEMENT wrote:
> On 03/20/2013 06:26 PM, Gregory CLEMENT wrote:
>> On 03/20/2013 06:20 PM, Stephen Boyd wrote:
>>> On 03/20/13 10:09, Gregory CLEMENT wrote:
>>>> On 03/13/2013 07:17 PM, Stephen Boyd wrote:
>>>>> Separate the armada 370xp local timers from the local timer API.
>>>>> This will allow us to remove ARM local timer support in the near
>>>>> future and makes this driver multi-architecture friendly.
>>>> At first view the code looks good, but when I applied your patch set on
>>>> linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores),
>>>> it crashed:
>>> [...]
>>>> Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
>>>> Modules linked in:
>>>> CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
>>>> PC is at 0xe92d45f0
>>>> LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
>>>> pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3
>>>> sp : c0635eb8  ip : 00000000  fp : c063c3f0
>>>> r10: 000003ff  r9 : 00000000  r8 : 00000010
>>>> r7 : c22b3c40  r6 : ef007c00  r5 : c0640fcc  r4 : c0053e30
>>>> r3 : e92d45f0  r2 : fffffffe  r1 : c22b3c40  r0 : c0053e30
>>>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
>>>> Control: 10c53c7d  Table: 0000406a  DAC: 00000015
>>>> Process swapper/0 (pid: 0, stack limit = 0xc0634238)
>>>> Stack: (0xc0635eb8 to 0xc0636000)
>>>> 5ea0:                                                       ef004c80 c0063224
>>>> 5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
>>>> 5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
>>>> 5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
>>>> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
>>>> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
>>>> 5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
>>>> 5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
>>>> 5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
>>>> 5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
>>>> 5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
>>>> [<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80)
>>>> [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30)
>>>> [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90)
>>>> [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0)
>>>> [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)
>>>> Exception stack(0xc0635f18 to 0xc0635f60)
>>>> 5f00:                                                       0000001a ffff8ad0
>>>> 5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
>>>> 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
>>>> [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528)
>>>> [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc)
>>>> [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)
>>>> Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
>>>>
>>>>
>>>>
>>>> I am trying to figure out what happened.
>>> Argh. Stupid casting again. Can you try this?
>> Our emails  must have crossed, your fix is also fine :)
>>
> I have also tested the patch set on a Armada 370 based board (Armada 370 DB),
> and it works well.
> So with this last chunk you can add my:
> Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> and also my
> tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>

Great. Thanks for testing.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-13 18:17 ` [PATCHv3 01/10] clocksource: add generic dummy timer driver Stephen Boyd
@ 2013-03-21 18:09   ` Mark Rutland
  2013-03-21 18:13     ` Stephen Boyd
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Rutland @ 2013-03-21 18:09 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner

Hi Stephen,

I've just been trying to test the dummy timer, and realised it's broken, as it
registers a cpu notifier from a device_initcall (after SMP's been brought up),
and doesn't ensure all active CPUs have been set up. Evidently no-one else has
attempted to test it thus far, and I'm not able to throughly test it at the
moment.

For now I'd recommend going back to the rearranging the existing arm
implementation as you had in v1. We seem to have ironed out the kinks with
having a simultaneous arch_timer and dummy, so I don't imagine we'll have major
issues taking that route.

Thanks,
Mark.

On Wed, Mar 13, 2013 at 06:17:47PM +0000, Stephen Boyd wrote:
> From: Mark Rutland <mark.rutland@arm.com>
> 
> Several architectures have a dummy timer driver tightly coupled with
> their broadcast code to support machines without cpu-local timers (or
> where there is a lack of driver support).
> 
> Since 12ad100046: "clockevents: Add generic timer broadcast function"
> it's been possible to write broadcast-capable timer drivers decoupled
> from the broadcast mechanism. We can use this functionality to implement
> a generic dummy timer driver that can be shared by all architectures
> with generic tick broadcast (ARCH_HAS_TICK_BROADCAST).
> 
> This patch implements a generic dummy timer using this facility.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> [sboyd: Make percpu data static and use __this_cpu_ptr()]
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/clocksource/Makefile      |  1 +
>  drivers/clocksource/dummy_timer.c | 67 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+)
>  create mode 100644 drivers/clocksource/dummy_timer.c
> 
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 4d8283a..655d718 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_VT8500_TIMER)	+= vt8500_timer.o
>  
>  obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>  obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
> +obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
> diff --git a/drivers/clocksource/dummy_timer.c b/drivers/clocksource/dummy_timer.c
> new file mode 100644
> index 0000000..7e1ce45
> --- /dev/null
> +++ b/drivers/clocksource/dummy_timer.c
> @@ -0,0 +1,67 @@
> +/*
> + *  linux/drivers/clocksource/dummy_timer.c
> + *
> + *  Copyright (C) 2013 ARM Ltd.
> + *  All Rights Reserved
> + *
> + * 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/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/init.h>
> +#include <linux/percpu.h>
> +
> +static DEFINE_PER_CPU(struct clock_event_device, dummy_timer_evt);
> +
> +static void dummy_timer_set_mode(enum clock_event_mode mode,
> +			   struct clock_event_device *evt)
> +{
> +	/*
> +	 * Core clockevents code will call this when exchanging timer devices.
> +	 * We don't need to do anything here.
> +	 */
> +}
> +
> +static void __cpuinit dummy_timer_setup(void)
> +{
> +	int cpu = smp_processor_id();
> +	struct clock_event_device *evt = __this_cpu_ptr(&dummy_timer_evt);
> +
> +	evt->name	= "dummy_timer";
> +	evt->features	= CLOCK_EVT_FEAT_PERIODIC |
> +			  CLOCK_EVT_FEAT_ONESHOT |
> +			  CLOCK_EVT_FEAT_DUMMY;
> +	evt->rating	= 100;
> +	evt->set_mode	= dummy_timer_set_mode;
> +	evt->cpumask	= cpumask_of(cpu);
> +
> +	clockevents_register_device(evt);
> +}
> +
> +static int __cpuinit dummy_timer_cpu_notify(struct notifier_block *self,
> +				      unsigned long action, void *hcpu)
> +{
> +	if ((action & ~CPU_TASKS_FROZEN) == CPU_STARTING)
> +		dummy_timer_setup();
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block dummy_timer_cpu_nb __cpuinitdata = {
> +	.notifier_call = dummy_timer_cpu_notify,
> +};
> +
> +static int __init dummy_timer_register(void)
> +{
> +	int err = register_cpu_notifier(&dummy_timer_cpu_nb);
> +	if (err)
> +		return err;
> +
> +	/* We won't get a call on the boot CPU, so register immediately */
> +	dummy_timer_setup();
> +
> +	return 0;
> +}
> +device_initcall(dummy_timer_register);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> 

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-21 18:09   ` Mark Rutland
@ 2013-03-21 18:13     ` Stephen Boyd
  2013-03-22 18:03       ` Mark Rutland
  0 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-21 18:13 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner

On 03/21/13 11:09, Mark Rutland wrote:
> Hi Stephen,
>
> I've just been trying to test the dummy timer, and realised it's broken, as it
> registers a cpu notifier from a device_initcall (after SMP's been brought up),
> and doesn't ensure all active CPUs have been set up. Evidently no-one else has
> attempted to test it thus far, and I'm not able to throughly test it at the
> moment.

Would it be sufficient to register as a pre-smp initcall?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-21 18:13     ` Stephen Boyd
@ 2013-03-22 18:03       ` Mark Rutland
  2013-03-25 16:49         ` Stephen Boyd
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Rutland @ 2013-03-22 18:03 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner

On Thu, Mar 21, 2013 at 06:13:17PM +0000, Stephen Boyd wrote:
> On 03/21/13 11:09, Mark Rutland wrote:
> > Hi Stephen,
> >
> > I've just been trying to test the dummy timer, and realised it's broken, as it
> > registers a cpu notifier from a device_initcall (after SMP's been brought up),
> > and doesn't ensure all active CPUs have been set up. Evidently no-one else has
> > attempted to test it thus far, and I'm not able to throughly test it at the
> > moment.
> 
> Would it be sufficient to register as a pre-smp initcall?

I've looked a bit further into the problem, and I believe using early_initcall
will make it work as well as the current arm-specific dummy timers work with a
rating of 100 (this means the recent patch lowering the rating broke tc2 as
explained below).

I've spent the last few hours trying to get the dummy_timer driver working on
tc2 with the sp804 as the broadcast source (with architected timer support
disabled). It turns out that having dummy timer's rating so low means that it
won't be selected as the tick device on cpu0 in preference to the sp804, and
thus won't push the sp804 out of that spot (allowing it to become the broadcast
source). This leads to boot stalling.

Jumping the dummy_timer's rating up would fix this, but that doesn't seem
great. Registering the dummy before all other clocks would also fix this (I
tried calling dummy_timer_register from time_init), but I can't see a way to do
that while keeping the driver self-contained.

Thanks,
Mark.

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-22 18:03       ` Mark Rutland
@ 2013-03-25 16:49         ` Stephen Boyd
  2013-03-25 18:00           ` Mark Rutland
  0 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-25 16:49 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner

On 03/22/13 11:03, Mark Rutland wrote:
> On Thu, Mar 21, 2013 at 06:13:17PM +0000, Stephen Boyd wrote:
>> On 03/21/13 11:09, Mark Rutland wrote:
>>> Hi Stephen,
>>>
>>> I've just been trying to test the dummy timer, and realised it's broken, as it
>>> registers a cpu notifier from a device_initcall (after SMP's been brought up),
>>> and doesn't ensure all active CPUs have been set up. Evidently no-one else has
>>> attempted to test it thus far, and I'm not able to throughly test it at the
>>> moment.
>> Would it be sufficient to register as a pre-smp initcall?
> I've looked a bit further into the problem, and I believe using early_initcall
> will make it work as well as the current arm-specific dummy timers work with a
> rating of 100 (this means the recent patch lowering the rating broke tc2 as
> explained below).

Yes, moving to early_initcall() should make the dummy timer equivalent
to the code that is already in the arm directory. It sounds like there
is a problem with mainline though?

>
> I've spent the last few hours trying to get the dummy_timer driver working on
> tc2 with the sp804 as the broadcast source (with architected timer support
> disabled). It turns out that having dummy timer's rating so low means that it
> won't be selected as the tick device on cpu0 in preference to the sp804, and
> thus won't push the sp804 out of that spot (allowing it to become the broadcast
> source). This leads to boot stalling.

I'm not following here. Why would we want to remove sp804 from the tick
duty?

>
> Jumping the dummy_timer's rating up would fix this, but that doesn't seem
> great. Registering the dummy before all other clocks would also fix this (I
> tried calling dummy_timer_register from time_init), but I can't see a way to do
> that while keeping the driver self-contained.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-25 16:49         ` Stephen Boyd
@ 2013-03-25 18:00           ` Mark Rutland
  2013-03-25 20:47             ` Thomas Gleixner
  2013-03-26  2:14             ` Stephen Boyd
  0 siblings, 2 replies; 31+ messages in thread
From: Mark Rutland @ 2013-03-25 18:00 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner, Santosh Shilimkar

On Mon, Mar 25, 2013 at 04:49:18PM +0000, Stephen Boyd wrote:
> On 03/22/13 11:03, Mark Rutland wrote:
> > On Thu, Mar 21, 2013 at 06:13:17PM +0000, Stephen Boyd wrote:
> >> On 03/21/13 11:09, Mark Rutland wrote:
> >>> Hi Stephen,
> >>>
> >>> I've just been trying to test the dummy timer, and realised it's broken, as it
> >>> registers a cpu notifier from a device_initcall (after SMP's been brought up),
> >>> and doesn't ensure all active CPUs have been set up. Evidently no-one else has
> >>> attempted to test it thus far, and I'm not able to throughly test it at the
> >>> moment.
> >> Would it be sufficient to register as a pre-smp initcall?
> > I've looked a bit further into the problem, and I believe using early_initcall
> > will make it work as well as the current arm-specific dummy timers work with a
> > rating of 100 (this means the recent patch lowering the rating broke tc2 as
> > explained below).
> 
> Yes, moving to early_initcall() should make the dummy timer equivalent
> to the code that is already in the arm directory. It sounds like there
> is a problem with mainline though?

Sorry, I rushed writing my earlier email in an attempt to get something on the
list, I should've spent some time making it clearer.

Using early_initcall will make it equivalent to the current code.

Unfortunately the existing code's current rating of 100 (as of Santosh's patch
[1]) breaks running an SMP system without local timers (at least using the
sp804 as a broadcast source on tc2 with architected timer support disabled).

It seems my patch making the timer core always reject dummy timers as broadcast
sources hasn't gotten to mainline yet, so we can't revert the patch without
breaking the platform Santosh noticed the problem on.

Thomas, do you have an idea of if/when tip/timers/urgent will hit mainline?

> 
> >
> > I've spent the last few hours trying to get the dummy_timer driver working on
> > tc2 with the sp804 as the broadcast source (with architected timer support
> > disabled). It turns out that having dummy timer's rating so low means that it
> > won't be selected as the tick device on cpu0 in preference to the sp804, and
> > thus won't push the sp804 out of that spot (allowing it to become the broadcast
> > source). This leads to boot stalling.
> 
> I'm not following here. Why would we want to remove sp804 from the tick
> duty?

To run an SMP system without local timers, we need the sp804 to be the
broadcast timer. Unfortunately the tick device and broadcast timer are mutually
exclusive positions, so we need to have a dummy timer knock the sp804 out of
tick device duty to enable broadcast.

When the dummy timer's rating was 400 (against the sp804's 350), this worked.
The sp804 would be registered, and would become cpu0's tick device. Later the
dummy would get registered, knocking the sp804 out (whereupon it would get
cycled back through tick_check_new_device and become the broadcast timer).

With the dummy timer's rating lower, the sp804 stays as cpu0's tick device, all
other cpus get dummy timers, but there's no broadcast source, so the system
locks up waiting for secondary cpus.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/154582.html

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-25 18:00           ` Mark Rutland
@ 2013-03-25 20:47             ` Thomas Gleixner
  2013-03-26 15:26               ` Mark Rutland
  2013-03-26  2:14             ` Stephen Boyd
  1 sibling, 1 reply; 31+ messages in thread
From: Thomas Gleixner @ 2013-03-25 20:47 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Stephen Boyd, linux-arm-kernel, linux-kernel, linux-arm-msm,
	John Stultz, Santosh Shilimkar

On Mon, 25 Mar 2013, Mark Rutland wrote:
> Thomas, do you have an idea of if/when tip/timers/urgent will hit mainline?

Thanks for the reminder. I just sent a pull request Linuswards.

Thanks,

	tglx

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-25 18:00           ` Mark Rutland
  2013-03-25 20:47             ` Thomas Gleixner
@ 2013-03-26  2:14             ` Stephen Boyd
  2013-03-26 11:28               ` Mark Rutland
  1 sibling, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-26  2:14 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner, Santosh Shilimkar

On 03/25/13 11:00, Mark Rutland wrote:
>
>>> I've spent the last few hours trying to get the dummy_timer driver working on
>>> tc2 with the sp804 as the broadcast source (with architected timer support
>>> disabled). It turns out that having dummy timer's rating so low means that it
>>> won't be selected as the tick device on cpu0 in preference to the sp804, and
>>> thus won't push the sp804 out of that spot (allowing it to become the broadcast
>>> source). This leads to boot stalling.
>> I'm not following here. Why would we want to remove sp804 from the tick
>> duty?
> To run an SMP system without local timers, we need the sp804 to be the
> broadcast timer. Unfortunately the tick device and broadcast timer are mutually
> exclusive positions, so we need to have a dummy timer knock the sp804 out of
> tick device duty to enable broadcast.
>
> When the dummy timer's rating was 400 (against the sp804's 350), this worked.
> The sp804 would be registered, and would become cpu0's tick device. Later the
> dummy would get registered, knocking the sp804 out (whereupon it would get
> cycled back through tick_check_new_device and become the broadcast timer).
>
> With the dummy timer's rating lower, the sp804 stays as cpu0's tick device, all
> other cpus get dummy timers, but there's no broadcast source, so the system
> locks up waiting for secondary cpus.

Ok. Thanks for clearing up my confusion.

Like you say, increasing the dummy timer rating seems like a hack. But
it also sounds like you want to keep the dummy timer driver fully self
contained. I'm not opposed to calling dummy_timer_register() at the
bottom of tick_init() if we have to, but it sounds like you don't like that.

An alternative would be to push the dummy timer logic into the core
clockevents layer under the ifdef for arch has broadcast. This is
probably the correct approach because most devices don't want to have a
dummy timer sitting around unused. I might be able to take a look at
this tomorrow.

One final question, if you remove all other CPUs besides the CPU that is
servicing the sp804 interrupt do we end up in a situation where the
sp804 is broadcasting to the dummy tick device? I haven't read through
all the code yet for that one. I would think tick_switch_to_oneshot()
would complain on your device?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-26  2:14             ` Stephen Boyd
@ 2013-03-26 11:28               ` Mark Rutland
  2013-04-05  1:46                 ` Stephen Boyd
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Rutland @ 2013-03-26 11:28 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner, Santosh Shilimkar

On Tue, Mar 26, 2013 at 02:14:53AM +0000, Stephen Boyd wrote:
> On 03/25/13 11:00, Mark Rutland wrote:
> >
> >>> I've spent the last few hours trying to get the dummy_timer driver working on
> >>> tc2 with the sp804 as the broadcast source (with architected timer support
> >>> disabled). It turns out that having dummy timer's rating so low means that it
> >>> won't be selected as the tick device on cpu0 in preference to the sp804, and
> >>> thus won't push the sp804 out of that spot (allowing it to become the broadcast
> >>> source). This leads to boot stalling.
> >> I'm not following here. Why would we want to remove sp804 from the tick
> >> duty?
> > To run an SMP system without local timers, we need the sp804 to be the
> > broadcast timer. Unfortunately the tick device and broadcast timer are mutually
> > exclusive positions, so we need to have a dummy timer knock the sp804 out of
> > tick device duty to enable broadcast.
> >
> > When the dummy timer's rating was 400 (against the sp804's 350), this worked.
> > The sp804 would be registered, and would become cpu0's tick device. Later the
> > dummy would get registered, knocking the sp804 out (whereupon it would get
> > cycled back through tick_check_new_device and become the broadcast timer).
> >
> > With the dummy timer's rating lower, the sp804 stays as cpu0's tick device, all
> > other cpus get dummy timers, but there's no broadcast source, so the system
> > locks up waiting for secondary cpus.
> 
> Ok. Thanks for clearing up my confusion.
> 
> Like you say, increasing the dummy timer rating seems like a hack. But
> it also sounds like you want to keep the dummy timer driver fully self
> contained. I'm not opposed to calling dummy_timer_register() at the
> bottom of tick_init() if we have to, but it sounds like you don't like that.

I'd like to keep the dummy timer driver self-contained if we can, but if it
makes it more robust and/or easier to deal with by having an external call to
register the driver, then I'm not opposed to that.

> 
> An alternative would be to push the dummy timer logic into the core
> clockevents layer under the ifdef for arch has broadcast. This is
> probably the correct approach because most devices don't want to have a
> dummy timer sitting around unused. I might be able to take a look at
> this tomorrow.

I'm also not opposed to this idea.

> 
> One final question, if you remove all other CPUs besides the CPU that is
> servicing the sp804 interrupt do we end up in a situation where the
> sp804 is broadcasting to the dummy tick device? I haven't read through
> all the code yet for that one. I would think tick_switch_to_oneshot()
> would complain on your device?

The current code in arch/arm/kernel/smp.c only registers the local timer on
cpu0 if we have more than one CPU, so the sp804 stays as cpu0's tick device.

With the generic dummy timer (with a rating bumped up to 400, using an
early_initcall, and the dummy timers rejected as broadcast sources [1]), even
when booting only with one cpu described in the dt the sp804 is relegated to
broadcast timer duty, broadcasting to the dummy timer on cpu0. Echoing 'q' to
/proc/sysrq-trigger gives me:

Tick Device: mode:     0
Broadcast device
Clock Event Device: v2m-timer0
 max_delta_ns:   4294966591000
 min_delta_ns:   14999
 mult:           2147484
 shift:          31
 mode:           2
 next_event:     9223372036854775807 nsecs
 set_next_event: sp804_set_next_event
 set_mode:       sp804_set_mode
 event_handler:  tick_handle_periodic_broadcast
 retries:        0
tick_broadcast_mask: 00000001
tick_broadcast_oneshot_mask: 00000000


Tick Device: mode:     0
Per CPU device: 0
Clock Event Device: dummy_timer
 max_delta_ns:   0
 min_delta_ns:   0
 mult:           0
 shift:          0
 mode:           1
 next_event:     9223372036854775807 nsecs
 set_next_event: <00000000>
 set_mode:       dummy_timer_set_mode
 event_handler:  tick_handle_periodic
 retries:        0

Though "broadcasting" to the same cpu is special-cased in tick_do_broadcast,
which will call the tick device's event_handler directly rather than having the
cpu attempt to IPI to itself.

As you suggest, tick_switch_to_oneshot does complain:

Clockevents: could not switch to one-shot mode: dummy_timer is not functional.
Could not switch to high resolution mode on CPU 0

To handle this case we could check cpu_possible_mask when initialising the
dummy and only register it if more than 1 cpu is possible. That would be
roughly in line with how we handle this case in smp.c.

Thanks,
Mark.

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a7dc19b8652c862d5b7c4d2339bd3c428bd29c4a

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-25 20:47             ` Thomas Gleixner
@ 2013-03-26 15:26               ` Mark Rutland
  0 siblings, 0 replies; 31+ messages in thread
From: Mark Rutland @ 2013-03-26 15:26 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-arm-msm, linux-kernel, John Stultz, Santosh Shilimkar,
	linux-arm-kernel

On Mon, Mar 25, 2013 at 08:47:42PM +0000, Thomas Gleixner wrote:
> On Mon, 25 Mar 2013, Mark Rutland wrote:
> > Thomas, do you have an idea of if/when tip/timers/urgent will hit mainline?
> 
> Thanks for the reminder. I just sent a pull request Linuswards.

Cheers!

Mark.

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

* Re: [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API
  2013-03-13 18:17 ` [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API Stephen Boyd
@ 2013-03-28 15:22   ` Mark Rutland
  2013-03-28 20:09     ` Stephen Boyd
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Rutland @ 2013-03-28 15:22 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Tony Lindgren, linux-arm-msm, Russell King,
	linux-kernel

This works on my A9x4 coretile, bringing CPUs up and down via
/sys/devices/system/cpu/*/online, so:

Tested-by: Mark Rutland <mark.rutland@arm.com>

Otherwise, is there any reason we couldn't now use the twd driver on a UP
system? Or would the overhead of handling frequency change make this pointless?

On Wed, Mar 13, 2013 at 06:17:49PM +0000, Stephen Boyd wrote:
> Separate the smp_twd timers from the local timer API. This will
> allow us to remove ARM local timer support in the near future and
> gets us closer to moving this driver to drivers/clocksource.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> 
> Changes since v2:
>  * Fix booting on qemu and omap
> 
>  arch/arm/Kconfig          |  2 +-
>  arch/arm/kernel/smp_twd.c | 64 +++++++++++++++++++++++++++++++----------------
>  2 files changed, 44 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5b71469..5ad2ccf 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1527,6 +1527,7 @@ config SMP
>  	depends on HAVE_SMP
>  	depends on MMU
>  	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
> +	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)

Could you not depend on your "Push selects for TWD/SCU into machine entries"
for this?

Mark.

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

* Re: [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API
  2013-03-28 15:22   ` Mark Rutland
@ 2013-03-28 20:09     ` Stephen Boyd
  2013-04-02  8:41       ` Mark Rutland
  0 siblings, 1 reply; 31+ messages in thread
From: Stephen Boyd @ 2013-03-28 20:09 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, Tony Lindgren, linux-arm-msm, Russell King,
	linux-kernel

On 03/28/13 08:22, Mark Rutland wrote:
> This works on my A9x4 coretile, bringing CPUs up and down via
> /sys/devices/system/cpu/*/online, so:
>
> Tested-by: Mark Rutland <mark.rutland@arm.com>

Thanks. I still need to resolve patch #1 though.

>
> Otherwise, is there any reason we couldn't now use the twd driver on a UP
> system? Or would the overhead of handling frequency change make this pointless?

I don't see why not but I don't have any interest in pursuing it.

>
> On Wed, Mar 13, 2013 at 06:17:49PM +0000, Stephen Boyd wrote:
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 5b71469..5ad2ccf 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1527,6 +1527,7 @@ config SMP
>>  	depends on HAVE_SMP
>>  	depends on MMU
>>  	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
>> +	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
> Could you not depend on your "Push selects for TWD/SCU into machine entries"
> for this?

Right now the patches don't depend on the push down patch. Are you
saying it would be better to depend on that patch?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API
  2013-03-28 20:09     ` Stephen Boyd
@ 2013-04-02  8:41       ` Mark Rutland
  0 siblings, 0 replies; 31+ messages in thread
From: Mark Rutland @ 2013-04-02  8:41 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Tony Lindgren, linux-arm-msm, Russell King,
	linux-kernel

On Thu, Mar 28, 2013 at 08:09:31PM +0000, Stephen Boyd wrote:
> On 03/28/13 08:22, Mark Rutland wrote:
> > This works on my A9x4 coretile, bringing CPUs up and down via
> > /sys/devices/system/cpu/*/online, so:
> >
> > Tested-by: Mark Rutland <mark.rutland@arm.com>
> 
> Thanks. I still need to resolve patch #1 though.
> 
> >
> > Otherwise, is there any reason we couldn't now use the twd driver on a UP
> > system? Or would the overhead of handling frequency change make this pointless?
> 
> I don't see why not but I don't have any interest in pursuing it.

Ok.

> 
> >
> > On Wed, Mar 13, 2013 at 06:17:49PM +0000, Stephen Boyd wrote:
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 5b71469..5ad2ccf 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -1527,6 +1527,7 @@ config SMP
> >>  	depends on HAVE_SMP
> >>  	depends on MMU
> >>  	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
> >> +	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
> > Could you not depend on your "Push selects for TWD/SCU into machine entries"
> > for this?
> 
> Right now the patches don't depend on the push down patch. Are you
> saying it would be better to depend on that patch?

It just seemed odd to me that the two series should conflict (though
trivially) here.

Mark.

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

* Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver
  2013-03-26 11:28               ` Mark Rutland
@ 2013-04-05  1:46                 ` Stephen Boyd
  0 siblings, 0 replies; 31+ messages in thread
From: Stephen Boyd @ 2013-04-05  1:46 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, John Stultz,
	Thomas Gleixner, Santosh Shilimkar

On 03/26/13 04:28, Mark Rutland wrote:
> On Tue, Mar 26, 2013 at 02:14:53AM +0000, Stephen Boyd wrote:
>> Ok. Thanks for clearing up my confusion.
>>
>> Like you say, increasing the dummy timer rating seems like a hack. But
>> it also sounds like you want to keep the dummy timer driver fully self
>> contained. I'm not opposed to calling dummy_timer_register() at the
>> bottom of tick_init() if we have to, but it sounds like you don't like that.
> I'd like to keep the dummy timer driver self-contained if we can, but if it
> makes it more robust and/or easier to deal with by having an external call to
> register the driver, then I'm not opposed to that.
>
>> An alternative would be to push the dummy timer logic into the core
>> clockevents layer under the ifdef for arch has broadcast. This is
>> probably the correct approach because most devices don't want to have a
>> dummy timer sitting around unused. I might be able to take a look at
>> this tomorrow.
> I'm also not opposed to this idea.

What if we only check the rating if the two devices are for the same
CPU, i.e. always prefer percpu devices over global ones? This would make
your dummy timers into tick devices.

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index b1600a6..9ea59b9 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -251,9 +251,10 @@ static int tick_check_new_device(struct clock_event_device *newdev)
                    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
                        goto out_bc;
                /*
-                * Check the rating
+                * Check the rating, but prefer CPU local devices
                 */
-               if (curdev->rating >= newdev->rating)
+               if (curdev->rating >= newdev->rating &&
+                   cpumask_equal(curdev->cpumask, newdev->cpumask))
                        goto out_bc;
        }
 

I think it will work with smp_twd, sp804, and dummy timers registered in
any order too.

> Though "broadcasting" to the same cpu is special-cased in tick_do_broadcast,
> which will call the tick device's event_handler directly rather than having the
> cpu attempt to IPI to itself.
>
> As you suggest, tick_switch_to_oneshot does complain:
>
> Clockevents: could not switch to one-shot mode: dummy_timer is not functional.
> Could not switch to high resolution mode on CPU 0
>
> To handle this case we could check cpu_possible_mask when initialising the
> dummy and only register it if more than 1 cpu is possible. That would be
> roughly in line with how we handle this case in smp.c.

Yes, this will work well enough. Squashed it into this patch.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

end of thread, other threads:[~2013-04-05  1:46 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-13 18:17 [PATCHv3 00/10] Remove ARM local timer API Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 01/10] clocksource: add generic dummy timer driver Stephen Boyd
2013-03-21 18:09   ` Mark Rutland
2013-03-21 18:13     ` Stephen Boyd
2013-03-22 18:03       ` Mark Rutland
2013-03-25 16:49         ` Stephen Boyd
2013-03-25 18:00           ` Mark Rutland
2013-03-25 20:47             ` Thomas Gleixner
2013-03-26 15:26               ` Mark Rutland
2013-03-26  2:14             ` Stephen Boyd
2013-03-26 11:28               ` Mark Rutland
2013-04-05  1:46                 ` Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 02/10] ARM: smp: Remove duplicate dummy timer implementation Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API Stephen Boyd
2013-03-28 15:22   ` Mark Rutland
2013-03-28 20:09     ` Stephen Boyd
2013-04-02  8:41       ` Mark Rutland
2013-03-13 18:17 ` [PATCHv3 04/10] ARM: OMAP2+: Divorce " Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 05/10] ARM: EXYNOS4: Divorce mct " Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 06/10] ARM: PRIMA2: Divorce timer-marco " Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 07/10] ARM: msm: Divorce msm_timer " Stephen Boyd
2013-03-13 18:17 ` [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning Stephen Boyd
2013-03-20 17:06   ` Gregory CLEMENT
2013-03-13 18:17 ` [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API Stephen Boyd
2013-03-20 17:09   ` Gregory CLEMENT
2013-03-20 17:20     ` Stephen Boyd
2013-03-20 17:26       ` Gregory CLEMENT
2013-03-20 17:44         ` Gregory CLEMENT
2013-03-20 18:00           ` Stephen Boyd
2013-03-20 17:21     ` Gregory CLEMENT
2013-03-13 18:17 ` [PATCHv3 10/10] ARM: smp: Remove " Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).