[PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Will Deacon]

Hello,

This RFC series attempts to define a portable (i.e. cross-architecture)
definition of the {readX,writeX}_relaxed MMIO accessor functions. These
functions are already in widespread use amongst drivers (mainly those supporting
devices embedded in ARM SoCs), but lack any well-defined semantics and,
subsequently, any portable definitions to allow these drivers to be compiled for
other architectures.

The two main motivations for this series are:

 (1) To promote use of the _relaxed MMIO accessors on weakly-ordered
     architectures, where they can bring significant performance improvements
     over their non-relaxed counterparts.

 (2) To allow COMPILE_TEST to build drivers using the relaxed accessors across
     all architectures.

The proposed semantics largely match exactly those provided by the ARM
implementation (i.e. no weaker), with one exception (see below).

Informally:

  - Relaxed accesses to the same device are ordered with respect to each other.

  - Relaxed accesses are *not* guaranteed to be ordered with respect to normal
    memory accesses (e.g. DMA buffers -- this is what gives us the performance
    boost over the non-relaxed versions).

  - Relaxed accesses are not guaranteed to be ordered with respect to
    LOCK/UNLOCK operations.

In actual fact, the relaxed accessors *are* ordered with respect to LOCK/UNLOCK
operations on ARM[64], but I have added this constraint for the benefit of
PowerPC, which has expensive I/O barriers in the spin_unlock path for the
non-relaxed accessors.

A corollary to this is that mmiowb() probably needs rethinking. As it currently
stands, an mmiowb() is required to order MMIO writes to a device from multiple
CPUs, even if that device is protected by a lock. However, this isn't often used
in practice, leading to PowerPC implementing both mmiowb() *and* synchronising
I/O in spin_unlock.

I would propose making the non-relaxed I/O accessors ordered with respect to
LOCK/UNLOCK, leaving mmiowb() to be used with the relaxed accessors, if
required, but would welcome thoughts/suggestions on this topic.

All feedback welcome,

Will


Will Deacon (18):
  asm-generic: io: implement relaxed accessor macros as conditional
    wrappers
  microblaze: io: remove dummy relaxed accessor macros
  s390: io: remove dummy relaxed accessor macros for reads
  xtensa: io: remove dummy relaxed accessor macros for reads
  alpha: io: implement relaxed accessor macros for writes
  frv: io: implement dummy relaxed accessor macros for writes
  cris: io: implement dummy relaxed accessor macros for writes
  ia64: io: implement dummy relaxed accessor macros for writes
  m32r: io: implement dummy relaxed accessor macros for writes
  m68k: io: implement dummy relaxed accessor macros for writes
  mn10300: io: implement dummy relaxed accessor macros for writes
  parisc: io: implement dummy relaxed accessor macros for writes
  powerpc: io: implement dummy relaxed accessor macros for writes
  sparc: io: implement dummy relaxed accessor macros for writes
  tile: io: implement dummy relaxed accessor macros for writes
  x86: io: implement dummy relaxed accessor macros for writes
  documentation: memory-barriers: clarify relaxed io accessor semantics
  asm-generic: io: define relaxed accessor macros unconditionally

 Documentation/memory-barriers.txt | 13 +++++++++----
 arch/alpha/include/asm/io.h       | 12 ++++++++----
 arch/cris/include/asm/io.h        |  3 +++
 arch/frv/include/asm/io.h         |  3 +++
 arch/ia64/include/asm/io.h        |  4 ++++
 arch/m32r/include/asm/io.h        |  3 +++
 arch/m68k/include/asm/io.h        |  8 ++++++++
 arch/m68k/include/asm/io_no.h     |  4 ----
 arch/microblaze/include/asm/io.h  |  8 --------
 arch/mn10300/include/asm/io.h     |  4 ++++
 arch/parisc/include/asm/io.h      | 12 ++++++++----
 arch/powerpc/include/asm/io.h     | 12 ++++++++----
 arch/s390/include/asm/io.h        |  5 -----
 arch/sparc/include/asm/io.h       |  9 +++++++++
 arch/sparc/include/asm/io_32.h    |  3 ---
 arch/sparc/include/asm/io_64.h    | 22 ++++++++++------------
 arch/tile/include/asm/io.h        |  4 ++++
 arch/x86/include/asm/io.h         |  4 ++++
 arch/xtensa/include/asm/io.h      |  7 -------
 include/asm-generic/io.h          | 23 ++++++++++++++++-------
 20 files changed, 101 insertions(+), 62 deletions(-)

-- 
1.9.1

[PATCH 01/18] asm-generic: io: implement relaxed accessor macros as conditional wrappers

[Will Deacon]

{read,write}{b,w,l,q}_relaxed are implemented by some architectures in
order to permit memory-mapped I/O accesses with weaker barrier semantics
than the non-relaxed variants.

This patch adds wrappers to asm-generic so that drivers can rely on the
relaxed accessors being available, even if they don't always provide
weaker ordering guarantees. Since some architectures both include
asm-generic/io.h and define some relaxed accessors, the definitions here
are conditional for the time being.

Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 include/asm-generic/io.h | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 975e1cc75edb..6a93889aeb0d 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -52,15 +52,24 @@ static inline u32 __raw_readl(const volatile void __iomem *addr)
 }
 #endif
 
-#define readb __raw_readb
+#define readb		__raw_readb
+#ifndef readb_relaxed
+#define readb_relaxed	readb
+#endif
 
-#define readw readw
+#define readw		readw
+#ifndef readw_relaxed
+#define readw_relaxed	readw
+#endif
 static inline u16 readw(const volatile void __iomem *addr)
 {
 	return __le16_to_cpu(__raw_readw(addr));
 }
 
-#define readl readl
+#define readl		readl
+#ifndef readl_relaxed
+#define readl_relaxed	readl
+#endif
 static inline u32 readl(const volatile void __iomem *addr)
 {
 	return __le32_to_cpu(__raw_readl(addr));
@@ -87,9 +96,21 @@ static inline void __raw_writel(u32 b, volatile void __iomem *addr)
 }
 #endif
 
-#define writeb __raw_writeb
-#define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr)
-#define writel(b,addr) __raw_writel(__cpu_to_le32(b),addr)
+#define writeb		__raw_writeb
+#define writew(b,addr)	__raw_writew(__cpu_to_le16(b),addr)
+#define writel(b,addr)	__raw_writel(__cpu_to_le32(b),addr)
+
+#ifndef writeb_relaxed
+#define writeb_relaxed	writeb
+#endif
+
+#ifndef writew_relaxed
+#define writew_relaxed	writew
+#endif
+
+#ifndef writel_relaxed
+#define writel_relaxed	writel
+#endif
 
 #ifdef CONFIG_64BIT
 #ifndef __raw_readq
@@ -99,7 +120,10 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
 }
 #endif
 
