Re: Linux 5.11: i2c: Confusing error message

Re: Linux 5.11: i2c: Confusing error message

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Hello Klaus,

On Mon, Mar 15, 2021 at 07:58:21AM +0100, Klaus Kudielka wrote:

Thanks for the report! I am CCing the I2C list, please add it next time,
too.

> Hello,
> 
> I recently upgraded my Turris Omnia (Marvell Armada 385) to 5.11, and now
> get the following error message during boot:
> 
>     i2c i2c-0: Not using recovery: no recover_bus() found
> 
> As far as I understand the situation:
> 
>  * After commit 9c7cae2427715502227f823364a6a77828fdf3ea mv64xxx-i2c
>    unconditionally sets bus_recovery_info and bus_recovery_info->pinctrl
>  * The i2c bus node in the turris-omnia DTS does *not* have a pinctrl
>    state "gpio" or "recovery" (like any other Marvell Armada board, AFAICS)
>  * i2c_gpio_init_recovery() throws a debug message "no gpio or recovery
>    state found for GPIO recovery", which (under normal circumstances)
>    users won't see
>  * After i2c_gpio_init_recovery() returns, i2c_init_recovery() throws
>    above-mentioned error message
> 
> Is this the intended behaviour?
> Personally, I believe an "error" message is quite strong for this common
> scenario, and a bit misleading.

You are right. The case that a pinctrl is provided but not the necessary
states for recovery has been overlooked so far.

I will think of something right now.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Linux 5.11: i2c: Confusing error message

[Mark Tomlinson]

On Mon, 2021-03-15 at 07:58 +0100, Klaus Kudielka wrote:
> Hello,
> 
> I recently upgraded my Turris Omnia (Marvell Armada 385) to 5.11, and 
> now get the following error message during boot:
> 
>      i2c i2c-0: Not using recovery: no recover_bus() found
> 
> Is this the intended behaviour?
> Personally, I believe an "error" message is quite strong for this common 
> scenario, and a bit misleading.
> 
> Thanks, Klaus
> 

I admit that I added a "gpio" state to pinctrl for our board, so didn't
see this error message. I think the easiest fix for this is to check that
the gpio pinctrl state exists. e.g. something like:

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index c590d36b5fd1..55c366f402f2 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -894,6 +894,9 @@ static int mv64xxx_i2c_init_recovery_info(struct mv64xxx_i2c_data *drv_data,
                return PTR_ERR(rinfo->pinctrl);
        } else if (!rinfo->pinctrl) {
                return -ENODEV;
+       } else if (IS_ERR(pinctrl_lookup_state(rinfo->pinctrl, "gpio"))) {
+               dev_info(dev, "pinctrl states incomplete for recovery\n");
+               return -ENODEV;
        }
 
        drv_data->adapter.bus_recovery_info = rinfo;

Re: Linux 5.11: i2c: Confusing error message

[wsa]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]


> I admit that I added a "gpio" state to pinctrl for our board, so didn't
> see this error message. I think the easiest fix for this is to check that
> the gpio pinctrl state exists. e.g. something like:

It affects all drivers, so it should be fixed in the core. See:

http://patchwork.ozlabs.org/project/linux-i2c/patch/20210315115008.19110-1-wsa+renesas@sang-engineering.com/

from today.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]