[PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> > On Mon, 18 Feb 2013, Arnd Bergmann wrote:
> > 
> > Sorry, I'm not sure how best to describe it, and I don't have sufficient 
> > information myself. In any case on a block-diagram of sh73a0 SDHI devices 
> > aren't directly connected to a common super-highway bus, instead they are 
> > on a bus-splitter. One more thing I forgot to mention - this error has 
> > been observed on SMP.
> 
> Ah, I guess that explains it. If you have one CPU filling the buffer
> and another CPU reading it, you need an smp_wmb/wmp_rmb pair.
> 
> That case can easily happen with tmio interrupt handler in PIO
> mode.

Ok, I also managed to reproduce it on UP, and it definitely is related to 
MMC_CAP_POWER_OFF_CARD, but I'll have to do some more work on it. I asked 
Chris to hold back on this patch for now.

Thanks
Guennadi

> > I checked clock code under drivers/sh/clk, I don't see any relevant 
> > __raw_* operations there. One I do see, however, GIC code does use 
> > *_relaxed() accessors, but under raw spinlocks... Do those provide 
> > sufficient barriers?
> 
> A spinlock by definition contains an SMP barrier, which seems to be
> what is missing here. I don't know if that is enough here, because
> the spinlock only needs barriers to protect against other users
> behind the same spinlock.
> 
> In architecture independent code, any smp_wmb() on the writer side
> must be paired with an smp_rmb on the reader. You probably have the
> sufficient barriers on the side that initiates the I/O because
> that uses a dmb() in the MMIO instructions, but that is only
> by accident and not portable.
> 
> Also, in the uniprocessor case, you have more barriers than you
> need, which limits the performance. The best solution for
> both performance and readability would be to use readw_relaxed()
> and writew_relaxed() and manually add the barriers you actually
> need, i.e. two smp_rmb()/smp_wmb() in the PIO case, and a
> wmb() before you start a real DMA to the device and an rmb()
> after a DMA from the device is complete.
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/