Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1299911 > unrolled thread

[PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays

Started byRoman Volkov <v1ron@mail.ru>
First post2016-01-01 14:40 +0100
Last post2016-01-07 11:50 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays Roman Volkov <v1ron@mail.ru> - 2016-01-01 14:40 +0100
    [PATCH v3 3/3] clocksource/vt8500: Add register R/W functions Roman Volkov <v1ron@mail.ru> - 2016-01-01 14:40 +0100
    Re: [PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-01-06 15:30 +0100
      Re: [PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays Roman Volkov <v1ron@mail.ru> - 2016-01-06 16:40 +0100
        Re: [PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-01-07 11:50 +0100

#1299911 — [PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays

FromRoman Volkov <v1ron@mail.ru>
Date2016-01-01 14:40 +0100
Subject[PATCH v3 0/3] clocksource/vt8500: Fix hangs in small delays
Message-ID<qM85Y-31n-7@gated-at.bofh.it>
From: Roman Volkov <rvolkov@v1ros.org>

vt8500 hangs in nanosleep() function, starting from commit
c6eb3f70d4482806dc2d3e1e3c7736f497b1d418, making the system unusable.
Per investigation, looks like set_next_event() now receives too small
delta and fails with -ETIME.

Google group discussion:
https://groups.google.com/forum/#!topic/vt8500-wm8505-linux-kernel/vDMF_mDOb1k

v2:
Address comments by Alexey Charkov. Merge patches to get less amount of
changes (three patches instead of four).

v3:
Address comments by Thomas Gleixner. Edit the changelog.

Tested on my WM8650, no issues in three days uptime.

Roman Volkov (3):
  clocksource/vt8500: Use MIN_OSCR_DELTA from PXA
  clocksource/vt8500: Remove the 'loops' variable
  clocksource/vt8500: Add register R/W functions

 drivers/clocksource/vt8500_timer.c | 98 +++++++++++++++++++++++++++-----------
 1 file changed, 69 insertions(+), 29 deletions(-)

-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1299912 — [PATCH v3 3/3] clocksource/vt8500: Add register R/W functions

FromRoman Volkov <v1ron@mail.ru>
Date2016-01-01 14:40 +0100
Subject[PATCH v3 3/3] clocksource/vt8500: Add register R/W functions
Message-ID<qM860-31n-49@gated-at.bofh.it>
In reply to#1299911
From: Roman Volkov <rvolkov@v1ros.org>

vt8500 timer requires special synchronization for accessing some of its
registers. Define special read and write functions to handle this process
transparently.

Use relaxed read/write, according to the following:
http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658:
"For accesses to the same device we don't
actually need any barriers on ARM as this is guaranteed by the
architecture."

To perform a read from the Timer Count register, user must write a one
to the Timer Control register and wait for completion flag by polling the
Timer Read Count Active bit.

To perform a write to the Count or Match registers, user must poll the
write completion flag for the corresponding register to ensure that the
previous write completed and then write the actual value.

Signed-off-by: Roman Volkov <rvolkov@v1ros.org>
Acked-by: Alexey Charkov <alchark@gmail.com>
---
 drivers/clocksource/vt8500_timer.c | 88 ++++++++++++++++++++++++++++----------
 1 file changed, 66 insertions(+), 22 deletions(-)

diff --git a/drivers/clocksource/vt8500_timer.c b/drivers/clocksource/vt8500_timer.c
index eb08d96..fd88a5b 100644
--- a/drivers/clocksource/vt8500_timer.c
+++ b/drivers/clocksource/vt8500_timer.c
@@ -38,32 +38,75 @@
 
 #define VT8500_TIMER_OFFSET	0x0100
 #define VT8500_TIMER_HZ		3000000
-#define TIMER_MATCH_VAL		0x0000
+#define TIMER_MATCH0_VAL	0x0000
+#define TIMER_MATCH1_VAL	0x0004
+#define TIMER_MATCH2_VAL	0x0008
+#define TIMER_MATCH3_VAL	0x000c
 #define TIMER_COUNT_VAL		0x0010
 #define TIMER_STATUS_VAL	0x0014
 #define TIMER_IER_VAL		0x001c		/* interrupt enable */
 #define TIMER_CTRL_VAL		0x0020
 #define TIMER_AS_VAL		0x0024		/* access status */
-#define TIMER_COUNT_R_ACTIVE	(1 << 5)	/* not ready for read */
-#define TIMER_COUNT_W_ACTIVE	(1 << 4)	/* not ready for write */
-#define TIMER_MATCH_W_ACTIVE	(1 << 0)	/* not ready for write */
+/* R/W status flags */
+#define TIMER_COUNT_R_ACTIVE	(1 << 5)
+#define TIMER_COUNT_W_ACTIVE	(1 << 4)
+#define TIMER_MATCH3_W_ACTIVE	(1 << 3)
+#define TIMER_MATCH2_W_ACTIVE	(1 << 2)
+#define TIMER_MATCH1_W_ACTIVE	(1 << 1)
+#define TIMER_MATCH0_W_ACTIVE	(1 << 0)
 
 #define MIN_OSCR_DELTA		16
 
+#define vt8500_timer_sync(bit)	{ while (readl_relaxed \
+				    (regbase + TIMER_AS_VAL) & bit) \
+					cpu_relax(); }
+
 static void __iomem *regbase;
 
-static cycle_t vt8500_timer_read(struct clocksource *cs)
+static void vt8500_timer_write(u32 value, unsigned long reg)
 {
-	writel(3, regbase + TIMER_CTRL_VAL);
-	while (readl(regbase + TIMER_AS_VAL) & TIMER_COUNT_R_ACTIVE)
-		cpu_relax();
-	return readl(regbase + TIMER_COUNT_VAL);
+	switch (reg) {
+	case TIMER_COUNT_VAL:
+		vt8500_timer_sync(TIMER_COUNT_W_ACTIVE);
+		break;
+	case TIMER_MATCH0_VAL:
+		vt8500_timer_sync(TIMER_MATCH0_W_ACTIVE);
+		break;
+	case TIMER_MATCH1_VAL:
+		vt8500_timer_sync(TIMER_MATCH1_W_ACTIVE);
+		break;
+	case TIMER_MATCH2_VAL:
+		vt8500_timer_sync(TIMER_MATCH2_W_ACTIVE);
+		break;
+	case TIMER_MATCH3_VAL:
+		vt8500_timer_sync(TIMER_MATCH3_W_ACTIVE);
+		break;
+	}
+
+	writel_relaxed(value, regbase + reg);
+}
+
+static u32 vt8500_timer_read(unsigned long reg)
+{
+	if (reg == TIMER_COUNT_VAL) {
+		vt8500_timer_write(3, TIMER_CTRL_VAL);
+		vt8500_timer_sync(TIMER_COUNT_R_ACTIVE);
+
+		return readl_relaxed(regbase + TIMER_COUNT_VAL);
+	}
+
+	return readl_relaxed(regbase + reg);
+}
+
+static cycle_t vt8500_oscr0_read(struct clocksource *cs)
+{
+	return vt8500_timer_read(TIMER_COUNT_VAL);
 }
 
 static struct clocksource clocksource = {
 	.name           = "vt8500_timer",
 	.rating         = 200,
-	.read           = vt8500_timer_read,
+	.read           = vt8500_oscr0_read,
 	.mask           = CLOCKSOURCE_MASK(32),
 	.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
@@ -71,23 +114,24 @@ static struct clocksource clocksource = {
 static int vt8500_timer_set_next_event(unsigned long cycles,
 				    struct clock_event_device *evt)
 {
-	cycle_t alarm = clocksource.read(&clocksource) + cycles;
-	while (readl(regbase + TIMER_AS_VAL) & TIMER_MATCH_W_ACTIVE)
-		cpu_relax();
-	writel((unsigned long)alarm, regbase + TIMER_MATCH_VAL);
+	unsigned long alarm = vt8500_timer_read(TIMER_COUNT_VAL) + cycles;
 
-	if ((signed)(alarm - clocksource.read(&clocksource)) <= MIN_OSCR_DELTA)
+	vt8500_timer_write(alarm, TIMER_MATCH0_VAL);
+	if ((signed)(alarm - vt8500_timer_read(
+				TIMER_COUNT_VAL)) <= MIN_OSCR_DELTA) {
 		return -ETIME;
+	}
 
-	writel(1, regbase + TIMER_IER_VAL);
+	vt8500_timer_write(1, TIMER_IER_VAL);
 
 	return 0;
 }
 
 static int vt8500_shutdown(struct clock_event_device *evt)
 {
-	writel(readl(regbase + TIMER_CTRL_VAL) | 1, regbase + TIMER_CTRL_VAL);
-	writel(0, regbase + TIMER_IER_VAL);
+	vt8500_timer_write(vt8500_timer_read(TIMER_CTRL_VAL) | 1,
+					TIMER_CTRL_VAL);
+	vt8500_timer_write(0, TIMER_IER_VAL);
 	return 0;
 }
 
@@ -103,7 +147,7 @@ static struct clock_event_device clockevent = {
 static irqreturn_t vt8500_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
-	writel(0xf, regbase + TIMER_STATUS_VAL);
+	vt8500_timer_write(0xf, TIMER_STATUS_VAL);
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;
@@ -133,9 +177,9 @@ static void __init vt8500_timer_init(struct device_node *np)
 		return;
 	}
 
-	writel(1, regbase + TIMER_CTRL_VAL);
-	writel(0xf, regbase + TIMER_STATUS_VAL);
-	writel(~0, regbase + TIMER_MATCH_VAL);
+	vt8500_timer_write(1, TIMER_CTRL_VAL);
+	vt8500_timer_write(0xf, TIMER_STATUS_VAL);
+	vt8500_timer_write(~0, TIMER_MATCH0_VAL);
 
 	if (clocksource_register_hz(&clocksource, VT8500_TIMER_HZ))
 		pr_err("%s: vt8500_timer_init: clocksource_register failed for %s\n",
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302807

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2016-01-06 15:30 +0100
Message-ID<qNXg6-2iB-13@gated-at.bofh.it>
In reply to#1299911
On 01/01/2016 02:24 PM, Roman Volkov wrote:
> From: Roman Volkov <rvolkov@v1ros.org>
>
> vt8500 hangs in nanosleep() function, starting from commit
> c6eb3f70d4482806dc2d3e1e3c7736f497b1d418, making the system unusable.
> Per investigation, looks like set_next_event() now receives too small
> delta and fails with -ETIME.


Hi Roman,

I think the patch 1/3 should go as a fix for 4.4-rc7 and stable and the 
two other should go for the next version as they are improvements 
regarding the current code.

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302859

FromRoman Volkov <v1ron@mail.ru>
Date2016-01-06 16:40 +0100
Message-ID<qNYlQ-326-29@gated-at.bofh.it>
In reply to#1302807
В Wed, 6 Jan 2016 15:24:07 +0100
Daniel Lezcano <daniel.lezcano@linaro.org> пишет:

> On 01/01/2016 02:24 PM, Roman Volkov wrote:
> > From: Roman Volkov <rvolkov@v1ros.org>
> >
> > vt8500 hangs in nanosleep() function, starting from commit
> > c6eb3f70d4482806dc2d3e1e3c7736f497b1d418, making the system
> > unusable. Per investigation, looks like set_next_event() now
> > receives too small delta and fails with -ETIME.  
> 
> 
> Hi Roman,
> 
> I think the patch 1/3 should go as a fix for 4.4-rc7 and stable and
> the two other should go for the next version as they are improvements 
> regarding the current code.
> 

Hi Daniel,

I agree, -stable can live without the patches 2 an 3.

Regards,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303475

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2016-01-07 11:50 +0100
Message-ID<qOgiK-6Tx-23@gated-at.bofh.it>
In reply to#1302859
On 01/06/2016 04:30 PM, Roman Volkov wrote:
> В Wed, 6 Jan 2016 15:24:07 +0100
> Daniel Lezcano <daniel.lezcano@linaro.org> пишет:
>
>> On 01/01/2016 02:24 PM, Roman Volkov wrote:
>>> From: Roman Volkov <rvolkov@v1ros.org>
>>>
>>> vt8500 hangs in nanosleep() function, starting from commit
>>> c6eb3f70d4482806dc2d3e1e3c7736f497b1d418, making the system
>>> unusable. Per investigation, looks like set_next_event() now
>>> receives too small delta and fails with -ETIME.
>>
>>
>> Hi Roman,
>>
>> I think the patch 1/3 should go as a fix for 4.4-rc7 and stable and
>> the two other should go for the next version as they are improvements
>> regarding the current code.
>>
>
> Hi Daniel,
>
> I agree, -stable can live without the patches 2 an 3.

Ok, I applied patch 1 for fix/urgent + stable and 2/3 for the next release.

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web