[PATCH v2] hwmon: (ntc_thermistor): try reading processed

[PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Linus Walleij]

Before trying the custom method of reading the sensor
as raw and then converting, just use
iio_read_channel_processed() which first tries to
see if the ADC can provide a processed value directly,
else reads raw and applies scaling inside of IIO
using the scale attributes of the ADC. We need to
multiply the scaled value with 1000 to get to
microvolts from millivolts which is what processed
IIO channels returns.

Keep the code that assumes 12bit ADC around as a
fallback.

This gives correct readings on the AB8500 thermistor
inputs used in the Ux500 HREFP520 platform for reading
battery and board temperature.

Cc: Peter Rosin <peda@axentia.se>
Cc: Chris Lesiak <chris.lesiak@licor.com>
Cc: Jonathan Cameron <jic23@cam.ac.uk>
Cc: linux-iio@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Fix the patch to multiply the processed value by
  1000 to get to microvolts from millivolts.
- Fix up the confusion in the commit message.
- Drop pointless comments about the code, we keep the
  original code path around if processed reads don't
  work, nothing bad with that.
---
 drivers/hwmon/ntc_thermistor.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index 3aad62a0e661..c1c02cc454fc 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -326,18 +326,33 @@ struct ntc_data {
 static int ntc_adc_iio_read(struct ntc_thermistor_platform_data *pdata)
 {
 	struct iio_channel *channel = pdata->chan;
-	int raw, uv, ret;
+	int uv, ret;
 
-	ret = iio_read_channel_raw(channel, &raw);
+	ret = iio_read_channel_processed(channel, &uv);
 	if (ret < 0) {
-		pr_err("read channel() error: %d\n", ret);
-		return ret;
-	}
+		int raw;
 
-	ret = iio_convert_raw_to_processed(channel, raw, &uv, 1000);
-	if (ret < 0) {
-		/* Assume 12 bit ADC with vref at pullup_uv */
-		uv = (pdata->pullup_uv * (s64)raw) >> 12;
+		/*
+		 * This fallback uses a raw read and then
+		 * assumes the ADC is 12 bits, scaling with
+		 * a factor 1000 to get to microvolts.
+		 */
+		ret = iio_read_channel_raw(channel, &raw);
+		if (ret < 0) {
+			pr_err("read channel() error: %d\n", ret);
+			return ret;
+		}
+		ret = iio_convert_raw_to_processed(channel, raw, &uv, 1000);
+		if (ret < 0) {
+			/* Assume 12 bit ADC with vref at pullup_uv */
+			uv = (pdata->pullup_uv * (s64)raw) >> 12;
+		}
+	} else {
+		/*
+		 * The processed channel is in millivolts, so scale this
+		 * to microvolts.
+		 */
+		uv *= 1000;
 	}
 
 	return uv;
-- 
2.29.2

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Chris Lesiak]

Please don't use iio_read_channel_processed and convert from milliVolts to
microVolts by multiplying by 1000.  My use case requires the additional
precision that iio_read_channel_raw followed by iio_convert_raw_to_processed
with the 1000X scaler provides.

But I'm unsure about keeping the fallback 12-bit ADC in place.  I kept it so as
not to break Naveen Krishna Chatradhi's use case.  But I'm not sure it still works
after commit adc8ec5ff183d09ae7a9d2dd31125401d302ba63
"iio: inkern: pass through raw values if no scaling".  Before the commit,
iio_convert_raw_to_processed returned a negative number if there was no
scaling available.  Now, it returns the raw value.
Does that mean that the raw value is already scaled to the correct units?
Or does that mean that the scale is unknown and all you get is counts?

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Chris Lesiak]

A closer reading of the "iio: inkern: pass through raw values if no scaling"
commit leads me to believe that even when the sensor provides no scale,
the returned value is expected to be in the correct units.

If that is true, there was a bug in the commit.
It failed to apply the caller supplied scale that ntc_thermistor.c relies on
to convert from milliVolts to microVolts.

Linus, would this change address your original problem?

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index fe30bcb6a57b..79787474d511 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -587,7 +587,7 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
                 * Just pass raw values as processed if no scaling is
                 * available.
                 */
-               *processed = raw;
+               *processed = raw * scale;
                return 0;
        }




From: Chris Lesiak <chris.lesiak@licor.com>
Sent: Wednesday, December 23, 2020 7:39 PM
To: Linus Walleij <linus.walleij@linaro.org>; Jean Delvare <jdelvare@suse.com>; Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org <linux-hwmon@vger.kernel.org>; Peter Rosin <peda@axentia.se>; Jonathan Cameron <jic23@cam.ac.uk>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Subject: Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed 
 
Please don't use iio_read_channel_processed and convert from milliVolts to
microVolts by multiplying by 1000.  My use case requires the additional
precision that iio_read_channel_raw followed by iio_convert_raw_to_processed
with the 1000X scaler provides.

