Re: [PATCH linux dev-4.13 3/5] pmbus (core): One-shot retries for failure to set page

[Eddie James <eajames@linux.vnet.ibm.com>]

On 04/03/2018 09:26 AM, Andrew Jeffery wrote:
> Work around the shonky behaviour seen with the MAX31785 where we fail
> to set the page register in some circumstances.
>
> There's no real elegant way to do this. We can propagate the error up,
> but that forces us to retry the operation way up the call tree in any
> number of places. It also forces callers to split out pmbus_set_page()
> from the pmbus_{read,write}_{byte,word}_data() functions in order to
> differentiate between a failure to set the page and a failure to read a
> register (that might not exist, in which case an error is anticiptated).

Reviewed-by: Eddie James <eajames@linux.vnet.ibm.com>

>
> Cc: Eddie James <eajames@linux.vnet.ibm.com>
> Cc: Matt Spinler <mspinler@linux.vnet.ibm.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>   drivers/hwmon/pmbus/pmbus_core.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index ee7e0684c29f..ee1bf11c7fc6 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -167,9 +167,17 @@ int pmbus_set_page(struct i2c_client *client, int page)
>   		return 0;
>
>   	if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL)) {
> +		dev_dbg(&client->dev, "Want page %u, %u cached\n", page,
> +			data->currpage);
> +
>   		rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> -		if (rv < 0)
> -			return rv;
> +		if (rv) {
> +			rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE,
> +						       page);
> +			dev_dbg(&client->dev,
> +				"Failed to set page %u, performed one-shot retry %s: %d\n",
> +				page, rv ? "and failed" : "with success", rv);
> +		}
>
>   		rv = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
>   		if (rv < 0)