Re: [PATCH v5 RESEND 5/5] lib, pci: unify generic pci_iounmap()

[Arnd Bergmann]

On Fri, Jan 26, 2024, at 14:59, Philipp Stanner wrote:
> On Tue, 2024-01-23 at 15:05 -0600, Bjorn Helgaas wrote:
>> On Thu, Jan 11, 2024 at 09:55:40AM +0100, Philipp Stanner wrote:
>> 
>> The kernel-doc addition could possibly also move there since it isn't
>> related to the unification.
>
> You mean the one from my devres-patch-series? Or documentation
> specifically about pci_iounmap()?
>
>> 
>> >  {
>> > -#ifdef ARCH_HAS_GENERIC_IOPORT_MAP
>> > -       uintptr_t start = (uintptr_t) PCI_IOBASE;
>> > -       uintptr_t addr = (uintptr_t) p;
>> > -
>> > -       if (addr >= start && addr < start + IO_SPACE_LIMIT) {
>> > -               ioport_unmap(p);
>> > +#ifdef CONFIG_HAS_IOPORT_MAP
>> > +       if (iomem_is_ioport(addr)) {
>> > +               ioport_unmap(addr);
>> >                 return;
>> >         }
>> >  #endif
>> > -       iounmap(p);
>> > +
>> > +       iounmap(addr);
>> >  }
>> 
>> > + * If CONFIG_GENERIC_IOMAP is selected and the architecture does
>> > NOT provide its
>> > + * own version, ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT makes sure that
>> > the generic
>> > + * version from asm-generic/io.h is NOT used and instead the
>> > second "generic"
>> > + * version from this file here is used.
>> > + *
>> > + * There are currently two generic versions because of a difficult
>> > cleanup
>> > + * process. Namely, the version in lib/iomap.c once was really
>> > generic when IA64
>> > + * still existed. Today, it's only really used by x86.
>> > + *
>> > + * TODO: Move this function to x86-specific code.
>> 
>> Some of these TODOs look fairly simple.  Are they actually hard, or
>> could they just be done now?
>
> If they were simple from my humble POV I would have implemented them.
> The information about the x86-specficity is from Arnd Bergmann, the
> header-maintainer.
>
> I myself am not that sure how much work it would be to move the entire
> lib/iomap.c file to x86. At least some (possibley "dead") hooks to it
> still exist, for example here:
> arch/powerpc/platforms/Kconfig  # L.189

This one definitely takes some work to untangle, it's selected
by two powerpc platforms, but one doesn't actually need it any
more, and the other one uses it only for non-PCI devices.

I think the other architectures are easier to change and do
fix real bugs, but it's probably best done one at a time.

     Arnd