[PATCH 0/6] asm-generic: io.h cleanups

[PATCH 0/6] asm-generic: io.h cleanups

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

After the previous round of cleanups for asm-generic/io,h on the
ioread64 helpers, I had another look at the architecture specific
versions, especially those that caused build failures in the past.

These are some simplifications that I would like to merge at the same
time, please have a look. Hopefully these are all uncontroversial.

I have a few more patches for m68k that need a more thorough
review and testing, will post them after the merge window.

Arnd Bergmann (6):
  alpha: stop using asm-generic/iomap.h
  sh: remove duplicate ioread/iowrite helpers
  parisc: stop using asm-generic/iomap.h
  powerpc: asm/io.h: remove split ioread64/iowrite64 helpers
  mips: drop GENERIC_IOMAP wrapper
  m68k/nommu: stop using GENERIC_IOMAP

 arch/alpha/include/asm/io.h   |  31 ++++---
 arch/m68k/Kconfig             |   2 +-
 arch/m68k/include/asm/io_no.h |   4 -
 arch/mips/Kconfig             |   2 +-
 arch/mips/include/asm/io.h    |  21 ++---
 arch/mips/lib/iomap-pci.c     |   9 ++
 arch/parisc/include/asm/io.h  |  36 ++++++--
 arch/powerpc/include/asm/io.h |  48 ----------
 arch/sh/include/asm/io.h      |  30 ++-----
 arch/sh/kernel/Makefile       |   3 -
 arch/sh/kernel/iomap.c        | 162 ----------------------------------
 arch/sh/kernel/ioport.c       |   5 --
 arch/sh/lib/io.c              |   4 +-
 drivers/sh/clk/cpg.c          |  25 +++---
 14 files changed, 84 insertions(+), 298 deletions(-)
 delete mode 100644 arch/sh/kernel/iomap.c

-- 
2.39.5

Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Greg Ungerer <gerg@linux-m68k.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Julian Vetter <julian@outer-limits.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-sh@vger.kernel.org

[PATCH 1/6] alpha: stop using asm-generic/iomap.h

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Alpha has custom definitions for ioread64()/iowrite64(), which now
don't exist in the lib/iomap.c variant. This is an endless source
of build failures, since alpha tries to share a couple of function
declarations.

Change alpha to have its own prototypes that match the definitions
in arch/alpha/kerne/io.c instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/alpha/include/asm/io.h | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index 65fe1e54c6da..fa3e4c246cda 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -10,10 +10,6 @@
 #include <asm/machvec.h>
 #include <asm/hwrpb.h>
 
-/* The generic header contains only prototypes.  Including it ensures that
-   the implementation we have here matches that interface.  */
-#include <asm-generic/iomap.h>
-
 /*
  * Virtual -> physical identity mapping starts at this offset
  */
@@ -276,13 +272,24 @@ extern void		__raw_writeq(u64 b, volatile void __iomem *addr);
 #define __raw_writel __raw_writel
 #define __raw_writeq __raw_writeq
 
-/*
- * Mapping from port numbers to __iomem space is pretty easy.
- */
+extern unsigned int ioread8(const void __iomem *);
+extern unsigned int ioread16(const void __iomem *);
+extern unsigned int ioread32(const void __iomem *);
+extern u64 ioread64(const void __iomem *);
+
+extern void iowrite8(u8, void __iomem *);
+extern void iowrite16(u16, void __iomem *);
+extern void iowrite32(u32, void __iomem *);
+extern void iowrite64(u64, void __iomem *);
+
+extern void ioread8_rep(const void __iomem *port, void *buf, unsigned long count);
+extern void ioread16_rep(const void __iomem *port, void *buf, unsigned long count);
+extern void ioread32_rep(const void __iomem *port, void *buf, unsigned long count);
+
+extern void iowrite8_rep(void __iomem *port, const void *buf, unsigned long count);
+extern void iowrite16_rep(void __iomem *port, const void *buf, unsigned long count);
+extern void iowrite32_rep(void __iomem *port, const void *buf, unsigned long count);
 
-/* These two have to be extern inline because of the extern prototype from
-   <asm-generic/iomap.h>.  It is not legal to mix "extern" and "static" for
-   the same declaration.  */
 extern inline void __iomem *ioport_map(unsigned long port, unsigned int size)
 {
 	return IO_CONCAT(__IO_PREFIX,ioportmap) (port);
@@ -629,10 +636,6 @@ extern void outsl (unsigned long port, const void *src, unsigned long count);
 #define RTC_PORT(x)	(0x70 + (x))
 #define RTC_ALWAYS_BCD	0
 
-/*
- * These get provided from <asm-generic/iomap.h> since alpha does not
- * select GENERIC_IOMAP.
- */
 #define ioread64 ioread64
 #define iowrite64 iowrite64
 #define ioread8_rep ioread8_rep
-- 
2.39.5

[PATCH 2/6] sh: remove duplicate ioread/iowrite helpers

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The ioread/iowrite functions on sh only do memory mapped I/O like the
generic verion, and never map onto non-MMIO inb/outb variants, so they
just add complexity. In particular, the use of asm-generic/iomap.h
ties the declaration to the x86 implementation.

Remove the custom versions and use the architecture-independent fallback
code instead. Some of the calling conventions on sh are different here,
so fix that by adding 'volatile' keywords where required by the generic
implementation and change the cpg clock driver to no longer depend on
the interesting choice of return types for ioread8/ioread16/ioread32.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/sh/include/asm/io.h |  30 ++------
 arch/sh/kernel/Makefile  |   3 -
 arch/sh/kernel/iomap.c   | 162 ---------------------------------------
 arch/sh/kernel/ioport.c  |   5 --
 arch/sh/lib/io.c         |   4 +-
 drivers/sh/clk/cpg.c     |  25 +++---
 6 files changed, 21 insertions(+), 208 deletions(-)
 delete mode 100644 arch/sh/kernel/iomap.c

diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
index cf5eab840d57..0f663ebec700 100644
--- a/arch/sh/include/asm/io.h
+++ b/arch/sh/include/asm/io.h
@@ -19,7 +19,6 @@
 #include <asm/machvec.h>
 #include <asm/page.h>
 #include <linux/pgtable.h>
-#include <asm-generic/iomap.h>
 
 #define __IO_PREFIX     generic
 #include <asm/io_generic.h>
@@ -100,7 +99,7 @@ pfx##writes##bwlq(volatile void __iomem *mem, const void *addr,		\
 	}								\
 }									\
 									\
