[PATCH 0/8] MSM timer fixes and cleanups

[PATCH 0/8] MSM timer fixes and cleanups

[Stephen Boyd]

Currently the MSM timers use the same physical counter
for the clockevent and clocksource. This works as long
as the clocksource isn't stopped from ticking during normal
operation but unfortunately that isn't the case and the
clocksource is stopped when the clockevent is shutdown.
Even worse, switching the clocksource via sysfs at runtime will
hang the system.

This series reorganizes the MSM timer code so that one counter
is only used for either a clocksource or a clockevent, and not 
both. In the process we reduce the lines of code and fix
a few long-standing bugs.

I plan to add sched_clock support on top of this series once the
ARM generic patches are posted again (hopefully next week).

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>

Stephen Boyd (8):
  msm: timer: Tighten #ifdef for local timer support
  msm: timer: Cleanup #includes and #defines
  msm: timer: Use GPT for clockevents and DGT for clocksource
  msm: timer: Fix ONESHOT mode interrupts
  msm: timer: Remove msm_clocks[] and simplify code
  msm: timer: Remove SoC specific #ifdefs
  msm: timer: Setup interrupt after registering clockevent
  msm: timer: Use clockevents_config_and_register()

 arch/arm/mach-msm/timer.c |  347 ++++++++++++++++-----------------------------
 1 files changed, 124 insertions(+), 223 deletions(-)

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 1/8] msm: timer: Tighten #ifdef for local timer support

[Stephen Boyd]

It is more correct to only define the local timer support code
when CONFIG_LOCAL_TIMERS=y.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-msm/timer.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index afeeca5..1cab559 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -279,7 +279,7 @@ static void __init msm_timer_init(void)
 	}
 }
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_LOCAL_TIMERS
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
 	static bool local_timer_inited;
@@ -321,8 +321,7 @@ void local_timer_stop(struct clock_event_device *evt)
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
-
-#endif
+#endif /* CONFIG_LOCAL_TIMERS */
 
 struct sys_timer msm_timer = {
 	.init = msm_timer_init
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 2/8] msm: timer: Cleanup #includes and #defines

[Stephen Boyd]

Remove unused/unnecessary #defines, #includes, and use the BIT
macro appropriately.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-msm/timer.c |   26 +++++++-------------------
 1 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 1cab559..ce389ca 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -13,44 +13,32 @@
  *
  */
 
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
 #include <linux/init.h>
-#include <linux/time.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/clk.h>
-#include <linux/clockchips.h>
-#include <linux/delay.h>
 #include <linux/io.h>
 
 #include <asm/mach/time.h>
 #include <asm/hardware/gic.h>
+#include <asm/localtimer.h>
 
 #include <mach/msm_iomap.h>
 #include <mach/cpu.h>
+#include <mach/board.h>
 
 #define TIMER_MATCH_VAL         0x0000
 #define TIMER_COUNT_VAL         0x0004
 #define TIMER_ENABLE            0x0008
-#define TIMER_ENABLE_CLR_ON_MATCH_EN    2
-#define TIMER_ENABLE_EN                 1
+#define TIMER_ENABLE_CLR_ON_MATCH_EN    BIT(1)
+#define TIMER_ENABLE_EN                 BIT(0)
 #define TIMER_CLEAR             0x000C
 #define DGT_CLK_CTL             0x0034
-enum {
-	DGT_CLK_CTL_DIV_1 = 0,
-	DGT_CLK_CTL_DIV_2 = 1,
-	DGT_CLK_CTL_DIV_3 = 2,
-	DGT_CLK_CTL_DIV_4 = 3,
-};
-#define CSR_PROTECTION          0x0020
-#define CSR_PROTECTION_EN               1
+#define DGT_CLK_CTL_DIV_4	0x3
 
 #define GPT_HZ 32768
 
-enum timer_location {
-	LOCAL_TIMER = 0,
-	GLOBAL_TIMER = 1,
-};
-
 #define MSM_GLOBAL_TIMER MSM_CLOCK_DGT
 
 /* TODO: Remove these ifdefs */
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 3/8] msm: timer: Use GPT for clockevents and DGT for clocksource

[Stephen Boyd]

The clocksource shouldn't stop ticking when the clockevent stops.
This is exactly what happens today with MSM timers. The same
hardware is used for both the clockevent and the clocksource
because the ratings of the two are the same.

Fix this by registering a clockevent based on the GPT and a
clocksource based on the DGT. This removes any other possible
configuration (e.g. a GPT clocksource and a DGT clockevent) but
that shouldn't be a big issue since we want higher precision
timing than high precision scheduling interrupts.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/timer.c |  121 +++++++++++++++++++-------------------------
 1 files changed, 52 insertions(+), 69 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index ce389ca..405e8a9 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -1,6 +1,7 @@
