[PATCH linux dev-5.8 2/3] pmbus: (max31785) Add a local pmbus_set_page() implementation

[Joel Stanley]

On Thu, 27 Aug 2020 at 13:30, Andrew Jeffery <andrew at aj.id.au> wrote:
>
> Extensive testing and tracing has shown that the MAX31785 is unreliable
> in the face of PAGE write commands, ACK'ing the PAGE request but
> reporting a value of 0 on some subsequent PAGE reads. The trace data
> suggests that a one-shot retry of the PAGE write is enough to get the
> requested value to stick.
>
> As we configure the device before registering with the PMBus core,
> centralise PAGE handling inside the driver and implement the one-shot
> retry semantics there.

Are you still doing a retry with every operation due to the macro magic?

>
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
>  drivers/hwmon/pmbus/max31785.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index 88b7156d777e..a392c0efe0a6 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -361,6 +361,27 @@ static int max31785_write_word_data(struct i2c_client *client, int page,
>         return -ENXIO;
>  }
>
> +static int max31785_pmbus_set_page(struct i2c_client *client, int page)
> +{
> +       int ret;
> +       int i;
> +
> +       for (i = 0; i < 2; i++) {
> +               ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> +               if (ret < 0)
> +                       return ret;
> +
> +               ret = max31785_i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> +               if (ret < 0)
> +                       return ret;
> +
> +               if (ret == page)
> +                       return 0;
> +       }
> +
> +       return -EIO;
> +}
> +
>  /*
>   * Returns negative error codes if an unrecoverable problem is detected, 0 if a
>   * recoverable problem is detected, or a positive value on success.
> @@ -392,7 +413,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
>                 return -ENXIO;
>         }
>
> -       ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> +       ret = max31785_pmbus_set_page(client, page);
>         if (ret < 0)
>                 return ret;
>
> @@ -613,7 +634,7 @@ static int max31785_of_tmp_config(struct i2c_client *client,
>                 return -ENXIO;
>         }
>
> -       ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> +       ret = max31785_pmbus_set_page(client, page);
>         if (ret < 0)
>                 return ret;
>
> @@ -714,7 +735,7 @@ static int max31785_configure_dual_tach(struct i2c_client *client,
>         int i;
>
>         for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) {
> -               ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
> +               ret = max31785_pmbus_set_page(client, i);
>                 if (ret < 0)
>                         return ret;
>
> @@ -756,7 +777,7 @@ static int max31785_probe(struct i2c_client *client,
>
>         *info = max31785_info;
>
> -       ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);
> +       ret = max31785_pmbus_set_page(client, 255);
>         if (ret < 0)
>                 return ret;
>
> @@ -798,8 +819,7 @@ static int max31785_probe(struct i2c_client *client,
>                 if (!have_fan || fan_configured)
>                         continue;
>
> -               ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE,
> -                                                        i);
> +               ret = max31785_pmbus_set_page(client, i);
>                 if (ret < 0)
>                         return ret;
>
> --
> 2.25.1
>