diff mbox

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

Message ID 201106101731.08578.hartleys@visionengravers.com (mailing list archive)
State Under Review
Headers show

Commit Message

Hartley Sweeten June 11, 2011, 12:31 a.m. UTC
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>

---

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paul Mundt June 13, 2011, 4 a.m. UTC | #1
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Catalin Marinas June 13, 2011, 10:08 a.m. UTC | #2
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.
Arnd Bergmann June 13, 2011, 1:37 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux June 13, 2011, 1:57 p.m. UTC | #4
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).
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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