linux-kernel - Re: [PATCH] hwmon: (pmbus) Clear pmbus fault/warning bits before read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <46684682-a718-ca9a-b502-2031afd3a756@roeck-us.net>
Date:   Thu, 10 Feb 2022 06:44:07 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Vikash Chandola <vikash.chandola@...ux.intel.com>,
        iwona.winiarska@...el.com, jdelvare@...e.com,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (pmbus) Clear pmbus fault/warning bits before read

On 2/10/22 04:41, Vikash Chandola wrote:
> pmbus fault and warning bits are not cleared by itself once fault/warning
> condition is not valid anymore. As per pmbus datasheet faults must be
> cleared by user.
> Modify hwmon behavior to clear latched status bytes if any bit in status
> register is high prior to returning fresh data to userspace. If
> fault/warning conditions are still applicable fault/warning bits will be
> set and we will get updated data in second read.
> 
> Hwmon behavior is changed here. Now sysfs reads will reflect latest
> values from pmbus slave, not latched values.
> In case a transient warning/fault has happened in the past, it will no
> longer be reported to userspace.
> 


NACK.

Reporting that information is exactly the point of the current code.
We _do_ want to report at least once that a problem occurred in the past,
and only clear the warning flag(s) afterwards.

Guenter

> Signed-off-by: Vikash Chandola <vikash.chandola@...ux.intel.com>
> ---
>   drivers/hwmon/pmbus/pmbus_core.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 776ee2237be2..1cc82d644079 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -577,6 +577,15 @@ static int pmbus_get_status(struct i2c_client *client, int page, int reg)
>   		break;
>   	default:
>   		status = _pmbus_read_byte_data(client, page, reg);
> +		if (status > 0) {
> +			/*
> +			 * Status greater than 0 could mean that there was a fault/warning.
> +			 * Clear faults and do a second read to make sure we are not getting
> +			 * stale values.
> +			 */
> +			pmbus_clear_fault_page(client, page);
> +			status = _pmbus_read_byte_data(client, page, reg);
> +		}
>   		break;
>   	}
>   	if (status < 0)