From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EA6A9A21 for ; Sat, 11 Jun 2022 15:08:02 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 43331C34116; Sat, 11 Jun 2022 15:07:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654960082; bh=EKJoSxvZYNzgGAFmHi7iG2ynvSq/HPXxuHetTKr9pBk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HhAeJlE9d10Rja9kdwI4GCHTGlTiTq7oU7PaoamRWAGpdIt+QuvcIlGnBezmOYYP2 pEHqdmxBs/DvwlzV6jGI6snaGa/BWHb+sgeKfMGta2ne1iyLKEgLAsrQ2ooxkvEx4b fjX6mazu1WcOiDuKllQ012dJktz+He5+Av7oQqRBqlnCmhE9WyCETz0AH2tkBtKvmw RZtcI6/3EvhhE6ooGI2zs6kzEWBlh0Omkf1NkdNGomu3nOxcQQGD/RRGfbi/nACeLh a99U2nK3JUkuZiF1tNbUsMpOhmC0ROFpxQ+e/LRCdItjmnT5ZHi+Qhwaw53ogkD3ki OIZ1e2fgNr16A== Date: Sat, 11 Jun 2022 16:17:01 +0100 From: Jonathan Cameron To: Nuno =?UTF-8?B?U8Oh?= Cc: , , , , , Lad Prabhakar , , , , , , Cai Huoqing , Benjamin Fair , Jishnu Prakash , "Linus Walleij" , Lars-Peter Clausen , Alexandre Torgue , Amit Kucheria , Andy Gross , Michael Hennerich , Haibo Chen , Benson Leung , "Rafael J. Wysocki" , "Alexandre Belloni" , Christophe Branchereau , Patrick Venture , Arnd Bergmann , Nancy Yuen , Sascha Hauer , Daniel Lezcano , "Gwendal Grignou" , Saravanan Sekar , "Tali Perry" , Maxime Coquelin , Paul Cercueil , Thara Gopinath , Avi Fishman , "Lorenzo Bianconi" , Claudiu Beznea , Pengutronix Kernel Team , Andy Shevchenko , Fabrice Gasnier , Matthias Brugger , Tomer Maimon , Bjorn Andersson , "Nicolas Ferre" , Zhang Rui , Shawn Guo , "Guenter Roeck" , Fabio Estevam , "Olivier Moysan" , Eugen Hristev , Miquel Raynal Subject: Re: [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs Message-ID: <20220611161701.46a68837@jic23-huawei> In-Reply-To: <20220610084545.547700-23-nuno.sa@analog.com> References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-23-nuno.sa@analog.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: chrome-platform@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Fri, 10 Jun 2022 10:45:33 +0200 Nuno S=C3=A1 wrote: > APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were > returning a mix of NULL and error pointers being NULL the way to pointers with NULL being the way to... > "notify" that we should do a "system" lookup for channels. This make > it very confusing and prone to errors as commit dbbccf7c20bf > ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()") > proves. On top of this, patterns like 'if (channel !=3D NULL) return chan= nel' > were being used where channel could actually be an error code which > makes the code hard to read. >=20 > Signed-off-by: Nuno S=C3=A1 > --- > drivers/iio/inkern.c | 24 +++++++++++------------- > 1 file changed, 11 insertions(+), 13 deletions(-) >=20 > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > index 87fd2a0d44f2..31d9c122199a 100644 > --- a/drivers/iio/inkern.c > +++ b/drivers/iio/inkern.c > @@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct = device_node *np, int index) > struct iio_channel *of_iio_channel_get_by_name(struct device_node *np, > const char *name) > { > - struct iio_channel *chan =3D NULL; > + struct iio_channel *chan; > =20 > /* Walk up the tree of devices looking for a matching iio channel */ > while (np) { > @@ -231,11 +231,11 @@ struct iio_channel *of_iio_channel_get_by_name(stru= ct device_node *np, > name); > chan =3D of_iio_channel_get(np, index); > if (!IS_ERR(chan) || PTR_ERR(chan) =3D=3D -EPROBE_DEFER) > - break; This original behaviour is 'interesting'. If we get a error like -ENOMEM we should return it rather than carry on. Do we have enough knowledge of possible errors here to be more explicit on when we keep looking up the tree? I think we can get -ENOENT from of_parse_phandle_with_args() That raises an interesting question on whether -ENODEV is the right response for the previously NULL case or is -ENOENT more consistent with other of_ functions? No device could be thought of as being the case that needs to defer (in hope it turns up later) whereas no entry means it will never succeed. > - else if (name && index >=3D 0) { > + return chan; > + if (name && index >=3D 0) { > pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n", > np, name ? name : "", index); > - return NULL; > + return chan; > } > =20 > /* > @@ -245,10 +245,10 @@ struct iio_channel *of_iio_channel_get_by_name(stru= ct device_node *np, > */ > np =3D np->parent; > if (np && !of_get_property(np, "io-channel-ranges", NULL)) > - return NULL; > + return chan; > } > =20 > - return chan; > + return ERR_PTR(-ENODEV); > } > EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name); > =20 > @@ -267,8 +267,8 @@ static struct iio_channel *of_iio_channel_get_all(str= uct device *dev) > break; > } while (++nummaps); > =20 > - if (nummaps =3D=3D 0) /* no error, return NULL to search map table */ > - return NULL; > + if (nummaps =3D=3D 0) /* return -ENODEV to search map table */ > + return ERR_PTR(-ENODEV); > =20 > /* NULL terminated array to save passing size */ > chans =3D kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL); > @@ -295,7 +295,7 @@ static struct iio_channel *of_iio_channel_get_all(str= uct device *dev) > =20 > static inline struct iio_channel *of_iio_channel_get_all(struct device *= dev) > { > - return NULL; > + return ERR_PTR(-ENODEV); > } > =20 > #endif /* CONFIG_OF */ > @@ -362,7 +362,7 @@ struct iio_channel *iio_channel_get(struct device *de= v, > if (dev) { > channel =3D of_iio_channel_get_by_name(dev->of_node, > channel_name); > - if (channel !=3D NULL) > + if (!IS_ERR(channel) || PTR_ERR(channel) =3D=3D -EPROBE_DEFER) > return channel; > } > =20 > @@ -412,8 +412,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(s= truct device *dev, > channel =3D of_iio_channel_get_by_name(np, channel_name); > if (IS_ERR(channel)) > return channel; > - if (!channel) > - return ERR_PTR(-ENODEV); > =20 > ret =3D devm_add_action_or_reset(dev, devm_iio_channel_free, channel); > if (ret) > @@ -436,7 +434,7 @@ struct iio_channel *iio_channel_get_all(struct device= *dev) > return ERR_PTR(-EINVAL); > =20 > chans =3D of_iio_channel_get_all(dev); > - if (chans) > + if (!IS_ERR(chans) || PTR_ERR(chans) =3D=3D -EPROBE_DEFER) Hmm. We only want to carry on if the error is -ENODEV. Anything else should be reported up the stack. That might be the only error left, but I think we should be explicit. > return chans; > =20 > name =3D dev_name(dev); 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 86FC9C433EF for ; Sat, 11 Jun 2022 15:08:17 +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=8frhk8xmaMSCnBaL+iRiMYS/hA0WJ+Elq7mKX2EPGA0=; b=aD1dOJ1TiMQNFo r0JcvJCf/nMdLNFsDiahZrExxpga8w0i/ZWZUqhECoeen3jy0mlHmA1wJQyse1wv8BMzSluLJZEcx k0qoUlRGylib0IwfNUWNhnTSiu69A1cA6MkZwpQ7yPCG9SvQTyWa3IC3t0vlOqmjjaSGRmQSYHcdB HvkiIAzqQVFnybxQOaOjDUpFVe6ceuacJJoGKRr4NPHZU22vVMXlie8jYPtpMYaU9C1aQiqh7XVKm 7VcE/bviDcBD8ZUbiH6uBpttkosC+zevYCIFfspizdsESr+Uc5CxVo82TRGkHi+vq1CIArHAQMsB5 SoGlqZqXl8iGT29syHgQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o02id-00DUj7-Qd; Sat, 11 Jun 2022 15:08:07 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o02ia-00DUi8-2U; Sat, 11 Jun 2022 15:08:05 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 3953861001; Sat, 11 Jun 2022 15:08:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 43331C34116; Sat, 11 Jun 2022 15:07:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654960082; bh=EKJoSxvZYNzgGAFmHi7iG2ynvSq/HPXxuHetTKr9pBk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HhAeJlE9d10Rja9kdwI4GCHTGlTiTq7oU7PaoamRWAGpdIt+QuvcIlGnBezmOYYP2 pEHqdmxBs/DvwlzV6jGI6snaGa/BWHb+sgeKfMGta2ne1iyLKEgLAsrQ2ooxkvEx4b fjX6mazu1WcOiDuKllQ012dJktz+He5+Av7oQqRBqlnCmhE9WyCETz0AH2tkBtKvmw RZtcI6/3EvhhE6ooGI2zs6kzEWBlh0Omkf1NkdNGomu3nOxcQQGD/RRGfbi/nACeLh a99U2nK3JUkuZiF1tNbUsMpOhmC0ROFpxQ+e/LRCdItjmnT5ZHi+Qhwaw53ogkD3ki OIZ1e2fgNr16A== Date: Sat, 11 Jun 2022 16:17:01 +0100 From: Jonathan Cameron To: Nuno =?UTF-8?B?U8Oh?= Cc: , , , , , Lad Prabhakar , , , , , , Cai Huoqing , Benjamin Fair , Jishnu Prakash , "Linus Walleij" , Lars-Peter Clausen , Alexandre Torgue , Amit Kucheria , Andy Gross , Michael Hennerich , Haibo Chen , Benson Leung , "Rafael J. Wysocki" , "Alexandre Belloni" , Christophe Branchereau , Patrick Venture , Arnd Bergmann , Nancy Yuen , Sascha Hauer , Daniel Lezcano , "Gwendal Grignou" , Saravanan Sekar , "Tali Perry" , Maxime Coquelin , Paul Cercueil , Thara Gopinath , Avi Fishman , "Lorenzo Bianconi" , Claudiu Beznea , Pengutronix Kernel Team , Andy Shevchenko , Fabrice Gasnier , Matthias Brugger , Tomer Maimon , Bjorn Andersson , "Nicolas Ferre" , Zhang Rui , Shawn Guo , "Guenter Roeck" , Fabio Estevam , "Olivier Moysan" , Eugen Hristev , Miquel Raynal Subject: Re: [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs Message-ID: <20220611161701.46a68837@jic23-huawei> In-Reply-To: <20220610084545.547700-23-nuno.sa@analog.com> References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-23-nuno.sa@analog.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; 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-20220611_080804_235575_2D602C64 X-CRM114-Status: GOOD ( 31.38 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org T24gRnJpLCAxMCBKdW4gMjAyMiAxMDo0NTozMyArMDIwMApOdW5vIFPDoSA8bnVuby5zYUBhbmFs b2cuY29tPiB3cm90ZToKCj4gQVBJcyBsaWtlIG9mX2lpb19jaGFubmVsX2dldF9ieV9uYW1lKCkg YW5kIG9mX2lpb19jaGFubmVsX2dldF9hbGwoKSB3ZXJlCj4gcmV0dXJuaW5nIGEgbWl4IG9mIE5V TEwgYW5kIGVycm9yIHBvaW50ZXJzIGJlaW5nIE5VTEwgdGhlIHdheSB0bwoKcG9pbnRlcnMgd2l0 aCBOVUxMIGJlaW5nIHRoZSB3YXkgdG8uLi4KCj4gIm5vdGlmeSIgdGhhdCB3ZSBzaG91bGQgZG8g YSAic3lzdGVtIiBsb29rdXAgZm9yIGNoYW5uZWxzLiBUaGlzIG1ha2UKPiBpdCB2ZXJ5IGNvbmZ1 c2luZyBhbmQgcHJvbmUgdG8gZXJyb3JzIGFzIGNvbW1pdCBkYmJjY2Y3YzIwYmYKPiAoImlpbzog aW5rZXJuOiBmaXggcmV0dXJuIHZhbHVlIGluIGRldm1fb2ZfaWlvX2NoYW5uZWxfZ2V0X2J5X25h bWUoKSIpCj4gcHJvdmVzLiBPbiB0b3Agb2YgdGhpcywgcGF0dGVybnMgbGlrZSAnaWYgKGNoYW5u ZWwgIT0gTlVMTCkgcmV0dXJuIGNoYW5uZWwnCj4gd2VyZSBiZWluZyB1c2VkIHdoZXJlIGNoYW5u ZWwgY291bGQgYWN0dWFsbHkgYmUgYW4gZXJyb3IgY29kZSB3aGljaAo+IG1ha2VzIHRoZSBjb2Rl IGhhcmQgdG8gcmVhZC4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBOdW5vIFPDoSA8bnVuby5zYUBhbmFs b2cuY29tPgo+IC0tLQo+ICBkcml2ZXJzL2lpby9pbmtlcm4uYyB8IDI0ICsrKysrKysrKysrLS0t LS0tLS0tLS0tLQo+ICAxIGZpbGUgY2hhbmdlZCwgMTEgaW5zZXJ0aW9ucygrKSwgMTMgZGVsZXRp b25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvaWlvL2lua2Vybi5jIGIvZHJpdmVycy9p aW8vaW5rZXJuLmMKPiBpbmRleCA4N2ZkMmEwZDQ0ZjIuLjMxZDljMTIyMTk5YSAxMDA2NDQKPiAt LS0gYS9kcml2ZXJzL2lpby9pbmtlcm4uYwo+ICsrKyBiL2RyaXZlcnMvaWlvL2lua2Vybi5jCj4g QEAgLTIxNCw3ICsyMTQsNyBAQCBzdGF0aWMgc3RydWN0IGlpb19jaGFubmVsICpvZl9paW9fY2hh bm5lbF9nZXQoc3RydWN0IGRldmljZV9ub2RlICpucCwgaW50IGluZGV4KQo+ICBzdHJ1Y3QgaWlv X2NoYW5uZWwgKm9mX2lpb19jaGFubmVsX2dldF9ieV9uYW1lKHN0cnVjdCBkZXZpY2Vfbm9kZSAq bnAsCj4gIAkJCQkJICAgICAgIGNvbnN0IGNoYXIgKm5hbWUpCj4gIHsKPiAtCXN0cnVjdCBpaW9f Y2hhbm5lbCAqY2hhbiA9IE5VTEw7Cj4gKwlzdHJ1Y3QgaWlvX2NoYW5uZWwgKmNoYW47Cj4gIAo+ ICAJLyogV2FsayB1cCB0aGUgdHJlZSBvZiBkZXZpY2VzIGxvb2tpbmcgZm9yIGEgbWF0Y2hpbmcg aWlvIGNoYW5uZWwgKi8KPiAgCXdoaWxlIChucCkgewo+IEBAIC0yMzEsMTEgKzIzMSwxMSBAQCBz dHJ1Y3QgaWlvX2NoYW5uZWwgKm9mX2lpb19jaGFubmVsX2dldF9ieV9uYW1lKHN0cnVjdCBkZXZp Y2Vfbm9kZSAqbnAsCj4gIAkJCQkJCQkgbmFtZSk7Cj4gIAkJY2hhbiA9IG9mX2lpb19jaGFubmVs X2dldChucCwgaW5kZXgpOwo+ICAJCWlmICghSVNfRVJSKGNoYW4pIHx8IFBUUl9FUlIoY2hhbikg PT0gLUVQUk9CRV9ERUZFUikKPiAtCQkJYnJlYWs7CgpUaGlzIG9yaWdpbmFsIGJlaGF2aW91ciBp cyAnaW50ZXJlc3RpbmcnLiBJZiB3ZSBnZXQgYSBlcnJvciBsaWtlIC1FTk9NRU0Kd2Ugc2hvdWxk IHJldHVybiBpdCByYXRoZXIgdGhhbiBjYXJyeSBvbi4gIERvIHdlIGhhdmUgZW5vdWdoIGtub3ds ZWRnZQpvZiBwb3NzaWJsZSBlcnJvcnMgaGVyZSB0byBiZSBtb3JlIGV4cGxpY2l0IG9uIHdoZW4g d2Uga2VlcCBsb29raW5nIHVwCnRoZSB0cmVlPyAgSSB0aGluayB3ZSBjYW4gZ2V0IC1FTk9FTlQg ZnJvbSBvZl9wYXJzZV9waGFuZGxlX3dpdGhfYXJncygpCgpUaGF0IHJhaXNlcyBhbiBpbnRlcmVz dGluZyBxdWVzdGlvbiBvbiB3aGV0aGVyIC1FTk9ERVYgaXMgdGhlIHJpZ2h0IHJlc3BvbnNlCmZv ciB0aGUgcHJldmlvdXNseSBOVUxMIGNhc2Ugb3IgaXMgLUVOT0VOVCBtb3JlIGNvbnNpc3RlbnQg d2l0aCBvdGhlcgpvZl8gZnVuY3Rpb25zPyAgTm8gZGV2aWNlIGNvdWxkIGJlIHRob3VnaHQgb2Yg YXMgYmVpbmcgdGhlIGNhc2UgdGhhdCBuZWVkcwp0byBkZWZlciAoaW4gaG9wZSBpdCB0dXJucyB1 cCBsYXRlcikgd2hlcmVhcyBubyBlbnRyeSBtZWFucyBpdCB3aWxsIG5ldmVyCnN1Y2NlZWQuCgo+ IC0JCWVsc2UgaWYgKG5hbWUgJiYgaW5kZXggPj0gMCkgewo+ICsJCQlyZXR1cm4gY2hhbjsKPiAr CQlpZiAobmFtZSAmJiBpbmRleCA+PSAwKSB7Cj4gIAkJCXByX2VycigiRVJST1I6IGNvdWxkIG5v dCBnZXQgSUlPIGNoYW5uZWwgJXBPRjolcyglaSlcbiIsCj4gIAkJCQlucCwgbmFtZSA/IG5hbWUg OiAiIiwgaW5kZXgpOwo+IC0JCQlyZXR1cm4gTlVMTDsKPiArCQkJcmV0dXJuIGNoYW47Cj4gIAkJ fQo+ICAKPiAgCQkvKgo+IEBAIC0yNDUsMTAgKzI0NSwxMCBAQCBzdHJ1Y3QgaWlvX2NoYW5uZWwg Km9mX2lpb19jaGFubmVsX2dldF9ieV9uYW1lKHN0cnVjdCBkZXZpY2Vfbm9kZSAqbnAsCj4gIAkJ ICovCj4gIAkJbnAgPSBucC0+cGFyZW50Owo+ICAJCWlmIChucCAmJiAhb2ZfZ2V0X3Byb3BlcnR5 KG5wLCAiaW8tY2hhbm5lbC1yYW5nZXMiLCBOVUxMKSkKPiAtCQkJcmV0dXJuIE5VTEw7Cj4gKwkJ CXJldHVybiBjaGFuOwo+ICAJfQo+ICAKPiAtCXJldHVybiBjaGFuOwo+ICsJcmV0dXJuIEVSUl9Q VFIoLUVOT0RFVik7Cj4gIH0KPiAgRVhQT1JUX1NZTUJPTF9HUEwob2ZfaWlvX2NoYW5uZWxfZ2V0 X2J5X25hbWUpOwo+ICAKPiBAQCAtMjY3LDggKzI2Nyw4IEBAIHN0YXRpYyBzdHJ1Y3QgaWlvX2No YW5uZWwgKm9mX2lpb19jaGFubmVsX2dldF9hbGwoc3RydWN0IGRldmljZSAqZGV2KQo+ICAJCQli cmVhazsKPiAgCX0gd2hpbGUgKCsrbnVtbWFwcyk7Cj4gIAo+IC0JaWYgKG51bW1hcHMgPT0gMCkJ Lyogbm8gZXJyb3IsIHJldHVybiBOVUxMIHRvIHNlYXJjaCBtYXAgdGFibGUgKi8KPiAtCQlyZXR1 cm4gTlVMTDsKPiArCWlmIChudW1tYXBzID09IDApCS8qIHJldHVybiAtRU5PREVWIHRvIHNlYXJj aCBtYXAgdGFibGUgKi8KPiArCQlyZXR1cm4gRVJSX1BUUigtRU5PREVWKTsKPiAgCj4gIAkvKiBO VUxMIHRlcm1pbmF0ZWQgYXJyYXkgdG8gc2F2ZSBwYXNzaW5nIHNpemUgKi8KPiAgCWNoYW5zID0g a2NhbGxvYyhudW1tYXBzICsgMSwgc2l6ZW9mKCpjaGFucyksIEdGUF9LRVJORUwpOwo+IEBAIC0y OTUsNyArMjk1LDcgQEAgc3RhdGljIHN0cnVjdCBpaW9fY2hhbm5lbCAqb2ZfaWlvX2NoYW5uZWxf Z2V0X2FsbChzdHJ1Y3QgZGV2aWNlICpkZXYpCj4gIAo+ICBzdGF0aWMgaW5saW5lIHN0cnVjdCBp aW9fY2hhbm5lbCAqb2ZfaWlvX2NoYW5uZWxfZ2V0X2FsbChzdHJ1Y3QgZGV2aWNlICpkZXYpCj4g IHsKPiAtCXJldHVybiBOVUxMOwo+ICsJcmV0dXJuIEVSUl9QVFIoLUVOT0RFVik7Cj4gIH0KPiAg Cj4gICNlbmRpZiAvKiBDT05GSUdfT0YgKi8KPiBAQCAtMzYyLDcgKzM2Miw3IEBAIHN0cnVjdCBp aW9fY2hhbm5lbCAqaWlvX2NoYW5uZWxfZ2V0KHN0cnVjdCBkZXZpY2UgKmRldiwKPiAgCWlmIChk ZXYpIHsKPiAgCQljaGFubmVsID0gb2ZfaWlvX2NoYW5uZWxfZ2V0X2J5X25hbWUoZGV2LT5vZl9u b2RlLAo+ICAJCQkJCQkgICAgIGNoYW5uZWxfbmFtZSk7Cj4gLQkJaWYgKGNoYW5uZWwgIT0gTlVM TCkKPiArCQlpZiAoIUlTX0VSUihjaGFubmVsKSB8fCBQVFJfRVJSKGNoYW5uZWwpID09IC1FUFJP QkVfREVGRVIpCj4gIAkJCXJldHVybiBjaGFubmVsOwo+ICAJfQo+ICAKPiBAQCAtNDEyLDggKzQx Miw2IEBAIHN0cnVjdCBpaW9fY2hhbm5lbCAqZGV2bV9vZl9paW9fY2hhbm5lbF9nZXRfYnlfbmFt ZShzdHJ1Y3QgZGV2aWNlICpkZXYsCj4gIAljaGFubmVsID0gb2ZfaWlvX2NoYW5uZWxfZ2V0X2J5 X25hbWUobnAsIGNoYW5uZWxfbmFtZSk7Cj4gIAlpZiAoSVNfRVJSKGNoYW5uZWwpKQo+ICAJCXJl dHVybiBjaGFubmVsOwo+IC0JaWYgKCFjaGFubmVsKQo+IC0JCXJldHVybiBFUlJfUFRSKC1FTk9E RVYpOwo+ICAKPiAgCXJldCA9IGRldm1fYWRkX2FjdGlvbl9vcl9yZXNldChkZXYsIGRldm1faWlv X2NoYW5uZWxfZnJlZSwgY2hhbm5lbCk7Cj4gIAlpZiAocmV0KQo+IEBAIC00MzYsNyArNDM0LDcg QEAgc3RydWN0IGlpb19jaGFubmVsICppaW9fY2hhbm5lbF9nZXRfYWxsKHN0cnVjdCBkZXZpY2Ug KmRldikKPiAgCQlyZXR1cm4gRVJSX1BUUigtRUlOVkFMKTsKPiAgCj4gIAljaGFucyA9IG9mX2lp b19jaGFubmVsX2dldF9hbGwoZGV2KTsKPiAtCWlmIChjaGFucykKPiArCWlmICghSVNfRVJSKGNo YW5zKSB8fCBQVFJfRVJSKGNoYW5zKSA9PSAtRVBST0JFX0RFRkVSKQoKSG1tLiBXZSBvbmx5IHdh bnQgdG8gY2Fycnkgb24gaWYgdGhlIGVycm9yIGlzIC1FTk9ERVYuICBBbnl0aGluZyBlbHNlCnNo b3VsZCBiZSByZXBvcnRlZCB1cCB0aGUgc3RhY2suCgpUaGF0IG1pZ2h0IGJlIHRoZSBvbmx5IGVy cm9yIGxlZnQsIGJ1dCBJIHRoaW5rIHdlIHNob3VsZCBiZSBleHBsaWNpdC4KCj4gIAkJcmV0dXJu IGNoYW5zOwo+ICAKPiAgCW5hbWUgPSBkZXZfbmFtZShkZXYpOwoKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxpbnV4LW1lZGlhdGVrIG1haWxpbmcgbGlz dApMaW51eC1tZWRpYXRla0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRl YWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtbWVkaWF0ZWsK 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 E7916C43334 for ; Thu, 16 Jun 2022 02:38:28 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4LNmZC3F9Bz3fBn for ; Thu, 16 Jun 2022 12:38:27 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=HhAeJlE9; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=139.178.84.217; helo=dfw.source.kernel.org; envelope-from=jic23@kernel.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=HhAeJlE9; dkim-atps=neutral Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4LL1RT1spBz3bkS for ; Sun, 12 Jun 2022 01:08:05 +1000 (AEST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 3953861001; Sat, 11 Jun 2022 15:08:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 43331C34116; Sat, 11 Jun 2022 15:07:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654960082; bh=EKJoSxvZYNzgGAFmHi7iG2ynvSq/HPXxuHetTKr9pBk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HhAeJlE9d10Rja9kdwI4GCHTGlTiTq7oU7PaoamRWAGpdIt+QuvcIlGnBezmOYYP2 pEHqdmxBs/DvwlzV6jGI6snaGa/BWHb+sgeKfMGta2ne1iyLKEgLAsrQ2ooxkvEx4b fjX6mazu1WcOiDuKllQ012dJktz+He5+Av7oQqRBqlnCmhE9WyCETz0AH2tkBtKvmw RZtcI6/3EvhhE6ooGI2zs6kzEWBlh0Omkf1NkdNGomu3nOxcQQGD/RRGfbi/nACeLh a99U2nK3JUkuZiF1tNbUsMpOhmC0ROFpxQ+e/LRCdItjmnT5ZHi+Qhwaw53ogkD3ki OIZ1e2fgNr16A== Date: Sat, 11 Jun 2022 16:17:01 +0100 From: Jonathan Cameron To: Nuno =?UTF-8?B?U8Oh?= Subject: Re: [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs Message-ID: <20220611161701.46a68837@jic23-huawei> In-Reply-To: <20220610084545.547700-23-nuno.sa@analog.com> References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-23-nuno.sa@analog.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Thu, 16 Jun 2022 12:05:37 +1000 X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alexandre Belloni , Daniel Lezcano , Tomer Maimon , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , Alexandre Torgue , Tali Perry , Paul Cercueil , Miquel Raynal , Guenter Roeck , Fabio Estevam , linux-stm32@st-md-mailman.stormreply.com, chrome-platform@lists.linux.dev, Lars-Peter Clausen , Benjamin Fair , openbmc@lists.ozlabs.org, Jishnu Prakash , Haibo Chen , Andy Shevchenko , Andy Gross , linux-imx@nxp.com, Olivier Moysan , Zhang Rui , Christophe Branchereau , Saravanan Sekar , Michael Hennerich , linux-arm-msm@vger.kernel.org, Sascha Hauer , Nicolas Ferre , Lad Prabhakar , Fabrice Gasnier , linux-mediatek@lists.infradead.org, Eugen Hristev , Matthias Brugger , Gwendal Grignou , Bjorn Andersson , Benson Leung , Pengutronix Kernel Team , linux-arm-kernel@lists.infradead.org, Lorenzo Bianconi , Avi Fishman , Patrick Venture , linux-mips@vger.kernel.org, Thara Gopinath , linux-renesas-soc@vger.kernel.org, Arnd Bergmann , Maxime Coquelin , Cai Huoqing , Shawn Guo , Claudiu Beznea Errors-To: openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Sender: "openbmc" On Fri, 10 Jun 2022 10:45:33 +0200 Nuno S=C3=A1 wrote: > APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were > returning a mix of NULL and error pointers being NULL the way to pointers with NULL being the way to... > "notify" that we should do a "system" lookup for channels. This make > it very confusing and prone to errors as commit dbbccf7c20bf > ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()") > proves. On top of this, patterns like 'if (channel !=3D NULL) return chan= nel' > were being used where channel could actually be an error code which > makes the code hard to read. >=20 > Signed-off-by: Nuno S=C3=A1 > --- > drivers/iio/inkern.c | 24 +++++++++++------------- > 1 file changed, 11 insertions(+), 13 deletions(-) >=20 > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > index 87fd2a0d44f2..31d9c122199a 100644 > --- a/drivers/iio/inkern.c > +++ b/drivers/iio/inkern.c > @@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct = device_node *np, int index) > struct iio_channel *of_iio_channel_get_by_name(struct device_node *np, > const char *name) > { > - struct iio_channel *chan =3D NULL; > + struct iio_channel *chan; > =20 > /* Walk up the tree of devices looking for a matching iio channel */ > while (np) { > @@ -231,11 +231,11 @@ struct iio_channel *of_iio_channel_get_by_name(stru= ct device_node *np, > name); > chan =3D of_iio_channel_get(np, index); > if (!IS_ERR(chan) || PTR_ERR(chan) =3D=3D -EPROBE_DEFER) > - break; This original behaviour is 'interesting'. If we get a error like -ENOMEM we should return it rather than carry on. Do we have enough knowledge of possible errors here to be more explicit on when we keep looking up the tree? I think we can get -ENOENT from of_parse_phandle_with_args() That raises an interesting question on whether -ENODEV is the right response for the previously NULL case or is -ENOENT more consistent with other of_ functions? No device could be thought of as being the case that needs to defer (in hope it turns up later) whereas no entry means it will never succeed. > - else if (name && index >=3D 0) { > + return chan; > + if (name && index >=3D 0) { > pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n", > np, name ? name : "", index); > - return NULL; > + return chan; > } > =20 > /* > @@ -245,10 +245,10 @@ struct iio_channel *of_iio_channel_get_by_name(stru= ct device_node *np, > */ > np =3D np->parent; > if (np && !of_get_property(np, "io-channel-ranges", NULL)) > - return NULL; > + return chan; > } > =20 > - return chan; > + return ERR_PTR(-ENODEV); > } > EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name); > =20 > @@ -267,8 +267,8 @@ static struct iio_channel *of_iio_channel_get_all(str= uct device *dev) > break; > } while (++nummaps); > =20 > - if (nummaps =3D=3D 0) /* no error, return NULL to search map table */ > - return NULL; > + if (nummaps =3D=3D 0) /* return -ENODEV to search map table */ > + return ERR_PTR(-ENODEV); > =20 > /* NULL terminated array to save passing size */ > chans =3D kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL); > @@ -295,7 +295,7 @@ static struct iio_channel *of_iio_channel_get_all(str= uct device *dev) > =20 > static inline struct iio_channel *of_iio_channel_get_all(struct device *= dev) > { > - return NULL; > + return ERR_PTR(-ENODEV); > } > =20 > #endif /* CONFIG_OF */ > @@ -362,7 +362,7 @@ struct iio_channel *iio_channel_get(struct device *de= v, > if (dev) { > channel =3D of_iio_channel_get_by_name(dev->of_node, > channel_name); > - if (channel !=3D NULL) > + if (!IS_ERR(channel) || PTR_ERR(channel) =3D=3D -EPROBE_DEFER) > return channel; > } > =20 > @@ -412,8 +412,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(s= truct device *dev, > channel =3D of_iio_channel_get_by_name(np, channel_name); > if (IS_ERR(channel)) > return channel; > - if (!channel) > - return ERR_PTR(-ENODEV); > =20 > ret =3D devm_add_action_or_reset(dev, devm_iio_channel_free, channel); > if (ret) > @@ -436,7 +434,7 @@ struct iio_channel *iio_channel_get_all(struct device= *dev) > return ERR_PTR(-EINVAL); > =20 > chans =3D of_iio_channel_get_all(dev); > - if (chans) > + if (!IS_ERR(chans) || PTR_ERR(chans) =3D=3D -EPROBE_DEFER) Hmm. We only want to carry on if the error is -ENODEV. Anything else should be reported up the stack. That might be the only error left, but I think we should be explicit. > return chans; > =20 > name =3D dev_name(dev);