Hi Guenter,
On Sat, 11 Apr 2020 at 10:46, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> 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.
I did not refer this page.
>
> FWIW, I implemented prototype code for this some time ago as
> extension of the k10temp driver, and the arithmetic wasn't that
> difficult.
Will take clues and implement.
>
> Guenter
Thank you.
--
Shine bright,
(: Nav :)
