2017-03-17 17:47:25

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v1 0/3] ioremap() tidy-up

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(-)


2017-03-17 17:47:29

by Bjorn Helgaas

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

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]>
---
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)

2017-03-17 17:47:28

by Bjorn Helgaas

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

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]>
---
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

2017-03-17 17:47:26

by Bjorn Helgaas

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

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]>
---
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.
*/

2017-03-17 22:00:09

by Arnd Bergmann

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

On Fri, Mar 17, 2017 at 6:46 PM, Bjorn Helgaas <[email protected]> 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 <[email protected]>

Do you want to take them through your PCI tree?

Arnd

2017-03-20 18:18:58

by Bjorn Helgaas

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

On Fri, Mar 17, 2017 at 11:00:03PM +0100, Arnd Bergmann wrote:
> On Fri, Mar 17, 2017 at 6:46 PM, Bjorn Helgaas <[email protected]> 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 <[email protected]>
>
> 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