-#define readq readq
+#define readq		readq
+#ifndef readq_relaxed
+#define readq_relaxed	readq
+#endif
 static inline u64 readq(const volatile void __iomem *addr)
 {
 	return __le64_to_cpu(__raw_readq(addr));
@@ -113,6 +137,9 @@ static inline void __raw_writeq(u64 b, volatile void __iomem *addr)
 #endif
 
 #define writeq(b, addr) __raw_writeq(__cpu_to_le64(b), addr)
+#ifndef writeq_relaxed
+#define writeq_relaxed	writeq
+#endif
 #endif /* CONFIG_64BIT */
 
 #ifndef PCI_IOBASE
-- 
1.9.1

[PATCH 02/18] microblaze: io: remove dummy relaxed accessor macros

[Will Deacon]

These are now defined by asm-generic/io.h, so we don't need the private
definitions anymore.

Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/microblaze/include/asm/io.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/microblaze/include/asm/io.h b/arch/microblaze/include/asm/io.h
index 1e4c3329f62e..2987fd4c73d6 100644
--- a/arch/microblaze/include/asm/io.h
+++ b/arch/microblaze/include/asm/io.h
@@ -72,12 +72,4 @@ extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
 
 #include <asm-generic/io.h>
 
-#define readb_relaxed	readb
-#define readw_relaxed	readw
-#define readl_relaxed	readl
-
-#define writeb_relaxed	writeb
-#define writew_relaxed	writew
-#define writel_relaxed	writel
-
 #endif /* _ASM_MICROBLAZE_IO_H */
-- 
1.9.1

Re: [PATCH 02/18] microblaze: io: remove dummy relaxed accessor macros

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

On 04/17/2014 03:44 PM, Will Deacon wrote:
> These are now defined by asm-generic/io.h, so we don't need the private
> definitions anymore.
> 
> Cc: Michal Simek <monstr@monstr.eu>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/microblaze/include/asm/io.h | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/io.h b/arch/microblaze/include/asm/io.h
> index 1e4c3329f62e..2987fd4c73d6 100644
> --- a/arch/microblaze/include/asm/io.h
> +++ b/arch/microblaze/include/asm/io.h
> @@ -72,12 +72,4 @@ extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
>  
>  #include <asm-generic/io.h>
>  
> -#define readb_relaxed	readb
> -#define readw_relaxed	readw
> -#define readl_relaxed	readl
> -
> -#define writeb_relaxed	writeb
> -#define writew_relaxed	writew
> -#define writel_relaxed	writel
> -
>  #endif /* _ASM_MICROBLAZE_IO_H */
> 

Acked-by: Michal Simek <monstr@monstr.eu>

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH 03/18] s390: io: remove dummy relaxed accessor macros for reads

[Will Deacon]

These are now defined by asm-generic/io.h, so we don't need the private
definitions anymore.

Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/s390/include/asm/io.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index cd6b9ee7b69c..722befdf8332 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -60,11 +60,6 @@ static inline void iounmap(volatile void __iomem *addr)
 #define __raw_writel	zpci_write_u32
 #define __raw_writeq	zpci_write_u64
 
-#define readb_relaxed	readb
-#define readw_relaxed	readw
-#define readl_relaxed	readl
-#define readq_relaxed	readq
-
 #endif /* CONFIG_PCI */
 
 #include <asm-generic/io.h>
-- 
1.9.1

[PATCH 04/18] xtensa: io: remove dummy relaxed accessor macros for reads

[Will Deacon]

These are now defined by asm-generic/io.h, so we don't need the private
definitions anymore.

Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/xtensa/include/asm/io.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/xtensa/include/asm/io.h b/arch/xtensa/include/asm/io.h
index 74944207167e..fe1600a09438 100644
--- a/arch/xtensa/include/asm/io.h
+++ b/arch/xtensa/include/asm/io.h
@@ -74,13 +74,6 @@ static inline void iounmap(volatile void __iomem *addr)
 
 #endif /* CONFIG_MMU */
 
-/*
- * Generic I/O
- */
-#define readb_relaxed readb
-#define readw_relaxed readw
-#define readl_relaxed readl
-
 #endif	/* __KERNEL__ */
 
 #include <asm-generic/io.h>
-- 
1.9.1

[PATCH 05/18] alpha: io: implement relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O writes with weaker barrier semantics than the
non-relaxed variants.

This patch implements these write macros for Alpha, in the same vein as
the relaxed read macros, which are already implemented.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/alpha/include/asm/io.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index 5ebab5895edb..f05bdb4b1cb9 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -500,10 +500,14 @@ extern inline void writeq(u64 b, volatile void __iomem *addr)
 #define outb_p		outb
 #define outw_p		outw
 #define outl_p		outl
-#define readb_relaxed(addr) __raw_readb(addr)
-#define readw_relaxed(addr) __raw_readw(addr)
-#define readl_relaxed(addr) __raw_readl(addr)
-#define readq_relaxed(addr) __raw_readq(addr)
+#define readb_relaxed(addr)	__raw_readb(addr)
+#define readw_relaxed(addr)	__raw_readw(addr)
+#define readl_relaxed(addr)	__raw_readl(addr)
+#define readq_relaxed(addr)	__raw_readq(addr)
+#define writeb_relaxed(b, addr)	__raw_writeb(b, addr)
+#define writew_relaxed(b, addr)	__raw_writew(b, addr)
+#define writel_relaxed(b, addr)	__raw_writel(b, addr)
+#define writeq_relaxed(b, addr)	__raw_writeq(b, addr)
 
 #define mmiowb()
 
-- 
1.9.1

[PATCH 06/18] frv: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to frv, in the same
vein as the dummy definitions for the relaxed read accessors.

Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/frv/include/asm/io.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
index 8cb50a2fbcb2..99bb7efaf9b7 100644
--- a/arch/frv/include/asm/io.h
+++ b/arch/frv/include/asm/io.h
@@ -243,6 +243,9 @@ static inline void writel(uint32_t datum, volatile void __iomem *addr)
 		__flush_PCI_writes();
 }
 
+#define writeb_relaxed writeb
+#define writew_relaxed writew
+#define writel_relaxed writel
 
 /* Values for nocacheflag and cmode */
 #define IOMAP_FULL_CACHING		0
-- 
1.9.1

[PATCH 07/18] cris: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to Cris, in the same
vein as the dummy definitions for the relaxed read accessors.

Cc: Mikael Starvik <starvik@axis.com>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/cris/include/asm/io.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/cris/include/asm/io.h b/arch/cris/include/asm/io.h
index e59dba12ce94..752a3f45df60 100644
--- a/arch/cris/include/asm/io.h
+++ b/arch/cris/include/asm/io.h
@@ -112,6 +112,9 @@ static inline void writel(unsigned int b, volatile void __iomem *addr)
 	else
 		*(volatile unsigned int __force *) addr = b;
 }
