On 2023/8/18 7:07, 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
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,
>
