Re: [PATCH -next v2] I2C: Fix return value check for devm_pinctrl_get()

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Aug 18, 2023 at 01:07:27AM +0200, Yann Sionneau wrote:
> Hi,
> 
> Le 17/08/2023 à 19:30, Leo Li a écrit :
> 
> > > The devm_pinctrl_get() function returns error pointers and never returns
> > > NULL. Update the checks accordingly.
> > Not exactly.  It can return NULL when CONFIG_PINCTRL is not defined.  We probably should fix that API too.
> > 
> > include/linux/pinctrl/consumer.h:
> > static inline struct pinctrl * __must_check devm_pinctrl_get(struct device *dev)
> > {
> >          return NULL;
> > }
> 
> So, as Leo pointed out it seems devm_pinctrl_get() can in fact return NULL,
> when CONFIG_PINCTRL is not defined.
> 
> What do we do about this?
> 
> Proposals:
> 
> 1/ make sure all call sites of devm_pinctrl_get() do check for error with
> IS_ERR *and* check for NULL => therefore using IS_ERR_OR_NULL
> 
> 2/ change the fallback implementation in include/linux/pinctrl/consumer.h to
> return ERR_PTR(-Esomething) (which errno?)
> 
> 3/ another solution?

NULL is returned on purpose. When PINCTRL is disabled NULL becomes a
valid pinctrl cookie which can be passed to the other stub functions.
With this drivers using pinctrl can get through their probe function
without an error when PINCTRL is disabled.

The same approach is taken by the clk and regulator API.

It is correct to test the return value of devm_pinctrl_get() with
IS_ERR(), only the commit message of these patches is a bit inaccurate.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |