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

From: Linus Walleij <linus.walleij@linaro.org>
To: Chris Lesiak <chris.lesiak@licor.com>
Cc: Jean Delvare <jdelvare@suse.com>,
	Guenter Roeck <linux@roeck-us.net>,
	"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
Date: Fri, 25 Dec 2020 23:55:04 +0100	[thread overview]
Message-ID: <CACRpkdZJT386M5y5cyCLYaWDhS+AnpXgB__VEshvYKbxN_=wPg@mail.gmail.com> (raw)
In-Reply-To: <SN6PR08MB556580B228FEC722C4A75B669ADD0@SN6PR08MB5565.namprd08.prod.outlook.com>

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