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@xxxxxxxxxx>
>>> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
