From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: dmaengine: stm32-dma: fix residue calculation in stm32-dma From: Vinod Koul Message-Id: <20190426121751.GC28103@vkoul-mobl> Date: Fri, 26 Apr 2019 17:47:51 +0530 To: Arnaud Pouliquen Cc: Dan Williams , Pierre-Yves MORDRET , linux-stm32@st-md-mailman.stormreply.com, linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org List-ID: SGkgQXJuYXVkLAoKU29ycnkgZm9yIGRlbGF5IGluIHJldmlldywgdGhlIGNvbmZlcmVuY2UgdHJh dmVsL3ZhY2F0aW9uIHBsYW4gZGVsYXllZAp0aGlzLgoKT24gMjctMDMtMTksIDEzOjIxLCBBcm5h dWQgUG91bGlxdWVuIHdyb3RlOgo+IER1cmluZyByZXNpZHVlIGNhbGN1bGF0aW9uLiB0aGUgRE1B IGNhbiBzd2l0Y2ggdG8gdGhlIG5leHQgc2cuIFdoZW4KPiB0aGlzIHJhY2UgY29uZGl0aW9uIG9j Y3VycywgdGhlIHJlc2lkdWUgcmV0dXJuZWQgdmFsdWUgaXMgbm90IHZhbGlkLgo+IEluZGVlZCB0 aGUgcG9zaXRpb24gaW4gdGhlIHNnIHJldHVybmVkIGJ5IHRoZSBoYXJkd2FyZSBpcyB0aGUgcG9z aXRpb24KPiBvZiB0aGUgbmV4dCBzZywgbm90IHRoZSBjdXJyZW50IHNnLgo+IFNvbHV0aW9uIGlz IHRvIGNoZWNrIHRoZSBzZyBhZnRlciB0aGUgY2FsY3VsYXRpb24gdG8gdmVyaWZ5IGl0Lgo+IElm IGEgdHJhbnNpdGlvbiBpcyBkZXRlY3RlZCB3ZSBjb25zaWRlciB0aGF0IHRoZSBETUEgaGFzIHN3 aXRjaGVkIHRvCj4gdGhlIGJlZ2lubmluZyBvZiBuZXh0IHNnLgoKTm93LCB0aGF0IHNvdW5kcyBs aWtlIGR1Y3QgdGFwZS4gV2h5IHNob3VsZCB3ZSBib3RoZXIgZG9pbmcgdGhhdC4KCkFsc28gbG9v a2luZyBiYWNrIGF0IHRoZSBzdG0zMl9kbWFfZGVzY19yZXNpZHVlKCkgYW5kIGNhbGxzIHRvIGl0 IGZyb20Kc3RtMzJfZG1hX3R4X3N0YXR1cygpIGFtIG5vdCBzdXJlIHdlIGFyZSBkb2luZyB0aGUg cmlnaHQgdGhpbmcKCndoeSBhcmUgd2UgbG9va2luZyBhdCBuZXh0X3NnIGhlcmUsIGNhbiB5b3Ug ZXhwbGFpbiBtZSB0aGF0IHBsZWFzZQoKPiAKPiBTaWduZWQtb2ZmLWJ5OiBBcm5hdWQgUG91bGlx dWVuIDxhcm5hdWQucG91bGlxdWVuQHN0LmNvbT4KPiBTaWduZWQtb2ZmLWJ5OiBQaWVycmUtWXZl cyBNT1JEUkVUIDxwaWVycmUteXZlcy5tb3JkcmV0QHN0LmNvbT4KPiAtLS0KPiAgZHJpdmVycy9k bWEvc3RtMzItZG1hLmMgfCA3MCArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrLS0tLS0tLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCA1NyBpbnNlcnRpb25zKCspLCAxMyBkZWxl dGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9kbWEvc3RtMzItZG1hLmMgYi9kcml2 ZXJzL2RtYS9zdG0zMi1kbWEuYwo+IGluZGV4IDQ5MDNhNDAuLjMwMzA5ZDIgMTAwNjQ0Cj4gLS0t IGEvZHJpdmVycy9kbWEvc3RtMzItZG1hLmMKPiArKysgYi9kcml2ZXJzL2RtYS9zdG0zMi1kbWEu Ywo+IEBAIC0xMDM4LDMzICsxMDM4LDc3IEBAIHN0YXRpYyB1MzIgc3RtMzJfZG1hX2dldF9yZW1h aW5pbmdfYnl0ZXMoc3RydWN0IHN0bTMyX2RtYV9jaGFuICpjaGFuKQo+ICAJcmV0dXJuIG5kdHIg PDwgd2lkdGg7Cj4gIH0KPiAgCj4gK3N0YXRpYyBib29sIHN0bTMyX2RtYV9pc19jdXJyZW50X3Nn KHN0cnVjdCBzdG0zMl9kbWFfY2hhbiAqY2hhbikKPiArewo+ICsJc3RydWN0IHN0bTMyX2RtYV9k ZXZpY2UgKmRtYWRldiA9IHN0bTMyX2RtYV9nZXRfZGV2KGNoYW4pOwo+ICsJc3RydWN0IHN0bTMy X2RtYV9zZ19yZXEgKnNnX3JlcTsKPiArCXUzMiBkbWFfc2NyLCBkbWFfc21hciwgaWQ7Cj4gKwo+ ICsJaWQgPSBjaGFuLT5pZDsKPiArCWRtYV9zY3IgPSBzdG0zMl9kbWFfcmVhZChkbWFkZXYsIFNU TTMyX0RNQV9TQ1IoaWQpKTsKPiArCj4gKwlpZiAoIShkbWFfc2NyICYgU1RNMzJfRE1BX1NDUl9E Qk0pKQo+ICsJCXJldHVybiB0cnVlOwo+ICsKPiArCXNnX3JlcSA9ICZjaGFuLT5kZXNjLT5zZ19y ZXFbY2hhbi0+bmV4dF9zZ107Cj4gKwo+ICsJaWYgKGRtYV9zY3IgJiBTVE0zMl9ETUFfU0NSX0NU KSB7Cj4gKwkJZG1hX3NtYXIgPSBzdG0zMl9kbWFfcmVhZChkbWFkZXYsIFNUTTMyX0RNQV9TTTBB UihpZCkpOwo+ICsJCXJldHVybiAoZG1hX3NtYXIgPT0gc2dfcmVxLT5jaGFuX3JlZy5kbWFfc20w YXIpOwo+ICsJfQo+ICsKPiArCWRtYV9zbWFyID0gc3RtMzJfZG1hX3JlYWQoZG1hZGV2LCBTVE0z Ml9ETUFfU00xQVIoaWQpKTsKPiArCj4gKwlyZXR1cm4gKGRtYV9zbWFyID09IHNnX3JlcS0+Y2hh bl9yZWcuZG1hX3NtMWFyKTsKPiArfQo+ICsKPiAgc3RhdGljIHNpemVfdCBzdG0zMl9kbWFfZGVz Y19yZXNpZHVlKHN0cnVjdCBzdG0zMl9kbWFfY2hhbiAqY2hhbiwKPiAgCQkJCSAgICAgc3RydWN0 IHN0bTMyX2RtYV9kZXNjICpkZXNjLAo+ICAJCQkJICAgICB1MzIgbmV4dF9zZykKPiAgewo+ICAJ dTMyIG1vZHVsbywgYnVyc3Rfc2l6ZTsKPiAtCXUzMiByZXNpZHVlID0gMDsKPiArCXUzMiByZXNp ZHVlOwo+ICsJdTMyIG5fc2cgPSBuZXh0X3NnOwo+ICsJc3RydWN0IHN0bTMyX2RtYV9zZ19yZXEg KnNnX3JlcSA9ICZjaGFuLT5kZXNjLT5zZ19yZXFbY2hhbi0+bmV4dF9zZ107Cj4gIAlpbnQgaTsK PiAgCj4gKwlyZXNpZHVlID0gc3RtMzJfZG1hX2dldF9yZW1haW5pbmdfYnl0ZXMoY2hhbik7Cj4g Kwo+ICAJLyoKPiAtCSAqIEluIGN5Y2xpYyBtb2RlLCBmb3IgdGhlIGxhc3QgcGVyaW9kLCByZXNp ZHVlID0gcmVtYWluaW5nIGJ5dGVzIGZyb20KPiAtCSAqIE5EVFIKPiArCSAqIENhbGN1bGF0ZSB0 aGUgcmVzaWR1ZSBtZWFucyBjb21wdXRlIHRoZSBkZXNjcmlwdG9ycwo+ICsJICogaW5mb3JtYXRp b246Cj4gKwkgKiAtIHRoZSBzZyBjdXJyZW50bHkgdHJhbnNmZXJyZWQKPiArCSAqIC0gdGhlIHJl bWFpbmluZyBwb3NpdGlvbiBpbiB0aGlzIHNnIChORFRSKS4KPiArCSAqCj4gKwkgKiBUaGUgaXNz dWUgaXMgdGhhdCBhIHJhY2UgY29uZGl0aW9uIGNhbiBvY2N1ciBpZiBETUEgaXMKPiArCSAqIHJ1 bm5pbmcuIERNQSBjYW4gaGF2ZSBzdGFydGVkIHRvIHRyYW5zZmVyIHRoZSBuZXh0IHNnIGJlZm9y ZQo+ICsJICogdGhlIHBvc2l0aW9uIGluIHNnIGlzIHJlYWQuIEluIHRoaXMgY2FzZSB0aGUgcmVt YWluZyBwb3NpdGlvbgo+ICsJICogY2FuIGNvcnJlc3BvbmQgdG8gdGhlIG5ldyBzZyBwb3NpdGlv bi4KPiArCSAqIFRoZSBzdHJhdGVneSBpbXBsZW1lbnRlZCBpbiB0aGUgc3RtMzIgZHJpdmVyIGlz IHRvIGNoZWNrIHRoZQo+ICsJICogc2cgdHJhbnNpdGlvbi4gSWYgZGV0ZWN0ZWQgd2UgY2FuIG5v dCB0cnVzdCB0aGUgU3hORFRSIHJlZ2lzdGVyCj4gKwkgKiB2YWx1ZSwgdGhpcyByZWdpc3RlciBj YW4gbm90IGJlIHVwIHRvIGRhdGUgZHVyaW5nIHRoZSB0cmFuc2l0aW9uLgo+ICsJICogSW4gdGhp cyBjYXNlIHdlIGNhbiBhc3N1bWUgdGhhdCB0aGUgZG1hIGlzIGF0IHRoZSBiZWdpbm5pbmcgb2Yg bmV4dAo+ICsJICogc2cgc28gd2UgY2FsY3VsYXRlIHRoZSByZXNpZHVlIGluIGNvbnNlcXVlbmNl Lgo+ICAJICovCj4gLQlpZiAoY2hhbi0+ZGVzYy0+Y3ljbGljICYmIG5leHRfc2cgPT0gMCkgewo+ IC0JCXJlc2lkdWUgPSBzdG0zMl9kbWFfZ2V0X3JlbWFpbmluZ19ieXRlcyhjaGFuKTsKPiAtCQln b3RvIGVuZDsKPiArCj4gKwlpZiAoIXN0bTMyX2RtYV9pc19jdXJyZW50X3NnKGNoYW4pKSB7Cj4g KwkJbl9zZysrOwo+ICsJCWlmIChuX3NnID09IGNoYW4tPmRlc2MtPm51bV9zZ3MpCj4gKwkJCW5f c2cgPSAwOwo+ICsJCXJlc2lkdWUgPSBzZ19yZXEtPmxlbjsKPiAgCX0KPiAgCj4gIAkvKgo+IC0J ICogRm9yIGFsbCBvdGhlciBwZXJpb2RzIGluIGN5Y2xpYyBtb2RlLCBhbmQgaW4gc2cgbW9kZSwK PiAtCSAqIHJlc2lkdWUgPSByZW1haW5pbmcgYnl0ZXMgZnJvbSBORFRSICsgcmVtYWluaW5nIHBl cmlvZHMvc2cgdG8gYmUKPiAtCSAqIHRyYW5zZmVycmVkCj4gKwkgKiBJbiBjeWNsaWMgbW9kZSwg Zm9yIHRoZSBsYXN0IHBlcmlvZCwgcmVzaWR1ZSA9IHJlbWFpbmluZyBieXRlcwo+ICsJICogZnJv bSBORFRSLAo+ICsJICogZWxzZSBmb3IgYWxsIG90aGVyIHBlcmlvZHMgaW4gY3ljbGljIG1vZGUs IGFuZCBpbiBzZyBtb2RlLAo+ICsJICogcmVzaWR1ZSA9IHJlbWFpbmluZyBieXRlcyBmcm9tIE5E VFIgKyByZW1haW5pbmcKPiArCSAqIHBlcmlvZHMvc2cgdG8gYmUgdHJhbnNmZXJyZWQKPiAgCSAq Lwo+IC0JZm9yIChpID0gbmV4dF9zZzsgaSA8IGRlc2MtPm51bV9zZ3M7IGkrKykKPiAtCQlyZXNp ZHVlICs9IGRlc2MtPnNnX3JlcVtpXS5sZW47Cj4gLQlyZXNpZHVlICs9IHN0bTMyX2RtYV9nZXRf cmVtYWluaW5nX2J5dGVzKGNoYW4pOwo+ICsJaWYgKCFjaGFuLT5kZXNjLT5jeWNsaWMgfHwgbl9z ZyAhPSAwKQo+ICsJCWZvciAoaSA9IG5fc2c7IGkgPCBkZXNjLT5udW1fc2dzOyBpKyspCj4gKwkJ CXJlc2lkdWUgKz0gZGVzYy0+c2dfcmVxW2ldLmxlbjsKPiAgCj4gLWVuZDoKPiAgCWlmICghY2hh bi0+bWVtX2J1cnN0KQo+ICAJCXJldHVybiByZXNpZHVlOwo+ICAKPiAtLSAKPiAyLjcuNAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS, T_DKIMWL_WL_HIGH,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34CF5C43218 for ; Fri, 26 Apr 2019 12:18:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 065012089E for ; Fri, 26 Apr 2019 12:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556281083; bh=de4ui+tuZDXUFHeZfBrIGjZoQ8NU2gODJGhbz3Cpnjk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=k7QmQeXQC98b6Ue0BcJA+Ni9FIS3tDz0DT32Ps2wT5FiszFnHL+a4SLrrNl81trRl IMz4yns0WxhhHGZ33q+ibaqHlzl0QY7Eps92uToreHHmxpkiGL+rrH1W7aOetGX89q YeYdCYzlEtdn/UCRVSc9zmIa1KJqTET+vdv+jpg4= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726191AbfDZMR6 (ORCPT ); Fri, 26 Apr 2019 08:17:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:57504 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725901AbfDZMR6 (ORCPT ); Fri, 26 Apr 2019 08:17:58 -0400 Received: from localhost (unknown [171.76.113.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 73FCF2089E; Fri, 26 Apr 2019 12:17:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556281077; bh=de4ui+tuZDXUFHeZfBrIGjZoQ8NU2gODJGhbz3Cpnjk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=t0pB7TXqBuFjcpZPFv/22gI3vnaBkSJqd/a0HdtbiXx8m09Kaqn1uUEJ3GVwyMfdH JK80Bd0SMsJ5Vu8ZXnynkAN3+SPvc64/IokPcSYcGcq6KMNyETIM4xyzJJzDzM6QYI rNhtzv3TXhjKZYHwJN/JGf6y9nrvQHXsFp5RuwZc= Date: Fri, 26 Apr 2019 17:47:51 +0530 From: Vinod Koul To: Arnaud Pouliquen Cc: Dan Williams , Pierre-Yves MORDRET , linux-stm32@st-md-mailman.stormreply.com, linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org Subject: Re: [PATCH] dmaengine: stm32-dma: fix residue calculation in stm32-dma Message-ID: <20190426121751.GC28103@vkoul-mobl> References: <1553689316-6231-1-git-send-email-arnaud.pouliquen@st.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <1553689316-6231-1-git-send-email-arnaud.pouliquen@st.com> User-Agent: Mutt/1.11.3 (2019-02-01) Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org Message-ID: <20190426121751.qCjW3wvcH9D7AtuUEk5_uz1E7bwQZYrVRueqw7w7qVo@z> Hi Arnaud, Sorry for delay in review, the conference travel/vacation plan delayed this. On 27-03-19, 13:21, Arnaud Pouliquen wrote: > During residue calculation. the DMA can switch to the next sg. When > this race condition occurs, the residue returned value is not valid. > Indeed the position in the sg returned by the hardware is the position > of the next sg, not the current sg. > Solution is to check the sg after the calculation to verify it. > If a transition is detected we consider that the DMA has switched to > the beginning of next sg. Now, that sounds like duct tape. Why should we bother doing that. Also looking back at the stm32_dma_desc_residue() and calls to it from stm32_dma_tx_status() am not sure we are doing the right thing why are we looking at next_sg here, can you explain me that please > > Signed-off-by: Arnaud Pouliquen > Signed-off-by: Pierre-Yves MORDRET > --- > drivers/dma/stm32-dma.c | 70 ++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 57 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c > index 4903a40..30309d2 100644 > --- a/drivers/dma/stm32-dma.c > +++ b/drivers/dma/stm32-dma.c > @@ -1038,33 +1038,77 @@ static u32 stm32_dma_get_remaining_bytes(struct stm32_dma_chan *chan) > return ndtr << width; > } > > +static bool stm32_dma_is_current_sg(struct stm32_dma_chan *chan) > +{ > + struct stm32_dma_device *dmadev = stm32_dma_get_dev(chan); > + struct stm32_dma_sg_req *sg_req; > + u32 dma_scr, dma_smar, id; > + > + id = chan->id; > + dma_scr = stm32_dma_read(dmadev, STM32_DMA_SCR(id)); > + > + if (!(dma_scr & STM32_DMA_SCR_DBM)) > + return true; > + > + sg_req = &chan->desc->sg_req[chan->next_sg]; > + > + if (dma_scr & STM32_DMA_SCR_CT) { > + dma_smar = stm32_dma_read(dmadev, STM32_DMA_SM0AR(id)); > + return (dma_smar == sg_req->chan_reg.dma_sm0ar); > + } > + > + dma_smar = stm32_dma_read(dmadev, STM32_DMA_SM1AR(id)); > + > + return (dma_smar == sg_req->chan_reg.dma_sm1ar); > +} > + > static size_t stm32_dma_desc_residue(struct stm32_dma_chan *chan, > struct stm32_dma_desc *desc, > u32 next_sg) > { > u32 modulo, burst_size; > - u32 residue = 0; > + u32 residue; > + u32 n_sg = next_sg; > + struct stm32_dma_sg_req *sg_req = &chan->desc->sg_req[chan->next_sg]; > int i; > > + residue = stm32_dma_get_remaining_bytes(chan); > + > /* > - * In cyclic mode, for the last period, residue = remaining bytes from > - * NDTR > + * Calculate the residue means compute the descriptors > + * information: > + * - the sg currently transferred > + * - the remaining position in this sg (NDTR). > + * > + * The issue is that a race condition can occur if DMA is > + * running. DMA can have started to transfer the next sg before > + * the position in sg is read. In this case the remaing position > + * can correspond to the new sg position. > + * The strategy implemented in the stm32 driver is to check the > + * sg transition. If detected we can not trust the SxNDTR register > + * value, this register can not be up to date during the transition. > + * In this case we can assume that the dma is at the beginning of next > + * sg so we calculate the residue in consequence. > */ > - if (chan->desc->cyclic && next_sg == 0) { > - residue = stm32_dma_get_remaining_bytes(chan); > - goto end; > + > + if (!stm32_dma_is_current_sg(chan)) { > + n_sg++; > + if (n_sg == chan->desc->num_sgs) > + n_sg = 0; > + residue = sg_req->len; > } > > /* > - * For all other periods in cyclic mode, and in sg mode, > - * residue = remaining bytes from NDTR + remaining periods/sg to be > - * transferred > + * In cyclic mode, for the last period, residue = remaining bytes > + * from NDTR, > + * else for all other periods in cyclic mode, and in sg mode, > + * residue = remaining bytes from NDTR + remaining > + * periods/sg to be transferred > */ > - for (i = next_sg; i < desc->num_sgs; i++) > - residue += desc->sg_req[i].len; > - residue += stm32_dma_get_remaining_bytes(chan); > + if (!chan->desc->cyclic || n_sg != 0) > + for (i = n_sg; i < desc->num_sgs; i++) > + residue += desc->sg_req[i].len; > > -end: > if (!chan->mem_burst) > return residue; > > -- > 2.7.4 -- ~Vinod