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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0312EC433EF for ; Tue, 12 Apr 2022 07:55:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=5QGC3tZ/zi4KL4Uns1H3pJxR+VbnQDhtGvnqGNYc+ac=; b=vJVjQhQdt3AKug BGYXB7sVoRfYTtSeuVucFSeykshvwSzfSqI4yY54SOyUb/hKUd2I9/dk6TlW22BJXfuqu8qqJDOTV HZ4zt/77QaTZD86PW0UyWXxfGYzqhmNHvGaJW3Pr85I3IRhX3DIHFO+g7BpaOps0SuU//kJZz4ENu Kh0M152EOUb5j/A0Vxm7VmCwT7FeEmnppNACOfxCYhRheYFVSebwNkSnWv6lUYPLzcX8TLrFGop4b FKuD7VR4FspENF3+fAeCz5dccCfFwINoHzodAvdYJFlvfKxtK+H3GFbWYSrJOkyZ+oIPOw6E1oDVY QDdRjS6H7yZXyJoSw5jw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1neBMc-00CMV7-Ds; Tue, 12 Apr 2022 07:55:02 +0000 Received: from relay9-d.mail.gandi.net ([217.70.183.199]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1neBGG-00CJNc-Uj for linux-mtd@lists.infradead.org; Tue, 12 Apr 2022 07:48:30 +0000 Received: (Authenticated sender: miquel.raynal@bootlin.com) by mail.gandi.net (Postfix) with ESMTPSA id 0A4B9FF80A; Tue, 12 Apr 2022 07:48:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1649749706; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YW/FpTIll39+9L5ULUaE9f07ZfIeCCQhSd38iCsA3pg=; b=aXzEGK9EGcOVOJifzQG1YR9RgJX9bx4SRppY92QDfZCpkF7UG+SuAE+Vt3fD+s4lUG9Dp/ 6E2GOvRJUD3BrQQRuyBjEG+aPdyB+l36E0p6D5FAv0+/lUp9U/0BO+UexyjYN4nbjbBKi3 yKTP2jtMkAMg4BZuEJtHoDGlf3stW/95B1L1ZFDcIx1wGTsTUjAmLnV+PLRNQbOc8s11D6 FHRkgZg4BlnxjYRczCsHCgpPCxz242Zd0t2mGEjZrB8JJvC4krFD1gtGPAUu6q1j90u+c/ 8BFlf51DjUxZS/ikaKKj/5jV85k1S3INssgYwm81Xq1Zsnt5wDKZwG/775U/ug== Date: Tue, 12 Apr 2022 09:48:22 +0200 From: Miquel Raynal To: Miaoqian Lin Cc: Richard Weinberger , Vignesh Raghavendra , Pratyush Yadav , Paul Cercueil , Alexandre Belloni , Bastian Hecht , Artem Bityutskiy , Guennadi Liakhovetski , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mtd: rawnand: Fix return value check of wait_for_completion_timeout Message-ID: <20220412094822.3818ebc2@xps13> In-Reply-To: <61783400-4df2-47af-78a7-7acb715c3a71@gmail.com> References: <202204121253.NcZifMQi-lkp@intel.com> <20220412063703.8537-1-linmq006@gmail.com> <20220412090649.33bb3f8b@xps13> <61783400-4df2-47af-78a7-7acb715c3a71@gmail.com> Organization: Bootlin X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220412_004829_297769_D582E347 X-CRM114-Status: GOOD ( 28.25 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org SGkgTWlhb3FpYW4sCgpsaW5tcTAwNkBnbWFpbC5jb20gd3JvdGUgb24gVHVlLCAxMiBBcHIgMjAy MiAxNTo0MjowMiArMDgwMDoKCj4gSGkgTWlxdWVsLAo+IAo+IFRoYW5rcyBmb3IgeW91ciByZXBs eS4KPiAKPiBPbiAyMDIyLzQvMTIgMTU6MDYsIE1pcXVlbCBSYXluYWwgd3JvdGU6Cj4gPiBIaSBN aWFvcWlhbiwKPiA+Cj4gPiBsaW5tcTAwNkBnbWFpbC5jb20gd3JvdGUgb24gVHVlLCAxMiBBcHIg MjAyMiAwNjozNjo1MiArMDAwMDoKPiA+ICAKPiA+PiB3YWl0X2Zvcl9jb21wbGV0aW9uX3RpbWVv dXQoKSByZXR1cm5zIHVuc2lnbmVkIGxvbmcgbm90IGludC4KPiA+PiBJdCByZXR1cm5zIDAgaWYg dGltZWQgb3V0LCBhbmQgcG9zaXRpdmUgaWYgY29tcGxldGVkLgo+ID4+IFRoZSBjaGVjayBmb3Ig PD0gMCBpcyBhbWJpZ3VvdXMgYW5kIHNob3VsZCBiZSA9PSAwIGhlcmUKPiA+PiBpbmRpY2F0aW5n IHRpbWVvdXQgd2hpY2ggaXMgdGhlIG9ubHkgZXJyb3IgY2FzZS4KPiA+Pgo+ID4+IEZpeGVzOiA4 MzczOGQ4N2UzYTAgKCJtdGQ6IHNoX2ZsY3RsOiBBZGQgRE1BIGNhcGFiaWx0eSIpCj4gPj4gU2ln bmVkLW9mZi1ieTogTWlhb3FpYW4gTGluIDxsaW5tcTAwNkBnbWFpbC5jb20+Cj4gPj4gLS0tCj4g Pj4gY2hhbmdlIGluIHYyOgo+ID4+IC0gaW5pdGlhbGl6ZSByZXQgdG8gMS4KPiA+PiAtLS0KPiA+ PiAgZHJpdmVycy9tdGQvbmFuZC9yYXcvc2hfZmxjdGwuYyB8IDggKysrKystLS0KPiA+PiAgMSBm aWxlIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkKPiA+Pgo+ID4+IGRp ZmYgLS1naXQgYS9kcml2ZXJzL210ZC9uYW5kL3Jhdy9zaF9mbGN0bC5jIGIvZHJpdmVycy9tdGQv bmFuZC9yYXcvc2hfZmxjdGwuYwo+ID4+IGluZGV4IGI4NWI5YzZmY2M0Mi4uMjM3MzI1MWY1ODVi IDEwMDY0NAo+ID4+IC0tLSBhL2RyaXZlcnMvbXRkL25hbmQvcmF3L3NoX2ZsY3RsLmMKPiA+PiAr KysgYi9kcml2ZXJzL210ZC9uYW5kL3Jhdy9zaF9mbGN0bC5jCj4gPj4gQEAgLTM4NCw3ICszODQs OCBAQCBzdGF0aWMgaW50IGZsY3RsX2RtYV9maWZvMF90cmFuc2ZlcihzdHJ1Y3Qgc2hfZmxjdGwg KmZsY3RsLCB1bnNpZ25lZCBsb25nICpidWYsCj4gPj4gIAlkbWFfYWRkcl90IGRtYV9hZGRyOwo+ ID4+ICAJZG1hX2Nvb2tpZV90IGNvb2tpZTsKPiA+PiAgCXVpbnQzMl90IHJlZzsKPiA+PiAtCWlu dCByZXQ7Cj4gPj4gKwlpbnQgcmV0ID0gMTsgIAo+ID4gRG9lcyBub3QgbG9vayByaWdodC4gSSBr bm93IHRoaXMgZnVuY3Rpb24gcmV0dXJucyA+IDAgb24gcG9zaXRpdmUKPiA+IG91dGNvbWVzIGJ1 dCB0aGlzIGRvZXMgbm90IG1ha2UgYW55IHNlbnNlIGluIHRoZSBmaXJzdCBwbGFjZS4gIAo+IAo+ IFllcywgSSBtYWRlIGEgbWlzdGFrZSwgTm93IEkgcmVhbGl6ZSB0aGF0IGluIHYyLCBpdCB3aWxs IHJldHVybiAxIGluIGVycm9yIHBhdGgKPiAKPiB3aGVuIERNQSBzdWJtaXQgZmFpbGVkLgoKTm90 IDEsIGJ1dCBhIHByb3BlciBlcnJvciBjb2RlIHBsZWFzZSAoLUVUSU1FRE9VVCwgLUVJTlZBTCwg d2hhdGV2ZXIpCgo+IAo+IEFuZCBmb3IgcGF0Y2ggdjEsIGl0IHdpbGwgcmV0dXJuIDDCoCBpZiBj YWxscyB3YWl0X2Zvcl9jb21wbGV0aW9uX3RpbWVvdXQgc3VjY2VlZHMuCj4gCj4gPiBUaGlzIGZ1 bmN0aW9uIGlzIHN0YXRpYyBhbmQgb25seSBjYWxsZWQgdHdpY2UsIHBsZWFzZSB0dXJuIGl0IGlu dG8KPiA+IHNvbWV0aGluZyBsaWtlOgo+ID4KPiA+IGlmIChkbWFfZmlmb190cmFuc2ZlcigpKQo+ ID4gCWVycm9yCj4gPiBlbHNlCj4gPiAJb2sgIAo+IAo+IFNvIEkgd2FudCB0byBrZWVwIHJldD4w IG1lYW5zIHN1Y2Nlc3MuCj4gCj4gT3IgY291bGQgSSBzZXQgcmV0ID4gMCBhZnRlciBpbiB3YWl0 X2Zvcl9jb21wbGV0aW9uX3RpbWVvdXQoKSBzdWNjZXNzIHBhdGg/Cj4gCj4gbGlrZToKPiAKPiDC oMKgwqAgaWYodGltZV9sZWZ0ID09IDApCj4gCj4gwqDCoMKgIMKgwqDCoCDCoMKgwqAgcmV0ID0g LUVUSU1FRE9VVDsKPiAKPiDCoMKgwqAgZWxzZQo+IAo+IMKgwqDCoCDCoMKgwqDCoMKgIMKgIHJl dCA9IDE7CgpZb3UgY2FuIGluaXRpYWxpemUgcmV0IHRvIHplcm8gYXQgdG8gdG9wLiBTbyB0aGF0 IGFueXRoaW5nICE9IDAgaXMgYW4KZXJyb3IgKGxpa2UgYSBsb3Qgb2YgZnVuY3Rpb25zIGluIHRo ZSBrZXJuZWwpLiBBbmQgdXNlOgoKCWlmIChkbWFfZmlmb190cmFuc2ZlcigpKQoJCWVycm9yKCk7 Cgo+IAo+IFdoYXQgZG8geW91IHRoaW5rPwo+IAo+IAo+IFRoYW5rcywKPiAKPiA+PiArCXVuc2ln bmVkIGxvbmcgdGltZV9sZWZ0Owo+ID4+ICAKPiA+PiAgCWlmIChkaXIgPT0gRE1BX0ZST01fREVW SUNFKSB7Cj4gPj4gIAkJY2hhbiA9IGZsY3RsLT5jaGFuX2ZpZm8wX3J4Owo+ID4+IEBAIC00MjUs MTMgKzQyNiwxNCBAQCBzdGF0aWMgaW50IGZsY3RsX2RtYV9maWZvMF90cmFuc2ZlcihzdHJ1Y3Qg c2hfZmxjdGwgKmZsY3RsLCB1bnNpZ25lZCBsb25nICpidWYsCj4gPj4gIAkJZ290byBvdXQ7Cj4g Pj4gIAl9Cj4gPj4gIAo+ID4+IC0JcmV0ID0KPiA+PiArCXRpbWVfbGVmdCA9Cj4gPj4gIAl3YWl0 X2Zvcl9jb21wbGV0aW9uX3RpbWVvdXQoJmZsY3RsLT5kbWFfY29tcGxldGUsCj4gPj4gIAkJCQlt c2Vjc190b19qaWZmaWVzKDMwMDApKTsKPiA+PiAgCj4gPj4gLQlpZiAocmV0IDw9IDApIHsKPiA+ PiArCWlmICh0aW1lX2xlZnQgPT0gMCkgewo+ID4+ICAJCWRtYWVuZ2luZV90ZXJtaW5hdGVfYWxs KGNoYW4pOwo+ID4+ICAJCWRldl9lcnIoJmZsY3RsLT5wZGV2LT5kZXYsICJ3YWl0X2Zvcl9jb21w bGV0aW9uX3RpbWVvdXRcbiIpOwo+ID4+ICsJCXJldCA9IC1FVElNRURPVVQ7Cj4gPj4gIAl9Cj4g Pj4gIAo+ID4+ICBvdXQ6ICAKPiA+Cj4gPiBUaGFua3MsCj4gPiBNaXF1w6hsICAKCgpUaGFua3Ms Ck1pcXXDqGwKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpMaW51eCBNVEQgZGlzY3Vzc2lvbiBtYWlsaW5nIGxpc3QKaHR0cDovL2xpc3RzLmlu ZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1tdGQvCg== 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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0E3EC433EF for ; Tue, 12 Apr 2022 09:30:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377295AbiDLJ2B (ORCPT ); Tue, 12 Apr 2022 05:28:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34882 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1379577AbiDLIUj (ORCPT ); Tue, 12 Apr 2022 04:20:39 -0400 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::229]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AD78B46154 for ; Tue, 12 Apr 2022 00:48:27 -0700 (PDT) Received: (Authenticated sender: miquel.raynal@bootlin.com) by mail.gandi.net (Postfix) with ESMTPSA id 0A4B9FF80A; Tue, 12 Apr 2022 07:48:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1649749706; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YW/FpTIll39+9L5ULUaE9f07ZfIeCCQhSd38iCsA3pg=; b=aXzEGK9EGcOVOJifzQG1YR9RgJX9bx4SRppY92QDfZCpkF7UG+SuAE+Vt3fD+s4lUG9Dp/ 6E2GOvRJUD3BrQQRuyBjEG+aPdyB+l36E0p6D5FAv0+/lUp9U/0BO+UexyjYN4nbjbBKi3 yKTP2jtMkAMg4BZuEJtHoDGlf3stW/95B1L1ZFDcIx1wGTsTUjAmLnV+PLRNQbOc8s11D6 FHRkgZg4BlnxjYRczCsHCgpPCxz242Zd0t2mGEjZrB8JJvC4krFD1gtGPAUu6q1j90u+c/ 8BFlf51DjUxZS/ikaKKj/5jV85k1S3INssgYwm81Xq1Zsnt5wDKZwG/775U/ug== Date: Tue, 12 Apr 2022 09:48:22 +0200 From: Miquel Raynal To: Miaoqian Lin Cc: Richard Weinberger , Vignesh Raghavendra , Pratyush Yadav , Paul Cercueil , Alexandre Belloni , Bastian Hecht , Artem Bityutskiy , Guennadi Liakhovetski , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mtd: rawnand: Fix return value check of wait_for_completion_timeout Message-ID: <20220412094822.3818ebc2@xps13> In-Reply-To: <61783400-4df2-47af-78a7-7acb715c3a71@gmail.com> References: <202204121253.NcZifMQi-lkp@intel.com> <20220412063703.8537-1-linmq006@gmail.com> <20220412090649.33bb3f8b@xps13> <61783400-4df2-47af-78a7-7acb715c3a71@gmail.com> Organization: Bootlin X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Miaoqian, linmq006@gmail.com wrote on Tue, 12 Apr 2022 15:42:02 +0800: > Hi Miquel, >=20 > Thanks for your reply. >=20 > On 2022/4/12 15:06, Miquel Raynal wrote: > > Hi Miaoqian, > > > > linmq006@gmail.com wrote on Tue, 12 Apr 2022 06:36:52 +0000: > > =20 > >> wait_for_completion_timeout() returns unsigned long not int. > >> It returns 0 if timed out, and positive if completed. > >> The check for <=3D 0 is ambiguous and should be =3D=3D 0 here > >> indicating timeout which is the only error case. > >> > >> Fixes: 83738d87e3a0 ("mtd: sh_flctl: Add DMA capabilty") > >> Signed-off-by: Miaoqian Lin > >> --- > >> change in v2: > >> - initialize ret to 1. > >> --- > >> drivers/mtd/nand/raw/sh_flctl.c | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/mtd/nand/raw/sh_flctl.c b/drivers/mtd/nand/raw/sh= _flctl.c > >> index b85b9c6fcc42..2373251f585b 100644 > >> --- a/drivers/mtd/nand/raw/sh_flctl.c > >> +++ b/drivers/mtd/nand/raw/sh_flctl.c > >> @@ -384,7 +384,8 @@ static int flctl_dma_fifo0_transfer(struct sh_flct= l *flctl, unsigned long *buf, > >> dma_addr_t dma_addr; > >> dma_cookie_t cookie; > >> uint32_t reg; > >> - int ret; > >> + int ret =3D 1; =20 > > Does not look right. I know this function returns > 0 on positive > > outcomes but this does not make any sense in the first place. =20 >=20 > Yes, I made a mistake, Now I realize that in v2, it will return 1 in erro= r path >=20 > when DMA submit failed. Not 1, but a proper error code please (-ETIMEDOUT, -EINVAL, whatever) >=20 > And for patch v1, it will return 0=C2=A0 if calls wait_for_completion_tim= eout succeeds. >=20 > > This function is static and only called twice, please turn it into > > something like: > > > > if (dma_fifo_transfer()) > > error > > else > > ok =20 >=20 > So I want to keep ret>0 means success. >=20 > Or could I set ret > 0 after in wait_for_completion_timeout() success pat= h? >=20 > like: >=20 > =C2=A0=C2=A0=C2=A0 if(time_left =3D=3D 0) >=20 > =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 ret =3D -ETIMEDO= UT; >=20 > =C2=A0=C2=A0=C2=A0 else >=20 > =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0 ret =3D 1; You can initialize ret to zero at to top. So that anything !=3D 0 is an error (like a lot of functions in the kernel). And use: if (dma_fifo_transfer()) error(); >=20 > What do you think? >=20 >=20 > Thanks, >=20 > >> + unsigned long time_left; > >> =20 > >> if (dir =3D=3D DMA_FROM_DEVICE) { > >> chan =3D flctl->chan_fifo0_rx; > >> @@ -425,13 +426,14 @@ static int flctl_dma_fifo0_transfer(struct sh_fl= ctl *flctl, unsigned long *buf, > >> goto out; > >> } > >> =20 > >> - ret =3D > >> + time_left =3D > >> wait_for_completion_timeout(&flctl->dma_complete, > >> msecs_to_jiffies(3000)); > >> =20 > >> - if (ret <=3D 0) { > >> + if (time_left =3D=3D 0) { > >> dmaengine_terminate_all(chan); > >> dev_err(&flctl->pdev->dev, "wait_for_completion_timeout\n"); > >> + ret =3D -ETIMEDOUT; > >> } > >> =20 > >> out: =20 > > > > Thanks, > > Miqu=C3=A8l =20 Thanks, Miqu=C3=A8l