All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv5 00/10] clocksource: sunxi: Timer fixes and cleanup
@ 2013-07-11 16:31 ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

Hi everyone,

The first timer code we merged when adding support for the A13 some
time back was mostly a clean up from the source drop we had, without
any documentation.  This happened to work, but the code merged in
turned out to be far from perfect, and had several flaws.

This patchset hopefully fixes these flaws, and cleanup most of the
driver as well, to end up in an almost complete rewrite of it (even
though it's not that long).

It also finally adds a clocksource driver using the second timer as
our monotonic clock source.

These flaws have all been spotted when trying to add the A31 support,
work that is still ongoing, but will hopefully benefit from this
patchset as well.

Thanks,
Maxime

Changes from v4:
  - Removed the last clk_get_rate() user
  - Got a few beers

Changes from v3:
  - Reintroduce the rate variable to cache the parent clock rate
  - Remove the interval programming at probe time that was
    reintroduced in the v3 due to a poor rebase.

Changes from v2:
  - Use the clocksource timer to get the amount of time we have to
    wait for when disabling and enabling back a timer
  - Added patch to add parenthesis around the macros arguments
  - Renamed the AUTORELOAD register define to the more meaningful
    RELOAD name

Changes from v1:
  - Rebased on top of linux-next to benefit from the move to all
    architectures of the sched_clock functions
  - Moved the clock source to the second timer instead of the 64 bits
    free-running counter like suggested by Thomas.

Maxime Ripard (10):
  clocksource: sun4i: Use the BIT macros where possible
  clocksource: sun4i: Wrap macros arguments in parenthesis
  clocksource: sun4i: rename AUTORELOAD define to RELOAD
  clocksource: sun4i: Add clocksource and sched clock drivers
  clocksource: sun4i: Don't forget to enable the clock we use
  clocksource: sun4i: Fix the next event code
  clocksource: sun4i: Factor out some timer code
  clocksource: sun4i: Remove TIMER_SCAL variable
  clocksource: sun4i: Cleanup parent clock setup
  clocksource: sun4i: Fix bug when switching from periodic to oneshot
    modes

 drivers/clocksource/sun4i_timer.c | 110 +++++++++++++++++++++++++++-----------
 1 file changed, 78 insertions(+), 32 deletions(-)

-- 
1.8.3.2


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

* [PATCHv5 00/10] clocksource: sunxi: Timer fixes and cleanup
@ 2013-07-11 16:31 ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi everyone,

The first timer code we merged when adding support for the A13 some
time back was mostly a clean up from the source drop we had, without
any documentation.  This happened to work, but the code merged in
turned out to be far from perfect, and had several flaws.

This patchset hopefully fixes these flaws, and cleanup most of the
driver as well, to end up in an almost complete rewrite of it (even
though it's not that long).

It also finally adds a clocksource driver using the second timer as
our monotonic clock source.

These flaws have all been spotted when trying to add the A31 support,
work that is still ongoing, but will hopefully benefit from this
patchset as well.

Thanks,
Maxime

Changes from v4:
  - Removed the last clk_get_rate() user
  - Got a few beers

Changes from v3:
  - Reintroduce the rate variable to cache the parent clock rate
  - Remove the interval programming at probe time that was
    reintroduced in the v3 due to a poor rebase.

Changes from v2:
  - Use the clocksource timer to get the amount of time we have to
    wait for when disabling and enabling back a timer
  - Added patch to add parenthesis around the macros arguments
  - Renamed the AUTORELOAD register define to the more meaningful
    RELOAD name

Changes from v1:
  - Rebased on top of linux-next to benefit from the move to all
    architectures of the sched_clock functions
  - Moved the clock source to the second timer instead of the 64 bits
    free-running counter like suggested by Thomas.

Maxime Ripard (10):
  clocksource: sun4i: Use the BIT macros where possible
  clocksource: sun4i: Wrap macros arguments in parenthesis
  clocksource: sun4i: rename AUTORELOAD define to RELOAD
  clocksource: sun4i: Add clocksource and sched clock drivers
  clocksource: sun4i: Don't forget to enable the clock we use
  clocksource: sun4i: Fix the next event code
  clocksource: sun4i: Factor out some timer code
  clocksource: sun4i: Remove TIMER_SCAL variable
  clocksource: sun4i: Cleanup parent clock setup
  clocksource: sun4i: Fix bug when switching from periodic to oneshot
    modes

 drivers/clocksource/sun4i_timer.c | 110 +++++++++++++++++++++++++++-----------
 1 file changed, 78 insertions(+), 32 deletions(-)

-- 
1.8.3.2

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

* [PATCHv5 01/10] clocksource: sun4i: Use the BIT macros where possible
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index d4674e7..bdf34d9 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -24,12 +24,12 @@
 #include <linux/of_irq.h>
 
 #define TIMER_IRQ_EN_REG	0x00
-#define TIMER_IRQ_EN(val)		(1 << val)
+#define TIMER_IRQ_EN(val)		BIT(val)
 #define TIMER_IRQ_ST_REG	0x04
 #define TIMER_CTL_REG(val)	(0x10 * val + 0x10)
-#define TIMER_CTL_ENABLE		(1 << 0)
-#define TIMER_CTL_AUTORELOAD		(1 << 1)
-#define TIMER_CTL_ONESHOT		(1 << 7)
+#define TIMER_CTL_ENABLE		BIT(0)
+#define TIMER_CTL_AUTORELOAD		BIT(1)
+#define TIMER_CTL_ONESHOT		BIT(7)
 #define TIMER_INTVAL_REG(val)	(0x10 * val + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * val + 0x18)
 
-- 
1.8.3.2


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

* [PATCHv5 01/10] clocksource: sun4i: Use the BIT macros where possible
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index d4674e7..bdf34d9 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -24,12 +24,12 @@
 #include <linux/of_irq.h>
 
 #define TIMER_IRQ_EN_REG	0x00
-#define TIMER_IRQ_EN(val)		(1 << val)
+#define TIMER_IRQ_EN(val)		BIT(val)
 #define TIMER_IRQ_ST_REG	0x04
 #define TIMER_CTL_REG(val)	(0x10 * val + 0x10)
-#define TIMER_CTL_ENABLE		(1 << 0)
-#define TIMER_CTL_AUTORELOAD		(1 << 1)
-#define TIMER_CTL_ONESHOT		(1 << 7)
+#define TIMER_CTL_ENABLE		BIT(0)
+#define TIMER_CTL_AUTORELOAD		BIT(1)
+#define TIMER_CTL_ONESHOT		BIT(7)
 #define TIMER_INTVAL_REG(val)	(0x10 * val + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * val + 0x18)
 
-- 
1.8.3.2

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

* [PATCHv5 02/10] clocksource: sun4i: Wrap macros arguments in parenthesis
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The macros were not using parenthesis to escape the arguments passed to
them. It is pretty unsafe, so add those parenthesis.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index bdf34d9..34ab658 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -30,8 +30,8 @@
 #define TIMER_CTL_ENABLE		BIT(0)
 #define TIMER_CTL_AUTORELOAD		BIT(1)
 #define TIMER_CTL_ONESHOT		BIT(7)
-#define TIMER_INTVAL_REG(val)	(0x10 * val + 0x14)
-#define TIMER_CNTVAL_REG(val)	(0x10 * val + 0x18)
+#define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
+#define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
 
 #define TIMER_SCAL		16
 
-- 
1.8.3.2


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

* [PATCHv5 02/10] clocksource: sun4i: Wrap macros arguments in parenthesis
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The macros were not using parenthesis to escape the arguments passed to
them. It is pretty unsafe, so add those parenthesis.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index bdf34d9..34ab658 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -30,8 +30,8 @@
 #define TIMER_CTL_ENABLE		BIT(0)
 #define TIMER_CTL_AUTORELOAD		BIT(1)
 #define TIMER_CTL_ONESHOT		BIT(7)
-#define TIMER_INTVAL_REG(val)	(0x10 * val + 0x14)
-#define TIMER_CNTVAL_REG(val)	(0x10 * val + 0x18)
+#define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
+#define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
 
 #define TIMER_SCAL		16
 
-- 
1.8.3.2

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

* [PATCHv5 03/10] clocksource: sun4i: rename AUTORELOAD define to RELOAD
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The name AUTORELOAD was actually pretty bad since it doesn't make the
register reload the previous interval when it expires, but setting this
value pushes the new programmed interval to the internal timer counter.
Rename it to RELOAD instead.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 34ab658..f5e227b 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -28,7 +28,7 @@
 #define TIMER_IRQ_ST_REG	0x04
 #define TIMER_CTL_REG(val)	(0x10 * val + 0x10)
 #define TIMER_CTL_ENABLE		BIT(0)
-#define TIMER_CTL_AUTORELOAD		BIT(1)
+#define TIMER_CTL_RELOAD		BIT(1)
 #define TIMER_CTL_ONESHOT		BIT(7)
 #define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
@@ -129,7 +129,7 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	/* set mode to auto reload */
 	val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val | TIMER_CTL_AUTORELOAD, timer_base + TIMER_CTL_REG(0));
+	writel(val | TIMER_CTL_RELOAD, timer_base + TIMER_CTL_REG(0));
 
 	ret = setup_irq(irq, &sun4i_timer_irq);
 	if (ret)
-- 
1.8.3.2


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

* [PATCHv5 03/10] clocksource: sun4i: rename AUTORELOAD define to RELOAD
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The name AUTORELOAD was actually pretty bad since it doesn't make the
register reload the previous interval when it expires, but setting this
value pushes the new programmed interval to the internal timer counter.
Rename it to RELOAD instead.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 34ab658..f5e227b 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -28,7 +28,7 @@
 #define TIMER_IRQ_ST_REG	0x04
 #define TIMER_CTL_REG(val)	(0x10 * val + 0x10)
 #define TIMER_CTL_ENABLE		BIT(0)
-#define TIMER_CTL_AUTORELOAD		BIT(1)
+#define TIMER_CTL_RELOAD		BIT(1)
 #define TIMER_CTL_ONESHOT		BIT(7)
 #define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
@@ -129,7 +129,7 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	/* set mode to auto reload */
 	val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val | TIMER_CTL_AUTORELOAD, timer_base + TIMER_CTL_REG(0));
