[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210812194330.GA2500473@bjorn-Precision-5520>
Date: Thu, 12 Aug 2021 14:43:30 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Peter Zijlstra <peterz@...radead.org>,
Andy Lutomirski <luto@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Richard Henderson <rth@...ddle.net>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
James E J Bottomley <James.Bottomley@...senPartnership.com>,
Helge Deller <deller@....de>,
"David S . Miller" <davem@...emloft.net>,
Arnd Bergmann <arnd@...db.de>,
Jonathan Corbet <corbet@....net>,
"Michael S . Tsirkin" <mst@...hat.com>,
Peter H Anvin <hpa@...or.com>,
Dave Hansen <dave.hansen@...el.com>,
Tony Luck <tony.luck@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Andi Kleen <ak@...ux.intel.com>,
Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
Sean Christopherson <seanjc@...gle.com>,
Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
x86@...nel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linux-alpha@...r.kernel.org,
linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
sparclinux@...r.kernel.org, linux-arch@...r.kernel.org,
linux-doc@...r.kernel.org,
virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v4 09/15] pci: Consolidate pci_iomap* and pci_iomap*wc
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 <ak@...ux.intel.com>
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().
> This will allow to add more variants without excessive code
> duplications. This patch should have no behavior change.
>
> Signed-off-by: Andi Kleen <ak@...ux.intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.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
>
Powered by blists - more mailing lists