-static inline void pfx##reads##bwlq(volatile void __iomem *mem,		\
+static inline void pfx##reads##bwlq(const volatile void __iomem *mem,	\
 				    void *addr, unsigned int count)	\
 {									\
 	volatile type *__addr = addr;					\
@@ -114,37 +113,18 @@ static inline void pfx##reads##bwlq(volatile void __iomem *mem,		\
 __BUILD_MEMORY_STRING(__raw_, b, u8)
 __BUILD_MEMORY_STRING(__raw_, w, u16)
 
-void __raw_writesl(void __iomem *addr, const void *data, int longlen);
-void __raw_readsl(const void __iomem *addr, void *data, int longlen);
+void __raw_writesl(void volatile __iomem *addr, const void *data, int longlen);
+void __raw_readsl(const volatile void __iomem *addr, void *data, int longlen);
 
 __BUILD_MEMORY_STRING(__raw_, q, u64)
 
 #define ioport_map ioport_map
-#define ioport_unmap ioport_unmap
 #define pci_iounmap pci_iounmap
 
-#define ioread8 ioread8
-#define ioread16 ioread16
-#define ioread16be ioread16be
-#define ioread32 ioread32
-#define ioread32be ioread32be
-
-#define iowrite8 iowrite8
-#define iowrite16 iowrite16
-#define iowrite16be iowrite16be
-#define iowrite32 iowrite32
-#define iowrite32be iowrite32be
-
-#define ioread8_rep ioread8_rep
-#define ioread16_rep ioread16_rep
-#define ioread32_rep ioread32_rep
-
-#define iowrite8_rep iowrite8_rep
-#define iowrite16_rep iowrite16_rep
-#define iowrite32_rep iowrite32_rep
-
 #ifdef CONFIG_HAS_IOPORT_MAP
 
+extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
+
 /*
  * Slowdown I/O port space accesses for antique hardware.
  */
diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index ba917008d63e..7b453592adaf 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -21,10 +21,7 @@ obj-y	:= head_32.o debugtraps.o dumpstack.o				\
 	   syscalls_32.o time.o topology.o traps.o			\
 	   traps_32.o unwinder.o
 
-ifndef CONFIG_GENERIC_IOMAP
-obj-y				+= iomap.o
 obj-$(CONFIG_HAS_IOPORT_MAP)	+= ioport.o
-endif
 
 obj-y				+= sys_sh32.o
 obj-y				+= cpu/
diff --git a/arch/sh/kernel/iomap.c b/arch/sh/kernel/iomap.c
deleted file mode 100644
index 0a0dff4e66de..000000000000
--- a/arch/sh/kernel/iomap.c
+++ /dev/null
@@ -1,162 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * arch/sh/kernel/iomap.c
- *
- * Copyright (C) 2000  Niibe Yutaka
- * Copyright (C) 2005 - 2007 Paul Mundt
- */
-#include <linux/module.h>
-#include <linux/io.h>
-
-unsigned int ioread8(const void __iomem *addr)
-{
-	return readb(addr);
-}
-EXPORT_SYMBOL(ioread8);
-
-unsigned int ioread16(const void __iomem *addr)
-{
-	return readw(addr);
-}
-EXPORT_SYMBOL(ioread16);
-
-unsigned int ioread16be(const void __iomem *addr)
-{
-	return be16_to_cpu(__raw_readw(addr));
-}
-EXPORT_SYMBOL(ioread16be);
-
-unsigned int ioread32(const void __iomem *addr)
-{
-	return readl(addr);
-}
-EXPORT_SYMBOL(ioread32);
-
-unsigned int ioread32be(const void __iomem *addr)
-{
-	return be32_to_cpu(__raw_readl(addr));
-}
-EXPORT_SYMBOL(ioread32be);
-
-void iowrite8(u8 val, void __iomem *addr)
-{
-	writeb(val, addr);
-}
-EXPORT_SYMBOL(iowrite8);
-
-void iowrite16(u16 val, void __iomem *addr)
-{
-	writew(val, addr);
-}
-EXPORT_SYMBOL(iowrite16);
-
-void iowrite16be(u16 val, void __iomem *addr)
-{
-	__raw_writew(cpu_to_be16(val), addr);
-}
-EXPORT_SYMBOL(iowrite16be);
-
-void iowrite32(u32 val, void __iomem *addr)
-{
-	writel(val, addr);
-}
-EXPORT_SYMBOL(iowrite32);
-
-void iowrite32be(u32 val, void __iomem *addr)
-{
-	__raw_writel(cpu_to_be32(val), addr);
-}
-EXPORT_SYMBOL(iowrite32be);
-
-/*
- * These are the "repeat MMIO read/write" functions.
- * Note the "__raw" accesses, since we don't want to
- * convert to CPU byte order. We write in "IO byte
- * order" (we also don't have IO barriers).
- */
-static inline void mmio_insb(const void __iomem *addr, u8 *dst, int count)
-{
-	while (--count >= 0) {
-		u8 data = __raw_readb(addr);
-		*dst = data;
-		dst++;
-	}
-}
-
-static inline void mmio_insw(const void __iomem *addr, u16 *dst, int count)
-{
-	while (--count >= 0) {
-		u16 data = __raw_readw(addr);
-		*dst = data;
-		dst++;
-	}
-}
-
-static inline void mmio_insl(const void __iomem *addr, u32 *dst, int count)
-{
-	while (--count >= 0) {
-		u32 data = __raw_readl(addr);
-		*dst = data;
-		dst++;
-	}
-}
-
-static inline void mmio_outsb(void __iomem *addr, const u8 *src, int count)
-{
-	while (--count >= 0) {
-		__raw_writeb(*src, addr);
-		src++;
-	}
-}
-
-static inline void mmio_outsw(void __iomem *addr, const u16 *src, int count)
-{
-	while (--count >= 0) {
-		__raw_writew(*src, addr);
-		src++;
-	}
-}
-
-static inline void mmio_outsl(void __iomem *addr, const u32 *src, int count)
-{
-	while (--count >= 0) {
-		__raw_writel(*src, addr);
-		src++;
-	}
-}
-
-void ioread8_rep(const void __iomem *addr, void *dst, unsigned long count)
-{
-	mmio_insb(addr, dst, count);
-}
-EXPORT_SYMBOL(ioread8_rep);
-
-void ioread16_rep(const void __iomem *addr, void *dst, unsigned long count)
-{
-	mmio_insw(addr, dst, count);
-}
-EXPORT_SYMBOL(ioread16_rep);
-
-void ioread32_rep(const void __iomem *addr, void *dst, unsigned long count)
-{
-	mmio_insl(addr, dst, count);
-}
-EXPORT_SYMBOL(ioread32_rep);
-
-void iowrite8_rep(void __iomem *addr, const void *src, unsigned long count)
-{
-	mmio_outsb(addr, src, count);
-}
-EXPORT_SYMBOL(iowrite8_rep);
-
-void iowrite16_rep(void __iomem *addr, const void *src, unsigned long count)
-{
-	mmio_outsw(addr, src, count);
-}
-EXPORT_SYMBOL(iowrite16_rep);
-
-void iowrite32_rep(void __iomem *addr, const void *src, unsigned long count)
-{
-	mmio_outsl(addr, src, count);
-}
-EXPORT_SYMBOL(iowrite32_rep);
diff --git a/arch/sh/kernel/ioport.c b/arch/sh/kernel/ioport.c
index c8aff8a20164..915a3dfd9f02 100644
--- a/arch/sh/kernel/ioport.c
+++ b/arch/sh/kernel/ioport.c
@@ -23,8 +23,3 @@ void __iomem *ioport_map(unsigned long port, unsigned int nr)
 	return (void __iomem *)(port + sh_io_port_base);
 }
 EXPORT_SYMBOL(ioport_map);
-
-void ioport_unmap(void __iomem *addr)
-{
-}
-EXPORT_SYMBOL(ioport_unmap);
diff --git a/arch/sh/lib/io.c b/arch/sh/lib/io.c
index ebcf7c0a7335..dc6345e4c53b 100644
--- a/arch/sh/lib/io.c
+++ b/arch/sh/lib/io.c
@@ -11,7 +11,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 
-void __raw_readsl(const void __iomem *addr, void *datap, int len)
+void __raw_readsl(const volatile void __iomem *addr, void *datap, int len)
 {
 	u32 *data;
 
@@ -60,7 +60,7 @@ void __raw_readsl(const void __iomem *addr, void *datap, int len)
 }
 EXPORT_SYMBOL(__raw_readsl);
 
