[PATCH] fb.h: ARM uses __raw_{read/write}

[PATCH] fb.h: ARM uses __raw_{read/write}

[H Hartley Sweeten]

ARM provides __raw_{read/write}* functions for memory access. These
should be used instead of the default '(*(volatile' stuff to make sure the
memory accesses are typesafe (void __iomem *).

This also fixes a number of sparse warning like:

  warning: cast removes address space of expression

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Russell King <linux@arm.linux.org.uk>

---

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 6a82748..a040e92e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -937,7 +937,7 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
 #define fb_memcpy_fromfb sbus_memcpy_fromio
 #define fb_memcpy_tofb sbus_memcpy_toio
 
-#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__)
+#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__) || defined(__arm__)
 
 #define fb_readb __raw_readb
 #define fb_readw __raw_readw

[PATCH] fb.h: ARM uses __raw_{read/write}

[H Hartley Sweeten]

ARM provides __raw_{read/write}* functions for memory access. These
should be used instead of the default '(*(volatile' stuff to make sure the
memory accesses are typesafe (void __iomem *).

This also fixes a number of sparse warning like:

  warning: cast removes address space of expression

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Russell King <linux@arm.linux.org.uk>

---

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 6a82748..a040e92e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -937,7 +937,7 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
 #define fb_memcpy_fromfb sbus_memcpy_fromio
 #define fb_memcpy_tofb sbus_memcpy_toio
 
-#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__)
+#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__) || defined(__arm__)
 
 #define fb_readb __raw_readb
 #define fb_readw __raw_readw

[PATCH] fb.h: ARM uses __raw_{read/write}

[H Hartley Sweeten]

ARM provides __raw_{read/write}* functions for memory access. These
should be used instead of the default '(*(volatile' stuff to make sure the
memory accesses are typesafe (void __iomem *).

This also fixes a number of sparse warning like:

  warning: cast removes address space of expression

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Russell King <linux@arm.linux.org.uk>

---

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 6a82748..a040e92e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -937,7 +937,7 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
 #define fb_memcpy_fromfb sbus_memcpy_fromio
 #define fb_memcpy_tofb sbus_memcpy_toio
 
-#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__)
+#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__) || defined(__arm__)
 
 #define fb_readb __raw_readb
 #define fb_readw __raw_readw

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Paul Mundt]

On Fri, Jun 10, 2011 at 05:31:08PM -0700, H Hartley Sweeten wrote:
> ARM provides __raw_{read/write}* functions for memory access. These
> should be used instead of the default '(*(volatile' stuff to make sure the
> memory accesses are typesafe (void __iomem *).
> 
> This also fixes a number of sparse warning like:
> 
>   warning: cast removes address space of expression
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> 
I'm not sure what semantics are desirable for ARM here, so I'll wait for
Russell to reply.

This wrapping will basically mean that the fb_read/write ops are using
__raw_xxx variants while the memset and memcpy wrappers will be using the
regular read/write[bwl] routines which contain __iormb() calls. Given
that ioread/write and friends all wrap in to the normal versions with the
barriers, I would suppose that this is the default behaviour that is
desired, as opposed to wrapping in to the __raw_xxx variants directly.

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Paul Mundt]

On Fri, Jun 10, 2011 at 05:31:08PM -0700, H Hartley Sweeten wrote:
> ARM provides __raw_{read/write}* functions for memory access. These
> should be used instead of the default '(*(volatile' stuff to make sure the
> memory accesses are typesafe (void __iomem *).
> 
> This also fixes a number of sparse warning like:
> 
>   warning: cast removes address space of expression
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> 
I'm not sure what semantics are desirable for ARM here, so I'll wait for
Russell to reply.

This wrapping will basically mean that the fb_read/write ops are using
__raw_xxx variants while the memset and memcpy wrappers will be using the
regular read/write[bwl] routines which contain __iormb() calls. Given
that ioread/write and friends all wrap in to the normal versions with the
barriers, I would suppose that this is the default behaviour that is
desired, as opposed to wrapping in to the __raw_xxx variants directly.

[PATCH] fb.h: ARM uses __raw_{read/write}

[Paul Mundt]

