linux-kernel - Re: [RFC PATCH] hwmon: (iio_hwmon) Enable power exporting from IIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2bbbc5e9-7f11-1522-b827-0c0614bb9b69@xilinx.com>
Date:   Thu, 22 Aug 2019 15:43:39 +0200
From:   Michal Simek <michal.simek@...inx.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        Michal Simek <michal.simek@...inx.com>,
        linux-kernel@...r.kernel.org, monstr@...str.eu
Cc:     linux-hwmon@...r.kernel.org, Jean Delvare <jdelvare@...e.com>
Subject: Re: [RFC PATCH] hwmon: (iio_hwmon) Enable power exporting from IIO

On 20. 08. 19 17:39, Guenter Roeck wrote:
> On 8/12/19 6:29 AM, Michal Simek wrote:
>> There is no reason why power channel shouldn't be exported as is done for
>> voltage, current, temperature and humidity.
>>
>> Power channel is available on iio ina226 driver.
>>
>> Tested on Xilinx ZCU102 board.
>>
>> Signed-off-by: Michal Simek <michal.simek@...inx.com>
>> ---
>>
>> But I don't think values are properly converted. Voltage1 is fine but the
>> rest is IMHO wrong. But this patch should enable power channel to be
>> shown
>> which looks good.
>>
> 
> No idea what is going on there. I don't really know what "scaled" units
> the iio subsystem reports. power is supposed to be reported to user space
> in micro-Watt. Is that what the scaled value from iio reports ?

I have take a look at it again.

root@...qmp-debian:~/libiio/examples# sensors ina226_u76-*
ina226_u76-isa-0000
Adapter: ISA adapter
in1:          +0.01 V
in2:          +0.85 V
power1:        1.16 mW
curr1:        +1.60 A

from iio_monitor
voltage0          0.007 V
voltage1          0.848 V
power2            1.150 W
current3          1.361 A

from iio_info
 IIO context has 18 devices:
	iio:device0: ps_pmbus (buffer capable)
		9 channels found:
			voltage0:  (input, index: 0, format: le:U16/16>>0)
			3 channel-specific attributes found:
				attr  0: integration_time value: 0.001100
				attr  1: raw value: 3152
				attr  2: scale value: 0.002500000
			voltage1:  (input, index: 1, format: le:U16/16>>0)
			3 channel-specific attributes found:
				attr  0: integration_time value: 0.001100
				attr  1: raw value: 678
				attr  2: scale value: 1.250000000
			power2:  (input, index: 2, format: le:U16/16>>0)
			2 channel-specific attributes found:
				attr  0: raw value: 106
				attr  1: scale value: 12.500000000
			current3:  (input, index: 3, format: le:U16/16>>0)
			2 channel-specific attributes found:
				attr  0: raw value: 3152
				attr  1: scale value: 0.500000000


And if you look at power2 (in iio_info) then you see that calculation is
returning mili-Watts not micro-Watts.

And looking at it back it is also said there.

What:           /sys/bus/iio/devices/iio:deviceX/in_powerY_raw
KernelVersion:  4.5
Contact:        linux-iio@...r.kernel.org
Description:
                Raw (unscaled no bias removal etc.) power measurement from
                channel Y. The number must always be specified and
                unique to allow association with event codes. Units after
                application of scale and offset are milliwatts.

That means for power channel value needs to be multiply by 1000.

Are you OK with this solution?

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index aedb95fa24e3..7ea105bd195b 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -44,12 +44,20 @@ static ssize_t iio_hwmon_read_val(struct device *dev,
        int ret;
        struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
        struct iio_hwmon_state *state = dev_get_drvdata(dev);
+       struct iio_channel *chan = &state->channels[sattr->index];
+       enum iio_chan_type type;

-       ret = iio_read_channel_processed(&state->channels[sattr->index],
-                                       &result);
+       ret = iio_read_channel_processed(chan, &result);
        if (ret < 0)
                return ret;

+       ret = iio_get_channel_type(chan, &type);
+       if (ret < 0)
+               return ret;
+
+       if (type == IIO_POWER)
+               result *= 1000;
+
        return sprintf(buf, "%d\n", result);
 }


Thanks,
Michal