-void __raw_writesl(void __iomem *addr, const void *data, int len)
+void __raw_writesl(volatile void __iomem *addr, const void *data, int len)
 {
 	if (likely(len != 0)) {
 		int tmp1;
diff --git a/drivers/sh/clk/cpg.c b/drivers/sh/clk/cpg.c
index fd72d9088bdc..64ed7d64458a 100644
--- a/drivers/sh/clk/cpg.c
+++ b/drivers/sh/clk/cpg.c
@@ -26,6 +26,19 @@ static unsigned int sh_clk_read(struct clk *clk)
 	return ioread32(clk->mapped_reg);
 }
 
+static unsigned int sh_clk_read_status(struct clk *clk)
+{
+	void __iomem *mapped_status = (phys_addr_t)clk->status_reg -
+		(phys_addr_t)clk->enable_reg + clk->mapped_reg;
+
+	if (clk->flags & CLK_ENABLE_REG_8BIT)
+		return ioread8(mapped_status);
+	else if (clk->flags & CLK_ENABLE_REG_16BIT)
+		return ioread16(mapped_status);
+
+	return ioread32(mapped_status);
+}
+
 static void sh_clk_write(int value, struct clk *clk)
 {
 	if (clk->flags & CLK_ENABLE_REG_8BIT)
@@ -40,20 +53,10 @@ static int sh_clk_mstp_enable(struct clk *clk)
 {
 	sh_clk_write(sh_clk_read(clk) & ~(1 << clk->enable_bit), clk);
 	if (clk->status_reg) {
-		unsigned int (*read)(const void __iomem *addr);
 		int i;
-		void __iomem *mapped_status = (phys_addr_t)clk->status_reg -
-			(phys_addr_t)clk->enable_reg + clk->mapped_reg;
-
-		if (clk->flags & CLK_ENABLE_REG_8BIT)
-			read = ioread8;
-		else if (clk->flags & CLK_ENABLE_REG_16BIT)
-			read = ioread16;
-		else
-			read = ioread32;
 
 		for (i = 1000;
-		     (read(mapped_status) & (1 << clk->enable_bit)) && i;
+		     (sh_clk_read_status(clk) & (1 << clk->enable_bit)) && i;
 		     i--)
 			cpu_relax();
 		if (!i) {
-- 
2.39.5

Re: [PATCH 2/6] sh: remove duplicate ioread/iowrite helpers

[John Paul Adrian Glaubitz]

Hi Arnd,

On Sat, 2025-03-15 at 11:59 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The ioread/iowrite functions on sh only do memory mapped I/O like the
> generic verion, and never map onto non-MMIO inb/outb variants, so they
> just add complexity. In particular, the use of asm-generic/iomap.h
> ties the declaration to the x86 implementation.
> 
> Remove the custom versions and use the architecture-independent fallback
> code instead. Some of the calling conventions on sh are different here,
> so fix that by adding 'volatile' keywords where required by the generic
> implementation and change the cpg clock driver to no longer depend on
> the interesting choice of return types for ioread8/ioread16/ioread32.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/sh/include/asm/io.h |  30 ++------
>  arch/sh/kernel/Makefile  |   3 -
>  arch/sh/kernel/iomap.c   | 162 ---------------------------------------
>  arch/sh/kernel/ioport.c  |   5 --
>  arch/sh/lib/io.c         |   4 +-
>  drivers/sh/clk/cpg.c     |  25 +++---
>  6 files changed, 21 insertions(+), 208 deletions(-)
>  delete mode 100644 arch/sh/kernel/iomap.c
> 
> diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
> index cf5eab840d57..0f663ebec700 100644
> --- a/arch/sh/include/asm/io.h
> +++ b/arch/sh/include/asm/io.h
> @@ -19,7 +19,6 @@
>  #include <asm/machvec.h>
>  #include <asm/page.h>
>  #include <linux/pgtable.h>
> -#include <asm-generic/iomap.h>
>  
>  #define __IO_PREFIX     generic
>  #include <asm/io_generic.h>
> @@ -100,7 +99,7 @@ pfx##writes##bwlq(volatile void __iomem *mem, const void *addr,		\
>  	}								\
>  }									\
>  									\
> -static inline void pfx##reads##bwlq(volatile void __iomem *mem,		\
> +static inline void pfx##reads##bwlq(const volatile void __iomem *mem,	\
>  				    void *addr, unsigned int count)	\
>  {									\
>  	volatile type *__addr = addr;					\
> @@ -114,37 +113,18 @@ static inline void pfx##reads##bwlq(volatile void __iomem *mem,		\
>  __BUILD_MEMORY_STRING(__raw_, b, u8)
>  __BUILD_MEMORY_STRING(__raw_, w, u16)
>  
> -void __raw_writesl(void __iomem *addr, const void *data, int longlen);
> -void __raw_readsl(const void __iomem *addr, void *data, int longlen);
> +void __raw_writesl(void volatile __iomem *addr, const void *data, int longlen);
> +void __raw_readsl(const volatile void __iomem *addr, void *data, int longlen);
>  
>  __BUILD_MEMORY_STRING(__raw_, q, u64)
>  
>  #define ioport_map ioport_map
> -#define ioport_unmap ioport_unmap
>  #define pci_iounmap pci_iounmap
>  
> -#define ioread8 ioread8
> -#define ioread16 ioread16
> -#define ioread16be ioread16be
> -#define ioread32 ioread32
> -#define ioread32be ioread32be
> -
> -#define iowrite8 iowrite8
> -#define iowrite16 iowrite16
> -#define iowrite16be iowrite16be
> -#define iowrite32 iowrite32
> -#define iowrite32be iowrite32be
> -
> -#define ioread8_rep ioread8_rep
> -#define ioread16_rep ioread16_rep
> -#define ioread32_rep ioread32_rep
> -
> -#define iowrite8_rep iowrite8_rep
> -#define iowrite16_rep iowrite16_rep
> -#define iowrite32_rep iowrite32_rep
> -
>  #ifdef CONFIG_HAS_IOPORT_MAP
>  
> +extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
> +
>  /*
>   * Slowdown I/O port space accesses for antique hardware.
>   */
> diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
> index ba917008d63e..7b453592adaf 100644
> --- a/arch/sh/kernel/Makefile
> +++ b/arch/sh/kernel/Makefile
> @@ -21,10 +21,7 @@ obj-y	:= head_32.o debugtraps.o dumpstack.o				\
>  	   syscalls_32.o time.o topology.o traps.o			\
>  	   traps_32.o unwinder.o
>  
> -ifndef CONFIG_GENERIC_IOMAP
> -obj-y				+= iomap.o
>  obj-$(CONFIG_HAS_IOPORT_MAP)	+= ioport.o
> -endif
>  
>  obj-y				+= sys_sh32.o
>  obj-y				+= cpu/
> diff --git a/arch/sh/kernel/iomap.c b/arch/sh/kernel/iomap.c
> deleted file mode 100644
> index 0a0dff4e66de..000000000000
> --- a/arch/sh/kernel/iomap.c
> +++ /dev/null
> @@ -1,162 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * arch/sh/kernel/iomap.c
> - *
> - * Copyright (C) 2000  Niibe Yutaka
> - * Copyright (C) 2005 - 2007 Paul Mundt
> - */
> -#include <linux/module.h>
> -#include <linux/io.h>
> -
> -unsigned int ioread8(const void __iomem *addr)
> -{
> -	return readb(addr);
> -}
> -EXPORT_SYMBOL(ioread8);
> -
> -unsigned int ioread16(const void __iomem *addr)
> -{
> -	return readw(addr);
> -}
> -EXPORT_SYMBOL(ioread16);
> -
> -unsigned int ioread16be(const void __iomem *addr)
> -{
> -	return be16_to_cpu(__raw_readw(addr));
> -}
> -EXPORT_SYMBOL(ioread16be);
> -
> -unsigned int ioread32(const void __iomem *addr)
> -{
> -	return readl(addr);
> -}
> -EXPORT_SYMBOL(ioread32);
> -
> -unsigned int ioread32be(const void __iomem *addr)
> -{
> -	return be32_to_cpu(__raw_readl(addr));
> -}
> -EXPORT_SYMBOL(ioread32be);
> -
> -void iowrite8(u8 val, void __iomem *addr)
> -{
> -	writeb(val, addr);
> -}
> -EXPORT_SYMBOL(iowrite8);
> -
> -void iowrite16(u16 val, void __iomem *addr)
> -{
> -	writew(val, addr);
> -}
> -EXPORT_SYMBOL(iowrite16);
> -
> -void iowrite16be(u16 val, void __iomem *addr)
> -{
> -	__raw_writew(cpu_to_be16(val), addr);
> -}
> -EXPORT_SYMBOL(iowrite16be);
> -
> -void iowrite32(u32 val, void __iomem *addr)
> -{
> -	writel(val, addr);
> -}
> -EXPORT_SYMBOL(iowrite32);
> -
> -void iowrite32be(u32 val, void __iomem *addr)
> -{
> -	__raw_writel(cpu_to_be32(val), addr);
> -}
> -EXPORT_SYMBOL(iowrite32be);
> -
> -/*
> - * These are the "repeat MMIO read/write" functions.
> - * Note the "__raw" accesses, since we don't want to
> - * convert to CPU byte order. We write in "IO byte
> - * order" (we also don't have IO barriers).
> - */
> -static inline void mmio_insb(const void __iomem *addr, u8 *dst, int count)
> -{
> -	while (--count >= 0) {
> -		u8 data = __raw_readb(addr);
> -		*dst = data;
> -		dst++;
> -	}
> -}
> -
> -static inline void mmio_insw(const void __iomem *addr, u16 *dst, int count)
> -{
> -	while (--count >= 0) {
> -		u16 data = __raw_readw(addr);
> -		*dst = data;
> -		dst++;
> -	}
> -}
> -
> -static inline void mmio_insl(const void __iomem *addr, u32 *dst, int count)
> -{
> -	while (--count >= 0) {
> -		u32 data = __raw_readl(addr);
> -		*dst = data;
> -		dst++;
> -	}
> -}
> -
> -static inline void mmio_outsb(void __iomem *addr, const u8 *src, int count)
> -{
> -	while (--count >= 0) {
> -		__raw_writeb(*src, addr);
> -		src++;
> -	}
> -}
> -
> -static inline void mmio_outsw(void __iomem *addr, const u16 *src, int count)
> -{
> -	while (--count >= 0) {
> -		__raw_writew(*src, addr);
> -		src++;
> -	}
> -}
> -
> -static inline void mmio_outsl(void __iomem *addr, const u32 *src, int count)
> -{
> -	while (--count >= 0) {
> -		__raw_writel(*src, addr);
> -		src++;
> -	}
> -}
> -
> -void ioread8_rep(const void __iomem *addr, void *dst, unsigned long count)
> -{
> -	mmio_insb(addr, dst, count);
> -}
> -EXPORT_SYMBOL(ioread8_rep);
> -
> -void ioread16_rep(const void __iomem *addr, void *dst, unsigned long count)
> -{
> -	mmio_insw(addr, dst, count);
> -}
> -EXPORT_SYMBOL(ioread16_rep);
> -
> -void ioread32_rep(const void __iomem *addr, void *dst, unsigned long count)
> -{
> -	mmio_insl(addr, dst, count);
> -}
> -EXPORT_SYMBOL(ioread32_rep);
> -
> -void iowrite8_rep(void __iomem *addr, const void *src, unsigned long count)
> -{
> -	mmio_outsb(addr, src, count);
> -}
> -EXPORT_SYMBOL(iowrite8_rep);
> -
> -void iowrite16_rep(void __iomem *addr, const void *src, unsigned long count)
> -{
> -	mmio_outsw(addr, src, count);
> -}
> -EXPORT_SYMBOL(iowrite16_rep);
> -
> -void iowrite32_rep(void __iomem *addr, const void *src, unsigned long count)
> -{
> -	mmio_outsl(addr, src, count);
> -}
> -EXPORT_SYMBOL(iowrite32_rep);
> diff --git a/arch/sh/kernel/ioport.c b/arch/sh/kernel/ioport.c
> index c8aff8a20164..915a3dfd9f02 100644
> --- a/arch/sh/kernel/ioport.c
> +++ b/arch/sh/kernel/ioport.c
> @@ -23,8 +23,3 @@ void __iomem *ioport_map(unsigned long port, unsigned int nr)
>  	return (void __iomem *)(port + sh_io_port_base);
>  }
>  EXPORT_SYMBOL(ioport_map);
> -
> -void ioport_unmap(void __iomem *addr)
> -{
> -}
> -EXPORT_SYMBOL(ioport_unmap);
> diff --git a/arch/sh/lib/io.c b/arch/sh/lib/io.c
> index ebcf7c0a7335..dc6345e4c53b 100644
> --- a/arch/sh/lib/io.c
> +++ b/arch/sh/lib/io.c
> @@ -11,7 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/io.h>
>  
> -void __raw_readsl(const void __iomem *addr, void *datap, int len)
> +void __raw_readsl(const volatile void __iomem *addr, void *datap, int len)
>  {
>  	u32 *data;
>  
> @@ -60,7 +60,7 @@ void __raw_readsl(const void __iomem *addr, void *datap, int len)
>  }
>  EXPORT_SYMBOL(__raw_readsl);
>  
> -void __raw_writesl(void __iomem *addr, const void *data, int len)
> +void __raw_writesl(volatile void __iomem *addr, const void *data, int len)
>  {
>  	if (likely(len != 0)) {
>  		int tmp1;
> diff --git a/drivers/sh/clk/cpg.c b/drivers/sh/clk/cpg.c
> index fd72d9088bdc..64ed7d64458a 100644
> --- a/drivers/sh/clk/cpg.c
> +++ b/drivers/sh/clk/cpg.c
> @@ -26,6 +26,19 @@ static unsigned int sh_clk_read(struct clk *clk)
>  	return ioread32(clk->mapped_reg);
>  }
>  
> +static unsigned int sh_clk_read_status(struct clk *clk)
> +{
> +	void __iomem *mapped_status = (phys_addr_t)clk->status_reg -
> +		(phys_addr_t)clk->enable_reg + clk->mapped_reg;
> +
> +	if (clk->flags & CLK_ENABLE_REG_8BIT)
> +		return ioread8(mapped_status);
> +	else if (clk->flags & CLK_ENABLE_REG_16BIT)
> +		return ioread16(mapped_status);
> +
> +	return ioread32(mapped_status);
> +}
> +
>  static void sh_clk_write(int value, struct clk *clk)
>  {
>  	if (clk->flags & CLK_ENABLE_REG_8BIT)
> @@ -40,20 +53,10 @@ static int sh_clk_mstp_enable(struct clk *clk)
>  {
>  	sh_clk_write(sh_clk_read(clk) & ~(1 << clk->enable_bit), clk);
>  	if (clk->status_reg) {
> -		unsigned int (*read)(const void __iomem *addr);
>  		int i;
> -		void __iomem *mapped_status = (phys_addr_t)clk->status_reg -
> -			(phys_addr_t)clk->enable_reg + clk->mapped_reg;
> -
> -		if (clk->flags & CLK_ENABLE_REG_8BIT)
> -			read = ioread8;
> -		else if (clk->flags & CLK_ENABLE_REG_16BIT)
> -			read = ioread16;
> -		else
> -			read = ioread32;
>  
>  		for (i = 1000;
> -		     (read(mapped_status) & (1 << clk->enable_bit)) && i;
> +		     (sh_clk_read_status(clk) & (1 << clk->enable_bit)) && i;
>  		     i--)
>  			cpu_relax();
>  		if (!i) {

Those are quite a number of changes that I would like to test on real hardware
first before merging them into the kernel.

@Geert: Could you test it on your SH-7751 LANDISK board as well?

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 2/6] sh: remove duplicate ioread/iowrite helpers

[Geert Uytterhoeven]

Hi Adrian,

On Sat, 7 Jun 2025 at 14:08, John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Sat, 2025-03-15 at 11:59 +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > The ioread/iowrite functions on sh only do memory mapped I/O like the
> > generic verion, and never map onto non-MMIO inb/outb variants, so they
> > just add complexity. In particular, the use of asm-generic/iomap.h
> > ties the declaration to the x86 implementation.
> >
> > Remove the custom versions and use the architecture-independent fallback
> > code instead. Some of the calling conventions on sh are different here,
> > so fix that by adding 'volatile' keywords where required by the generic
> > implementation and change the cpg clock driver to no longer depend on
> > the interesting choice of return types for ioread8/ioread16/ioread32.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> Those are quite a number of changes that I would like to test on real hardware
> first before merging them into the kernel.
>
> @Geert: Could you test it on your SH-7751 LANDISK board as well?

Already done for a while, as this patch is commit 2494fce26e434071 ("sh:
remove duplicate ioread/iowrite helpers") in v6.15-rc1 ;-)

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

Re: [PATCH 2/6] sh: remove duplicate ioread/iowrite helpers

[John Paul Adrian Glaubitz]

Hello Geert,

On Sun, 2025-06-08 at 11:39 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Sat, 7 Jun 2025 at 14:08, John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Sat, 2025-03-15 at 11:59 +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > The ioread/iowrite functions on sh only do memory mapped I/O like the
> > > generic verion, and never map onto non-MMIO inb/outb variants, so they
> > > just add complexity. In particular, the use of asm-generic/iomap.h
> > > ties the declaration to the x86 implementation.
> > > 
> > > Remove the custom versions and use the architecture-independent fallback
> > > code instead. Some of the calling conventions on sh are different here,
> > > so fix that by adding 'volatile' keywords where required by the generic
> > > implementation and change the cpg clock driver to no longer depend on
> > > the interesting choice of return types for ioread8/ioread16/ioread32.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> > Those are quite a number of changes that I would like to test on real hardware
> > first before merging them into the kernel.
> > 
> > @Geert: Could you test it on your SH-7751 LANDISK board as well?
> 
> Already done for a while, as this patch is commit 2494fce26e434071 ("sh:
> remove duplicate ioread/iowrite helpers") in v6.15-rc1 ;-)

Well, there is no Tested-By from either of us, so this isn't optimal.

I wished Arnd could have at least pinged me back regarding this. He knows I'm
actively maintaining arch/sh and I would like to properly test and review
such changes.

But I'm not doing this professionally, so I cannot be always there with 100%
capacity. Just pushing such changes in without any input from me defeats the
purpose of a maintainer.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

[PATCH 3/6] parisc: stop using asm-generic/iomap.h

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

parisc uses custom iomap helpers to map bus specific function calls into
a linear __iomem token, but it tries to use the declarations from the x86
"generic iomap" code.

Untangle the two by duplicating the required declations and dropping
the #include that pulls in more stuff that is not needed here, to
allow simplify the generic version later.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/parisc/include/asm/io.h | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index 3143cf29ce27..59d85d2386bb 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -227,36 +227,54 @@ extern void outsl (unsigned long port, const void *src, unsigned long count);
 #define F_EXTEND(x) ((unsigned long)((x) | (0xffffffff00000000ULL)))
 
 #ifdef CONFIG_64BIT
-#define ioread64 ioread64
-#define ioread64be ioread64be
-#define iowrite64 iowrite64
-#define iowrite64be iowrite64be
 extern u64 ioread64(const void __iomem *addr);
 extern u64 ioread64be(const void __iomem *addr);
+#define ioread64 ioread64
+#define ioread64be ioread64be
+
 extern void iowrite64(u64 val, void __iomem *addr);
 extern void iowrite64be(u64 val, void __iomem *addr);
+#define iowrite64 iowrite64
+#define iowrite64be iowrite64be
 #endif
 
-#include <asm-generic/iomap.h>
-/*
- * These get provided from <asm-generic/iomap.h> since parisc does not
- * select GENERIC_IOMAP.
- */
+extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
+extern void ioport_unmap(void __iomem *);
 #define ioport_map ioport_map
 #define ioport_unmap ioport_unmap
+
+extern unsigned int ioread8(const void __iomem *);
+extern unsigned int ioread16(const void __iomem *);
+extern unsigned int ioread16be(const void __iomem *);
+extern unsigned int ioread32(const void __iomem *);
+extern unsigned int ioread32be(const void __iomem *);
 #define ioread8 ioread8
 #define ioread16 ioread16
 #define ioread32 ioread32
 #define ioread16be ioread16be
 #define ioread32be ioread32be
+
+extern void iowrite8(u8, void __iomem *);
+extern void iowrite16(u16, void __iomem *);
+extern void iowrite16be(u16, void __iomem *);
+extern void iowrite32(u32, void __iomem *);
+extern void iowrite32be(u32, void __iomem *);
 #define iowrite8 iowrite8
 #define iowrite16 iowrite16
 #define iowrite32 iowrite32
 #define iowrite16be iowrite16be
 #define iowrite32be iowrite32be
+
+extern void ioread8_rep(const void __iomem *port, void *buf, unsigned long count);
+extern void ioread16_rep(const void __iomem *port, void *buf, unsigned long count);
+extern void ioread32_rep(const void __iomem *port, void *buf, unsigned long count);
 #define ioread8_rep ioread8_rep
 #define ioread16_rep ioread16_rep
 #define ioread32_rep ioread32_rep
+
+extern void iowrite8_rep(void __iomem *port, const void *buf, unsigned long count);
+extern void iowrite16_rep(void __iomem *port, const void *buf, unsigned long count);
+extern void iowrite32_rep(void __iomem *port, const void *buf, unsigned long count);
 #define iowrite8_rep iowrite8_rep
 #define iowrite16_rep iowrite16_rep
 #define iowrite32_rep iowrite32_rep
-- 
2.39.5

[PATCH 4/6] powerpc: asm/io.h: remove split ioread64/iowrite64 helpers

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

In previous kernels, there were conflicting definitions for what
ioread64_lo_hi() and similar functions were supposed to do on
architectures with native 64-bit MMIO. Based on the actual usage in
drivers, they are in fact expected to be a pair of 32-bit accesses on
all architectures, which makes the powerpc64 definition wrong.

Remove it and use the generic implementation instead.

Drivers that want to have split lo/hi or hi/lo accesses on 32-bit
architectures but can use 64-bit accesses where supported should instead
use ioread64()/iowrite64() after including the corresponding header file.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/include/asm/io.h | 48 -----------------------------------
 1 file changed, 48 deletions(-)

diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index fd92ac450169..d36c4ccaca08 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -738,35 +738,11 @@ static inline unsigned int ioread32be(const void __iomem *addr)
 #define ioread32be ioread32be
 
 #ifdef __powerpc64__
-static inline u64 ioread64_lo_hi(const void __iomem *addr)
-{
-	return readq(addr);
-}
-#define ioread64_lo_hi ioread64_lo_hi
-
-static inline u64 ioread64_hi_lo(const void __iomem *addr)
-{
-	return readq(addr);
-}
-#define ioread64_hi_lo ioread64_hi_lo
-
 static inline u64 ioread64be(const void __iomem *addr)
 {
 	return readq_be(addr);
 }
 #define ioread64be ioread64be
-
-static inline u64 ioread64be_lo_hi(const void __iomem *addr)
-{
-	return readq_be(addr);
-}
-#define ioread64be_lo_hi ioread64be_lo_hi
-
-static inline u64 ioread64be_hi_lo(const void __iomem *addr)
-{
-	return readq_be(addr);
-}
-#define ioread64be_hi_lo ioread64be_hi_lo
 #endif /* __powerpc64__ */
 
 static inline void iowrite16be(u16 val, void __iomem *addr)
@@ -782,35 +758,11 @@ static inline void iowrite32be(u32 val, void __iomem *addr)
 #define iowrite32be iowrite32be
 
 #ifdef __powerpc64__
-static inline void iowrite64_lo_hi(u64 val, void __iomem *addr)
-{
-	writeq(val, addr);
-}
-#define iowrite64_lo_hi iowrite64_lo_hi
-
-static inline void iowrite64_hi_lo(u64 val, void __iomem *addr)
-{
-	writeq(val, addr);
-}
-#define iowrite64_hi_lo iowrite64_hi_lo
-
 static inline void iowrite64be(u64 val, void __iomem *addr)
 {
 	writeq_be(val, addr);
 }
 #define iowrite64be iowrite64be
-
-static inline void iowrite64be_lo_hi(u64 val, void __iomem *addr)
-{
-	writeq_be(val, addr);
-}
-#define iowrite64be_lo_hi iowrite64be_lo_hi
-
-static inline void iowrite64be_hi_lo(u64 val, void __iomem *addr)
-{
-	writeq_be(val, addr);
-}
-#define iowrite64be_hi_lo iowrite64be_hi_lo
 #endif /* __powerpc64__ */
 
 struct pci_dev;
-- 
2.39.5

Re: [PATCH 4/6] powerpc: asm/io.h: remove split ioread64/iowrite64 helpers

[Michael Ellerman]

Arnd Bergmann <arnd@kernel.org> writes:
> From: Arnd Bergmann <arnd@arndb.de>
>
> In previous kernels, there were conflicting definitions for what
> ioread64_lo_hi() and similar functions were supposed to do on
> architectures with native 64-bit MMIO. Based on the actual usage in
> drivers, they are in fact expected to be a pair of 32-bit accesses on
> all architectures, which makes the powerpc64 definition wrong.
>
> Remove it and use the generic implementation instead.
>
> Drivers that want to have split lo/hi or hi/lo accesses on 32-bit
> architectures but can use 64-bit accesses where supported should instead
> use ioread64()/iowrite64() after including the corresponding header file.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/powerpc/include/asm/io.h | 48 -----------------------------------
>  1 file changed, 48 deletions(-)

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

[PATCH 5/6] mips: drop GENERIC_IOMAP wrapper

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

All PIO on MIPS platforms is memory mapped, so there is no benefit in
the lib/iomap.c wrappers that switch between inb/outb and readb/writeb
style accessses.

In fact, the '#define PIO_RESERVED 0' setting completely disables
the GENERIC_IOMAP functionality, and the '#define PIO_OFFSET
mips_io_port_base' setting is based on a misunderstanding of what the
offset is meant to do.

MIPS started using GENERIC_IOMAP in 2018 with commit b962aeb02205 ("MIPS:
Use GENERIC_IOMAP") replacing a simple custom implementation of the same
interfaces, but at the time the asm-generic/io.h version was not usable
yet. Since the header is now always included, it's now possible to go
back to the even simpler version.

Use the normal GENERIC_PCI_IOMAP functionality for all mips platforms
without the hacky GENERIC_IOMAP, and provide a custom pci_iounmap()
for the CONFIG_PCI_DRIVERS_LEGACY case to ensure the I/O port base never
gets unmapped.

The readsl() prototype needs an extra 'const' keyword to make it
compatible with the generic ioread32_rep() alias.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/mips/Kconfig          |  2 +-
 arch/mips/include/asm/io.h | 21 ++++++++-------------
 arch/mips/lib/iomap-pci.c  |  9 +++++++++
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 1924f2d83932..2a2120a6d852 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -38,7 +38,6 @@ config MIPS
 	select GENERIC_CMOS_UPDATE
 	select GENERIC_CPU_AUTOPROBE
 	select GENERIC_GETTIMEOFDAY
-	select GENERIC_IOMAP
 	select GENERIC_IRQ_PROBE
 	select GENERIC_IRQ_SHOW
 	select GENERIC_ISA_DMA if EISA
@@ -47,6 +46,7 @@ config MIPS
 	select GENERIC_LIB_CMPDI2
 	select GENERIC_LIB_LSHRDI3
 	select GENERIC_LIB_UCMPDI2
+	select GENERIC_PCI_IOMAP
 	select GENERIC_SCHED_CLOCK if !CAVIUM_OCTEON_SOC
 	select GENERIC_SMP_IDLE_THREAD
 	select GENERIC_IDLE_POLL_SETUP
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index 0bddb568af7c..1fe56d1870a6 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -66,17 +66,6 @@ static inline void set_io_port_base(unsigned long base)
 	mips_io_port_base = base;
 }
 
-/*
- * Provide the necessary definitions for generic iomap. We make use of
- * mips_io_port_base for iomap(), but we don't reserve any low addresses for
- * use with I/O ports.
- */
-
-#define HAVE_ARCH_PIO_SIZE
-#define PIO_OFFSET	mips_io_port_base
-#define PIO_MASK	IO_SPACE_LIMIT
-#define PIO_RESERVED	0x0UL
-
 /*
  * Enforce in-order execution of data I/O.  In the MIPS architecture
  * these are equivalent to corresponding platform-specific memory
@@ -397,8 +386,8 @@ static inline void writes##bwlq(volatile void __iomem *mem,		\
 	}								\
 }									\
 									\
-static inline void reads##bwlq(volatile void __iomem *mem, void *addr,	\
-			       unsigned int count)			\
+static inline void reads##bwlq(const volatile void __iomem *mem,	\
+			       void *addr, unsigned int count)		\
 {									\
 	volatile type *__addr = addr;					\
 									\
@@ -555,6 +544,12 @@ extern void (*_dma_cache_inv)(unsigned long start, unsigned long size);
 
 void __ioread64_copy(void *to, const void __iomem *from, size_t count);
 
+#ifdef CONFIG_PCI_DRIVERS_LEGACY
+struct pci_dev;
+void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
+#define pci_iounmap pci_iounmap
+#endif
+
 #include <asm-generic/io.h>
 
 static inline void *isa_bus_to_virt(unsigned long address)
diff --git a/arch/mips/lib/iomap-pci.c b/arch/mips/lib/iomap-pci.c
index a9cb28813f0b..2f82c776c6d0 100644
--- a/arch/mips/lib/iomap-pci.c
+++ b/arch/mips/lib/iomap-pci.c
@@ -43,4 +43,13 @@ void __iomem *__pci_ioport_map(struct pci_dev *dev,
 	return (void __iomem *) (ctrl->io_map_base + port);
 }
 
+void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
+{
+	struct pci_controller *ctrl = dev->bus->sysdata;
+	void __iomem *base = (void __iomem *)ctrl->io_map_base;
+
+	if (addr < base || addr > (base + resource_size(ctrl->io_resource)))
+		iounmap(addr);
+}
+
 #endif /* CONFIG_PCI_DRIVERS_LEGACY */
-- 
2.39.5

Re: [PATCH 5/6] mips: drop GENERIC_IOMAP wrapper

[Nathan Chancellor]

Hi Arnd,

On Sat, Mar 15, 2025 at 11:59:06AM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> All PIO on MIPS platforms is memory mapped, so there is no benefit in
> the lib/iomap.c wrappers that switch between inb/outb and readb/writeb
> style accessses.
> 
> In fact, the '#define PIO_RESERVED 0' setting completely disables
> the GENERIC_IOMAP functionality, and the '#define PIO_OFFSET
> mips_io_port_base' setting is based on a misunderstanding of what the
> offset is meant to do.
> 
> MIPS started using GENERIC_IOMAP in 2018 with commit b962aeb02205 ("MIPS:
> Use GENERIC_IOMAP") replacing a simple custom implementation of the same
> interfaces, but at the time the asm-generic/io.h version was not usable
> yet. Since the header is now always included, it's now possible to go
> back to the even simpler version.
> 
> Use the normal GENERIC_PCI_IOMAP functionality for all mips platforms
> without the hacky GENERIC_IOMAP, and provide a custom pci_iounmap()
> for the CONFIG_PCI_DRIVERS_LEGACY case to ensure the I/O port base never
> gets unmapped.
> 
> The readsl() prototype needs an extra 'const' keyword to make it
> compatible with the generic ioread32_rep() alias.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/mips/Kconfig          |  2 +-
>  arch/mips/include/asm/io.h | 21 ++++++++-------------
>  arch/mips/lib/iomap-pci.c  |  9 +++++++++
>  3 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 1924f2d83932..2a2120a6d852 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -38,7 +38,6 @@ config MIPS
>  	select GENERIC_CMOS_UPDATE
>  	select GENERIC_CPU_AUTOPROBE
>  	select GENERIC_GETTIMEOFDAY
> -	select GENERIC_IOMAP
>  	select GENERIC_IRQ_PROBE
>  	select GENERIC_IRQ_SHOW
>  	select GENERIC_ISA_DMA if EISA
> @@ -47,6 +46,7 @@ config MIPS
>  	select GENERIC_LIB_CMPDI2
>  	select GENERIC_LIB_LSHRDI3
>  	select GENERIC_LIB_UCMPDI2
> +	select GENERIC_PCI_IOMAP
>  	select GENERIC_SCHED_CLOCK if !CAVIUM_OCTEON_SOC
>  	select GENERIC_SMP_IDLE_THREAD
>  	select GENERIC_IDLE_POLL_SETUP
> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
> index 0bddb568af7c..1fe56d1870a6 100644
> --- a/arch/mips/include/asm/io.h
> +++ b/arch/mips/include/asm/io.h
> @@ -66,17 +66,6 @@ static inline void set_io_port_base(unsigned long base)
>  	mips_io_port_base = base;
>  }
>  
> -/*
> - * Provide the necessary definitions for generic iomap. We make use of
> - * mips_io_port_base for iomap(), but we don't reserve any low addresses for
> - * use with I/O ports.
> - */
> -
> -#define HAVE_ARCH_PIO_SIZE
> -#define PIO_OFFSET	mips_io_port_base
> -#define PIO_MASK	IO_SPACE_LIMIT
> -#define PIO_RESERVED	0x0UL
> -
>  /*
>   * Enforce in-order execution of data I/O.  In the MIPS architecture
>   * these are equivalent to corresponding platform-specific memory
> @@ -397,8 +386,8 @@ static inline void writes##bwlq(volatile void __iomem *mem,		\
>  	}								\
>  }									\
>  									\
> -static inline void reads##bwlq(volatile void __iomem *mem, void *addr,	\
> -			       unsigned int count)			\
> +static inline void reads##bwlq(const volatile void __iomem *mem,	\
> +			       void *addr, unsigned int count)		\
>  {									\
>  	volatile type *__addr = addr;					\
>  									\
> @@ -555,6 +544,12 @@ extern void (*_dma_cache_inv)(unsigned long start, unsigned long size);
>  
>  void __ioread64_copy(void *to, const void __iomem *from, size_t count);
>  
> +#ifdef CONFIG_PCI_DRIVERS_LEGACY
> +struct pci_dev;
> +void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
> +#define pci_iounmap pci_iounmap
> +#endif
> +
>  #include <asm-generic/io.h>
>  
>  static inline void *isa_bus_to_virt(unsigned long address)
> diff --git a/arch/mips/lib/iomap-pci.c b/arch/mips/lib/iomap-pci.c
> index a9cb28813f0b..2f82c776c6d0 100644
> --- a/arch/mips/lib/iomap-pci.c
> +++ b/arch/mips/lib/iomap-pci.c
> @@ -43,4 +43,13 @@ void __iomem *__pci_ioport_map(struct pci_dev *dev,
>  	return (void __iomem *) (ctrl->io_map_base + port);
>  }
>  
> +void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
> +{
> +	struct pci_controller *ctrl = dev->bus->sysdata;
> +	void __iomem *base = (void __iomem *)ctrl->io_map_base;
> +
> +	if (addr < base || addr > (base + resource_size(ctrl->io_resource)))
> +		iounmap(addr);
> +}
> +
>  #endif /* CONFIG_PCI_DRIVERS_LEGACY */
> -- 
> 2.39.5
> 

This change as commit 976bf3aec388 ("mips: drop GENERIC_IOMAP wrapper") in
-next introduces new instances of -Wnull-pointer-arithmetic when building
certain mips configurations with clang.

  $ make -skj"$(nproc)" ARCH=mips LLVM=1 mrproper malta_defconfig init/main.o
  ...
  In file included from init/main.c:17:
  In file included from include/linux/module.h:17:
  In file included from include/linux/kmod.h:9:
  In file included from include/linux/umh.h:4:
  In file included from include/linux/gfp.h:7:
  In file included from include/linux/mmzone.h:22:
  In file included from include/linux/mm_types.h:5:
  In file included from include/linux/mm_types_task.h:14:
  In file included from arch/mips/include/asm/page.h:181:
  In file included from arch/mips/include/asm/io.h:553:
  include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
        |                                                   ~~~~~~~~~~ ^
  1 warning generated.

Cheers,
Nathan

Re: [PATCH 5/6] mips: drop GENERIC_IOMAP wrapper

[Arnd Bergmann]

On Tue, Mar 18, 2025, at 21:39, Nathan Chancellor wrote:
> On Sat, Mar 15, 2025 at 11:59:06AM +0100, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h

>>  void __ioread64_copy(void *to, const void __iomem *from, size_t count);
>>  
>> +#ifdef CONFIG_PCI_DRIVERS_LEGACY
>> +struct pci_dev;
>> +void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
>> +#define pci_iounmap pci_iounmap
>> +#endif
>> +
>>  #include <asm-generic/io.h>
>>  
>>  static inline void *isa_bus_to_virt(unsigned long address)
>> diff --git a/arch/mips/lib/iomap-pci.c b/arch/mips/lib/iomap-pci.c
>> index a9cb28813f0b..2f82c776c6d0 100644
>> --- a/arch/mips/lib/iomap-pci.c
>> +++ b/arch/mips/lib/iomap-pci.c
>> @@ -43,4 +43,13 @@ void __iomem *__pci_ioport_map(struct pci_dev *dev,
>>  	return (void __iomem *) (ctrl->io_map_base + port);
>>  }
>>  
>> +void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
>> +{
>> +	struct pci_controller *ctrl = dev->bus->sysdata;
>> +	void __iomem *base = (void __iomem *)ctrl->io_map_base;
>> +
>> +	if (addr < base || addr > (base + resource_size(ctrl->io_resource)))
>> +		iounmap(addr);
>> +}
>> +
>>  #endif /* CONFIG_PCI_DRIVERS_LEGACY */
>> -- 
>> 2.39.5
>> 
>
> This change as commit 976bf3aec388 ("mips: drop GENERIC_IOMAP wrapper") in
> -next introduces new instances of -Wnull-pointer-arithmetic when building
> certain mips configurations with clang.
>

Thanks for the report, I missed that the generic ioport_map() function
is missing the PCI_IOBASE macro, we should probably remove that from
the asm-generic/io.h header and require architectures to define it
themselves, since the NULL fallback is pretty much always wrong.

There is also a type mismatch between the MIPS
PCI_IOBASE/mips_io_port_base and the one that asm-generic/io.h
expects, so I had to add a couple of extra typecasts, which
makes it rather ugly, but the change below seems to work.

     Arnd

diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index 1fe56d1870a6..78c6573f91f2 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -544,12 +544,16 @@ extern void (*_dma_cache_inv)(unsigned long start, unsigned long size);
 
 void __ioread64_copy(void *to, const void __iomem *from, size_t count);
 
-#ifdef CONFIG_PCI_DRIVERS_LEGACY
+#if defined(CONFIG_PCI) && defined(CONFIG_PCI_DRIVERS_LEGACY)
 struct pci_dev;
 void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
 #define pci_iounmap pci_iounmap
 #endif
 
+#ifndef PCI_IOBASE
+#define PCI_IOBASE ((void __iomem *)mips_io_port_base)
+#endif
+
 #include <asm-generic/io.h>
 
 static inline void *isa_bus_to_virt(unsigned long address)
diff --git a/arch/mips/include/asm/mach-loongson64/spaces.h b/arch/mips/include/asm/mach-loongson64/spaces.h
index ce04e998a37b..dbd26db5f2c5 100644
--- a/arch/mips/include/asm/mach-loongson64/spaces.h
+++ b/arch/mips/include/asm/mach-loongson64/spaces.h
@@ -7,9 +7,10 @@
 #endif /* CONFIG_64BIT */
 
 /* Skip 128k to trap NULL pointer dereferences */
-#define PCI_IOBASE	_AC(0xc000000000000000 + SZ_128K, UL)
+#define PCI_PORT_BASE	_AC(0xc000000000000000 + SZ_128K, UL)
+#define PCI_IOBASE	(void __iomem *)PCI_PORT_BASE
 #define PCI_IOSIZE	SZ_16M
-#define MAP_BASE	(PCI_IOBASE + PCI_IOSIZE)
+#define MAP_BASE	(PCI_PORT_BASE + PCI_IOSIZE)
 
 #define IO_SPACE_LIMIT  (PCI_IOSIZE - 1)
 
diff --git a/arch/mips/include/asm/mach-ralink/spaces.h b/arch/mips/include/asm/mach-ralink/spaces.h
index a9f0570d0f04..a63d106c89c6 100644
--- a/arch/mips/include/asm/mach-ralink/spaces.h
+++ b/arch/mips/include/asm/mach-ralink/spaces.h
@@ -2,7 +2,7 @@
 #ifndef __ASM_MACH_RALINK_SPACES_H_
 #define __ASM_MACH_RALINK_SPACES_H_
 
-#define PCI_IOBASE	mips_io_port_base
+#define PCI_IOBASE	(void __iomem *)mips_io_port_base
 #define PCI_IOSIZE	SZ_64K
 #define IO_SPACE_LIMIT	(PCI_IOSIZE - 1)
 
diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c
index a35dd7311795..b9f90f33fc9a 100644
--- a/arch/mips/loongson64/init.c
+++ b/arch/mips/loongson64/init.c
@@ -128,7 +128,7 @@ void __init prom_init(void)
 	}
 
 	/* init base address of io space */
-	set_io_port_base(PCI_IOBASE);
+	set_io_port_base((unsigned long)PCI_IOBASE);
 
 	if (loongson_sysconf.early_config)
 		loongson_sysconf.early_config();
@@ -178,7 +178,7 @@ static int __init add_legacy_isa_io(struct fwnode_handle *fwnode, resource_size_
 		return -EINVAL;
 	}
 
-	vaddr = PCI_IOBASE + range->io_start;
+	vaddr = (unsigned long)PCI_IOBASE + range->io_start;
 
 	vmap_page_range(vaddr, vaddr + size, hw_start, pgprot_device(PAGE_KERNEL));

Re: [PATCH 5/6] mips: drop GENERIC_IOMAP wrapper

[Nathan Chancellor]

On Tue, Mar 18, 2025 at 10:13:35PM +0100, Arnd Bergmann wrote:
> Thanks for the report, I missed that the generic ioport_map() function
> is missing the PCI_IOBASE macro, we should probably remove that from
> the asm-generic/io.h header and require architectures to define it
> themselves, since the NULL fallback is pretty much always wrong.
> 
> There is also a type mismatch between the MIPS
> PCI_IOBASE/mips_io_port_base and the one that asm-generic/io.h
> expects, so I had to add a couple of extra typecasts, which
> makes it rather ugly, but the change below seems to work.

Thanks, that does make the -Wnull-pointer-arithmetic warnings disappear.
That build still fails in next-20250319 (which includes that change) at
the end with:

  $ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mips-linux- mrproper malta_defconfig all
  ERROR: modpost: "pci_iounmap" [drivers/net/wireless/intel/ipw2x00/ipw2100.ko] undefined!

which appears related to this original change.

Cheers,
Nathan

Re: [PATCH 5/6] mips: drop GENERIC_IOMAP wrapper

[Arnd Bergmann]

On Wed, Mar 19, 2025, at 18:30, Nathan Chancellor wrote:
> On Tue, Mar 18, 2025 at 10:13:35PM +0100, Arnd Bergmann wrote:
>> Thanks for the report, I missed that the generic ioport_map() function
>> is missing the PCI_IOBASE macro, we should probably remove that from
>> the asm-generic/io.h header and require architectures to define it
>> themselves, since the NULL fallback is pretty much always wrong.
>> 
>> There is also a type mismatch between the MIPS
>> PCI_IOBASE/mips_io_port_base and the one that asm-generic/io.h
>> expects, so I had to add a couple of extra typecasts, which
>> makes it rather ugly, but the change below seems to work.
>
> Thanks, that does make the -Wnull-pointer-arithmetic warnings disappear.
> That build still fails in next-20250319 (which includes that change) at
> the end with:
>
>   $ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mips-linux- mrproper 
> malta_defconfig all
>   ERROR: modpost: "pci_iounmap" 
> [drivers/net/wireless/intel/ipw2x00/ipw2100.ko] undefined!
>
> which appears related to this original change.

Right, I had seen and fixed a problem like this in an earlier
version, but forgot the EXPORT_SYMBOL on the legacy host version.

Fixed it better now.

      Arnd

[PATCH 6/6] m68k/nommu: stop using GENERIC_IOMAP

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

There is no need to go through the GENERIC_IOMAP wrapper for PIO on
nommu platforms, since these always come from PCI I/O space that is
itself memory mapped.

Instead, the generic ioport_map() can just return the MMIO location
of the ports directly by applying the PCI_IO_PA offset, while
ioread32/iowrite32 trivially turn into readl/writel as they do
on most other architectures.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/m68k/Kconfig             | 2 +-
 arch/m68k/include/asm/io_no.h | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index b2ed0308c0ea..b50c275fa94d 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -18,7 +18,7 @@ config M68K
 	select DMA_DIRECT_REMAP if M68K_NONCOHERENT_DMA && !COLDFIRE
 	select GENERIC_ATOMIC64
 	select GENERIC_CPU_DEVICES
-	select GENERIC_IOMAP if HAS_IOPORT
+	select GENERIC_IOMAP if HAS_IOPORT && MMU
 	select GENERIC_IRQ_SHOW
 	select GENERIC_LIB_ASHLDI3
 	select GENERIC_LIB_ASHRDI3
diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
index 2c96e8480173..516371d5587a 100644
--- a/arch/m68k/include/asm/io_no.h
+++ b/arch/m68k/include/asm/io_no.h
@@ -123,10 +123,6 @@ static inline void writel(u32 value, volatile void __iomem *addr)
 #define PCI_IO_SIZE	0x00010000		/* 64k */
 #define PCI_IO_MASK	(PCI_IO_SIZE - 1)
 
-#define HAVE_ARCH_PIO_SIZE
-#define PIO_OFFSET	0
-#define PIO_MASK	0xffff
-#define PIO_RESERVED	0x10000
 #define PCI_IOBASE	((void __iomem *) PCI_IO_PA)
 #define PCI_SPACE_LIMIT	PCI_IO_MASK
 #endif /* CONFIG_PCI */
-- 
2.39.5

Re: [PATCH 6/6] m68k/nommu: stop using GENERIC_IOMAP

[Greg Ungerer]

Hi Arnd,

On 15/3/25 20:59, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> There is no need to go through the GENERIC_IOMAP wrapper for PIO on
> nommu platforms, since these always come from PCI I/O space that is
> itself memory mapped.
> 
> Instead, the generic ioport_map() can just return the MMIO location
> of the ports directly by applying the PCI_IO_PA offset, while
> ioread32/iowrite32 trivially turn into readl/writel as they do
> on most other architectures.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

With this applied this fails to build for me:

   UPD     include/generated/utsversion.h
   CC      init/version-timestamp.o
   LD      vmlinux
m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function `quirk_switchtec_ntb_dma_alias':
quirks.c:(.text+0x23e4): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x24fe): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function `disable_igfx_irq':
quirks.c:(.text+0x32f4): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3348): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x338a): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x33d2): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function `reset_ivb_igd':
quirks.c:(.text+0x3502): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3658): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3682): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function `reset_hinic_vf_dev':
quirks.c:(.text+0x3844): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x39fc): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3a86): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3ab4): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function `quirk_reset_lenovo_thinkpad_p50_nvgpu':
quirks.c:(.text+0x3cf6): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function `nvme_disable_and_flr':
quirks.c:(.text+0x3e32): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3eac): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: quirks.c:(.text+0x3fc0): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/devres.o: in function `pcim_addr_resource_release':
devres.c:(.text+0x414): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: devres.c:(.text+0x420): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/devres.o: in function `pcim_iomap':
devres.c:(.text+0x524): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: devres.c:(.text+0x576): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/pci/devres.o: in function `pcim_iomap_range':
devres.c:(.text+0x980): undefined reference to `pci_iomap_range'
m68k-linux-uclibc-ld: drivers/pci/devres.o: in function `pcim_iomap_region':
devres.c:(.text+0xc0e): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: drivers/net/ethernet/intel/e100.o: in function `e100_remove':
e100.c:(.text+0x1fe6): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: drivers/net/ethernet/intel/e100.o: in function `e100_probe':
e100.c:(.text+0x362a): undefined reference to `pci_iomap'
m68k-linux-uclibc-ld: e100.c:(.text+0x381c): undefined reference to `pci_iounmap'
m68k-linux-uclibc-ld: e100.c:(.text+0x3928): undefined reference to `pci_iounmap'
make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1
make[1]: *** [/home/gerg/accelerated-linux.lkml/linux/Makefile:1231: vmlinux] Error 2
make: *** [Makefile:251: __sub-make] Error 2

