On Mon, Sep 09, 2024 at 11:30:28AM +0200, Patryk Biel wrote:
> This change adds fetching PMBus revision and using it to conditionally
> clear individual status bits while calling pmbus_show_boolean, only if
> the device is compliant with PMBus specs >= 1.2.
>
> Signed-off-by: Patryk Biel <pbiel7@xxxxxxxxx>
> ---
> Current implementation of pmbus_show_boolean assumes that all devices
> support write-back operation of status register so as to clear pending
> warning or faults. Since clearing individual bits in the status registers
> was introduced in PMBus specification 1.2, this operation may not be
> supported by some older devices, thus resulting in error while reading
> boolean attributes like e.g. temp1_max_alarm.
>
> This change adds fetching PMBus revision supported by device and
> modifies pmbus_show_boolean so that it only tries to clear individual
> status bits if the device is compilant with PMBus specs >= 1.2.
>
> Tested on: LTC2971, LTC2971-1, LTC2974, LTC2977.
> ---
> drivers/hwmon/pmbus/pmbus.h | 6 ++++++
> drivers/hwmon/pmbus/pmbus_core.c | 12 +++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
>
> ---
> base-commit: c763c43396883456ef57e5e78b64d3c259c4babc
> change-id: 20240905-pmbus-status-reg-clearing-abc9c0184c3b
>
> Best regards,
>
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index fb442fae7b3e..0bea603994e7 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -418,6 +418,12 @@ enum pmbus_sensor_classes {
> enum pmbus_data_format { linear = 0, ieee754, direct, vid };
> enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv };
>
> +/* PMBus revision identifiers */
> +#define PMBUS_REV_10 0x00 /* PMBus revision 1.0 */
> +#define PMBUS_REV_11 0x11 /* PMBus revision 1.1 */
> +#define PMBUS_REV_12 0x22 /* PMBus revision 1.2 */
> +#define PMBUS_REV_13 0x33 /* PMBus revision 1.3 */
> +
> struct pmbus_driver_info {
> int pages; /* Total number of pages */
> u8 phases[PMBUS_PAGES]; /* Number of phases per page */
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index cb4c65a7f288..50ba093a38e8 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -108,6 +108,8 @@ struct pmbus_data {
>
> int vout_low[PMBUS_PAGES]; /* voltage low margin */
> int vout_high[PMBUS_PAGES]; /* voltage high margin */
> +
> + u8 revision; /* The PMBus revision the device is compliant with */
> };
>
> struct pmbus_debugfs_entry {
> @@ -1095,7 +1097,11 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
>
> regval = status & mask;
> if (regval) {
> - ret = _pmbus_write_byte_data(client, page, reg, regval);
> + if (data->revision >= PMBUS_REV_12)
> + ret = _pmbus_write_byte_data(client, page, reg, regval);
> + else
> + pmbus_clear_fault_page(client, page);
> +
> if (ret)
> goto unlock;
That check needs to be part of the if() statement above. Never mind, though,
I fixed that up.
Guenter