-/* linux/arch/arm/mach-msm/timer.c
+/*
  *
  * Copyright (C) 2007 Google, Inc.
+ * Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -39,7 +40,7 @@
 
 #define GPT_HZ 32768
 
-#define MSM_GLOBAL_TIMER MSM_CLOCK_DGT
+#define MSM_GLOBAL_TIMER MSM_CLOCK_GPT
 
 /* TODO: Remove these ifdefs */
 #if defined(CONFIG_ARCH_QSD8X50)
@@ -153,25 +154,10 @@ static struct msm_clock msm_clocks[] = {
 			.set_next_event = msm_timer_set_next_event,
 			.set_mode       = msm_timer_set_mode,
 		},
-		.clocksource = {
-			.name           = "gp_timer",
-			.rating         = 200,
-			.read           = msm_read_timer_count,
-			.mask           = CLOCKSOURCE_MASK(32),
-			.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
-		},
 		.irq = INT_GP_TIMER_EXP,
 		.freq = GPT_HZ,
 	},
 	[MSM_CLOCK_DGT] = {
-		.clockevent = {
-			.name           = "dg_timer",
-			.features       = CLOCK_EVT_FEAT_ONESHOT,
-			.shift          = 32 + MSM_DGT_SHIFT,
-			.rating         = 300,
-			.set_next_event = msm_timer_set_next_event,
-			.set_mode       = msm_timer_set_mode,
-		},
 		.clocksource = {
 			.name           = "dg_timer",
 			.rating         = 300,
@@ -179,7 +165,6 @@ static struct msm_clock msm_clocks[] = {
 			.mask           = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
 			.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 		},
-		.irq = INT_DEBUG_TIMER_EXP,
 		.freq = DGT_HZ >> MSM_DGT_SHIFT,
 		.shift = MSM_DGT_SHIFT,
 	}
@@ -187,10 +172,13 @@ static struct msm_clock msm_clocks[] = {
 
 static void __init msm_timer_init(void)
 {
-	int i;
+	struct msm_clock *clock;
+	struct clock_event_device *ce = &msm_clocks[MSM_CLOCK_GPT].clockevent;
+	struct clocksource *cs = &msm_clocks[MSM_CLOCK_DGT].clocksource;
 	int res;
 	int global_offset = 0;
 
+
 	if (cpu_is_msm7x01()) {
 		msm_clocks[MSM_CLOCK_GPT].regbase = MSM_CSR_BASE;
 		msm_clocks[MSM_CLOCK_DGT].regbase = MSM_CSR_BASE + 0x10;
@@ -213,58 +201,55 @@ static void __init msm_timer_init(void)
 	writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
 #endif
 
-	for (i = 0; i < ARRAY_SIZE(msm_clocks); i++) {
-		struct msm_clock *clock = &msm_clocks[i];
-		struct clock_event_device *ce = &clock->clockevent;
-		struct clocksource *cs = &clock->clocksource;
-
-		clock->local_counter = clock->regbase + TIMER_COUNT_VAL;
-		clock->global_counter = clock->local_counter + global_offset;
-
-		writel(0, clock->regbase + TIMER_ENABLE);
-		writel(0, clock->regbase + TIMER_CLEAR);
-		writel(~0, clock->regbase + TIMER_MATCH_VAL);
+	clock = &msm_clocks[MSM_CLOCK_GPT];
+	clock->local_counter = clock->regbase + TIMER_COUNT_VAL;
 
-		ce->mult = div_sc(clock->freq, NSEC_PER_SEC, ce->shift);
-		/* allow at least 10 seconds to notice that the timer wrapped */
-		ce->max_delta_ns =
-			clockevent_delta2ns(0xf0000000 >> clock->shift, ce);
-		/* 4 gets rounded down to 3 */
-		ce->min_delta_ns = clockevent_delta2ns(4, ce);
-		ce->cpumask = cpumask_of(0);
-
-		res = clocksource_register_hz(cs, clock->freq);
-		if (res)
-			printk(KERN_ERR "msm_timer_init: clocksource_register "
-			       "failed for %s\n", cs->name);
-
-		ce->irq = clock->irq;
-		if (cpu_is_msm8x60() || cpu_is_msm8960()) {
-			clock->percpu_evt = alloc_percpu(struct clock_event_device *);
-			if (!clock->percpu_evt) {
-				pr_err("msm_timer_init: memory allocation "
-				       "failed for %s\n", ce->name);
-				continue;
-			}
-
-			*__this_cpu_ptr(clock->percpu_evt) = ce;
-			res = request_percpu_irq(ce->irq, msm_timer_interrupt,
-						 ce->name, clock->percpu_evt);
-			if (!res)
-				enable_percpu_irq(ce->irq, 0);
-		} else {
-			clock->evt = ce;
-			res = request_irq(ce->irq, msm_timer_interrupt,
-					  IRQF_TIMER | IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
-					  ce->name, &clock->evt);
+	writel_relaxed(0, clock->regbase + TIMER_ENABLE);
+	writel_relaxed(0, clock->regbase + TIMER_CLEAR);
+	writel_relaxed(~0, clock->regbase + TIMER_MATCH_VAL);
+	ce->mult = div_sc(clock->freq, NSEC_PER_SEC, ce->shift);
+	/*
+	 * allow at least 10 seconds to notice that the timer
+	 * wrapped
+	 */
+	ce->max_delta_ns =
+		clockevent_delta2ns(0xf0000000 >> clock->shift, ce);
+	/* 4 gets rounded down to 3 */
+	ce->min_delta_ns = clockevent_delta2ns(4, ce);
+	ce->cpumask = cpumask_of(0);
+
+	ce->irq = clock->irq;
+	if (cpu_is_msm8x60() || cpu_is_msm8960()) {
+		clock->percpu_evt = alloc_percpu(struct clock_event_device *);
+		if (!clock->percpu_evt) {
+			pr_err("memory allocation failed for %s\n", ce->name);
+			goto err;
 		}
 
-		if (res)
-			pr_err("msm_timer_init: request_irq failed for %s\n",
-			       ce->name);
-
-		clockevents_register_device(ce);
+		*__this_cpu_ptr(clock->percpu_evt) = ce;
+		res = request_percpu_irq(ce->irq, msm_timer_interrupt,
+					 ce->name, clock->percpu_evt);
+		if (!res)
+			enable_percpu_irq(ce->irq, 0);
+	} else {
+		clock->evt = ce;
+		res = request_irq(ce->irq, msm_timer_interrupt,
+				  IRQF_TIMER | IRQF_NOBALANCING |
+				  IRQF_TRIGGER_RISING, ce->name, &clock->evt);
 	}
+
+	if (res)
+		pr_err("request_irq failed for %s\n", ce->name);
+
+	clockevents_register_device(ce);
+err:
+	clock = &msm_clocks[MSM_CLOCK_DGT];
+	clock->local_counter = clock->regbase + TIMER_COUNT_VAL;
+	clock->global_counter = clock->local_counter + global_offset;
+	writel_relaxed(TIMER_ENABLE_EN, clock->regbase + TIMER_ENABLE);
+	res = clocksource_register_hz(cs, clock->freq);
+	if (res)
+		pr_err("clocksource_register failed for %s\n", cs->name);
 }
 
 #ifdef CONFIG_LOCAL_TIMERS
@@ -277,8 +262,6 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 	if (!smp_processor_id())
 		return 0;
 
-	writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
-
 	if (!local_timer_inited) {
 		writel(0, clock->regbase  + TIMER_ENABLE);
 		writel(0, clock->regbase + TIMER_CLEAR);
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 4/8] msm: timer: Fix ONESHOT mode interrupts

[Stephen Boyd]

MSM timers don't support an interrupt enable/disable bit.
Therefore, when the timer is free running it's possible for the
count to wrap and the match value to match again even though a
set_next_event() call hasn't been made since the last match.

Workaround the lack of an interrupt enable bit by explicitly
stopping the timer in the interrupt handler when the clockevent
is in ONESHOT mode. This should prevent any possibility of the
timer wrapping and matching more than once per set_next_event().

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/timer.c |   25 ++++++++++++++++++++-----
 1 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 405e8a9..9f3671a 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -81,11 +81,20 @@ enum {
 
 static struct msm_clock msm_clocks[];
 
+static struct msm_clock *clockevent_to_clock(struct clock_event_device *evt);
+
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
 	if (evt->event_handler == NULL)
 		return IRQ_HANDLED;
+	/* Stop the timer tick */
+	if (evt->mode == CLOCK_EVT_MODE_ONESHOT) {
+		struct msm_clock *clock = clockevent_to_clock(evt);
+		u32 ctrl = readl_relaxed(clock->regbase + TIMER_ENABLE);
+		ctrl &= ~TIMER_ENABLE_EN;
+		writel_relaxed(ctrl, clock->regbase + TIMER_ENABLE);
+	}
 	evt->event_handler(evt);
 	return IRQ_HANDLED;
 }
@@ -118,10 +127,12 @@ static int msm_timer_set_next_event(unsigned long cycles,
 				    struct clock_event_device *evt)
 {
 	struct msm_clock *clock = clockevent_to_clock(evt);
-	uint32_t now = readl(clock->local_counter);
-	uint32_t alarm = now + (cycles << clock->shift);
+	u32 match = cycles << clock->shift;
+	u32 ctrl = readl_relaxed(clock->regbase + TIMER_ENABLE);
 
-	writel(alarm, clock->regbase + TIMER_MATCH_VAL);
+	writel_relaxed(0, clock->regbase + TIMER_CLEAR);
+	writel_relaxed(match, clock->regbase + TIMER_MATCH_VAL);
+	writel_relaxed(ctrl | TIMER_ENABLE_EN, clock->regbase + TIMER_ENABLE);
 	return 0;
 }
 
@@ -129,19 +140,23 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
 			      struct clock_event_device *evt)
 {
 	struct msm_clock *clock = clockevent_to_clock(evt);
+	u32 ctrl;
+
+	ctrl = readl_relaxed(clock->regbase + TIMER_ENABLE);
+	ctrl &= ~(TIMER_ENABLE_EN | TIMER_ENABLE_CLR_ON_MATCH_EN);
 
 	switch (mode) {
 	case CLOCK_EVT_MODE_RESUME:
 	case CLOCK_EVT_MODE_PERIODIC:
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
-		writel(TIMER_ENABLE_EN, clock->regbase + TIMER_ENABLE);
+		/* Timer is enabled in set_next_event */
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
-		writel(0, clock->regbase + TIMER_ENABLE);
 		break;
 	}
+	writel_relaxed(ctrl, clock->regbase + TIMER_ENABLE);
 }
 
 static struct msm_clock msm_clocks[] = {
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 5/8] msm: timer: Remove msm_clocks[] and simplify code

[Stephen Boyd]

We can simplify the timer code now that we only use the DGT for
the clocksource and the GPT for the clockevent. Get rid of the
msm_clocks[] array and propagate the changes throughout the code.
This reduces the lines of code in this file and improves
readability.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/timer.c |  221 ++++++++++++++++-----------------------------
 1 files changed, 76 insertions(+), 145 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 9f3671a..fc06464 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -40,8 +40,6 @@
 
 #define GPT_HZ 32768
 
-#define MSM_GLOBAL_TIMER MSM_CLOCK_GPT
-
 /* TODO: Remove these ifdefs */
 #if defined(CONFIG_ARCH_QSD8X50)
 #define DGT_HZ (19200000 / 4) /* 19.2 MHz / 4 by default */
@@ -57,31 +55,7 @@
 #define MSM_DGT_SHIFT (5)
 #endif
 
-struct msm_clock {
-	struct clock_event_device   clockevent;
-	struct clocksource          clocksource;
-	unsigned int		    irq;
-	void __iomem                *regbase;
-	uint32_t                    freq;
-	uint32_t                    shift;
-	void __iomem                *global_counter;
-	void __iomem                *local_counter;
-	union {
-		struct clock_event_device		*evt;
-		struct clock_event_device __percpu	**percpu_evt;
-	};		
-};
-
-enum {
-	MSM_CLOCK_GPT,
-	MSM_CLOCK_DGT,
-	NR_TIMERS,
-};
-
-
-static struct msm_clock msm_clocks[];
-
-static struct msm_clock *clockevent_to_clock(struct clock_event_device *evt);
+static void __iomem *event_base;
 
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
@@ -90,59 +64,31 @@ static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 		return IRQ_HANDLED;
 	/* Stop the timer tick */
 	if (evt->mode == CLOCK_EVT_MODE_ONESHOT) {
-		struct msm_clock *clock = clockevent_to_clock(evt);
-		u32 ctrl = readl_relaxed(clock->regbase + TIMER_ENABLE);
+		u32 ctrl = readl_relaxed(event_base + TIMER_ENABLE);
 		ctrl &= ~TIMER_ENABLE_EN;
-		writel_relaxed(ctrl, clock->regbase + TIMER_ENABLE);
+		writel_relaxed(ctrl, event_base + TIMER_ENABLE);
 	}
 	evt->event_handler(evt);
 	return IRQ_HANDLED;
 }
 
-static cycle_t msm_read_timer_count(struct clocksource *cs)
-{
-	struct msm_clock *clk = container_of(cs, struct msm_clock, clocksource);
-
-	/*
-	 * Shift timer count down by a constant due to unreliable lower bits
-	 * on some targets.
-	 */
-	return readl(clk->global_counter) >> clk->shift;
-}
-
-static struct msm_clock *clockevent_to_clock(struct clock_event_device *evt)
-{
-#ifdef CONFIG_SMP
-	int i;
-	for (i = 0; i < NR_TIMERS; i++)
-		if (evt == &(msm_clocks[i].clockevent))
-			return &msm_clocks[i];
-	return &msm_clocks[MSM_GLOBAL_TIMER];
-#else
-	return container_of(evt, struct msm_clock, clockevent);
-#endif
-}
-
 static int msm_timer_set_next_event(unsigned long cycles,
 				    struct clock_event_device *evt)
 {
-	struct msm_clock *clock = clockevent_to_clock(evt);
-	u32 match = cycles << clock->shift;
-	u32 ctrl = readl_relaxed(clock->regbase + TIMER_ENABLE);
+	u32 ctrl = readl_relaxed(event_base + TIMER_ENABLE);
 
-	writel_relaxed(0, clock->regbase + TIMER_CLEAR);
-	writel_relaxed(match, clock->regbase + TIMER_MATCH_VAL);
-	writel_relaxed(ctrl | TIMER_ENABLE_EN, clock->regbase + TIMER_ENABLE);
+	writel_relaxed(0, event_base + TIMER_CLEAR);
+	writel_relaxed(cycles, event_base + TIMER_MATCH_VAL);
+	writel_relaxed(ctrl | TIMER_ENABLE_EN, event_base + TIMER_ENABLE);
 	return 0;
 }
 
 static void msm_timer_set_mode(enum clock_event_mode mode,
 			      struct clock_event_device *evt)
 {
-	struct msm_clock *clock = clockevent_to_clock(evt);
 	u32 ctrl;
 
-	ctrl = readl_relaxed(clock->regbase + TIMER_ENABLE);
+	ctrl = readl_relaxed(event_base + TIMER_ENABLE);
 	ctrl &= ~(TIMER_ENABLE_EN | TIMER_ENABLE_CLR_ON_MATCH_EN);
 
 	switch (mode) {
@@ -156,59 +102,61 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
 	case CLOCK_EVT_MODE_SHUTDOWN:
 		break;
 	}
-	writel_relaxed(ctrl, clock->regbase + TIMER_ENABLE);
+	writel_relaxed(ctrl, event_base + TIMER_ENABLE);
 }
 
-static struct msm_clock msm_clocks[] = {
-	[MSM_CLOCK_GPT] = {
-		.clockevent = {
-			.name           = "gp_timer",
-			.features       = CLOCK_EVT_FEAT_ONESHOT,
-			.shift          = 32,
-			.rating         = 200,
-			.set_next_event = msm_timer_set_next_event,
-			.set_mode       = msm_timer_set_mode,
-		},
-		.irq = INT_GP_TIMER_EXP,
-		.freq = GPT_HZ,
-	},
-	[MSM_CLOCK_DGT] = {
-		.clocksource = {
-			.name           = "dg_timer",
-			.rating         = 300,
-			.read           = msm_read_timer_count,
-			.mask           = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
-			.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
-		},
-		.freq = DGT_HZ >> MSM_DGT_SHIFT,
-		.shift = MSM_DGT_SHIFT,
-	}
+static struct clock_event_device msm_clockevent = {
+	.name		= "gp_timer",
+	.features	= CLOCK_EVT_FEAT_ONESHOT,
+	.shift		= 32,
+	.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 void __iomem *source_base;
+
+static cycle_t msm_read_timer_count(struct clocksource *cs)
+{
+	/*
+	 * Shift timer count down by a constant due to unreliable lower bits
+	 * on some targets.
+	 */
+	return readl_relaxed(source_base + TIMER_COUNT_VAL) >> MSM_DGT_SHIFT;
+}
+
+static struct clocksource msm_clocksource = {
+	.name	= "dg_timer",
+	.rating	= 300,
+	.read	= msm_read_timer_count,
+	.mask	= CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 static void __init msm_timer_init(void)
 {
-	struct msm_clock *clock;
-	struct clock_event_device *ce = &msm_clocks[MSM_CLOCK_GPT].clockevent;
-	struct clocksource *cs = &msm_clocks[MSM_CLOCK_DGT].clocksource;
+	struct clock_event_device *ce = &msm_clockevent;
+	struct clocksource *cs = &msm_clocksource;
 	int res;
-	int global_offset = 0;
-
 
 	if (cpu_is_msm7x01()) {
-		msm_clocks[MSM_CLOCK_GPT].regbase = MSM_CSR_BASE;
-		msm_clocks[MSM_CLOCK_DGT].regbase = MSM_CSR_BASE + 0x10;
+		event_base = MSM_CSR_BASE;
+		source_base = MSM_CSR_BASE + 0x10;
 	} else if (cpu_is_msm7x30()) {
-		msm_clocks[MSM_CLOCK_GPT].regbase = MSM_CSR_BASE + 0x04;
-		msm_clocks[MSM_CLOCK_DGT].regbase = MSM_CSR_BASE + 0x24;
+		event_base = MSM_CSR_BASE + 0x04;
+		source_base = MSM_CSR_BASE + 0x24;
 	} else if (cpu_is_qsd8x50()) {
-		msm_clocks[MSM_CLOCK_GPT].regbase = MSM_CSR_BASE;
-		msm_clocks[MSM_CLOCK_DGT].regbase = MSM_CSR_BASE + 0x10;
+		event_base = MSM_CSR_BASE;
+		source_base = MSM_CSR_BASE + 0x10;
 	} else if (cpu_is_msm8x60() || cpu_is_msm8960()) {
-		msm_clocks[MSM_CLOCK_GPT].regbase = MSM_TMR_BASE + 0x04;
-		msm_clocks[MSM_CLOCK_DGT].regbase = MSM_TMR_BASE + 0x24;
-
-		/* Use CPU0's timer as the global timer. */
-		global_offset = MSM_TMR0_BASE - MSM_TMR_BASE;
+		event_base = MSM_TMR_BASE + 0x04;
+		/* Use CPU0's timer as the global clock source. */
+		source_base = MSM_TMR0_BASE + 0x24;
 	} else
 		BUG();
 
@@ -216,88 +164,71 @@ static void __init msm_timer_init(void)
 	writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
 #endif
 
-	clock = &msm_clocks[MSM_CLOCK_GPT];
-	clock->local_counter = clock->regbase + TIMER_COUNT_VAL;
-
-	writel_relaxed(0, clock->regbase + TIMER_ENABLE);
-	writel_relaxed(0, clock->regbase + TIMER_CLEAR);
-	writel_relaxed(~0, clock->regbase + TIMER_MATCH_VAL);
-	ce->mult = div_sc(clock->freq, NSEC_PER_SEC, ce->shift);
+	writel_relaxed(0, event_base + TIMER_ENABLE);
+	writel_relaxed(0, event_base + TIMER_CLEAR);
+	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
+	ce->mult = div_sc(GPT_HZ, NSEC_PER_SEC, ce->shift);
 	/*
 	 * allow at least 10 seconds to notice that the timer
 	 * wrapped
 	 */
-	ce->max_delta_ns =
-		clockevent_delta2ns(0xf0000000 >> clock->shift, ce);
+	ce->max_delta_ns = clockevent_delta2ns(0xf0000000, ce);
 	/* 4 gets rounded down to 3 */
 	ce->min_delta_ns = clockevent_delta2ns(4, ce);
 	ce->cpumask = cpumask_of(0);
 
-	ce->irq = clock->irq;
+	ce->irq = INT_GP_TIMER_EXP;
 	if (cpu_is_msm8x60() || cpu_is_msm8960()) {
-		clock->percpu_evt = alloc_percpu(struct clock_event_device *);
-		if (!clock->percpu_evt) {
+		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);
 			goto err;
 		}
-
-		*__this_cpu_ptr(clock->percpu_evt) = ce;
+		*__this_cpu_ptr(msm_evt.percpu_evt) = ce;
 		res = request_percpu_irq(ce->irq, msm_timer_interrupt,
-					 ce->name, clock->percpu_evt);
+					 ce->name, msm_evt.percpu_evt);
 		if (!res)
 			enable_percpu_irq(ce->irq, 0);
 	} else {
-		clock->evt = ce;
+		msm_evt.evt = ce;
 		res = request_irq(ce->irq, msm_timer_interrupt,
 				  IRQF_TIMER | IRQF_NOBALANCING |
-				  IRQF_TRIGGER_RISING, ce->name, &clock->evt);
+				  IRQF_TRIGGER_RISING, ce->name, &msm_evt.evt);
 	}
 
 	if (res)
 		pr_err("request_irq failed for %s\n", ce->name);
-
 	clockevents_register_device(ce);
 err:
-	clock = &msm_clocks[MSM_CLOCK_DGT];
-	clock->local_counter = clock->regbase + TIMER_COUNT_VAL;
-	clock->global_counter = clock->local_counter + global_offset;
-	writel_relaxed(TIMER_ENABLE_EN, clock->regbase + TIMER_ENABLE);
-	res = clocksource_register_hz(cs, clock->freq);
+	writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
+	res = clocksource_register_hz(cs, DGT_HZ >> MSM_DGT_SHIFT);
 	if (res)
-		pr_err("clocksource_register failed for %s\n", cs->name);
+		pr_err("clocksource_register failed\n");
 }
 
 #ifdef CONFIG_LOCAL_TIMERS
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
-	static bool local_timer_inited;
-	struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER];
-
 	/* Use existing clock_event for cpu 0 */
 	if (!smp_processor_id())
 		return 0;
 
-	if (!local_timer_inited) {
-		writel(0, clock->regbase  + TIMER_ENABLE);
-		writel(0, clock->regbase + TIMER_CLEAR);
-		writel(~0, clock->regbase + TIMER_MATCH_VAL);
-		local_timer_inited = true;
-	}
-	evt->irq = clock->irq;
+	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->name = "local_timer";
-	evt->features = CLOCK_EVT_FEAT_ONESHOT;
-	evt->rating = clock->clockevent.rating;
+	evt->features = msm_clockevent.features;
+	evt->rating = msm_clockevent.rating;
 	evt->set_mode = msm_timer_set_mode;
 	evt->set_next_event = msm_timer_set_next_event;
-	evt->shift = clock->clockevent.shift;
-	evt->mult = div_sc(clock->freq, NSEC_PER_SEC, evt->shift);
-	evt->max_delta_ns =
-		clockevent_delta2ns(0xf0000000 >> clock->shift, evt);
+	evt->shift = msm_clockevent.shift;
+	evt->mult = div_sc(GPT_HZ, NSEC_PER_SEC, evt->shift);
+	evt->max_delta_ns = clockevent_delta2ns(0xf0000000, evt);
 	evt->min_delta_ns = clockevent_delta2ns(4, evt);
 
-	*__this_cpu_ptr(clock->percpu_evt) = evt;
+	*__this_cpu_ptr(msm_evt.percpu_evt) = evt;
 	enable_percpu_irq(evt->irq, 0);
-
 	clockevents_register_device(evt);
 	return 0;
 }
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 6/8] msm: timer: Remove SoC specific #ifdefs

