[PATCH 1/9] ARM i.MX51: Add ipu clock support

Russell King - ARM Linux linux at arm.linux.org.uk
Wed Dec 15 12:12:46 EST 2010


On Wed, Dec 15, 2010 at 05:49:59PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 December 2010, Russell King - ARM Linux wrote:
> > Lastly, I don't see where atomicity comes into it - __raw_writel vs writel
> > have the same atomicity.  Both are single access atomic provided they're
> > naturally aligned.  Misaligned device accesses are not predictable.
> 
> This is just what gcc turns it into today. In theory, a future gcc or
> a future cpu might change that. If you mark a pointer as
> '__attribute__((packed))', it probably already does, even for aligned
> pointers, while it does not when using writel_{,relaxed}. The point is
> that __raw_* means just that -- we don't give any guarantees on what
> happens on the bus, so people should not use it.

No.  It does give guarantees on what happens on the bus.  The kind of
pointer you pass in has no bearing on what happens on the bus because it's
casted away immediately.

__raw_writel(v, ptr) doesn't just do *(ptr) = v - that doesn't work when
ptr is void.  Instead, it has to do a cast to ensure that we have a valid
C dereference (void pointers are illegal to dereference):

#define __raw_writel(v,a)  (__chk_io_ptr(a), \
	*(volatile unsigned int __force   *)(a) = (v))

Doesn't matter if 'a' was marked as packed or not - that's all casted away.
That's not something that should ever change - otherwise we'll all be
screwed as you could never cast away pointer attributes.

It _may_ possible that the compiler decides that accessing an 'unsigned int'
will not be done as a single word load, in which case we have exactly the
same problems with writel() too.

And in any case, if __raw_writel() was as you're suggesting, then it would
serve absolutely no purpose at all.



More information about the linux-arm-kernel mailing list