Various architectures have almost the same implementations for
memcpy_{to,from}io and memset_io functions. So, consolidate them
into a common lib/iomem_copy.c.

Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com>
Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
---
Changes for v9:
- Moved all functions to iomem_copy.c
- Build the new iomem_copy.c unconditionally
- Guard prototypes in asm-generic/io.h with ifdefs
---
 include/asm-generic/io.h |  62 +++---------------
 lib/Makefile             |   2 +-
 lib/iomem_copy.c         | 134 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 53 deletions(-)
 create mode 100644 lib/iomem_copy.c

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 80de699bf6af..4ab47ef7095f 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -102,6 +102,16 @@ static inline void log_post_read_mmio(u64 val, u8 width, const volatile void __i
 
 #endif /* CONFIG_TRACE_MMIO_ACCESS */
 
+#ifndef memcpy_fromio
+void memcpy_fromio(void *to, const volatile void __iomem *from, size_t count);
+#endif
+#ifndef memcpy_toio
+void memcpy_toio(volatile void __iomem *to, const void *from, size_t count);
+#endif
+#ifndef memset_io
+void memset_io(volatile void __iomem *dst, int c, size_t count);
+#endif
+
 /*
  * __raw_{read,write}{b,w,l,q}() access memory in native endianness.
  *
@@ -1150,58 +1160,6 @@ static inline void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
 }
 #endif
 
-#ifndef memset_io
-#define memset_io memset_io
-/**
- * memset_io	Set a range of I/O memory to a constant value
- * @addr:	The beginning of the I/O-memory range to set
- * @val:	The value to set the memory to
- * @count:	The number of bytes to set
- *
- * Set a range of I/O memory to a given value.
- */
-static inline void memset_io(volatile void __iomem *addr, int value,
-			     size_t size)
-{
-	memset(__io_virt(addr), value, size);
-}
-#endif
-
-#ifndef memcpy_fromio
-#define memcpy_fromio memcpy_fromio
-/**
- * memcpy_fromio	Copy a block of data from I/O memory
- * @dst:		The (RAM) destination for the copy
- * @src:		The (I/O memory) source for the data
- * @count:		The number of bytes to copy
- *
- * Copy a block of data from I/O memory.
- */
-static inline void memcpy_fromio(void *buffer,
-				 const volatile void __iomem *addr,
-				 size_t size)
-{
-	memcpy(buffer, __io_virt(addr), size);
-}
-#endif
-
-#ifndef memcpy_toio
-#define memcpy_toio memcpy_toio
-/**
- * memcpy_toio		Copy a block of data into I/O memory
- * @dst:		The (I/O memory) destination for the copy
- * @src:		The (RAM) source for the data
- * @count:		The number of bytes to copy
- *
- * Copy a block of data to I/O memory.
- */
-static inline void memcpy_toio(volatile void __iomem *addr, const void *buffer,
-			       size_t size)
-{
-	memcpy(__io_virt(addr), buffer, size);
-}
-#endif
-
 extern int devmem_is_allowed(unsigned long pfn);
 
 #endif /* __KERNEL__ */
diff --git a/lib/Makefile b/lib/Makefile
index 773adf88af41..db4717538fad 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -35,7 +35,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
 	 earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
 	 nmi_backtrace.o win_minmax.o memcat_p.o \
-	 buildid.o objpool.o union_find.o
+	 buildid.o objpool.o union_find.o iomem_copy.o
 
 lib-$(CONFIG_PRINTK) += dump_stack.o
 lib-$(CONFIG_SMP) += cpumask.o
diff --git a/lib/iomem_copy.c b/lib/iomem_copy.c
new file mode 100644
index 000000000000..4de710c2a9f0
--- /dev/null
+++ b/lib/iomem_copy.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Kalray, Inc.  All Rights Reserved.
+ */
+
+#include <linux/align.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#ifndef memcpy_fromio
+/**
+ * memcpy_fromio	Copy a block of data from I/O memory
+ * @to:			The (RAM) destination for the copy
+ * @from:		The (I/O memory) source for the data
+ * @count:		The number of bytes to copy
+ *
+ * Copy a block of data from I/O memory.
+ */
+void memcpy_fromio(void *to, const volatile void __iomem *from, size_t count)
+{
+	while (count && !IS_ALIGNED((long)from, sizeof(long))) {
+		*(u8 *)to = __raw_readb(from);
+		from++;
+		to++;
+		count--;
+	}
+
+	while (count >= sizeof(long)) {
+#ifdef CONFIG_64BIT
+		long val = __raw_readq(from);
+#else
+		long val = __raw_readl(from);
+#endif
+		put_unaligned(val, (long *)to);
+
+
+		from += sizeof(long);
+		to += sizeof(long);
+		count -= sizeof(long);
+	}
+
+	while (count) {
+		*(u8 *)to = __raw_readb(from);
+		from++;
+		to++;
+		count--;
+	}
+}
+EXPORT_SYMBOL(memcpy_fromio);
+#endif
+
+#ifndef memcpy_toio
+/**
+ * memcpy_toio		Copy a block of data into I/O memory
+ * @to:			The (I/O memory) destination for the copy
+ * @from:		The (RAM) source for the data
+ * @count:		The number of bytes to copy
+ *
+ * Copy a block of data to I/O memory.
+ */
+void memcpy_toio(volatile void __iomem *to, const void *from, size_t count)
+{
+	while (count && !IS_ALIGNED((long)to, sizeof(long))) {
+		__raw_writeb(*(u8 *)from, to);
+		from++;
+		to++;
+		count--;
+	}
+
+	while (count >= sizeof(long)) {
+		long val = get_unaligned((long *)from);
+#ifdef CONFIG_64BIT
+		__raw_writeq(val, to);
+#else
+		__raw_writel(val, to);
+#endif
+
+		from += sizeof(long);
+		to += sizeof(long);
+		count -= sizeof(long);
+	}
+
+	while (count) {
+		__raw_writeb(*(u8 *)from, to);
+		from++;
+		to++;
+		count--;
+	}
+}
+EXPORT_SYMBOL(memcpy_toio);
+#endif
+
+#ifndef memset_io
+/**
+ * memset_io		Set a range of I/O memory to a constant value
+ * @dst:		The beginning of the I/O-memory range to set
+ * @c:			The value to set the memory to
+ * @count:		The number of bytes to set
+ *
+ * Set a range of I/O memory to a given value.
+ */
+void memset_io(volatile void __iomem *dst, int c, size_t count)
+{
+	long qc = (u8)c;
+
+	qc *= ~0UL / 0xff;
+
+	while (count && !IS_ALIGNED((long)dst, sizeof(long))) {
+		__raw_writeb(c, dst);
+		dst++;
+		count--;
+	}
+
+	while (count >= sizeof(long)) {
+#ifdef CONFIG_64BIT
+		__raw_writeq(qc, dst);
+#else
+		__raw_writel(qc, dst);
+#endif
+
+		dst += sizeof(long);
+		count -= sizeof(long);
+	}
+
+	while (count) {
+		__raw_writeb(c, dst);
+		dst++;
+		count--;
+	}
+}
+EXPORT_SYMBOL(memset_io);
+#endif
-- 
2.34.1

On Thu, Oct 10, 2024, at 12:36, Julian Vetter wrote:
> Various architectures have almost the same implementations for
> memcpy_{to,from}io and memset_io functions. So, consolidate them
> into a common lib/iomem_copy.c.
>
> Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com>
> Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
> ---
> Changes for v9:
> - Moved all functions to iomem_copy.c
> - Build the new iomem_copy.c unconditionally
> - Guard prototypes in asm-generic/io.h with ifdefs

I'm happy with the patch contents, but it looks like you forgot
to update the description as this is not what this version of the
patch does: Instead of consolidating the identical versions (which
you do in patches 2-4), this changes the ones that use the
memcpy/memset fallback: arc, microblaze, mips, nios2, openrisc,
riscv, and xtensa.

> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -102,6 +102,16 @@ static inline void log_post_read_mmio(u64 val, u8 
> width, const volatile void __i
> 
>  #endif /* CONFIG_TRACE_MMIO_ACCESS */
> 
> +#ifndef memcpy_fromio
> +void memcpy_fromio(void *to, const volatile void __iomem *from, size_t 
> count);
> +#endif
> +#ifndef memcpy_toio
> +void memcpy_toio(volatile void __iomem *to, const void *from, size_t 
> count);
> +#endif
> +#ifndef memset_io
> +void memset_io(volatile void __iomem *dst, int c, size_t count);
> +#endif
> +
>  /*
>   * __raw_{read,write}{b,w,l,q}() access memory in native endianness.
>   *
> @@ -1150,58 +1160,6 @@ static inline void 
> unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
>  }
>  #endif
> 
> -#ifndef memset_io
> -#define memset_io memset_io
> -/**
> - * memset_io	Set a range of I/O memory to a constant value
> - * @addr:	The beginning of the I/O-memory range to set
> - * @val:	The value to set the memory to
> - * @count:	The number of bytes to set
> - *
> - * Set a range of I/O memory to a given value.
> - */
> -static inline void memset_io(volatile void __iomem *addr, int value,
> -			     size_t size)
> -{
> -	memset(__io_virt(addr), value, size);
> -}
> -#endif

Unless there is a technical reason to move the location of
these I think it would be clearer to change it in place
and keep the three #ifdef but replace the contents.

You can also choose to add the definition in one patch
and then change the header in the next patch, which would
let you have more descriptive changelog texts for the
new common implementation and the second patch that changes
the fallback.

     Arnd

On Thu, Oct 10, 2024 at 02:36:24PM +0200, Julian Vetter wrote:
> Various architectures have almost the same implementations for
> memcpy_{to,from}io and memset_io functions. So, consolidate them
> into a common lib/iomem_copy.c.

That might be the final intent of this seris, but what this patch does is
to replace the trivial memset/memcpy inline implementations with a more 
complex one that actually use the accessors.

So this actually changes behavior for those architectures that did not
provide them before quite a lot.  You'll need to explain in more
detail why that is safe and desireable.