[PATCH] Clarify pci_iomap() usage for MMIO-only devices

[PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Luis R. Rodriguez]

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

This patch updates the pci_iomap() kernel-doc to make it clarify the
case when read*()/write*() can be called over ioread*/iowrite*(). When
driver writers read this documenation sometimes it is assumed you just
*need* to use ioread*()/iorwrite*(). We have an exception so lets just
clarify this is not true for the exception.

 lib/iomap.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>

  Luis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pci_iomap-doc-clarification.diff --]
[-- Type: text/x-patch; name="pci_iomap-doc-clarification.diff", Size: 1208 bytes --]

diff --git a/lib/iomap.c b/lib/iomap.c
index 864f2ec..33651f8 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -247,9 +247,14 @@ EXPORT_SYMBOL(ioport_unmap);
  * @maxlen: length of the memory to map
  *
  * Using this function you will get a __iomem address to your device BAR.
- * You can access it using ioread*() and iowrite*(). These functions hide
- * the details if this is a MMIO or PIO address space and will just do what
- * you expect from them in the correct way.
+ * If you use pci_iomap() it is recommended you use ioread*() and iowrite*(). If
+ * your device is MMIO-only and as long as future architectures don't break the
+ * assumption that pcio_iomap()-returned cookie can be used by read*()/write*()
+ * then you can use read*()/write*() instead. The ioread*() and iowrite*() 
+ * functions hide the details if this is a MMIO or PIO address space and will
+ * just do what you expect from them in the correct way. Drivers for MMIO-only 
+ * devices would use this only to save themselves from having to call 
+ * &pci_resource_start().
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
  * the complete BAR without checking for its length first, pass %0 here.

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Alan Cox]

On Mon, 17 Sep 2007 16:22:07 -0400
"Luis R. Rodriguez" <mcgrof@gmail.com> wrote:

> This patch updates the pci_iomap() kernel-doc to make it clarify the
> case when read*()/write*() can be called over ioread*/iowrite*(). When
> driver writers read this documenation sometimes it is assumed you just
> *need* to use ioread*()/iorwrite*(). We have an exception so lets just
> clarify this is not true for the exception.
> 
>  lib/iomap.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>

Reviewed-by: Alan Cox <alan@redhat.com>

Although I would say s/recommended/required/

Alan

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Luis R. Rodriguez]

On 9/18/07, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Mon, 17 Sep 2007 16:22:07 -0400
> "Luis R. Rodriguez" <mcgrof@gmail.com> wrote:
>
> > This patch updates the pci_iomap() kernel-doc to make it clarify the
> > case when read*()/write*() can be called over ioread*/iowrite*(). When
> > driver writers read this documenation sometimes it is assumed you just
> > *need* to use ioread*()/iorwrite*(). We have an exception so lets just
> > clarify this is not true for the exception.
> >
> >  lib/iomap.c |   11 ++++++++---
> >  1 files changed, 8 insertions(+), 3 deletions(-)
> >
> > Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
>
> Reviewed-by: Alan Cox <alan@redhat.com>
>
> Although I would say s/recommended/required/

Alright, here is the same patch inline with s/recommended/required/ language:

Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>

  Luis

---
 lib/iomap.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/iomap.c b/lib/iomap.c
index 864f2ec..33651f8 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -247,9 +247,14 @@ EXPORT_SYMBOL(ioport_unmap);
  * @maxlen: length of the memory to map
  *
  * Using this function you will get a __iomem address to your device BAR.
- * You can access it using ioread*() and iowrite*(). These functions hide
- * the details if this is a MMIO or PIO address space and will just do what
- * you expect from them in the correct way.
+ * If you use pci_iomap() it is recommended you use ioread*() and
iowrite*(). If
+ * your device is MMIO-only and as long as future architectures don't break the
+ * assumption that pcio_iomap()-returned cookie can be used by read*()/write*()
+ * then you can use read*()/write*() instead. The ioread*() and iowrite*()
+ * functions hide the details if this is a MMIO or PIO address space and will
+ * just do what you expect from them in the correct way. Drivers for MMIO-only
+ * devices would use this only to save themselves from having to call
+ * &pci_resource_start().
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
  * the complete BAR without checking for its length first, pass %0 here.

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> 
> Alright, here is the same patch inline with s/recommended/required/ language:

Well, the thing is, I'm not at all sure that I agree with this.

If you use ioport_map/unmap, then you really *should* access them with the 
proper iomem accessors (ioread/iowrite). The fact that it may happen to 
work (when using the default lib/iomap.c implementation, at least) on 
some architectures and with the current implementation still doesn't mean 
that you should necessarily use readb/writeb.

After all, you cannot use "inb/outb" on it, even if would happen to be an 
IO address.

So what is this usage that wants to use the bogus accessor? Why not fix 
that instead of adding documentation for something that is very arguably 
something we want to *avoid* having people do!

		Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Luis R. Rodriguez]

On 9/18/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> >
> > Alright, here is the same patch inline with s/recommended/required/ language:
>
> Well, the thing is, I'm not at all sure that I agree with this.
>
> If you use ioport_map/unmap, then you really *should* access them with the
> proper iomem accessors (ioread/iowrite). The fact that it may happen to
> work (when using the default lib/iomap.c implementation, at least) on
> some architectures and with the current implementation still doesn't mean
> that you should necessarily use readb/writeb.
>
> After all, you cannot use "inb/outb" on it, even if would happen to be an
> IO address.
>
> So what is this usage that wants to use the bogus accessor? Why not fix
> that instead of adding documentation for something that is very arguably
> something we want to *avoid* having people do!

ACK -- driver developers use this just to save themselves a few lines
from calling pci_resource_start() and friends. How about having an
inline which does what pci_iomap() does except it doesn't call
ioport_map() ? I am just not sure where this would go..

  Luis

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> 
> ACK -- driver developers use this just to save themselves a few lines
> from calling pci_resource_start() and friends. How about having an
> inline which does what pci_iomap() does except it doesn't call
> ioport_map() ? I am just not sure where this would go..

I'm not understanding what the problem is?

Why don't these people just use "ioread*()/iowrite*()"? 

In other words, the whole point of *not* using "read*/write*()" is that 
you get a whole slew of much nicer interfaces.

So can people explain this fundamental issue? Why do people insist on 
using the old interfaces (and matching them with the new setup)?

			Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Luis R. Rodriguez]

On 9/18/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> >
> > ACK -- driver developers use this just to save themselves a few lines
> > from calling pci_resource_start() and friends. How about having an
> > inline which does what pci_iomap() does except it doesn't call
> > ioport_map() ? I am just not sure where this would go..
>
> I'm not understanding what the problem is?
>
> Why don't these people just use "ioread*()/iowrite*()"?
>
> In other words, the whole point of *not* using "read*/write*()" is that
> you get a whole slew of much nicer interfaces.
>
> So can people explain this fundamental issue? Why do people insist on
> using the old interfaces (and matching them with the new setup)?

An extra branch is created on MMIO-only devices on read/writes on the
IO_COND macro using this interface -- or is this optimized out?

  Luis

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> 
> An extra branch is created on MMIO-only devices on read/writes on the
> IO_COND macro using this interface -- or is this optimized out?

Umm. Does anybody actually have any performance numbers? 

The thing is, those things are *cheap* compared to the IO. And any 
high-performance device will be using DMA for the real IO, so we're not 
generally even talking about any performance-critical stuff. 

Quite frankly, if performance is a _real_ reason to avoid 
ioread*/iowrite*, I'll happily accept read*/write*, but it would be needed 
to be backed up by real numbers. Can you even measure it?

I would definitely *not* encourage the notion that people should use 
readl/writel because of "performance reasons". That may be valid for some 
fbcon driver, but those drivers go to other extremes (ie they use 
"__raw_writel()" etc, and MTRR's etc).

If you don't use write-combining memory regions, the performance argument 
is not really valid.

			Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Luis R. Rodriguez]

On 9/18/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> >
> > An extra branch is created on MMIO-only devices on read/writes on the
> > IO_COND macro using this interface -- or is this optimized out?
>
> Umm. Does anybody actually have any performance numbers?
>
> The thing is, those things are *cheap* compared to the IO. And any
> high-performance device will be using DMA for the real IO, so we're not
> generally even talking about any performance-critical stuff.

ACK but do we really need to benchmark this if we *know* we are
creating unnecessary branches? How about Jeff's suggestion of
introducing pci_mmio_map() ? This way we leave the current
documenation as it is and just introduce this for MMIO-only devices.
No benchmarks are necessary then, and no need to confuse people with
the old API.

  Luis

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> 
> ACK but do we really need to benchmark this if we *know* we are
> creating unnecessary branches? How about Jeff's suggestion of
> introducing pci_mmio_map() ?

