On Fri, Mar 02, 2018 at 09:54:11AM -0500, David Miller wrote:
> From: Pavel Machek <pa...@ucw.cz>
> Date: Fri, 2 Mar 2018 10:20:00 +0100

Hello Pavel, David

> >> This barrier cannot be a simple dma_wmb(), since a dma_wmb() is only
> >> used to guarantee the ordering, with respect to other writes,
> >> to cache coherent DMA memory.
> > 
> > Could you explain this a bit more (and perhaps in code comment)?

Have a look at:


AFAICT, a dma_wmb() can only be used to guarantee that the
writes to cache coherent memory (e.g. memory allocated with
dma_alloc_coherent()) before the dma_wmb() will be performed
before the writes to cache coherent memory after the dma_wmb().

Since most of our writes are simply writing new buffer addresses
and sizes to TDES0/TDES1/TDES2/TDES3, and since these TX DMA
descriptors have been allocated with dma_alloc_coherent(),
a dma_wmb() should be enough to e.g. make sure that TDES3
(which contains the OWN bit), is written after the writes to

However, the last write we do is "DMA start transmission",
this is a register in the IP, i.e. it is a write to the cache
incoherent MMIO region (rather than a write to cache coherent memory).
To ensure that all writes to cache coherent memory have
completed before we start the DMA, we have to use the barrier
wmb() (which performs a more extensive flush compared to

So the only place where we have to use a wmb() instead
of a dma_wmb() is where we have a write to coherent memory,
followed by a write to cache incoherent MMIO.
The only obvious place where we have this situtation is
where we write the OWN bit immediately followed by a write
to the "DMA start transmission" register.

Note that this also matches how it's done in other other drivers:

There is already a comment describing the barrier in
stmmac_xmit() and stmmac_tso_xmit() that says:
/* The own bit must be the latest setting done when prepare the
 * descriptor and then barrier is needed to make sure that
 * all is coherent before granting the DMA engine.
However, if you want, we could mention wmb() explicitly in this comment.

> > 
> > Ensuring other writes are done before writing the "GO!" bit should be
> > enough, no?
> Indeed, the chip should never look at the descriptor contents unless
> the GO bit is set.
> If there are ways that it can, this must be explained and documented
> since it is quite unusual compared to other hardware.
> > (If it is not, do we need heavier barriers in other places, too?)
> Right.

I hope that my explaination above has cleared any potential confusion.

Best regards,

Reply via email to