Why is only one int returned in iio_read_channel_processed?

Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

Hello,

The read_raw operation in iio_info has a signature that takes two
integer as in/out parameters. This lets the driver return values with
micro- or nano-unit precision.

However, the high-level iio_read_channel_raw and
iio_read_channel_processed functions' signature only has one integer
in/out parameter. That makes sense in the context of _raw because the
value isn't yet processed.

However, as the scale is a number encoded over two ints, the
_processed value should also span two ints. Is there a reason why it's
still only one int?

Thanks,
Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Jonathan Cameron]

On 22/05/13 08:49, Guillaume Ballet wrote:
> Hello,
>
> The read_raw operation in iio_info has a signature that takes two
> integer as in/out parameters. This lets the driver return values with
> micro- or nano-unit precision.
>
> However, the high-level iio_read_channel_raw and
> iio_read_channel_processed
Do we even have an iio_read_channel_processed? I think you are referring
to the in kernel consumer interface.  In iio/consumer.h we have an 
iio_read_channel_raw but unless I am going mad no 
iio_read_channel_processed (as of yet).

https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/tree/include/linux/iio/consumer.h?h=togreg
(is the latest development tree).

Which tree are you looking at?

> functions' signature only has one integer
> in/out parameter. That makes sense in the context of _raw because the
> value isn't yet processed.
>
> However, as the scale is a number encoded over two ints, the
> _processed value should also span two ints. Is there a reason why it's
> still only one int?
No it certainly should not be one int for exactly the reasons you have 
stated.
>
> Thanks,
> Guillaume
> --
> 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: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

On 05/22/2013 10:04 AM, Jonathan Cameron wrote:
> On 22/05/13 08:49, Guillaume Ballet wrote:
>> Hello,
>>
>> The read_raw operation in iio_info has a signature that takes two
>> integer as in/out parameters. This lets the driver return values with
>> micro- or nano-unit precision.
>>
>> However, the high-level iio_read_channel_raw and
>> iio_read_channel_processed
> Do we even have an iio_read_channel_processed? I think you are referring
> to the in kernel consumer interface.  In iio/consumer.h we have an
> iio_read_channel_raw but unless I am going mad no iio_read_channel_processed
> (as of yet).
> 
> https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/tree/include/linux/iio/consumer.h?h=togreg
> 
> (is the latest development tree).
> 
> Which tree are you looking at?

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/iio/consumer.h#n145

> 
>> functions' signature only has one integer
>> in/out parameter. That makes sense in the context of _raw because the
>> value isn't yet processed.
>>
>> However, as the scale is a number encoded over two ints, the
>> _processed value should also span two ints. Is there a reason why it's
>> still only one int?
> No it certainly should not be one int for exactly the reasons you have stated.
>>

I'm not to sure about that. I'd rather add a scale parameter to the
iio_read_channel_processed, just in the same way the
convert_raw_to_processed function takes a scale parameter.

- Lars

Re: Why is only one int returned in iio_read_channel_processed?

[Jonathan Cameron]

On 22/05/13 09:19, Lars-Peter Clausen wrote:
> On 05/22/2013 10:04 AM, Jonathan Cameron wrote:
>> On 22/05/13 08:49, Guillaume Ballet wrote:
>>> Hello,
>>>
>>> The read_raw operation in iio_info has a signature that takes two
>>> integer as in/out parameters. This lets the driver return values with
>>> micro- or nano-unit precision.
>>>
>>> However, the high-level iio_read_channel_raw and
>>> iio_read_channel_processed
>> Do we even have an iio_read_channel_processed? I think you are referring
>> to the in kernel consumer interface.  In iio/consumer.h we have an
>> iio_read_channel_raw but unless I am going mad no iio_read_channel_processed
>> (as of yet).
>>
>> https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/tree/include/linux/iio/consumer.h?h=togreg
>>
>> (is the latest development tree).
>>
>> Which tree are you looking at?
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/iio/consumer.h#n145
Weird. I'm clearly going to be doing some checking of why that isn't in
my togreg branch (at least on kernel.org).  I did have some strange 
problems pushing the other day. I wonder if that branch has gotten into
a weird state.

