From: Lars-Peter Clausen <lars@metafoo.de>
To: Guillaume Ballet <gballetwork@gmail.com>
Cc: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>,
linux-iio@vger.kernel.org
Subject: Re: Why is only one int returned in iio_read_channel_processed?
Date: Thu, 23 May 2013 12:39:03 +0200 [thread overview]
Message-ID: <519DF1C7.9010604@metafoo.de> (raw)
In-Reply-To: <CAGi_vrgk=x0JmVhqWPFRbdEBiHiURrUBoezKapJd5wj4zUXr7w@mail.gmail.com>
On 05/23/2013 11:52 AM, Guillaume Ballet wrote:
> On Wed, May 22, 2013 at 7:14 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 05/22/2013 05:24 PM, Guillaume Ballet wrote:
>> [...]
>>>>> Hence my need to call iio_read_channel_processed
>>>>> and not entrust anyone else with the conversion.
>>>>
>>>> Ah, ok, so your driver implements IIO_CHAN_INFO_PROCESSED instead of
>>>> IIO_CHAN_INFO_RAW. And you want to be able to specify your value with
>>>> sub-decimal precession, is this correct?
>>>
>>> Absolutely.
>>>
>>>>
>>>>>
>>>>> Could _you_ please explain what your concern with using the same format is?
>>>>
>>>> Because the definition of IIO_CHAN_INFO_PROCESSED is that the value has
>>>> already the proper unit and no unit conversion is necessary.
>>>
>>> Now I see, thanks.
>>>
>>> Getting back to the precision issue, I see that in
>>> iio_convert_raw_to_processed_unlocked() there is the following code:
>>>
>>> case IIO_VAL_INT_PLUS_MICRO:
>>> if (scale_val2 < 0)
>>> *processed = -raw64 * scale_val;
>>> else
>>> *processed = raw64 * scale_val;
>>> *processed += div_s64(raw64 * (s64)scale_val2 * scale,
>>> 1000000LL);
>>> break;
>>> case IIO_VAL_INT_PLUS_NANO:
>>> if (scale_val2 < 0)
>>> *processed = -raw64 * scale_val;
>>> else
>>> *processed = raw64 * scale_val;
>>> *processed += div_s64(raw64 * (s64)scale_val2 * scale,
>>>
>>> with processed being of type int *. So the sub-decimal precision is
>>> indeed lost. Is there a big issue with adapting the code to also
>>> handle sub-decimal precision, then?
>>
>> Well it's not an issue per se, it's just that there are no in kernel users
>> which would be able to make use of this. The iio_convert_raw_to_processed()
>> function takes an additional scale parameter though which allows you to get
>> a value with a high precession. E.g. if you read a voltage channel with
>> scale set to 1000 you'll get the result in micro Volts instead of milli
>> Volts. The same parameter could be added to iio_read_channel_processed() and
>> you'd do similar calculations as in iio_convert_raw_to_processed(). Instead
>> of 'raw64 * scale_val' you'd just use 'val' and instead of 'raw64 *
>> scale_val2' you'd use 'val2'.
>>
>> E.g. for IIO_VAL_INT_PLUS_MICRO:
>> if (val2 < 0)
>> *processed = -val;
>> else
>> *processed = val;
>> *processed += div_s64((s64)val2 * scale, 1000000LL);
>>
>> and so on.
>>
>>
>> And I think for sysfs nodes it should already work fine, e.g. if you return
>> IIO_VAL_INT_PLUS_MICRO.
>>
>
> That makes sense. However, the following reasons make me think passing
> the scale is not the correct way to proceed:
>
> - if IIO_INT_VAL_PLUS_NANO is returned (common when dealing with
> current sources), 32 bits is a bit tight - which is why the read_raw
> function pointer in iio_info has (val, val2) in the first place.
> - People like me who do not use the iio_convert_raw_to_processed
> path() but need to support IIO_CHAN_INFO_PROCESSED directly in their
> driver have an issue: we would need to be passed the scale in the
> read_raw function of iio_info. That would impact _all_ IIO drivers.
IIO_CHAN_INFO_PROCESSED is by definition supposed to return the value in the
proper unit. If that doesn't work for you use IIO_CHAN_INFO_RAW +
IIO_CHAN_INFO_SCALE. Think of IIO_CHAN_INFO_PROCESSED as IIO_CHAN_INFO_RAW
with the scale set to 1.0
> - The scale parameter to iio_convert_raw_to_processed() itself is an
> int, and IIO_CHAN_INFO_SCALE can return a scale in the
> IIO_VAL_INT_PLUS_NANO scheme. It means somewhere along the road,
> precision is lost.
The scale would be passed in by the consumer, so the consumer is able to
specify the amount of precision it wants.
- Lars
next prev parent reply other threads:[~2013-05-23 10:39 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-22 7:49 Why is only one int returned in iio_read_channel_processed? Guillaume Ballet
2013-05-22 8:04 ` Jonathan Cameron
2013-05-22 8:19 ` Lars-Peter Clausen
2013-05-22 9:00 ` Jonathan Cameron
2013-05-22 9:37 ` Guillaume Ballet
2013-05-22 11:43 ` Lars-Peter Clausen
2013-05-22 13:29 ` Guillaume Ballet
2013-05-22 13:39 ` Lars-Peter Clausen
2013-05-22 14:00 ` Guillaume Ballet
2013-05-22 14:15 ` Lars-Peter Clausen
2013-05-22 15:24 ` Guillaume Ballet
2013-05-22 17:14 ` Lars-Peter Clausen
2013-05-23 9:52 ` Guillaume Ballet
2013-05-23 10:39 ` Lars-Peter Clausen [this message]
2013-05-23 13:18 ` Guillaume Ballet
2013-05-23 13:28 ` Lars-Peter Clausen
2013-06-02 16:00 ` Jonathan Cameron
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=519DF1C7.9010604@metafoo.de \
--to=lars@metafoo.de \
--cc=gballetwork@gmail.com \
--cc=jic23@jic23.retrosnub.co.uk \
--cc=linux-iio@vger.kernel.org \
/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).