Is in_le32 and out_le32 atomic?

Hello,

I'm puzzled about the following read and write funtions in 
include/asm-ppc/mv64x60.h:


/* Define I/O routines for accessing registers on the 64x60 bridge. */
extern inline void
mv64x60_write(struct mv64x60_handle *bh, u32 offset, u32 val) {
         ulong   flags;

         spin_lock_irqsave(&mv64x60_lock, flags);
         out_le32(bh->v_base + offset, val);
         spin_unlock_irqrestore(&mv64x60_lock, flags);
}

extern inline u32
mv64x60_read(struct mv64x60_handle *bh, u32 offset) {
         ulong   flags;
         u32     reg;

         spin_lock_irqsave(&mv64x60_lock, flags);
         reg = in_le32(bh->v_base + offset);
         spin_unlock_irqrestore(&mv64x60_lock, flags);
         return reg;
}

Can anybody tell me why the spin_* protection is needed? I thought that 
32-bit read and write operations are atomic.

TIA.

Wolfgang.

On Friday 08 December 2006 21:05, Wolfgang Grandegger wrote:

Can anybody tell me why the spin_* protection is needed? I thought that 
32-bit read and write operations are atomic.

The spinlocks are needed to guarantee ordering between the completion of
the i/o access and other code. A typical problem is that a store is
still on its way to the I/O device while the CPU has already left the
function that initiated it, and might call code that relies on the
value having arrived there.

	Arnd <><

Can anybody tell me why the spin_* protection is needed? I thought that 
32-bit read and write operations are atomic.

It doesn't seem to make sense indeed...

Ben.

On Fri, 2006-12-08 at 21:15 +0100, Arnd Bergmann wrote:

On Friday 08 December 2006 21:05, Wolfgang Grandegger wrote:

quoted

Can anybody tell me why the spin_* protection is needed? I thought that 
32-bit read and write operations are atomic.

The spinlocks are needed to guarantee ordering between the completion of
the i/o access and other code. A typical problem is that a store is
still on its way to the I/O device while the CPU has already left the
function that initiated it, and might call code that relies on the
value having arrived there.

That will not help much with the spinlock, especially not seeing how
they are used in the code.

I think the lock is totally spurrious in that case.

Ben.

Benjamin Herrenschmidt wrote:

On Fri, 2006-12-08 at 21:15 +0100, Arnd Bergmann wrote:

quoted

On Friday 08 December 2006 21:05, Wolfgang Grandegger wrote:

quoted

Can anybody tell me why the spin_* protection is needed? I thought that 
32-bit read and write operations are atomic.

The spinlocks are needed to guarantee ordering between the completion of
the i/o access and other code. A typical problem is that a store is
still on its way to the I/O device while the CPU has already left the
function that initiated it, and might call code that relies on the
value having arrived there.

That will not help much with the spinlock, especially not seeing how
they are used in the code.

I think the lock is totally spurrious in that case.

I just realized that there is also a mv64x60_modify function:

/* Define I/O routines for accessing registers on the 64x60 bridge. */
extern inline void
mv64x60_write(struct mv64x60_handle *bh, u32 offset, u32 val) {
         ulong   flags;

         spin_lock_irqsave(&mv64x60_lock, flags);
         out_le32(bh->v_base + offset, val);
         spin_unlock_irqrestore(&mv64x60_lock, flags);
}

extern inline u32
mv64x60_read(struct mv64x60_handle *bh, u32 offset) {
         ulong   flags;
         u32     reg;

         spin_lock_irqsave(&mv64x60_lock, flags);
         reg = in_le32(bh->v_base + offset);
         spin_unlock_irqrestore(&mv64x60_lock, flags);
         return reg;
}

extern inline void
mv64x60_modify(struct mv64x60_handle *bh, u32 offs, u32 data, u32 mask)
{
         u32     reg;
         ulong   flags;

         spin_lock_irqsave(&mv64x60_lock, flags);
         reg = in_le32(bh->v_base + offs) & (~mask);
         reg |= data & mask;
         out_le32(bh->v_base + offs, reg);
         spin_unlock_irqrestore(&mv64x60_lock, flags);
}

Then the spinlock makes sense avoiding the interruption of the 
subsequent read write accesses.

Sorry for the noise.

Wolfgang.

Then the spinlock makes sense avoiding the interruption of the 
subsequent read write accesses.

Sorry for the noise.

Ok, yes, it does make sense in that context then.

Ben.