On Friday 18 February 2011 10:52:12 Guan Xuetao wrote:

quoted

On Wednesday 16 February 2011, Guan Xuetao wrote:

quoted

+#define io_p2v(x)      ((x) - PKUNITY_IOSPACE_BASE)
+#define io_v2p(x)      ((x) + PKUNITY_IOSPACE_BASE)
+
+#ifndef __ASSEMBLY__
+
+# define __REG(x)      (*((volatile unsigned long *)io_p2v(x)))
+# define __PREG(x)     (io_v2p((unsigned long)&(x)))
+
+#else
+
+# define __REG(x)      io_p2v(x)
+# define __PREG(x)     io_v2p(x)

quoted

#define PKUNITY_IOSPACE_BASE            0x80000000 /* 0x80000000 - 0xFFFFFFFF 2GB */

The typecasts look wrong here:

- "volatile unsigned long*" is not the right pointer type to do I/O on. It
  should instead be "void __iomem *". Please use the "sparse" tool with "make C=1"
  to get warnings about incorrect pointer type accesses.

__REG() macro could be used in both left and right sides of assignment sentence.
This idea is borrowed from arm/sa1100.

When used in left side, __REG is a register port, and when used in right side, 
__REG is just the value of the register.
It is a trick, but very useful. I'd like to remain the macros.

 
Ok, I didn't realize this. However, this is a really bad idea
and I would strongly advise removing this throughout the code.

The only way we do MMIO accesses in Linux is through the
readl()/writel() functions. On many architectures, these
contain more than just a pointer dereference, and if you
need that, you can change it in just one place.

Things that can go wrong with volatile pointer dereferences include:

* compiler turns 32 bit access into four 8-bit accesses
* I/O bus reorders access and it moves outside of a spinlock
* non-volatile accesses to system RAM get reordered around
  MMIO access, which may break if the MMIO triggers a DMA.

This change is ok.

The result of io_p2v really needs to be a (void __iomem *) pointer, not
an integer, so the accesses can work.

 # define __REG(x)	(*((volatile unsigned long *)io_p2v(x)))
 # define __PREG(x)	(io_v2p((unsigned long)&(x)))

And as explained, these should be removed and all users converted to
use readl()/writel().

	Arnd