>
>>
>>> functions' signature only has one integer
>>> in/out parameter. That makes sense in the context of _raw because the
>>> value isn't yet processed.
>>>
>>> However, as the scale is a number encoded over two ints, the
>>> _processed value should also span two ints. Is there a reason why it's
>>> still only one int?
>> No it certainly should not be one int for exactly the reasons you have stated.
>>>
>
> I'm not to sure about that. I'd rather add a scale parameter to the
> iio_read_channel_processed, just in the same way the
> convert_raw_to_processed function takes a scale parameter.
That may be tricky to do given we often have nasty non linear functions
that are the reason we are using processed in the first place.  Hmm.
Not sure which way works better.
>
> - Lars
>
> --
> 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: Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

>>
>>>
>>>> functions' signature only has one integer
>>>> in/out parameter. That makes sense in the context of _raw because the
>>>> value isn't yet processed.
>>>>
>>>> However, as the scale is a number encoded over two ints, the
>>>> _processed value should also span two ints. Is there a reason why it's
>>>> still only one int?
>>>
>>> No it certainly should not be one int for exactly the reasons you have
>>> stated.
>>>>
>>>>
>>
>> I'm not to sure about that. I'd rather add a scale parameter to the
>> iio_read_channel_processed, just in the same way the
>> convert_raw_to_processed function takes a scale parameter.
>
> That may be tricky to do given we often have nasty non linear functions
> that are the reason we are using processed in the first place.  Hmm.
> Not sure which way works better.

I agree, this is the whole point of using processed. Lars, is there a
specific reason why you want to keep reading the value and the scale
in different function calls?

Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

On 05/22/2013 11:37 AM, Guillaume Ballet wrote:
>>>
>>>>
>>>>> functions' signature only has one integer
>>>>> in/out parameter. That makes sense in the context of _raw because the
>>>>> value isn't yet processed.
>>>>>
>>>>> However, as the scale is a number encoded over two ints, the
>>>>> _processed value should also span two ints. Is there a reason why it's
>>>>> still only one int?
>>>>
>>>> No it certainly should not be one int for exactly the reasons you have
>>>> stated.
>>>>>
>>>>>
>>>
>>> I'm not to sure about that. I'd rather add a scale parameter to the
>>> iio_read_channel_processed, just in the same way the
>>> convert_raw_to_processed function takes a scale parameter.
>>
>> That may be tricky to do given we often have nasty non linear functions
>> that are the reason we are using processed in the first place.  Hmm.
>> Not sure which way works better.
> 
> I agree, this is the whole point of using processed. Lars, is there a
> specific reason why you want to keep reading the value and the scale
> in different function calls?

I don't want to keep reading scale and value in different function calls.

What's you use case and how do you want to split the data between the two
integers?

- Lars

Re: Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

On Wed, May 22, 2013 at 1:43 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 05/22/2013 11:37 AM, Guillaume Ballet wrote:
>>>>
>>>>>
>>>>>> functions' signature only has one integer
>>>>>> in/out parameter. That makes sense in the context of _raw because the
>>>>>> value isn't yet processed.
>>>>>>
>>>>>> However, as the scale is a number encoded over two ints, the
>>>>>> _processed value should also span two ints. Is there a reason why it's
>>>>>> still only one int?
>>>>>
>>>>> No it certainly should not be one int for exactly the reasons you have
>>>>> stated.
>>>>>>
>>>>>>
>>>>
>>>> I'm not to sure about that. I'd rather add a scale parameter to the
>>>> iio_read_channel_processed, just in the same way the
>>>> convert_raw_to_processed function takes a scale parameter.
>>>
>>> That may be tricky to do given we often have nasty non linear functions
>>> that are the reason we are using processed in the first place.  Hmm.
>>> Not sure which way works better.
>>
>> I agree, this is the whole point of using processed. Lars, is there a
>> specific reason why you want to keep reading the value and the scale
>> in different function calls?
>
> I don't want to keep reading scale and value in different function calls.
>
> What's you use case and how do you want to split the data between the two
> integers?

