| Message ID | 66457f7750d7d14229fcf8d0b011aba63185a75d.1322163031.git.mst@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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?
On Sat, Jan 28, 2012 at 02:38:10PM -0800, Kevin Cernekee wrote: > 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: Yes, my bad, I missed the difference between ioport_map_pci and ioport_map for both MIPS and SH. I'll post a patch to fix this, which is probably preferable to reintroducing the code duplication where it might trip us up again.
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index d46f1da..b70c96f 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -2317,6 +2317,7 @@ config PCI bool "Support for PCI controller" depends on HW_HAS_PCI select PCI_DOMAINS + select GENERIC_PCI_IOMAP help Find out whether you have a PCI motherboard. PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside diff --git a/arch/mips/lib/iomap-pci.c b/arch/mips/lib/iomap-pci.c index 2ab899c..2635b1a 100644 --- a/arch/mips/lib/iomap-pci.c +++ b/arch/mips/lib/iomap-pci.c @@ -40,32 +40,6 @@ static void __iomem *ioport_map_pci(struct pci_dev *dev, return (void __iomem *) (ctrl->io_map_base + port); } -/* - * Create a virtual mapping cookie for a PCI BAR (memory or IO) - */ -void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen) -{ - resource_size_t start = pci_resource_start(dev, bar); - resource_size_t len = pci_resource_len(dev, bar); - unsigned long flags = pci_resource_flags(dev, bar); - - if (!len || !start) - return NULL; - if (maxlen && len > maxlen) - len = maxlen; - if (flags & IORESOURCE_IO) - return ioport_map_pci(dev, start, len); - if (flags & IORESOURCE_MEM) { - if (flags & IORESOURCE_CACHEABLE) - return ioremap(start, len); - return ioremap_nocache(start, len); - } - /* What? */ - return NULL; -} - -EXPORT_SYMBOL(pci_iomap); - void pci_iounmap(struct pci_dev *dev, void __iomem * addr) { iounmap(addr);
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. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- arch/mips/Kconfig | 1 + arch/mips/lib/iomap-pci.c | 26 -------------------------- 2 files changed, 1 insertions(+), 26 deletions(-)