On Tue, 29 Dec 2020 08:33:46 -0800 Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > 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. Agreed - definitely makes more sense to do it that way. Jonathan > > Guenter