But I'm unsure about keeping the fallback 12-bit ADC in place.  I kept it so as
not to break Naveen Krishna Chatradhi's use case.  But I'm not sure it still works
after commit adc8ec5ff183d09ae7a9d2dd31125401d302ba63
"iio: inkern: pass through raw values if no scaling".  Before the commit,
iio_convert_raw_to_processed returned a negative number if there was no
scaling available.  Now, it returns the raw value.
Does that mean that the raw value is already scaled to the correct units?
Or does that mean that the scale is unknown and all you get is counts?

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Linus Walleij]

On Thu, Dec 24, 2020 at 4:15 AM Chris Lesiak <chris.lesiak@licor.com> wrote:

> A closer reading of the "iio: inkern: pass through raw values if no scaling"
> commit leads me to believe that even when the sensor provides no scale,
> the returned value is expected to be in the correct units.
>
> If that is true, there was a bug in the commit.
> It failed to apply the caller supplied scale that ntc_thermistor.c relies on
> to convert from milliVolts to microVolts.
>
> Linus, would this change address your original problem?
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index fe30bcb6a57b..79787474d511 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -587,7 +587,7 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
>                  * Just pass raw values as processed if no scaling is
>                  * available.
>                  */
> -               *processed = raw;
> +               *processed = raw * scale;
>                 return 0;
>         }

I do not think the raw values have any obligation to be in
e.g. millivolts. They may be, by chance.

In my case, it is just an unscaled byte [0..255].

So it unfortunately does not solve my problem, because in my driver,
that does not support scaling, the result will be the raw value
* 1000 which is not going to be microvolts at all, because the
raw values are not millivolts, rather a value 0..255.

However if I used the processed values, the value will be
adjusted non-linearly to millivolts.

Yours,
Linus Walleij

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Linus Walleij]

On Thu, Dec 24, 2020 at 2:39 AM Chris Lesiak <chris.lesiak@licor.com> wrote:

> Please don't use iio_read_channel_processed and convert from milliVolts to
> microVolts by multiplying by 1000.  My use case requires the additional
> precision that iio_read_channel_raw followed by iio_convert_raw_to_processed
> with the 1000X scaler provides.

I have to do this change because my ADC driver only provides processed
channels (drivers/iio/adc/ab8500-gpadc.c). It provides raw values and
it provides processed values but no scale. That means your code will
not work, sadly. It will result in the raw value being used without scaling.

The reason that the ADC cannot provide scaling is that the scale is not
linear and based on calibration. IIO scaling is only linear.

After this change the driver will use the processed values directly if possible,
since these are in millivolts they need to be multiplied by 1000.

Notice that actually this NTC driver is the only driver in the entire
kernel that uses iio_convert_raw_to_processed(). (Well lmp91000.c
calls it to convert its own raw value to a processed one, so will
result in a recursive call.) I kind of find the call
dubious outside of IIO itself, it feels like calling
iio_read_channel_processed() is more
natural? Who outside of IIO needs the raw value really?
It's what I used in all of my drivers.

> But I'm unsure about keeping the fallback 12-bit ADC in place.  I kept it so as
> not to break Naveen Krishna Chatradhi's use case.  But I'm not sure it still works
> after commit adc8ec5ff183d09ae7a9d2dd31125401d302ba63
> "iio: inkern: pass through raw values if no scaling".  Before the commit,
> iio_convert_raw_to_processed returned a negative number if there was no
> scaling available.  Now, it returns the raw value.
> Does that mean that the raw value is already scaled to the correct units?
> Or does that mean that the scale is unknown and all you get is counts?

As far as I can tell it is the former of these two, as you point out
in your second mail.

Yours,
Linus Walleij

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Chris Lesiak]

Linus Walleij <linus.walleij@linaro.org>wrote:
> I have to do this change because my ADC driver only provides processed
> channels (drivers/iio/adc/ab8500-gpadc.c). It provides raw values and
> it provides processed values but no scale. That means your code will
> not work, sadly. It will result in the raw value being used without scaling.

> The reason that the ADC cannot provide scaling is that the scale is not
> linear and based on calibration. IIO scaling is only linear.

I haven't been able to find detailed documentation on the ab8500-gpadc,
so I have a couple of questions / comments:

1. The driver appears to support temperature output directly.  Why do
you need ntc_thermistor?

2. I don't understand how the ab8500_gpadc_read_raw output of processed
data could possibly be correct.

        if (mask == IIO_CHAN_INFO_PROCESSED) {
                processed = ab8500_gpadc_ad_to_voltage(gpadc, ch->id, raw_val);
                if (processed < 0)
                        return processed;

                /* Return millivolt or milliamps or millicentigrades */
                *val = processed * 1000;
                return IIO_VAL_INT;
        }

Note that both processed and *val are both of type int.

If *val really does end up with milliVolt units, then processed must
have had Volt units.  And you only have single Volt resolution.

Either you are working with a lot higher voltages than I usually see,
or something must be wrong.

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Linus Walleij]

