| Message ID | 1390295561-3466-2-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday 21 January 2014 06:12:27 Ezequiel Garcia wrote: > 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. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> You add the new atomic mmio interfaces in an ARM global header file, but at the same time make them ARM-only. I'm not opposed to having interfaces like that, but I'm not convinced they are actually needed for this case and if we go there, it needs to be done more carefully and should be available for all architectures so that portable drivers can use them. It also seems to duplicate functionality that is already present in regmap-mmio. Arnd
On Tue, Jan 21, 2014 at 10:45:21AM +0100, Arnd Bergmann wrote: > On Tuesday 21 January 2014 06:12:27 Ezequiel Garcia wrote: > > 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. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > You add the new atomic mmio interfaces in an ARM global header file, > but at the same time make them ARM-only. I'm not opposed to having > interfaces like that, but I'm not convinced they are actually needed > for this case and if we go there, it needs to be done more carefully > and should be available for all architectures so that portable drivers > can use them. > Yes, true. We've discussed about this and even post an arch-generic API: http://lwn.net/Articles/564709/ In the end it was decided to keep it ARM-specific for now: http://www.spinics.net/lists/arm-kernel/msg271773.html Just for reference, the last posted patchset for the atomic I/O API is this: http://www.spinics.net/lists/arm-kernel/msg292775.html > It also seems to duplicate functionality that is already present in > regmap-mmio. > Yes, this is true. We tried to use regmap-mmio/syscon but we need this to initialize the clocksource which is too early for that to work: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg484535.html
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index fbeb39c..8aa4cca 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -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. */ diff --git a/arch/arm/kernel/io.c b/arch/arm/kernel/io.c index dcd5b4d..9203cf8 100644 --- a/arch/arm/kernel/io.c +++ b/arch/arm/kernel/io.c @@ -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.
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. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- Submitted to ARM patch tracker: http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7930/1 arch/arm/include/asm/io.h | 6 ++++++ arch/arm/kernel/io.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)