+#define writeb_relaxed(b, addr) writeb(b, addr)
+#define writew_relaxed(b, addr) writew(b, addr)
+#define writel_relaxed(b, addr) writel(b, addr)
 #define __raw_writeb writeb
 #define __raw_writew writew
 #define __raw_writel writel
-- 
1.9.1

Re: [PATCH 07/18] cris: io: implement dummy relaxed accessor macros for writes

[Jesper Nilsson]

On Thu, Apr 17, 2014 at 02:44:10PM +0100, Will Deacon wrote:
> write{b,w,l}_relaxed are implemented by some architectures in order to
> permit memory-mapped I/O accesses with weaker barrier semantics than the
> non-relaxed variants.
> 
> This patch adds dummy macros for the write accessors to Cris, in the same
> vein as the dummy definitions for the relaxed read accessors.
> 
> Cc: Mikael Starvik <starvik@axis.com>

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

> Signed-off-by: Will Deacon <will.deacon@arm.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[PATCH 08/18] ia64: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to ia64, which may
be able to be optimised in a similar manner to the relaxed read
accessors at a later date.

Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/ia64/include/asm/io.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index 0d2bcb37ec35..379165154c27 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -393,6 +393,10 @@ __writeq (unsigned long val, volatile void __iomem *addr)
 #define writew(v,a)	__writew((v), (a))
 #define writel(v,a)	__writel((v), (a))
 #define writeq(v,a)	__writeq((v), (a))
+#define writeb_relaxed(v,a)	__writeb((v), (a))
+#define writew_relaxed(v,a)	__writew((v), (a))
+#define writel_relaxed(v,a)	__writel((v), (a))
+#define writeq_relaxed(v,a)	__writeq((v), (a))
 #define __raw_writeb	writeb
 #define __raw_writew	writew
 #define __raw_writel	writel
-- 
1.9.1

[PATCH 09/18] m32r: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to m32r, in the
same vein as the dummy definitions for the relaxed read accessors.

Cc: Hirokazu Takata <takata@linux-m32r.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/m32r/include/asm/io.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/m32r/include/asm/io.h b/arch/m32r/include/asm/io.h
index 4010f1fc5b65..6e7787f3dac7 100644
--- a/arch/m32r/include/asm/io.h
+++ b/arch/m32r/include/asm/io.h
@@ -161,6 +161,9 @@ static inline void _writel(unsigned long l, unsigned long addr)
 #define __raw_writeb writeb
 #define __raw_writew writew
 #define __raw_writel writel
+#define writeb_relaxed writeb
+#define writew_relaxed writew
+#define writel_relaxed writel
 
 #define ioread8 read
 #define ioread16 readw
-- 
1.9.1

[PATCH 10/18] m68k: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to m68k, in the
same vein as the dummy definitions for the relaxed read accessors.
Additionally, the existing relaxed read accessors are moved into
asm/io.h, so that they can be used by m68k targets with an MMU.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/m68k/include/asm/io.h    | 8 ++++++++
 arch/m68k/include/asm/io_no.h | 4 ----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/m68k/include/asm/io.h b/arch/m68k/include/asm/io.h
index c70cc9155003..bccd5a914eb6 100644
--- a/arch/m68k/include/asm/io.h
+++ b/arch/m68k/include/asm/io.h
@@ -3,3 +3,11 @@
 #else
 #include <asm/io_mm.h>
 #endif
+
+#define readb_relaxed(addr)	readb(addr)
+#define readw_relaxed(addr)	readw(addr)
+#define readl_relaxed(addr)	readl(addr)
+
+#define writeb_relaxed(b, addr)	writeb(b, addr)
+#define writew_relaxed(b, addr)	writew(b, addr)
+#define writel_relaxed(b, addr)	writel(b, addr)
diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
index 52f7e8499172..19c237c63dc2 100644
--- a/arch/m68k/include/asm/io_no.h
+++ b/arch/m68k/include/asm/io_no.h
@@ -40,10 +40,6 @@ static inline unsigned int _swapl(volatile unsigned long v)
 #define readl(addr) \
     ({ unsigned int __v = (*(volatile unsigned int *) (addr)); __v; })
 
-#define readb_relaxed(addr) readb(addr)
-#define readw_relaxed(addr) readw(addr)
-#define readl_relaxed(addr) readl(addr)
-
 #define writeb(b,addr) (void)((*(volatile unsigned char *) (addr)) = (b))
 #define writew(b,addr) (void)((*(volatile unsigned short *) (addr)) = (b))
 #define writel(b,addr) (void)((*(volatile unsigned int *) (addr)) = (b))
-- 
1.9.1

Re: [PATCH 10/18] m68k: io: implement dummy relaxed accessor macros for writes

[Geert Uytterhoeven]

On Thu, Apr 17, 2014 at 3:44 PM, Will Deacon <will.deacon@arm.com> wrote:
> write{b,w,l}_relaxed are implemented by some architectures in order to
> permit memory-mapped I/O accesses with weaker barrier semantics than the
> non-relaxed variants.
>
> This patch adds dummy macros for the write accessors to m68k, in the
> same vein as the dummy definitions for the relaxed read accessors.
> Additionally, the existing relaxed read accessors are moved into
> asm/io.h, so that they can be used by m68k targets with an MMU.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 11/18] mn10300: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to mn10300, in the
same vein as the dummy definitions for the relaxed read accessors.

Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/mn10300/include/asm/io.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/mn10300/include/asm/io.h b/arch/mn10300/include/asm/io.h
index e6ed0d897ccc..897ba3c12b32 100644
--- a/arch/mn10300/include/asm/io.h
+++ b/arch/mn10300/include/asm/io.h
@@ -67,6 +67,10 @@ static inline void writel(u32 b, volatile void __iomem *addr)
 #define __raw_writew writew
 #define __raw_writel writel
 
+#define writeb_relaxed writeb
+#define writew_relaxed writew
+#define writel_relaxed writel
+
 /*****************************************************************************/
 /*
  * traditional input/output functions
-- 
1.9.1

[PATCH 12/18] parisc: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to parisc, in the
same vein as the dummy definitions for the relaxed read accessors.

Cc: Helge Deller <deller@gmx.de>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/parisc/include/asm/io.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index 1f6d2ae7aba5..8cd0abf28ffb 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -217,10 +217,14 @@ static inline void writeq(unsigned long long q, volatile void __iomem *addr)
 #define writel	writel
 #define writeq	writeq
 
-#define readb_relaxed(addr) readb(addr)
-#define readw_relaxed(addr) readw(addr)
-#define readl_relaxed(addr) readl(addr)
-#define readq_relaxed(addr) readq(addr)
+#define readb_relaxed(addr)	readb(addr)
+#define readw_relaxed(addr)	readw(addr)
+#define readl_relaxed(addr)	readl(addr)
+#define readq_relaxed(addr)	readq(addr)
+#define writeb_relaxed(b, addr)	writeb(b, addr)
+#define writew_relaxed(w, addr)	writew(w, addr)
+#define writel_relaxed(l, addr)	writel(l, addr)
+#define writeq_relaxed(q, addr)	writeq(q, addr)
 
 #define mmiowb() do { } while (0)
 
-- 
1.9.1

[PATCH 13/18] powerpc: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to powerpc, in the
same vein as the dummy definitions for the relaxed read accessors.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/powerpc/include/asm/io.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 97d3869991ca..9eaf301ac952 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -617,10 +617,14 @@ static inline void name at					\
 /*
  * We don't do relaxed operations yet, at least not with this semantic
  */