On Sat, Dec 26, 2020 at 2:45 AM Chris Lesiak <chris.lesiak@licor.com> wrote:

> I haven't been able to find detailed documentation on the ab8500-gpadc,
> so I have a couple of questions / comments:

The documentation is available here:
https://web.archive.org/web/20130614115108/http://www.stericsson.com/developers/CD00291561_UM1031_AB8500_user_manual-rev5_CTDS_public.pdf

As it is in the wayback machine I put a copy here:
https://dflund.se/~triad/krad/CD00291561_UM1031_AB8500_user_manual-rev5_CTDS_public.pdf

As with many mixsig products the ADC isn't very well documented.
The code is the documentation...

> 1. The driver appears to support temperature output directly.  Why do
> you need ntc_thermistor?

Actually these channels (if you mean AB8500_GPADC_CHAN_BAT_TEMP,
AB8505_GPADC_CHAN_DIE_TEMP, AB8500_GPADC_CHAN_XTAL_TEMP)
are just voltages and they need to be processed because they are
some kind of thermistors and not temperatures at all, these are
voltages. It's just that the channels are named like this.

However in this case (the current patch), the two channels used for the
thermistors are
AB8500_GPADC_CHAN_ADC_AUX_1
AB8500_GPADC_CHAN_ADC_AUX_2
which are just common arbitrary voltage ADCs, not related to
the above, so it doesn't really apply anyways.

> 2. I don't understand how the ab8500_gpadc_read_raw output of processed
> data could possibly be correct.
>
>         if (mask == IIO_CHAN_INFO_PROCESSED) {
>                 processed = ab8500_gpadc_ad_to_voltage(gpadc, ch->id, raw_val);
>                 if (processed < 0)
>                         return processed;
>
>                 /* Return millivolt or milliamps or millicentigrades */
>                 *val = processed * 1000;
>                 return IIO_VAL_INT;
>         }
>
> Note that both processed and *val are both of type int.
>
> If *val really does end up with milliVolt units, then processed must
> have had Volt units.  And you only have single Volt resolution.

Sorry there is a bug there and I sent a patch the other day:
https://lore.kernel.org/linux-iio/20201224011700.1059659-1-linus.walleij@linaro.org/T/#u

"processed" contains millivolts, the * 1000 is a bug. This is why
reading the processed channel without multiplying with 1000
was working for me before, then I discovered that the contract
of the API is to pass millivolts and then I fixed this bug.

Sorry about the confusion :/

> Either you are working with a lot higher voltages than I usually see,
> or something must be wrong.

Yeah something was wrong. Fixed with the patch.

But do you agree with the general stance that we should
give precedence to using iio_read_channel_processed()
and multiply the result with 1000? It should work with
any driver I think.

Yours,
Linus Walleij

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Chris Lesiak]

Linus Walleij <linus.walleij@linaro.org> wrote:
> But do you agree with the general stance that we should
> give precedence to using iio_read_channel_processed()
> and multiply the result with 1000? It should work with
> any driver I think.

In order to preserve resolution of the microVolt value for existing code,
I'd prefer a solution similar to the existing implementation of
iio_read_channel_processed.   Sans error handling:

int uv;
if (iio_channel_has_info(chan->channel, IIO_CHAN_INFO_PROCESSED)) {
        int mv;
        iio_read_channel_processed(channel, &mv);
        uv = 1000 * mv;
} else {
        int raw;
        iio_read_channel_raw(channel, &raw);
        iio_convert_raw_to_processed(channel, raw, &uv, 1000);
}
return uv;

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Linus Walleij]

On Sun, Dec 27, 2020 at 7:54 PM Chris Lesiak <chris.lesiak@licor.com> wrote:
> Linus Walleij <linus.walleij@linaro.org> wrote:
> > But do you agree with the general stance that we should
> > give precedence to using iio_read_channel_processed()
> > and multiply the result with 1000? It should work with
> > any driver I think.
>
> In order to preserve resolution of the microVolt value for existing code,

Aha you mean that iio_read_channel_processed() loses
precision when converting raw to scaled?

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

Yours,
Linus Walleij

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Chris Lesiak]

Linus Walleij <linus.walleij@linaro.org> 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@licor.com> 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.

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Jonathan Cameron]

On Sun, 27 Dec 2020 22:08:24 +0000
Chris Lesiak <chris.lesiak@licor.com> wrote:

> Linus Walleij <linus.walleij@linaro.org> 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@licor.com> 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).

Jonathan

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Guenter Roeck]

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@licor.com> wrote:
> 
> > Linus Walleij <linus.walleij@linaro.org> 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@licor.com> 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

Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed

[Jonathan Cameron]

On Tue, 29 Dec 2020 08:33:46 -0800
Guenter Roeck <linux@roeck-us.net> 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@licor.com> wrote:
> >   
> > > Linus Walleij <linus.walleij@linaro.org> 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@licor.com> 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