+	writel(val | TIMER_CTL_RELOAD, timer_base + TIMER_CTL_REG(0));
 
 	ret = setup_irq(irq, &sun4i_timer_irq);
 	if (ret)
-- 
1.8.3.2

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

* [PATCHv5 04/10] clocksource: sun4i: Add clocksource and sched clock drivers
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

Use the second timer found on the Allwinner SoCs as a clock source and
sched clock, that were both not used yet on these platforms.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index f5e227b..b581c93 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqreturn.h>
+#include <linux/sched_clock.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -96,6 +97,11 @@ static struct irqaction sun4i_timer_irq = {
 	.dev_id = &sun4i_clockevent,
 };
 
+static u32 sun4i_timer_sched_read(void)
+{
+	return ~readl(timer_base + TIMER_CNTVAL_REG(1));
+}
+
 static void __init sun4i_timer_init(struct device_node *node)
 {
 	unsigned long rate = 0;
@@ -117,6 +123,15 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	rate = clk_get_rate(clk);
 
+	writel(~0, timer_base + TIMER_INTVAL_REG(1));
+	writel(TIMER_CTL_ENABLE | TIMER_CTL_RELOAD |
+	       TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M),
+	       timer_base + TIMER_CTL_REG(1));
+
+	setup_sched_clock(sun4i_timer_sched_read, 32, rate);
+	clocksource_mmio_init(timer_base + TIMER_CNTVAL_REG(1), node->name,
+			      rate, 300, 32, clocksource_mmio_readl_down);
+
 	writel(rate / (TIMER_SCAL * HZ),
 	       timer_base + TIMER_INTVAL_REG(0));
 
