diff mbox series

[RFC] hwmon: (iio_hwmon) Enable power exporting from IIO

Message ID 71aec0191e0e5f32cc760f95844d8ee215b48c7f.1565616579.git.michal.simek@xilinx.com (mailing list archive)
State Superseded
Headers show
Series [RFC] hwmon: (iio_hwmon) Enable power exporting from IIO | expand

Commit Message

Michal Simek Aug. 12, 2019, 1:29 p.m. UTC
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@xilinx.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.

root@zynqmp-debian:~# iio_info -a && sensors -u
Library version: 0.16 (git tag: v0.16)
Compiled with backends: local xml ip usb serial
Using auto-detected IIO context at URI "local:"
IIO context created with local backend.
Backend version: 0.16 (git tag: v0.16)
Backend description string: Linux zynqmp-debian 5.3.0-rc4-00004-ga7ca33daed22-dirty #41 SMP PREEMPT Mon Aug 12 15:06:58 CEST 2019 aarch64
IIO context has 1 attributes:
	local,kernel: 5.3.0-rc4-00004-ga7ca33daed22-dirty
IIO context has 1 devices:
	iio:device0: ina226 (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: 70
				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: 958
				attr  2: scale value: 1.250000000
			power2:  (input, index: 2, format: le:U16/16>>0)
			2 channel-specific attributes found:
				attr  0: raw value: 3
				attr  1: scale value: 0.006250000
			current3:  (input, index: 3, format: le:U16/16>>0)
			2 channel-specific attributes found:
				attr  0: raw value: 70
				attr  1: scale value: 0.000250000
			timestamp:  (input, index: 4, format: le:S64/64>>0)
			allow:  (input)
			1 channel-specific attributes found:
				attr  0: async_readout value: 0
			oversampling:  (input)
			1 channel-specific attributes found:
				attr  0: ratio value: 4
			sampling:  (input)
			1 channel-specific attributes found:
				attr  0: frequency value: 114
			shunt:  (input)
			1 channel-specific attributes found:
				attr  0: resistor value: 10.000000000
		2 device-specific attributes found:
				attr  0: current_timestamp_clock value: realtime

				attr  1: integration_time_available value: 0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244
		2 buffer-specific attributes found:
				attr  0: data_available value: 0
				attr  1: watermark value: 1
		1 debug attributes found:
				debug attr  0: direct_reg_access value: 0x4327
ina226_fourth-isa-0000
Adapter: ISA adapter
in1:
  in1_input: 0.000
in2:
  in2_input: 1.198
power1:
  power1_input: 0.000
curr1:
  curr1_input: 0.000
---
 drivers/hwmon/iio_hwmon.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Guenter Roeck Aug. 20, 2019, 3:39 p.m. UTC | #1
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@xilinx.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 ?

Thanks,
Guenter

> root@zynqmp-debian:~# iio_info -a && sensors -u
> Library version: 0.16 (git tag: v0.16)
> Compiled with backends: local xml ip usb serial
> Using auto-detected IIO context at URI "local:"
> IIO context created with local backend.
> Backend version: 0.16 (git tag: v0.16)
> Backend description string: Linux zynqmp-debian 5.3.0-rc4-00004-ga7ca33daed22-dirty #41 SMP PREEMPT Mon Aug 12 15:06:58 CEST 2019 aarch64
> IIO context has 1 attributes:
> 	local,kernel: 5.3.0-rc4-00004-ga7ca33daed22-dirty
> IIO context has 1 devices:
> 	iio:device0: ina226 (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: 70
> 				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: 958
> 				attr  2: scale value: 1.250000000
> 			power2:  (input, index: 2, format: le:U16/16>>0)
> 			2 channel-specific attributes found:
> 				attr  0: raw value: 3
> 				attr  1: scale value: 0.006250000
> 			current3:  (input, index: 3, format: le:U16/16>>0)
> 			2 channel-specific attributes found:
> 				attr  0: raw value: 70
> 				attr  1: scale value: 0.000250000
> 			timestamp:  (input, index: 4, format: le:S64/64>>0)
> 			allow:  (input)
> 			1 channel-specific attributes found:
> 				attr  0: async_readout value: 0
> 			oversampling:  (input)
> 			1 channel-specific attributes found:
> 				attr  0: ratio value: 4
> 			sampling:  (input)
> 			1 channel-specific attributes found:
> 				attr  0: frequency value: 114
> 			shunt:  (input)
> 			1 channel-specific attributes found:
> 				attr  0: resistor value: 10.000000000
> 		2 device-specific attributes found:
> 				attr  0: current_timestamp_clock value: realtime
> 
> 				attr  1: integration_time_available value: 0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244
> 		2 buffer-specific attributes found:
> 				attr  0: data_available value: 0
> 				attr  1: watermark value: 1
> 		1 debug attributes found:
> 				debug attr  0: direct_reg_access value: 0x4327
> ina226_fourth-isa-0000
> Adapter: ISA adapter
> in1:
>    in1_input: 0.000
> in2:
>    in2_input: 1.198
> power1:
>    power1_input: 0.000
> curr1:
>    curr1_input: 0.000
> ---
>   drivers/hwmon/iio_hwmon.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
> index f1c2d5faedf0..aedb95fa24e3 100644
> --- a/drivers/hwmon/iio_hwmon.c
> +++ b/drivers/hwmon/iio_hwmon.c
> @@ -59,7 +59,7 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   	struct iio_hwmon_state *st;
>   	struct sensor_device_attribute *a;
>   	int ret, i;
> -	int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1;
> +	int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1, power_i = 1;
>   	enum iio_chan_type type;
>   	struct iio_channel *channels;
>   	struct device *hwmon_dev;
> @@ -114,6 +114,10 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   			n = curr_i++;
>   			prefix = "curr";
>   			break;
> +		case IIO_POWER:
> +			n = power_i++;
> +			prefix = "power";
> +			break;
>   		case IIO_HUMIDITYRELATIVE:
>   			n = humidity_i++;
>   			prefix = "humidity";
>
Michal Simek Aug. 22, 2019, 1:43 p.m. UTC | #2
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@xilinx.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@zynqmp-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@vger.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
Guenter Roeck Aug. 22, 2019, 2:10 p.m. UTC | #3
On Thu, Aug 22, 2019 at 03:43:39PM +0200, Michal Simek wrote:
> 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@xilinx.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@zynqmp-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@vger.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?
> 
Yes, looks good.

Thanks,
Guenter

> 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
diff mbox series

Patch

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index f1c2d5faedf0..aedb95fa24e3 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -59,7 +59,7 @@  static int iio_hwmon_probe(struct platform_device *pdev)
 	struct iio_hwmon_state *st;
 	struct sensor_device_attribute *a;
 	int ret, i;
-	int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1;
+	int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1, power_i = 1;
 	enum iio_chan_type type;
 	struct iio_channel *channels;
 	struct device *hwmon_dev;
@@ -114,6 +114,10 @@  static int iio_hwmon_probe(struct platform_device *pdev)
 			n = curr_i++;
 			prefix = "curr";
 			break;
+		case IIO_POWER:
+			n = power_i++;
+			prefix = "power";
+			break;
 		case IIO_HUMIDITYRELATIVE:
 			n = humidity_i++;
 			prefix = "humidity";