Since the scale's format can be IIO_VAL_INT_PLUS_MICRO and
IIO_VAL_INT_PLUS_NANO, etc... and since iio_read_channel_processed
returns a value that is homogeneous to (value * scale), it seems to me
the same format should be used as when calling iio_read_channel_raw
with IIO_CHAN_INFO_SCALE. My use case is not more complicated than
making sure I keep the same precision when getting processed values.

Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

On 05/22/2013 03:29 PM, Guillaume Ballet wrote:
> On Wed, May 22, 2013 at 1:43 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 05/22/2013 11:37 AM, Guillaume Ballet wrote:
>>>>>
>>>>>>
>>>>>>> functions' signature only has one integer
>>>>>>> in/out parameter. That makes sense in the context of _raw because the
>>>>>>> value isn't yet processed.
>>>>>>>
>>>>>>> However, as the scale is a number encoded over two ints, the
>>>>>>> _processed value should also span two ints. Is there a reason why it's
>>>>>>> still only one int?
>>>>>>
>>>>>> No it certainly should not be one int for exactly the reasons you have
>>>>>> stated.
>>>>>>>
>>>>>>>
>>>>>
>>>>> I'm not to sure about that. I'd rather add a scale parameter to the
>>>>> iio_read_channel_processed, just in the same way the
>>>>> convert_raw_to_processed function takes a scale parameter.
>>>>
>>>> That may be tricky to do given we often have nasty non linear functions
>>>> that are the reason we are using processed in the first place.  Hmm.
>>>> Not sure which way works better.
>>>
>>> I agree, this is the whole point of using processed. Lars, is there a
>>> specific reason why you want to keep reading the value and the scale
>>> in different function calls?
>>
>> I don't want to keep reading scale and value in different function calls.
>>
>> What's you use case and how do you want to split the data between the two
>> integers?
> 
> Since the scale's format can be IIO_VAL_INT_PLUS_MICRO and
> IIO_VAL_INT_PLUS_NANO, etc... and since iio_read_channel_processed
> returns a value that is homogeneous to (value * scale), it seems to me
> the same format should be used as when calling iio_read_channel_raw
> with IIO_CHAN_INFO_SCALE. My use case is not more complicated than
> making sure I keep the same precision when getting processed values.

That doesn't really help me understand what you are trying to do. What is
your application, where do you call iio_read_channel_processed and how do
you process the returned value.

- Lars

Re: Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

On Wed, May 22, 2013 at 3:39 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 05/22/2013 03:29 PM, Guillaume Ballet wrote:
>> On Wed, May 22, 2013 at 1:43 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> On 05/22/2013 11:37 AM, Guillaume Ballet wrote:
>>>>>>
>>>>>>>
>>>>>>>> functions' signature only has one integer
>>>>>>>> in/out parameter. That makes sense in the context of _raw because the
>>>>>>>> value isn't yet processed.
>>>>>>>>
>>>>>>>> However, as the scale is a number encoded over two ints, the
>>>>>>>> _processed value should also span two ints. Is there a reason why it's
>>>>>>>> still only one int?
>>>>>>>
>>>>>>> No it certainly should not be one int for exactly the reasons you have
>>>>>>> stated.
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> I'm not to sure about that. I'd rather add a scale parameter to the
>>>>>> iio_read_channel_processed, just in the same way the
>>>>>> convert_raw_to_processed function takes a scale parameter.
>>>>>
>>>>> That may be tricky to do given we often have nasty non linear functions
>>>>> that are the reason we are using processed in the first place.  Hmm.
>>>>> Not sure which way works better.
>>>>
>>>> I agree, this is the whole point of using processed. Lars, is there a
>>>> specific reason why you want to keep reading the value and the scale
>>>> in different function calls?
>>>
>>> I don't want to keep reading scale and value in different function calls.
>>>
>>> What's you use case and how do you want to split the data between the two
>>> integers?
>>
>> Since the scale's format can be IIO_VAL_INT_PLUS_MICRO and
>> IIO_VAL_INT_PLUS_NANO, etc... and since iio_read_channel_processed
>> returns a value that is homogeneous to (value * scale), it seems to me
>> the same format should be used as when calling iio_read_channel_raw
>> with IIO_CHAN_INFO_SCALE. My use case is not more complicated than
>> making sure I keep the same precision when getting processed values.
>
> That doesn't really help me understand what you are trying to do. What is
> your application, where do you call iio_read_channel_processed and how do
> you process the returned value.