Here's the counter-argument:

 - the run-time expense of this is basically *zero*

 - the expense of badly written drivers is absolutely *huge*

The fewer "clever" interfaces we have for no good reason, and the less 
unnecessary stuff that really doesn't matter, the more we help the 
*second* case.

The fact is, for device drivers, the *true* performance issues are nowhere 
*near* the actual IO accessor functions, and are all at a much higher 
level for all but a very very limited set of drivers. And I think you're 
making the important issues *worse*.

Really. "pci_mmio_map()" may not be a bad interface, but you're simply 
working on entirely the wrong problem. The problem is not an out-of-line 
IO accessor function - the problem is that you're making it even more 
complicated to write drivers.

The whole point of the "iomap()" interface was *never* about making the 
driver interfaces faster. It was about making them *cleaner*. And you're 
screwing that up!

The old situation with SATA drivers that had

	if (iomem)
		writel(..)
	else
		outl(..)

in the cases that needed it (and used hardcoded writel/outl in the cases 
that didn't) was an example of code that "in theory" is faster. But 
dammit, in practice that mattered not one whit, and what iomap() tries to 
do is to attack the _real_ problem we had in that area. 

Which had nothing what-so-ever to do with any branches.

		Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Jeff Garzik]

Linus Torvalds wrote:
> The old situation with SATA drivers that had
> 
> 	if (iomem)
> 		writel(..)
> 	else
> 		outl(..)
> 
> in the cases that needed it (and used hardcoded writel/outl in the cases 
> that didn't) was an example of code that "in theory" is faster. But 
> dammit, in practice that mattered not one whit, and what iomap() tries to 
> do is to attack the _real_ problem we had in that area. 
> 
> Which had nothing what-so-ever to do with any branches.

And none of those issues are a factor at all in ath5k (which spawned 
this sub-discussion), or indeed many other drivers.  The above code you 
cite is -generic-, hardware agnostic code.

Most new hardware is MMIO-only, making ioread32() only for drivers that 
care about legacy IO support, something that is being slowly phased out. 
  e.g. legacy IDE, legacy 10/100 mbps ethernet NICs with the dual 
MMIO/PIO register spaces.

	Jeff

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Jeff Garzik wrote:
> 
> Most new hardware is MMIO-only, making ioread32() only for drivers that care
> about legacy IO support, something that is being slowly phased out.  e.g.
> legacy IDE, legacy 10/100 mbps ethernet NICs with the dual MMIO/PIO register
> spaces.

Hey, I think your patch was better than the one that just assumed that you 
can do "read*/write*()" on an iomap(). If people really want it, I don't 
care *that* much.

I just think that the arguments that have been raised so far have really 
been very weak. There really is no "performance" argument without some 
kinds of numbers.

I just think that proliferation of IO interfaces is a bad idea. 

			Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Linus Torvalds wrote:
> 
> I just think that proliferation of IO interfaces is a bad idea. 

Side note: just allowing readl/writel on the iomap'ed pointers obviously 
avoids that proliferation, but iomap really is architecture-specific, so 
not only will different architectures act very differently, it's not 
necessarily the case that iomap() will always have the same pointer as the 
old ioremap(), even though the routines in the *default* implementation in 
lib/iomap.c do that.

In fact, for IO port accesses, we already do some debugging to make sure 
that people don't misuse the iomap() interfaces (ie the whole "Bad IO 
access.." thing). I could well see the same thing happening for MMIO under 
some debugging option, explicitly using some high bits in a 64-bit address 
as a cookie - and that would make using regular readl/writel on the cookie 
just fail spectacularly.

That said, it does seem unlikely (ie if we added a cookie for MMIO 
pointers, we'd probably do it *both* for iomap and for ioremap(), and 
readl/writel would continue to work). So in practice the readl/writel 
probably works.

		Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Benjamin Herrenschmidt]

On Tue, 2007-09-18 at 13:30 -0700, Linus Torvalds wrote:
> Quite frankly, if performance is a _real_ reason to avoid 
> ioread*/iowrite*, I'll happily accept read*/write*, but it would be
> needed 
> to be backed up by real numbers. Can you even measure it?
> 

Also, I've been told that modern x86 chipsets have the ability to remap
IO space in the CPU physical address space. Is that true ? That would
allow even x86 to get rid of the condition and just use some magic
offset at map time.