On Fri, Jun 10, 2011 at 05:31:08PM -0700, H Hartley Sweeten wrote:
> ARM provides __raw_{read/write}* functions for memory access. These
> should be used instead of the default '(*(volatile' stuff to make sure the
> memory accesses are typesafe (void __iomem *).
> 
> This also fixes a number of sparse warning like:
> 
>   warning: cast removes address space of expression
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> 
I'm not sure what semantics are desirable for ARM here, so I'll wait for
Russell to reply.

This wrapping will basically mean that the fb_read/write ops are using
__raw_xxx variants while the memset and memcpy wrappers will be using the
regular read/write[bwl] routines which contain __iormb() calls. Given
that ioread/write and friends all wrap in to the normal versions with the
barriers, I would suppose that this is the default behaviour that is
desired, as opposed to wrapping in to the __raw_xxx variants directly.

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Catalin Marinas]

On Mon, Jun 13, 2011 at 01:00:04PM +0900, Paul Mundt wrote:
> On Fri, Jun 10, 2011 at 05:31:08PM -0700, H Hartley Sweeten wrote:
> > ARM provides __raw_{read/write}* functions for memory access. These
> > should be used instead of the default '(*(volatile' stuff to make sure the
> > memory accesses are typesafe (void __iomem *).
> > 
> > This also fixes a number of sparse warning like:
> > 
> >   warning: cast removes address space of expression
> > 
> > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> > Cc: Paul Mundt <lethal@linux-sh.org>
> > Cc: Russell King <linux@arm.linux.org.uk>
> 
> I'm not sure what semantics are desirable for ARM here, so I'll wait for
> Russell to reply.
> 
> This wrapping will basically mean that the fb_read/write ops are using
> __raw_xxx variants while the memset and memcpy wrappers will be using the
> regular read/write[bwl] routines which contain __iormb() calls. Given
> that ioread/write and friends all wrap in to the normal versions with the
> barriers, I would suppose that this is the default behaviour that is
> desired, as opposed to wrapping in to the __raw_xxx variants directly.

The intention for the __iormb/__iowmb calls is in relation to DMA
transfers where a buffer in normal RAM is filled in with data and the
transfer started by a writel() to a device. We need to make sure that
the normal RAM writing completes before the writel().

The change proposed by Hartley wouldn't make much difference from the
current volatile accesses (__raw_* accessors are implemented as volatile
on ARM).

I think the memcpy_(from|to)io could be optimised on ARM to only add a
barrier before or after he copying loop.

-- 
Catalin

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Catalin Marinas]

On Mon, Jun 13, 2011 at 01:00:04PM +0900, Paul Mundt wrote:
> On Fri, Jun 10, 2011 at 05:31:08PM -0700, H Hartley Sweeten wrote:
> > ARM provides __raw_{read/write}* functions for memory access. These
> > should be used instead of the default '(*(volatile' stuff to make sure the
> > memory accesses are typesafe (void __iomem *).
> > 
> > This also fixes a number of sparse warning like:
> > 
> >   warning: cast removes address space of expression
> > 
> > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> > Cc: Paul Mundt <lethal@linux-sh.org>
> > Cc: Russell King <linux@arm.linux.org.uk>
> 
> I'm not sure what semantics are desirable for ARM here, so I'll wait for
> Russell to reply.
> 
> This wrapping will basically mean that the fb_read/write ops are using
> __raw_xxx variants while the memset and memcpy wrappers will be using the
> regular read/write[bwl] routines which contain __iormb() calls. Given
> that ioread/write and friends all wrap in to the normal versions with the
> barriers, I would suppose that this is the default behaviour that is
> desired, as opposed to wrapping in to the __raw_xxx variants directly.

The intention for the __iormb/__iowmb calls is in relation to DMA
transfers where a buffer in normal RAM is filled in with data and the
transfer started by a writel() to a device. We need to make sure that
the normal RAM writing completes before the writel().

The change proposed by Hartley wouldn't make much difference from the
current volatile accesses (__raw_* accessors are implemented as volatile
on ARM).

I think the memcpy_(from|to)io could be optimised on ARM to only add a
barrier before or after he copying loop.

-- 
Catalin

[PATCH] fb.h: ARM uses __raw_{read/write}

[Catalin Marinas]

