Re: [PATCH-RFC 06/10] mips: switch to GENERIC_PCI_IOMAP

[Kevin Cernekee <cernekee@gmail.com>]

On Thu, Nov 24, 2011 at 12:18 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> mips copied pci_iomap from generic code, probably to avoid
> pulling the rest of iomap.c in.  Since that's in
> a separate file now, we can reuse the common implementation.

[snip]

> -       if (flags & IORESOURCE_IO)
> -               return ioport_map_pci(dev, start, len);

While investigating a new warning on the 3.3-rc1 MIPS build (unused
static function ioport_map_pci()), I noticed that this patch has shown
up in Linus' tree as commit eab90291d35438bcebf7c3dc85be66d0f24e3002.

I am not completely clear on the implications it has on mapping PCI I/O regions:

Prior to this change, the MIPS version of pci_iomap() called a
MIPS-specific function, ioport_map_pci(), which tried to use the PCI
controller's io_map_base field to determine the base address of the
PCI I/O space.  It also had a fallback mechanism to deal with the case
where io_map_base is unset.

Now, in 3.3-rc1, the generic version of pci_iomap() is used instead.
This code just calls arch/mips/lib/iomap.c:ioport_map() on these
regions.  ioport_map() falls through to ioport_map_legacy(), which
always uses mips_io_port_base (not the PCI controller's io_map_base)
as the base address.  But on MIPS, it is still permissible to use
different I/O port bases for PCI devices and for legacy (ISA?)
devices.

Is this new behavior desirable, or are there any supported platforms
on which adverse effects might be seen?

As for my part, I don't use PCI I/O regions at all - just memory
regions.  I'm more worried about making sure my tree builds with 0
warnings.

If we do want to move ahead with the switch to GENERIC_PCI_IOMAP now,
I have patches to scrap iomap-pci.c entirely and squash the unused
function warning.

If we still want to support the case where io_map_base !=
mips_io_port_base, maybe it would be better to revert commit eab90291
for 3.3.

Might also want to take a look at SH since it also appears to have an
orphaned ioport_map_pci() function.

What are your thoughts?