[Stephen Boyd]

The timer frequency is currently ifdefed in addition to setting
the DGT clock's divider value on SCORPIONMP targets. Setup the
frequency dynamically using the existing cpu_is_*() branches and
assign a custom clocksource read function for 7x01a to get the
shift out of the generic path.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/timer.c |   38 +++++++++++++++++---------------------
 1 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index fc06464..ca0a957 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -40,20 +40,7 @@
 
 #define GPT_HZ 32768
 
-/* TODO: Remove these ifdefs */
-#if defined(CONFIG_ARCH_QSD8X50)
-#define DGT_HZ (19200000 / 4) /* 19.2 MHz / 4 by default */
-#define MSM_DGT_SHIFT (0)
-#elif defined(CONFIG_ARCH_MSM7X30)
-#define DGT_HZ (24576000 / 4) /* 24.576 MHz (LPXO) / 4 by default */
-#define MSM_DGT_SHIFT (0)
-#elif defined(CONFIG_ARCH_MSM8X60) || defined(CONFIG_ARCH_MSM8960)
-#define DGT_HZ (27000000 / 4) /* 27 MHz (PXO) / 4 by default */
-#define MSM_DGT_SHIFT (0)
-#else
-#define DGT_HZ 19200000 /* 19.2 MHz or 600 KHz after shift */
-#define MSM_DGT_SHIFT (5)
-#endif
+#define MSM_DGT_SHIFT 5
 
 static void __iomem *event_base;
 
