On 7/8/20 3:50 AM, zlukwins wrote: > > On 6/29/20 6:59 PM, Guenter Roeck wrote: >> +linux-hwmon@xxxxxxxxxxxxxxx >> >> On Mon, Jun 29, 2020 at 08:31:11AM +0200, zlukwins wrote: >>> Hi, >>> >>> >>> I am OpenBMC FW developer working currently on some power measurement stuff. >>> >>> I would like to have maximum rated input power for pmubus device available >>> in hwmon sysfs. This value is read by MFR_PIN_MAX command: >>> >>> /MFR_PIN_MAX// >>> //The MFR_PIN_MIN command sets or retrieves the maximum rated value, in >>> watts, of// >>> //the input power./ >> Interesting typo in the PMBus specification. Yes, it really does associate >> MFR_PIN_MIN - which doesn't seem to exist - with the maximum rated output >> power. >>> And I wondering which attribute shell be used to expose that value in sysfs. >>> I went through documentation >>> (https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface) and found >>> the following: >>> >>> power[1-*]_max Maximum power. >>> Unit: microWatt >>> RW >>> >>> But it looks like it is already occupied by PIN_OP_WARN_LIMIT. >>> >>> Maybe new attribute shall be used? If so how to call that? >>> >> None of the standard attributes reports (or is supposed to report) rated >> values, so we can not just use any of those. >> >> Also, we can not just add a single attribute to handle this situation, >> for the simple reason that there are many more similar attributes. >> PMBus specifies (this is from version 1.3.1): >> >> MFR_VIN_MIN >> MFR_VIN_MAX >> MFR_IIN_MAX >> MFR_PIN_MAX >> MFR_VOUT_MIN >> MFR_VOUT_MAX >> MFR_IOUT_MAX >> MFR_POUT_MAX >> MFR_TAMBIENT_MAX >> MFR_TAMBIENT_MIN >> MFR_MAX_TEMP_{1,2.3} >> >> All those report rated values. I do see the need/desire for reporting such >> information. The only real solution I can see is to add a new set of >> attributes to the hwmon ABI. Something like: >> >> currentX_rated_min # for consistency >> currentX_rated_max >> inX_rated_min >> inX_rated_max >> powerX_rated_min # for consistency >> powerX_rated_max >> tempX_rated_min >> tempX_rated_max >> plus maybe, for consistency: >> humidityX_rated_min >> humidityX_rated_max >> >> Those would be read-only attributes. >> >> Thoughts, comments, feedback anyone ? >> >> Thanks, >> Guenter > > > I really like your proposition but I guess we need to wait few more days for the feedback. > > But have some questions here. > > What about potential next steps when we all agreed to follow that approach. Should documentation modification reach upstream repository first and then e.g. pmbus hwmon module implementation? > We'll need a series of patches. One to amend the documentation, one to add the necessary code to the hwmon core (so that the core supports it with the _with_info API), one to add support to the PMBus core, and one each to add support to affected drivers. Once this is all complete, the lm-sensors package should be updated as well. Guenter
