Re: [PATCH 4/4] sh: machvec: remove custom ioport_{un,}map()

[Geert Uytterhoeven]

Hi Arnd,

On Wed, Aug 2, 2023 at 8:49 PM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> These functions were only used on the microdev
> board that is now gone, so remove them to simplify
> the ioport handling.
>
> This could be further simplified to use the generic
> I/O port accessors now.
>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>

Thanks for your patch!

> --- a/arch/sh/include/asm/io.h
> +++ b/arch/sh/include/asm/io.h
> @@ -181,7 +181,7 @@ static inline void pfx##out##bwlq##p(type val, unsigned long port)  \
>  {                                                                      \
>         volatile type *__addr;                                          \
>                                                                         \
> -       __addr = __ioport_map(port, sizeof(type));                      \
> +       __addr = (void __iomem *)sh_io_port_base + port;                \

Note that this adds unconditional users of sh_io_port_base, while
sh_io_port_base is provided by arch/sh/kernel/ioport.c, which is
currently only built if CONFIG_GENERIC_IOMAP=n.

This is not a problem yet, as the final part to enable GENERIC_IOMAP
on SH never made it upstream.  However, Sato-san's series enables
GENERIC_IOMAP for SH_DEVICE_TREE=y builds, leading to a link failure.

A quick fix would be to always build the relevant parts:

--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -23,8 +23,8 @@ obj-y := head_32.o debugtraps.o dumpstack.o
                 \

 ifndef CONFIG_GENERIC_IOMAP
 obj-y                          += iomap.o
-obj-$(CONFIG_HAS_IOPORT_MAP)   += ioport.o
 endif
+obj-$(CONFIG_HAS_IOPORT_MAP)   += ioport.o

 obj-y                          += sys_sh32.o
 obj-y                          += cpu/
--- a/arch/sh/kernel/ioport.c
+++ b/arch/sh/kernel/ioport.c
@@ -12,6 +12,7 @@
 unsigned long sh_io_port_base __read_mostly = -1;
 EXPORT_SYMBOL(sh_io_port_base);

+#ifndef CONFIG_GENERIC_IOMAP
 void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
        void __iomem *ret;
@@ -28,3 +29,4 @@ void ioport_unmap(void __iomem *addr)
 {
 }
 EXPORT_SYMBOL(ioport_unmap);
+#endif /* !CONFIG_GENERIC_IOMAP */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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