Messages in this thread | | | Date | Thu, 17 Mar 2022 11:50:53 -0700 | From | Guenter Roeck <> | Subject | Re: [PATCH v2] hwmon: (pmbus/ibm-cffps) Add clear_faults debugfs entry |
| |
On 3/17/22 09:12, Brandon Wyman wrote: > On Wed, Mar 16, 2022 at 3:14 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 3/16/22 13:03, Brandon Wyman wrote: >>> On Sun, Mar 13, 2022 at 11:36 PM Guenter Roeck <linux@roeck-us.net> wrote: >>>> >>>> On 3/11/22 10:10, Brandon Wyman wrote: >>>>> Add a clear_faults write-only debugfs entry for the ibm-cffps device >>>>> driver. >>>>> >>>>> Certain IBM power supplies require clearing some latched faults in order >>>>> to indicate that the fault has indeed been observed/noticed. >>>>> >>>> >>>> That is insufficient, sorry. Please provide the affected power supplies as >>>> well as the affected faults, and confirm that the problem still exists >>>> in v5.17-rc6 or later kernels - or, more specifically, in any kernel which >>>> includes commit 35f165f08950 ("hwmon: (pmbus) Clear pmbus fault/warning >>>> bits after read"). >>>> >>>> Thanks, >>>> Guenter >>> >>> Sorry for the delay in responding. I did some testing with commit >>> 35f165f08950. I could not get that code to send the CLEAR_FAULTS >>> command to the power supplies. >>> >>> I can update the commit message to be more specific about which power >>> supplies need this CLEAR_FAULTS sent, and which faults. It is observed >>> with the 1600W power supplies (2B1E model). The faults that latch are >>> the VIN_UV and INPUT faults in the STATUS_WORD. The corresponding >>> STATUS_INPUT fault bits are VIN_UV_FAULT and Unit is Off. >>> >> >> The point is that the respective fault bits should be reset when the >> corresponding alarm attributes are read. This isn't about executing >> a CLEAR_FAULTS command, but about selectively resetting fault bits >> while ensuring that faults are reported at least once. Executing >> CLEAR_FAULTS is a big hammer. >> >> With the patch I pointed to in place, input (and other) faults should >> be reset after the corresponding alarm attributes are read, assuming >> that the condition no longer exists. If that does not happen, we should >> fix the problem instead of deploying the big hammer. >> >> Thanks, >> Guenter > > Okay, I see what you are pointing out there. I had been mostly looking > at the "files" in the debugfs paths. Those do not end up running > through that pmbus_get_boolean() function, so the individual fault > clearing was not being attempted. The fault I was interested in > appears to be associated with in1_lcrti_alarm. Reading that will give > me a 1 if there is a VIN_UV fault, and then it sends 0x10 to > STATUS_INPUT. That clears out VIN_UV, but the STATUS_INPUT command was > returning 0x18. Nothing appears to handle clearing BIT(3), that 0x08 > mask. > > Should there be some kind of define for BIT(3) over in pmbus.h? > Something like PB_VOLTAGE_OFF? Somehow we need something using that in > sbit of the attributes. I had a quick hack that just OR'ed BIT(3) with > BIT(4) for that PB_VOLTAGE_UV_FAULT. That resulted in a clear of both > bits in STATUS_INPUT, and the faults clearing in STATUS_WORD. > > It is not clear if there should be a separate alarm for that "Unit Off > For Insufficient Input Voltage", or if the one for in1_lcrit_alarm > could just be the two bits OR'ed into one mask. I can send a patch > with a proposal on how to fix this one bit not getting cleared. >
We don't have a separate standard attribute. I think the best approach would be to add a mask for bit 3 and or that mask for lcrit in vin_limit_attrs with PB_VOLTAGE_UV_FAULT. I'd suggest to name the define something like PB_VOLTAGE_VIN_OFF or PB_VOLTAGE_VIN_FAULT to clarify that the bit applies to the input.
Thanks, Guenter
| |