On Mon, Jun 13, 2011 at 01:00:04PM +0900, Paul Mundt wrote:
> On Fri, Jun 10, 2011 at 05:31:08PM -0700, H Hartley Sweeten wrote:
> > ARM provides __raw_{read/write}* functions for memory access. These
> > should be used instead of the default '(*(volatile' stuff to make sure the
> > memory accesses are typesafe (void __iomem *).
> > 
> > This also fixes a number of sparse warning like:
> > 
> >   warning: cast removes address space of expression
> > 
> > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> > Cc: Paul Mundt <lethal@linux-sh.org>
> > Cc: Russell King <linux@arm.linux.org.uk>
> 
> I'm not sure what semantics are desirable for ARM here, so I'll wait for
> Russell to reply.
> 
> This wrapping will basically mean that the fb_read/write ops are using
> __raw_xxx variants while the memset and memcpy wrappers will be using the
> regular read/write[bwl] routines which contain __iormb() calls. Given
> that ioread/write and friends all wrap in to the normal versions with the
> barriers, I would suppose that this is the default behaviour that is
> desired, as opposed to wrapping in to the __raw_xxx variants directly.

The intention for the __iormb/__iowmb calls is in relation to DMA
transfers where a buffer in normal RAM is filled in with data and the
transfer started by a writel() to a device. We need to make sure that
the normal RAM writing completes before the writel().

The change proposed by Hartley wouldn't make much difference from the
current volatile accesses (__raw_* accessors are implemented as volatile
on ARM).

I think the memcpy_(from|to)io could be optimised on ARM to only add a
barrier before or after he copying loop.

-- 
Catalin

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Arnd Bergmann]

On Monday 13 June 2011 12:08:19 Catalin Marinas wrote:
> The change proposed by Hartley wouldn't make much difference from the
> current volatile accesses (__raw_* accessors are implemented as volatile
> on ARM).

I guess it would mainly make a difference if we get a platform where
the PCI bus window is not in the same address space as the regular
physical memory and the accessors actually need to do a computation
on the address. AFAICT, ARM does not currently have any such platform
and I would hope that it says that way.

> I think the memcpy_(from|to)io could be optimised on ARM to only add a
> barrier before or after he copying loop.

Agreed, good idea.

	Arnd

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Arnd Bergmann]

On Monday 13 June 2011 12:08:19 Catalin Marinas wrote:
> The change proposed by Hartley wouldn't make much difference from the
> current volatile accesses (__raw_* accessors are implemented as volatile
> on ARM).

I guess it would mainly make a difference if we get a platform where
the PCI bus window is not in the same address space as the regular
physical memory and the accessors actually need to do a computation
on the address. AFAICT, ARM does not currently have any such platform
and I would hope that it says that way.

> I think the memcpy_(from|to)io could be optimised on ARM to only add a
> barrier before or after he copying loop.

Agreed, good idea.

	Arnd

[PATCH] fb.h: ARM uses __raw_{read/write}

[Arnd Bergmann]

On Monday 13 June 2011 12:08:19 Catalin Marinas wrote:
> The change proposed by Hartley wouldn't make much difference from the
> current volatile accesses (__raw_* accessors are implemented as volatile
> on ARM).

I guess it would mainly make a difference if we get a platform where
the PCI bus window is not in the same address space as the regular
physical memory and the accessors actually need to do a computation
on the address. AFAICT, ARM does not currently have any such platform
and I would hope that it says that way.

> I think the memcpy_(from|to)io could be optimised on ARM to only add a
> barrier before or after he copying loop.

Agreed, good idea.

	Arnd

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Russell King - ARM Linux]

On Mon, Jun 13, 2011 at 03:37:10PM +0200, Arnd Bergmann wrote:
> On Monday 13 June 2011 12:08:19 Catalin Marinas wrote:
> > The change proposed by Hartley wouldn't make much difference from the
> > current volatile accesses (__raw_* accessors are implemented as volatile
> > on ARM).
> 
> I guess it would mainly make a difference if we get a platform where
> the PCI bus window is not in the same address space as the regular
> physical memory and the accessors actually need to do a computation
> on the address. AFAICT, ARM does not currently have any such platform
> and I would hope that it says that way.

Actually we do.

Footbridge has this situation: PCI memory space is 0 to 4GB, and appears
in physical space at 2GB-4GB physical, and can be switched between the
low and high 2GB of PCI space.

Non-PCI peripherals are located at 0 to 2GB physical.

