[PATCH 1/2] asm-generic: add MMU variants of io.h functions

[PATCH 1/2] asm-generic: add MMU variants of io.h functions

[Jonas Bonn]

Some of the implementations, in particular the ioremap variants, in
asm-generic/io.h are for systems with an MMU.  In order to be able to
use the generic header file for systems with an MMU, this patch wraps
these implementations in checks for CONFIG_MMU.

Tested on OpenRISC.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
Cc: vapier@gentoo.org
Cc: liqin.chen@sunplusct.com
Cc: gxt@mprc.pku.edu.cn
---

I'm not totally certain that this doesn't have implications for the
other architectures, in particular Score which seems to have an MMU yet
uses these ioremap functions anyway...  Comments on this would be
appreciated.

Thanks,
Jonas

 include/asm-generic/io.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index e0ffa3d..50f4f43 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -307,7 +307,11 @@ static inline void *phys_to_virt(unsigned long address)
 
 /*
  * 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.
  */
+#ifndef CONFIG_MMU
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;
@@ -326,6 +330,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void *addr)
 {
 }
+#endif /* CONFIG_MMU */
 
 #ifndef CONFIG_GENERIC_IOMAP
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
@@ -342,7 +347,12 @@ extern void ioport_unmap(void __iomem *p);
 #endif /* CONFIG_GENERIC_IOMAP */
 
 #define xlate_dev_kmem_ptr(p)	p
+
+#ifdef CONFIG_MMU
+#define xlate_dev_mem_ptr(p)	__va(p)
+#else
 #define xlate_dev_mem_ptr(p)	((void *) (p))
+#endif
 
 #ifndef virt_to_bus
 static inline unsigned long virt_to_bus(volatile void *address)
-- 
1.7.4.1

[PATCH 1/2] asm-generic: add MMU variants of io.h functions

[Jonas Bonn]

Some of the implementations, in particular the ioremap variants, in
asm-generic/io.h are for systems with an MMU.  In order to be able to
use the generic header file for systems with an MMU, this patch wraps
these implementations in checks for CONFIG_MMU.

Tested on OpenRISC.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
Cc: vapier@gentoo.org
Cc: liqin.chen@sunplusct.com
Cc: gxt@mprc.pku.edu.cn
---

I'm not totally certain that this doesn't have implications for the
other architectures, in particular Score which seems to have an MMU yet
uses these ioremap functions anyway...  Comments on this would be
appreciated.

Thanks,
Jonas

 include/asm-generic/io.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index e0ffa3d..50f4f43 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -307,7 +307,11 @@ static inline void *phys_to_virt(unsigned long address)
 
 /*
  * 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.
  */
+#ifndef CONFIG_MMU
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;
@@ -326,6 +330,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void *addr)
 {
 }
+#endif /* CONFIG_MMU */
 
 #ifndef CONFIG_GENERIC_IOMAP
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
@@ -342,7 +347,12 @@ extern void ioport_unmap(void __iomem *p);
 #endif /* CONFIG_GENERIC_IOMAP */
 
 #define xlate_dev_kmem_ptr(p)	p
+
+#ifdef CONFIG_MMU
+#define xlate_dev_mem_ptr(p)	__va(p)
+#else
 #define xlate_dev_mem_ptr(p)	((void *) (p))
+#endif
 
 #ifndef virt_to_bus
 static inline unsigned long virt_to_bus(volatile void *address)
-- 
1.7.4.1

[PATCH 2/2] iomap: make IOPORT/PCI mapping functions conditional

[Jonas Bonn]

Use the CONFIG_HAS_IOPORT and CONFIG_PCI options to decide whether or
not functions for mapping these areas are provided.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 include/asm-generic/io.h    |    2 ++
 include/asm-generic/iomap.h |    4 ++++
 lib/iomap.c                 |    4 ++++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 50f4f43..49969c2 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -332,6 +332,7 @@ static inline void iounmap(void *addr)
 }
 #endif /* CONFIG_MMU */
 
+#ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
@@ -345,6 +346,7 @@ static inline void ioport_unmap(void __iomem *p)
 extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
 extern void ioport_unmap(void __iomem *p);
 #endif /* CONFIG_GENERIC_IOMAP */
+#endif /* CONFIG_HAS_IOPORT */
 
 #define xlate_dev_kmem_ptr(p)	p
 
diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index 76b0cc5..c74ef2c 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -56,17 +56,21 @@ extern void iowrite8_rep(void __iomem *port, const void *buf, unsigned long coun
 extern void iowrite16_rep(void __iomem *port, const void *buf, unsigned long count);
 extern void iowrite32_rep(void __iomem *port, const void *buf, unsigned long count);
 
+#ifdef CONFIG_HAS_IOPORT
 /* Create a virtual mapping cookie for an IO port range */
 extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
 extern void ioport_unmap(void __iomem *);
+#endif
 
 #ifndef ARCH_HAS_IOREMAP_WC
 #define ioremap_wc ioremap_nocache
 #endif
 
+#ifdef CONFIG_PCI
 /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
 struct pci_dev;
 extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
 extern void pci_iounmap(struct pci_dev *dev, void __iomem *);
+#endif
 
 #endif
diff --git a/lib/iomap.c b/lib/iomap.c
index d322293..5dbcb4b 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -224,6 +224,7 @@ EXPORT_SYMBOL(iowrite8_rep);
 EXPORT_SYMBOL(iowrite16_rep);
 EXPORT_SYMBOL(iowrite32_rep);
 
+#ifdef CONFIG_HAS_IOPORT
 /* Create a virtual mapping cookie for an IO port range */
 void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
@@ -238,7 +239,9 @@ void ioport_unmap(void __iomem *addr)
 }
 EXPORT_SYMBOL(ioport_map);
 EXPORT_SYMBOL(ioport_unmap);
+#endif /* CONFIG_HAS_IOPORT */
 
+#ifdef CONFIG_PCI
 /**
  * pci_iomap - create a virtual mapping cookie for a PCI BAR
  * @dev: PCI device that owns the BAR
@@ -280,3 +283,4 @@ void pci_iounmap(struct pci_dev *dev, void __iomem * addr)
 }
 EXPORT_SYMBOL(pci_iomap);
 EXPORT_SYMBOL(pci_iounmap);
+#endif /* CONFIG_PCI */
-- 
1.7.4.1

Re: [PATCH 2/2] iomap: make IOPORT/PCI mapping functions conditional

[Arnd Bergmann]

On Saturday 02 July 2011 17:53:26 Jonas Bonn wrote:
> Use the CONFIG_HAS_IOPORT and CONFIG_PCI options to decide whether or
> not functions for mapping these areas are provided.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>

Good catch!

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

Did you get a build error without the two options, or did you just have an
idea to save a bit of code size?

	Arnd

Re: [PATCH 2/2] iomap: make IOPORT/PCI mapping functions conditional

[Jonas Bonn]

On Sat, 2011-07-02 at 21:46 +0200, Arnd Bergmann wrote:
> On Saturday 02 July 2011 17:53:26 Jonas Bonn wrote:
> > Use the CONFIG_HAS_IOPORT and CONFIG_PCI options to decide whether or
> > not functions for mapping these areas are provided.
> > 
> > Signed-off-by: Jonas Bonn <jonas@southpole.se>
> 
> Good catch!
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> Did you get a build error without the two options, or did you just have an
> idea to save a bit of code size?
> 

There was no build error; I just figured it would aid in debugging if
you got a compile-time error when trying to build drivers that use these
functions for a platform that lacked them.  Smaller code is always nice,
too.

/Jonas

Re: [PATCH 2/2] iomap: make IOPORT/PCI mapping functions conditional

[Arnd Bergmann]

On Saturday 02 July 2011 21:54:26 Jonas Bonn wrote:
> On Sat, 2011-07-02 at 21:46 +0200, Arnd Bergmann wrote:
> > On Saturday 02 July 2011 17:53:26 Jonas Bonn wrote:
> > > Use the CONFIG_HAS_IOPORT and CONFIG_PCI options to decide whether or
> > > not functions for mapping these areas are provided.
> > > 
> > > Signed-off-by: Jonas Bonn <jonas@southpole.se>
> > 
> > Good catch!
> > 
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > Did you get a build error without the two options, or did you just have an
> > idea to save a bit of code size?
> > 
> 
> There was no build error; I just figured it would aid in debugging if
> you got a compile-time error when trying to build drivers that use these
> functions for a platform that lacked them.  Smaller code is always nice,
> too.

Right. Did you also see the related discussion at
http://lkml.org/lkml/2011/7/1/422 ?

	Arnd

Re: [PATCH 1/2] asm-generic: add MMU variants of io.h functions

[Mike Frysinger]

On Sat, Jul 2, 2011 at 11:53, Jonas Bonn wrote:
> Some of the implementations, in particular the ioremap variants, in
> asm-generic/io.h are for systems with an MMU.

do you mean "without an MMU" ?

> +#ifdef CONFIG_MMU
> +#define xlate_dev_mem_ptr(p)   __va(p)
> +#else
>  #define xlate_dev_mem_ptr(p)   ((void *) (p))
> +#endif

i wonder if we could do:
#ifndef __va
#define __va(p) ((void *)(p))
#endif
#define xlate_dev_mem_ptr(p)   __va(p)
-mike

Re: [PATCH 1/2] asm-generic: add MMU variants of io.h functions

[Jonas Bonn]

On Sat, 2011-07-02 at 12:38 -0400, Mike Frysinger wrote:
> On Sat, Jul 2, 2011 at 11:53, Jonas Bonn wrote:
> > Some of the implementations, in particular the ioremap variants, in
> > asm-generic/io.h are for systems with an MMU.
> 
> do you mean "without an MMU" ?

Yes, of course... that's supposed to be _without_ an MMU.

> 
> > +#ifdef CONFIG_MMU
> > +#define xlate_dev_mem_ptr(p)   __va(p)
> > +#else
> >  #define xlate_dev_mem_ptr(p)   ((void *) (p))
> > +#endif
> 
> i wonder if we could do:
> #ifndef __va
> #define __va(p) ((void *)(p))
> #endif
> #define xlate_dev_mem_ptr(p)   __va(p)
> -mike

This seems to introduce an artificial definition of a "virtual" address
for systems without an MMU, which strikes me as a bit odd.  If this is
what people prefer, that's fine... I think I prefer the former variant,
though.

/Jonas

Re: [PATCH 1/2] asm-generic: add MMU variants of io.h functions

[Mike Frysinger]

On Sat, Jul 2, 2011 at 12:47, Jonas Bonn wrote:
> On Sat, 2011-07-02 at 12:38 -0400, Mike Frysinger wrote:
>> On Sat, Jul 2, 2011 at 11:53, Jonas Bonn wrote:
>> > +#ifdef CONFIG_MMU
>> > +#define xlate_dev_mem_ptr(p)   __va(p)
>> > +#else
>> >  #define xlate_dev_mem_ptr(p)   ((void *) (p))
>> > +#endif
>>
>> i wonder if we could do:
>> #ifndef __va
>> #define __va(p) ((void *)(p))
>> #endif
>> #define xlate_dev_mem_ptr(p)   __va(p)
>> -mike
>
> This seems to introduce an artificial definition of a "virtual" address
> for systems without an MMU, which strikes me as a bit odd.  If this is
> what people prefer, that's fine... I think I prefer the former variant,
> though.

this isnt a problem.  we do this sort of thing in many places since
people develop under MMU and use the API that exists there.

however, i see that we already have __va() in asm-generic/page.h.  so
you can assume __va() exists.  so just change the existing definition
to:
#define xlate_dev_mem_ptr(p)   __va(p)
-mike

Re: [PATCH 1/2] asm-generic: add MMU variants of io.h functions

[Mike Frysinger]

On Sat, Jul 2, 2011 at 12:47, Jonas Bonn wrote:
> On Sat, 2011-07-02 at 12:38 -0400, Mike Frysinger wrote:
>> On Sat, Jul 2, 2011 at 11:53, Jonas Bonn wrote:
>> > +#ifdef CONFIG_MMU
>> > +#define xlate_dev_mem_ptr(p)   __va(p)
>> > +#else
>> >  #define xlate_dev_mem_ptr(p)   ((void *) (p))
>> > +#endif
>>
>> i wonder if we could do:
>> #ifndef __va
>> #define __va(p) ((void *)(p))
>> #endif
>> #define xlate_dev_mem_ptr(p)   __va(p)
>> -mike
>
> This seems to introduce an artificial definition of a "virtual" address
> for systems without an MMU, which strikes me as a bit odd.  If this is
> what people prefer, that's fine... I think I prefer the former variant,
> though.

this isnt a problem.  we do this sort of thing in many places since
people develop under MMU and use the API that exists there.

however, i see that we already have __va() in asm-generic/page.h.  so
you can assume __va() exists.  so just change the existing definition
to:
#define xlate_dev_mem_ptr(p)   __va(p)
-mike

[PATCH v2 1/1] asm-generic: add MMU variants of io.h functions

[Jonas Bonn]

Some of the implementations, in particular the ioremap variants, in
asm-generic/io.h are for systems without an MMU.  In order to be able to
use the generic header file for systems with an MMU, this patch wraps
these implementations in checks for CONFIG_MMU.

Tested on OpenRISC.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
Cc: vapier@gentoo.org
Cc: liqin.chen@sunplusct.com
Cc: gxt@mprc.pku.edu.cn
---
 include/asm-generic/io.h |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index e0ffa3d..a1caf2d 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -307,7 +307,11 @@ static inline void *phys_to_virt(unsigned long address)
 
 /*
  * 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.
  */
+#ifndef CONFIG_MMU
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;
@@ -326,6 +330,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void *addr)
 {
 }
+#endif /* CONFIG_MMU */
 
 #ifndef CONFIG_GENERIC_IOMAP
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
@@ -342,7 +347,7 @@ extern void ioport_unmap(void __iomem *p);
 #endif /* CONFIG_GENERIC_IOMAP */
 
 #define xlate_dev_kmem_ptr(p)	p
-#define xlate_dev_mem_ptr(p)	((void *) (p))
+#define xlate_dev_mem_ptr(p)	__va(p)
 
 #ifndef virt_to_bus
 static inline unsigned long virt_to_bus(volatile void *address)
-- 
1.7.4.1

Re: [PATCH v2 1/1] asm-generic: add MMU variants of io.h functions

[Mike Frysinger]

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

Re: [PATCH v2 1/1] asm-generic: add MMU variants of io.h functions

[Arnd Bergmann]

On Saturday 02 July 2011 19:36:08 Jonas Bonn wrote:
> 
> Some of the implementations, in particular the ioremap variants, in
> asm-generic/io.h are for systems without an MMU.  In order to be able to
> use the generic header file for systems with an MMU, this patch wraps
> these implementations in checks for CONFIG_MMU.
> 
> Tested on OpenRISC.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> Cc: vapier@gentoo.org
> Cc: liqin.chen@sunplusct.com
> Cc: gxt@mprc.pku.edu.cn

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

Re: [PATCH v2 1/1] asm-generic: add MMU variants of io.h functions

[Guan Xuetao]

It's very good to me, thanks.

Guan Xuetao

On Sat, 2011-07-02 at 19:36 +0200, Jonas Bonn wrote:
> Some of the implementations, in particular the ioremap variants, in
> asm-generic/io.h are for systems without an MMU.  In order to be able to
> use the generic header file for systems with an MMU, this patch wraps
> these implementations in checks for CONFIG_MMU.
> 
> Tested on OpenRISC.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> Cc: vapier@gentoo.org
> Cc: liqin.chen@sunplusct.com
> Cc: gxt@mprc.pku.edu.cn
> ---
>  include/asm-generic/io.h |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index e0ffa3d..a1caf2d 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -307,7 +307,11 @@ static inline void *phys_to_virt(unsigned long address)
>  
>  /*
>   * 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.
>   */
> +#ifndef CONFIG_MMU
>  static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>  {
>  	return (void __iomem*) (unsigned long)offset;
> @@ -326,6 +330,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>  static inline void iounmap(void *addr)
>  {
>  }
> +#endif /* CONFIG_MMU */
>  
>  #ifndef CONFIG_GENERIC_IOMAP
>  static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
> @@ -342,7 +347,7 @@ extern void ioport_unmap(void __iomem *p);
>  #endif /* CONFIG_GENERIC_IOMAP */
>  
>  #define xlate_dev_kmem_ptr(p)	p
> -#define xlate_dev_mem_ptr(p)	((void *) (p))
> +#define xlate_dev_mem_ptr(p)	__va(p)
>  
>  #ifndef virt_to_bus
>  static inline unsigned long virt_to_bus(volatile void *address)