FWIW this was a m5475evb_defconfig with CONFIG_MMU disabled.

Regards
Greg



> ---
>   arch/m68k/Kconfig             | 2 +-
>   arch/m68k/include/asm/io_no.h | 4 ----
>   2 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> index b2ed0308c0ea..b50c275fa94d 100644
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -18,7 +18,7 @@ config M68K
>   	select DMA_DIRECT_REMAP if M68K_NONCOHERENT_DMA && !COLDFIRE
>   	select GENERIC_ATOMIC64
>   	select GENERIC_CPU_DEVICES
> -	select GENERIC_IOMAP if HAS_IOPORT
> +	select GENERIC_IOMAP if HAS_IOPORT && MMU
>   	select GENERIC_IRQ_SHOW
>   	select GENERIC_LIB_ASHLDI3
>   	select GENERIC_LIB_ASHRDI3
> diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
> index 2c96e8480173..516371d5587a 100644
> --- a/arch/m68k/include/asm/io_no.h
> +++ b/arch/m68k/include/asm/io_no.h
> @@ -123,10 +123,6 @@ static inline void writel(u32 value, volatile void __iomem *addr)
>   #define PCI_IO_SIZE	0x00010000		/* 64k */
>   #define PCI_IO_MASK	(PCI_IO_SIZE - 1)
>   
> -#define HAVE_ARCH_PIO_SIZE
> -#define PIO_OFFSET	0
> -#define PIO_MASK	0xffff
> -#define PIO_RESERVED	0x10000
>   #define PCI_IOBASE	((void __iomem *) PCI_IO_PA)
>   #define PCI_SPACE_LIMIT	PCI_IO_MASK
>   #endif /* CONFIG_PCI */

Re: [PATCH 6/6] m68k/nommu: stop using GENERIC_IOMAP

[Arnd Bergmann]

On Mon, Mar 24, 2025, at 02:33, Greg Ungerer wrote:
> Hi Arnd,
>
> On 15/3/25 20:59, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> There is no need to go through the GENERIC_IOMAP wrapper for PIO on
>> nommu platforms, since these always come from PCI I/O space that is
>> itself memory mapped.
>> 
>> Instead, the generic ioport_map() can just return the MMIO location
>> of the ports directly by applying the PCI_IO_PA offset, while
>> ioread32/iowrite32 trivially turn into readl/writel as they do
>> on most other architectures.
>> 
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> With this applied this fails to build for me:
>
>    UPD     include/generated/utsversion.h
>    CC      init/version-timestamp.o
>    LD      vmlinux
> m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function 
> `quirk_switchtec_ntb_dma_alias':
> quirks.c:(.text+0x23e4): undefined reference to `pci_iomap'
> m68k-linux-uclibc-ld: quirks.c:(.text+0x24fe): undefined reference to 
> `pci_iounmap'
> m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function 
> `disable_igfx_irq':
> quirks.c:(.text+0x32f4): undefined reference to `pci_iomap'
> m68k-linux-uclibc-ld: quirks.c:(.text+0x3348): undefined reference to 
> `pci_iounmap'