@@ -123,18 +110,23 @@ static void __iomem *source_base;
 
 static cycle_t msm_read_timer_count(struct clocksource *cs)
 {
+	return readl_relaxed(source_base + TIMER_COUNT_VAL);
+}
+
+static cycle_t msm_read_timer_count_shift(struct clocksource *cs)
+{
 	/*
 	 * Shift timer count down by a constant due to unreliable lower bits
 	 * on some targets.
 	 */
-	return readl_relaxed(source_base + TIMER_COUNT_VAL) >> MSM_DGT_SHIFT;
+	return msm_read_timer_count(cs) >> MSM_DGT_SHIFT;
 }
 
 static struct clocksource msm_clocksource = {
 	.name	= "dg_timer",
 	.rating	= 300,
 	.read	= msm_read_timer_count,
-	.mask	= CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
+	.mask	= CLOCKSOURCE_MASK(32),
 	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -143,27 +135,31 @@ static void __init msm_timer_init(void)
 	struct clock_event_device *ce = &msm_clockevent;
 	struct clocksource *cs = &msm_clocksource;
 	int res;
+	u32 dgt_hz;
 
 	if (cpu_is_msm7x01()) {
 		event_base = MSM_CSR_BASE;
 		source_base = MSM_CSR_BASE + 0x10;
+		dgt_hz = 19200000 >> MSM_DGT_SHIFT; /* 600 KHz */
+		cs->read = msm_read_timer_count_shift;
+		cs->mask = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT));
 	} else if (cpu_is_msm7x30()) {
 		event_base = MSM_CSR_BASE + 0x04;
 		source_base = MSM_CSR_BASE + 0x24;
+		dgt_hz = 24576000 / 4;
 	} else if (cpu_is_qsd8x50()) {
 		event_base = MSM_CSR_BASE;
 		source_base = MSM_CSR_BASE + 0x10;
+		dgt_hz = 19200000 / 4;
 	} else if (cpu_is_msm8x60() || cpu_is_msm8960()) {
 		event_base = MSM_TMR_BASE + 0x04;
 		/* Use CPU0's timer as the global clock source. */
 		source_base = MSM_TMR0_BASE + 0x24;
+		dgt_hz = 27000000 / 4;
+		writel_relaxed(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
 	} else
 		BUG();
 
-#ifdef CONFIG_ARCH_MSM_SCORPIONMP
-	writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
-#endif
-
 	writel_relaxed(0, event_base + TIMER_ENABLE);
 	writel_relaxed(0, event_base + TIMER_CLEAR);
 	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
@@ -201,7 +197,7 @@ static void __init msm_timer_init(void)
 	clockevents_register_device(ce);
 err:
 	writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
-	res = clocksource_register_hz(cs, DGT_HZ >> MSM_DGT_SHIFT);
+	res = clocksource_register_hz(cs, dgt_hz);
 	if (res)
 		pr_err("clocksource_register failed\n");
 }
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 7/8] msm: timer: Setup interrupt after registering clockevent