-- 
1.8.3.2


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

* [PATCHv5 04/10] clocksource: sun4i: Add clocksource and sched clock drivers
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

Use the second timer found on the Allwinner SoCs as a clock source and
sched clock, that were both not used yet on these platforms.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index f5e227b..b581c93 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqreturn.h>
+#include <linux/sched_clock.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -96,6 +97,11 @@ static struct irqaction sun4i_timer_irq = {
 	.dev_id = &sun4i_clockevent,
 };
 
+static u32 sun4i_timer_sched_read(void)
+{
+	return ~readl(timer_base + TIMER_CNTVAL_REG(1));
+}
+
 static void __init sun4i_timer_init(struct device_node *node)
 {
 	unsigned long rate = 0;
@@ -117,6 +123,15 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	rate = clk_get_rate(clk);
 
+	writel(~0, timer_base + TIMER_INTVAL_REG(1));
+	writel(TIMER_CTL_ENABLE | TIMER_CTL_RELOAD |
+	       TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M),
+	       timer_base + TIMER_CTL_REG(1));
+
+	setup_sched_clock(sun4i_timer_sched_read, 32, rate);
+	clocksource_mmio_init(timer_base + TIMER_CNTVAL_REG(1), node->name,
+			      rate, 300, 32, clocksource_mmio_readl_down);
+
 	writel(rate / (TIMER_SCAL * HZ),
 	       timer_base + TIMER_INTVAL_REG(0));
 
-- 
1.8.3.2

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

* [PATCHv5 05/10] clocksource: sun4i: Don't forget to enable the clock we use
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

Even if in our case, this clock was non-gatable, used as a parent clock
for several IPs, it still is a good idea to enable it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index b581c93..8e9c651 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -120,6 +120,7 @@ static void __init sun4i_timer_init(struct device_node *node)
 	clk = of_clk_get(node, 0);
 	if (IS_ERR(clk))
 		panic("Can't get timer clock");
+	clk_prepare_enable(clk);
 
 	rate = clk_get_rate(clk);
 
-- 
1.8.3.2


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

* [PATCHv5 05/10] clocksource: sun4i: Don't forget to enable the clock we use
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

Even if in our case, this clock was non-gatable, used as a parent clock
for several IPs, it still is a good idea to enable it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index b581c93..8e9c651 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -120,6 +120,7 @@ static void __init sun4i_timer_init(struct device_node *node)
 	clk = of_clk_get(node, 0);
 	if (IS_ERR(clk))
 		panic("Can't get timer clock");
+	clk_prepare_enable(clk);
 
 	rate = clk_get_rate(clk);
 
-- 
1.8.3.2

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

* [PATCHv5 06/10] clocksource: sun4i: Fix the next event code
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The next_event logic was setting the next interval to fire in the
current timer value instead of the interval value register, which is
obviously wrong.

Plus, the logic to set the actual value was wrong as well: the interval
register can only be modified when the timer is disabled, and then
enable it back, otherwise, it'll have no effect. Fix this logic as well
since that code couldn't possibly work.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 8e9c651..7123f65 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -38,6 +38,20 @@
 
 static void __iomem *timer_base;
 
+/*
+ * When we disable a timer, we need to wait at least for 2 cycles of
+ * the timer source clock. We will use for that the clocksource timer
+ * that is already setup and runs at the same frequency than the other
+ * timers, and we never will be disabled.
+ */
+static void sun4i_clkevt_sync(void)
+{
+	u32 old = readl(timer_base + TIMER_CNTVAL_REG(1));
+
+	while ((old - readl(timer_base + TIMER_CNTVAL_REG(1))) < 3)
+		cpu_relax();
+}
+
 static void sun4i_clkevt_mode(enum clock_event_mode mode,
 			      struct clock_event_device *clk)
 {
@@ -63,9 +77,14 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode,
 static int sun4i_clkevt_next_event(unsigned long evt,
 				   struct clock_event_device *unused)
 {
-	u32 u = readl(timer_base + TIMER_CTL_REG(0));
-	writel(evt, timer_base + TIMER_CNTVAL_REG(0));
-	writel(u | TIMER_CTL_ENABLE | TIMER_CTL_AUTORELOAD,
+	u32 val = readl(timer_base + TIMER_CTL_REG(0));
+	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(0));
+	sun4i_clkevt_sync();
+
+	writel(evt, timer_base + TIMER_INTVAL_REG(0));
+
+	val = readl(timer_base + TIMER_CTL_REG(0));
+	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_AUTORELOAD,
 	       timer_base + TIMER_CTL_REG(0));
 
 	return 0;
-- 
1.8.3.2


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

* [PATCHv5 06/10] clocksource: sun4i: Fix the next event code
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The next_event logic was setting the next interval to fire in the
current timer value instead of the interval value register, which is
obviously wrong.

Plus, the logic to set the actual value was wrong as well: the interval
register can only be modified when the timer is disabled, and then
enable it back, otherwise, it'll have no effect. Fix this logic as well
since that code couldn't possibly work.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 8e9c651..7123f65 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -38,6 +38,20 @@
 
 static void __iomem *timer_base;
 
+/*
+ * When we disable a timer, we need to wait at least for 2 cycles of
+ * the timer source clock. We will use for that the clocksource timer
+ * that is already setup and runs at the same frequency than the other
+ * timers, and we never will be disabled.
+ */
+static void sun4i_clkevt_sync(void)
+{
+	u32 old = readl(timer_base + TIMER_CNTVAL_REG(1));
+
+	while ((old - readl(timer_base + TIMER_CNTVAL_REG(1))) < 3)
+		cpu_relax();
+}
+
 static void sun4i_clkevt_mode(enum clock_event_mode mode,
 			      struct clock_event_device *clk)
 {
@@ -63,9 +77,14 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode,
 static int sun4i_clkevt_next_event(unsigned long evt,
 				   struct clock_event_device *unused)
 {
-	u32 u = readl(timer_base + TIMER_CTL_REG(0));
-	writel(evt, timer_base + TIMER_CNTVAL_REG(0));
-	writel(u | TIMER_CTL_ENABLE | TIMER_CTL_AUTORELOAD,
+	u32 val = readl(timer_base + TIMER_CTL_REG(0));
+	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(0));
+	sun4i_clkevt_sync();
+
+	writel(evt, timer_base + TIMER_INTVAL_REG(0));
+
+	val = readl(timer_base + TIMER_CTL_REG(0));
+	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_AUTORELOAD,
 	       timer_base + TIMER_CTL_REG(0));
 
 	return 0;
-- 
1.8.3.2

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

* [PATCHv5 07/10] clocksource: sun4i: Factor out some timer code
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The set_next_event and set_mode callbacks share a lot of common code we
can easily factor to avoid duplication and mistakes.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 48 ++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 7123f65..dd78b63 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -52,24 +52,46 @@ static void sun4i_clkevt_sync(void)
 		cpu_relax();
 }
 
+static void sun4i_clkevt_time_stop(u8 timer)
+{
+	u32 val = readl(timer_base + TIMER_CTL_REG(timer));
+	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+	sun4i_clkevt_sync();
+}
+
+static void sun4i_clkevt_time_setup(u8 timer, unsigned long delay)
+{
+	writel(delay, timer_base + TIMER_INTVAL_REG(timer));
+}
+
+static void sun4i_clkevt_time_start(u8 timer, bool periodic)
+{
+	u32 val = readl(timer_base + TIMER_CTL_REG(timer));
+
+	if (periodic)
+		val &= ~TIMER_CTL_ONESHOT;
+	else
+		val |= TIMER_CTL_ONESHOT;
+
+	writel(val | TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+}
+
 static void sun4i_clkevt_mode(enum clock_event_mode mode,
 			      struct clock_event_device *clk)
 {
-	u32 u = readl(timer_base + TIMER_CTL_REG(0));
-
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
-		u &= ~(TIMER_CTL_ONESHOT);
-		writel(u | TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(0));
+		sun4i_clkevt_time_stop(0);
+		sun4i_clkevt_time_start(0, true);
 		break;
-
 	case CLOCK_EVT_MODE_ONESHOT:
-		writel(u | TIMER_CTL_ONESHOT, timer_base + TIMER_CTL_REG(0));
+		sun4i_clkevt_time_stop(0);
+		sun4i_clkevt_time_start(0, false);
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
 	default:
-		writel(u & ~(TIMER_CTL_ENABLE), timer_base + TIMER_CTL_REG(0));
+		sun4i_clkevt_time_stop(0);
 		break;
 	}
 }
@@ -77,15 +99,9 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode,
 static int sun4i_clkevt_next_event(unsigned long evt,
 				   struct clock_event_device *unused)
 {
-	u32 val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(0));
-	sun4i_clkevt_sync();
-
-	writel(evt, timer_base + TIMER_INTVAL_REG(0));
-
-	val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_AUTORELOAD,
-	       timer_base + TIMER_CTL_REG(0));
+	sun4i_clkevt_time_stop(0);
+	sun4i_clkevt_time_setup(0, evt);
+	sun4i_clkevt_time_start(0, false);
 
 	return 0;
 }