Thanks for the report, I was able to reproduce the problem now
and applied the fixup below. I had tested m5475evb_defconfig earlier,
and that built cleanly with PCI enabled, but I had missed how
that still used GENERIC_IOMAP because it has CONFIG_MMU enabled.

Does this fixup work for you?

On a related note, I'm curious about how the MCF54xx chips are
used in practice, as I see that they are the only coldfire chips
with PCI and they all have an MMU. Are there actual users of these
chips that have PCI but choose not to use the MMU? 

      Arnd

8<-----
From a36995e2a64711556c6773797367d165828f6705 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 24 Mar 2025 07:53:47 +0100
Subject: [PATCH] m68k: coldfire: select PCI_IOMAP for PCI

After I dropped CONFIG_GENERIC_IOMAP, some PCI drivers started failing
to link when CONFIG_MMU is disabled:

ERROR: modpost: "pci_iounmap" [drivers/video/fbdev/i740fb.ko] undefined!
ERROR: modpost: "pci_iounmap" [drivers/video/fbdev/vt8623fb.ko] undefined!
ERROR: modpost: "pci_iomap_wc" [drivers/video/fbdev/vt8623fb.ko] undefined!
ERROR: modpost: "pci_iomap" [drivers/video/fbdev/vt8623fb.ko] undefined!
ERROR: modpost: "pci_iounmap" [drivers/video/fbdev/s3fb.ko] undefined!
...

