Re: Is in_le32 and out_le32 atomic?

[Wolfgang Grandegger <wg@grandegger.com>]

Benjamin Herrenschmidt wrote:
> On Fri, 2006-12-08 at 21:15 +0100, Arnd Bergmann wrote:
>> 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.
> 
> 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.