linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Michal Simek <michal.simek@xilinx.com>
Cc: linux-kernel@vger.kernel.org, monstr@monstr.eu,
	linux-hwmon@vger.kernel.org, Jean Delvare <jdelvare@suse.com>
Subject: Re: [RFC PATCH] hwmon: (iio_hwmon) Enable power exporting from IIO
Date: Thu, 22 Aug 2019 07:10:39 -0700	[thread overview]
Message-ID: <20190822141039.GD8144@roeck-us.net> (raw)
In-Reply-To: <2bbbc5e9-7f11-1522-b827-0c0614bb9b69@xilinx.com>

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

      reply	other threads:[~2019-08-22 14:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12 13:29 [RFC PATCH] hwmon: (iio_hwmon) Enable power exporting from IIO Michal Simek
2019-08-20 15:39 ` Guenter Roeck
2019-08-22 13:43   ` Michal Simek
2019-08-22 14:10     ` Guenter Roeck [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190822141039.GD8144@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=monstr@monstr.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).