From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753966AbcGUT55 (ORCPT ); Thu, 21 Jul 2016 15:57:57 -0400 Received: from mail.warmcat.com ([163.172.24.82]:39882 "EHLO mail.warmcat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752077AbcGUT5x (ORCPT ); Thu, 21 Jul 2016 15:57:53 -0400 In-Reply-To: References: <1469073189-9167-1-git-send-email-john.stultz@linaro.org> <1469073189-9167-7-git-send-email-john.stultz@linaro.org> <57904F08.7020109@linaro.org> Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PATCH 6/7] k3dma: Fix occasional DMA ERR issue by using proper dma api From: Andy Green Date: Fri, 22 Jul 2016 03:57:24 +0800 To: John Stultz CC: zhangfei , lkml , Jingoo Han , Krzysztof Kozlowski , Maxime Ripard , Vinod Koul , Dan Williams , Mark Brown Message-ID: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On July 22, 2016 12:18:48 AM GMT+08:00, John Stultz wrote: >On Wed, Jul 20, 2016 at 11:27 PM, Andy Green wrote: >> On July 21, 2016 1:22:02 PM GMT+08:00, John Stultz > wrote: >>>On Wed, Jul 20, 2016 at 9:26 PM, zhangfei >>>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? Yes, since on Arm writel() sticks a wmb (ultimately a dsb instruction + outer_sync()) after every write it does. -Andy >thanks >-john