-#define readb_relaxed(addr) readb(addr)
-#define readw_relaxed(addr) readw(addr)
-#define readl_relaxed(addr) readl(addr)
-#define readq_relaxed(addr) readq(addr)
+#define readb_relaxed(addr)	readb(addr)
+#define readw_relaxed(addr)	readw(addr)
+#define readl_relaxed(addr)	readl(addr)
+#define readq_relaxed(addr)	readq(addr)
+#define writeb_relaxed(v, addr)	writeb(v, addr)
+#define writew_relaxed(v, addr)	writew(v, addr)
+#define writel_relaxed(v, addr)	writel(v, addr)
+#define writeq_relaxed(v, addr)	writeq(v, addr)
 
 #ifdef CONFIG_PPC32
 #define mmiowb()
-- 
1.9.1

[PATCH 14/18] sparc: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to sparc, in the
same vein as the dummy definitions for the relaxed read accessors. The
existing relaxed read{b,w,l} accessors are moved into asm/io.h, since
they are identical between 32-bit and 64-bit machines.

Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/sparc/include/asm/io.h    |  9 +++++++++
 arch/sparc/include/asm/io_32.h |  3 ---
 arch/sparc/include/asm/io_64.h | 22 ++++++++++------------
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
index f6902cf3cbe9..493f22c4684f 100644
--- a/arch/sparc/include/asm/io.h
+++ b/arch/sparc/include/asm/io.h
@@ -10,6 +10,15 @@
  * Defines used for both SPARC32 and SPARC64
  */
 
+/* Relaxed accessors for MMIO */
+#define readb_relaxed(__addr)		readb(__addr)
+#define readw_relaxed(__addr)		readw(__addr)
+#define readl_relaxed(__addr)		readl(__addr)
+
+#define writeb_relaxed(__b, __addr)	writeb(__b, __addr)
+#define writew_relaxed(__w, __addr)	writew(__w, __addr)
+#define writel_relaxed(__l, __addr)	writel(__l, __addr)
+
 /* Big endian versions of memory read/write routines */
 #define readb_be(__addr)	__raw_readb(__addr)
 #define readw_be(__addr)	__raw_readw(__addr)
diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h
index c1acbd891cbc..41d33e567f1d 100644
--- a/arch/sparc/include/asm/io_32.h
+++ b/arch/sparc/include/asm/io_32.h
@@ -89,9 +89,6 @@ static inline void __writel(u32 l, volatile void __iomem *addr)
 #define readb(__addr)		__readb(__addr)
 #define readw(__addr)		__readw(__addr)
 #define readl(__addr)		__readl(__addr)
-#define readb_relaxed(__addr)	readb(__addr)
-#define readw_relaxed(__addr)	readw(__addr)
-#define readl_relaxed(__addr)	readl(__addr)
 
 #define writeb(__b, __addr)	__writeb((__b),(__addr))
 #define writew(__w, __addr)	__writew((__w),(__addr))
diff --git a/arch/sparc/include/asm/io_64.h b/arch/sparc/include/asm/io_64.h
index 09b0b88aeb2a..2b4cd52831d0 100644
--- a/arch/sparc/include/asm/io_64.h
+++ b/arch/sparc/include/asm/io_64.h
@@ -203,18 +203,16 @@ static inline void _writeq(u64 q, volatile void __iomem *addr)
 			     : "memory");
 }
 
-#define readb(__addr)		_readb(__addr)
-#define readw(__addr)		_readw(__addr)
-#define readl(__addr)		_readl(__addr)
-#define readq(__addr)		_readq(__addr)
-#define readb_relaxed(__addr)	_readb(__addr)
-#define readw_relaxed(__addr)	_readw(__addr)
-#define readl_relaxed(__addr)	_readl(__addr)
-#define readq_relaxed(__addr)	_readq(__addr)
-#define writeb(__b, __addr)	_writeb(__b, __addr)
-#define writew(__w, __addr)	_writew(__w, __addr)
-#define writel(__l, __addr)	_writel(__l, __addr)
-#define writeq(__q, __addr)	_writeq(__q, __addr)
+#define readb(__addr)			_readb(__addr)
+#define readw(__addr)			_readw(__addr)
+#define readl(__addr)			_readl(__addr)
+#define readq(__addr)			_readq(__addr)
+#define readq_relaxed(__addr)		_readq(__addr)
+#define writeb(__b, __addr)		_writeb(__b, __addr)
+#define writew(__w, __addr)		_writew(__w, __addr)
+#define writel(__l, __addr)		_writel(__l, __addr)
+#define writeq(__q, __addr)		_writeq(__q, __addr)
+#define writeq_relaxed(__q, __addr)	_writeq(__q, __addr)
 
 /* Now versions without byte-swapping. */
 static inline u8 _raw_readb(unsigned long addr)
-- 
1.9.1

[PATCH 15/18] tile: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to tile, in the
same vein as the dummy definitions for the relaxed read accessors.

Cc: Chris Metcalf <cmetcalf@tilera.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/tile/include/asm/io.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/tile/include/asm/io.h b/arch/tile/include/asm/io.h
index 9fe434969fab..d372641054d9 100644
--- a/arch/tile/include/asm/io.h
+++ b/arch/tile/include/asm/io.h
@@ -241,6 +241,10 @@ static inline void writeq(u64 val, unsigned long addr)
 #define readw_relaxed readw
 #define readl_relaxed readl
 #define readq_relaxed readq
+#define writeb_relaxed writeb
+#define writew_relaxed writew
+#define writel_relaxed writel
+#define writeq_relaxed writeq
 
 #define ioread8 readb
 #define ioread16 readw
-- 
1.9.1

Re: [PATCH 15/18] tile: io: implement dummy relaxed accessor macros for writes

[Chris Metcalf]

On 4/17/2014 9:44 AM, Will Deacon wrote:
> write{b,w,l,q}_relaxed are implemented by some architectures in order to
> permit memory-mapped I/O accesses with weaker barrier semantics than the
> non-relaxed variants.
>
> This patch adds dummy macros for the write accessors to tile, in the
> same vein as the dummy definitions for the relaxed read accessors.
>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/tile/include/asm/io.h | 4 ++++
>  1 file changed, 4 insertions(+)

Acked-by: Chris Metcalf <cmetcalf@tilera.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

[PATCH 16/18] x86: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

write{b,w,l,q}_relaxed are implemented by some architectures in order to
permit memory-mapped I/O accesses with weaker barrier semantics than the
non-relaxed variants.

This patch adds dummy macros for the write accessors to x86, in the
same vein as the dummy definitions for the relaxed read accessors.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/x86/include/asm/io.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index b8237d8a1e0c..2ea07f5ec7b7 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -74,6 +74,9 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
 #define __raw_readw __readw
 #define __raw_readl __readl
 
+#define writeb_relaxed(v, a) __writeb(v, a)
+#define writew_relaxed(v, a) __writew(v, a)
+#define writel_relaxed(v, a) __writel(v, a)
 #define __raw_writeb __writeb
 #define __raw_writew __writew
 #define __raw_writel __writel
@@ -86,6 +89,7 @@ build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
 build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
 
 #define readq_relaxed(a)	readq(a)
+#define writeq_relaxed(v, a)	writeq(v, a)
 
 #define __raw_readq(a)		readq(a)
 #define __raw_writeq(val, addr)	writeq(val, addr)
-- 
1.9.1

Re: [PATCH 16/18] x86: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

On Thu, Apr 17, 2014 at 02:44:19PM +0100, Will Deacon wrote:
> write{b,w,l,q}_relaxed are implemented by some architectures in order to
> permit memory-mapped I/O accesses with weaker barrier semantics than the
> non-relaxed variants.
> 
> This patch adds dummy macros for the write accessors to x86, in the
> same vein as the dummy definitions for the relaxed read accessors.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/x86/include/asm/io.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index b8237d8a1e0c..2ea07f5ec7b7 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -74,6 +74,9 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
>  #define __raw_readw __readw
>  #define __raw_readl __readl
>  
> +#define writeb_relaxed(v, a) __writeb(v, a)
> +#define writew_relaxed(v, a) __writew(v, a)
> +#define writel_relaxed(v, a) __writel(v, a)
>  #define __raw_writeb __writeb
>  #define __raw_writew __writew
>  #define __raw_writel __writel
> @@ -86,6 +89,7 @@ build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
>  build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
>  
>  #define readq_relaxed(a)	readq(a)
> +#define writeq_relaxed(v, a)	writeq(v, a)

Actually, I should be using the regular (i.e. without the double underscore
prefix) accessors for the relaxed variants, including the existing read
flavours here. The proposed semantics are that the accessors are ordered
with respect to each other, which necessitates a compiler barrier.

Will

Re: [PATCH 16/18] x86: io: implement dummy relaxed accessor macros for writes

[Brian Norris]

Hi Will,

On Tue, Apr 22, 2014 at 05:08:21PM +0100, Will Deacon wrote:
> On Thu, Apr 17, 2014 at 02:44:19PM +0100, Will Deacon wrote:
> > write{b,w,l,q}_relaxed are implemented by some architectures in order to
> > permit memory-mapped I/O accesses with weaker barrier semantics than the
> > non-relaxed variants.
> > 
> > This patch adds dummy macros for the write accessors to x86, in the
> > same vein as the dummy definitions for the relaxed read accessors.
[...]
> > --- a/arch/x86/include/asm/io.h
> > +++ b/arch/x86/include/asm/io.h
> > @@ -74,6 +74,9 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
> >  #define __raw_readw __readw
> >  #define __raw_readl __readl
> >  
> > +#define writeb_relaxed(v, a) __writeb(v, a)
> > +#define writew_relaxed(v, a) __writew(v, a)
> > +#define writel_relaxed(v, a) __writel(v, a)
> >  #define __raw_writeb __writeb
> >  #define __raw_writew __writew
> >  #define __raw_writel __writel
[...]
> 
> Actually, I should be using the regular (i.e. without the double underscore
> prefix) accessors for the relaxed variants, including the existing read
> flavours here. The proposed semantics are that the accessors are ordered
> with respect to each other, which necessitates a compiler barrier.

Are you planning on resubmitting this series? I've run into several
situations in which I can't compile-test a driver on a different ARCH
just because of this issue.

Thanks,
Brian

Re: [PATCH 16/18] x86: io: implement dummy relaxed accessor macros for writes

[Will Deacon]

On Wed, May 21, 2014 at 02:53:27AM +0100, Brian Norris wrote:
> Hi Will,

Hello,

> On Tue, Apr 22, 2014 at 05:08:21PM +0100, Will Deacon wrote:
> > On Thu, Apr 17, 2014 at 02:44:19PM +0100, Will Deacon wrote:
> > > write{b,w,l,q}_relaxed are implemented by some architectures in order to
> > > permit memory-mapped I/O accesses with weaker barrier semantics than the
> > > non-relaxed variants.
> > > 
> > > This patch adds dummy macros for the write accessors to x86, in the
> > > same vein as the dummy definitions for the relaxed read accessors.
> [...]
> > > --- a/arch/x86/include/asm/io.h
> > > +++ b/arch/x86/include/asm/io.h
> > > @@ -74,6 +74,9 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
> > >  #define __raw_readw __readw
> > >  #define __raw_readl __readl
> > >  
> > > +#define writeb_relaxed(v, a) __writeb(v, a)
> > > +#define writew_relaxed(v, a) __writew(v, a)
> > > +#define writel_relaxed(v, a) __writel(v, a)
> > >  #define __raw_writeb __writeb
> > >  #define __raw_writew __writew
> > >  #define __raw_writel __writel
> [...]
> > 
> > Actually, I should be using the regular (i.e. without the double underscore
> > prefix) accessors for the relaxed variants, including the existing read
> > flavours here. The proposed semantics are that the accessors are ordered
> > with respect to each other, which necessitates a compiler barrier.
> 
> Are you planning on resubmitting this series? I've run into several
> situations in which I can't compile-test a driver on a different ARCH
> just because of this issue.

Yeah, I was just hoping for some input from Ben on the semantics I proposed.

I have a fix for the x86 patch, so I guess that justifies a v2. I'll post
something later on.

Cheers,

Will

[PATCH 17/18] documentation: memory-barriers: clarify relaxed io accessor semantics

[Will Deacon]

This patch extends the paragraph describing the relaxed read io accessors
so that the relaxed accessors are defined to be:

 - Ordered with respect to each other if accessing the same peripheral

 - Unordered with respect to normal memory accesses

 - Unordered with respect to LOCK/UNLOCK operations

Whilst many architectures will provide stricter semantics, ARM, Alpha and
PPC can achieve significant performance gains by taking advantage of some
or all of the above relaxations.

Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 Documentation/memory-barriers.txt | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 556f951f8626..f31c88691ee9 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -2462,10 +2462,15 @@ functions:
      Please refer to the PCI specification for more information on interactions
      between PCI transactions.
 
- (*) readX_relaxed()
-
-     These are similar to readX(), but are not guaranteed to be ordered in any
-     way. Be aware that there is no I/O read barrier available.
+ (*) readX_relaxed(), writeX_relaxed()
+
+     These are similar to readX() and writeX(), but provide weaker memory
+     ordering guarantees. Specifically, they do not guarantee ordering with
+     respect to normal memory accesses (e.g. DMA buffers) nor do they guarantee
+     ordering with respect to LOCK or UNLOCK operations. If the latter is
+     required, an mmiowb() barrier can be used. Note that relaxed accesses to
+     the same peripheral are guaranteed to be ordered with respect to each
+     other.
 
  (*) ioreadX(), iowriteX()
 
-- 
1.9.1

[PATCH 18/18] asm-generic: io: define relaxed accessor macros unconditionally

[Will Deacon]

Now that no architectures using asm-generic/io.h define their own relaxed
accessors, the dummy definitions can be used unconditionally.

Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 include/asm-generic/io.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 6a93889aeb0d..ad80cab2a758 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -53,23 +53,17 @@ static inline u32 __raw_readl(const volatile void __iomem *addr)
 #endif
 
 #define readb		__raw_readb
-#ifndef readb_relaxed
 #define readb_relaxed	readb
-#endif
 
 #define readw		readw
-#ifndef readw_relaxed
 #define readw_relaxed	readw
-#endif
 static inline u16 readw(const volatile void __iomem *addr)
 {
 	return __le16_to_cpu(__raw_readw(addr));
 }
 
 #define readl		readl
-#ifndef readl_relaxed
 #define readl_relaxed	readl
-#endif
 static inline u32 readl(const volatile void __iomem *addr)
 {
 	return __le32_to_cpu(__raw_readl(addr));
@@ -100,17 +94,9 @@ static inline void __raw_writel(u32 b, volatile void __iomem *addr)
 #define writew(b,addr)	__raw_writew(__cpu_to_le16(b),addr)
 #define writel(b,addr)	__raw_writel(__cpu_to_le32(b),addr)
 
-#ifndef writeb_relaxed
 #define writeb_relaxed	writeb
-#endif
-
-#ifndef writew_relaxed
 #define writew_relaxed	writew
-#endif
-
-#ifndef writel_relaxed
 #define writel_relaxed	writel
-#endif
 
 #ifdef CONFIG_64BIT
 #ifndef __raw_readq
@@ -121,9 +107,7 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
 #endif
 
 #define readq		readq
-#ifndef readq_relaxed
 #define readq_relaxed	readq
-#endif
 static inline u64 readq(const volatile void __iomem *addr)
 {
 	return __le64_to_cpu(__raw_readq(addr));
@@ -137,9 +121,7 @@ static inline void __raw_writeq(u64 b, volatile void __iomem *addr)
 #endif
 
 #define writeq(b, addr) __raw_writeq(__cpu_to_le64(b), addr)
-#ifndef writeq_relaxed
 #define writeq_relaxed	writeq
-#endif
 #endif /* CONFIG_64BIT */
 
 #ifndef PCI_IOBASE
-- 
1.9.1

Re: [PATCH 18/18] asm-generic: io: define relaxed accessor macros unconditionally

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On 04/17/2014 03:44 PM, Will Deacon wrote:
> Now that no architectures using asm-generic/io.h define their own relaxed
> accessors, the dummy definitions can be used unconditionally.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  include/asm-generic/io.h | 18 ------------------
>  1 file changed, 18 deletions(-)

Do we need this? I think there could be a need to overwrite them.
Currently we are just lucky that architecture which uses asm-generic/io.h
don't need to overwrite it.
But I expect that all archs should use asm-generic/io.h to clean
architecture io.h exactly I have done it for Microblaze.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 18/18] asm-generic: io: define relaxed accessor macros unconditionally

[Will Deacon]

Hi Michal,

On Tue, Apr 22, 2014 at 03:09:46PM +0100, Michal Simek wrote:
> On 04/17/2014 03:44 PM, Will Deacon wrote:
> > Now that no architectures using asm-generic/io.h define their own relaxed
> > accessors, the dummy definitions can be used unconditionally.
> > 
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  include/asm-generic/io.h | 18 ------------------
> >  1 file changed, 18 deletions(-)
> 
> Do we need this? I think there could be a need to overwrite them.
> Currently we are just lucky that architecture which uses asm-generic/io.h
> don't need to overwrite it.
> But I expect that all archs should use asm-generic/io.h to clean
> architecture io.h exactly I have done it for Microblaze.

I'm open to keeping the conditional definitions, but it introduces a
discrepancy with the non-relaxed versions, which are defined unconditionally
(although the underlying __raw_* accessors can be overridden).

Will

Re: [PATCH 18/18] asm-generic: io: define relaxed accessor macros unconditionally

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]

Hi Will,

On 04/22/2014 05:18 PM, Will Deacon wrote:
> Hi Michal,
> 
> On Tue, Apr 22, 2014 at 03:09:46PM +0100, Michal Simek wrote:
>> On 04/17/2014 03:44 PM, Will Deacon wrote:
>>> Now that no architectures using asm-generic/io.h define their own relaxed
>>> accessors, the dummy definitions can be used unconditionally.
>>>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>>> ---
>>>  include/asm-generic/io.h | 18 ------------------
>>>  1 file changed, 18 deletions(-)
>>
>> Do we need this? I think there could be a need to overwrite them.
>> Currently we are just lucky that architecture which uses asm-generic/io.h
>> don't need to overwrite it.
>> But I expect that all archs should use asm-generic/io.h to clean
>> architecture io.h exactly I have done it for Microblaze.
> 
> I'm open to keeping the conditional definitions, but it introduces a
> discrepancy with the non-relaxed versions, which are defined unconditionally
> (although the underlying __raw_* accessors can be overridden).

up to you.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 18/18] asm-generic: io: define relaxed accessor macros unconditionally

[Sam Ravnborg]

> But I expect that all archs should use asm-generic/io.h to clean
> architecture io.h exactly I have done it for Microblaze.
I took a quick look at sparc32 - doing this is a nice cleanup.
Thanks for the hint.

	Sam

Re: [PATCH 18/18] asm-generic: io: define relaxed accessor macros unconditionally

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On 04/23/2014 09:23 AM, Sam Ravnborg wrote:
>> But I expect that all archs should use asm-generic/io.h to clean
>> architecture io.h exactly I have done it for Microblaze.
> I took a quick look at sparc32 - doing this is a nice cleanup.
> Thanks for the hint.

Nice. I had to fix some sparse warning which was introduce
by this change. You should check them too if you haven't done it.

As follow up we should move these to asm-generic/io.h too
ioremap_writethrough
ioremap_nocache
ioremap_fullcache - used nowhere
ioremap_wc

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Peter Zijlstra]

