Re: Is in_le32 and out_le32 atomic?

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.