From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH] net: dsa: lan9303: correctly check return value of devm_gpiod_get_optional Date: Mon, 13 Nov 2017 05:38:15 +0100 Message-ID: <20171113043815.GB22473@lunn.ch> References: <1510501089-9451-1-git-send-email-bianpan2016@163.com> <251e5e33-8bbb-d745-63c8-95d7b34d95f0@electromag.com.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Pan Bian , Vivien Didelot , Florian Fainelli , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org To: Phil Reid Return-path: Content-Disposition: inline In-Reply-To: <251e5e33-8bbb-d745-63c8-95d7b34d95f0@electromag.com.au> Sender: stable-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Mon, Nov 13, 2017 at 12:08:49PM +0800, Phil Reid wrote: > On 12/11/2017 23:38, Pan Bian wrote: > >Function devm_gpiod_get_optional() returns an ERR_PTR on failure. Its > >return value should not be validated by a NULL check. Instead, use IS_ERR. > > > >Signed-off-by: Pan Bian > >--- > > drivers/net/dsa/lan9303-core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c > >index b471413..6d3fc8f 100644 > >--- a/drivers/net/dsa/lan9303-core.c > >+++ b/drivers/net/dsa/lan9303-core.c > >@@ -828,7 +828,7 @@ static void lan9303_probe_reset_gpio(struct lan9303 *chip, > > chip->reset_gpio = devm_gpiod_get_optional(chip->dev, "reset", > > GPIOD_OUT_LOW); > >- if (!chip->reset_gpio) { > >+ if (IS_ERR(chip->reset_gpio)) { > > dev_dbg(chip->dev, "No reset GPIO defined\n"); > > return; > > } > > > Should not an error actually report the error and error out (ie fail probe). > But a null is the optional return and ok. (ie when -ENOENT return from sub gpiod_get call). > > IS_ERR should be a separate condition check I think. Hi Phil Yes, you are right. In particular, -EPROBE_DEFFER should be propagated up and cause the probe to fail and be called later. Care to submit a patch? Andrew