[PATCH 0/2] ARC io-accessors updates for 7.4

[PATCH 0/2] ARC io-accessors updates for 7.4

[Vineet Gupta]

Hi,

A couple of patches that I plan to send for v7.4.
One is an old issue and other a cleanup out of it.
CC'ing Arnd to ensure 2/2 is in the right direction.


Thx,
-Vineet:


Vineet Gupta (2):
  ARC: io: work around GCC PR127718 in __raw_write{w,l}()
  ARC: io: use asm-generic MMIO accessors

 arch/arc/include/asm/io.h | 134 ++++++--------------------------------
 1 file changed, 19 insertions(+), 115 deletions(-)

-- 
2.53.0

[PATCH 1/2] ARC: io: work around GCC PR127718 in __raw_write{w,l}()

[Vineet Gupta]

The __raw_write{w,l}() accessors use inline asm with an "m" operand for
the store destination. GCC's ARC backend lets "m" match any legitimate
address, including the register-indexed [b,c] forms that only loads
support: stores only have [reg], [reg,s9] and [limm] addressing. With
newer GCC (late-combine in GCC 15, combine in GCC 16), a writew() with a
scaled, non-constant offset gets

	stw.as r2,[r0,r1]

which the assembler rejects:

  Error: inappropriate arguments for opcode 'stw'

With GCC 16 this breaks even hsdk_defconfig (drivers/net/pcs/
pcs-xpcs-plat.c) and axs101_defconfig. allyesconfig additionally trips
in drivers/pinctrl/renesas/pinctrl-rza2.c and drivers/video/fbdev/
s3c-fb.c.

Work around it by passing the address in a register and using a fixed
[%1] address in the asm template, so the compiler can't pick an
addressing mode that stores don't support. This is the minimal fix.
The next patch drops the inline asm altogether.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127718
Cc: stable@vger.kernel.org
Signed-off-by: Vineet Gupta <vgupta@kernel.org>
---
 arch/arc/include/asm/io.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 00171a212b3c..b4a3393d1bdc 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -133,10 +133,14 @@ static inline void __raw_writeb(u8 b, volatile void __iomem *addr)
 #define __raw_writew __raw_writew
 static inline void __raw_writew(u16 s, volatile void __iomem *addr)
 {
+	/*
+	 * Not an "m" operand: GCC can pick a register-indexed address for it,
+	 * which ST doesn't support (GCC PR target/127718).
+	 */
 	__asm__ __volatile__(
-	"	stw%U1 %0, %1	\n"
+	"	stw %0, [%1]	\n"
 	:
-	: "r" (s), "m" (*(volatile u16 __force *)addr)
+	: "r" (s), "r" (addr)
 	: "memory");
 
 }
@@ -144,10 +148,11 @@ static inline void __raw_writew(u16 s, volatile void __iomem *addr)
 #define __raw_writel __raw_writel
 static inline void __raw_writel(u32 w, volatile void __iomem *addr)
 {
+	/* See __raw_writew() above for why this doesn't use an "m" operand. */
 	__asm__ __volatile__(
-	"	st%U1 %0, %1	\n"
+	"	st %0, [%1]	\n"
 	:
-	: "r" (w), "m" (*(volatile u32 __force *)addr)
+	: "r" (w), "r" (addr)
 	: "memory");
 
 }
-- 
2.53.0

[PATCH 2/2] ARC: io: use asm-generic MMIO accessors

[Vineet Gupta]

ARC MMIO accessors use inline asm, which isn't really needed. It was
introduced with the original port back in 2013.

As the previous patch showed, inline asm can be error-prone. Letting the
compiler emit the accesses also allows 16-bit encodings, scaled-index
loads and folding constant offsets (s9) into the access.

Without the asm, ARC's __raw_{read,write}{b,w,l}(), *_relaxed(),
{read,write}{b,w,l}() and io{read,write}{16,32}be() are the same as the
asm-generic/io.h versions, so drop them and use the generic ones.

Two things stay ARC-specific:

- The barriers, now hooked into the generic accessors through
  __io_ar()/__io_bw(). On ARCv2 they remain dmb (rmb()/wmb()).
  On ARCompact they were empty, relying on the asm "memory" clobber
  as a compiler barrier, so make them barrier() explicitly.
  The generic defaults would be mb(), i.e. the heavyweight SYNC.
  The *_relaxed() accessors only need ordering against other MMIO
  accesses, which volatile guarantees.

- The {read,write}s{b,w,l}() string helpers, which, unlike the generic
  ones, cope with an unaligned memory buffer. They are defined before
  asm-generic/io.h is included, as it calls them (e.g. ioread8_rep()),
  so they do the volatile access directly rather than via __raw_*().

As a side effect, readX()/writeX() gain MMIO access tracing
(CONFIG_TRACE_MMIO_ACCESS).

vmlinux .text with GCC 16.2 (hsdk_defconfig): -5412 bytes.

Signed-off-by: Vineet Gupta <vgupta@kernel.org>
Assisted-by: Claude:claude-opus-5
---
 arch/arc/include/asm/io.h | 139 ++++++--------------------------------
 1 file changed, 19 insertions(+), 120 deletions(-)

diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index b4a3393d1bdc..de929136c1a3 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -11,15 +11,26 @@
 #include <asm/page.h>
 #include <linux/unaligned.h>
 
-#ifdef CONFIG_ISA_ARCV2
 #include <asm/barrier.h>
+
+#ifdef CONFIG_ISA_ARCV2
 #define __iormb()		rmb()
 #define __iowmb()		wmb()
 #else
-#define __iormb()		do { } while (0)
-#define __iowmb()		do { } while (0)
+/*
+ * The accessors are plain volatile accesses, which the compiler only orders
+ * against other volatile accesses. Make readX()/writeX() compiler barriers so
+ * that e.g. DMA buffer writes are not moved past the MMIO write that starts
+ * the DMA. The asm-generic default, mb(), would be the heavyweight SYNC.
+ */
+#define __iormb()		barrier()
+#define __iowmb()		barrier()
 #endif
 
+/* Barriers for the asm-generic readX()/writeX() */
+#define __io_ar(v)		__iormb()
+#define __io_bw()		__iowmb()
+
 extern void __iomem *ioremap(phys_addr_t paddr, unsigned long size);
 #define ioremap ioremap
 #define ioremap_prot ioremap_prot
@@ -33,57 +44,6 @@ static inline void ioport_unmap(void __iomem *addr)
 {
 }
 
-/*
- * io{read,write}{16,32}be() macros
- */
-#define ioread16be(p)		({ u16 __v = be16_to_cpu((__force __be16)__raw_readw(p)); __iormb(); __v; })
-#define ioread32be(p)		({ u32 __v = be32_to_cpu((__force __be32)__raw_readl(p)); __iormb(); __v; })
-
-#define iowrite16be(v,p)	({ __iowmb(); __raw_writew((__force u16)cpu_to_be16(v), p); })
-#define iowrite32be(v,p)	({ __iowmb(); __raw_writel((__force u32)cpu_to_be32(v), p); })
-
-#define __raw_readb __raw_readb
-static inline u8 __raw_readb(const volatile void __iomem *addr)
-{
-	u8 b;
-
-	__asm__ __volatile__(
-	"	ldb%U1 %0, %1	\n"
-	: "=r" (b)
-	: "m" (*(volatile u8 __force *)addr)
-	: "memory");
-
-	return b;
-}
-
-#define __raw_readw __raw_readw
-static inline u16 __raw_readw(const volatile void __iomem *addr)
-{
-	u16 s;
-
-	__asm__ __volatile__(
-	"	ldw%U1 %0, %1	\n"
-	: "=r" (s)
-	: "m" (*(volatile u16 __force *)addr)
-	: "memory");
-
-	return s;
-}
-
-#define __raw_readl __raw_readl
-static inline u32 __raw_readl(const volatile void __iomem *addr)
-{
-	u32 w;
-
-	__asm__ __volatile__(
-	"	ld%U1 %0, %1	\n"
-	: "=r" (w)
-	: "m" (*(volatile u32 __force *)addr)
-	: "memory");
-
-	return w;
-}
-
 /*
  * {read,write}s{b,w,l}() repeatedly access the same IO address in
  * native endianness in 8-, 16-, 32-bit chunks {into,from} memory,
@@ -102,12 +62,12 @@ static inline void __raw_reads##f(const volatile void __iomem *addr,	\
 	/* Some ARC CPU's don't support unaligned accesses */		\
 	if (is_aligned) {						\
 		do {							\
-			u##t x = __raw_read##f(addr);			\
+			u##t x = *(const volatile u##t __force *)addr;	\
 			*buf++ = x;					\
 		} while (--count);					\
 	} else {							\
 		do {							\
-			u##t x = __raw_read##f(addr);			\
+			u##t x = *(const volatile u##t __force *)addr;	\
 			put_unaligned(x, buf++);			\
 		} while (--count);					\
 	}								\
@@ -120,43 +80,6 @@ __raw_readsx(16, w)
 #define __raw_readsl __raw_readsl
 __raw_readsx(32, l)
 
-#define __raw_writeb __raw_writeb
-static inline void __raw_writeb(u8 b, volatile void __iomem *addr)
-{
-	__asm__ __volatile__(
-	"	stb%U1 %0, %1	\n"
-	:
-	: "r" (b), "m" (*(volatile u8 __force *)addr)
-	: "memory");
-}
-
-#define __raw_writew __raw_writew
-static inline void __raw_writew(u16 s, volatile void __iomem *addr)
-{
-	/*
-	 * Not an "m" operand: GCC can pick a register-indexed address for it,
-	 * which ST doesn't support (GCC PR target/127718).
-	 */
-	__asm__ __volatile__(
-	"	stw %0, [%1]	\n"
-	:
-	: "r" (s), "r" (addr)
-	: "memory");
-
-}
-
-#define __raw_writel __raw_writel
-static inline void __raw_writel(u32 w, volatile void __iomem *addr)
-{
-	/* See __raw_writew() above for why this doesn't use an "m" operand. */
-	__asm__ __volatile__(
-	"	st %0, [%1]	\n"
-	:
-	: "r" (w), "r" (addr)
-	: "memory");
-
-}
-
 #define __raw_writesx(t,f)						\
 static inline void __raw_writes##f(volatile void __iomem *addr, 	\
 				   const void *ptr, unsigned int count)	\
@@ -170,11 +93,12 @@ static inline void __raw_writes##f(volatile void __iomem *addr, 	\
 	/* Some ARC CPU's don't support unaligned accesses */		\
 	if (is_aligned) {						\
 		do {							\
-			__raw_write##f(*buf++, addr);			\
+			*(volatile u##t __force *)addr = *buf++;	\
 		} while (--count);					\
 	} else {							\
 		do {							\
-			__raw_write##f(get_unaligned(buf++), addr);	\
+			u##t x = get_unaligned(buf++);			\
+			*(volatile u##t __force *)addr = x;		\
 		} while (--count);					\
 	}								\
 }
@@ -198,39 +122,14 @@ __raw_writesx(32, l)
  *
  * http://lkml.kernel.org/r/20150622133656.GG1583@arm.com
  */
-#define readb(c)		({ u8  __v = readb_relaxed(c); __iormb(); __v; })
-#define readw(c)		({ u16 __v = readw_relaxed(c); __iormb(); __v; })
-#define readl(c)		({ u32 __v = readl_relaxed(c); __iormb(); __v; })
 #define readsb(p,d,l)		({ __raw_readsb(p,d,l); __iormb(); })
 #define readsw(p,d,l)		({ __raw_readsw(p,d,l); __iormb(); })
 #define readsl(p,d,l)		({ __raw_readsl(p,d,l); __iormb(); })
 
-#define writeb(v,c)		({ __iowmb(); writeb_relaxed(v,c); })
-#define writew(v,c)		({ __iowmb(); writew_relaxed(v,c); })
-#define writel(v,c)		({ __iowmb(); writel_relaxed(v,c); })
 #define writesb(p,d,l)		({ __iowmb(); __raw_writesb(p,d,l); })
 #define writesw(p,d,l)		({ __iowmb(); __raw_writesw(p,d,l); })
 #define writesl(p,d,l)		({ __iowmb(); __raw_writesl(p,d,l); })
 
-/*
- * Relaxed API for drivers which can handle barrier ordering themselves
- *
- * Also these are defined to perform little endian accesses.
- * To provide the typical device register semantics of fixed endian,
- * swap the byte order for Big Endian
- *
- * http://lkml.kernel.org/r/201603100845.30602.arnd@arndb.de
- */
-#define readb_relaxed(c)	__raw_readb(c)
-#define readw_relaxed(c) ({ u16 __r = le16_to_cpu((__force __le16) \
-					__raw_readw(c)); __r; })
-#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
-					__raw_readl(c)); __r; })
-
-#define writeb_relaxed(v,c)	__raw_writeb(v,c)
-#define writew_relaxed(v,c)	__raw_writew((__force u16) cpu_to_le16(v),c)
-#define writel_relaxed(v,c)	__raw_writel((__force u32) cpu_to_le32(v),c)
-
 #include <asm-generic/io.h>
 
 #endif /* _ASM_ARC_IO_H */
-- 
2.53.0

Re: [PATCH 2/2] ARC: io: use asm-generic MMIO accessors

[Arnd Bergmann]

On Wed, Nov 4, 2026, at 05:35, Vineet Gupta wrote:
> ARC MMIO accessors use inline asm, which isn't really needed. It was
> introduced with the original port back in 2013.
>
> As the previous patch showed, inline asm can be error-prone. Letting the
> compiler emit the accesses also allows 16-bit encodings, scaled-index
> loads and folding constant offsets (s9) into the access.
>
> Without the asm, ARC's __raw_{read,write}{b,w,l}(), *_relaxed(),
> {read,write}{b,w,l}() and io{read,write}{16,32}be() are the same as the
> asm-generic/io.h versions, so drop them and use the generic ones.

Hi Vineet,

In my experience, the generic helpers are also fragile, they just lead
to different problems when the compiler optimizes them in ways it
should not.

I would recommend keeping the __raw helpers with the workaround from
the first patch, possibly both variants depending on the compiler
version.

Using the generic higher level interfaces is a very welcome change
though, I hope we can eventually get to the point where every
architecture just needs to provide the __raw_*() ones.

> Two things stay ARC-specific:
>
> - The barriers, now hooked into the generic accessors through
>   __io_ar()/__io_bw(). On ARCv2 they remain dmb (rmb()/wmb()).
>   On ARCompact they were empty, relying on the asm "memory" clobber
>   as a compiler barrier, so make them barrier() explicitly.
>   The generic defaults would be mb(), i.e. the heavyweight SYNC.
>   The *_relaxed() accessors only need ordering against other MMIO
>   accesses, which volatile guarantees.

Sounds good.

> - The {read,write}s{b,w,l}() string helpers, which, unlike the generic
>   ones, cope with an unaligned memory buffer. They are defined before
>   asm-generic/io.h is included, as it calls them (e.g. ioread8_rep()),
>   so they do the volatile access directly rather than via __raw_*().

I don't think we want a custom version here. Do you know the reason this
tries to handle unaligned buffers? If there are drivers that require it,
we can change the generic implementation to be more like the arc version,
otherwise this likely just causes larger object code for all users that
don't ever need unaligned access.

I see that the sparc version does the same thing, other ones without
unaligned access (in particular armv5 and riscv) assume that the
memory buffer is aligned. On alpha and parisc, there is an alignment
check for insl()/outsl() but not readsl()/writesl(). I suspect this
is not intentional.

If they do run into this problem, they will fix it up with a slow
fault handler on the unaligned memory access.

      Arnd