[PATCH 0/2] Make Armada 370/XP and Orion clocksource use atomic I/O API

[PATCH 0/2] Make Armada 370/XP and Orion clocksource use atomic I/O API

[Ezequiel Garcia]

The orion and armada-370-xp clocksource drivers are used in SoCs that can
use the orion-watchdog driver.

On these SoCs, the watchdog and timer blocks are part of the same set of
registers, and in particular the "control" register is used to enable/disable
both the watchdog and the timer.

Therefore, just as was done for orion-watchdog, we need to access the shared
register using the atomic I/O API.

This is v3.15 material, and meant to be merged through the clocksource tree,
if at all possible.

The first patch was already sent previously, but I'm resending it here to keep
the two patches together.

Thanks!

Ezequiel Garcia (2):
  clocksource: orion: Use atomic access for shared registers
  clocksource: armada-370-xp: Use atomic access for shared registers

 drivers/clocksource/time-armada-370-xp.c | 12 ++++--------
 drivers/clocksource/time-orion.c         | 28 ++++++++++------------------
 2 files changed, 14 insertions(+), 26 deletions(-)

-- 
1.8.1.5

[PATCH 1/2] clocksource: orion: Use atomic access for shared registers

[Ezequiel Garcia]

Replace the driver-specific thread-safe shared register API
by the recently introduced atomic_io_clear_set().

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Tested-by: Willy Tarreau <w@1wt.eu>
Acked-by: Jason Cooper <jason@lakedaemon.net>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/clocksource/time-orion.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/clocksource/time-orion.c b/drivers/clocksource/time-orion.c
index 2006622..0b3ce03 100644
--- a/drivers/clocksource/time-orion.c
+++ b/drivers/clocksource/time-orion.c
@@ -35,20 +35,6 @@
 #define ORION_ONESHOT_MAX	0xfffffffe
 
 static void __iomem *timer_base;
-static DEFINE_SPINLOCK(timer_ctrl_lock);
-
-/*
- * Thread-safe access to TIMER_CTRL register
- * (shared with watchdog timer)
- */
-void orion_timer_ctrl_clrset(u32 clr, u32 set)
-{
-	spin_lock(&timer_ctrl_lock);
-	writel((readl(timer_base + TIMER_CTRL) & ~clr) | set,
-		timer_base + TIMER_CTRL);
-	spin_unlock(&timer_ctrl_lock);
-}
-EXPORT_SYMBOL(orion_timer_ctrl_clrset);
 
 /*
  * Free-running clocksource handling.
@@ -68,7 +54,8 @@ static int orion_clkevt_next_event(unsigned long delta,
 {
 	/* setup and enable one-shot timer */
 	writel(delta, timer_base + TIMER1_VAL);
-	orion_timer_ctrl_clrset(TIMER1_RELOAD_EN, TIMER1_EN);
+	atomic_io_modify(timer_base + TIMER_CTRL,
+		TIMER1_RELOAD_EN | TIMER1_EN, TIMER1_EN);
 
 	return 0;
 }
@@ -80,10 +67,13 @@ static void orion_clkevt_mode(enum clock_event_mode mode,
 		/* setup and enable periodic timer at 1/HZ intervals */
 		writel(ticks_per_jiffy - 1, timer_base + TIMER1_RELOAD);
 		writel(ticks_per_jiffy - 1, timer_base + TIMER1_VAL);
-		orion_timer_ctrl_clrset(0, TIMER1_RELOAD_EN | TIMER1_EN);
+		atomic_io_modify(timer_base + TIMER_CTRL,
+			TIMER1_RELOAD_EN | TIMER1_EN,
+			TIMER1_RELOAD_EN | TIMER1_EN);
 	} else {
 		/* disable timer */
-		orion_timer_ctrl_clrset(TIMER1_RELOAD_EN | TIMER1_EN, 0);
+		atomic_io_modify(timer_base + TIMER_CTRL,
+			TIMER1_RELOAD_EN | TIMER1_EN, 0);
 	}
 }
 
@@ -131,7 +121,9 @@ static void __init orion_timer_init(struct device_node *np)
 	/* setup timer0 as free-running clocksource */
 	writel(~0, timer_base + TIMER0_VAL);
 	writel(~0, timer_base + TIMER0_RELOAD);
-	orion_timer_ctrl_clrset(0, TIMER0_RELOAD_EN | TIMER0_EN);
+	atomic_io_modify(timer_base + TIMER_CTRL,
+		TIMER0_RELOAD_EN | TIMER0_EN,
+		TIMER0_RELOAD_EN | TIMER0_EN);
 	clocksource_mmio_init(timer_base + TIMER0_VAL, "orion_clocksource",
 			      clk_get_rate(clk), 300, 32,
 			      clocksource_mmio_readl_down);
-- 
1.8.1.5

[PATCH 2/2] clocksource: armada-370-xp: Use atomic access for shared registers

[Ezequiel Garcia]

Replace the driver-specific thread-safe shared register API
by the recently introduced atomic_io_clear_set().

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/clocksource/time-armada-370-xp.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index ee8691b..0451e62 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -85,12 +85,6 @@ static u32 ticks_per_jiffy;
 
 static struct clock_event_device __percpu *armada_370_xp_evt;
 
-static void timer_ctrl_clrset(u32 clr, u32 set)
-{
-	writel((readl(timer_base + TIMER_CTRL_OFF) & ~clr) | set,
-		timer_base + TIMER_CTRL_OFF);
-}
-
 static void local_timer_ctrl_clrset(u32 clr, u32 set)
 {
 	writel((readl(local_base + TIMER_CTRL_OFF) & ~clr) | set,
@@ -245,7 +239,7 @@ static void __init armada_370_xp_timer_common_init(struct device_node *np)
 		clr = TIMER0_25MHZ;
 		enable_mask = TIMER0_EN | TIMER0_DIV(TIMER_DIVIDER_SHIFT);
 	}
-	timer_ctrl_clrset(clr, set);
+	atomic_io_modify(timer_base + TIMER_CTRL_OFF, clr | set, set);
 	local_timer_ctrl_clrset(clr, set);
 
 	/*
@@ -263,7 +257,9 @@ static void __init armada_370_xp_timer_common_init(struct device_node *np)
 	writel(0xffffffff, timer_base + TIMER0_VAL_OFF);
 	writel(0xffffffff, timer_base + TIMER0_RELOAD_OFF);
 
-	timer_ctrl_clrset(0, TIMER0_RELOAD_EN | enable_mask);
+	atomic_io_modify(timer_base + TIMER_CTRL_OFF,
+		TIMER0_RELOAD_EN | enable_mask,
+		TIMER0_RELOAD_EN | enable_mask);
 
 	/*
 	 * Set scale and timer for sched_clock.
-- 
1.8.1.5

[PATCH 1/2] clocksource: orion: Use atomic access for shared registers

[Daniel Lezcano]

On 02/19/2014 09:05 PM, Ezequiel Garcia wrote:
> Replace the driver-specific thread-safe shared register API
> by the recently introduced atomic_io_clear_set().

Applied to my tree for 3.15.

Thanks
   -- Daniel

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

[PATCH 2/2] clocksource: armada-370-xp: Use atomic access for shared registers

[Daniel Lezcano]

On 02/19/2014 09:05 PM, Ezequiel Garcia wrote:
> Replace the driver-specific thread-safe shared register API
> by the recently introduced atomic_io_clear_set().

Hi Ezequiel,

AFAICS, the code is lockless, your change adds a lock.

What race is it supposed to fix ?

> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>   drivers/clocksource/time-armada-370-xp.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index ee8691b..0451e62 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -85,12 +85,6 @@ static u32 ticks_per_jiffy;
>
>   static struct clock_event_device __percpu *armada_370_xp_evt;
>
> -static void timer_ctrl_clrset(u32 clr, u32 set)
> -{
> -	writel((readl(timer_base + TIMER_CTRL_OFF) & ~clr) | set,
> -		timer_base + TIMER_CTRL_OFF);
> -}
> -
>   static void local_timer_ctrl_clrset(u32 clr, u32 set)
>   {
>   	writel((readl(local_base + TIMER_CTRL_OFF) & ~clr) | set,
> @@ -245,7 +239,7 @@ static void __init armada_370_xp_timer_common_init(struct device_node *np)
>   		clr = TIMER0_25MHZ;
>   		enable_mask = TIMER0_EN | TIMER0_DIV(TIMER_DIVIDER_SHIFT);
>   	}
> -	timer_ctrl_clrset(clr, set);
> +	atomic_io_modify(timer_base + TIMER_CTRL_OFF, clr | set, set);
>   	local_timer_ctrl_clrset(clr, set);
>
>   	/*
> @@ -263,7 +257,9 @@ static void __init armada_370_xp_timer_common_init(struct device_node *np)
>   	writel(0xffffffff, timer_base + TIMER0_VAL_OFF);
>   	writel(0xffffffff, timer_base + TIMER0_RELOAD_OFF);
>
> -	timer_ctrl_clrset(0, TIMER0_RELOAD_EN | enable_mask);
> +	atomic_io_modify(timer_base + TIMER_CTRL_OFF,
> +		TIMER0_RELOAD_EN | enable_mask,
> +		TIMER0_RELOAD_EN | enable_mask);
>
>   	/*
>   	 * Set scale and timer for sched_clock.
>


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

[PATCH 2/2] clocksource: armada-370-xp: Use atomic access for shared registers

[Daniel Lezcano]

On 02/20/2014 08:59 AM, Daniel Lezcano wrote:
> On 02/19/2014 09:05 PM, Ezequiel Garcia wrote:
>> Replace the driver-specific thread-safe shared register API
>> by the recently introduced atomic_io_clear_set().
>
> Hi Ezequiel,
>
> AFAICS, the code is lockless, your change adds a lock.
>
> What race is it supposed to fix ?

Nevermind, I just noticed it is related to the watchdog driver.



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

[PATCH 2/2] clocksource: armada-370-xp: Use atomic access for shared registers

[Daniel Lezcano]

On 02/19/2014 09:05 PM, Ezequiel Garcia wrote:
> Replace the driver-specific thread-safe shared register API
> by the recently introduced atomic_io_clear_set().
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Applied to my tree for 3.15

Thanks !

   -- Daniel

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