[Stephen Boyd]

Some bootloaders may leave a pending interrupt for the timer and
thus msm_timer_interrupt() has a check for a NULL event handler.
Unmask and register for the interrupt after registering the
clockevent so that we can get the NULL pointer check out of the
fast path.
 
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/timer.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index ca0a957..3d80fbf 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -47,8 +47,6 @@ 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;
-	if (evt->event_handler == NULL)
-		return IRQ_HANDLED;
 	/* Stop the timer tick */
 	if (evt->mode == CLOCK_EVT_MODE_ONESHOT) {
 		u32 ctrl = readl_relaxed(event_base + TIMER_ENABLE);
@@ -174,6 +172,7 @@ static void __init msm_timer_init(void)
 	ce->cpumask = cpumask_of(0);
 
 	ce->irq = INT_GP_TIMER_EXP;
+	clockevents_register_device(ce);
 	if (cpu_is_msm8x60() || cpu_is_msm8960()) {
 		msm_evt.percpu_evt = alloc_percpu(struct clock_event_device *);
 		if (!msm_evt.percpu_evt) {
@@ -194,7 +193,6 @@ static void __init msm_timer_init(void)
 
 	if (res)
 		pr_err("request_irq failed for %s\n", ce->name);
-	clockevents_register_device(ce);
 err:
 	writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
 	res = clocksource_register_hz(cs, dgt_hz);
@@ -224,8 +222,8 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 	evt->min_delta_ns = clockevent_delta2ns(4, evt);
 
 	*__this_cpu_ptr(msm_evt.percpu_evt) = evt;
-	enable_percpu_irq(evt->irq, 0);
 	clockevents_register_device(evt);
+	enable_percpu_irq(evt->irq, 0);
 	return 0;
 }
 
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 8/8] msm: timer: Use clockevents_config_and_register()

[Stephen Boyd]

Don't open code the min/max delta logic. Use the generic
version instead. Also expand the number of bits we can handle
because there isn't anything that says we can't handle all 32
bits.

Before:
 max_delta_ns:   122880426391799
 min_delta_ns:   122070
 mult:           140737
 shift:          32

After:
 max_delta_ns:   131071523464981
 min_delta_ns:   122069
 mult:           70369
 shift:          31

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/timer.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 3d80fbf..11d0d8f 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -93,7 +93,6 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
 static struct clock_event_device msm_clockevent = {
 	.name		= "gp_timer",
 	.features	= CLOCK_EVT_FEAT_ONESHOT,
-	.shift		= 32,
 	.rating		= 200,
 	.set_next_event	= msm_timer_set_next_event,
 	.set_mode	= msm_timer_set_mode,
@@ -161,18 +160,10 @@ static void __init msm_timer_init(void)
 	writel_relaxed(0, event_base + TIMER_ENABLE);
 	writel_relaxed(0, event_base + TIMER_CLEAR);
 	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
-	ce->mult = div_sc(GPT_HZ, NSEC_PER_SEC, ce->shift);
-	/*
-	 * allow at least 10 seconds to notice that the timer
-	 * wrapped
-	 */
-	ce->max_delta_ns = clockevent_delta2ns(0xf0000000, ce);
-	/* 4 gets rounded down to 3 */
-	ce->min_delta_ns = clockevent_delta2ns(4, ce);
 	ce->cpumask = cpumask_of(0);
 
 	ce->irq = INT_GP_TIMER_EXP;
-	clockevents_register_device(ce);
+	clockevents_config_and_register(ce, GPT_HZ, 4, 0xffffffff);
 	if (cpu_is_msm8x60() || cpu_is_msm8960()) {
 		msm_evt.percpu_evt = alloc_percpu(struct clock_event_device *);
 		if (!msm_evt.percpu_evt) {
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 0/8] MSM timer fixes and cleanups

[David Brown]

On Tue, Nov 08, 2011 at 10:34:02AM -0800, Stephen Boyd wrote:
> Currently the MSM timers use the same physical counter
> for the clockevent and clocksource. This works as long
> as the clocksource isn't stopped from ticking during normal
> operation but unfortunately that isn't the case and the
> clocksource is stopped when the clockevent is shutdown.
> Even worse, switching the clocksource via sysfs at runtime will
> hang the system.
> 
> This series reorganizes the MSM timer code so that one counter
> is only used for either a clocksource or a clockevent, and not 
> both. In the process we reduce the lines of code and fix
> a few long-standing bugs.

How do you test these on the upstream kernel?  Booting successfully is
probably a good starting point, though.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 0/8] MSM timer fixes and cleanups

[Stephen Boyd]

On 11/10/11 10:33, David Brown wrote:
> On Tue, Nov 08, 2011 at 10:34:02AM -0800, Stephen Boyd wrote:
>> Currently the MSM timers use the same physical counter
>> for the clockevent and clocksource. This works as long
>> as the clocksource isn't stopped from ticking during normal
>> operation but unfortunately that isn't the case and the
>> clocksource is stopped when the clockevent is shutdown.
>> Even worse, switching the clocksource via sysfs at runtime will
>> hang the system.
>>
>> This series reorganizes the MSM timer code so that one counter
>> is only used for either a clocksource or a clockevent, and not 
>> both. In the process we reduce the lines of code and fix
>> a few long-standing bugs.
> How do you test these on the upstream kernel?  Booting successfully is
> probably a good starting point, though.
>

Booting is good because that means you're getting timer interrupts.
After that, do some sleep 10, 20, 30, etc. tests with a stopwatch to
make sure that time isn't completely off.

You can also check bogoMIPS and see what the before and after is. Beyond
that I suppose some kind of ntp or gettimeofday test would be good.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.