On Fri, 2023-12-01 at 17:27 +0100, Arnd Bergmann wrote: > On Fri, Dec 1, 2023, at 13:16, Philipp Stanner wrote: > > > > Arnd has suggested that architectures defining a custom inb() need > > their > > own iomem_is_ioport(), as well. I've grepped for inb() and found > > the > > following list of archs that define their own: > > - alpha > > - arm > > - m68k <-- > > - parisc > > - powerpc > > - sh > > - sparc > > - x86 <-- > > > > All of those have their own definitons of pci_iounmap(). Therefore, > > they > > don't need our generic version in the first place and, thus, also > > need > > no iomem_is_ioport(). > > What I meant of course is that they should define iomem_is_ioport() > in order to drop the custom pci_iounmap() and have only one remaining > definition of that function left. Ah, gotcha! Yes, that would be neat. Would also allow for droping ARCH_WANTS_GENERIC_PCI_IOUNMAP. > > The one special case that I missed the last time is s390, which > does not use GENERIC_PCI_IOMAP and will just require a separate > copy of pci_iounmap() to go along with the is custom pci_iomap(). > > > The two exceptions are x86 and m68k. The former uses lib/iomap.c > > through > > CONFIG_GENERIC_IOMAP, as Arnd pointed out in the previous > > discussion > > (thus, CONFIG_GENERIC_IOMAP is not really generic in this regard). > > > > So as I see it, only m68k WOULD need its own custom definition of > > iomem_is_ioport(). But as I understand it it doesn't because it > > uses the > > one from asm-generic/pci_iomap.h ?? > > At the moment, m68k gets the pci_iounmap() from lib/iomap.c > if PCI is enabled for coldfire, but that incorrectly calls > iounmap() on PCI_IO_PA if it gets passed a PIO address. > > The version from asm-generic/io.h should fix this. So, to be sure: m68k will use the generic iomem_is_ioport() despite defining its own inb()? > > For classic m68k, there is no PCI, so nothing calls pci_iounmap(). > > > I wasn't entirely sure how to deal with the address ranges for the > > generic implementation in asm-generic/io.h. It's marked with a > > TODO. > > Input appreciated. > > I commented on the function directly. To clarify, I think we should > be able to directly turn each pci_iounmap() definition into > a iomem_is_ioport() definition by keeping the logic unchanged > and just return 'true' for the PIO variant or 'false' for the MMIO > version. > > > I removed the guard around define pci_iounmap in asm-generic/io.h. > > An > > alternative would be to have it be guarded by CONFIG_GENERIC_IOMAP > > and > > CONFIG_GENERIC_PCI_IOMAP, both. Without such a guard, there is no > > collision however, because generic pci_iounmap() from > > drivers/pci/iomap.c will only get pulled in when > > CONFIG_GENERIC_PCI_IOMAP is actually set. > > The "#define pci_iomap" can be removed entirely I think. I also think it can, because first arch/asm/io.h includes asm- generic/io.h. I was just wondering why many other functions in asm-generic/io.h always define their own names.. It's obviously very hard to test which config will break, so I thought it's better safe than sorry here P. > > Arnd >