We handle this by having pcibios_bus_to_resource() and
pcibios_resource_to_bus(), which translates from the bus space to a
cookie suitable for ioremap() and /proc/iomem.  This cookie happens to
be the physical address.

We avoid the obvious problem with the upper 2GB of PCI memory space by
placing the system RAM at 0xe0000000 on the PCI bus, and not allocating
any peripherals in the upper 2GB of PCI memory space.

So, I rather wish that people would stop saying that "ioremap takes the
PCI bus address".  It doesn't - it takes a platform specific cookie to
allow a mapping to be setup, which in the case of pci will be a cookie
provided from the PCI bus address by the pcibios_bus_to_resource()
platform code.

And the final thing to note on this is that fbmem/fbcon does work - see
the Cyber2000/CyberPro VGA driver which has been - and still is - used
on Netwinder (footbridge).

Re: [PATCH] fb.h: ARM uses __raw_{read/write}

[Russell King - ARM Linux]

On Mon, Jun 13, 2011 at 03:37:10PM +0200, Arnd Bergmann wrote:
> On Monday 13 June 2011 12:08:19 Catalin Marinas wrote:
> > The change proposed by Hartley wouldn't make much difference from the
> > current volatile accesses (__raw_* accessors are implemented as volatile
> > on ARM).
> 
> I guess it would mainly make a difference if we get a platform where
> the PCI bus window is not in the same address space as the regular
> physical memory and the accessors actually need to do a computation
> on the address. AFAICT, ARM does not currently have any such platform
> and I would hope that it says that way.

Actually we do.

Footbridge has this situation: PCI memory space is 0 to 4GB, and appears
in physical space at 2GB-4GB physical, and can be switched between the
low and high 2GB of PCI space.

Non-PCI peripherals are located at 0 to 2GB physical.

We handle this by having pcibios_bus_to_resource() and
pcibios_resource_to_bus(), which translates from the bus space to a
cookie suitable for ioremap() and /proc/iomem.  This cookie happens to
be the physical address.

We avoid the obvious problem with the upper 2GB of PCI memory space by
placing the system RAM at 0xe0000000 on the PCI bus, and not allocating
any peripherals in the upper 2GB of PCI memory space.

So, I rather wish that people would stop saying that "ioremap takes the
PCI bus address".  It doesn't - it takes a platform specific cookie to
allow a mapping to be setup, which in the case of pci will be a cookie
provided from the PCI bus address by the pcibios_bus_to_resource()
platform code.

And the final thing to note on this is that fbmem/fbcon does work - see
the Cyber2000/CyberPro VGA driver which has been - and still is - used
on Netwinder (footbridge).

[PATCH] fb.h: ARM uses __raw_{read/write}

[Russell King - ARM Linux]

On Mon, Jun 13, 2011 at 03:37:10PM +0200, Arnd Bergmann wrote:
> On Monday 13 June 2011 12:08:19 Catalin Marinas wrote:
> > The change proposed by Hartley wouldn't make much difference from the
> > current volatile accesses (__raw_* accessors are implemented as volatile
> > on ARM).
> 
> I guess it would mainly make a difference if we get a platform where
> the PCI bus window is not in the same address space as the regular
> physical memory and the accessors actually need to do a computation
> on the address. AFAICT, ARM does not currently have any such platform
> and I would hope that it says that way.

Actually we do.

Footbridge has this situation: PCI memory space is 0 to 4GB, and appears
in physical space at 2GB-4GB physical, and can be switched between the
low and high 2GB of PCI space.

Non-PCI peripherals are located at 0 to 2GB physical.

We handle this by having pcibios_bus_to_resource() and
pcibios_resource_to_bus(), which translates from the bus space to a
cookie suitable for ioremap() and /proc/iomem.  This cookie happens to
be the physical address.

We avoid the obvious problem with the upper 2GB of PCI memory space by
placing the system RAM at 0xe0000000 on the PCI bus, and not allocating
any peripherals in the upper 2GB of PCI memory space.

So, I rather wish that people would stop saying that "ioremap takes the
PCI bus address".  It doesn't - it takes a platform specific cookie to
allow a mapping to be setup, which in the case of pci will be a cookie
provided from the PCI bus address by the pcibios_bus_to_resource()
platform code.

And the final thing to note on this is that fbmem/fbcon does work - see
the Cyber2000/CyberPro VGA driver which has been - and still is - used
on Netwinder (footbridge).