Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756870AbcCRMzM (ORCPT ); Fri, 18 Mar 2016 08:55:12 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:46874 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752439AbcCRMzH (ORCPT ); Fri, 18 Mar 2016 08:55:07 -0400 Date: Fri, 18 Mar 2016 13:54:55 +0100 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Sebastian Frias Cc: "David S. Miller" , netdev@vger.kernel.org, lkml , mason , Daniel Mack , Florian Fainelli , Mans Rullgard , Fabio Estevam , Martin Blumenstingl , Linus Walleij Subject: Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB Message-ID: <20160318125455.GN4292@pengutronix.de> References: <56E99727.9040702@laposte.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <56E99727.9040702@laposte.net> User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2127 Lines: 59 [expand cc a bit more] Hello, On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote: > Commit 687908c2b649 ("net: phy: at803x: simplify using > devm_gpiod_get_optional and its 4th argument") introduced a dependency > on GPIOLIB that was not there before. > > This commit removes such dependency by checking the return code and > comparing it against ENOSYS which is returned when GPIOLIB is not > selected. > > Fixes: 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") > > Signed-off-by: Sebastian Frias > --- > drivers/net/phy/at803x.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c > index 2174ec9..88b7ff3 100644 > --- a/drivers/net/phy/at803x.c > +++ b/drivers/net/phy/at803x.c > @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev) > return -ENOMEM; > > gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > - if (IS_ERR(gpiod_reset)) > + if (PTR_ERR(gpiod_reset) == -ENOSYS) > + gpiod_reset = NULL; > + else if (IS_ERR(gpiod_reset)) this isn't better either (IMHO it's worse, but maybe this is debatable and also depends on your POV). With 687908c2b649 I made kernels without GPIOLIB fail to bind this device. I admit this is not maximally nice. Your change makes the driver bind in this case again and then the reset gpio isn't handled at all which might result in a later and harder to debug error. The better approach to fix your problem is: make the driver depend (or force) on GPIOLIB. Or alternatively, drop reset-handling from the driver. >From a driver perspecitive, it would be nice if devm_gpiod_get_optional returned NULL iff the respective gpio isn't specified even with GPIOLIB=n, but this isn't sensible either because it would result in quite some gpiolib code to not being conditionally compiled on CONFIG_GPIOLIB any more. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |