Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757887AbbKSIIc (ORCPT ); Thu, 19 Nov 2015 03:08:32 -0500 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:43641 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756203AbbKSIIb (ORCPT ); Thu, 19 Nov 2015 03:08:31 -0500 Date: Thu, 19 Nov 2015 09:08:22 +0100 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Li Yang Cc: lkml , Gao Pan , "linux-i2c@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Wolfram Sang , Linus Walleij , linux-gpio@vger.kernel.org Subject: Re: [PATCH] i2c: imx: make bus recovery through pinctrl optional Message-ID: <20151119080822.GY22022@pengutronix.de> References: <1447804979-29550-1-git-send-email-leoli@freescale.com> <20151118072437.GS22022@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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: 2341 Lines: 52 Hello, Cc += Linus Walleij, linux-gpio On Wed, Nov 18, 2015 at 04:59:11PM -0600, Li Yang wrote: > On Wed, Nov 18, 2015 at 1:24 AM, Uwe Kleine-K?nig > wrote: > > On Tue, Nov 17, 2015 at 06:02:59PM -0600, Li Yang wrote: > >> @@ -1125,7 +1119,12 @@ static int i2c_imx_probe(struct platform_device *pdev) > >> goto clk_disable; > >> } > >> > >> - i2c_imx_init_recovery_info(i2c_imx, pdev); > >> + /* optional bus recovery feature through pinctrl */ > >> + i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev); > >> + if (IS_ERR_OR_NULL(i2c_imx->pinctrl)) > >> + dev_info(&pdev->dev, "can't get pinctrl, bus recovery feature disabled\n"); > >> + else > >> + i2c_imx_init_recovery_info(i2c_imx, pdev); > > > > I'm pretty sure this is wrong. If pinctrl isn't available > > devm_pinctrl_get returns NULL? But AFAIK you must not ignore an error, > > so the better thing to do is: > > If CONFIG_PINCTRL is not enabled, the devm_pinctrl_get() will return > NULL directly as defined in the include/linux/pinctrl/consumer.h. > > If CONFIG_PINCTRL is enabled because we are using a multi-platform > image but the actual hardware used doesn't have a pinctrl driver or > pinctrl device tree nodes. It is expected that the devm_pinctrl_get() > will return error. But as the pinctrl is only used for bus recovery > which is just an optional function of this driver. We shouldn't > bailout the probe but keep the driver working without the bus recovery > function. As for generic errors like (!dev) or out-of-memory, the > probe will fail elsewhere anyway. IMHO you shouldn't ignore -ENOMEM from devm_pinctrl_get but just the case for "there was no pinctrl specified in dt". Also consider the i2c driver is loaded before the pinctrl driver and devm_pinctrl_get returns -EPROBE_DEFER (I assume it does, didn't check), this should be handled. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/