Re: [PATCH 1/2] hwmon: Add amd_energy driver to report energy counters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/10/20 9:49 PM, Naveen Krishna Ch wrote:
> Hi,
> 
> Thank you for taking time to review and provide feedback.
> If you can kindly address a few questions, i would start addressing
> your comments and submit another version.
> 

[ ... ]

>>> +
>>> +static ssize_t amd_core_u64_show(struct device *dev,
>>> +             struct device_attribute *dev_attr, char *buffer)
>>> +{
>>> +     unsigned long long value;
>>> +     struct sensor_data *sensor;
>>> +     int ret = 0;
>>> +
>>> +     sensor = container_of(dev_attr, struct sensor_data, dev_attr_input);
>>> +     ret = rdmsrl_safe_on_cpu(sensor->cpu_nr, ENERGY_CORE_MSR, &value);
>>> +     if (ret)
>>> +             return -EAGAIN;
>>> +
>>> +     return snprintf(buffer, 24, "%llu\n", value);
>>
>> It seems to me this reports raw values. This is unacceptable. Values need
>> to be scaled to match the ABI. Energy is reported in microJoule.
> I was of the opinion that driver provides the raw values and user-space (can use
> float/double)  will be able to scale the value, which would involve
> calculation of
> "1/2^ESU * RAW value"
> 

Please see Documentation/hwmon/sysfs-interface.rst. Sure, there is
/etc/sensors3.conf, but that only applies if the kernel driver can not
provide scaled values. This is not the case here.

FWIW, I implemented prototype code for this some time ago as
extension of the k10temp driver, and the arithmetic wasn't that
difficult.

Guenter



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux