Introduce atomic MMIO register modify
v5 cover Introduce atomic MMIO register modify
After this patchset stalled for several months, I'm picking where we left. The last conclusion [1] was that instead of doing the arch-generic from scratch, we'd start by implementing the API in ARM only.
In other words, this is basically a resend of v3, with some of the typos fixed and using raw spinlocks.
[1] http://www.spinics.net/lists/arm-kernel/msg271773.html
The patch that adds the atomic-modify API is PATCH 1. The other two are sent as usage examples.
Changes from v4:
* Returned to the ARM-specific implementation for this API. The arch-generic can be added later.
Changes from v3:
* Implemented an arch-generic atomic_io_modify(), as suggested by Baruch and Catalin.
* Add an ARM-specific variant, using relaxed R/W as Will suggested.
* Replaced spin_locks by raw_spin_locks, to protect the registers even on RT.
* Fixed two stupid typos.
Changes from v2:
* As suggested by Will Deacon, dropped the iowmb() barrier and use relaxed variants instead. See Will's explanation for
details: http://www.spinics.net/lists/arm-kernel/msg268775.html
* Use spin_{}_irqsave/restore to allow irq-context usage also suggested by Will Deacon.
* Re-worked the API semantics as proposed by Russell King.
Changes from v1:
* Added an io barrier iowmb() as suggested by Will Deacon, to ensure the writel gets completed before the spin_unlock().
If nobody has objections, I'll add patch 1/3 to ARM's tracking system as usual.
Thanks!
Ezequiel Garcia (3):
ARM: Introduce atomic MMIO modify
clocksource: orion: Use atomic access for shared registers
watchdog: orion: Use atomic access for shared registers
arch/arm/include/asm/io.h | 6 ++++++ arch/arm/kernel/io.c | 35 +++++++++++++++++++++++++++++++++++ drivers/clocksource/time-orion.c | 28 ++++++++++------------------ drivers/watchdog/orion_wdt.c | 8 ++------ 4 files changed, 53 insertions(+), 24 deletions(-)
Signature · 1 line
1.8.1.5
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Some SoC have MMIO regions that are shared across orthogonal subsystems. This commit implements a possible solution for the thread-safe access of such regions through a spinlock-protected API.
Concurrent access is protected with a single spinlock for the entire MMIO address space. While this protects shared-registers, it also serializes access to unrelated/unshared registers.
We add relaxed and non-relaxed variants, by using writel_relaxed and writel, respectively. The rationale for this is that some users may not require register write completion but only thread-safe access to a register.
--- arch/arm/include/asm/io.h | 6 ++++++ arch/arm/kernel/io.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)
arch/arm/include/asm/io.h +6 -0
@@ -38,6 +38,12 @@#define isa_bus_to_virt phys_to_virt/* +* Atomic MMIO-wide IO modify +*/ +extern void atomic_io_modify (void __iomem *reg,u32 mask,u32 set); +extern void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set); + +/* * Generic IO read/write. These perform native-endian accesses. Note * that some architectures will want to re-define __raw_{read,write}w. */
arch/arm/kernel/io.c +35 -0
@@ -1,6 +1,41 @@#include <linux/export.h> #include <linux/types.h> #include <linux/io.h> +#include <linux/spinlock.h> + +static DEFINE_RAW_SPINLOCK (__io_lock); + +/* +* Generic atomic MMIO modify. +* +* Allows thread-safe access to registers shared by unrelated subsystems. +* The access is protected by a single MMIO-wide lock. +*/ +void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed); + +void atomic_io_modify (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify);/* * Copy data from IO memory space to "real" memory space.
Signature · 1 line
1.8.1.5
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
arch/arm/kernel/io.c line 25
Quoted 12 lines · Ezequiel Garcia
arch/arm/kernel/io.c +12
-0+void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed);
This looks quite generic - why is it in architecture specific code?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131210/a9b3a487/attachment.sig>
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 16 lines · Mark Brown
arch/arm/kernel/io.c line 25
Quoted 12 lines · Ezequiel Garcia
arch/arm/kernel/io.c +12
-0+void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
because the _relaxed IO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 9 lines · Russell King - ARM Linux
Quoted 3 lines · Mark Brown
Quoted 1 line
+EXPORT_SYMBOL(atomic_io_modify_relaxed);
This looks quite generic - why is it in architecture specific code?
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.
Oh, that's depressing. I guess the non-relaxed version could be made
generic but meh.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131210/97105f31/attachment.sig>
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
In reply to Russell King - ARM LinuxQuoted 18 lines · Russell King - ARM Linux
Quoted 16 lines · Mark Brown
arch/arm/kernel/io.c line 25
Quoted 12 lines · Ezequiel Garcia
arch/arm/kernel/io.c +12
-0+void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
Quoted 3 lines
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.
Exactly.
Will? Catalin? Any comments on this?
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 19 lines · Ezequiel Garcia
Quoted 18 lines · Russell King - ARM Linux
Quoted 16 lines · Mark Brown
arch/arm/kernel/io.c line 25
Quoted 12 lines · Ezequiel Garcia
arch/arm/kernel/io.c +12
-0+void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
Quoted 7 lines
Quoted 3 lines
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.Exactly.
Will? Catalin? Any comments on this?
Yes: BenH and I managed to come up with an agreement on the relaxed I/O accessors during kernel summit which I need to write up and send out again. This would allow for a generic definition of the accessors, then this could potentially be done in core code.
That said, I think the code you have will work correctly for ARM.
Will
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 24 lines · Will Deacon
Quoted 23 lines · Ezequiel Garcia
Quoted 21 lines · Russell King - ARM Linux
Quoted 16 lines · Mark Brown
arch/arm/kernel/io.c line 25
Quoted 12 lines · Ezequiel Garcia
arch/arm/kernel/io.c +12
-0+void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.
Quoted 8 lines
Quoted 3 lines
Exactly.
Will? Catalin? Any comments on this?
Yes: BenH and I managed to come up with an agreement on the relaxed I/O accessors during kernel summit which I need to write up and send out again. This would allow for a generic definition of the accessors, then this could potentially be done in core code.
Do you prefer this series to wait then?
thx,
Jason.
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 27 lines · Jason Cooper
Quoted 26 lines · Will Deacon
Quoted 25 lines · Ezequiel Garcia
Quoted 18 lines · Russell King - ARM Linux
Quoted 16 lines · Mark Brown
arch/arm/kernel/io.c line 25
Quoted 12 lines · Ezequiel Garcia
arch/arm/kernel/io.c +12
-0+void atomic_io_modify_relaxed (void __iomem *reg,u32 mask,u32 set) +{ +unsigned long flags; +u32 value; + +raw_spin_lock_irqsave (&__io_lock, flags); + value =readl_relaxed (reg) & ~mask; + value |= (set & mask); +writel_relaxed (value, reg); +raw_spin_unlock_irqrestore (&__io_lock, flags); +} +EXPORT_SYMBOL (atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
Quoted 3 lines
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.Exactly.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Will? Catalin? Any comments on this?
Yes: BenH and I managed to come up with an agreement on the relaxed I/O accessors during kernel summit which I need to write up and send out again. This would allow for a generic definition of the accessors, then this could potentially be done in core code.
Do you prefer this series to wait then?
It'd be pretty unfair of me to insist that you wait; particularly when there's bound to be further discussion once I get a proposal out. I guess all I can ask is that you guys try to move this into core code once we have standard definitions for the relaxed accessors.
Is that reasonable?
Will
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 35 lines · Will Deacon
Quoted 34 lines · Jason Cooper
Quoted 32 lines · Will Deacon
Quoted 19 lines · Ezequiel Garcia
Quoted 18 lines · Russell King - ARM Linux
Quoted 16 lines · Mark Brown
Review
Quoted 12 lines · Ezequiel Garcia
(quoted diff) +12
-0+void atomic_io_modify_relaxed(void __iomem *reg, u32 mask, u32 set) +{ + unsigned long flags; + u32 value; + + raw_spin_lock_irqsave(&__io_lock, flags); + value = readl_relaxed(reg) & ~mask; + value |= (set & mask); + writel_relaxed(value, reg); + raw_spin_unlock_irqrestore(&__io_lock, flags); +} +EXPORT_SYMBOL(atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
Quoted 7 lines
Quoted 3 lines
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.Exactly.
Will? Catalin? Any comments on this?
Yes: BenH and I managed to come up with an agreement on the relaxed I/O accessors during kernel summit which I need to write up and send out again. This would allow for a generic definition of the accessors, then this could potentially be done in core code.
Quoted 8 lines
Quoted 1 line
Do you prefer this series to wait then?
It'd be pretty unfair of me to insist that you wait; particularly when there's bound to be further discussion once I get a proposal out. I guess all I can ask is that you guys try to move this into core code once we have standard definitions for the relaxed accessors.
Is that reasonable?
Fair enough, I'll add this email to my onice mail folder and we'll re-attack it once the core code is in place. No guarantees on timeline, though.
thx,
Jason.
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
In reply to Will DeaconQuoted 35 lines · Will Deacon
Quoted 34 lines · Jason Cooper
Quoted 32 lines · Will Deacon
Quoted 19 lines · Ezequiel Garcia
Quoted 18 lines · Russell King - ARM Linux
Quoted 16 lines · Mark Brown
Review
Quoted 12 lines · Ezequiel Garcia
(quoted diff) +12
-0+void atomic_io_modify_relaxed(void __iomem *reg, u32 mask, u32 set) +{ + unsigned long flags; + u32 value; + + raw_spin_lock_irqsave(&__io_lock, flags); + value = readl_relaxed(reg) & ~mask; + value |= (set & mask); + writel_relaxed(value, reg); + raw_spin_unlock_irqrestore(&__io_lock, flags); +} +EXPORT_SYMBOL(atomic_io_modify_relaxed);This looks quite generic - why is it in architecture specific code?
Quoted 7 lines
Quoted 3 lines
because the
_relaxedIO operators don't exist on other architectures, and there's been some discussion around whether they should with no conclusions being reached.Exactly.
Will? Catalin? Any comments on this?
Yes: BenH and I managed to come up with an agreement on the relaxed I/O accessors during kernel summit which I need to write up and send out again. This would allow for a generic definition of the accessors, then this could potentially be done in core code.
Quoted 9 lines
Quoted 1 line
Do you prefer this series to wait then?
It'd be pretty unfair of me to insist that you wait; particularly when there's bound to be further discussion once I get a proposal out. I guess all I can ask is that you guys try to move this into core code once we have standard definitions for the relaxed accessors.
Is that reasonable?
Of course. We'll take care of doing the proper work when the relaxed I/O moves forward.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
In reply to Ezequiel GarciaQuoted 8 lines · Ezequiel Garcia
Some SoC have MMIO regions that are shared across orthogonal subsystems. This commit implements a possible solution for the thread-safe access of such regions through a spinlock-protected API.
Concurrent access is protected with a single spinlock for the entire MMIO address space. While this protects shared-registers, it also serializes access to unrelated/unshared registers.
Quoted 5 lines
We add relaxed and non-relaxed variants, by using
writel_relaxedand writel, respectively. The rationale for this is that some users may not require register write completion but only thread-safe access to a register.Signed-off-by Ezequiel Garcia <[email protected]>
Okay, so this patch has been submitted to the patch system, but it contains no other tags other than Ezequiel's sign-off. Clearly other people *have* reviewed it.
Can we please have some acks etc for it please?
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit".
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 13 lines · Russell King - ARM Linux
Quoted 12 lines · Ezequiel Garcia
Some SoC have MMIO regions that are shared across orthogonal subsystems. This commit implements a possible solution for the thread-safe access of such regions through a spinlock-protected API.
Concurrent access is protected with a single spinlock for the entire MMIO address space. While this protects shared-registers, it also serializes access to unrelated/unshared registers.
We add relaxed and non-relaxed variants, by using
writel_relaxedand writel, respectively. The rationale for this is that some users may not require register write completion but only thread-safe access to a register.
Quoted 7 lines
Quoted 1 line
Signed-off-by Ezequiel Garcia <[email protected]>Okay, so this patch has been submitted to the patch system, but it contains no other tags other than Ezequiel's sign-off. Clearly other people *have* reviewed it.
Can we please have some acks etc for it please?
Happy New Year!
Jason.
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 16 lines · Jason Cooper
Quoted 15 lines · Russell King - ARM Linux
Quoted 13 lines · Ezequiel Garcia
Some SoC have MMIO regions that are shared across orthogonal subsystems. This commit implements a possible solution for the thread-safe access of such regions through a spinlock-protected API.
Concurrent access is protected with a single spinlock for the entire MMIO address space. While this protects shared-registers, it also serializes access to unrelated/unshared registers.
We add relaxed and non-relaxed variants, by using
writel_relaxedand writel, respectively. The rationale for this is that some users may not require register write completion but only thread-safe access to a register.Signed-off-by Ezequiel Garcia <[email protected]>
Quoted 8 lines
Quoted 5 lines
Okay, so this patch has been submitted to the patch system, but it contains no other tags other than Ezequiel's sign-off. Clearly other people *have* reviewed it.
Can we please have some acks etc for it please?
Acked-by Jason Cooper <[email protected]>
Thanks!
Will: Can you ack this patch as well?
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 17 lines · Ezequiel Garcia
Quoted 16 lines · Jason Cooper
Quoted 15 lines · Russell King - ARM Linux
Quoted 13 lines · Ezequiel Garcia
Some SoC have MMIO regions that are shared across orthogonal subsystems. This commit implements a possible solution for the thread-safe access of such regions through a spinlock-protected API.
Concurrent access is protected with a single spinlock for the entire MMIO address space. While this protects shared-registers, it also serializes access to unrelated/unshared registers.
We add relaxed and non-relaxed variants, by using
writel_relaxedand writel, respectively. The rationale for this is that some users may not require register write completion but only thread-safe access to a register.Signed-off-by Ezequiel Garcia <[email protected]>
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Okay, so this patch has been submitted to the patch system, but it contains no other tags other than Ezequiel's sign-off. Clearly other people *have* reviewed it.
Can we please have some acks etc for it please?
Acked-by Jason Cooper <[email protected]>
Catalin, Will: Can you ack as well, so Russell can take this?
Thanks!
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 22 lines · Ezequiel Garcia
Quoted 21 lines · Ezequiel Garcia
Quoted 20 lines · Jason Cooper
Quoted 19 lines · Russell King - ARM Linux
Quoted 8 lines · Ezequiel Garcia
Some SoC have MMIO regions that are shared across orthogonal subsystems. This commit implements a possible solution for the thread-safe access of such regions through a spinlock-protected API.
Concurrent access is protected with a single spinlock for the entire MMIO address space. While this protects shared-registers, it also serializes access to unrelated/unshared registers.
Quoted 5 lines
We add relaxed and non-relaxed variants, by using
writel_relaxedand writel, respectively. The rationale for this is that some users may not require register write completion but only thread-safe access to a register.Signed-off-by Ezequiel Garcia <[email protected]>Okay, so this patch has been submitted to the patch system, but it contains no other tags other than Ezequiel's sign-off. Clearly other people *have* reviewed it.
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Can we please have some acks etc for it please?
Acked-by Jason Cooper <[email protected]>Catalin, Will: Can you ack as well, so Russell can take this?
(with the condition that Will promises to sort out the generic relaxed IO accessors and move this to generic code ;))
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 7 lines · Catalin Marinas
Quoted 1 line · Ezequiel Garcia
Catalin, Will: Can you ack as well, so Russell can take this?
Acked-by Catalin Marinas <[email protected]>(with the condition that Will promises to sort out the generic relaxed IO accessors and move this to generic code ;))
I think that's an impossibility, as there's no such thing as a "generic relaxed IO accessor" and agreement on what that should be is very hard to come by.
Signature · 3 lines
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit".
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
Quoted 4 lines · Russell King - ARM Linux
Quoted 3 lines · Catalin Marinas
Quoted 1 line · Ezequiel Garcia
Catalin, Will: Can you ack as well, so Russell can take this?
Quoted 8 lines
Quoted 4 lines
Acked-by Catalin Marinas <[email protected]>(with the condition that Will promises to sort out the generic relaxed IO accessors and move this to generic code ;))
I think that's an impossibility, as there's no such thing as a "generic relaxed IO accessor" and agreement on what that should be is very hard to come by.
We'll see... I managed to get agreement with powerpc, and I think most other people (esp. x86) don't really care. I'll post an RFC when I've written something up. It will likely involve use of things like mmiowb for truly portable code, so how many drivers will actually be portable even after the generic definition of the relaxed accessors remains to be seen.
Will
PATCH v5 1/3 ARM: Introduce atomic MMIO modify
In reply to Russell King - ARM LinuxHey Russell,
Quoted 4 lines · Russell King - ARM Linux
Quoted 3 lines · Catalin Marinas
Quoted 1 line · Ezequiel Garcia
Catalin, Will: Can you ack as well, so Russell can take this?
Quoted 8 lines
Quoted 4 lines
Acked-by Catalin Marinas <[email protected]>(with the condition that Will promises to sort out the generic relaxed IO accessors and move this to generic code ;))
I think that's an impossibility, as there's no such thing as a "generic relaxed IO accessor" and agreement on what that should be is very hard to come by.
This has been Acked by myself and Catalin. Do think it will make it in to v3.14?
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7930/1
thx,
Jason.
PATCH v5 2/3 clocksource: orion: Use atomic access for shared registers
In reply to Ezequiel GarciaReplace the driver-specific thread-safe shared register API by the recently introduced atomic_io_clear_set().
--- drivers/clocksource/time-orion.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
drivers/clocksource/time-orion.c +10 -18
@@ -35,20 +35,6 @@#define ORION_ONESHOT_MAX 0xfffffffestatic 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);
Signature · 1 line
1.8.1.5
PATCH v5 2/3 clocksource: orion: Use atomic access for shared registers
Quoted 7 lines · Ezequiel Garcia
Replace the driver-specific thread-safe shared register API by the recently introduced
atomic_io_clear_set().Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/clocksource/time-orion.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
thx,
Jason.
PATCH v5 3/3 watchdog: orion: Use atomic access for shared registers
In reply to Ezequiel GarciaSince the timer control register is shared with the clocksource driver, use the recently introduced atomic_io_clear_set() to access such register.
--- drivers/watchdog/orion_wdt.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/watchdog/orion_wdt.c +2 -6
@@ -73,9 +73,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)writel (~WDT_INT_REQ ,BRIDGE_CAUSE );/* Enable watchdog timer */ - reg =readl (wdt_reg +TIMER_CTRL ); - reg |=WDT_EN ; -writel (reg, wdt_reg +TIMER_CTRL ); +atomic_io_modify (wdt_reg +TIMER_CTRL ,WDT_EN ,WDT_EN );/* Enable reset on watchdog */ reg =readl (RSTOUTn_MASK);
@@ -98,9 +96,7 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)writel (reg, RSTOUTn_MASK);/* Disable watchdog timer */ - reg =readl (wdt_reg +TIMER_CTRL ); - reg &= ~WDT_EN ; -writel (reg, wdt_reg +TIMER_CTRL ); +atomic_io_modify (wdt_reg +TIMER_CTRL ,WDT_EN ,0 );spin_unlock (&wdt_lock);return 0 ;
Signature · 1 line
1.8.1.5
PATCH v5 3/3 watchdog: orion: Use atomic access for shared registers
Quoted 7 lines · Ezequiel Garcia
Since the timer control register is shared with the clocksource driver, use the recently introduced
atomic_io_clear_set()to access such register.Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/watchdog/orion_wdt.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
thx,
Jason.
v5 cover Introduce atomic MMIO register modify
In reply to Ezequiel GarciaQuoted 8 lines · Ezequiel Garcia
After this patchset stalled for several months, I'm picking where we left. The last conclusion [1] was that instead of doing the arch-generic from scratch, we'd start by implementing the API in ARM only.
In other words, this is basically a resend of v3, with some of the typos fixed and using raw spinlocks.
...
Quoted 2 lines
If nobody has objections, I'll add patch 1/3 to ARM's tracking system as usual.
It looks like everyone's ok with that. We can submit the drivers for v3.15 unless Russell's willing to set up a topic branch for patches 2 and 3 to depend upon.
thx,
Jason.
v5 cover Introduce atomic MMIO register modify
Quoted 8 lines · Jason Cooper
Quoted 7 lines · Ezequiel Garcia
After this patchset stalled for several months, I'm picking where we left. The last conclusion [1] was that instead of doing the arch-generic from scratch, we'd start by implementing the API in ARM only.
In other words, this is basically a resend of v3, with some of the typos fixed and using raw spinlocks.
Quoted 9 lines
Quoted 1 line
...
Quoted 2 lines
If nobody has objections, I'll add patch 1/3 to ARM's tracking system as usual.
It looks like everyone's ok with that. We can submit the drivers for v3.15 unless Russell's willing to set up a topic branch for patches 2 and 3 to depend upon.
IMO, v3.15 is perfectly fine, there's no rush on the drivers.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com