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(-)
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 <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
---
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)
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 <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
---
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
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 <[email protected]>
---
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.
*/
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 <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
---
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.
*/
Hi Björn,
On Mon, Mar 20, 2017 at 7:42 PM, Bjorn Helgaas <[email protected]> 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 <[email protected]>
> Reviewed-by: Arnd Bergmann <[email protected]>
Regardless,
Acked-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
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 <[email protected]> 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 <[email protected]>
> > Reviewed-by: Arnd Bergmann <[email protected]>
>
> Regardless,
> Acked-by: Geert Uytterhoeven <[email protected]>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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