[PATCH linux dev-5.8 1/3] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG

[Joel Stanley]

On Thu, 27 Aug 2020 at 13:30, Andrew Jeffery <andrew at aj.id.au> wrote:
>
> It has been observed across large fleets of systems that a small subset
> of those systems occasionally loose control of some number of fans

lose

> across a BMC reboot (their hwmon fan attributes are missing from sysfs).
>
> From extensive testing and tracing it was discovered that writes
> enabling a fan in FAN_CONFIG_1_2 failed to stick on the system under
> test with a frequency of about 1 in 1000 re-binds of the driver.
>
> The MAX31785 datasheet recommends in the documentation for
> MFR_FAN_CONFIG that the asssociated fan(s) be disabled before updating

associated

> the register. The sequence in question implements this suggestion, and
> the observed loss-of-fans symptom occurs when the write to re-enable the
> fan in FAN_CONFIG_1_2 fails to stick.
>
> The trace data suggests a one-shot retry is enough to successfully
> update FAN_CONFIG_1_2. With the workaround, no loss of fans was observed
> in over 20,000 consecutive rebinds of the driver.

Once you add your i2c throttling you should back out this change and re-test.

>
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
>  drivers/hwmon/pmbus/max31785.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index cbcd0b2301f4..88b7156d777e 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -376,6 +376,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
>         u32 page;
>         u32 uval;
>         int ret;
> +       int i;
>
>         if (!of_device_is_compatible(child, "pmbus-fan"))
>                 return 0;
> @@ -552,10 +553,24 @@ static int max31785_of_fan_config(struct i2c_client *client,
>         if (ret < 0)
>                 return ret;
>
> -       ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,
> -                                                pb_cfg);
> -       if (ret < 0)
> -               return ret;
> +       for (i = 0; i < 2; i++) {
> +               ret = max31785_i2c_smbus_write_byte_data(client,
> +                                                        PMBUS_FAN_CONFIG_12,
> +                                                        pb_cfg);
> +               if (ret < 0)
> +                       continue;
> +
> +               ret = max31785_i2c_smbus_read_byte_data(client,
> +                                                       PMBUS_FAN_CONFIG_12);
> +               if (ret < 0)
> +                       continue;
> +
> +               if (ret == pb_cfg)
> +                       break;
> +       }
> +
> +       if (i == 2)
> +               return -EIO;
>
>         /*
>          * Fans are on pages 0 - 5. If the page property of a fan node is
> --
> 2.25.1
>