On Thu, Jul 07, 2022 at 04:01:54PM +0200, Kallas, Pawel wrote:We cannot "read energy". Raw value from READ_EIN and READ_EOUT is not energy.
On 06-Jul-22 3:17 PM, Guenter Roeck wrote:Yes, but that is not the responsibility of the kernel. Just like we don't add
On Wed, Jul 06, 2022 at 12:40:21PM +0200, Kallas, Pawel wrote:According to PMBus-Specification-Rev-1-3-1-Part-II-20150313 "READ_EIN and
Add support for reading EIN or EOUT registers and expose power calculatedI don't think this is a good idea. EIN/EOUT report energy consumption,
from energy. This is more accurate than PIN and POUT power readings.
Readings are exposed in new hwmon files power1_average and power2_average.
Also add support for QUERY command that is needed to check availability
of EIN and EOUT reads and its data format. Only direct data format is
supported due to lack of test devices supporting other formats.
not power.
READ_EOUT commands provide information that can be used to calculate power
consumption". That is accumulator summing instantaneous input power
expressed in "watt-samples" and counter indicating number of samples.
The only reasonable thing that can be done with those values is calculating
power.
up power measurements to calculate energy, we don't take energy measurements
and calculate power consumption. Similar, we don't take voltage and current
measurements and report power consumption from it either.
Another reason for doing it in userspace. Read energy every N seconds, and useThe "average" attributes as implemented don't really reportAgree, it is calculating average power since last read, which could be
a reliable number since the averaging period is not defined.
incorrect with multiple consumers. However, this is the only possibility
without adding some timer logic.
the difference to calculate average power consumption average over that time
period.
Expressing raw value from READ_EIN or READ_EOUT is not in line withI did not suggest that. Just use the "energyX_in" attributes.Also, kernelReturning direct value of read registers would also work for our use case,
drivers should not make up such numbers. I don't mind adding energy
attribute support, but that should be reported as what it is, energy.
What userspace does with it would then be a userspace concern; it can
calculate all kinds of averages from it as much as it wants.
but it is not in line with sysfs interface.
Thanks,
Guenter
Also, new attributes should not depend on query command support.
I don't mind adding support for that, but it would have to be independent
of energy attribute support.
Thanks,
Guenter
Kallas, Pawel (3):
hwmon: (pmbus) add support for QUERY command
hwmon: (pmbus) refactor sensor initialization
hwmon: (pmbus) add EIN and EOUT readings
Documentation/hwmon/pmbus-core.rst | 7 +
drivers/hwmon/pmbus/pmbus.c | 20 +++
drivers/hwmon/pmbus/pmbus.h | 19 +++
drivers/hwmon/pmbus/pmbus_core.c | 261 +++++++++++++++++++++++++++--
4 files changed, 291 insertions(+), 16 deletions(-)
base-commit: 7c1de25c06f31b04744beae891baf147af9ba0cb