Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
MIME-Version: 1.0
References: <20230802184849.1019466-1-arnd@kernel.org> <20230802184849.1019466-4-arnd@kernel.org>
In-Reply-To: <20230802184849.1019466-4-arnd@kernel.org>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Wed, 13 Sep 2023 14:32:38 +0200
Message-ID: <CAMuHMdVjmD357K-yxxW-jn-6vKsXTg+u1Psw9DftyxH=dQoMEg@mail.gmail.com>
Subject: Re: [PATCH 4/4] sh: machvec: remove custom ioport_{un,}map()
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
        linux-sh@vger.kernel.org, Rich Felker <dalias@libc.org>,
        Yoshinori Sato <ysato@users.sourceforge.jp>,
        linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Arnd,

On Wed, Aug 2, 2023 at 8:49 PM Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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@arndb.de>

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@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