| Message ID | 201106101731.08578.hartleys@visionengravers.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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.
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
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).
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
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> ---