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

[Vikash Chandola]

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.

> > 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)