On Wed, Jul 20, 2016 at 11:27 PM, Andy Green <a...@warmcat.com> wrote: > On July 21, 2016 1:22:02 PM GMT+08:00, John Stultz <john.stu...@linaro.org> > wrote: >>On Wed, Jul 20, 2016 at 9:26 PM, zhangfei <zhangfei....@linaro.org> >>wrote: >>> >>> >>> On 07/21/2016 11:53 AM, John Stultz wrote: >>>> >>>> After lots of debugging on an occasional DMA ERR issue, I realized >>>> that the desc structures which we point the dma hardware are being >>>> allocated out of regular memory. This means when we fill the desc >>>> structures, that data doesn't always get flushed out to memory by >>>> the time we start the dma transfer, resulting in the dma engine >>getting >>>> some null values, resulting in a DMA ERR on the first irq. >>> >>> >>> How about using wmb() flush before start dma to sync desc? >> >>So I'm not going to pretend to be an expert here, but my understanding >>is that wmb() syncrhonizes cpu write ordering operations across cpus, > > IIUI what the memory barrier does is tell the *compiler* to actually do any > writes that the code asked for, but which otherwise might actually be > deferred past that point. The compiler doesn't know that buffer area has > other hardware snooping it, so by default it feels it can play tricks with > what seems to it like just generally deferring spilling registers to memory. > wmb makes sure the compiler's pending writes actually happen right there. > (writel() etc definitions have one built-in, so they always do what you asked > when you asked). > > That can be of interest when syncing with other cores but also to other IPs > that intend to use the written-to area immediately, which is what's happening > here. Without the barrier the write may not be issued anywhere, even to > local cpu cache, until after the hw is asked to go read the buffer, > corrupting what the hw sees. > >>so the cpus see all the changes before the wmb() before they see any >>changes after. But I'm not sure what effect wmb() has across cpu >>cache to device ordering. I don't think it works as a cache flush to >>memory. >> >>Andy's patch introducing the cyclic support actually had a wmb() in it >>that I removed as I couldn't understand clearly why it was there (and >>there wasn't a comment explaining, as required by checkpatch :). But >>even with that wmb(), the DMA ERR was still seen. > > The rule about the comment is there partially because there's a general > tendancy for throwing voodoo smbs on broken things in case it helps. But > writing out memory descriptors that other hw is going to read is a legit use > for it I think. If the compiler you use wasn't deferring the write, you > won't notice any difference removing it, but that doesn't mean it shouldn't > be there. >
Though from your comments above, wouldn't using writel() instead of writel_relaxed(), followed by a wmb() be sufficient? thanks -john
