diff mbox

[v8,9/9] pci: Remap I/O bus resources into CPU space with pci_remap_iospace()

Message ID 1404240214-9804-10-git-send-email-Liviu.Dudau@arm.com
State New
Headers show

Commit Message

Liviu Dudau July 1, 2014, 6:43 p.m. UTC
Introduce a default implementation for remapping PCI bus I/O resources
onto the CPU address space. Architectures with special needs may
provide their own version, but most should be able to use this one.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/pci/pci.c   | 33 +++++++++++++++++++++++++++++++++
 include/linux/pci.h |  3 +++
 2 files changed, 36 insertions(+)

Comments

Catalin Marinas July 14, 2014, 4:54 p.m. UTC | #1
On Tue, Jul 01, 2014 at 07:43:34PM +0100, Liviu Dudau wrote:
> Introduce a default implementation for remapping PCI bus I/O resources
> onto the CPU address space. Architectures with special needs may
> provide their own version, but most should be able to use this one.
[...]
> +/**
> + *	pci_remap_iospace - Remap the memory mapped I/O space
> + *	@res: Resource describing the I/O space
> + *	@phys_addr: physical address where the range will be mapped.
> + *
> + *	Remap the memory mapped I/O space described by the @res
> + *	into the CPU physical address space. Only architectures
> + *	that have memory mapped IO defined (and hence PCI_IOBASE)
> + *	should call this function.
> + */
> +int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> +{
> +	int err = -ENODEV;
> +
> +#ifdef PCI_IOBASE
> +	if (!(res->flags & IORESOURCE_IO))
> +		return -EINVAL;
> +
> +	if (res->end > IO_SPACE_LIMIT)
> +		return -EINVAL;
> +
> +	err = ioremap_page_range(res->start + (unsigned long)PCI_IOBASE,
> +				res->end + 1 + (unsigned long)PCI_IOBASE,
> +				phys_addr, __pgprot(PROT_DEVICE_nGnRE));

Except that PROT_DEVICE_nGnRE is arm64 only. I think that's a function
that should remain arch specific.
Liviu Dudau July 14, 2014, 4:56 p.m. UTC | #2
On Mon, Jul 14, 2014 at 05:54:43PM +0100, Catalin Marinas wrote:
> On Tue, Jul 01, 2014 at 07:43:34PM +0100, Liviu Dudau wrote:
> > Introduce a default implementation for remapping PCI bus I/O resources
> > onto the CPU address space. Architectures with special needs may
> > provide their own version, but most should be able to use this one.
> [...]
> > +/**
> > + *	pci_remap_iospace - Remap the memory mapped I/O space
> > + *	@res: Resource describing the I/O space
> > + *	@phys_addr: physical address where the range will be mapped.
> > + *
> > + *	Remap the memory mapped I/O space described by the @res
> > + *	into the CPU physical address space. Only architectures
> > + *	that have memory mapped IO defined (and hence PCI_IOBASE)
> > + *	should call this function.
> > + */
> > +int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> > +{
> > +	int err = -ENODEV;
> > +
> > +#ifdef PCI_IOBASE
> > +	if (!(res->flags & IORESOURCE_IO))
> > +		return -EINVAL;
> > +
> > +	if (res->end > IO_SPACE_LIMIT)
> > +		return -EINVAL;
> > +
> > +	err = ioremap_page_range(res->start + (unsigned long)PCI_IOBASE,
> > +				res->end + 1 + (unsigned long)PCI_IOBASE,
> > +				phys_addr, __pgprot(PROT_DEVICE_nGnRE));
> 
> Except that PROT_DEVICE_nGnRE is arm64 only. I think that's a function
> that should remain arch specific.

Yes, I was following Arnd's suggestion and lost track of the fact that
PROT_DEVICE_nGnRE is arm64 specific.

Best regards,
Liviu

> 
> -- 
> Catalin
Arnd Bergmann July 14, 2014, 6:15 p.m. UTC | #3
On Monday 14 July 2014 17:54:43 Catalin Marinas wrote:
> On Tue, Jul 01, 2014 at 07:43:34PM +0100, Liviu Dudau wrote:
> > Introduce a default implementation for remapping PCI bus I/O resources
> > onto the CPU address space. Architectures with special needs may
> > provide their own version, but most should be able to use this one.
> [...]
> > +/**
> > + *   pci_remap_iospace - Remap the memory mapped I/O space
> > + *   @res: Resource describing the I/O space
> > + *   @phys_addr: physical address where the range will be mapped.
> > + *
> > + *   Remap the memory mapped I/O space described by the @res
> > + *   into the CPU physical address space. Only architectures
> > + *   that have memory mapped IO defined (and hence PCI_IOBASE)
> > + *   should call this function.
> > + */
> > +int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> > +{
> > +     int err = -ENODEV;
> > +
> > +#ifdef PCI_IOBASE
> > +     if (!(res->flags & IORESOURCE_IO))
> > +             return -EINVAL;
> > +
> > +     if (res->end > IO_SPACE_LIMIT)
> > +             return -EINVAL;
> > +
> > +     err = ioremap_page_range(res->start + (unsigned long)PCI_IOBASE,
> > +                             res->end + 1 + (unsigned long)PCI_IOBASE,
> > +                             phys_addr, __pgprot(PROT_DEVICE_nGnRE));
> 
> Except that PROT_DEVICE_nGnRE is arm64 only. I think that's a function
> that should remain arch specific.
> 

How about #defining a macro with the correct pgprot value in asm/pci.h
or asm/pgtable.h?
We can provide a default for that in another architecture independent
location.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liviu Dudau July 15, 2014, 12:14 a.m. UTC | #4
On Mon, Jul 14, 2014 at 08:15:48PM +0200, Arnd Bergmann wrote:
> On Monday 14 July 2014 17:54:43 Catalin Marinas wrote:
> > On Tue, Jul 01, 2014 at 07:43:34PM +0100, Liviu Dudau wrote:
> > > Introduce a default implementation for remapping PCI bus I/O resources
> > > onto the CPU address space. Architectures with special needs may
> > > provide their own version, but most should be able to use this one.
> > [...]
> > > +/**
> > > + *   pci_remap_iospace - Remap the memory mapped I/O space
> > > + *   @res: Resource describing the I/O space
> > > + *   @phys_addr: physical address where the range will be mapped.
> > > + *
> > > + *   Remap the memory mapped I/O space described by the @res
> > > + *   into the CPU physical address space. Only architectures
> > > + *   that have memory mapped IO defined (and hence PCI_IOBASE)
> > > + *   should call this function.
> > > + */
> > > +int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> > > +{
> > > +     int err = -ENODEV;
> > > +
> > > +#ifdef PCI_IOBASE
> > > +     if (!(res->flags & IORESOURCE_IO))
> > > +             return -EINVAL;
> > > +
> > > +     if (res->end > IO_SPACE_LIMIT)
> > > +             return -EINVAL;
> > > +
> > > +     err = ioremap_page_range(res->start + (unsigned long)PCI_IOBASE,
> > > +                             res->end + 1 + (unsigned long)PCI_IOBASE,
> > > +                             phys_addr, __pgprot(PROT_DEVICE_nGnRE));
> > 
> > Except that PROT_DEVICE_nGnRE is arm64 only. I think that's a function
> > that should remain arch specific.
> > 
> 
> How about #defining a macro with the correct pgprot value in asm/pci.h
> or asm/pgtable.h?
> We can provide a default for that in another architecture independent
> location.

I was discussing the same thing with Catalin today. It is the most reasonable
approach, as the host bridge driver that is likely to call this function should
not be aware of the architectural flags used here.

Best regards,
Liviu

> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Catalin Marinas July 15, 2014, 9:09 a.m. UTC | #5
On Mon, Jul 14, 2014 at 07:15:48PM +0100, Arnd Bergmann wrote:
> On Monday 14 July 2014 17:54:43 Catalin Marinas wrote:
> > On Tue, Jul 01, 2014 at 07:43:34PM +0100, Liviu Dudau wrote:
> > > Introduce a default implementation for remapping PCI bus I/O resources
> > > onto the CPU address space. Architectures with special needs may
> > > provide their own version, but most should be able to use this one.
> > [...]
> > > +/**
> > > + *   pci_remap_iospace - Remap the memory mapped I/O space
> > > + *   @res: Resource describing the I/O space
> > > + *   @phys_addr: physical address where the range will be mapped.
> > > + *
> > > + *   Remap the memory mapped I/O space described by the @res
> > > + *   into the CPU physical address space. Only architectures
> > > + *   that have memory mapped IO defined (and hence PCI_IOBASE)
> > > + *   should call this function.
> > > + */
> > > +int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> > > +{
> > > +     int err = -ENODEV;
> > > +
> > > +#ifdef PCI_IOBASE
> > > +     if (!(res->flags & IORESOURCE_IO))
> > > +             return -EINVAL;
> > > +
> > > +     if (res->end > IO_SPACE_LIMIT)
> > > +             return -EINVAL;
> > > +
> > > +     err = ioremap_page_range(res->start + (unsigned long)PCI_IOBASE,
> > > +                             res->end + 1 + (unsigned long)PCI_IOBASE,
> > > +                             phys_addr, __pgprot(PROT_DEVICE_nGnRE));
> > 
> > Except that PROT_DEVICE_nGnRE is arm64 only. I think that's a function
> > that should remain arch specific.
> > 
> 
> How about #defining a macro with the correct pgprot value in asm/pci.h
> or asm/pgtable.h?
> We can provide a default for that in another architecture independent
> location.

That should work. We already have pgprot_noncached/writecombine in
asm-generic/pgtable.h which all architectures seem to include. The
default can be pgprot_noncached and we would invoke it as
pgprot_device(PAGE_KERNEL).
diff mbox

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8e65dc3..a90df97 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2708,6 +2708,39 @@  int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
 }
 EXPORT_SYMBOL(pci_request_regions_exclusive);
 
+/**
+ *	pci_remap_iospace - Remap the memory mapped I/O space
+ *	@res: Resource describing the I/O space
+ *	@phys_addr: physical address where the range will be mapped.
+ *
+ *	Remap the memory mapped I/O space described by the @res
+ *	into the CPU physical address space. Only architectures
+ *	that have memory mapped IO defined (and hence PCI_IOBASE)
+ *	should call this function.
+ */
+int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
+{
+	int err = -ENODEV;
+
+#ifdef PCI_IOBASE
+	if (!(res->flags & IORESOURCE_IO))
+		return -EINVAL;
+
+	if (res->end > IO_SPACE_LIMIT)
+		return -EINVAL;
+
+	err = ioremap_page_range(res->start + (unsigned long)PCI_IOBASE,
+				res->end + 1 + (unsigned long)PCI_IOBASE,
+				phys_addr, __pgprot(PROT_DEVICE_nGnRE));
+#else
+	/* this architecture does not have memory mapped I/O space,
+	   so this function should never be called */
+	WARN_ON(1);
+#endif
+
+	return err;
+}
+
 static void __pci_set_master(struct pci_dev *dev, bool enable)
 {
 	u16 old_cmd, cmd;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 556dc5f..65fb1fc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1100,6 +1100,9 @@  int __must_check pci_bus_alloc_resource(struct pci_bus *bus,
 						  resource_size_t),
 			void *alignf_data);
 
+
+int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
+
 static inline dma_addr_t pci_bus_address(struct pci_dev *pdev, int bar)
 {
 	struct pci_bus_region region;