-- 
1.8.3.2


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

* [PATCHv5 07/10] clocksource: sun4i: Factor out some timer code
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The set_next_event and set_mode callbacks share a lot of common code we
can easily factor to avoid duplication and mistakes.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 48 ++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 7123f65..dd78b63 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -52,24 +52,46 @@ static void sun4i_clkevt_sync(void)
 		cpu_relax();
 }
 
+static void sun4i_clkevt_time_stop(u8 timer)
+{
+	u32 val = readl(timer_base + TIMER_CTL_REG(timer));
+	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+	sun4i_clkevt_sync();
+}
+
+static void sun4i_clkevt_time_setup(u8 timer, unsigned long delay)
+{
+	writel(delay, timer_base + TIMER_INTVAL_REG(timer));
+}
+
+static void sun4i_clkevt_time_start(u8 timer, bool periodic)
+{
+	u32 val = readl(timer_base + TIMER_CTL_REG(timer));
+
+	if (periodic)
+		val &= ~TIMER_CTL_ONESHOT;
+	else
+		val |= TIMER_CTL_ONESHOT;
+
+	writel(val | TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+}
+
 static void sun4i_clkevt_mode(enum clock_event_mode mode,
 			      struct clock_event_device *clk)
 {
-	u32 u = readl(timer_base + TIMER_CTL_REG(0));
-
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
-		u &= ~(TIMER_CTL_ONESHOT);
-		writel(u | TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(0));
+		sun4i_clkevt_time_stop(0);
+		sun4i_clkevt_time_start(0, true);
 		break;
-
 	case CLOCK_EVT_MODE_ONESHOT:
-		writel(u | TIMER_CTL_ONESHOT, timer_base + TIMER_CTL_REG(0));
+		sun4i_clkevt_time_stop(0);
+		sun4i_clkevt_time_start(0, false);
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
 	default:
-		writel(u & ~(TIMER_CTL_ENABLE), timer_base + TIMER_CTL_REG(0));
+		sun4i_clkevt_time_stop(0);
 		break;
 	}
 }
@@ -77,15 +99,9 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode,
 static int sun4i_clkevt_next_event(unsigned long evt,
 				   struct clock_event_device *unused)
 {
-	u32 val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(0));
-	sun4i_clkevt_sync();
-
-	writel(evt, timer_base + TIMER_INTVAL_REG(0));
-
-	val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_AUTORELOAD,
-	       timer_base + TIMER_CTL_REG(0));
+	sun4i_clkevt_time_stop(0);
+	sun4i_clkevt_time_setup(0, evt);
+	sun4i_clkevt_time_start(0, false);
 
 	return 0;
 }
-- 
1.8.3.2

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

* [PATCHv5 08/10] clocksource: sun4i: Remove TIMER_SCAL variable
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The prescaler is only used when using the internal low frequency
oscillator (at 32kHz). Since we're using the higher frequency oscillator
at 24MHz, we can just remove it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index dd78b63..3217adc 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -34,8 +34,6 @@
 #define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
 
-#define TIMER_SCAL		16
-
 static void __iomem *timer_base;
 
 /*
@@ -168,8 +166,7 @@ static void __init sun4i_timer_init(struct device_node *node)
 	clocksource_mmio_init(timer_base + TIMER_CNTVAL_REG(1), node->name,
 			      rate, 300, 32, clocksource_mmio_readl_down);
 
-	writel(rate / (TIMER_SCAL * HZ),
-	       timer_base + TIMER_INTVAL_REG(0));
+	writel(rate / HZ, timer_base + TIMER_INTVAL_REG(0));
 
 	/* set clock source to HOSC, 16 pre-division */
 	val = readl(timer_base + TIMER_CTL_REG(0));
@@ -192,8 +189,8 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	sun4i_clockevent.cpumask = cpumask_of(0);
 
-	clockevents_config_and_register(&sun4i_clockevent, rate / TIMER_SCAL,
-					0x1, 0xff);
+	clockevents_config_and_register(&sun4i_clockevent, rate, 0x1,
+					0xffffffff);
 }
 CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-timer",
 		       sun4i_timer_init);
-- 
1.8.3.2


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

* [PATCHv5 08/10] clocksource: sun4i: Remove TIMER_SCAL variable
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The prescaler is only used when using the internal low frequency
oscillator (at 32kHz). Since we're using the higher frequency oscillator
at 24MHz, we can just remove it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index dd78b63..3217adc 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -34,8 +34,6 @@
 #define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
 
