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

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

[Guenter Roeck]

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>
---
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.

 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;
-- 
1.9.1

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

[Jonathan Cameron]

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)

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;
> 

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

[Guenter Roeck]

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;
> > 

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

[Jonathan Cameron]

On 29/09/14 20:13, Guenter Roeck wrote:
> 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.
It's per type unfortunately.  Humidity is indeed in milli-units
but power is probably in micro-units (as per hwmon).
Right now it is undocumented and we only have one user so pretty much
need to go with whatever that does (unless it's really silly).

I haven't had a chance to go datasheet diving to work out what it is
using as yet.
> 
> 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;
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Guenter Roeck]

On 10/04/2014 04:37 AM, Jonathan Cameron wrote:
> On 29/09/14 20:13, Guenter Roeck wrote:
>> 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.
> It's per type unfortunately.  Humidity is indeed in milli-units
> but power is probably in micro-units (as per hwmon).
> Right now it is undocumented and we only have one user so pretty much
> need to go with whatever that does (unless it's really silly).
>
> I haven't had a chance to go datasheet diving to work out what it is
> using as yet.

No problem. I'll simplify the patch and only add support for humidity for now.

Thanks,
Guenter