On Thu, Apr 17, 2014 at 02:44:03PM +0100, Will Deacon wrote:
> Hello,
> 
> This RFC series attempts to define a portable (i.e. cross-architecture)
> definition of the {readX,writeX}_relaxed MMIO accessor functions. These
> functions are already in widespread use amongst drivers (mainly those supporting
> devices embedded in ARM SoCs), but lack any well-defined semantics and,
> subsequently, any portable definitions to allow these drivers to be compiled for
> other architectures.
> 
> The two main motivations for this series are:
> 
>  (1) To promote use of the _relaxed MMIO accessors on weakly-ordered
>      architectures, where they can bring significant performance improvements
>      over their non-relaxed counterparts.
> 
>  (2) To allow COMPILE_TEST to build drivers using the relaxed accessors across
>      all architectures.
> 
> The proposed semantics largely match exactly those provided by the ARM
> implementation (i.e. no weaker), with one exception (see below).
> 
> Informally:
> 
>   - Relaxed accesses to the same device are ordered with respect to each other.
> 
>   - Relaxed accesses are *not* guaranteed to be ordered with respect to normal
>     memory accesses (e.g. DMA buffers -- this is what gives us the performance
>     boost over the non-relaxed versions).
> 
>   - Relaxed accesses are not guaranteed to be ordered with respect to
>     LOCK/UNLOCK operations.
> 
> In actual fact, the relaxed accessors *are* ordered with respect to LOCK/UNLOCK
> operations on ARM[64], but I have added this constraint for the benefit of
> PowerPC, which has expensive I/O barriers in the spin_unlock path for the
> non-relaxed accessors.
> 
> A corollary to this is that mmiowb() probably needs rethinking. As it currently
> stands, an mmiowb() is required to order MMIO writes to a device from multiple
> CPUs, even if that device is protected by a lock. However, this isn't often used
> in practice, leading to PowerPC implementing both mmiowb() *and* synchronising
> I/O in spin_unlock.
> 
> I would propose making the non-relaxed I/O accessors ordered with respect to
> LOCK/UNLOCK, leaving mmiowb() to be used with the relaxed accessors, if
> required, but would welcome thoughts/suggestions on this topic.

So the non-relaxed ops already imply the expensive I/O barrier (mmiowb?)
and therefore, PPC can drop it from spin_unlock()?

Also, I read mmiowb() as MMIO-write-barrier(), what do we have to
order/contain mmio-reads?

I have _0_ experience with MMIO, so I've no idea if ordering/containing
reads is silly or not. 

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Will Deacon]

Hi Peter,

On Thu, Apr 17, 2014 at 03:00:36PM +0100, Peter Zijlstra wrote:
> On Thu, Apr 17, 2014 at 02:44:03PM +0100, Will Deacon wrote:
> > In actual fact, the relaxed accessors *are* ordered with respect to LOCK/UNLOCK
> > operations on ARM[64], but I have added this constraint for the benefit of
> > PowerPC, which has expensive I/O barriers in the spin_unlock path for the
> > non-relaxed accessors.
> > 
> > A corollary to this is that mmiowb() probably needs rethinking. As it currently
> > stands, an mmiowb() is required to order MMIO writes to a device from multiple
> > CPUs, even if that device is protected by a lock. However, this isn't often used
> > in practice, leading to PowerPC implementing both mmiowb() *and* synchronising
> > I/O in spin_unlock.
> > 
> > I would propose making the non-relaxed I/O accessors ordered with respect to
> > LOCK/UNLOCK, leaving mmiowb() to be used with the relaxed accessors, if
> > required, but would welcome thoughts/suggestions on this topic.
> 
> So the non-relaxed ops already imply the expensive I/O barrier (mmiowb?)
> and therefore, PPC can drop it from spin_unlock()?

Ben can probably help out here, but if my proposal went ahead (that is,
only the non-relaxed ops would imply mmiowb()), then it would actually
be implemented on PPC by having only those accessors call IO_SET_SYNC_FLAG,
which is checked during unlock (in SYNC_IO).

> Also, I read mmiowb() as MMIO-write-barrier(), what do we have to
> order/contain mmio-reads?

My understanding is that this is related to posted stores from different
CPUs being re-ordered on the bus, so I wouldn't expect reads to suffer
(although, since this isn't permitted on ARM, I'm guessing here).

Will

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Benjamin Herrenschmidt]

On Thu, 2014-04-17 at 16:00 +0200, Peter Zijlstra wrote:

> So the non-relaxed ops already imply the expensive I/O barrier (mmiowb?)
> and therefore, PPC can drop it from spin_unlock()?

We play a trick. We set a per-cpu flag in writeX and test it in unlock
before doing the barrier. Still better than having the barrier in every
MMIO at this stage for us.

Whether we want to change that with then new scheme ... we'll see.

> Also, I read mmiowb() as MMIO-write-barrier(), what do we have to
> order/contain mmio-reads?
> 
> I have _0_ experience with MMIO, so I've no idea if ordering/containing
> reads is silly or not. 

I will review the rest when I'm back from vacation (or maybe this
week-end).

Thanks Will for picking that up, it's long overdue :)

Cheers,
Ben.

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Will Deacon]

Hi Ben,

On Thu, Apr 17, 2014 at 10:36:58PM +0100, Benjamin Herrenschmidt wrote:
> On Thu, 2014-04-17 at 16:00 +0200, Peter Zijlstra wrote:
> 
> > So the non-relaxed ops already imply the expensive I/O barrier (mmiowb?)
> > and therefore, PPC can drop it from spin_unlock()?
> 
> We play a trick. We set a per-cpu flag in writeX and test it in unlock
> before doing the barrier. Still better than having the barrier in every
> MMIO at this stage for us.
> 
> Whether we want to change that with then new scheme ... we'll see.
> 
> > Also, I read mmiowb() as MMIO-write-barrier(), what do we have to
> > order/contain mmio-reads?
> > 
> > I have _0_ experience with MMIO, so I've no idea if ordering/containing
> > reads is silly or not. 
> 
> I will review the rest when I'm back from vacation (or maybe this
> week-end).

Did you get a chance to look at this? I've got a handful of Acks from other
architectures, and there's a bug to fix in the x86 patch but it seems daft
to send a v2 without talking about the fundamental rules of the accessors.

Cheers,

Will

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Sam Ravnborg]

On Thu, Apr 17, 2014 at 02:44:03PM +0100, Will Deacon wrote:
> Hello,
> 
> This RFC series attempts to define a portable (i.e. cross-architecture)
> definition of the {readX,writeX}_relaxed MMIO accessor functions. These
> functions are already in widespread use amongst drivers (mainly those supporting
> devices embedded in ARM SoCs), but lack any well-defined semantics and,
> subsequently, any portable definitions to allow these drivers to be compiled for
> other architectures.

Could this be made in such a way that only architectures that need
to provide their own versions actually have to add them?

The current patch-set adds the same dummy defines all over,
and will put this burden also on new architectures.

	Sam

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Will Deacon]

Hi Sam,

On Thu, Apr 17, 2014 at 04:36:38PM +0100, Sam Ravnborg wrote:
> On Thu, Apr 17, 2014 at 02:44:03PM +0100, Will Deacon wrote:
> > Hello,
> > 
> > This RFC series attempts to define a portable (i.e. cross-architecture)
> > definition of the {readX,writeX}_relaxed MMIO accessor functions. These
> > functions are already in widespread use amongst drivers (mainly those supporting
> > devices embedded in ARM SoCs), but lack any well-defined semantics and,
> > subsequently, any portable definitions to allow these drivers to be compiled for
> > other architectures.
> 
> Could this be made in such a way that only architectures that need
> to provide their own versions actually have to add them?
> 
> The current patch-set adds the same dummy defines all over,
> and will put this burden also on new architectures.

It shouldn't be a burden for new architectures, as they will use
asm-generic/io.h and get the definitions from there.

Will

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Sam Ravnborg]

On Thu, Apr 17, 2014 at 04:47:15PM +0100, Will Deacon wrote:
> Hi Sam,
> 
> On Thu, Apr 17, 2014 at 04:36:38PM +0100, Sam Ravnborg wrote:
> > On Thu, Apr 17, 2014 at 02:44:03PM +0100, Will Deacon wrote:
> > > Hello,
> > > 
> > > This RFC series attempts to define a portable (i.e. cross-architecture)
> > > definition of the {readX,writeX}_relaxed MMIO accessor functions. These
> > > functions are already in widespread use amongst drivers (mainly those supporting
> > > devices embedded in ARM SoCs), but lack any well-defined semantics and,
> > > subsequently, any portable definitions to allow these drivers to be compiled for
> > > other architectures.
> > 
> > Could this be made in such a way that only architectures that need
> > to provide their own versions actually have to add them?
> > 
> > The current patch-set adds the same dummy defines all over,
> > and will put this burden also on new architectures.
> 
> It shouldn't be a burden for new architectures, as they will use
> asm-generic/io.h and get the definitions from there.

Why is it then necesary to do this for sparc:
diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
index f6902cf3cbe9..493f22c4684f 100644
--- a/arch/sparc/include/asm/io.h
+++ b/arch/sparc/include/asm/io.h
@@ -10,6 +10,15 @@
  * Defines used for both SPARC32 and SPARC64
  */

+/* Relaxed accessors for MMIO */
+#define readb_relaxed(__addr)          readb(__addr)
+#define readw_relaxed(__addr)          readw(__addr)
+#define readl_relaxed(__addr)          readl(__addr)
+
+#define writeb_relaxed(__b, __addr)    writeb(__b, __addr)
+#define writew_relaxed(__w, __addr)    writew(__w, __addr)
+#define writel_relaxed(__l, __addr)    writel(__l, __addr)

And similar for several other architectures.

For asm-generic/io.h:
+#ifndef readb_relaxed
+#define readb_relaxed  readb
+#endif

This has same effect as the above.
Only difference is that the implementation in asm-generic lacks the arguments.

The patch also breaks the pattern that the #define foobar foobar is
on the line just above the static inline that implements the function.

-#define readw readw
+#define readw          readw

+#ifndef readw_relaxed
+#define readw_relaxed  readw
+#endif
Move this blow below the static inline would make this easier to understand.

 static inline u16 readw(const volatile void __iomem *addr)
 {
        return __le16_to_cpu(__raw_readw(addr));
 }


	Sam

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Will Deacon]

Hello Sam,

On Thu, Apr 17, 2014 at 08:15:55PM +0100, Sam Ravnborg wrote:
> On Thu, Apr 17, 2014 at 04:47:15PM +0100, Will Deacon wrote:
> > On Thu, Apr 17, 2014 at 04:36:38PM +0100, Sam Ravnborg wrote:
> > > On Thu, Apr 17, 2014 at 02:44:03PM +0100, Will Deacon wrote:
> > > > This RFC series attempts to define a portable (i.e. cross-architecture)
> > > > definition of the {readX,writeX}_relaxed MMIO accessor functions. These
> > > > functions are already in widespread use amongst drivers (mainly those supporting
> > > > devices embedded in ARM SoCs), but lack any well-defined semantics and,
> > > > subsequently, any portable definitions to allow these drivers to be compiled for
> > > > other architectures.
> > > 
> > > Could this be made in such a way that only architectures that need
> > > to provide their own versions actually have to add them?
> > > 
> > > The current patch-set adds the same dummy defines all over,
> > > and will put this burden also on new architectures.
> > 
> > It shouldn't be a burden for new architectures, as they will use
> > asm-generic/io.h and get the definitions from there.
> 
> Why is it then necesary to do this for sparc:
> diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
> index f6902cf3cbe9..493f22c4684f 100644
> --- a/arch/sparc/include/asm/io.h
> +++ b/arch/sparc/include/asm/io.h
> @@ -10,6 +10,15 @@
>   * Defines used for both SPARC32 and SPARC64
>   */
> 
> +/* Relaxed accessors for MMIO */
> +#define readb_relaxed(__addr)          readb(__addr)
> +#define readw_relaxed(__addr)          readw(__addr)
> +#define readl_relaxed(__addr)          readl(__addr)
> +
> +#define writeb_relaxed(__b, __addr)    writeb(__b, __addr)
> +#define writew_relaxed(__w, __addr)    writew(__w, __addr)
> +#define writel_relaxed(__l, __addr)    writel(__l, __addr)
> 
> And similar for several other architectures.

This is because Sparc (and the other architectures I had to modify) don't
make use of asm-generic/io.h. Furthermore, it's not as simple as adding an
include, since you'll pull in the generic definitions of things like readw
and inb as it stands.

We could make a new asm-generic file purely for the relaxed accessors, but
I really don't think it's worth the hassle.

> For asm-generic/io.h:
> +#ifndef readb_relaxed
> +#define readb_relaxed  readb
> +#endif
> 
> This has same effect as the above.
> Only difference is that the implementation in asm-generic lacks the arguments.

I just followed the existing style in asm-generic/io.h.

> The patch also breaks the pattern that the #define foobar foobar is
> on the line just above the static inline that implements the function.
> 
> -#define readw readw
> +#define readw          readw
> 
> +#ifndef readw_relaxed
> +#define readw_relaxed  readw
> +#endif
> Move this blow below the static inline would make this easier to understand.
> 
>  static inline u16 readw(const volatile void __iomem *addr)
>  {
>         return __le16_to_cpu(__raw_readw(addr));
>  }

Sure, I can fix that.

Will

Re: [PATCH 00/18] Cross-architecture definitions of relaxed MMIO accessors

[Sam Ravnborg]

> 
> This is because Sparc (and the other architectures I had to modify) don't
> make use of asm-generic/io.h.
Somehow I missed this.

Added to my todo list to look into sparc in this respect.
A quick lokk reveal that sparc32 can use most/all of the generic io.h

	Sam