Re: [PATCH 1/3] hwmon: (pmbus) Add new flag PMBUS_READ_STATUS_AFTER_FAILED_CHECK

[Guenter Roeck]

On 5/5/21 11:32 AM, Erik Rosen wrote:
> Some PMBus chips end up in an undefined state when trying to read an
> unsupported register. For such chips, it is necessary to reset the
> chip pmbus controller to a known state after a failed register check.
> This can be done by reading a known register. By setting this flag the
> driver will try to read the STATUS register after each failed
> register check. This read may fail, but it will put the chip into a
> known state.
> 
> Signed-off-by: Erik Rosen <erik.rosen@xxxxxxxxxxxxx>

For my reference:

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>  drivers/hwmon/pmbus/pmbus_core.c |  2 ++
>  include/linux/pmbus.h            | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index aadea85fe630..cb0b3c7c3434 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -512,6 +512,8 @@ static bool pmbus_check_register(struct i2c_client *client,
>  	rv = func(client, page, reg);
>  	if (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))
>  		rv = pmbus_check_status_cml(client);
> +	if (rv < 0 && (data->flags & PMBUS_READ_STATUS_AFTER_FAILED_CHECK))
> +		data->read_status(client, -1);
>  	pmbus_clear_fault_page(client, -1);
>  	return rv >= 0;
>  }
> diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
> index 12cbbf305969..edd7c84fef65 100644
> --- a/include/linux/pmbus.h
> +++ b/include/linux/pmbus.h
> @@ -43,6 +43,19 @@
>   */
>  #define PMBUS_NO_CAPABILITY			BIT(2)
>  
> +/*
> + * PMBUS_READ_STATUS_AFTER_FAILED_CHECK
> + *
> + * Some PMBus chips end up in an undefined state when trying to read an
> + * unsupported register. For such chips, it is necessary to reset the
> + * chip pmbus controller to a known state after a failed register check.
> + * This can be done by reading a known register. By setting this flag the
> + * driver will try to read the STATUS register after each failed
> + * register check. This read may fail, but it will put the chip in a
> + * known state.
> + */
> +#define PMBUS_READ_STATUS_AFTER_FAILED_CHECK	BIT(3)
> +
>  struct pmbus_platform_data {
>  	u32 flags;		/* Device specific flags */
>  
>