On 2023/8/18 7:07, Yann Sionneau wrote:

Hi,

Le 17/08/2023 à 19:30, Leo Li a écrit :

quoted

quoted

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

I think it's the best.

2/ change the fallback implementation in
include/linux/pinctrl/consumer.h to return ERR_PTR(-Esomething) (which
errno?)

It seems a convention to return NULL if the related macro is not defined.

3/ another solution?

Make I2C_IMX and I2C_AT91 config depends on PINCTRL config is another
option. However it seems that the function call devm_pinctrl_get() has
an optional recovery feature from the following notes and dev_info(). So
this dependency is not necessary.

1378 /*
1379  * We switch SCL and SDA to their GPIO function and do some bitbanging
1380  * for bus recovery. These alternative pinmux settings can be
1381  * described in the device tree by a separate pinctrl state "gpio". If
1382  * this is missing this is not a big problem, the only implication is
1383  * that we can't do bus recovery.
1384  */
1385 static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
1386         struct platform_device *pdev)
1387 {
1388     struct i2c_bus_recovery_info *rinfo = &i2c_imx->rinfo;
1389
1390     i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);

828 static int at91_init_twi_recovery_gpio(struct platform_device *pdev,
829                        struct at91_twi_dev *dev)
830 {
831     struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
832
833     rinfo->pinctrl = devm_pinctrl_get(&pdev->dev);
834     if (!rinfo->pinctrl || IS_ERR(rinfo->pinctrl)) {
835         dev_info(dev->dev, "can't get pinctrl, bus recovery not
supported\n");
836         return PTR_ERR(rinfo->pinctrl);
837     }

Regards,

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