* [PATCH v2 01/22] PCI: remove __weak tag from pci_remap_iospace()
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface Lorenzo Pieralisi
` (20 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
pci_remap_iospace() is marked as a weak symbol even though
no architecture is currently overriding it; given that its
implementation internals have already code paths that are
arch specific (ie PCI_IOBASE and ioremap_page_range() attributes)
there is no need to leave the weak symbol in the kernel since the
same functionality can be achieved by customizing per-arch the
corresponding functionality.
Remove the __weak symbol from pci_remap_iospace().
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7904d02..bd98674 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3363,7 +3363,7 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
* Only architectures that have memory mapped IO functions defined
* (and the PCI_IOBASE value defined) should call this function.
*/
-int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
+int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
{
#if defined(PCI_IOBASE) && defined(CONFIG_MMU)
unsigned long vaddr = (unsigned long)PCI_IOBASE + res->start;
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 01/22] PCI: remove __weak tag from pci_remap_iospace() Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-28 1:41 ` Bjorn Helgaas
2017-03-27 9:49 ` [PATCH v2 03/22] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute Lorenzo Pieralisi
` (19 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
Posting") mandate non-posted configuration transactions. As further
highlighted in the PCIe specifications (4.0 - Rev0.3, "Ordering
Considerations for the Enhanced Configuration Access Mechanism"),
through ECAM and ECAM-derivative configuration mechanism, the memory
mapped transactions from the host CPU into Configuration Requests on the
PCI express fabric may create ordering problems for software because
writes to memory address are typically posted transactions (unless the
architecture can enforce through virtual address mapping non-posted
write transactions behaviour) but writes to Configuration Space are not
posted on the PCI express fabric.
Current DT and ACPI host bridge controllers map PCI configuration space
(ECAM and ECAM-derivative) into the virtual address space through
ioremap() calls, that are non-cacheable device accesses on most
architectures, but may provide "bufferable" or "posted" write semantics
in architecture like eg ARM/ARM64 that allow ioremap'ed regions writes
to be buffered in the bus connecting the host CPU to the PCI fabric;
this behaviour, as underlined in the PCIe specifications, may trigger
transactions ordering rules and must be prevented.
Introduce a new generic and explicit API to create a memory
mapping for ECAM and ECAM-derivative config space area that
defaults to ioremap_nocache() (which should provide a sane default
behaviour) but still allowing architectures on which ioremap_nocache()
results in posted write transactions to override the function
call with an arch specific implementation that complies with
the PCI specifications for configuration transactions.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
---
arch/alpha/include/asm/io.h | 1 +
arch/avr32/include/asm/io.h | 1 +
arch/frv/include/asm/io.h | 1 +
arch/ia64/include/asm/io.h | 1 +
arch/x86/include/asm/io.h | 1 +
include/asm-generic/io.h | 4 ++++
6 files changed, 9 insertions(+)
diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index ff40491..27379ea 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -300,6 +300,7 @@ static inline void __iomem * ioremap_nocache(unsigned long offset,
}
#define ioremap_uc ioremap_nocache
+#define ioremap_nopost ioremap_nocache
static inline void iounmap(volatile void __iomem *addr)
{
diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index f855646..3f1ced8 100644
--- a/arch/avr32/include/asm/io.h
+++ b/arch/avr32/include/asm/io.h
@@ -298,6 +298,7 @@ extern void __iounmap(void __iomem *addr);
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
#define ioremap_uc ioremap_nocache
+#define ioremap_nopost ioremap_nocache
#define cached(addr) P1SEGADDR(addr)
#define uncached(addr) P2SEGADDR(addr)
diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
index 8062fc7..302fb8c 100644
--- a/arch/frv/include/asm/io.h
+++ b/arch/frv/include/asm/io.h
@@ -290,6 +290,7 @@ static inline void __iomem *ioremap_fullcache(unsigned long physaddr, unsigned l
#define ioremap_wc ioremap_nocache
#define ioremap_uc ioremap_nocache
+#define ioremap_nopost ioremap_nocache
extern void iounmap(void volatile __iomem *addr);
diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index 5de673a..70a4985 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -434,6 +434,7 @@ static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned lo
}
#define ioremap_cache ioremap_cache
#define ioremap_uc ioremap_nocache
+#define ioremap_nopost ioremap_nocache
/*
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 7afb0e2..50b292f 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
#define ioremap_uc ioremap_uc
+#define ioremap_nopost ioremap_nocache
extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ef015e..0e81938 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
#endif /* CONFIG_GENERIC_IOMAP */
#endif /* CONFIG_HAS_IOPORT_MAP */
+#ifndef ioremap_nopost
+#define ioremap_nopost ioremap_nocache
+#endif
+
#ifndef xlate_dev_kmem_ptr
#define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
static inline void *xlate_dev_kmem_ptr(void *addr)
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-03-27 9:49 ` [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface Lorenzo Pieralisi
@ 2017-03-28 1:41 ` Bjorn Helgaas
2017-03-28 14:45 ` Lorenzo Pieralisi
0 siblings, 1 reply; 48+ messages in thread
From: Bjorn Helgaas @ 2017-03-28 1:41 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> Posting") mandate non-posted configuration transactions. As further
> highlighted in the PCIe specifications (4.0 - Rev0.3, "Ordering
> Considerations for the Enhanced Configuration Access Mechanism"),
> through ECAM and ECAM-derivative configuration mechanism, the memory
> mapped transactions from the host CPU into Configuration Requests on the
> PCI express fabric may create ordering problems for software because
> writes to memory address are typically posted transactions (unless the
> architecture can enforce through virtual address mapping non-posted
> write transactions behaviour) but writes to Configuration Space are not
> posted on the PCI express fabric.
>
> Current DT and ACPI host bridge controllers map PCI configuration space
> (ECAM and ECAM-derivative) into the virtual address space through
> ioremap() calls, that are non-cacheable device accesses on most
> architectures, but may provide "bufferable" or "posted" write semantics
> in architecture like eg ARM/ARM64 that allow ioremap'ed regions writes
> to be buffered in the bus connecting the host CPU to the PCI fabric;
> this behaviour, as underlined in the PCIe specifications, may trigger
> transactions ordering rules and must be prevented.
>
> Introduce a new generic and explicit API to create a memory
> mapping for ECAM and ECAM-derivative config space area that
> defaults to ioremap_nocache() (which should provide a sane default
> behaviour) but still allowing architectures on which ioremap_nocache()
> results in posted write transactions to override the function
> call with an arch specific implementation that complies with
> the PCI specifications for configuration transactions.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
> ---
> arch/alpha/include/asm/io.h | 1 +
> arch/avr32/include/asm/io.h | 1 +
> arch/frv/include/asm/io.h | 1 +
> arch/ia64/include/asm/io.h | 1 +
> arch/x86/include/asm/io.h | 1 +
> include/asm-generic/io.h | 4 ++++
> 6 files changed, 9 insertions(+)
>
> diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
> index ff40491..27379ea 100644
> --- a/arch/alpha/include/asm/io.h
> +++ b/arch/alpha/include/asm/io.h
> @@ -300,6 +300,7 @@ static inline void __iomem * ioremap_nocache(unsigned long offset,
> }
>
> #define ioremap_uc ioremap_nocache
> +#define ioremap_nopost ioremap_nocache
>
> static inline void iounmap(volatile void __iomem *addr)
> {
> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> index f855646..3f1ced8 100644
> --- a/arch/avr32/include/asm/io.h
> +++ b/arch/avr32/include/asm/io.h
> @@ -298,6 +298,7 @@ extern void __iounmap(void __iomem *addr);
> #define ioremap_wc ioremap_nocache
> #define ioremap_wt ioremap_nocache
> #define ioremap_uc ioremap_nocache
> +#define ioremap_nopost ioremap_nocache
>
> #define cached(addr) P1SEGADDR(addr)
> #define uncached(addr) P2SEGADDR(addr)
> diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
> index 8062fc7..302fb8c 100644
> --- a/arch/frv/include/asm/io.h
> +++ b/arch/frv/include/asm/io.h
> @@ -290,6 +290,7 @@ static inline void __iomem *ioremap_fullcache(unsigned long physaddr, unsigned l
>
> #define ioremap_wc ioremap_nocache
> #define ioremap_uc ioremap_nocache
> +#define ioremap_nopost ioremap_nocache
>
> extern void iounmap(void volatile __iomem *addr);
>
> diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
> index 5de673a..70a4985 100644
> --- a/arch/ia64/include/asm/io.h
> +++ b/arch/ia64/include/asm/io.h
> @@ -434,6 +434,7 @@ static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned lo
> }
> #define ioremap_cache ioremap_cache
> #define ioremap_uc ioremap_nocache
> +#define ioremap_nopost ioremap_nocache
>
>
> /*
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index 7afb0e2..50b292f 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> #define ioremap_uc ioremap_uc
> +#define ioremap_nopost ioremap_nocache
These are all the same as the default from asm-generic.h. Do we really
need these definitions in alpha, avr32, frv, ia64, x86?
> extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
> extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index 7ef015e..0e81938 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> #endif /* CONFIG_GENERIC_IOMAP */
> #endif /* CONFIG_HAS_IOPORT_MAP */
>
> +#ifndef ioremap_nopost
> +#define ioremap_nopost ioremap_nocache
> +#endif
> +
> #ifndef xlate_dev_kmem_ptr
> #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> static inline void *xlate_dev_kmem_ptr(void *addr)
> --
> 2.10.0
>
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-03-28 1:41 ` Bjorn Helgaas
@ 2017-03-28 14:45 ` Lorenzo Pieralisi
2017-03-30 16:47 ` Bjorn Helgaas
2017-04-05 10:58 ` Russell King - ARM Linux
0 siblings, 2 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> > Posting") mandate non-posted configuration transactions. As further
> > highlighted in the PCIe specifications (4.0 - Rev0.3, "Ordering
> > Considerations for the Enhanced Configuration Access Mechanism"),
> > through ECAM and ECAM-derivative configuration mechanism, the memory
> > mapped transactions from the host CPU into Configuration Requests on the
> > PCI express fabric may create ordering problems for software because
> > writes to memory address are typically posted transactions (unless the
> > architecture can enforce through virtual address mapping non-posted
> > write transactions behaviour) but writes to Configuration Space are not
> > posted on the PCI express fabric.
> >
> > Current DT and ACPI host bridge controllers map PCI configuration space
> > (ECAM and ECAM-derivative) into the virtual address space through
> > ioremap() calls, that are non-cacheable device accesses on most
> > architectures, but may provide "bufferable" or "posted" write semantics
> > in architecture like eg ARM/ARM64 that allow ioremap'ed regions writes
> > to be buffered in the bus connecting the host CPU to the PCI fabric;
> > this behaviour, as underlined in the PCIe specifications, may trigger
> > transactions ordering rules and must be prevented.
> >
> > Introduce a new generic and explicit API to create a memory
> > mapping for ECAM and ECAM-derivative config space area that
> > defaults to ioremap_nocache() (which should provide a sane default
> > behaviour) but still allowing architectures on which ioremap_nocache()
> > results in posted write transactions to override the function
> > call with an arch specific implementation that complies with
> > the PCI specifications for configuration transactions.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
> > ---
> > arch/alpha/include/asm/io.h | 1 +
> > arch/avr32/include/asm/io.h | 1 +
> > arch/frv/include/asm/io.h | 1 +
> > arch/ia64/include/asm/io.h | 1 +
> > arch/x86/include/asm/io.h | 1 +
> > include/asm-generic/io.h | 4 ++++
> > 6 files changed, 9 insertions(+)
> >
> > diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
> > index ff40491..27379ea 100644
> > --- a/arch/alpha/include/asm/io.h
> > +++ b/arch/alpha/include/asm/io.h
> > @@ -300,6 +300,7 @@ static inline void __iomem * ioremap_nocache(unsigned long offset,
> > }
> >
> > #define ioremap_uc ioremap_nocache
> > +#define ioremap_nopost ioremap_nocache
> >
> > static inline void iounmap(volatile void __iomem *addr)
> > {
> > diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> > index f855646..3f1ced8 100644
> > --- a/arch/avr32/include/asm/io.h
> > +++ b/arch/avr32/include/asm/io.h
> > @@ -298,6 +298,7 @@ extern void __iounmap(void __iomem *addr);
> > #define ioremap_wc ioremap_nocache
> > #define ioremap_wt ioremap_nocache
> > #define ioremap_uc ioremap_nocache
> > +#define ioremap_nopost ioremap_nocache
> >
> > #define cached(addr) P1SEGADDR(addr)
> > #define uncached(addr) P2SEGADDR(addr)
> > diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
> > index 8062fc7..302fb8c 100644
> > --- a/arch/frv/include/asm/io.h
> > +++ b/arch/frv/include/asm/io.h
> > @@ -290,6 +290,7 @@ static inline void __iomem *ioremap_fullcache(unsigned long physaddr, unsigned l
> >
> > #define ioremap_wc ioremap_nocache
> > #define ioremap_uc ioremap_nocache
> > +#define ioremap_nopost ioremap_nocache
> >
> > extern void iounmap(void volatile __iomem *addr);
> >
> > diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
> > index 5de673a..70a4985 100644
> > --- a/arch/ia64/include/asm/io.h
> > +++ b/arch/ia64/include/asm/io.h
> > @@ -434,6 +434,7 @@ static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned lo
> > }
> > #define ioremap_cache ioremap_cache
> > #define ioremap_uc ioremap_nocache
> > +#define ioremap_nopost ioremap_nocache
> >
> >
> > /*
> > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > index 7afb0e2..50b292f 100644
> > --- a/arch/x86/include/asm/io.h
> > +++ b/arch/x86/include/asm/io.h
> > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > #define ioremap_uc ioremap_uc
> > +#define ioremap_nopost ioremap_nocache
>
> These are all the same as the default from asm-generic.h. Do we really
> need these definitions in alpha, avr32, frv, ia64, x86?
Those arches do not include asm-generic.h (I suppose for a good reason)
but a definition is needed anyway if we want code using ioremap_nopost()
to be unconditional. This is one way of doing it, there are others not
sure they are any better, I am open to suggestions.
Thanks,
Lorenzo
> > extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
> > extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size, unsigned long prot_val);
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index 7ef015e..0e81938 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > #endif /* CONFIG_GENERIC_IOMAP */
> > #endif /* CONFIG_HAS_IOPORT_MAP */
> >
> > +#ifndef ioremap_nopost
> > +#define ioremap_nopost ioremap_nocache
> > +#endif
> > +
> > #ifndef xlate_dev_kmem_ptr
> > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > static inline void *xlate_dev_kmem_ptr(void *addr)
> > --
> > 2.10.0
> >
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-03-28 14:45 ` Lorenzo Pieralisi
@ 2017-03-30 16:47 ` Bjorn Helgaas
2017-04-05 10:58 ` Russell King - ARM Linux
1 sibling, 0 replies; 48+ messages in thread
From: Bjorn Helgaas @ 2017-03-30 16:47 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > #define ioremap_uc ioremap_uc
> > > +#define ioremap_nopost ioremap_nocache
> >
> > These are all the same as the default from asm-generic.h. Do we really
> > need these definitions in alpha, avr32, frv, ia64, x86?
>
> Those arches do not include asm-generic.h (I suppose for a good reason)
OK, that explains it. I'm not at all sure there's a good reason for
those arches not using asm-generic.h, but I haven't looked at the code
to see.
> but a definition is needed anyway if we want code using ioremap_nopost()
> to be unconditional. This is one way of doing it, there are others not
> sure they are any better, I am open to suggestions.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-03-28 14:45 ` Lorenzo Pieralisi
2017-03-30 16:47 ` Bjorn Helgaas
@ 2017-04-05 10:58 ` Russell King - ARM Linux
2017-04-05 12:38 ` Lorenzo Pieralisi
1 sibling, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2017-04-05 10:58 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > index 7afb0e2..50b292f 100644
> > > --- a/arch/x86/include/asm/io.h
> > > +++ b/arch/x86/include/asm/io.h
> > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > #define ioremap_uc ioremap_uc
> > > +#define ioremap_nopost ioremap_nocache
> >
> > These are all the same as the default from asm-generic.h. Do we really
> > need these definitions in alpha, avr32, frv, ia64, x86?
>
> Those arches do not include asm-generic.h (I suppose for a good reason)
> but a definition is needed anyway if we want code using ioremap_nopost()
> to be unconditional. This is one way of doing it, there are others not
> sure they are any better, I am open to suggestions.
We do have linux/io.h, which should be included in preference to asm/io.h.
linux/io.h has existed for years, but I still see (from time to time)
patches adding drivers that (imho incorrectly) use asm/io.h.
Also, this:
> > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > index 7ef015e..0e81938 100644
> > > --- a/include/asm-generic/io.h
> > > +++ b/include/asm-generic/io.h
> > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > #endif /* CONFIG_GENERIC_IOMAP */
> > > #endif /* CONFIG_HAS_IOPORT_MAP */
> > >
> > > +#ifndef ioremap_nopost
> > > +#define ioremap_nopost ioremap_nocache
> > > +#endif
> > > +
> > > #ifndef xlate_dev_kmem_ptr
> > > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > static inline void *xlate_dev_kmem_ptr(void *addr)
could well be located in linux/io.h, which would make it available
everywhere.
I'd suggest one change to this though:
#ifndef ioremap_nopost
static inline void __iomem *ioremap_nopost(resource_size_t offset,
unsigned long size)
{
return ioremap_nocache(offset, size);
}
#endif
This way, if someone forgets to define ioremap_nopost() in their
asm/io.h but provides a definition or extern prototype for
ioremap_nopost(), we end up with a compile error to highlight the
error, rather than the error being hidden by the preprocessor.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-05 10:58 ` Russell King - ARM Linux
@ 2017-04-05 12:38 ` Lorenzo Pieralisi
2017-04-06 10:26 ` Lorenzo Pieralisi
0 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-05 12:38 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > index 7afb0e2..50b292f 100644
> > > > --- a/arch/x86/include/asm/io.h
> > > > +++ b/arch/x86/include/asm/io.h
> > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > #define ioremap_uc ioremap_uc
> > > > +#define ioremap_nopost ioremap_nocache
> > >
> > > These are all the same as the default from asm-generic.h. Do we really
> > > need these definitions in alpha, avr32, frv, ia64, x86?
> >
> > Those arches do not include asm-generic.h (I suppose for a good reason)
> > but a definition is needed anyway if we want code using ioremap_nopost()
> > to be unconditional. This is one way of doing it, there are others not
> > sure they are any better, I am open to suggestions.
>
> We do have linux/io.h, which should be included in preference to asm/io.h.
> linux/io.h has existed for years, but I still see (from time to time)
> patches adding drivers that (imho incorrectly) use asm/io.h.
>
> Also, this:
>
> > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > index 7ef015e..0e81938 100644
> > > > --- a/include/asm-generic/io.h
> > > > +++ b/include/asm-generic/io.h
> > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > #endif /* CONFIG_GENERIC_IOMAP */
> > > > #endif /* CONFIG_HAS_IOPORT_MAP */
> > > >
> > > > +#ifndef ioremap_nopost
> > > > +#define ioremap_nopost ioremap_nocache
> > > > +#endif
> > > > +
> > > > #ifndef xlate_dev_kmem_ptr
> > > > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > static inline void *xlate_dev_kmem_ptr(void *addr)
>
> could well be located in linux/io.h, which would make it available
> everywhere.
>
> I'd suggest one change to this though:
>
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(resource_size_t offset,
> unsigned long size)
> {
> return ioremap_nocache(offset, size);
> }
> #endif
>
> This way, if someone forgets to define ioremap_nopost() in their
> asm/io.h but provides a definition or extern prototype for
> ioremap_nopost(), we end up with a compile error to highlight the
> error, rather than the error being hidden by the preprocessor.
Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
linux/io.h, for whatever reason, does not seem to contain ioremap_*
prototypes; this does not mean we should not add it there but that
would look odd (with all others ioremap_* in asm/io.h), all I am
saying. This stuff requires some clean-up regardless, for the records.
As for the static inline, I did that and that did not make the
kbuild robot happy at all on some arches:
eg: openrisc
>> include/asm-generic/io.h:922:9: error: implicit declaration of
>> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
return ioremap_nocache(offset, size);
I will have another stab at it since what you put forward makes sense,
I just have to find a way that works across arches.
Thanks !
Lorenzo
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-05 12:38 ` Lorenzo Pieralisi
@ 2017-04-06 10:26 ` Lorenzo Pieralisi
2017-04-06 10:47 ` Russell King - ARM Linux
2017-04-06 10:53 ` Luis R. Rodriguez
0 siblings, 2 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-06 10:26 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > index 7afb0e2..50b292f 100644
> > > > > --- a/arch/x86/include/asm/io.h
> > > > > +++ b/arch/x86/include/asm/io.h
> > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > #define ioremap_uc ioremap_uc
> > > > > +#define ioremap_nopost ioremap_nocache
> > > >
> > > > These are all the same as the default from asm-generic.h. Do we really
> > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > >
> > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > to be unconditional. This is one way of doing it, there are others not
> > > sure they are any better, I am open to suggestions.
> >
> > We do have linux/io.h, which should be included in preference to asm/io.h.
> > linux/io.h has existed for years, but I still see (from time to time)
> > patches adding drivers that (imho incorrectly) use asm/io.h.
> >
> > Also, this:
> >
> > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > index 7ef015e..0e81938 100644
> > > > > --- a/include/asm-generic/io.h
> > > > > +++ b/include/asm-generic/io.h
> > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > #endif /* CONFIG_GENERIC_IOMAP */
> > > > > #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > >
> > > > > +#ifndef ioremap_nopost
> > > > > +#define ioremap_nopost ioremap_nocache
> > > > > +#endif
> > > > > +
> > > > > #ifndef xlate_dev_kmem_ptr
> > > > > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > static inline void *xlate_dev_kmem_ptr(void *addr)
> >
> > could well be located in linux/io.h, which would make it available
> > everywhere.
> >
> > I'd suggest one change to this though:
> >
> > #ifndef ioremap_nopost
> > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > unsigned long size)
> > {
> > return ioremap_nocache(offset, size);
> > }
> > #endif
> >
> > This way, if someone forgets to define ioremap_nopost() in their
> > asm/io.h but provides a definition or extern prototype for
> > ioremap_nopost(), we end up with a compile error to highlight the
> > error, rather than the error being hidden by the preprocessor.
>
> Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> linux/io.h, for whatever reason, does not seem to contain ioremap_*
> prototypes; this does not mean we should not add it there but that
> would look odd (with all others ioremap_* in asm/io.h), all I am
> saying. This stuff requires some clean-up regardless, for the records.
>
> As for the static inline, I did that and that did not make the
> kbuild robot happy at all on some arches:
>
> eg: openrisc
>
> >> include/asm-generic/io.h:922:9: error: implicit declaration of
> >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> return ioremap_nocache(offset, size);
Indeed, the static inline ioremap_nocache() fallback does not work
on all arches (whether I add the fallback in linux/io.h or
asm-generic/io.h is irrelevant), I bump into issues such as the one
reported above.
I could make it:
#ifndef ioremap_nopost
static inline void __iomem *ioremap_nopost(resource_size_t offset,
unsigned long size)
{
return NULL;
}
#endif
which would force arches to define ioremap_nopost() (if they need it).
This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
but honestly history is hard to follow here.
Thoughts ?
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 10:26 ` Lorenzo Pieralisi
@ 2017-04-06 10:47 ` Russell King - ARM Linux
2017-04-10 14:30 ` Lorenzo Pieralisi
2017-04-06 10:53 ` Luis R. Rodriguez
1 sibling, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2017-04-06 10:47 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > eg: openrisc
> >
> > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > return ioremap_nocache(offset, size);
>
> Indeed, the static inline ioremap_nocache() fallback does not work
> on all arches (whether I add the fallback in linux/io.h or
> asm-generic/io.h is irrelevant), I bump into issues such as the one
> reported above.
>From what I can see:
(a) openrisc does define ioremap_nocache() in its asm/io.h
(b) these do not:
$ grep -L 'ioremap_nocache' arch/*/include/asm/io.h
arch/blackfin/include/asm/io.h
arch/h8300/include/asm/io.h
arch/m68k/include/asm/io.h
arch/score/include/asm/io.h
arch/sparc/include/asm/io.h
Out of those, blackfin, h8300 and score do not define it, whereas m68k
and sparc do in other headers included by asm/io.h. So it looks like
we have three problem architectures that don't define an ioremap_nocache().
PCI on blackfin depends on BROKEN, so it's not selectable. From what I
can tell, h8300 and score do not allow PCI to be enabled (but maybe its
buried elsewhere in their Kconfig files, I didn't check.)
So, I think a way around this is to make ioremap_nopost() conditional
on PCI in linux/io.h for the time being - if its scope wants to be
enlarged, then these three architectures would need to have either an
ioremap_nopost() implementation added, or an ioremap_nocache()
implementation.
> I could make it:
>
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(resource_size_t offset,
> unsigned long size)
> {
> return NULL;
> }
> #endif
>
> which would force arches to define ioremap_nopost() (if they need it).
>
> This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> but honestly history is hard to follow here.
If we are going to consider doing that, the correct question we need to be
asking is whether anything will break as a result of this - is there any
existing arch using the code as it stands that will end up being broken
when we switch PCI to use ioremap_nopost(), and we end up using this NULL-
returning default?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 10:47 ` Russell King - ARM Linux
@ 2017-04-10 14:30 ` Lorenzo Pieralisi
0 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-10 14:30 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 11:47:37AM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > > eg: openrisc
> > >
> > > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > > return ioremap_nocache(offset, size);
> >
> > Indeed, the static inline ioremap_nocache() fallback does not work
> > on all arches (whether I add the fallback in linux/io.h or
> > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > reported above.
>
> From what I can see:
>
> (a) openrisc does define ioremap_nocache() in its asm/io.h
> (b) these do not:
>
> $ grep -L 'ioremap_nocache' arch/*/include/asm/io.h
> arch/blackfin/include/asm/io.h
> arch/h8300/include/asm/io.h
> arch/m68k/include/asm/io.h
> arch/score/include/asm/io.h
> arch/sparc/include/asm/io.h
>
> Out of those, blackfin, h8300 and score do not define it, whereas m68k
> and sparc do in other headers included by asm/io.h. So it looks like
> we have three problem architectures that don't define an ioremap_nocache().
>
> PCI on blackfin depends on BROKEN, so it's not selectable. From what I
> can tell, h8300 and score do not allow PCI to be enabled (but maybe its
> buried elsewhere in their Kconfig files, I didn't check.)
>
> So, I think a way around this is to make ioremap_nopost() conditional
> on PCI in linux/io.h for the time being - if its scope wants to be
> enlarged, then these three architectures would need to have either an
> ioremap_nopost() implementation added, or an ioremap_nocache()
> implementation.
Ok, I implemented asm-generic/ioremap-nopost.h, I do not think we need
a CONFIG_PCI guard (kbuild has not barfed at it, yet), given that
blackfin and h8300 are !MMU and score selects NO_IOMEM.
Patch below is all arches squashed into one commit, I probably have
to split it one per arch which will make this a 50-patch series in
total:
https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git/commit/?h=pci/config-io-mappings-fix-v3&id=acc0be820c809101e02ab7cb170f802db53af934
If I hear no complaints I will split patch above one per arch and
repost the series shortly (even though I think I should make two series
out of it, one asm-generic/ioremap-nopost.h boilerplate and second
ARM/ARM64 implementation + PCI drivers).
Lorenzo
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 10:26 ` Lorenzo Pieralisi
2017-04-06 10:47 ` Russell King - ARM Linux
@ 2017-04-06 10:53 ` Luis R. Rodriguez
2017-04-06 11:38 ` Lorenzo Pieralisi
2017-04-06 12:11 ` Russell King - ARM Linux
1 sibling, 2 replies; 48+ messages in thread
From: Luis R. Rodriguez @ 2017-04-06 10:53 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > > index 7afb0e2..50b292f 100644
> > > > > > --- a/arch/x86/include/asm/io.h
> > > > > > +++ b/arch/x86/include/asm/io.h
> > > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > > #define ioremap_uc ioremap_uc
> > > > > > +#define ioremap_nopost ioremap_nocache
> > > > >
> > > > > These are all the same as the default from asm-generic.h. Do we really
> > > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > >
> > > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > > to be unconditional. This is one way of doing it, there are others not
> > > > sure they are any better, I am open to suggestions.
> > >
> > > We do have linux/io.h, which should be included in preference to asm/io.h.
> > > linux/io.h has existed for years, but I still see (from time to time)
> > > patches adding drivers that (imho incorrectly) use asm/io.h.
> > >
> > > Also, this:
> > >
> > > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > > index 7ef015e..0e81938 100644
> > > > > > --- a/include/asm-generic/io.h
> > > > > > +++ b/include/asm-generic/io.h
> > > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > > #endif /* CONFIG_GENERIC_IOMAP */
> > > > > > #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > > >
> > > > > > +#ifndef ioremap_nopost
> > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > +#endif
> > > > > > +
> > > > > > #ifndef xlate_dev_kmem_ptr
> > > > > > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > > static inline void *xlate_dev_kmem_ptr(void *addr)
> > >
> > > could well be located in linux/io.h, which would make it available
> > > everywhere.
> > >
> > > I'd suggest one change to this though:
> > >
> > > #ifndef ioremap_nopost
> > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > unsigned long size)
> > > {
> > > return ioremap_nocache(offset, size);
> > > }
> > > #endif
> > >
> > > This way, if someone forgets to define ioremap_nopost() in their
> > > asm/io.h but provides a definition or extern prototype for
> > > ioremap_nopost(), we end up with a compile error to highlight the
> > > error, rather than the error being hidden by the preprocessor.
> >
> > Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> > linux/io.h, for whatever reason, does not seem to contain ioremap_*
> > prototypes; this does not mean we should not add it there but that
> > would look odd (with all others ioremap_* in asm/io.h), all I am
> > saying. This stuff requires some clean-up regardless, for the records.
> >
> > As for the static inline, I did that and that did not make the
> > kbuild robot happy at all on some arches:
> >
> > eg: openrisc
> >
> > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > return ioremap_nocache(offset, size);
>
> Indeed, the static inline ioremap_nocache() fallback does not work
> on all arches (whether I add the fallback in linux/io.h or
> asm-generic/io.h is irrelevant), I bump into issues such as the one
> reported above.
Its also not *safe* to assume on behalf of all architectures a new ioremap
call is both a good idea and proper.
> I could make it:
>
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(resource_size_t offset,
> unsigned long size)
> {
> return NULL;
> }
> #endif
>
> which would force arches to define ioremap_nopost() (if they need it).
Yes, please do this.
> This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> but honestly history is hard to follow here.
>
> Thoughts ?
Hard to follow ?
I think commit 8c7ea50c010b2 ("x86/mm, asm-generic: Add IOMMU ioremap_uc()
variant default") makes it very clear that historically we have made bad
assumptions and these assumptions are not safe, so the only correct thing we
can do to not stall development is to do what you did above, and each
architecture then can add support for its proper mapping. Its up to each
architecture maintainer to be attentive and review these patches, if they don't
get to it, this will just not work for the new driver that needs it, such is
life, its better than having incorrect defaults spread for all architectures.
The old strategy was very sloppy. Its why I tried to be as clear as possible on
both the commit log and headers about this. If the commit log and headers were
not clear, please help clarify this more somehow in your patches.
Luis
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 10:53 ` Luis R. Rodriguez
@ 2017-04-06 11:38 ` Lorenzo Pieralisi
2017-04-06 11:59 ` Luis R. Rodriguez
2017-04-06 12:11 ` Russell King - ARM Linux
1 sibling, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-06 11:38 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > > > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > > > index 7afb0e2..50b292f 100644
> > > > > > > --- a/arch/x86/include/asm/io.h
> > > > > > > +++ b/arch/x86/include/asm/io.h
> > > > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > > > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > > > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > > > #define ioremap_uc ioremap_uc
> > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > >
> > > > > > These are all the same as the default from asm-generic.h. Do we really
> > > > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > > >
> > > > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > > > to be unconditional. This is one way of doing it, there are others not
> > > > > sure they are any better, I am open to suggestions.
> > > >
> > > > We do have linux/io.h, which should be included in preference to asm/io.h.
> > > > linux/io.h has existed for years, but I still see (from time to time)
> > > > patches adding drivers that (imho incorrectly) use asm/io.h.
> > > >
> > > > Also, this:
> > > >
> > > > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > > > index 7ef015e..0e81938 100644
> > > > > > > --- a/include/asm-generic/io.h
> > > > > > > +++ b/include/asm-generic/io.h
> > > > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > > > #endif /* CONFIG_GENERIC_IOMAP */
> > > > > > > #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > > > >
> > > > > > > +#ifndef ioremap_nopost
> > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > > +#endif
> > > > > > > +
> > > > > > > #ifndef xlate_dev_kmem_ptr
> > > > > > > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > > > static inline void *xlate_dev_kmem_ptr(void *addr)
> > > >
> > > > could well be located in linux/io.h, which would make it available
> > > > everywhere.
> > > >
> > > > I'd suggest one change to this though:
> > > >
> > > > #ifndef ioremap_nopost
> > > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > > unsigned long size)
> > > > {
> > > > return ioremap_nocache(offset, size);
> > > > }
> > > > #endif
> > > >
> > > > This way, if someone forgets to define ioremap_nopost() in their
> > > > asm/io.h but provides a definition or extern prototype for
> > > > ioremap_nopost(), we end up with a compile error to highlight the
> > > > error, rather than the error being hidden by the preprocessor.
> > >
> > > Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> > > linux/io.h, for whatever reason, does not seem to contain ioremap_*
> > > prototypes; this does not mean we should not add it there but that
> > > would look odd (with all others ioremap_* in asm/io.h), all I am
> > > saying. This stuff requires some clean-up regardless, for the records.
> > >
> > > As for the static inline, I did that and that did not make the
> > > kbuild robot happy at all on some arches:
> > >
> > > eg: openrisc
> > >
> > > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > > return ioremap_nocache(offset, size);
> >
> > Indeed, the static inline ioremap_nocache() fallback does not work
> > on all arches (whether I add the fallback in linux/io.h or
> > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > reported above.
>
> Its also not *safe* to assume on behalf of all architectures a new ioremap
> call is both a good idea and proper.
>
> > I could make it:
> >
> > #ifndef ioremap_nopost
> > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > unsigned long size)
> > {
> > return NULL;
> > }
> > #endif
> >
> > which would force arches to define ioremap_nopost() (if they need it).
>
> Yes, please do this.
Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
having it in linux/io.h is a bit odd given that it would be the only
ioremap_* implementation declared there, I think we need more
consistency here instead of deviating again from what you did.
> > This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> > but honestly history is hard to follow here.
> >
> > Thoughts ?
>
> Hard to follow ?
I was referring to the whole asm-generic/io.h history.
> I think commit 8c7ea50c010b2 ("x86/mm, asm-generic: Add IOMMU ioremap_uc()
> variant default") makes it very clear that historically we have made bad
> assumptions and these assumptions are not safe, so the only correct thing we
> can do to not stall development is to do what you did above, and each
> architecture then can add support for its proper mapping. Its up to each
> architecture maintainer to be attentive and review these patches, if they don't
> get to it, this will just not work for the new driver that needs it, such is
> life, its better than having incorrect defaults spread for all architectures.
>
> The old strategy was very sloppy. Its why I tried to be as clear as possible on
> both the commit log and headers about this. If the commit log and headers were
> not clear, please help clarify this more somehow in your patches.
I see your point and I can replicate (probably I will have to patch
microblaze too since some of the PCI drivers patches in this series
that I updated to use ioremap_nopost() will run on it too) what you
did for ioremap_uc() for ioremap_nopost(), I am ok with that, I need
to know if we all are.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 11:38 ` Lorenzo Pieralisi
@ 2017-04-06 11:59 ` Luis R. Rodriguez
2017-04-06 13:07 ` Russell King - ARM Linux
0 siblings, 1 reply; 48+ messages in thread
From: Luis R. Rodriguez @ 2017-04-06 11:59 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 12:38:45PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Apr 05, 2017 at 01:38:12PM +0100, Lorenzo Pieralisi wrote:
> > > > On Wed, Apr 05, 2017 at 11:58:41AM +0100, Russell King - ARM Linux wrote:
> > > > > On Tue, Mar 28, 2017 at 03:45:43PM +0100, Lorenzo Pieralisi wrote:
> > > > > > On Mon, Mar 27, 2017 at 08:41:10PM -0500, Bjorn Helgaas wrote:
> > > > > > > On Mon, Mar 27, 2017 at 10:49:30AM +0100, Lorenzo Pieralisi wrote:
> > > > > > > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > > > > > > > index 7afb0e2..50b292f 100644
> > > > > > > > --- a/arch/x86/include/asm/io.h
> > > > > > > > +++ b/arch/x86/include/asm/io.h
> > > > > > > > @@ -171,6 +171,7 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
> > > > > > > > extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
> > > > > > > > extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
> > > > > > > > #define ioremap_uc ioremap_uc
> > > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > >
> > > > > > > These are all the same as the default from asm-generic.h. Do we really
> > > > > > > need these definitions in alpha, avr32, frv, ia64, x86?
> > > > > >
> > > > > > Those arches do not include asm-generic.h (I suppose for a good reason)
> > > > > > but a definition is needed anyway if we want code using ioremap_nopost()
> > > > > > to be unconditional. This is one way of doing it, there are others not
> > > > > > sure they are any better, I am open to suggestions.
> > > > >
> > > > > We do have linux/io.h, which should be included in preference to asm/io.h.
> > > > > linux/io.h has existed for years, but I still see (from time to time)
> > > > > patches adding drivers that (imho incorrectly) use asm/io.h.
> > > > >
> > > > > Also, this:
> > > > >
> > > > > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > > > > > > index 7ef015e..0e81938 100644
> > > > > > > > --- a/include/asm-generic/io.h
> > > > > > > > +++ b/include/asm-generic/io.h
> > > > > > > > @@ -915,6 +915,10 @@ extern void ioport_unmap(void __iomem *p);
> > > > > > > > #endif /* CONFIG_GENERIC_IOMAP */
> > > > > > > > #endif /* CONFIG_HAS_IOPORT_MAP */
> > > > > > > >
> > > > > > > > +#ifndef ioremap_nopost
> > > > > > > > +#define ioremap_nopost ioremap_nocache
> > > > > > > > +#endif
> > > > > > > > +
> > > > > > > > #ifndef xlate_dev_kmem_ptr
> > > > > > > > #define xlate_dev_kmem_ptr xlate_dev_kmem_ptr
> > > > > > > > static inline void *xlate_dev_kmem_ptr(void *addr)
> > > > >
> > > > > could well be located in linux/io.h, which would make it available
> > > > > everywhere.
> > > > >
> > > > > I'd suggest one change to this though:
> > > > >
> > > > > #ifndef ioremap_nopost
> > > > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > > > unsigned long size)
> > > > > {
> > > > > return ioremap_nocache(offset, size);
> > > > > }
> > > > > #endif
> > > > >
> > > > > This way, if someone forgets to define ioremap_nopost() in their
> > > > > asm/io.h but provides a definition or extern prototype for
> > > > > ioremap_nopost(), we end up with a compile error to highlight the
> > > > > error, rather than the error being hidden by the preprocessor.
> > > >
> > > > Yes, I could add ioremap_nopost() to linux/io.h I did not do it because
> > > > linux/io.h, for whatever reason, does not seem to contain ioremap_*
> > > > prototypes; this does not mean we should not add it there but that
> > > > would look odd (with all others ioremap_* in asm/io.h), all I am
> > > > saying. This stuff requires some clean-up regardless, for the records.
> > > >
> > > > As for the static inline, I did that and that did not make the
> > > > kbuild robot happy at all on some arches:
> > > >
> > > > eg: openrisc
> > > >
> > > > >> include/asm-generic/io.h:922:9: error: implicit declaration of
> > > > >> function 'ioremap_nocache' [-Werror=implicit-function-declaration]
> > > > return ioremap_nocache(offset, size);
> > >
> > > Indeed, the static inline ioremap_nocache() fallback does not work
> > > on all arches (whether I add the fallback in linux/io.h or
> > > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > > reported above.
> >
> > Its also not *safe* to assume on behalf of all architectures a new ioremap
> > call is both a good idea and proper.
> >
> > > I could make it:
> > >
> > > #ifndef ioremap_nopost
> > > static inline void __iomem *ioremap_nopost(resource_size_t offset,
> > > unsigned long size)
> > > {
> > > return NULL;
> > > }
> > > #endif
> > >
> > > which would force arches to define ioremap_nopost() (if they need it).
> >
> > Yes, please do this.
>
> Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
> having it in linux/io.h is a bit odd given that it would be the only
> ioremap_* implementation declared there, I think we need more
> consistency here instead of deviating again from what you did.
asm-generic/io.h is the right place for generics which let you override.
> > > This is what I *assume* was done for ioremap_uc() in asm-generic/io.h,
> > > but honestly history is hard to follow here.
> > >
> > > Thoughts ?
> >
> > Hard to follow ?
>
> I was referring to the whole asm-generic/io.h history.
>
> > I think commit 8c7ea50c010b2 ("x86/mm, asm-generic: Add IOMMU ioremap_uc()
> > variant default") makes it very clear that historically we have made bad
> > assumptions and these assumptions are not safe, so the only correct thing we
> > can do to not stall development is to do what you did above, and each
> > architecture then can add support for its proper mapping. Its up to each
> > architecture maintainer to be attentive and review these patches, if they don't
> > get to it, this will just not work for the new driver that needs it, such is
> > life, its better than having incorrect defaults spread for all architectures.
> >
> > The old strategy was very sloppy. Its why I tried to be as clear as possible on
> > both the commit log and headers about this. If the commit log and headers were
> > not clear, please help clarify this more somehow in your patches.
>
> I see your point and I can replicate (probably I will have to patch
> microblaze too since some of the PCI drivers patches in this series
> that I updated to use ioremap_nopost() will run on it too)
Cool right so if you add support for the archs for the drivers that you know
use this new ioremap then there is no regressions, and then only if a new
driver gets added and an arch needs this they will also need this, but that
is precisely the sort of requirement thought process we want.
> what you
> did for ioremap_uc() for ioremap_nopost(), I am ok with that, I need
> to know if we all are.
Sure. And again, if consensus is built (again) on that strategy please
update the docs to ensure that this is even clearer for the next person.
I thought it was rather clear now but it does not seem so.
Luis
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 11:59 ` Luis R. Rodriguez
@ 2017-04-06 13:07 ` Russell King - ARM Linux
2017-04-06 16:21 ` Lorenzo Pieralisi
0 siblings, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2017-04-06 13:07 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 01:59:07PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 06, 2017 at 12:38:45PM +0100, Lorenzo Pieralisi wrote:
> > Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
> > having it in linux/io.h is a bit odd given that it would be the only
> > ioremap_* implementation declared there, I think we need more
> > consistency here instead of deviating again from what you did.
>
> asm-generic/io.h is the right place for generics which let you override.
I disagree for two reasons, and I will refer you to Linus' comments back
in 2005 at http://yarchive.net/comp/linux/generic.html
1) asm-generic/io.h has become an ifdef mess, every single definition in
there is conditional. The reason for this has happened is that
architectures can't pick and choose what they want from a single file
unless something like that is done. It would be _far_ better to
split this file up by functional group - eg, ISA IO accessors,
io(read|write)*(), read|write*(), and so forth.
2) We're at the point where we have various .c files around the kernel
_directly_ including asm-generic header files, which means the
use of asm-generic headers is no longer a choice of the architecture.
3) asm-generic/ started out life as the place where example
implementations of asm/*.h header files were found, and but has
grown into a place where we dump default implementations. We had
a place for default implementations for years, which were the
linux/*.h headers. We have now ended up with a mixture of both
techniques, even for something like io.h, we have the messy
asm-generic/io.h, the architecture's asm/io.h, and then linux/io.h.
We have ended up with both linux/io.h and asm-generic/io.h containing
default definitions.
We've had the rule that where both linux/foo.h and asm/foo.h exist, the
linux/ counterpart is the preferred include file. That didn't really
happen with asm/io.h due to the number of users that there were, but
when new stuff was added to asm/io.h which we wanted to be generic,
linux/io.h was created to contain that. This actually pre-dates the
"let's clutter up asm-generic with shared arch stuff" push.
Now, what you say _may_ make sense if we have an
asm-generic/ioremap-nopost.h header which just defines a default
ioremap_nopost.h implementation, and architectures would then be able to
choose whether to include that or not depending on whether they provide
their own implementation. No ugly ifdefs are necessary with that
approach.
Out of the three choices, I would much rather see the
asm-generic/ioremap-nopost.h approach. However, the down-side to that
is it means all architectures asm/io.h would need to be touched to
explicitly include that.
What I'm absolutely certain of, though, is that making asm-generic/io.h
even worse by adding yet more conditional stuff to it is not a sane way
forward.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 13:07 ` Russell King - ARM Linux
@ 2017-04-06 16:21 ` Lorenzo Pieralisi
2017-04-06 16:40 ` Russell King - ARM Linux
0 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-06 16:21 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 02:07:27PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 01:59:07PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 06, 2017 at 12:38:45PM +0100, Lorenzo Pieralisi wrote:
> > > Ok but where ? linux/io.h or asm-generic/io.h ? As I replied to Russell,
> > > having it in linux/io.h is a bit odd given that it would be the only
> > > ioremap_* implementation declared there, I think we need more
> > > consistency here instead of deviating again from what you did.
> >
> > asm-generic/io.h is the right place for generics which let you override.
>
> I disagree for two reasons, and I will refer you to Linus' comments back
> in 2005 at http://yarchive.net/comp/linux/generic.html
>
> 1) asm-generic/io.h has become an ifdef mess, every single definition in
> there is conditional. The reason for this has happened is that
> architectures can't pick and choose what they want from a single file
> unless something like that is done. It would be _far_ better to
> split this file up by functional group - eg, ISA IO accessors,
> io(read|write)*(), read|write*(), and so forth.
>
> 2) We're at the point where we have various .c files around the kernel
> _directly_ including asm-generic header files, which means the
> use of asm-generic headers is no longer a choice of the architecture.
>
> 3) asm-generic/ started out life as the place where example
> implementations of asm/*.h header files were found, and but has
> grown into a place where we dump default implementations. We had
> a place for default implementations for years, which were the
> linux/*.h headers. We have now ended up with a mixture of both
> techniques, even for something like io.h, we have the messy
> asm-generic/io.h, the architecture's asm/io.h, and then linux/io.h.
> We have ended up with both linux/io.h and asm-generic/io.h containing
> default definitions.
>
> We've had the rule that where both linux/foo.h and asm/foo.h exist, the
> linux/ counterpart is the preferred include file. That didn't really
> happen with asm/io.h due to the number of users that there were, but
> when new stuff was added to asm/io.h which we wanted to be generic,
> linux/io.h was created to contain that. This actually pre-dates the
> "let's clutter up asm-generic with shared arch stuff" push.
>
> Now, what you say _may_ make sense if we have an
> asm-generic/ioremap-nopost.h header which just defines a default
> ioremap_nopost.h implementation, and architectures would then be able to
> choose whether to include that or not depending on whether they provide
> their own implementation. No ugly ifdefs are necessary with that
> approach.
>
> Out of the three choices, I would much rather see the
> asm-generic/ioremap-nopost.h approach. However, the down-side to that
> is it means all architectures asm/io.h would need to be touched to
> explicitly include that.
>
> What I'm absolutely certain of, though, is that making asm-generic/io.h
> even worse by adding yet more conditional stuff to it is not a sane way
> forward.
Ok, so:
(1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
contain something like
static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
{
return ioremap_nocache(offset, size);
}
Funny bit is that it has to be included by asm*/io.h files _after_
ioremap_nocache has been #defined (that's the reason my approach was
failing miserably even on arches like eg powerpc (see [1] below) that
does have ioremap_nocache), not sure it is going to be very nice to have
an include in the middle of asm*/io.h include files (and I have to patch
all arches which I can do).
Or
(2) I add:
#ifndef ioremap_nopost
static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
{
return NULL; <-(making it return ioremap_nocache() does not
work, see [1] for the reason)
}
#endif
in linux/io.h
(3) ioremap_nopost follows Luis' ioremap_uc approach
(1)-(2) bypass completely what Luis did for ioremap_uc(), which implies
that we have yet another way of implementing ioremap_*.
I would like to get this patchset done so if you have an opinion it
is time to state it.
Thanks,
Lorenzo
[1] powerpc build report:
tree: https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/config-io-mappings-fix-v3
head: 283a324b549a662346c95c05b08983dd5b83354b
commit: e66b493fe93226c02b1a33355f79db7ed6efe718 [2/23] linux/io.h: add ioremap_nopost remap interface
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout e66b493fe93226c02b1a33355f79db7ed6efe718
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/io.h:28:0,
from arch/powerpc/oprofile/op_model_cell.c:29:
include/linux/io.h: In function 'ioremap_nopost':
include/linux/io.h:169:9: error: implicit declaration of function 'ioremap_nocache' [-Werror=implicit-function-declaration]
return ioremap_nocache(offset, size);
^~~~~~~~~~~~~~~
>> include/linux/io.h:169:9: error: return makes pointer from integer without a cast [-Werror=int-conversion]
return ioremap_nocache(offset, size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +169 include/linux/io.h
163 }
164 #endif
165
166 #ifndef ioremap_nopost
167 static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
168 {
> 169 return ioremap_nocache(offset, size);
170 }
171 #endif
172
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 16:21 ` Lorenzo Pieralisi
@ 2017-04-06 16:40 ` Russell King - ARM Linux
2017-04-06 17:09 ` Lorenzo Pieralisi
0 siblings, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2017-04-06 16:40 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> Ok, so:
>
> (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
> contain something like
>
> static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> {
> return ioremap_nocache(offset, size);
> }
>
> Funny bit is that it has to be included by asm*/io.h files _after_
> ioremap_nocache has been #defined (that's the reason my approach was
> failing miserably even on arches like eg powerpc (see [1] below) that
> does have ioremap_nocache),
PowerPC does have ioremap_nocache() though:
/**
* ioremap - map bus memory into CPU space
...
* * ioremap_nocache is identical to ioremap
extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size) ioremap((addr), (size))
and this include file is included very early on in linux/io.h. I don't
see anything that conditionalises it on anything except __KERNEL__. So,
the report from 0-day really doesn't make any sense to me.
Do we know how we're ending up in linux/io.h line 169 without having
picked up the ioremap_nocache() definition provided by PowerPC's
asm/io.h ?
> not sure it is going to be very nice to have
> an include in the middle of asm*/io.h include files (and I have to patch
> all arches which I can do).
You mean like we already have to do with this asm-generic/io.h thing in
the ARM io.h header file, because we need to define all the accessors
first, to prevent the asm-generic/io.h thing defining them for us?
Given how asm-generic has headed in this regard, having include files
at all sorts of strange locations within the architecture asm/*.h
header files has become quite normal, unfortunately.
> (2) I add:
>
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> {
> return NULL; <-(making it return ioremap_nocache() does not
> work, see [1] for the reason)
> }
> #endif
>
> in linux/io.h
... which breaks the kernel if ioremap_nopost is missing from the arch
header, and one of the drivers that you're modifying to use this new
ioremap variant happens to be built and used on such an architecture.
> (3) ioremap_nopost follows Luis' ioremap_uc approach
Same problem as (2), as I understand correctly.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 16:40 ` Russell King - ARM Linux
@ 2017-04-06 17:09 ` Lorenzo Pieralisi
2017-04-06 17:19 ` Russell King - ARM Linux
0 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-06 17:09 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 05:40:16PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> > Ok, so:
> >
> > (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
> > contain something like
> >
> > static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> > {
> > return ioremap_nocache(offset, size);
> > }
> >
> > Funny bit is that it has to be included by asm*/io.h files _after_
> > ioremap_nocache has been #defined (that's the reason my approach was
> > failing miserably even on arches like eg powerpc (see [1] below) that
> > does have ioremap_nocache),
>
> PowerPC does have ioremap_nocache() though:
>
> /**
> * ioremap - map bus memory into CPU space
> ...
> * * ioremap_nocache is identical to ioremap
> extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
> #define ioremap_nocache(addr, size) ioremap((addr), (size))
>
> and this include file is included very early on in linux/io.h. I don't
> see anything that conditionalises it on anything except __KERNEL__. So,
> the report from 0-day really doesn't make any sense to me.
>
> Do we know how we're ending up in linux/io.h line 169 without having
> picked up the ioremap_nocache() definition provided by PowerPC's
> asm/io.h ?
I will debug it further but I *think* it is because:
eg arch/powerpc/oprofile/op_model_cell.c includes <asm/io.h>
and <asm/io.h> includes <linux/io.h> before ioremap_nocache is defined
> > not sure it is going to be very nice to have
> > an include in the middle of asm*/io.h include files (and I have to patch
> > all arches which I can do).
>
> You mean like we already have to do with this asm-generic/io.h thing in
> the ARM io.h header file, because we need to define all the accessors
> first, to prevent the asm-generic/io.h thing defining them for us?
> Given how asm-generic has headed in this regard, having include files
> at all sorts of strange locations within the architecture asm/*.h
> header files has become quite normal, unfortunately.
Yes we won't make it any nicer that's for certain, my worry is that
it would end up being even harder to read.
> > (2) I add:
> >
> > #ifndef ioremap_nopost
> > static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> > {
> > return NULL; <-(making it return ioremap_nocache() does not
> > work, see [1] for the reason)
> > }
> > #endif
> >
> > in linux/io.h
>
> ... which breaks the kernel if ioremap_nopost is missing from the arch
> header, and one of the drivers that you're modifying to use this new
> ioremap variant happens to be built and used on such an architecture.
Yes agree.
> > (3) ioremap_nopost follows Luis' ioremap_uc approach
>
> Same problem as (2), as I understand correctly.
Agreed. We have to find the lesser evil, that's it.
Thanks !
Lorenzo
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 17:09 ` Lorenzo Pieralisi
@ 2017-04-06 17:19 ` Russell King - ARM Linux
0 siblings, 0 replies; 48+ messages in thread
From: Russell King - ARM Linux @ 2017-04-06 17:19 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 06:09:51PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Apr 06, 2017 at 05:40:16PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> > > Ok, so:
> > >
> > > (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
> > > contain something like
> > >
> > > static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> > > {
> > > return ioremap_nocache(offset, size);
> > > }
> > >
> > > Funny bit is that it has to be included by asm*/io.h files _after_
> > > ioremap_nocache has been #defined (that's the reason my approach was
> > > failing miserably even on arches like eg powerpc (see [1] below) that
> > > does have ioremap_nocache),
> >
> > PowerPC does have ioremap_nocache() though:
> >
> > /**
> > * ioremap - map bus memory into CPU space
> > ...
> > * * ioremap_nocache is identical to ioremap
> > extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
> > #define ioremap_nocache(addr, size) ioremap((addr), (size))
> >
> > and this include file is included very early on in linux/io.h. I don't
> > see anything that conditionalises it on anything except __KERNEL__. So,
> > the report from 0-day really doesn't make any sense to me.
> >
> > Do we know how we're ending up in linux/io.h line 169 without having
> > picked up the ioremap_nocache() definition provided by PowerPC's
> > asm/io.h ?
>
> I will debug it further but I *think* it is because:
>
> eg arch/powerpc/oprofile/op_model_cell.c includes <asm/io.h>
>
> and <asm/io.h> includes <linux/io.h> before ioremap_nocache is defined
Oh, that's just very wrong. asm/foo.h should never include linux/foo.h
especially when linux/foo.h already includes asm/foo.h. I think we
need PowerPC folk to fix this.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 10:53 ` Luis R. Rodriguez
2017-04-06 11:38 ` Lorenzo Pieralisi
@ 2017-04-06 12:11 ` Russell King - ARM Linux
2017-04-06 12:25 ` Luis R. Rodriguez
1 sibling, 1 reply; 48+ messages in thread
From: Russell King - ARM Linux @ 2017-04-06 12:11 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > Indeed, the static inline ioremap_nocache() fallback does not work
> > on all arches (whether I add the fallback in linux/io.h or
> > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > reported above.
>
> Its also not *safe* to assume on behalf of all architectures a new ioremap
> call is both a good idea and proper.
You may be right in general, but not in this case.
Currently, many drivers use plain ioremap() to map this resource. We
are replacing that existing call - which is known to work in the majority
of cases - with a new call to cater for different semantics required by
an architecture.
Doing a replace of these ioremap() calls with ioremap_nopost() in this
situation, and then having ioremap_nopost() fail is a recipe for causing
lots and lots of regressions.
The only sane approach is to have ioremap_post() default to modelling the
_existing_ behaviour everywhere that it is used.
Requiring it to fail until architecture folk trip over the failure is
totally insane, and I very strongly disagree with you on this.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface
2017-04-06 12:11 ` Russell King - ARM Linux
@ 2017-04-06 12:25 ` Luis R. Rodriguez
0 siblings, 0 replies; 48+ messages in thread
From: Luis R. Rodriguez @ 2017-04-06 12:25 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 06, 2017 at 01:11:57PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 06, 2017 at 12:53:12PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 06, 2017 at 11:26:36AM +0100, Lorenzo Pieralisi wrote:
> > > Indeed, the static inline ioremap_nocache() fallback does not work
> > > on all arches (whether I add the fallback in linux/io.h or
> > > asm-generic/io.h is irrelevant), I bump into issues such as the one
> > > reported above.
> >
> > Its also not *safe* to assume on behalf of all architectures a new ioremap
> > call is both a good idea and proper.
>
> You may be right in general, but not in this case.
>
> Currently, many drivers use plain ioremap() to map this resource. We
> are replacing that existing call - which is known to work in the majority
> of cases - with a new call to cater for different semantics required by
> an architecture.
>
> Doing a replace of these ioremap() calls with ioremap_nopost() in this
> situation, and then having ioremap_nopost() fail is a recipe for causing
> lots and lots of regressions.
>
> The only sane approach is to have ioremap_post() default to modelling the
> _existing_ behaviour everywhere that it is used.
>
> Requiring it to fail until architecture folk trip over the failure is
> totally insane, and I very strongly disagree with you on this.
Ah yes, what if with this modulo rule of thumb:
The litmus test then is if an existing set of calls are changed to
use a new ioremap then all archs that support those drivers where the new
call is being added must be modified to also have a correct corresponding
API call ?
This is more work on the new person introducing the new API, and should require
review still on arch maintainers but it seems like a fair compromise.
Then if an API is *new* though then things can move forward without requiring
all archs to add the respective call.
Luis
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 03/22] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 01/22] PCI: remove __weak tag from pci_remap_iospace() Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 04/22] PCI: fix pci_remap_iospace() " Lorenzo Pieralisi
` (18 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
According to the PCI local bus specifications (Revision 3.0, 3.2.5),
I/O Address space transactions are non-posted. On architectures where
I/O space is implemented through a chunk of memory mapped space mapped
to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the
region backing I/O Address Space transactions determines the I/O
transactions attributes (before the transactions actually reaches the
PCI bus where it is handled according to the PCI specifications).
The kernel lacks a pgprot_* attribute to map memory with type
generating non-posted writes transactions, which therefore needs to
be added.
Add a pgprot_nonposted mapping prot to create a memory mapping for
memory areas requiring non-posted write transactions; make it default to
pgprot_noncached (which should provide a sane default behaviour) but
still allowing architectures on which pgprot_noncached results in posted
write transactions to override the prot with an arch specific
implementation that guarantees non-posted writes transactions.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
include/asm-generic/pgtable.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 1fad160..2070172 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -399,6 +399,10 @@ static inline int pud_same(pud_t pud_a, pud_t pud_b)
#define pgprot_device pgprot_noncached
#endif
+#ifndef pgprot_nonposted
+#define pgprot_nonposted pgprot_noncached
+#endif
+
#ifndef pgprot_modify
#define pgprot_modify pgprot_modify
static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 04/22] PCI: fix pci_remap_iospace() remap attribute
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (2 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 03/22] asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 05/22] ARM64: implement ioremap_nopost() interface Lorenzo Pieralisi
` (17 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
According to the PCI local bus specifications (Revision 3.0, 3.2.5),
I/O Address space transactions are non-posted. On architectures where
I/O space is implemented through a chunk of memory mapped space mapped
to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the
region backing I/O Address Space transactions determines the I/O
transactions attributes (before the transactions actually reaches the
PCI bus where it is handled according to the PCI specifications).
Current pci_remap_iospace() interface, that is used to map the PCI I/O
Address Space into virtual address space, use pgprot_device() as memory
attribute for the virtual address mapping, that in some architectures
(ie ARM64) provides non-cacheable but write bufferable mappings (ie
posted writes), which clash with the non-posted write behaviour for I/O
Address Space mandated by the PCI specifications.
Update the prot ioremap_page_range() parameter in pci_remap_iospace()
to pgprot_nonposted to ensure that the virtual mapping backing
I/O Address Space guarantee non-posted write transactions issued
when addressing I/O Address Space through the MMIO mapping.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
drivers/pci/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bd98674..9e084c0 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
return -EINVAL;
return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
- pgprot_device(PAGE_KERNEL));
+ pgprot_nonposted(PAGE_KERNEL));
#else
/* this architecture does not have memory mapped I/O space,
so this function should never be called */
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 05/22] ARM64: implement ioremap_nopost() interface
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (3 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 04/22] PCI: fix pci_remap_iospace() " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-30 16:19 ` Will Deacon
2017-03-27 9:49 ` [PATCH v2 06/22] ARM: " Lorenzo Pieralisi
` (16 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
and Posting") defines rules for PCI configuration space transactions
ordering and posting, that state that configuration writes
are non-posted transactions.
This rule is reinforced by the ARM v8 architecture reference manual
(issue A.k, Early Write Acknowledgment) that explicitly recommends
that No Early Write Acknowledgment attribute should be used to map
PCI configuration (write) transactions.
Current ioremap interface on ARM64 implements mapping functions
where the Early Write Acknowledgment hint is enabled, so they
cannot be used to map PCI configuration space in a PCI specs
compliant way.
Implement an ARM64 specific ioremap_nopost() interface
that allows to map PCI config region with nGnRnE attributes, providing
a remap function that complies with PCI specifications and the ARMv8
architecture reference manual recommendations.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/io.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 0c00c87..1a703e5 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -173,6 +173,18 @@ extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
#define iounmap __iounmap
/*
+ * ioremap implementation providing non-posted writes (ie v8 no Early
+ * Write Acknowledgment) semantics.
+ *
+ * PCI specifications disallows posted write configuration transactions.
+ * Add an arch specific ioremap_post definition that is implemented
+ * through nGnRnE device memory attribute as recommended by the ARM v8
+ * Architecture reference manual Issue A.k B2.8.2 "Device memory" so
+ * that it can be used to map PCI config space memory areas.
+ */
+#define ioremap_nopost(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRnE))
+
+/*
* io{read,write}{16,32,64}be() macros
*/
#define ioread16be(p) ({ __u16 __v = be16_to_cpu((__force __be16)__raw_readw(p)); __iormb(); __v; })
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 05/22] ARM64: implement ioremap_nopost() interface
2017-03-27 9:49 ` [PATCH v2 05/22] ARM64: implement ioremap_nopost() interface Lorenzo Pieralisi
@ 2017-03-30 16:19 ` Will Deacon
0 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2017-03-30 16:19 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 10:49:33AM +0100, Lorenzo Pieralisi wrote:
> The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
> and Posting") defines rules for PCI configuration space transactions
> ordering and posting, that state that configuration writes
> are non-posted transactions.
>
> This rule is reinforced by the ARM v8 architecture reference manual
> (issue A.k, Early Write Acknowledgment) that explicitly recommends
> that No Early Write Acknowledgment attribute should be used to map
> PCI configuration (write) transactions.
>
> Current ioremap interface on ARM64 implements mapping functions
> where the Early Write Acknowledgment hint is enabled, so they
> cannot be used to map PCI configuration space in a PCI specs
> compliant way.
>
> Implement an ARM64 specific ioremap_nopost() interface
> that allows to map PCI config region with nGnRnE attributes, providing
> a remap function that complies with PCI specifications and the ARMv8
> architecture reference manual recommendations.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/io.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
Acked-by: Will Deacon <will.deacon@arm.com>
Let me know if you need this taken via the arm64 tree.
Will
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 06/22] ARM: implement ioremap_nopost() interface
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (4 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 05/22] ARM64: implement ioremap_nopost() interface Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-31 11:08 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region Lorenzo Pieralisi
` (15 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
and Posting") define rules for PCI configuration space transactions
ordering and posting, that state that configuration writes have to
be non-posted transactions.
Current ioremap interface on ARM provides mapping functions that
provide "bufferable" writes transactions (ie ioremap uses MT_DEVICE
memory type) aka posted writes, so PCI host controller drivers have
no arch interface to remap PCI configuration space with memory
attributes that comply with the PCI specifications for configuration
space.
Implement an ARM specific ioremap_nopost() interface that allows to
map PCI config memory regions with MT_UNCACHED memory type (ie strongly
ordered - non-posted writes), providing a remap function that complies
with PCI specifications for config space transactions.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <linux@armlinux.org.uk>
---
arch/arm/include/asm/io.h | 10 ++++++++++
arch/arm/mm/ioremap.c | 7 +++++++
arch/arm/mm/nommu.c | 9 +++++++++
3 files changed, 26 insertions(+)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 42871fb..49913d1 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -352,6 +352,7 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
* mapping has specific properties.
*
* Function Memory type Cacheability Cache hint
+ * ioremap_nopost() SO n/a n/a
* ioremap() Device n/a n/a
* ioremap_nocache() Device n/a n/a
* ioremap_cache() Normal Writeback Read allocate
@@ -372,6 +373,12 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
* compiler may generate unaligned accesses - eg, via inlining its own
* memcpy.
*
+ * ioremap_nopost() maps memory as strongly ordered, to be used for
+ * specific mappings (eg PCI config space) that require non-posted
+ * write transactions. Strongly ordered transactions are ordered wrt
+ * device mappings, which means that ioremap_nopost() is the same
+ * as ioremap() except for non-posted writes behaviour.
+ *
* All normal memory mappings have the following properties:
* - reads can be repeated with no side effects
* - repeated reads return the last value written
@@ -407,6 +414,9 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
#define ioremap_wc ioremap_wc
#define ioremap_wt ioremap_wc
+void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size);
+#define ioremap_nopost ioremap_nopost
+
void iounmap(volatile void __iomem *iomem_cookie);
#define iounmap iounmap
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ff0eed2..4ffaf16 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -463,6 +463,13 @@ void iounmap(volatile void __iomem *cookie)
}
EXPORT_SYMBOL(iounmap);
+void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
+{
+ return arch_ioremap_caller(res_cookie, size, MT_UNCACHED,
+ __builtin_return_address(0));
+}
+EXPORT_SYMBOL_GPL(ioremap_nopost);
+
#ifdef CONFIG_PCI
static int pci_ioremap_mem_type = MT_DEVICE;
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 3b5c7aa..dfd736a 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -21,6 +21,8 @@
#include <asm/mpu.h>
#include <asm/procinfo.h>
+#include <asm/mach/map.h>
+
#include "mm.h"
unsigned long vectors_base;
@@ -433,6 +435,13 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size)
}
EXPORT_SYMBOL(ioremap_wc);
+void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
+{
+ return __arm_ioremap_caller(res_cookie, size, MT_UNCACHED,
+ __builtin_return_address(0));
+}
+EXPORT_SYMBOL(ioremap_nopost);
+
void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
{
return (void *)phys_addr;
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 06/22] ARM: implement ioremap_nopost() interface
2017-03-27 9:49 ` [PATCH v2 06/22] ARM: " Lorenzo Pieralisi
@ 2017-03-31 11:08 ` Lorenzo Pieralisi
2017-04-05 10:21 ` Lorenzo Pieralisi
0 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-31 11:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi Russell,
On Mon, Mar 27, 2017 at 10:49:34AM +0100, Lorenzo Pieralisi wrote:
> The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
> and Posting") define rules for PCI configuration space transactions
> ordering and posting, that state that configuration writes have to
> be non-posted transactions.
>
> Current ioremap interface on ARM provides mapping functions that
> provide "bufferable" writes transactions (ie ioremap uses MT_DEVICE
> memory type) aka posted writes, so PCI host controller drivers have
> no arch interface to remap PCI configuration space with memory
> attributes that comply with the PCI specifications for configuration
> space.
>
> Implement an ARM specific ioremap_nopost() interface that allows to
> map PCI config memory regions with MT_UNCACHED memory type (ie strongly
> ordered - non-posted writes), providing a remap function that complies
> with PCI specifications for config space transactions.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Russell King <linux@armlinux.org.uk>
> ---
> arch/arm/include/asm/io.h | 10 ++++++++++
> arch/arm/mm/ioremap.c | 7 +++++++
> arch/arm/mm/nommu.c | 9 +++++++++
> 3 files changed, 26 insertions(+)
I have not added your ACK to this patch since I slightly tweaked
it to adapt it to ioremap_nopost() interface instead of a PCI
specific one. Furthermore, I mechanically added a ioremap_nopost()
version for nommu too in the process, would be good to have a look
if I did that properly please.
Can I add your ACK on this patch ? If you spot any issues please
do let me know.
Thank you !
Lorenzo
> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> index 42871fb..49913d1 100644
> --- a/arch/arm/include/asm/io.h
> +++ b/arch/arm/include/asm/io.h
> @@ -352,6 +352,7 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
> * mapping has specific properties.
> *
> * Function Memory type Cacheability Cache hint
> + * ioremap_nopost() SO n/a n/a
> * ioremap() Device n/a n/a
> * ioremap_nocache() Device n/a n/a
> * ioremap_cache() Normal Writeback Read allocate
> @@ -372,6 +373,12 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
> * compiler may generate unaligned accesses - eg, via inlining its own
> * memcpy.
> *
> + * ioremap_nopost() maps memory as strongly ordered, to be used for
> + * specific mappings (eg PCI config space) that require non-posted
> + * write transactions. Strongly ordered transactions are ordered wrt
> + * device mappings, which means that ioremap_nopost() is the same
> + * as ioremap() except for non-posted writes behaviour.
> + *
> * All normal memory mappings have the following properties:
> * - reads can be repeated with no side effects
> * - repeated reads return the last value written
> @@ -407,6 +414,9 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
> #define ioremap_wc ioremap_wc
> #define ioremap_wt ioremap_wc
>
> +void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size);
> +#define ioremap_nopost ioremap_nopost
> +
> void iounmap(volatile void __iomem *iomem_cookie);
> #define iounmap iounmap
>
> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index ff0eed2..4ffaf16 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -463,6 +463,13 @@ void iounmap(volatile void __iomem *cookie)
> }
> EXPORT_SYMBOL(iounmap);
>
> +void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
> +{
> + return arch_ioremap_caller(res_cookie, size, MT_UNCACHED,
> + __builtin_return_address(0));
> +}
> +EXPORT_SYMBOL_GPL(ioremap_nopost);
> +
> #ifdef CONFIG_PCI
> static int pci_ioremap_mem_type = MT_DEVICE;
>
> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> index 3b5c7aa..dfd736a 100644
> --- a/arch/arm/mm/nommu.c
> +++ b/arch/arm/mm/nommu.c
> @@ -21,6 +21,8 @@
> #include <asm/mpu.h>
> #include <asm/procinfo.h>
>
> +#include <asm/mach/map.h>
> +
> #include "mm.h"
>
> unsigned long vectors_base;
> @@ -433,6 +435,13 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size)
> }
> EXPORT_SYMBOL(ioremap_wc);
>
> +void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
> +{
> + return __arm_ioremap_caller(res_cookie, size, MT_UNCACHED,
> + __builtin_return_address(0));
> +}
> +EXPORT_SYMBOL(ioremap_nopost);
> +
> void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
> {
> return (void *)phys_addr;
> --
> 2.10.0
>
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 06/22] ARM: implement ioremap_nopost() interface
2017-03-31 11:08 ` Lorenzo Pieralisi
@ 2017-04-05 10:21 ` Lorenzo Pieralisi
0 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-04-05 10:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 31, 2017 at 12:08:13PM +0100, Lorenzo Pieralisi wrote:
> Hi Russell,
>
> On Mon, Mar 27, 2017 at 10:49:34AM +0100, Lorenzo Pieralisi wrote:
> > The PCI bus specifications (rev 3.0, 3.2.5 "Transaction Ordering
> > and Posting") define rules for PCI configuration space transactions
> > ordering and posting, that state that configuration writes have to
> > be non-posted transactions.
> >
> > Current ioremap interface on ARM provides mapping functions that
> > provide "bufferable" writes transactions (ie ioremap uses MT_DEVICE
> > memory type) aka posted writes, so PCI host controller drivers have
> > no arch interface to remap PCI configuration space with memory
> > attributes that comply with the PCI specifications for configuration
> > space.
> >
> > Implement an ARM specific ioremap_nopost() interface that allows to
> > map PCI config memory regions with MT_UNCACHED memory type (ie strongly
> > ordered - non-posted writes), providing a remap function that complies
> > with PCI specifications for config space transactions.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Russell King <linux@armlinux.org.uk>
> > ---
> > arch/arm/include/asm/io.h | 10 ++++++++++
> > arch/arm/mm/ioremap.c | 7 +++++++
> > arch/arm/mm/nommu.c | 9 +++++++++
> > 3 files changed, 26 insertions(+)
>
> I have not added your ACK to this patch since I slightly tweaked
> it to adapt it to ioremap_nopost() interface instead of a PCI
> specific one. Furthermore, I mechanically added a ioremap_nopost()
> version for nommu too in the process, would be good to have a look
> if I did that properly please.
>
> Can I add your ACK on this patch ? If you spot any issues please
> do let me know.
Hi Russell,
please let me know if your ACK on this patch stands as I have to respin
it shortly for hopefully final round of review.
Thanks !
Lorenzo
> Thank you !
> Lorenzo
>
> > diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> > index 42871fb..49913d1 100644
> > --- a/arch/arm/include/asm/io.h
> > +++ b/arch/arm/include/asm/io.h
> > @@ -352,6 +352,7 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
> > * mapping has specific properties.
> > *
> > * Function Memory type Cacheability Cache hint
> > + * ioremap_nopost() SO n/a n/a
> > * ioremap() Device n/a n/a
> > * ioremap_nocache() Device n/a n/a
> > * ioremap_cache() Normal Writeback Read allocate
> > @@ -372,6 +373,12 @@ static inline void memcpy_toio(volatile void __iomem *to, const void *from,
> > * compiler may generate unaligned accesses - eg, via inlining its own
> > * memcpy.
> > *
> > + * ioremap_nopost() maps memory as strongly ordered, to be used for
> > + * specific mappings (eg PCI config space) that require non-posted
> > + * write transactions. Strongly ordered transactions are ordered wrt
> > + * device mappings, which means that ioremap_nopost() is the same
> > + * as ioremap() except for non-posted writes behaviour.
> > + *
> > * All normal memory mappings have the following properties:
> > * - reads can be repeated with no side effects
> > * - repeated reads return the last value written
> > @@ -407,6 +414,9 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
> > #define ioremap_wc ioremap_wc
> > #define ioremap_wt ioremap_wc
> >
> > +void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size);
> > +#define ioremap_nopost ioremap_nopost
> > +
> > void iounmap(volatile void __iomem *iomem_cookie);
> > #define iounmap iounmap
> >
> > diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> > index ff0eed2..4ffaf16 100644
> > --- a/arch/arm/mm/ioremap.c
> > +++ b/arch/arm/mm/ioremap.c
> > @@ -463,6 +463,13 @@ void iounmap(volatile void __iomem *cookie)
> > }
> > EXPORT_SYMBOL(iounmap);
> >
> > +void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
> > +{
> > + return arch_ioremap_caller(res_cookie, size, MT_UNCACHED,
> > + __builtin_return_address(0));
> > +}
> > +EXPORT_SYMBOL_GPL(ioremap_nopost);
> > +
> > #ifdef CONFIG_PCI
> > static int pci_ioremap_mem_type = MT_DEVICE;
> >
> > diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> > index 3b5c7aa..dfd736a 100644
> > --- a/arch/arm/mm/nommu.c
> > +++ b/arch/arm/mm/nommu.c
> > @@ -21,6 +21,8 @@
> > #include <asm/mpu.h>
> > #include <asm/procinfo.h>
> >
> > +#include <asm/mach/map.h>
> > +
> > #include "mm.h"
> >
> > unsigned long vectors_base;
> > @@ -433,6 +435,13 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size)
> > }
> > EXPORT_SYMBOL(ioremap_wc);
> >
> > +void __iomem *ioremap_nopost(resource_size_t res_cookie, size_t size)
> > +{
> > + return __arm_ioremap_caller(res_cookie, size, MT_UNCACHED,
> > + __builtin_return_address(0));
> > +}
> > +EXPORT_SYMBOL(ioremap_nopost);
> > +
> > void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
> > {
> > return (void *)phys_addr;
> > --
> > 2.10.0
> >
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (5 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 06/22] ARM: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-30 16:20 ` Will Deacon
2017-03-27 9:49 ` [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface Lorenzo Pieralisi
` (14 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
Current ECAM kernel implementation uses ioremap() to map the ECAM
configuration space memory region; this is not safe in that on some
architectures the ioremap interface provides mappings that allow posted
write transactions. This, as highlighted in the PCIe specifications
(4.0 - Rev0.3, "Ordering Considerations for the Enhanced Configuration
Address Mechanism"), can create ordering issues for software because
posted writes transactions on the CPU host bus are non posted in the
PCI express fabric.
Update the ioremap() interface to use ioremap_nopost() whose
mapping attributes guarantee that non-posted writes transactions
are issued for memory writes within the ECAM memory mapped address
region.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/ecam.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 2fee61b..70d722a 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -84,12 +84,13 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
if (!cfg->winp)
goto err_exit_malloc;
for (i = 0; i < bus_range; i++) {
- cfg->winp[i] = ioremap(cfgres->start + i * bsz, bsz);
+ cfg->winp[i] = ioremap_nopost(cfgres->start + i * bsz,
+ bsz);
if (!cfg->winp[i])
goto err_exit_iomap;
}
} else {
- cfg->win = ioremap(cfgres->start, bus_range * bsz);
+ cfg->win = ioremap_nopost(cfgres->start, bus_range * bsz);
if (!cfg->win)
goto err_exit_iomap;
}
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region
2017-03-27 9:49 ` [PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region Lorenzo Pieralisi
@ 2017-03-30 16:20 ` Will Deacon
0 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2017-03-30 16:20 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 10:49:35AM +0100, Lorenzo Pieralisi wrote:
> Current ECAM kernel implementation uses ioremap() to map the ECAM
> configuration space memory region; this is not safe in that on some
> architectures the ioremap interface provides mappings that allow posted
> write transactions. This, as highlighted in the PCIe specifications
> (4.0 - Rev0.3, "Ordering Considerations for the Enhanced Configuration
> Address Mechanism"), can create ordering issues for software because
> posted writes transactions on the CPU host bus are non posted in the
> PCI express fabric.
>
> Update the ioremap() interface to use ioremap_nopost() whose
> mapping attributes guarantee that non-posted writes transactions
> are issued for memory writes within the ECAM memory mapped address
> region.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/ecam.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (6 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 07/22] PCI: ECAM: use ioremap_nopost() to map config region Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-28 1:41 ` Bjorn Helgaas
2017-03-27 9:49 ` [PATCH v2 09/22] PCI: xilinx: update PCI config space remap function Lorenzo Pieralisi
` (13 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
The introduction of the ioremap_nopost() interface allows
kernel drivers to map memory through a dedicated kernel
interface providing non-posted writes semantics.
Introduce two new functions in the Devres kernel layer and Devres
documentation:
- devm_ioremap_nopost()
- devm_ioremap_nopost_resource()
so that drivers can make use of devm_* interface to map memory
regions requiring non-posted writes memory attributes.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tejun Heo <tj@kernel.org>
---
Documentation/driver-model/devres.txt | 3 ++
include/linux/device.h | 2 +
include/linux/io.h | 2 +
lib/devres.c | 78 +++++++++++++++++++++++++++++++++++
4 files changed, 85 insertions(+)
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index bf34d5b..9991a66 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -294,7 +294,10 @@ IOMAP
devm_ioremap()
devm_ioremap_nocache()
devm_ioremap_wc()
+ devm_ioremap_nopost()
devm_ioremap_resource() : checks resource, requests memory region, ioremaps
+ devm_ioremap_nopost_resource() : do devm_ioremap_resource() with nopost
+ memory attributes
devm_iounmap()
pcim_iomap()
pcim_iomap_regions() : do request_region() and iomap() on multiple BARs
diff --git a/include/linux/device.h b/include/linux/device.h
index 9ef518a..1dce865 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -689,6 +689,8 @@ extern unsigned long devm_get_free_pages(struct device *dev,
extern void devm_free_pages(struct device *dev, unsigned long addr);
void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
+void __iomem *devm_ioremap_nopost_resource(struct device *dev,
+ struct resource *res);
/* allows to add/remove a custom action to devres stack */
int devm_add_action(struct device *dev, void (*action)(void *), void *data);
diff --git a/include/linux/io.h b/include/linux/io.h
index 82ef36e..e34d799 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -79,6 +79,8 @@ void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
resource_size_t size);
void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
resource_size_t size);
+void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
+ resource_size_t size);
void devm_iounmap(struct device *dev, void __iomem *addr);
int check_signature(const volatile void __iomem *io_addr,
const unsigned char *signature, int length);
diff --git a/lib/devres.c b/lib/devres.c
index cb1464c..4e05660 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -100,6 +100,34 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
EXPORT_SYMBOL(devm_ioremap_wc);
/**
+ * devm_ioremap_nopost - Managed ioremap_nopost()
+ * @dev: Generic device to remap IO address for
+ * @offset: BUS offset to map
+ * @size: Size of map
+ *
+ * Managed ioremap_nopost(). Map is automatically unmapped on driver detach.
+ */
+void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
+ resource_size_t size)
+{
+ void __iomem **ptr, *addr;
+
+ ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ addr = ioremap_nopost(offset, size);
+ if (addr) {
+ *ptr = addr;
+ devres_add(dev, ptr);
+ } else
+ devres_free(ptr);
+
+ return addr;
+}
+EXPORT_SYMBOL(devm_ioremap_nopost);
+
+/**
* devm_iounmap - Managed iounmap()
* @dev: Generic device to unmap for
* @addr: Address to unmap
@@ -163,6 +191,56 @@ void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
}
EXPORT_SYMBOL(devm_ioremap_resource);
+/**
+ * devm_ioremap_nopost_resource() - devm_ioremap_resource() nopost version
+ * @dev: generic device to handle the resource for
+ * @res: resource to be handled
+ *
+ * Checks that a resource is a valid memory region, requests the memory
+ * region and ioremaps it with ioremap_nopost() interface.
+ * All operations are managed and will be undone on driver detach.
+ *
+ * Returns a pointer to the remapped memory or an ERR_PTR() encoded error code
+ * on failure. Usage example:
+ *
+ * res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ * base = devm_ioremap_nopost_resource(&pdev->dev, res);
+ * if (IS_ERR(base))
+ * return PTR_ERR(base);
+ */
+void __iomem *devm_ioremap_nopost_resource(struct device *dev,
+ struct resource *res)
+{
+ resource_size_t size;
+ const char *name;
+ void __iomem *dest_ptr;
+
+ BUG_ON(!dev);
+
+ if (!res || resource_type(res) != IORESOURCE_MEM) {
+ dev_err(dev, "invalid resource\n");
+ return IOMEM_ERR_PTR(-EINVAL);
+ }
+
+ size = resource_size(res);
+ name = res->name ?: dev_name(dev);
+
+ if (!devm_request_mem_region(dev, res->start, size, name)) {
+ dev_err(dev, "can't request region for resource %pR\n", res);
+ return IOMEM_ERR_PTR(-EBUSY);
+ }
+
+ dest_ptr = devm_ioremap_nopost(dev, res->start, size);
+ if (!dest_ptr) {
+ dev_err(dev, "ioremap failed for resource %pR\n", res);
+ devm_release_mem_region(dev, res->start, size);
+ dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
+ }
+
+ return dest_ptr;
+}
+EXPORT_SYMBOL(devm_ioremap_nopost_resource);
+
#ifdef CONFIG_HAS_IOPORT_MAP
/*
* Generic iomap devres
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface
2017-03-27 9:49 ` [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface Lorenzo Pieralisi
@ 2017-03-28 1:41 ` Bjorn Helgaas
2017-03-28 14:50 ` Lorenzo Pieralisi
0 siblings, 1 reply; 48+ messages in thread
From: Bjorn Helgaas @ 2017-03-28 1:41 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 10:49:36AM +0100, Lorenzo Pieralisi wrote:
> The introduction of the ioremap_nopost() interface allows
> kernel drivers to map memory through a dedicated kernel
> interface providing non-posted writes semantics.
>
> Introduce two new functions in the Devres kernel layer and Devres
> documentation:
>
> - devm_ioremap_nopost()
> - devm_ioremap_nopost_resource()
>
> so that drivers can make use of devm_* interface to map memory
> regions requiring non-posted writes memory attributes.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
> Documentation/driver-model/devres.txt | 3 ++
> include/linux/device.h | 2 +
> include/linux/io.h | 2 +
> lib/devres.c | 78 +++++++++++++++++++++++++++++++++++
> 4 files changed, 85 insertions(+)
>
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> index bf34d5b..9991a66 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -294,7 +294,10 @@ IOMAP
> devm_ioremap()
> devm_ioremap_nocache()
> devm_ioremap_wc()
> + devm_ioremap_nopost()
> devm_ioremap_resource() : checks resource, requests memory region, ioremaps
> + devm_ioremap_nopost_resource() : do devm_ioremap_resource() with nopost
> + memory attributes
> devm_iounmap()
> pcim_iomap()
> pcim_iomap_regions() : do request_region() and iomap() on multiple BARs
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 9ef518a..1dce865 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -689,6 +689,8 @@ extern unsigned long devm_get_free_pages(struct device *dev,
> extern void devm_free_pages(struct device *dev, unsigned long addr);
>
> void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
> +void __iomem *devm_ioremap_nopost_resource(struct device *dev,
> + struct resource *res);
>
> /* allows to add/remove a custom action to devres stack */
> int devm_add_action(struct device *dev, void (*action)(void *), void *data);
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 82ef36e..e34d799 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -79,6 +79,8 @@ void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> resource_size_t size);
> void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> resource_size_t size);
> +void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
> + resource_size_t size);
> void devm_iounmap(struct device *dev, void __iomem *addr);
> int check_signature(const volatile void __iomem *io_addr,
> const unsigned char *signature, int length);
> diff --git a/lib/devres.c b/lib/devres.c
> index cb1464c..4e05660 100644
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -100,6 +100,34 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> EXPORT_SYMBOL(devm_ioremap_wc);
>
> /**
> + * devm_ioremap_nopost - Managed ioremap_nopost()
> + * @dev: Generic device to remap IO address for
> + * @offset: BUS offset to map
I know this comment was just copied from other devm_ioremap*()
interfaces, but I think it's slightly misleading to call this the "BUS
offset". That suggests this is a bus address, but it is not. Maybe
it should be "resource address" or similar?
I propose a separate patch to update the existing comments, followed
by this patch to add this new one.
> + * @size: Size of map
> + *
> + * Managed ioremap_nopost(). Map is automatically unmapped on driver detach.
> + */
> +void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
> + resource_size_t size)
> +{
> + void __iomem **ptr, *addr;
> +
> + ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return NULL;
> +
> + addr = ioremap_nopost(offset, size);
> + if (addr) {
> + *ptr = addr;
> + devres_add(dev, ptr);
> + } else
> + devres_free(ptr);
> +
> + return addr;
> +}
> +EXPORT_SYMBOL(devm_ioremap_nopost);
> +
> +/**
> * devm_iounmap - Managed iounmap()
> * @dev: Generic device to unmap for
> * @addr: Address to unmap
> @@ -163,6 +191,56 @@ void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
> }
> EXPORT_SYMBOL(devm_ioremap_resource);
>
> +/**
> + * devm_ioremap_nopost_resource() - devm_ioremap_resource() nopost version
> + * @dev: generic device to handle the resource for
> + * @res: resource to be handled
> + *
> + * Checks that a resource is a valid memory region, requests the memory
> + * region and ioremaps it with ioremap_nopost() interface.
> + * All operations are managed and will be undone on driver detach.
> + *
> + * Returns a pointer to the remapped memory or an ERR_PTR() encoded error code
> + * on failure. Usage example:
> + *
> + * res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + * base = devm_ioremap_nopost_resource(&pdev->dev, res);
> + * if (IS_ERR(base))
> + * return PTR_ERR(base);
> + */
> +void __iomem *devm_ioremap_nopost_resource(struct device *dev,
> + struct resource *res)
> +{
> + resource_size_t size;
> + const char *name;
> + void __iomem *dest_ptr;
> +
> + BUG_ON(!dev);
> +
> + if (!res || resource_type(res) != IORESOURCE_MEM) {
> + dev_err(dev, "invalid resource\n");
> + return IOMEM_ERR_PTR(-EINVAL);
> + }
> +
> + size = resource_size(res);
> + name = res->name ?: dev_name(dev);
> +
> + if (!devm_request_mem_region(dev, res->start, size, name)) {
> + dev_err(dev, "can't request region for resource %pR\n", res);
> + return IOMEM_ERR_PTR(-EBUSY);
> + }
> +
> + dest_ptr = devm_ioremap_nopost(dev, res->start, size);
> + if (!dest_ptr) {
> + dev_err(dev, "ioremap failed for resource %pR\n", res);
> + devm_release_mem_region(dev, res->start, size);
> + dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
> + }
> +
> + return dest_ptr;
> +}
> +EXPORT_SYMBOL(devm_ioremap_nopost_resource);
> +
> #ifdef CONFIG_HAS_IOPORT_MAP
> /*
> * Generic iomap devres
> --
> 2.10.0
>
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface
2017-03-28 1:41 ` Bjorn Helgaas
@ 2017-03-28 14:50 ` Lorenzo Pieralisi
2017-03-28 15:55 ` Tejun Heo
0 siblings, 1 reply; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-28 14:50 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 08:41:29PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 27, 2017 at 10:49:36AM +0100, Lorenzo Pieralisi wrote:
> > The introduction of the ioremap_nopost() interface allows
> > kernel drivers to map memory through a dedicated kernel
> > interface providing non-posted writes semantics.
> >
> > Introduce two new functions in the Devres kernel layer and Devres
> > documentation:
> >
> > - devm_ioremap_nopost()
> > - devm_ioremap_nopost_resource()
> >
> > so that drivers can make use of devm_* interface to map memory
> > regions requiring non-posted writes memory attributes.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > ---
> > Documentation/driver-model/devres.txt | 3 ++
> > include/linux/device.h | 2 +
> > include/linux/io.h | 2 +
> > lib/devres.c | 78 +++++++++++++++++++++++++++++++++++
> > 4 files changed, 85 insertions(+)
> >
> > diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> > index bf34d5b..9991a66 100644
> > --- a/Documentation/driver-model/devres.txt
> > +++ b/Documentation/driver-model/devres.txt
> > @@ -294,7 +294,10 @@ IOMAP
> > devm_ioremap()
> > devm_ioremap_nocache()
> > devm_ioremap_wc()
> > + devm_ioremap_nopost()
> > devm_ioremap_resource() : checks resource, requests memory region, ioremaps
> > + devm_ioremap_nopost_resource() : do devm_ioremap_resource() with nopost
> > + memory attributes
> > devm_iounmap()
> > pcim_iomap()
> > pcim_iomap_regions() : do request_region() and iomap() on multiple BARs
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 9ef518a..1dce865 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -689,6 +689,8 @@ extern unsigned long devm_get_free_pages(struct device *dev,
> > extern void devm_free_pages(struct device *dev, unsigned long addr);
> >
> > void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
> > +void __iomem *devm_ioremap_nopost_resource(struct device *dev,
> > + struct resource *res);
> >
> > /* allows to add/remove a custom action to devres stack */
> > int devm_add_action(struct device *dev, void (*action)(void *), void *data);
> > diff --git a/include/linux/io.h b/include/linux/io.h
> > index 82ef36e..e34d799 100644
> > --- a/include/linux/io.h
> > +++ b/include/linux/io.h
> > @@ -79,6 +79,8 @@ void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> > resource_size_t size);
> > void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> > resource_size_t size);
> > +void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
> > + resource_size_t size);
> > void devm_iounmap(struct device *dev, void __iomem *addr);
> > int check_signature(const volatile void __iomem *io_addr,
> > const unsigned char *signature, int length);
> > diff --git a/lib/devres.c b/lib/devres.c
> > index cb1464c..4e05660 100644
> > --- a/lib/devres.c
> > +++ b/lib/devres.c
> > @@ -100,6 +100,34 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> > EXPORT_SYMBOL(devm_ioremap_wc);
> >
> > /**
> > + * devm_ioremap_nopost - Managed ioremap_nopost()
> > + * @dev: Generic device to remap IO address for
> > + * @offset: BUS offset to map
>
> I know this comment was just copied from other devm_ioremap*()
> interfaces, but I think it's slightly misleading to call this the "BUS
> offset". That suggests this is a bus address, but it is not. Maybe
> it should be "resource address" or similar?
>
> I propose a separate patch to update the existing comments, followed
> by this patch to add this new one.
I agree with you, maybe Tejun has more context on why that parameter
is called BUS offset ? Anyway, I can easily do what you suggest above,
just asking Tejun if there is something I am missing.
Tejun ?
Thanks !
Lorenzo
>
> > + * @size: Size of map
> > + *
> > + * Managed ioremap_nopost(). Map is automatically unmapped on driver detach.
> > + */
> > +void __iomem *devm_ioremap_nopost(struct device *dev, resource_size_t offset,
> > + resource_size_t size)
> > +{
> > + void __iomem **ptr, *addr;
> > +
> > + ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> > + if (!ptr)
> > + return NULL;
> > +
> > + addr = ioremap_nopost(offset, size);
> > + if (addr) {
> > + *ptr = addr;
> > + devres_add(dev, ptr);
> > + } else
> > + devres_free(ptr);
> > +
> > + return addr;
> > +}
> > +EXPORT_SYMBOL(devm_ioremap_nopost);
> > +
> > +/**
> > * devm_iounmap - Managed iounmap()
> > * @dev: Generic device to unmap for
> > * @addr: Address to unmap
> > @@ -163,6 +191,56 @@ void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
> > }
> > EXPORT_SYMBOL(devm_ioremap_resource);
> >
> > +/**
> > + * devm_ioremap_nopost_resource() - devm_ioremap_resource() nopost version
> > + * @dev: generic device to handle the resource for
> > + * @res: resource to be handled
> > + *
> > + * Checks that a resource is a valid memory region, requests the memory
> > + * region and ioremaps it with ioremap_nopost() interface.
> > + * All operations are managed and will be undone on driver detach.
> > + *
> > + * Returns a pointer to the remapped memory or an ERR_PTR() encoded error code
> > + * on failure. Usage example:
> > + *
> > + * res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + * base = devm_ioremap_nopost_resource(&pdev->dev, res);
> > + * if (IS_ERR(base))
> > + * return PTR_ERR(base);
> > + */
> > +void __iomem *devm_ioremap_nopost_resource(struct device *dev,
> > + struct resource *res)
> > +{
> > + resource_size_t size;
> > + const char *name;
> > + void __iomem *dest_ptr;
> > +
> > + BUG_ON(!dev);
> > +
> > + if (!res || resource_type(res) != IORESOURCE_MEM) {
> > + dev_err(dev, "invalid resource\n");
> > + return IOMEM_ERR_PTR(-EINVAL);
> > + }
> > +
> > + size = resource_size(res);
> > + name = res->name ?: dev_name(dev);
> > +
> > + if (!devm_request_mem_region(dev, res->start, size, name)) {
> > + dev_err(dev, "can't request region for resource %pR\n", res);
> > + return IOMEM_ERR_PTR(-EBUSY);
> > + }
> > +
> > + dest_ptr = devm_ioremap_nopost(dev, res->start, size);
> > + if (!dest_ptr) {
> > + dev_err(dev, "ioremap failed for resource %pR\n", res);
> > + devm_release_mem_region(dev, res->start, size);
> > + dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
> > + }
> > +
> > + return dest_ptr;
> > +}
> > +EXPORT_SYMBOL(devm_ioremap_nopost_resource);
> > +
> > #ifdef CONFIG_HAS_IOPORT_MAP
> > /*
> > * Generic iomap devres
> > --
> > 2.10.0
> >
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface
2017-03-28 14:50 ` Lorenzo Pieralisi
@ 2017-03-28 15:55 ` Tejun Heo
0 siblings, 0 replies; 48+ messages in thread
From: Tejun Heo @ 2017-03-28 15:55 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Tue, Mar 28, 2017 at 03:50:03PM +0100, Lorenzo Pieralisi wrote:
> > I know this comment was just copied from other devm_ioremap*()
> > interfaces, but I think it's slightly misleading to call this the "BUS
> > offset". That suggests this is a bus address, but it is not. Maybe
> > it should be "resource address" or similar?
> >
> > I propose a separate patch to update the existing comments, followed
> > by this patch to add this new one.
>
> I agree with you, maybe Tejun has more context on why that parameter
> is called BUS offset ? Anyway, I can easily do what you suggest above,
> just asking Tejun if there is something I am missing.
I'm pretty sure I copied it from somewhere without thinking much about
it. Please feel free to change.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 09/22] PCI: xilinx: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (7 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 08/22] lib: implement Devres ioremap_nopost() interface Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 10/22] PCI: xilinx-nwl: " Lorenzo Pieralisi
` (12 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
drivers/pci/host/pcie-xilinx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 7f030f5..e96f4a7 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -606,7 +606,7 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
return err;
}
- port->reg_base = devm_ioremap_resource(dev, ®s);
+ port->reg_base = devm_ioremap_nopost_resource(dev, ®s);
if (IS_ERR(port->reg_base))
return PTR_ERR(port->reg_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 10/22] PCI: xilinx-nwl: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (8 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 09/22] PCI: xilinx: update PCI config space remap function Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 11/22] PCI: spear13xx: " Lorenzo Pieralisi
` (11 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
drivers/pci/host/pcie-xilinx-nwl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index 4c3e0ab..cc042b1 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -761,7 +761,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
pcie->phys_pcie_reg_base = res->start;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
- pcie->ecam_base = devm_ioremap_resource(dev, res);
+ pcie->ecam_base = devm_ioremap_nopost_resource(dev, res);
if (IS_ERR(pcie->ecam_base))
return PTR_ERR(pcie->ecam_base);
pcie->phys_ecam_base = res->start;
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 11/22] PCI: spear13xx: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (9 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 10/22] PCI: xilinx-nwl: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 12/22] PCI: rockchip: " Lorenzo Pieralisi
` (10 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generate on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Pratyush Anand <pratyush.anand@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/dwc/pcie-spear13xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/dwc/pcie-spear13xx.c b/drivers/pci/dwc/pcie-spear13xx.c
index eaa4ea8..0fe2b2b 100644
--- a/drivers/pci/dwc/pcie-spear13xx.c
+++ b/drivers/pci/dwc/pcie-spear13xx.c
@@ -273,7 +273,7 @@ static int spear13xx_pcie_probe(struct platform_device *pdev)
}
dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
- pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
+ pci->dbi_base = devm_ioremap_nopost_resource(dev, dbi_base);
if (IS_ERR(pci->dbi_base)) {
dev_err(dev, "couldn't remap dbi base %p\n", dbi_base);
ret = PTR_ERR(pci->dbi_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 12/22] PCI: rockchip: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (10 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 11/22] PCI: spear13xx: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 13/22] PCI: qcom: " Lorenzo Pieralisi
` (9 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Wenrui Li <wenrui.li@rock-chips.com>
Cc: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/pci/host/pcie-rockchip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 26ddd35..a078131 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -822,7 +822,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
regs = platform_get_resource_byname(pdev,
IORESOURCE_MEM,
"axi-base");
- rockchip->reg_base = devm_ioremap_resource(dev, regs);
+ rockchip->reg_base = devm_ioremap_nopost_resource(dev, regs);
if (IS_ERR(rockchip->reg_base))
return PTR_ERR(rockchip->reg_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 13/22] PCI: qcom: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (11 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 12/22] PCI: rockchip: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 14/22] PCI: iproc-platform: " Lorenzo Pieralisi
` (8 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
---
drivers/pci/dwc/pcie-qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
index 67eb7f5..393e379 100644
--- a/drivers/pci/dwc/pcie-qcom.c
+++ b/drivers/pci/dwc/pcie-qcom.c
@@ -700,7 +700,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
return PTR_ERR(pcie->parf);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
- pci->dbi_base = devm_ioremap_resource(dev, res);
+ pci->dbi_base = devm_ioremap_nopost_resource(dev, res);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 14/22] PCI: iproc-platform: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (12 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 13/22] PCI: qcom: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 15/22] PCI: hisi: " Lorenzo Pieralisi
` (7 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Jon Mason <jonmason@broadcom.com>
---
drivers/pci/host/pcie-iproc-platform.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index f4909bb..b13d4a2 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -67,7 +67,8 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
return ret;
}
- pcie->base = devm_ioremap(dev, reg.start, resource_size(®));
+ pcie->base = devm_ioremap_nopost(dev, reg.start,
+ resource_size(®));
if (!pcie->base) {
dev_err(dev, "unable to map controller registers\n");
return -ENOMEM;
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 15/22] PCI: hisi: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (13 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 14/22] PCI: iproc-platform: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 16/22] PCI: designware: " Lorenzo Pieralisi
` (6 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Zhou Wang <wangzhou1@hisilicon.com>
---
drivers/pci/dwc/pcie-hisi.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c
index fd66a31..66a91b2 100644
--- a/drivers/pci/dwc/pcie-hisi.c
+++ b/drivers/pci/dwc/pcie-hisi.c
@@ -99,7 +99,7 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
return -ENOMEM;
}
- reg_base = devm_ioremap(dev, res->start, resource_size(res));
+ reg_base = devm_ioremap_nopost(dev, res->start, resource_size(res));
if (!reg_base)
return -ENOMEM;
@@ -296,10 +296,9 @@ static int hisi_pcie_probe(struct platform_device *pdev)
}
reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
- pci->dbi_base = devm_ioremap_resource(dev, reg);
+ pci->dbi_base = devm_ioremap_nopost_resource(dev, reg);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
-
platform_set_drvdata(pdev, hisi_pcie);
ret = hisi_add_pcie_port(hisi_pcie, pdev);
@@ -360,7 +359,7 @@ static int hisi_pcie_platform_init(struct pci_config_window *cfg)
return -EINVAL;
}
- reg_base = devm_ioremap(dev, res->start, resource_size(res));
+ reg_base = devm_ioremap_nopost(dev, res->start, resource_size(res));
if (!reg_base)
return -ENOMEM;
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 16/22] PCI: designware: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (14 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 15/22] PCI: hisi: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 17/22] PCI: armada8k: " Lorenzo Pieralisi
` (5 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Joao Pinto <Joao.Pinto@synopsys.com>
---
drivers/pci/dwc/pcie-designware-host.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 5ba3349..d2dd228 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -338,8 +338,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
}
if (!pci->dbi_base) {
- pci->dbi_base = devm_ioremap(dev, pp->cfg->start,
- resource_size(pp->cfg));
+ pci->dbi_base = devm_ioremap_nopost(dev, pp->cfg->start,
+ resource_size(pp->cfg));
if (!pci->dbi_base) {
dev_err(dev, "error with ioremap\n");
ret = -ENOMEM;
@@ -350,8 +350,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
pp->mem_base = pp->mem->start;
if (!pp->va_cfg0_base) {
- pp->va_cfg0_base = devm_ioremap(dev, pp->cfg0_base,
- pp->cfg0_size);
+ pp->va_cfg0_base = devm_ioremap_nopost(dev, pp->cfg0_base,
+ pp->cfg0_size);
if (!pp->va_cfg0_base) {
dev_err(dev, "error with ioremap in function\n");
ret = -ENOMEM;
@@ -360,8 +360,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
}
if (!pp->va_cfg1_base) {
- pp->va_cfg1_base = devm_ioremap(dev, pp->cfg1_base,
- pp->cfg1_size);
+ pp->va_cfg1_base = devm_ioremap_nopost(dev, pp->cfg1_base,
+ pp->cfg1_size);
if (!pp->va_cfg1_base) {
dev_err(dev, "error with ioremap\n");
ret = -ENOMEM;
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 17/22] PCI: armada8k: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (15 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 16/22] PCI: designware: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 18/22] PCI: xgene: " Lorenzo Pieralisi
` (4 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/pci/dwc/pcie-armada8k.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/dwc/pcie-armada8k.c b/drivers/pci/dwc/pcie-armada8k.c
index f110e3b..6bc7c7f 100644
--- a/drivers/pci/dwc/pcie-armada8k.c
+++ b/drivers/pci/dwc/pcie-armada8k.c
@@ -230,7 +230,7 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
/* Get the dw-pcie unit configuration/control registers base. */
base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
- pci->dbi_base = devm_ioremap_resource(dev, base);
+ pci->dbi_base = devm_ioremap_nopost_resource(dev, base);
if (IS_ERR(pci->dbi_base)) {
dev_err(dev, "couldn't remap regs base %p\n", base);
ret = PTR_ERR(pci->dbi_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 18/22] PCI: xgene: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (16 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 17/22] PCI: armada8k: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 19/22] PCI: tegra: " Lorenzo Pieralisi
` (3 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tanmay Inamdar <tinamdar@apm.com>
---
drivers/pci/host/pci-xgene.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 1a61087..df8e13c 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -248,7 +248,7 @@ static int xgene_pcie_ecam_init(struct pci_config_window *cfg, u32 ipversion)
dev_err(dev, "can't get CSR resource\n");
return ret;
}
- port->csr_base = devm_ioremap_resource(dev, &csr);
+ port->csr_base = devm_ioremap_nopost_resource(dev, &csr);
if (IS_ERR(port->csr_base))
return PTR_ERR(port->csr_base);
@@ -359,7 +359,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
struct resource *res;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "csr");
- port->csr_base = devm_ioremap_resource(dev, res);
+ port->csr_base = devm_ioremap_nopost_resource(dev, res);
if (IS_ERR(port->csr_base))
return PTR_ERR(port->csr_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 19/22] PCI: tegra: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (17 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 18/22] PCI: xgene: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 20/22] PCI: layerscape: " Lorenzo Pieralisi
` (2 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use correct memory mapping attributes to map config space
regions to enforce configuration space non-posted writes behaviour.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
---
drivers/pci/host/pci-tegra.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index ed8a93f..ec72c3a 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -380,7 +380,7 @@ static struct tegra_pcie_bus *tegra_pcie_bus_alloc(struct tegra_pcie *pcie,
unsigned int busnr)
{
struct device *dev = pcie->dev;
- pgprot_t prot = pgprot_device(PAGE_KERNEL);
+ pgprot_t prot = pgprot_nonposted(PAGE_KERNEL);
phys_addr_t cs = pcie->cs->start;
struct tegra_pcie_bus *bus;
unsigned int i;
@@ -1962,7 +1962,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
rp->pcie = pcie;
rp->np = port;
- rp->base = devm_ioremap_resource(dev, &rp->regs);
+ rp->base = devm_ioremap_nopost_resource(dev, &rp->regs);
if (IS_ERR(rp->base))
return PTR_ERR(rp->base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 20/22] PCI: layerscape: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (18 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 19/22] PCI: tegra: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 21/22] PCI: keystone-dw: " Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 22/22] PCI: versatile: " Lorenzo Pieralisi
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Mingkai Hu <mingkai.hu@freescale.com>
Cc: Minghuan Lian <minghuan.Lian@freescale.com>
Cc: Roy Zang <tie-fei.zang@freescale.com>
---
drivers/pci/dwc/pci-layerscape.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/dwc/pci-layerscape.c b/drivers/pci/dwc/pci-layerscape.c
index c32e392..a1bd86a 100644
--- a/drivers/pci/dwc/pci-layerscape.c
+++ b/drivers/pci/dwc/pci-layerscape.c
@@ -283,7 +283,7 @@ static int __init ls_pcie_probe(struct platform_device *pdev)
pcie->pci = pci;
dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
- pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
+ pci->dbi_base = devm_ioremap_nopost_resource(dev, dbi_base);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 21/22] PCI: keystone-dw: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (19 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 20/22] PCI: layerscape: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
2017-03-27 9:49 ` [PATCH v2 22/22] PCI: versatile: " Lorenzo Pieralisi
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>
---
drivers/pci/dwc/pci-keystone-dw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
index 6b396f6..cba65ca 100644
--- a/drivers/pci/dwc/pci-keystone-dw.c
+++ b/drivers/pci/dwc/pci-keystone-dw.c
@@ -543,7 +543,7 @@ int __init ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
/* Index 0 is the config reg. space address */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pci->dbi_base = devm_ioremap_resource(dev, res);
+ pci->dbi_base = devm_ioremap_nopost_resource(dev, res);
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v2 22/22] PCI: versatile: update PCI config space remap function
2017-03-27 9:49 [PATCH v2 00/22] PCI: fix config and I/O Address space memory mappings Lorenzo Pieralisi
` (20 preceding siblings ...)
2017-03-27 9:49 ` [PATCH v2 21/22] PCI: keystone-dw: " Lorenzo Pieralisi
@ 2017-03-27 9:49 ` Lorenzo Pieralisi
21 siblings, 0 replies; 48+ messages in thread
From: Lorenzo Pieralisi @ 2017-03-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
PCI configuration space should be mapped with a memory region type that
generates on the CPU host bus non-posted write transations. Update the
driver to use the devm_ioremap_nopost* interface to make sure the correct
memory mappings for PCI configuration space are used.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh@kernel.org>
---
drivers/pci/host/pci-versatile.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
index 5ebee7d..60fd396 100644
--- a/drivers/pci/host/pci-versatile.c
+++ b/drivers/pci/host/pci-versatile.c
@@ -138,7 +138,8 @@ static int versatile_pci_probe(struct platform_device *pdev)
return PTR_ERR(versatile_cfg_base[0]);
res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
- versatile_cfg_base[1] = devm_ioremap_resource(&pdev->dev, res);
+ versatile_cfg_base[1] = devm_ioremap_nopost_resource(&pdev->dev,
+ res);
if (IS_ERR(versatile_cfg_base[1]))
return PTR_ERR(versatile_cfg_base[1]);
--
2.10.0
^ permalink raw reply related [flat|nested] 48+ messages in thread