[PATCH v2 0/4] ioremap() tidy-up

[PATCH v2 0/4] ioremap() tidy-up

[Bjorn Helgaas]

Fix some typos, remove unused code, simplify comments.

I added patch 4 to remove the default ioremap_uc() implementation on MMU
systems.

I hesitated about this because it basically reverts 8c7ea50c010b ("x86/mm,
asm-generic: Add IOMMU ioremap_uc() variant default"), and I'm worried that
I'm missing the real value of having that default implementation.  So this
is just a proposal; if it makes no sense, I'll drop patch 4 again.

As far as I can tell, having the default ioremap_uc() implementation means
we can *build* drivers that use it, but those drivers won't actually work.
It seems preferable to me to have those be build-time failures rather than
run-time failures, but if I'm missing something, let me know.

Change from v1 to v2:
  - Add Arnd's Reviewed-by on patches 1-3.
  - Add patch 4 to remove ioremap_uc() stub.

---

Bjorn Helgaas (4):
      asm-generic/io.h: Fix "IOMMU" typos
      asm-generic/io.h: Remove unused generic __ioremap() definition
      asm-generic/io.h: Simplify ioremap() comments
      asm-generic/io.h: Drop ioremap_uc() stub for systems with MMU


 include/asm-generic/io.h |   49 ++++++++--------------------------------------
 1 file changed, 8 insertions(+), 41 deletions(-)

[PATCH v2 1/4] 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>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 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 v2 2/4] 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>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 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 v2 3/4] 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>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 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

[PATCH v2 4/4] asm-generic/io.h: Drop ioremap_uc() stub for systems with MMU

[Bjorn Helgaas]

8c7ea50c010b ("x86/mm, asm-generic: Add IOMMU ioremap_uc() variant
default") added a default ioremap_uc() implementation that always returns
NULL to indicate failure.

For arches that don't implement their own ioremap_uc(), the default
implementation allows us to *build* drivers that use ioremap_uc(), but they
won't work.

Remove the default ioremap_uc() so a driver that depends on it will fail at
build-time rather than at run-time.

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

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 3f8a7e589071..7b0617d2d210 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -798,29 +798,15 @@ static inline void *phys_to_virt(unsigned long address)
 }
 #endif
 
+#ifndef CONFIG_MMU
+
 /**
  * DOC: ioremap() and ioremap_*() variants
  *
  * If you have an MMU, your architecture must implement both ioremap() and
- * iounmap().
+ * iounmap(), as well as variants like ioremap_nocache(), ioremap_uc(),
+ * and ioremap_wc().
  *
- * It must also implement variants such as ioremap_uc().  The default
- * implementation here returns failure (NULL) to avoid improper behavior.
- */
-
-#ifdef CONFIG_MMU
-
-#ifndef ioremap_uc
-#define ioremap_uc ioremap_uc
-static inline void __iomem *ioremap_uc(phys_addr_t offset, size_t size)
-{
-	return NULL;
-}
-#endif
-
-#else /* !CONFIG_MMU */
-
-/*
  * If you don't have an MMU, the default implementations here provide
  * direct identity mapping.  You can override these if necessary.
  */

Re: [PATCH v2 2/4] asm-generic/io.h: Remove unused generic __ioremap() definition

[Geert Uytterhoeven]

Hi Björn,

On Mon, Mar 20, 2017 at 7:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> 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.

These all predate the ioremap_*() variants, and can be converted to
either ioremap_nocache() or ioremap_wt().

However, PPC doesn't implement ioremap_wt() yet, so asm-generic will
fall back to the less-efficient nocache variant.
PPC does support __ioremap(..., _PAGE_WRITETHRU), so adding a wrapper
is trivial.

> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Regardless,
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 2/4] asm-generic/io.h: Remove unused generic __ioremap() definition

[Geert Uytterhoeven]

Hi Bj=C3=B6rn,

On Mon, Mar 20, 2017 at 7:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> 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 drive=
r
> 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.

These all predate the ioremap_*() variants, and can be converted to
either ioremap_nocache() or ioremap_wt().

However, PPC doesn't implement ioremap_wt() yet, so asm-generic will
fall back to the less-efficient nocache variant.
PPC does support __ioremap(..., _PAGE_WRITETHRU), so adding a wrapper
is trivial.

> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Regardless,
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
                                -- Linus Torvalds

Re: [PATCH v2 2/4] asm-generic/io.h: Remove unused generic __ioremap() definition

[Geert Uytterhoeven]

Hi Björn,

On Mon, Mar 20, 2017 at 7:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> 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.

These all predate the ioremap_*() variants, and can be converted to
either ioremap_nocache() or ioremap_wt().

However, PPC doesn't implement ioremap_wt() yet, so asm-generic will
fall back to the less-efficient nocache variant.
PPC does support __ioremap(..., _PAGE_WRITETHRU), so adding a wrapper
is trivial.

> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Regardless,
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH v2 2/4] asm-generic/io.h: Remove unused generic __ioremap() definition

[Bjorn Helgaas]

On Tue, Mar 21, 2017 at 11:37:11AM +0100, Geert Uytterhoeven wrote:
> Hi Björn,
> 
> On Mon, Mar 20, 2017 at 7:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > 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.
> 
> These all predate the ioremap_*() variants, and can be converted to
> either ioremap_nocache() or ioremap_wt().
> 
> However, PPC doesn't implement ioremap_wt() yet, so asm-generic will
> fall back to the less-efficient nocache variant.
> PPC does support __ioremap(..., _PAGE_WRITETHRU), so adding a wrapper
> is trivial.

Thanks, I'll try adding ioremap_wt() (at least for PPC32) and cleaning this
up.

> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> Regardless,
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds