On Tue, Dec 29, 2020 at 02:25:31PM +0000, Jonathan Cameron wrote: > On Sun, 27 Dec 2020 22:08:24 +0000 > Chris Lesiak <chris.lesiak@xxxxxxxxx> wrote: > > > Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > > Aha you mean that iio_read_channel_processed() loses > > > precision when converting raw to scaled? > > > > Yes. For example, take a 16-bit ADC with 4.096 V reference. > > The native resolution is 62.5 microVolts. > > Yet iio_read_channel_processed() can only give integer milliVolts. > > iio_read_channel_raw() followed by iio_convert_raw_to_processed() > > with a scale of 1000 will preserve more of the native resolution. > > User space can do this by using floating point numbers when > > converting to processed. > > > > You are likely to lose precision for all ADCs greater than about 12-bit. > > > > Chris Lesiak <chris.leisak@xxxxxxxxx> wrote: > > >> I'd prefer a solution similar to the existing implementation of > > >> iio_read_channel_processed. > > > > > That seems like the wrong way to work around a problem in > > > the core. > > > > > If iio_read_channel_processed() loses precision we should > > > fix iio_read_channel_processed() and not try to work around > > > the problem in the consumers. > > > > > It's fine to fix all the consumers in the kernel. > > > > > What about changing the signature of: > > > > > int iio_read_channel_processed(struct iio_channel *chan, int *val) > > > > > to: > > > > > int iio_read_channel_processed(struct iio_channel *chan, int *val, > > > unsigned int scale) > > > > > And just augment all calls to pass 1 except the ntc driver > > > which then passes 1000 in the last argument? > > > > > If Jonathan agrees I can fix a patch to alter all the ~50 > > > call sites like this and include the change to this NTC > > > driver. > > > > That would meet my needs and does address what I think is a > > shortcoming in the existing iio_read_channel_processed interface. > I'm fine with this proposal as well. Makes a lot of sense given > there is no particular reason why another subsystem should want to > convert to IIO base units (here milivolts). > > The only other way I could think of doing it would be to > have iio_read_channel_processed 'return' a pair of integers and > type etc IIO_VAL_INT_PLUS_MICRO much like read_raw etc does inside > the actual drivers. > > It would be a bit clunky to implement however and potentially require > some messy maths in the consumers. > > May want to think about whether we need additional sanity checks for > overflow etc. Seems unlikely we'd hit hit them for voltage, but > we might for some other types of sensor where the base unit is much > smaller (wrt to real world values). > All this makes me wonder if it would be better to add a separate API function (iio_read_channel_processed_scale ?) instead of replacing the existing one. Changing 50+ callers at the same time sounds messy. Guenter
