Re: [RFC 0/3] drivers/hwmon/pmbus: Use STATUS_WORD and add status sensors

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Aug 08, 2017 at 11:12:19AM -0500, Eddie James wrote:
> 
> 
> On 08/07/2017 11:00 PM, Guenter Roeck wrote:
> >On 08/07/2017 08:25 PM, Eddie James wrote:
> >>From: "Edward A. James" <eajames@us.ibm.com>
> >>
> >>Hi Guenter,
> >>
> >>I'm looking for some feedback for some extensions to the pmbus core.
> >>We're
> >>looking for some additional functionality, particularly with STATUS_WORD
> >>and
> >>obtaining raw status data.
> >>
> >>The first two patches enable the use of the STATUS_WORD register in the
> >>pmbus
> >>core. This allows the use of more default alarm/fault attributes for
> >>default
> >>pmbus sensors by allowing the use of the higher byte status bits.
> >>
> >>The third patch adds "status" attributes to each class of hwmon sensor
> >>created
> >>by pmbus. For example, in1_status and temp1_status. These will display
> >>the
> >>associated raw status register (e.g. STATUS_INPUT and
> >>STATUS_TEMPERATURE). I
> >>realize this is not really "normal" for hwmon or pmbus. These are
> >>potentially
> >>very useful in hardware diagnostic situations where it might be
> >>impossible
> >>to tell the origin of a failure from a simple alarm or fault bit set. We
> >>really
> >>want to access the status registers, and for a multi-page pmbus device,
> >>this is
> >>pretty tricky from userspace.
> >>
> >>Please let me know your thoughts,
> >>Thanks,
> >
> >I don't mind providing such data with debugfs, for example, but I don't
> >see
> >the point in providing it as part of the ABI. Which, in part, since it
> >requires
> >a lot of thought on my side, is part of the reason why I didn't provide
> >feedback to your earlier patches yet. Sorry, I've been exceptionally busy
> >lately, and non-standard requests tend to end up at the end of the queue
> >:-(.
> 
> No problem! Thanks for the quick response on this.
> 
> >
> >Any reason why debugfs is not sufficient and/or acceptable for your use
> >case ?
> >You _are_ talking about diagnostic situations, which seems to be an exact
> >fit
> >for debugfs.
> 
> Agreed, great idea, I think debugfs will work perfectly. I probably should
> have thought of that sooner...
> 
> How about the first two patches in the series? They are unrelated to adding
> any attributes. Mainly I would
> like to have the PB_STATUS_INPUT bit available to trigger the default
> boolean alarm attribute, as our hardware doesn't support any limits.
> 
Main problem with it is that patch 2 assumes that the word status register
and thus PB_STATUS_INPUT is available, which is not always the case.
On affected chips, the code will now generate the input attributes even
if those are not really supported.

Guenter

> Thanks again,
> Eddie
> 
> >
> >Guenter
> >
> >>
> >>Edward A. James (3):
> >>   drivers/hwmon/pmbus: Access word data for STATUS_WORD and use it by
> >>     default
> >>   drivers/hmwon/pmbus: store STATUS_WORD in status registers
> >>   drivers/hwmon/pmbus: Add sensor status to pmbus attributes
> >>
> >>  drivers/hwmon/pmbus/pmbus_core.c | 153
> >>+++++++++++++++++++++++++++++++++------
> >>  1 file changed, 130 insertions(+), 23 deletions(-)
> >>
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html