Re: [PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()

From: Andi Shyti <andi.shyti@kernel.org>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	linus.walleij@linaro.org, linux@armlinux.org.uk,
	linux-i2c@vger.kernel.org,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	Codrin Ciubotariu <codrin.ciubotariu@microchip.com>,
	Oleksij Rempel <linux@rempel-privat.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Jinjie Ruan <ruanjinjie@huawei.com>,
	linux-arm-kernel@lists.infradead.org,
	NXP Linux Team <linux-imx@nxp.com>
Subject: Re: [PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()
Date: Thu, 24 Aug 2023 18:17:27 +0200	[thread overview]
Message-ID: <20230824161727.czobz3p4d5bdxtrh@intel.intel> (raw)
In-Reply-To: <20230824083453.GB31399@pengutronix.de>

Hi,

On Thu, Aug 24, 2023 at 10:34:53AM +0200, Oleksij Rempel wrote:
> On Mon, Aug 21, 2023 at 11:29:14AM +0800, Jinjie Ruan wrote:
> > If pinctrl is not available (thus devm_pinctrl_get() returns NULL) then
> > recovery can't work, because we can't switch the I2C pins between the
> > I2C controller and GPIO. So, it is quite correct to print
> > "can't get pinctrl, bus recovery not supported" because the I2C bus
> > can't be recovered without pinctrl.
> > 
> > The PTR_ERR() is also fine - because if pinctrl is not present and
> > returns NULL, we'll end up returning zero, which is exactly what we
> > want.
> > 
> > However, open code that with a more accurate message will be more explicit
> > for NULL case when CONFIG_PINCTRL is not defined.
> > 
> > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> > Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
> > ---
> > v3:
> > - Split into two if() statements instead of removing the NULL checks.
> > - Remove the fix tags.
> > - Update the commit title and message.
> > - Update to bring the author's name before.
> > v2:
> > - Remove NULL check instead of using IS_ERR_OR_NULL() to avoid leaving them behind.
> > - Update the commit title and message.
> > ---
> >  drivers/i2c/busses/i2c-at91-master.c | 6 +++++-
> >  drivers/i2c/busses/i2c-imx.c         | 6 +++++-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> for the i2c-imx.c

For some reason lore+lei failed to deliver this message to my
inbox :/

Anyway, looks good!

Reviewed-by: Andi Shyti <andi.shyti@kernel.org> 

Thank you, Jinjie for following up.
Andi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel