[PATCH v1 0/3] ioremap() tidy-up

[PATCH v1 0/3] ioremap() tidy-up

[Bjorn Helgaas]

1) Fix some comments that say "IOMMU" when they mean "MMU".

2) Remove the generic __ioremap() definition, which I think is unused and
confusing.

3) Simplify the comments about ioremap() implementation.  I split this out
in case I went too far and made this controversial.

---

Bjorn Helgaas (3):
      asm-generic/io.h: Fix "IOMMU" typos
      asm-generic/io.h: Remove unused generic __ioremap() definition
      asm-generic/io.h: Simplify ioremap() comments


 include/asm-generic/io.h |   33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

[PATCH v1 1/3] asm-generic/io.h: Fix "IOMMU" typos

[Bjorn Helgaas]

The ioremap() block comment refers to "IOMMU" when it means "MMU".  Fix
these typos to avoid confusion.

Fixes: 8c7ea50c010b ("x86/mm, asm-generic: Add IOMMU ioremap_uc() variant default")
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/asm-generic/io.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ef015eb3403..978d2e27ce1d 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -802,16 +802,16 @@ static inline void *phys_to_virt(unsigned long address)
 /**
  * DOC: ioremap() and ioremap_*() variants
  *
- * If you have an IOMMU your architecture is expected to have both ioremap()
+ * If you have an MMU your architecture is expected to have both ioremap()
  * and iounmap() implemented otherwise the asm-generic helpers will provide a
  * direct mapping.
  *
- * There are ioremap_*() call variants, if you have no IOMMU we naturally will
+ * There are ioremap_*() call variants, if you have no MMU we naturally will
  * default to direct mapping for all of them, you can override these defaults.
- * If you have an IOMMU you are highly encouraged to provide your own
+ * If you have an MMU you are highly encouraged to provide your own
  * ioremap variant implementation as there currently is no safe architecture
  * agnostic default. To avoid possible improper behaviour default asm-generic
- * ioremap_*() variants all return NULL when an IOMMU is available. If you've
+ * ioremap_*() variants all return NULL when an MMU is available. If you've
  * defined your own ioremap_*() variant you must then declare your own
  * ioremap_*() variant as defined to itself to avoid the default NULL return.
  */

[PATCH v1 2/3] asm-generic/io.h: Remove unused generic __ioremap() definition

[Bjorn Helgaas]

Several arches use __ioremap() to help implement the generic ioremap(),
ioremap_nocache(), and ioremap_wc() interfaces, but this usage is all
inside the arch/ directory.

The only __ioremap() uses outside arch/ are in the ZorroII RAM disk driver
and some framebuffer drivers that are only buildable on m68k and powerpc,
and they use the versions provided by those arches.

There's no need for a generic version of __ioremap(), so remove it.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/asm-generic/io.h |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 978d2e27ce1d..e0a331a22346 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -843,15 +843,6 @@ static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
 }
 #endif
 
-#ifndef __ioremap
-#define __ioremap __ioremap
-static inline void __iomem *__ioremap(phys_addr_t offset, size_t size,
-				      unsigned long flags)
-{
-	return ioremap(offset, size);
-}
-#endif
-
 #ifndef ioremap_nocache
 #define ioremap_nocache ioremap_nocache
 static inline void __iomem *ioremap_nocache(phys_addr_t offset, size_t size)

[PATCH v1 3/3] asm-generic/io.h: Simplify ioremap() comments

[Bjorn Helgaas]

Simplify ioremap() comments to make it clear that arches with an MMU *must*
implement ioremap() and iounmap(), and that the default implementations
only apply to non-MMU arches.  It's obvious how to override the defaults;
no need to educate people here.  Remove the ancient "struct page" comment
that doesn't seem related to anything here.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/asm-generic/io.h |   24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index e0a331a22346..3f8a7e589071 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -780,8 +780,7 @@ static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
 #endif /* CONFIG_GENERIC_IOMAP */
 
 /*
- * Change virtual addresses to physical addresses and vv.
- * These are pretty trivial
+ * Change virtual addresses to physical addresses and vice versa.
  */
 #ifndef virt_to_phys
 #define virt_to_phys virt_to_phys
@@ -802,18 +801,11 @@ static inline void *phys_to_virt(unsigned long address)
 /**
  * DOC: ioremap() and ioremap_*() variants
  *
- * If you have an MMU your architecture is expected to have both ioremap()
- * and iounmap() implemented otherwise the asm-generic helpers will provide a
- * direct mapping.
+ * If you have an MMU, your architecture must implement both ioremap() and
+ * iounmap().
  *
- * There are ioremap_*() call variants, if you have no MMU we naturally will
- * default to direct mapping for all of them, you can override these defaults.
- * If you have an MMU you are highly encouraged to provide your own
- * ioremap variant implementation as there currently is no safe architecture
- * agnostic default. To avoid possible improper behaviour default asm-generic
- * ioremap_*() variants all return NULL when an MMU is available. If you've
- * defined your own ioremap_*() variant you must then declare your own
- * ioremap_*() variant as defined to itself to avoid the default NULL return.
+ * It must also implement variants such as ioremap_uc().  The default
+ * implementation here returns failure (NULL) to avoid improper behavior.
  */
 
 #ifdef CONFIG_MMU
@@ -829,10 +821,8 @@ static inline void __iomem *ioremap_uc(phys_addr_t offset, size_t size)
 #else /* !CONFIG_MMU */
 
 /*
- * Change "struct page" to physical address.
- *
- * This implementation is for the no-MMU case only... if you have an MMU
- * you'll need to provide your own definitions.
+ * If you don't have an MMU, the default implementations here provide
+ * direct identity mapping.  You can override these if necessary.
  */
 
 #ifndef ioremap

Re: [PATCH v1 0/3] ioremap() tidy-up

[Arnd Bergmann]

On Fri, Mar 17, 2017 at 6:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> 1) Fix some comments that say "IOMMU" when they mean "MMU".
>
> 2) Remove the generic __ioremap() definition, which I think is unused and
> confusing.
>
> 3) Simplify the comments about ioremap() implementation.  I split this out
> in case I went too far and made this controversial.

All look good

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Do you want to take them through your PCI tree?

    Arnd

Re: [PATCH v1 0/3] ioremap() tidy-up

[Bjorn Helgaas]

On Fri, Mar 17, 2017 at 11:00:03PM +0100, Arnd Bergmann wrote:
> On Fri, Mar 17, 2017 at 6:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > 1) Fix some comments that say "IOMMU" when they mean "MMU".
> >
> > 2) Remove the generic __ioremap() definition, which I think is unused and
> > confusing.
> >
> > 3) Simplify the comments about ioremap() implementation.  I split this out
> > in case I went too far and made this controversial.
> 
> All look good
> 
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> Do you want to take them through your PCI tree?

Sure, I can.  I should have copied linux-pci; this was all motivated by
reviewing Lorenzo's patches, will probably go via my tree and should be
coordinated with these.

Bjorn