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

From: Vikash Chandola
Date: Thu Feb 10 2022 - 13:29:46 EST




On 2/10/2022 10:55 PM, Guenter Roeck wrote:
On 2/10/22 07:57, Vikash Chandola wrote:


On 2/10/2022 8:14 PM, Guenter Roeck wrote:
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

But as per current implementation we will continue to report fault even after fault condition is cleared. I could not find sysfs entry or any other means by which user/kernel can clear the faults/warnings after it is reported. Please point if I am missing anything.


Normally a chip should clear the status after the fault condition cleared
and the register was read. At least that was my experience so far.
What chip (or chips) don't do that ?

Guenter

I see that pmbus spec says that once fault/warning bits are set
they won't be cleared by itself. Section 10.2.3 of
"PMBus Specification Rev. 1.3.1 Part II 2015-03-13" from
https://pmbus.org/specification-archives/ says

"
Almost all of the warning or fault bits set in the status registers remain set, even if the fault or warning condition is removed
or corrected, until one of the following occur:
* The bit is individually cleared (Section 10.2.4),
* The device receives a CLEAR_FAULTS command (Section 15.1),
* A RESET signal (if one exists) is asserted,
* The output is commanded through the CONTROL pin, the OPERATION command, or the combined action of the CONTROL pin and OPERATION
command, to turn off and then to turn back on, or
* Bias power is removed from the PMBus device...
"

From this I concluded that slave(PSU) following pmbus spec will not
clear the fault/warning in status registers.
I don't have exact chip details handy but I can get it by tomorrow.
However this looks to be issue not related to specific chip ?

If this is a problem, how should I approach this ? Shall I create a new
sysfs entry that user space application can invoke and clear all faults
or provide sysfs entry to clear individual fault/warning bits or
something else ?


Signed-off-by: Vikash Chandola <vikash.chandola@xxxxxxxxxxxxxxx>
---
  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)



--
Vikash