-#define TIMER_SCAL		16
-
 static void __iomem *timer_base;
 
 /*
@@ -168,8 +166,7 @@ static void __init sun4i_timer_init(struct device_node *node)
 	clocksource_mmio_init(timer_base + TIMER_CNTVAL_REG(1), node->name,
 			      rate, 300, 32, clocksource_mmio_readl_down);
 
-	writel(rate / (TIMER_SCAL * HZ),
-	       timer_base + TIMER_INTVAL_REG(0));
+	writel(rate / HZ, timer_base + TIMER_INTVAL_REG(0));
 
 	/* set clock source to HOSC, 16 pre-division */
 	val = readl(timer_base + TIMER_CTL_REG(0));
@@ -192,8 +189,8 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	sun4i_clockevent.cpumask = cpumask_of(0);
 
-	clockevents_config_and_register(&sun4i_clockevent, rate / TIMER_SCAL,
-					0x1, 0xff);
+	clockevents_config_and_register(&sun4i_clockevent, rate, 0x1,
+					0xffffffff);
 }
 CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-timer",
 		       sun4i_timer_init);
-- 
1.8.3.2

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

* [PATCHv5 09/10] clocksource: sun4i: Cleanup parent clock setup
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The current bring-up code for the timer was overly complicated. The only
thing we need is actually which clock we want to use as source and
that's pretty much all. Let's keep it that way.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 3217adc..2fadb3b 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -30,6 +30,9 @@
 #define TIMER_CTL_REG(val)	(0x10 * val + 0x10)
 #define TIMER_CTL_ENABLE		BIT(0)
 #define TIMER_CTL_RELOAD		BIT(1)
+#define TIMER_CTL_CLK_SRC(val)		(((val) & 0x3) << 2)
+#define TIMER_CTL_CLK_SRC_OSC24M		(1)
+#define TIMER_CTL_CLK_PRES(val)		(((val) & 0x7) << 4)
 #define TIMER_CTL_ONESHOT		BIT(7)
 #define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
@@ -168,16 +171,8 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	writel(rate / HZ, timer_base + TIMER_INTVAL_REG(0));
 
-	/* set clock source to HOSC, 16 pre-division */
-	val = readl(timer_base + TIMER_CTL_REG(0));
-	val &= ~(0x07 << 4);
-	val &= ~(0x03 << 2);
-	val |= (4 << 4) | (1 << 2);
-	writel(val, timer_base + TIMER_CTL_REG(0));
-
-	/* set mode to auto reload */
-	val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val | TIMER_CTL_RELOAD, timer_base + TIMER_CTL_REG(0));
+	writel(TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M) | TIMER_CTL_RELOAD,
+	       timer_base + TIMER_CTL_REG(0));
 
 	ret = setup_irq(irq, &sun4i_timer_irq);
 	if (ret)
-- 
1.8.3.2


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

* [PATCHv5 09/10] clocksource: sun4i: Cleanup parent clock setup
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The current bring-up code for the timer was overly complicated. The only
thing we need is actually which clock we want to use as source and
that's pretty much all. Let's keep it that way.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 3217adc..2fadb3b 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -30,6 +30,9 @@
 #define TIMER_CTL_REG(val)	(0x10 * val + 0x10)
 #define TIMER_CTL_ENABLE		BIT(0)
 #define TIMER_CTL_RELOAD		BIT(1)
+#define TIMER_CTL_CLK_SRC(val)		(((val) & 0x3) << 2)
+#define TIMER_CTL_CLK_SRC_OSC24M		(1)
+#define TIMER_CTL_CLK_PRES(val)		(((val) & 0x7) << 4)
 #define TIMER_CTL_ONESHOT		BIT(7)
 #define TIMER_INTVAL_REG(val)	(0x10 * (val) + 0x14)
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
@@ -168,16 +171,8 @@ static void __init sun4i_timer_init(struct device_node *node)
 
 	writel(rate / HZ, timer_base + TIMER_INTVAL_REG(0));
 
-	/* set clock source to HOSC, 16 pre-division */
-	val = readl(timer_base + TIMER_CTL_REG(0));
-	val &= ~(0x07 << 4);
-	val &= ~(0x03 << 2);
-	val |= (4 << 4) | (1 << 2);
-	writel(val, timer_base + TIMER_CTL_REG(0));
-
-	/* set mode to auto reload */
-	val = readl(timer_base + TIMER_CTL_REG(0));
-	writel(val | TIMER_CTL_RELOAD, timer_base + TIMER_CTL_REG(0));
+	writel(TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M) | TIMER_CTL_RELOAD,
+	       timer_base + TIMER_CTL_REG(0));
 
 	ret = setup_irq(irq, &sun4i_timer_irq);
 	if (ret)
-- 
1.8.3.2

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

* [PATCHv5 10/10] clocksource: sun4i: Fix bug when switching from periodic to oneshot modes
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-11 16:31   ` Maxime Ripard
  -1 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner
  Cc: linux-arm-kernel, linux-kernel, Emilio Lopez, kevin.z.m.zh,
	sunny, shuge, linux-sunxi, Maxime Ripard

The interval was firing at was set up at probe time, and only changed in
the set_next_event, and never changed back, which is not really what is
expected.

When enabling the periodic mode, now set an interval to tick every
jiffy.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 2fadb3b..8ead025 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -38,6 +38,7 @@
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
 
 static void __iomem *timer_base;
+static u32 ticks_per_jiffy;
 
 /*
  * When we disable a timer, we need to wait at least for 2 cycles of
@@ -74,7 +75,8 @@ static void sun4i_clkevt_time_start(u8 timer, bool periodic)
 	else
 		val |= TIMER_CTL_ONESHOT;
 
-	writel(val | TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
+	       timer_base + TIMER_CTL_REG(timer));
 }
 
 static void sun4i_clkevt_mode(enum clock_event_mode mode,
@@ -83,6 +85,7 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode,
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		sun4i_clkevt_time_stop(0);
+		sun4i_clkevt_time_setup(0, ticks_per_jiffy);
 		sun4i_clkevt_time_start(0, true);
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
@@ -169,9 +172,9 @@ static void __init sun4i_timer_init(struct device_node *node)
 	clocksource_mmio_init(timer_base + TIMER_CNTVAL_REG(1), node->name,
 			      rate, 300, 32, clocksource_mmio_readl_down);
 
-	writel(rate / HZ, timer_base + TIMER_INTVAL_REG(0));
+	ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
 
-	writel(TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M) | TIMER_CTL_RELOAD,
+	writel(TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M),
 	       timer_base + TIMER_CTL_REG(0));
 
 	ret = setup_irq(irq, &sun4i_timer_irq);
-- 
1.8.3.2


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

* [PATCHv5 10/10] clocksource: sun4i: Fix bug when switching from periodic to oneshot modes
@ 2013-07-11 16:31   ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2013-07-11 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

The interval was firing at was set up at probe time, and only changed in
the set_next_event, and never changed back, which is not really what is
expected.

When enabling the periodic mode, now set an interval to tick every
jiffy.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/sun4i_timer.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
index 2fadb3b..8ead025 100644
--- a/drivers/clocksource/sun4i_timer.c
+++ b/drivers/clocksource/sun4i_timer.c
@@ -38,6 +38,7 @@
 #define TIMER_CNTVAL_REG(val)	(0x10 * (val) + 0x18)
 
 static void __iomem *timer_base;
+static u32 ticks_per_jiffy;
 
 /*
  * When we disable a timer, we need to wait at least for 2 cycles of
@@ -74,7 +75,8 @@ static void sun4i_clkevt_time_start(u8 timer, bool periodic)
 	else
 		val |= TIMER_CTL_ONESHOT;
 
-	writel(val | TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
+	       timer_base + TIMER_CTL_REG(timer));
 }
 
 static void sun4i_clkevt_mode(enum clock_event_mode mode,
@@ -83,6 +85,7 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode,
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		sun4i_clkevt_time_stop(0);
+		sun4i_clkevt_time_setup(0, ticks_per_jiffy);
 		sun4i_clkevt_time_start(0, true);
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
@@ -169,9 +172,9 @@ static void __init sun4i_timer_init(struct device_node *node)
 	clocksource_mmio_init(timer_base + TIMER_CNTVAL_REG(1), node->name,
 			      rate, 300, 32, clocksource_mmio_readl_down);
 
-	writel(rate / HZ, timer_base + TIMER_INTVAL_REG(0));
+	ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
 
-	writel(TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M) | TIMER_CTL_RELOAD,
+	writel(TIMER_CTL_CLK_SRC(TIMER_CTL_CLK_SRC_OSC24M),
 	       timer_base + TIMER_CTL_REG(0));
 
 	ret = setup_irq(irq, &sun4i_timer_irq);
-- 
1.8.3.2

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

* Re: [PATCHv5 00/10] clocksource: sunxi: Timer fixes and cleanup
  2013-07-11 16:31 ` Maxime Ripard
@ 2013-07-16 14:47   ` Daniel Lezcano
  -1 siblings, 0 replies; 24+ messages in thread
From: Daniel Lezcano @ 2013-07-16 14:47 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: John Stultz, Thomas Gleixner, linux-arm-kernel, linux-kernel,
	Emilio Lopez, kevin.z.m.zh, sunny, shuge, linux-sunxi

On 07/11/2013 06:31 PM, Maxime Ripard wrote:

Applied to my tree for 3.12.

Thanks
  -- Daniel


> Hi everyone,
> 
> The first timer code we merged when adding support for the A13 some
> time back was mostly a clean up from the source drop we had, without
> any documentation.  This happened to work, but the code merged in
> turned out to be far from perfect, and had several flaws.
> 
> This patchset hopefully fixes these flaws, and cleanup most of the
> driver as well, to end up in an almost complete rewrite of it (even
> though it's not that long).
> 
> It also finally adds a clocksource driver using the second timer as
> our monotonic clock source.
> 
> These flaws have all been spotted when trying to add the A31 support,
> work that is still ongoing, but will hopefully benefit from this
> patchset as well.
> 
> Thanks,
> Maxime
> 
> Changes from v4:
>   - Removed the last clk_get_rate() user
>   - Got a few beers
> 
> Changes from v3:
>   - Reintroduce the rate variable to cache the parent clock rate
>   - Remove the interval programming at probe time that was
>     reintroduced in the v3 due to a poor rebase.
> 
> Changes from v2:
>   - Use the clocksource timer to get the amount of time we have to
>     wait for when disabling and enabling back a timer
>   - Added patch to add parenthesis around the macros arguments
>   - Renamed the AUTORELOAD register define to the more meaningful
>     RELOAD name
> 
> Changes from v1:
>   - Rebased on top of linux-next to benefit from the move to all
>     architectures of the sched_clock functions
>   - Moved the clock source to the second timer instead of the 64 bits
>     free-running counter like suggested by Thomas.
> 
> Maxime Ripard (10):
>   clocksource: sun4i: Use the BIT macros where possible
>   clocksource: sun4i: Wrap macros arguments in parenthesis
>   clocksource: sun4i: rename AUTORELOAD define to RELOAD
>   clocksource: sun4i: Add clocksource and sched clock drivers
>   clocksource: sun4i: Don't forget to enable the clock we use
>   clocksource: sun4i: Fix the next event code
>   clocksource: sun4i: Factor out some timer code
>   clocksource: sun4i: Remove TIMER_SCAL variable
>   clocksource: sun4i: Cleanup parent clock setup
>   clocksource: sun4i: Fix bug when switching from periodic to oneshot
>     modes
> 
>  drivers/clocksource/sun4i_timer.c | 110 +++++++++++++++++++++++++++-----------
>  1 file changed, 78 insertions(+), 32 deletions(-)
> 


-- 
 <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


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

* [PATCHv5 00/10] clocksource: sunxi: Timer fixes and cleanup
@ 2013-07-16 14:47   ` Daniel Lezcano
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Lezcano @ 2013-07-16 14:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2013 06:31 PM, Maxime Ripard wrote:

