Re: [PATCH v4 09/15] pci: Consolidate pci_iomap* and pci_iomap*wc
From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2021-08-12 19:43:37
Also in:
linux-arch, linux-doc, linux-mips, linux-pci, lkml, sparclinux, virtualization
Is there a branch with all of this applied? I was going to apply this to help take a look at it, but it doesn't apply to v5.14-rc1. I know you listed some prereqs in the cover letter, but it's a fair amount of work to sort all that out. On Wed, Aug 04, 2021 at 05:52:12PM -0700, Kuppuswamy Sathyanarayanan wrote:
From: Andi Kleen <redacted>
If I were applying these, I would silently update the subject lines to match previous commits. Since these will probably be merged via a different tree, you can update if there's a v5: PCI: Consolidate pci_iomap_range(), pci_iomap_wc_range() Also applies to 11/15 and 12/15.
pci_iomap* and pci_iomap*wc are currently duplicated code, except that the _wc variant does not support IO ports. Replace them with a common helper and a callback for the mapping. I used wrappers for the maps because some architectures implement ioremap and friends with macros.
Maybe spell some of this out: pci_iomap_range() and pci_iomap_wc_range() are currently duplicated code, ... Implement them using a common helper, pci_iomap_range_map(), ... Using "pci_iomap*" obscures the name and doesn't save any space. Why is it safe to make pci_iomap_wc_range() support IO ports when it didn't before? That might be desirable, but I think it *is* a functional change here. IIUC, pci_iomap_wc_range() on an IO port range previously returned NULL, and after this patch it will work the same as pci_iomap_range(), i.e., it will return the result of __pci_ioport_map().
quoted hunk ↗ jump to hunk
This will allow to add more variants without excessive code duplications. This patch should have no behavior change. Signed-off-by: Andi Kleen <redacted> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> --- lib/pci_iomap.c | 81 +++++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 37 deletions(-)diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c index 2d3eb1cb73b8..6251c3f651c6 100644 --- a/lib/pci_iomap.c +++ b/lib/pci_iomap.c@@ -10,6 +10,46 @@ #include <linux/export.h> #ifdef CONFIG_PCI + +/* + * Callback wrappers because some architectures define ioremap et.al. + * as macros. + */ +static void __iomem *map_ioremap(phys_addr_t addr, size_t size) +{ + return ioremap(addr, size); +} + +static void __iomem *map_ioremap_wc(phys_addr_t addr, size_t size) +{ + return ioremap_wc(addr, size); +} + +static void __iomem *pci_iomap_range_map(struct pci_dev *dev, + int bar, + unsigned long offset, + unsigned long maxlen, + void __iomem *(*mapm)(phys_addr_t, + size_t)) +{ + 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 <= offset || !start) + return NULL; + len -= offset; + start += offset; + if (maxlen && len > maxlen) + len = maxlen; + if (flags & IORESOURCE_IO) + return __pci_ioport_map(dev, start, len); + if (flags & IORESOURCE_MEM) + return mapm(start, len); + /* What? */ + return NULL; +} + /** * pci_iomap_range - create a virtual mapping cookie for a PCI BAR * @dev: PCI device that owns the BAR@@ -30,22 +70,8 @@ void __iomem *pci_iomap_range(struct pci_dev *dev, unsigned long offset, 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 <= offset || !start) - return NULL; - len -= offset; - start += offset; - if (maxlen && len > maxlen) - len = maxlen; - if (flags & IORESOURCE_IO) - return __pci_ioport_map(dev, start, len); - if (flags & IORESOURCE_MEM) - return ioremap(start, len); - /* What? */ - return NULL; + return pci_iomap_range_map(dev, bar, offset, maxlen, + map_ioremap); } EXPORT_SYMBOL(pci_iomap_range);@@ -70,27 +96,8 @@ void __iomem *pci_iomap_wc_range(struct pci_dev *dev, unsigned long offset, 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 (flags & IORESOURCE_IO) - return NULL; - - if (len <= offset || !start) - return NULL; - - len -= offset; - start += offset; - if (maxlen && len > maxlen) - len = maxlen; - - if (flags & IORESOURCE_MEM) - return ioremap_wc(start, len); - - /* What? */ - return NULL; + return pci_iomap_range_map(dev, bar, offset, maxlen, + map_ioremap_wc); } EXPORT_SYMBOL_GPL(pci_iomap_wc_range);-- 2.25.1