Re: [PATCH] hwmon: (iio_hwmon) Add support for humidity and power sensors

Guenter Roeck
2014-09-29T19:13:48Z

On Mon, Sep 29, 2014 at 06:36:13PM +0100, Jonathan Cameron wrote:
> On 27/09/14 16:49, Guenter Roeck wrote:
> > The iio subsystem supports power and humidity sensors, so it makes sense
> > to support those sensor types in the iio-hwmon bridge as well.
> >
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Looks good.
> > ---
> > Compile tested only.
> >
> > Question is if I got the scale conversion correct for power measurements.
> > I _assume_ that iio always reports processed values in milli-units,
> > but I am not entirely sure.
>
> IIO actually only keeps to milli units where we were copying from hwmom
> back when originally working this stuff out. Given we have the double
> int32 representation and IIO_TYPE_* we try to keep to the base SI units
> everywhere else. (If we were doing it again today, I'd be advocating
> using base units everywhere for consistency reasons)
>
What does that mean for the iio-hwmon bridge ? Should I get the base
units (or scaling factors) on a per-sensor basis, or does the above apply
on a per-type basis ? I checked the humidity ABI (which is documented),
and it says that humidity is reported in milli-units.

Thanks,
Guenter

> The only user of IIO_POWER right now is the ade7758 driver in staging.
> I note that hwmon uses micro watts so would imagine that driver
> does as well. However given the timescale it's probably a driver from the
> big dump from Analog's tree before we'd pinned down a lot of the ABIs
> Lars, don't suppose you'd care to take a look? The types got added
> by Michael when cleaning the driver up somewhat...
>
> Also we should really add the relevant ABI docs (once we've agreed the unit!)
>
> Jonathan
> >
> > drivers/hwmon/iio_hwmon.c | 30 +++++++++++++++++++++++++++---
> > 1 file changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
> > index 14c82da..efc55a9 100644
> > --- a/drivers/hwmon/iio_hwmon.c
> > +++ b/drivers/hwmon/iio_hwmon.c
> > @@ -21,6 +21,8 @@
> > /**
> > * struct iio_hwmon_state - device instance state
> > * @channels: filled with array of channels from iio
> > + * @scale: array of multipliers to convert from iio to hwmon
> > + * resolution
> > * @num_channels: number of channels in channels (saves counting twice)
> > * @hwmon_dev: associated hwmon device
> > * @attr_group: the group of attributes
> > @@ -28,6 +30,7 @@
> > */
> > struct iio_hwmon_state {
> > struct iio_channel *channels;
> > + int *scale;
> > int num_channels;
> > struct device *hwmon_dev;
> > struct attribute_group attr_group;
> > @@ -48,13 +51,16 @@ 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);
> > + s64 result_64;
> >
> > ret = iio_read_channel_processed(&state->channels[sattr->index],
> > - &result);
> > + &result);
> > if (ret < 0)
> > return ret;
> >
> > - return sprintf(buf, "%d\n", result);
> > + result_64 = (s64)result * state->scale[sattr->index];
> > +
> > + return sprintf(buf, "%lld\n", result_64);
> > }
> >
> > static int iio_hwmon_probe(struct platform_device *pdev)
> > @@ -63,7 +69,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;
> > + int in_i = 1, temp_i = 1, curr_i = 1, power_i = 1, humidity_i = 1;
> > enum iio_chan_type type;
> > struct iio_channel *channels;
> > const char *name = "iio_hwmon";
> > @@ -94,6 +100,12 @@ static int iio_hwmon_probe(struct platform_device *pdev)
> > ret = -ENOMEM;
> > goto error_release_channels;
> > }
> > + st->scale = devm_kzalloc(dev, sizeof(*st->scale) * st->num_channels,
> > + GFP_KERNEL);
> > + if (st->scale == NULL) {
> > + ret = -ENOMEM;
> > + goto error_release_channels;
> > + }
> >
> > for (i = 0; i < st->num_channels; i++) {
> > a = devm_kzalloc(dev, sizeof(*a), GFP_KERNEL);
> > @@ -107,6 +119,7 @@ static int iio_hwmon_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto error_release_channels;
> >
> > + st->scale[i] = 1;
> > switch (type) {
> > case IIO_VOLTAGE:
> > a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
> > @@ -123,6 +136,17 @@ static int iio_hwmon_probe(struct platform_device *pdev)
> > "curr%d_input",
> > curr_i++);
> > break;
> > + case IIO_POWER:
> > + st->scale[i] = 1000;
> > + a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
> > + "power%d_input",
> > + power_i++);
> > + break;
> > + case IIO_HUMIDITYRELATIVE:
> > + a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
> > + "humidity%d_input",
> > + humidity_i++);
> > + break;
> > default:
> > ret = -EINVAL;
> > goto error_release_channels;
> >