It turns out that there were two mistakes in my patch: on !MMU I forgot
to enable CONFIG_GENERIC_PCI_IOMAP, and for Coldfire with MMU enabled,
teh GENERIC_IOMAP was left in place but incorrectly configured.

Fixes: 9d48cc07d0d7 ("m68k/nommu: stop using GENERIC_IOMAP")
Reported-by: Greg Ungerer <gerg@linux-m68k.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index b50c275fa94d..eb5bb6d36899 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -18,12 +18,13 @@ config M68K
 	select DMA_DIRECT_REMAP if M68K_NONCOHERENT_DMA && !COLDFIRE
 	select GENERIC_ATOMIC64
 	select GENERIC_CPU_DEVICES
-	select GENERIC_IOMAP if HAS_IOPORT && MMU
+	select GENERIC_IOMAP if HAS_IOPORT && MMU && !COLDFIRE
 	select GENERIC_IRQ_SHOW
 	select GENERIC_LIB_ASHLDI3
 	select GENERIC_LIB_ASHRDI3
 	select GENERIC_LIB_LSHRDI3
 	select GENERIC_LIB_MULDI3
+	select GENERIC_PCI_IOMAP if PCI
 	select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA
 	select HAVE_ARCH_LIBGCC_H
 	select HAVE_ARCH_SECCOMP

Re: [PATCH 6/6] m68k/nommu: stop using GENERIC_IOMAP

[Greg Ungerer]

Hi Arnd,

On 24/3/25 18:02, Arnd Bergmann wrote:
> On Mon, Mar 24, 2025, at 02:33, Greg Ungerer wrote:
>> Hi Arnd,
>>
>> On 15/3/25 20:59, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> There is no need to go through the GENERIC_IOMAP wrapper for PIO on
>>> nommu platforms, since these always come from PCI I/O space that is
>>> itself memory mapped.
>>>
>>> Instead, the generic ioport_map() can just return the MMIO location
>>> of the ports directly by applying the PCI_IO_PA offset, while
>>> ioread32/iowrite32 trivially turn into readl/writel as they do
>>> on most other architectures.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> With this applied this fails to build for me:
>>
>>     UPD     include/generated/utsversion.h
>>     CC      init/version-timestamp.o
>>     LD      vmlinux
>> m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function
>> `quirk_switchtec_ntb_dma_alias':
>> quirks.c:(.text+0x23e4): undefined reference to `pci_iomap'
>> m68k-linux-uclibc-ld: quirks.c:(.text+0x24fe): undefined reference to
>> `pci_iounmap'
>> m68k-linux-uclibc-ld: drivers/pci/quirks.o: in function
>> `disable_igfx_irq':
>> quirks.c:(.text+0x32f4): undefined reference to `pci_iomap'
>> m68k-linux-uclibc-ld: quirks.c:(.text+0x3348): undefined reference to
>> `pci_iounmap'
> 
> Thanks for the report, I was able to reproduce the problem now
> and applied the fixup below. I had tested m5475evb_defconfig earlier,
> and that built cleanly with PCI enabled, but I had missed how
> that still used GENERIC_IOMAP because it has CONFIG_MMU enabled.
> 
> Does this fixup work for you?

Yes, this looks good, works for me.
Feel free to add this if you like:

Acked-by: Greg Ungerer <gerg@linux-m68k.org>


> On a related note, I'm curious about how the MCF54xx chips are
> used in practice, as I see that they are the only coldfire chips
> with PCI and they all have an MMU. Are there actual users of these
> chips that have PCI but choose not to use the MMU?

No, I think everyone with these uses them with MMU enabled.

It is probably more of an historical curiosity to use them with
the MMU disabled. That supported pre-dated mainline kernels having
full ColdFire MMU support by a good few years.

Regards
Greg



>        Arnd
> 
> 8<-----
>  From a36995e2a64711556c6773797367d165828f6705 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Mon, 24 Mar 2025 07:53:47 +0100
> Subject: [PATCH] m68k: coldfire: select PCI_IOMAP for PCI
> 
> After I dropped CONFIG_GENERIC_IOMAP, some PCI drivers started failing
> to link when CONFIG_MMU is disabled:
> 
> ERROR: modpost: "pci_iounmap" [drivers/video/fbdev/i740fb.ko] undefined!
> ERROR: modpost: "pci_iounmap" [drivers/video/fbdev/vt8623fb.ko] undefined!
> ERROR: modpost: "pci_iomap_wc" [drivers/video/fbdev/vt8623fb.ko] undefined!
> ERROR: modpost: "pci_iomap" [drivers/video/fbdev/vt8623fb.ko] undefined!
> ERROR: modpost: "pci_iounmap" [drivers/video/fbdev/s3fb.ko] undefined!
> ...
> 
> It turns out that there were two mistakes in my patch: on !MMU I forgot
> to enable CONFIG_GENERIC_PCI_IOMAP, and for Coldfire with MMU enabled,
> teh GENERIC_IOMAP was left in place but incorrectly configured.
> 
> Fixes: 9d48cc07d0d7 ("m68k/nommu: stop using GENERIC_IOMAP")
> Reported-by: Greg Ungerer <gerg@linux-m68k.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> index b50c275fa94d..eb5bb6d36899 100644
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -18,12 +18,13 @@ config M68K
>   	select DMA_DIRECT_REMAP if M68K_NONCOHERENT_DMA && !COLDFIRE
>   	select GENERIC_ATOMIC64
>   	select GENERIC_CPU_DEVICES
> -	select GENERIC_IOMAP if HAS_IOPORT && MMU
> +	select GENERIC_IOMAP if HAS_IOPORT && MMU && !COLDFIRE
>   	select GENERIC_IRQ_SHOW
>   	select GENERIC_LIB_ASHLDI3
>   	select GENERIC_LIB_ASHRDI3
>   	select GENERIC_LIB_LSHRDI3
>   	select GENERIC_LIB_MULDI3
> +	select GENERIC_PCI_IOMAP if PCI
>   	select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA
>   	select HAVE_ARCH_LIBGCC_H
>   	select HAVE_ARCH_SECCOMP

Re: [PATCH 6/6] m68k/nommu: stop using GENERIC_IOMAP

[Arnd Bergmann]

On Mon, Mar 24, 2025, at 14:50, Greg Ungerer wrote:
> On 24/3/25 18:02, Arnd Bergmann wrote:
>> On Mon, Mar 24, 2025, at 02:33, Greg Ungerer wrote:
>>> On 15/3/25 20:59, Arnd Bergmann wrote:
>> 
>> Does this fixup work for you?
>
> Yes, this looks good, works for me.
> Feel free to add this if you like:
>
> Acked-by: Greg Ungerer <gerg@linux-m68k.org>

Added now, thanks!

>> On a related note, I'm curious about how the MCF54xx chips are
>> used in practice, as I see that they are the only coldfire chips
>> with PCI and they all have an MMU. Are there actual users of these
>> chips that have PCI but choose not to use the MMU?
>
> No, I think everyone with these uses them with MMU enabled.
>
> It is probably more of an historical curiosity to use them with
> the MMU disabled. That supported pre-dated mainline kernels having
> full ColdFire MMU support by a good few years.

Ok, good to know. Given that there are no other chips that allow
PCI on !MMU kernels, I wonder if we should just make PCI itself
depend on MMU, and remove the "depends on MMU" for any PCI
drivers that currently have it.

There is no fundamental dependency here, but it is something that
breaks occasionally because of in-kernel interfaces that don't
work as expected on !MMU configurations, and with an extra
dependency we could stop fixing those.

      Arnd