I have a driver for an ADC block that measures miscellaneous values in
various units (temperature, voltage, current...) to be read from
drivers in other kernel subsystems (the power supply class, for
instance). Unit conversion has to be done by the IIO device itself as
it's done using tables that are provided by various vendors who don't
want them published. Hence my need to call iio_read_channel_processed
and not entrust anyone else with the conversion.

Could _you_ please explain what your concern with using the same format is?

Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

On 05/22/2013 04:00 PM, Guillaume Ballet wrote:
> On Wed, May 22, 2013 at 3:39 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 05/22/2013 03:29 PM, Guillaume Ballet wrote:
>>> On Wed, May 22, 2013 at 1:43 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>> On 05/22/2013 11:37 AM, Guillaume Ballet wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>> functions' signature only has one integer
>>>>>>>>> in/out parameter. That makes sense in the context of _raw because the
>>>>>>>>> value isn't yet processed.
>>>>>>>>>
>>>>>>>>> However, as the scale is a number encoded over two ints, the
>>>>>>>>> _processed value should also span two ints. Is there a reason why it's
>>>>>>>>> still only one int?
>>>>>>>>
>>>>>>>> No it certainly should not be one int for exactly the reasons you have
>>>>>>>> stated.
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>> I'm not to sure about that. I'd rather add a scale parameter to the
>>>>>>> iio_read_channel_processed, just in the same way the
>>>>>>> convert_raw_to_processed function takes a scale parameter.
>>>>>>
>>>>>> That may be tricky to do given we often have nasty non linear functions
>>>>>> that are the reason we are using processed in the first place.  Hmm.
>>>>>> Not sure which way works better.
>>>>>
>>>>> I agree, this is the whole point of using processed. Lars, is there a
>>>>> specific reason why you want to keep reading the value and the scale
>>>>> in different function calls?
>>>>
>>>> I don't want to keep reading scale and value in different function calls.
>>>>
>>>> What's you use case and how do you want to split the data between the two
>>>> integers?
>>>
>>> Since the scale's format can be IIO_VAL_INT_PLUS_MICRO and
>>> IIO_VAL_INT_PLUS_NANO, etc... and since iio_read_channel_processed
>>> returns a value that is homogeneous to (value * scale), it seems to me
>>> the same format should be used as when calling iio_read_channel_raw
>>> with IIO_CHAN_INFO_SCALE. My use case is not more complicated than
>>> making sure I keep the same precision when getting processed values.
>>
>> That doesn't really help me understand what you are trying to do. What is
>> your application, where do you call iio_read_channel_processed and how do
>> you process the returned value.
> 
> I have a driver for an ADC block that measures miscellaneous values in
> various units (temperature, voltage, current...) to be read from
> drivers in other kernel subsystems (the power supply class, for
> instance).

This already works today with the current implementation of
iio_read_channel_processed.

Unit conversion has to be done by the IIO device itself as
> it's done using tables that are provided by various vendors who don't
> want them published.

So how do you include the tables in the IIO driver if they can't be published?

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

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

- Lars

Re: Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

>> I have a driver for an ADC block that measures miscellaneous values in
>> various units (temperature, voltage, current...) to be read from
>> drivers in other kernel subsystems (the power supply class, for
>> instance).
>
> This already works today with the current implementation of
> iio_read_channel_processed.

Indeed.

>
> Unit conversion has to be done by the IIO device itself as
>> it's done using tables that are provided by various vendors who don't
>> want them published.
>
> So how do you include the tables in the IIO driver if they can't be published?