Ben.

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Wed, 19 Sep 2007, Benjamin Herrenschmidt wrote:
> 
> Also, I've been told that modern x86 chipsets have the ability to remap
> IO space in the CPU physical address space. Is that true ? That would
> allow even x86 to get rid of the condition and just use some magic
> offset at map time.

I've not seen that, but I wouldn't be entirely surprised if IO 
virtualization eventually causes something like this to happen: 
virtualizing PIO is just damn painful right now, due to the lack of any 
way to remap it.

I *think* you may be confused with the PCI config cycles, where the new 
MMIO configuration was introduced (for similar virtualization reasons). 
But it's also possible that this is one of those undocumented areas and 
CPU's actually do have some IO remapping facility.

		Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

Linus Torvalds wrote:
> Why do people insist on 
> using the old interfaces (and matching them with the new setup)?


readl/writel is [slightly] faster, and possibility of using even-write 
__raw_writel() exists, in the old interface...

  ...but pci_iomap() is a bus-friendly wrapper that handles a few checks 
and function calls for you, even if you know your device is always MMIO.

A new pci_mmio_map() helper, to be used with 100% MMIO hardware, might 
help eliminate confusion.

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1585 bytes --]

diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index cde592f..69e5390 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -63,6 +63,7 @@ extern void ioport_unmap(void __iomem *);
 /* 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 __iomem *pci_mmio_map(struct pci_dev *dev, int bar, unsigned long max);
 extern void pci_iounmap(struct pci_dev *dev, void __iomem *);
 
 #endif
diff --git a/lib/iomap.c b/lib/iomap.c
index 864f2ec..c87d7f3 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -275,9 +275,32 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
 	return NULL;
 }
 
+/* for people who wish to use readl/writel exclusively */
+void __iomem *pci_mmio_map(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+	unsigned long start = pci_resource_start(dev, bar);
+	unsigned long len = pci_resource_len(dev, bar);
+	unsigned long flags = pci_resource_flags(dev, bar);
+
+	if (!len || !start)
+		return NULL;
+	if (maxlen && len > maxlen)
+		len = maxlen;
+	if (flags & IORESOURCE_IO)
+		return NULL;
+	if (flags & IORESOURCE_MEM) {
+		if (flags & IORESOURCE_CACHEABLE)
+			return ioremap(start, len);
+		return ioremap_nocache(start, len);
+	}
+	/* What? */
+	return NULL;
+}
+
 void pci_iounmap(struct pci_dev *dev, void __iomem * addr)
 {
 	IO_COND(addr, /* nothing */, iounmap(addr));
 }
 EXPORT_SYMBOL(pci_iomap);
+EXPORT_SYMBOL(pci_mmio_map);
 EXPORT_SYMBOL(pci_iounmap);

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Benjamin Herrenschmidt]

On Tue, 2007-09-18 at 16:21 -0400, Jeff Garzik wrote:
> A new pci_mmio_map() helper, to be used with 100% MMIO hardware, might
> help eliminate confusion. 

Maybe not the best name in theory but at least would show that it
relates to existing ioremap would be pci_ioremap()

Ben.

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

Benjamin Herrenschmidt wrote:
> On Tue, 2007-09-18 at 16:21 -0400, Jeff Garzik wrote:
>> A new pci_mmio_map() helper, to be used with 100% MMIO hardware, might
>> help eliminate confusion. 
> 
> Maybe not the best name in theory but at least would show that it
> relates to existing ioremap would be pci_ioremap()


Easy enough... 'pcimap' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2495 bytes --]

commit 6e09c71822f76c618353682bf295fc7588284521
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Sep 18 19:06:08 2007 -0400

    Add pci_ioremap() to generic iomap lib.
    
    (arches that don't wish to use lib/iomap.c's version may fill in their own)
    
    Signed-off-by: Jeff Garzik <jeff@garzik.org>

 include/asm-generic/iomap.h |    1 +
 lib/iomap.c                 |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

6e09c71822f76c618353682bf295fc7588284521
diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index cde592f..611e6cf 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -63,6 +63,7 @@ extern void ioport_unmap(void __iomem *);
 /* 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 __iomem *pci_ioremap(struct pci_dev *dev, int bar, unsigned long max);
 extern void pci_iounmap(struct pci_dev *dev, void __iomem *);
 
 #endif
diff --git a/lib/iomap.c b/lib/iomap.c
index 864f2ec..0338da0 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -275,9 +275,43 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
 	return NULL;
 }
 
+/**
+ * pci_ioremap - create a virtual mapping cookie for a memory-based PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @maxlen: length of MMIO memory to map
+ *
+ * Using this function you will get a __iomem address to your device BAR.
+ * You can access it using read*() and write*().
+ *
+ * @maxlen specifies the maximum length to map. If you want to get access to
+ * the complete BAR without checking for its length first, pass %0 here.
+ * */
+void __iomem *pci_ioremap(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+	unsigned long start = pci_resource_start(dev, bar);
+	unsigned long len = pci_resource_len(dev, bar);
+	unsigned long flags = pci_resource_flags(dev, bar);
+
+	if (!len || !start)
+		return NULL;
+	if (maxlen && len > maxlen)
+		len = maxlen;
+	if (flags & IORESOURCE_IO)
+		return NULL;
+	if (flags & IORESOURCE_MEM) {
+		if (flags & IORESOURCE_CACHEABLE)
+			return ioremap(start, len);
+		return ioremap_nocache(start, len);
+	}
+	/* What? */
+	return NULL;
+}
+
 void pci_iounmap(struct pci_dev *dev, void __iomem * addr)
 {
 	IO_COND(addr, /* nothing */, iounmap(addr));
 }
 EXPORT_SYMBOL(pci_iomap);
+EXPORT_SYMBOL(pci_ioremap);
 EXPORT_SYMBOL(pci_iounmap);

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Linus Torvalds]

On Tue, 18 Sep 2007, Jeff Garzik wrote:
> 
> Easy enough... 'pcimap' branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git

This is wrong.

You must not put it in lib/iomap.c, since that file is only compiled for 
architectures that use CONFIG_GENERIC_IOMAP.

So you need to put it in some *generic* PCI place, like drivers/pci/pci.c 
or similar.

		Linus

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Benjamin Herrenschmidt]

On Tue, 2007-09-18 at 12:03 -0700, Linus Torvalds wrote:
> 
> On Tue, 18 Sep 2007, Luis R. Rodriguez wrote:
> > 
> > Alright, here is the same patch inline with s/recommended/required/ language:
> 
> Well, the thing is, I'm not at all sure that I agree with this.
> 
> If you use ioport_map/unmap, then you really *should* access them with the 
> proper iomem accessors (ioread/iowrite). The fact that it may happen to 
> work (when using the default lib/iomap.c implementation, at least) on 
> some architectures and with the current implementation still doesn't mean 
> that you should necessarily use readb/writeb.
> 
> After all, you cannot use "inb/outb" on it, even if would happen to be an 
> IO address.
> 
> So what is this usage that wants to use the bogus accessor? Why not fix 
> that instead of adding documentation for something that is very arguably 
> something we want to *avoid* having people do!

Agreed.

ioremap -> readb/writeb
ioport_map/pci_iomap -> ioread/iowrite

and nothing else should be allowed.

Cheers,
Ben.

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Alan Cox]

> If you use ioport_map/unmap, then you really *should* access them with the 
> proper iomem accessors (ioread/iowrite). The fact that it may happen to 
> work (when using the default lib/iomap.c implementation, at least) on 
> some architectures and with the current implementation still doesn't mean 
> that you should necessarily use readb/writeb.

Another reason we should enforce the use of ioread/iowrite is that some
platforms do daft things like map mmio type devices through indirect
register access. Sparc PCMCIA is a classic example. The only sane way to
make these work is to require ioread/iowrite is used with iomap

Re: [PATCH] Clarify pci_iomap() usage for MMIO-only devices

[Nick Kossifidis]

2007/9/19, Alan Cox <alan@lxorguk.ukuu.org.uk>:
> > If you use ioport_map/unmap, then you really *should* access them with the
> > proper iomem accessors (ioread/iowrite). The fact that it may happen to
> > work (when using the default lib/iomap.c implementation, at least) on
> > some architectures and with the current implementation still doesn't mean
> > that you should necessarily use readb/writeb.
>
> Another reason we should enforce the use of ioread/iowrite is that some
> platforms do daft things like map mmio type devices through indirect
> register access. Sparc PCMCIA is a classic example. The only sane way to
> make these work is to require ioread/iowrite is used with iomap
>

Hmm, we 've had bug reports about driver (madwifi-old-openhal) not
working on Sparc64, can someone test if this is the case (the fact
that we used readl/writel instead of ioread/iowrite on
madwifi-old-openhal) ? It was really weird because we handle
big-endian machines ok (madwifi-old-openhal on ppc worked fine).

(ath5k was based on madwifi-old-openhal)

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick