| Message ID | 20210308100219.2732156-2-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | [1/2,v4] iio: Provide iio_read_channel_processed_scale() API | expand |
I've tested this in a 4.14 based kernel with two different out-of-tree
iio adcs, neither of which supports IIO_CHAN_INFO_PROCESSED.
Tested-by: Chris Lesiak <chris.lesiak@licor.com>
On Mon, Mar 08, 2021 at 11:02:19AM +0100, Linus Walleij wrote: > Before trying the custom method of reading the sensor > as raw and then converting, we want to use > iio_read_channel_processed_scale() 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@kernel.org> > Cc: linux-iio@vger.kernel.org > Link: https://lore.kernel.org/linux-iio/20201224011607.1059534-1-linus.walleij@linaro.org/ > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Guenter Roeck <linux@roeck-us.net> Up to Jonathan to decide if he wants to take both patches. Thanks, Guenter > --- > ChangeLog v3->v4: > - Split out the new iio_read_channel_processed_scale() > API addition to a separate patch. > - My suggestion is to apply both patches to the hwmon > tree. > ChangeLog v2->v3: > - After discussion about v2 we concludes that > iio_read_channel_processed() could loose precision > so we introduce a new API to read processed and > scale. > - Include a link to the v2 discussion for reference. > - For ease of applying to the hwmon tree, keep it all > in one patch. > - This needs Jonathans ACK to be merged through hwmon. > 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 | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c > index 3aad62a0e661..8587189c7f15 100644 > --- a/drivers/hwmon/ntc_thermistor.c > +++ b/drivers/hwmon/ntc_thermistor.c > @@ -326,18 +326,27 @@ 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_scale(channel, &uv, 1000); > 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; > + } > } > > return uv; > -- > 2.29.2 >
On Tue, 9 Mar 2021 09:15:01 -0800 Guenter Roeck <linux@roeck-us.net> wrote: > On Mon, Mar 08, 2021 at 11:02:19AM +0100, Linus Walleij wrote: > > Before trying the custom method of reading the sensor > > as raw and then converting, we want to use > > iio_read_channel_processed_scale() 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@kernel.org> > > Cc: linux-iio@vger.kernel.org > > Link: https://lore.kernel.org/linux-iio/20201224011607.1059534-1-linus.walleij@linaro.org/ > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Acked-by: Guenter Roeck <linux@roeck-us.net> > > Up to Jonathan to decide if he wants to take both patches. It seems unlikely that it will cause trouble in either tree (as the code in question isn't changing that often). So on basis Guenter already acked I'll pick them up through IIO. Applied to the togreg branch of iio.git and pushed out as testing to see if the autobuilders notice anything we missed. thanks, Jonathan > > Thanks, > Guenter > > > --- > > ChangeLog v3->v4: > > - Split out the new iio_read_channel_processed_scale() > > API addition to a separate patch. > > - My suggestion is to apply both patches to the hwmon > > tree. > > ChangeLog v2->v3: > > - After discussion about v2 we concludes that > > iio_read_channel_processed() could loose precision > > so we introduce a new API to read processed and > > scale. > > - Include a link to the v2 discussion for reference. > > - For ease of applying to the hwmon tree, keep it all > > in one patch. > > - This needs Jonathans ACK to be merged through hwmon. > > 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 | 27 ++++++++++++++++++--------- > > 1 file changed, 18 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c > > index 3aad62a0e661..8587189c7f15 100644 > > --- a/drivers/hwmon/ntc_thermistor.c > > +++ b/drivers/hwmon/ntc_thermistor.c > > @@ -326,18 +326,27 @@ 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_scale(channel, &uv, 1000); > > 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; > > + } > > } > > > > return uv; > > -- > > 2.29.2 > >
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c index 3aad62a0e661..8587189c7f15 100644 --- a/drivers/hwmon/ntc_thermistor.c +++ b/drivers/hwmon/ntc_thermistor.c @@ -326,18 +326,27 @@ 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_scale(channel, &uv, 1000); 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; + } } return uv;
Before trying the custom method of reading the sensor as raw and then converting, we want to use iio_read_channel_processed_scale() 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@kernel.org> Cc: linux-iio@vger.kernel.org Link: https://lore.kernel.org/linux-iio/20201224011607.1059534-1-linus.walleij@linaro.org/ Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v3->v4: - Split out the new iio_read_channel_processed_scale() API addition to a separate patch. - My suggestion is to apply both patches to the hwmon tree. ChangeLog v2->v3: - After discussion about v2 we concludes that iio_read_channel_processed() could loose precision so we introduce a new API to read processed and scale. - Include a link to the v2 discussion for reference. - For ease of applying to the hwmon tree, keep it all in one patch. - This needs Jonathans ACK to be merged through hwmon. 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 | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)