I will load them as a blob from the device tree. It doesn't need to
have a very high level of protection - I just need to ensure that
those values aren't hard-coded in a file under the GPL.

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

Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

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.

- Lars

Re: Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

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

Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

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

Re: Why is only one int returned in iio_read_channel_processed?

[Guillaume Ballet]

>>
>> - 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_RA=
W
> with the scale set to 1.0

This isn't a unit problem, this is a precision problem: if I want to
return a current in Amp=E8res, for instance 5.000000001, I can't get
that by calling iio_read_channel_processed() (or
iio_read_channel_raw() for that matter) as the precision is too big to
fit in only one integer. The issue is that the callback that handles
IIO_CHAN_INFO_PROCESSED and IIO_CHAN_INFO_RAW does allow to return
such a value. There's an inconsistency in the interface.

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

Not if they want a precision as high as the IIO driver is able to
deliver: the scale returned by a IIO_CHAN_INFO_SCALE is a 64-bits
fixed point integer. The scaled passed to
iio_convert_raw_to_processed() is a 32 bit integer. If one needs great
precision on big numbers, it won't fit.

Guillaume

Re: Why is only one int returned in iio_read_channel_processed?

[Lars-Peter Clausen]

On 05/23/2013 03:18 PM, Guillaume Ballet wrote:
>>>
>>> - 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
> 
> This isn't a unit problem, this is a precision problem: if I want to
> return a current in Ampères, for instance 5.000000001, I can't get
> that by calling iio_read_channel_processed() (or
> iio_read_channel_raw() for that matter) as the precision is too big to
> fit in only one integer. The issue is that the callback that handles
> IIO_CHAN_INFO_PROCESSED and IIO_CHAN_INFO_RAW does allow to return
> such a value. There's an inconsistency in the interface.

I doubt anybody actually cares about the 0.000000001 in that case.

> 
>>
>>> - 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.
> 
> Not if they want a precision as high as the IIO driver is able to
> deliver: the scale returned by a IIO_CHAN_INFO_SCALE is a 64-bits
> fixed point integer. The scaled passed to
> iio_convert_raw_to_processed() is a 32 bit integer. If one needs great
> precision on big numbers, it won't fit.

The problem is that there is no in kernel user who can actually make use of
anything but a 32bit integer. If we need a larger range we should change the
return type to a 64bit integer rather than adopting the val1, val2 scheme
for the in-kernel API.

- Lars

Re: Why is only one int returned in iio_read_channel_processed?

[Jonathan Cameron]

On 05/23/2013 02:28 PM, Lars-Peter Clausen wrote:
> On 05/23/2013 03:18 PM, Guillaume Ballet wrote:
>>>>
>>>> - 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
>>
>> This isn't a unit problem, this is a precision problem: if I want to
>> return a current in Ampères, for instance 5.000000001, I can't get
>> that by calling iio_read_channel_processed() (or
>> iio_read_channel_raw() for that matter) as the precision is too big to
>> fit in only one integer. The issue is that the callback that handles
>> IIO_CHAN_INFO_PROCESSED and IIO_CHAN_INFO_RAW does allow to return
>> such a value. There's an inconsistency in the interface.
> 
> I doubt anybody actually cares about the 0.000000001 in that case.
> 
>>
>>>
>>>> - 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.
>>
>> Not if they want a precision as high as the IIO driver is able to
>> deliver: the scale returned by a IIO_CHAN_INFO_SCALE is a 64-bits
>> fixed point integer. The scaled passed to
>> iio_convert_raw_to_processed() is a 32 bit integer. If one needs great
>> precision on big numbers, it won't fit.
> 
> The problem is that there is no in kernel user who can actually make use of
> anything but a 32bit integer. If we need a larger range we should change the
> return type to a 64bit integer rather than adopting the val1, val2 scheme
> for the in-kernel API.

Just to make things clear....

If an in kernel user were proposed that actually did make good use
of this it might be a different matter. It's just that no one has suggested
one as yet. I can contrive of complex cases where it might make sense, but
until we have a real world usecase, let us leave this.
> 
> - Lars
> --
> 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
>