All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@codeaurora.org>
To: David Brown <davidb@codeaurora.org>,
	Daniel Walker <dwalker@fifo99.com>,
	Bryan Huntsman <bryanh@codeaurora.org>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] ARM: msm: Wait for timer clear to complete
Date: Thu, 14 Mar 2013 20:31:39 -0700	[thread overview]
Message-ID: <1363318299-10814-4-git-send-email-sboyd@codeaurora.org> (raw)
In-Reply-To: <1363318299-10814-1-git-send-email-sboyd@codeaurora.org>

Without looping on the status bit, there is no way to guarantee
that a clear of the timer has actually completed. This can cause
us to enable the timer before the count has cleared and miss a
timer interrupt. To simplify this patch, remove the timer
register setup done during timer init, since it's duplicate work
that is eventually done in the set_next_event() callback.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-msm/timer.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 9f033c3..284313f 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -30,20 +30,22 @@
 
 #include "common.h"
 
-#define TIMER_MATCH_VAL         0x0000
-#define TIMER_COUNT_VAL         0x0004
-#define TIMER_ENABLE            0x0008
-#define TIMER_ENABLE_CLR_ON_MATCH_EN    BIT(1)
-#define TIMER_ENABLE_EN                 BIT(0)
-#define TIMER_CLEAR             0x000C
-#define DGT_CLK_CTL		0x10
-#define DGT_CLK_CTL_DIV_4	0x3
+#define TIMER_MATCH_VAL			0x0000
+#define TIMER_COUNT_VAL			0x0004
+#define TIMER_ENABLE			0x0008
+#define TIMER_ENABLE_CLR_ON_MATCH_EN	BIT(1)
+#define TIMER_ENABLE_EN			BIT(0)
+#define TIMER_CLEAR			0x000C
+#define DGT_CLK_CTL			0x10
+#define DGT_CLK_CTL_DIV_4		0x3
+#define TIMER_STS_GPT0_CLR_PEND		BIT(10)
 
 #define GPT_HZ 32768
 
 #define MSM_DGT_SHIFT 5
 
 static void __iomem *event_base;
+static void __iomem *sts_base;
 
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
@@ -68,6 +70,11 @@ static int msm_timer_set_next_event(unsigned long cycles,
 
 	writel_relaxed(ctrl, event_base + TIMER_CLEAR);
 	writel_relaxed(cycles, event_base + TIMER_MATCH_VAL);
+
+	if (sts_base)
+		while (readl_relaxed(sts_base) & TIMER_STS_GPT0_CLR_PEND)
+			cpu_relax();
+
 	writel_relaxed(ctrl | TIMER_ENABLE_EN, event_base + TIMER_ENABLE);
 	return 0;
 }
@@ -138,9 +145,6 @@ static int __cpuinit msm_local_timer_setup(struct clock_event_device *evt)
 	if (!smp_processor_id())
 		return 0;
 
-	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 = msm_clockevent.features;
@@ -178,9 +182,6 @@ static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
 	struct clocksource *cs = &msm_clocksource;
 	int res;
 
-	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;
 
@@ -275,6 +276,7 @@ void __init msm_dt_timer_init(void)
 	of_node_put(np);
 
 	event_base = base + 0x4;
+	sts_base = base + 0x88;
 	source_base = cpu0_base + 0x24;
 	freq /= 4;
 	writel_relaxed(DGT_CLK_CTL_DIV_4, source_base + DGT_CLK_CTL);
@@ -283,7 +285,8 @@ void __init msm_dt_timer_init(void)
 }
 #endif
 
-static int __init msm_timer_map(phys_addr_t addr, u32 event, u32 source)
+static int __init msm_timer_map(phys_addr_t addr, u32 event, u32 source,
+				u32 sts)
 {
 	void __iomem *base;
 
@@ -294,6 +297,8 @@ static int __init msm_timer_map(phys_addr_t addr, u32 event, u32 source)
 	}
 	event_base = base + event;
 	source_base = base + source;
+	if (sts)
+		sts_base = base + sts;
 
 	return 0;
 }
@@ -302,7 +307,7 @@ void __init msm7x01_timer_init(void)
 {
 	struct clocksource *cs = &msm_clocksource;
 
-	if (msm_timer_map(0xc0100000, 0x0, 0x10))
+	if (msm_timer_map(0xc0100000, 0x0, 0x10, 0x0))
 		return;
 	cs->read = msm_read_timer_count_shift;
 	cs->mask = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT));
@@ -313,14 +318,14 @@ void __init msm7x01_timer_init(void)
 
 void __init msm7x30_timer_init(void)
 {
-	if (msm_timer_map(0xc0100000, 0x4, 0x24))
+	if (msm_timer_map(0xc0100000, 0x4, 0x24, 0x80))
 		return;
 	msm_timer_init(24576000 / 4, 32, 1, false);
 }
 
 void __init qsd8x50_timer_init(void)
 {
-	if (msm_timer_map(0xAC100000, 0x0, 0x10))
+	if (msm_timer_map(0xAC100000, 0x0, 0x10, 0x34))
 		return;
 	msm_timer_init(19200000 / 4, 32, 7, false);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] ARM: msm: Wait for timer clear to complete
Date: Thu, 14 Mar 2013 20:31:39 -0700	[thread overview]
Message-ID: <1363318299-10814-4-git-send-email-sboyd@codeaurora.org> (raw)
In-Reply-To: <1363318299-10814-1-git-send-email-sboyd@codeaurora.org>

Without looping on the status bit, there is no way to guarantee
that a clear of the timer has actually completed. This can cause
us to enable the timer before the count has cleared and miss a
timer interrupt. To simplify this patch, remove the timer
register setup done during timer init, since it's duplicate work
that is eventually done in the set_next_event() callback.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mach-msm/timer.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 9f033c3..284313f 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -30,20 +30,22 @@
 
 #include "common.h"
 
-#define TIMER_MATCH_VAL         0x0000
-#define TIMER_COUNT_VAL         0x0004
-#define TIMER_ENABLE            0x0008
-#define TIMER_ENABLE_CLR_ON_MATCH_EN    BIT(1)
-#define TIMER_ENABLE_EN                 BIT(0)
-#define TIMER_CLEAR             0x000C
-#define DGT_CLK_CTL		0x10
-#define DGT_CLK_CTL_DIV_4	0x3
+#define TIMER_MATCH_VAL			0x0000
+#define TIMER_COUNT_VAL			0x0004
+#define TIMER_ENABLE			0x0008
+#define TIMER_ENABLE_CLR_ON_MATCH_EN	BIT(1)
+#define TIMER_ENABLE_EN			BIT(0)
+#define TIMER_CLEAR			0x000C
+#define DGT_CLK_CTL			0x10
+#define DGT_CLK_CTL_DIV_4		0x3
+#define TIMER_STS_GPT0_CLR_PEND		BIT(10)
 
 #define GPT_HZ 32768
 
 #define MSM_DGT_SHIFT 5
 
 static void __iomem *event_base;
+static void __iomem *sts_base;
 
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
@@ -68,6 +70,11 @@ static int msm_timer_set_next_event(unsigned long cycles,
 
 	writel_relaxed(ctrl, event_base + TIMER_CLEAR);
 	writel_relaxed(cycles, event_base + TIMER_MATCH_VAL);
+
+	if (sts_base)
+		while (readl_relaxed(sts_base) & TIMER_STS_GPT0_CLR_PEND)
+			cpu_relax();
+
 	writel_relaxed(ctrl | TIMER_ENABLE_EN, event_base + TIMER_ENABLE);
 	return 0;
 }
@@ -138,9 +145,6 @@ static int __cpuinit msm_local_timer_setup(struct clock_event_device *evt)
 	if (!smp_processor_id())
 		return 0;
 
-	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 = msm_clockevent.features;
@@ -178,9 +182,6 @@ static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
 	struct clocksource *cs = &msm_clocksource;
 	int res;
 
-	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;
 
@@ -275,6 +276,7 @@ void __init msm_dt_timer_init(void)
 	of_node_put(np);
 
 	event_base = base + 0x4;
+	sts_base = base + 0x88;
 	source_base = cpu0_base + 0x24;
 	freq /= 4;
 	writel_relaxed(DGT_CLK_CTL_DIV_4, source_base + DGT_CLK_CTL);
@@ -283,7 +285,8 @@ void __init msm_dt_timer_init(void)
 }
 #endif
 
-static int __init msm_timer_map(phys_addr_t addr, u32 event, u32 source)
+static int __init msm_timer_map(phys_addr_t addr, u32 event, u32 source,
+				u32 sts)
 {
 	void __iomem *base;
 
@@ -294,6 +297,8 @@ static int __init msm_timer_map(phys_addr_t addr, u32 event, u32 source)
 	}
 	event_base = base + event;
 	source_base = base + source;
+	if (sts)
+		sts_base = base + sts;
 
 	return 0;
 }
@@ -302,7 +307,7 @@ void __init msm7x01_timer_init(void)
 {
 	struct clocksource *cs = &msm_clocksource;
 
-	if (msm_timer_map(0xc0100000, 0x0, 0x10))
+	if (msm_timer_map(0xc0100000, 0x0, 0x10, 0x0))
 		return;
 	cs->read = msm_read_timer_count_shift;
 	cs->mask = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT));
@@ -313,14 +318,14 @@ void __init msm7x01_timer_init(void)
 
 void __init msm7x30_timer_init(void)
 {
-	if (msm_timer_map(0xc0100000, 0x4, 0x24))
+	if (msm_timer_map(0xc0100000, 0x4, 0x24, 0x80))
 		return;
 	msm_timer_init(24576000 / 4, 32, 1, false);
 }
 
 void __init qsd8x50_timer_init(void)
 {
-	if (msm_timer_map(0xAC100000, 0x0, 0x10))
+	if (msm_timer_map(0xAC100000, 0x0, 0x10, 0x34))
 		return;
 	msm_timer_init(19200000 / 4, 32, 7, false);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

  parent reply	other threads:[~2013-03-15  3:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-15  3:31 [PATCH 0/3] Fix msm timer clearing bugs Stephen Boyd
2013-03-15  3:31 ` Stephen Boyd
2013-03-15  3:31 ` [PATCH 1/3] ARM: msm: Stop counting before reprogramming clockevent Stephen Boyd
2013-03-15  3:31   ` Stephen Boyd
2013-03-15  3:31 ` [PATCH 2/3] ARM: msm: Rework timer binding to be more general Stephen Boyd
2013-03-15  3:31   ` Stephen Boyd
2013-03-15  3:31 ` Stephen Boyd [this message]
2013-03-15  3:31   ` [PATCH 3/3] ARM: msm: Wait for timer clear to complete Stephen Boyd
2013-03-22 17:47 ` [PATCH 0/3] Fix msm timer clearing bugs David Brown
2013-03-22 17:47   ` David Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1363318299-10814-4-git-send-email-sboyd@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=bryanh@codeaurora.org \
    --cc=davidb@codeaurora.org \
    --cc=dwalker@fifo99.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.