On Wed, May 22, 2013 at 7:14 PM, Lars-Peter Clausen <lars@xxxxxxxxxx> 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 -- 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