Applied to my tree for 3.12.

Thanks
  -- Daniel


> Hi everyone,
> 
> The first timer code we merged when adding support for the A13 some
> time back was mostly a clean up from the source drop we had, without
> any documentation.  This happened to work, but the code merged in
> turned out to be far from perfect, and had several flaws.
> 
> This patchset hopefully fixes these flaws, and cleanup most of the
> driver as well, to end up in an almost complete rewrite of it (even
> though it's not that long).
> 
> It also finally adds a clocksource driver using the second timer as
> our monotonic clock source.
> 
> These flaws have all been spotted when trying to add the A31 support,
> work that is still ongoing, but will hopefully benefit from this
> patchset as well.
> 
> Thanks,
> Maxime
> 
> Changes from v4:
>   - Removed the last clk_get_rate() user
>   - Got a few beers
> 
> Changes from v3:
>   - Reintroduce the rate variable to cache the parent clock rate
>   - Remove the interval programming at probe time that was
>     reintroduced in the v3 due to a poor rebase.
> 
> Changes from v2:
>   - Use the clocksource timer to get the amount of time we have to
>     wait for when disabling and enabling back a timer
>   - Added patch to add parenthesis around the macros arguments
>   - Renamed the AUTORELOAD register define to the more meaningful
>     RELOAD name
> 
> Changes from v1:
>   - Rebased on top of linux-next to benefit from the move to all
>     architectures of the sched_clock functions
>   - Moved the clock source to the second timer instead of the 64 bits
>     free-running counter like suggested by Thomas.
> 
> Maxime Ripard (10):
>   clocksource: sun4i: Use the BIT macros where possible
>   clocksource: sun4i: Wrap macros arguments in parenthesis
>   clocksource: sun4i: rename AUTORELOAD define to RELOAD
>   clocksource: sun4i: Add clocksource and sched clock drivers
>   clocksource: sun4i: Don't forget to enable the clock we use
>   clocksource: sun4i: Fix the next event code
>   clocksource: sun4i: Factor out some timer code
>   clocksource: sun4i: Remove TIMER_SCAL variable
>   clocksource: sun4i: Cleanup parent clock setup
>   clocksource: sun4i: Fix bug when switching from periodic to oneshot
>     modes
> 
>  drivers/clocksource/sun4i_timer.c | 110 +++++++++++++++++++++++++++-----------
>  1 file changed, 78 insertions(+), 32 deletions(-)
> 


-- 
 <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

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

end of thread, other threads:[~2013-07-16 14:48 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-11 16:31 [PATCHv5 00/10] clocksource: sunxi: Timer fixes and cleanup Maxime Ripard
2013-07-11 16:31 ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 01/10] clocksource: sun4i: Use the BIT macros where possible Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 02/10] clocksource: sun4i: Wrap macros arguments in parenthesis Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 03/10] clocksource: sun4i: rename AUTORELOAD define to RELOAD Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 04/10] clocksource: sun4i: Add clocksource and sched clock drivers Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 05/10] clocksource: sun4i: Don't forget to enable the clock we use Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 06/10] clocksource: sun4i: Fix the next event code Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 07/10] clocksource: sun4i: Factor out some timer code Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 08/10] clocksource: sun4i: Remove TIMER_SCAL variable Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 09/10] clocksource: sun4i: Cleanup parent clock setup Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-11 16:31 ` [PATCHv5 10/10] clocksource: sun4i: Fix bug when switching from periodic to oneshot modes Maxime Ripard
2013-07-11 16:31   ` Maxime Ripard
2013-07-16 14:47 ` [PATCHv5 00/10] clocksource: sunxi: Timer fixes and cleanup Daniel Lezcano
2013-07-16 14:47   ` Daniel Lezcano

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.