On 7/21/20 11:31 AM, zlukwins wrote: > > On 7/8/20 3:33 PM, Guenter Roeck wrote: >> 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 > > Do you think we need to wait for feedback or maybe we could implement with your proposition? Maybe I could start working on patches? > I don't think we will get any further feedback. Please go ahead. Thanks, Guenter > > Thanks > > Zbigniew >
