Is in_le32 and out_le32 atomic?

Is in_le32 and out_le32 atomic?

[Wolfgang Grandegger]

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.

Re: Is in_le32 and out_le32 atomic?

[Arnd Bergmann]

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 <><

Re: Is in_le32 and out_le32 atomic?

[Benjamin Herrenschmidt]

> 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.

Re: Is in_le32 and out_le32 atomic?

[Benjamin Herrenschmidt]

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.

Ben.

Re: Is in_le32 and out_le32 atomic?

[Wolfgang Grandegger]

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.

Re: Is in_le32 and out_le32 atomic?

[Benjamin Herrenschmidt]

> 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.