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

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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>
>> 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 <[email protected]>


> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> 
> 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