[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220227125559.72d5d79a@jic23-huawei>
Date: Sun, 27 Feb 2022 12:55:59 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Liam Beguin <liambeguin@...il.com>
Cc: peda@...ntia.se, andy.shevchenko@...il.com, lars@...afoo.de,
linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, robh+dt@...nel.org
Subject: Re: [PATCH v15 00/10] iio: afe: add temperature rescaling support
On Sat, 12 Feb 2022 21:57:29 -0500
Liam Beguin <liambeguin@...il.com> wrote:
> Jonathan, Peter, Andy,
>
> This series focuses on adding temperature rescaling support to the IIO
> Analog Front End (AFE) driver.
>
> The main changes to the AFE driver include an initial Kunit test suite,
> support for IIO_VAL_INT_PLUS_{NANO,MICRO} scales, and support for RTDs
> and temperature transducer sensors.
>
> Thanks for your time,
> Liam
Hi Liam,
I was waiting for Andy to reply to this. Took a quick look back at
what was outstanding and realised he had given a
Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
for v13.
I'm assuming there wasn't a strong reason to drop that in the meantime
and it's a simple omission / crossed emails issue.
As such,
Series applied to the togreg branch of iio.git and pushed out
as testing to get some build coverage from 0-day.
Thanks,
Jonathan
>
> Changes since v14:
> - Revert units.h changes in favor of "raw" values
>
> Changes since v13:
> - fix SI prefix in rescale_temp_sense_rtd_props()
> - add comment explaining SI prefixes are sometimes used as mathematical
> multipliers with no particular physical meaning associated.
>
> Changes since v12:
> - rebase on latest testing branch
> - fix copyright holder in newly created header file
> - add myself as a copyright holder of the iio-rescale.c driver at
> Peter's suggestion
> - fix undefined behavior on left-shift operation
>
> Changes since v11:
> - update commits with my personal email since all this work was done on
> my own time
> - apply Peter's Reviewed-by to my local tree
> - fix use of units.h
> - make use of units.h more consistently in iio-rescale.c and in the
> tests
> - fix #include ordering
> - treat 04/16 as a fix. Move it, and add a Fixes: tag
> - fix undefined behavior on left-shift operation
> - add comment about fract_mult with iio_str_to_fixpoint()
> - reword commit message for 14/16, based on Andy's comments
>
> Changes since v10:
> - apply Andy's suggestion for offset calculations
> - make use of units.h more consistently
>
> Changes since v9:
> - make use of linux/units.h
> - reorder commits, fix fract_log2 before merging fract
> - keep fractional representation when not overflowing
>
> Changes since v8:
> - reword comment
> - fix erroneous 64-bit division
> - optimize and use 32-bit divisions when values are know to not overflow
> - keep IIO_VAL_FRACTIONAL scale when possible, if not default to fixed
> point
> - add test cases
> - use nano precision in test cases
> - simplify offset calculation in rtd_props()
>
> Changes since v7:
> - drop gcd() logic in rescale_process_scale()
> - use div_s64() instead of do_div() for signed 64-bit divisions
> - combine IIO_VAL_FRACTIONAL and IIO_VAL_FRACTIONAL_LOG2 scale cases
> - switch to INT_PLUS_NANO when accuracy is lost with FRACTIONAL scales
> - rework test logic to allow for small relative error
> - rename test variables to align error output messages
>
> Changes since v6:
> - rework IIO_VAL_INT_PLUS_{NANO,MICRO} based on Peter's suggestion
> - combine IIO_VAL_INT_PLUS_{NANO,MICRO} cases
> - add test cases for negative IIO_VAL_INT_PLUS_{NANO,MICRO} corner cases
> - force use of positive integers with gcd()
> - reduce risk of integer overflow in IIO_VAL_FRACTIONAL_LOG2
> - fix duplicate symbol build error
> - apply Reviewed-by
>
> Changes since v5:
> - add include/linux/iio/afe/rescale.h
> - expose functions use to process scale and offset
> - add basic iio-rescale kunit test cases
> - fix integer overflow case
> - improve precision for IIO_VAL_FRACTIONAL_LOG2
>
> Changes since v4:
> - only use gcd() when necessary in overflow mitigation
> - fix INT_PLUS_{MICRO,NANO} support
> - apply Reviewed-by
> - fix temperature-transducer bindings
>
> Changes since v3:
> - drop unnecessary fallthrough statements
> - drop redundant local variables in some calculations
> - fix s64 divisions on 32bit platforms by using do_div
> - add comment describing iio-rescaler offset calculation
> - drop unnecessary MAINTAINERS entry
>
> Changes since v2:
> - don't break implicit offset truncations
> - make a best effort to get a valid value for fractional types
> - drop return value change in iio_convert_raw_to_processed_unlocked()
> - don't rely on processed value for offset calculation
> - add INT_PLUS_{MICRO,NANO} support in iio-rescale
> - revert generic implementation in favor of temperature-sense-rtd and
> temperature-transducer
> - add separate section to MAINTAINERS file
>
> Changes since v1:
> - rebase on latest iio `testing` branch
> - also apply consumer scale on integer channel scale types
> - don't break implicit truncation in processed channel offset
> calculation
> - drop temperature AFE flavors in favor of a simpler generic
> implementation
>
> Liam Beguin (10):
> iio: afe: rescale: expose scale processing function
> iio: afe: rescale: add INT_PLUS_{MICRO,NANO} support
> iio: afe: rescale: add offset support
> iio: afe: rescale: fix accuracy for small fractional scales
> iio: afe: rescale: reduce risk of integer overflow
> iio: test: add basic tests for the iio-rescale driver
> iio: afe: rescale: add RTD temperature sensor support
> iio: afe: rescale: add temperature transducers
> dt-bindings: iio: afe: add bindings for temperature-sense-rtd
> dt-bindings: iio: afe: add bindings for temperature transducers
>
> .../iio/afe/temperature-sense-rtd.yaml | 101 +++
> .../iio/afe/temperature-transducer.yaml | 114 +++
> drivers/iio/afe/iio-rescale.c | 283 ++++++-
> drivers/iio/test/Kconfig | 10 +
> drivers/iio/test/Makefile | 1 +
> drivers/iio/test/iio-test-rescale.c | 710 ++++++++++++++++++
> include/linux/iio/afe/rescale.h | 36 +
> 7 files changed, 1220 insertions(+), 35 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-transducer.yaml
> create mode 100644 drivers/iio/test/iio-test-rescale.c
> create mode 100644 include/linux/iio/afe/rescale.h
>
> Range-diff against v14:
> -: ------------ > 1: ee26b0eeac65 iio: afe: rescale: expose scale processing function
> 1: a510097c83f1 ! 2: 78f9d37575a5 iio: afe: rescale: add INT_PLUS_{MICRO,NANO} support
> @@ Commit message
> Reviewed-by: Peter Rosin <peda@...ntia.se>
>
> ## drivers/iio/afe/iio-rescale.c ##
> -@@
> - #include <linux/of_device.h>
> - #include <linux/platform_device.h>
> - #include <linux/property.h>
> -+#include <linux/units.h>
> -
> - #include <linux/iio/afe/rescale.h>
> - #include <linux/iio/consumer.h>
> @@ drivers/iio/afe/iio-rescale.c: int rescale_process_scale(struct rescale *rescale, int scale_type,
> int *val, int *val2)
> {
> @@ drivers/iio/afe/iio-rescale.c: int rescale_process_scale(struct rescale *rescale
> + return scale_type;
> + case IIO_VAL_INT_PLUS_NANO:
> + case IIO_VAL_INT_PLUS_MICRO:
> -+ mult = scale_type == IIO_VAL_INT_PLUS_NANO ? GIGA : MEGA;
> ++ mult = scale_type == IIO_VAL_INT_PLUS_NANO ? 1000000000L : 1000000L;
> +
> + /*
> + * For IIO_VAL_INT_PLUS_{MICRO,NANO} scale types if either *val
> 2: 8f2f2699a9b4 ! 3: 5be82bd72453 iio: afe: rescale: add offset support
> @@ drivers/iio/afe/iio-rescale.c: int rescale_process_scale(struct rescale *rescale
> + *val = div_s64(tmp, scale) + schan_off;
> + return IIO_VAL_INT;
> + case IIO_VAL_INT_PLUS_NANO:
> -+ tmp = (s64)rescale->offset * GIGA;
> -+ tmp2 = ((s64)scale * GIGA) + scale2;
> ++ tmp = (s64)rescale->offset * 1000000000LL;
> ++ tmp2 = ((s64)scale * 1000000000LL) + scale2;
> + *val = div64_s64(tmp, tmp2) + schan_off;
> + return IIO_VAL_INT;
> + case IIO_VAL_INT_PLUS_MICRO:
> -+ tmp = (s64)rescale->offset * MEGA;
> -+ tmp2 = ((s64)scale * MEGA) + scale2;
> ++ tmp = (s64)rescale->offset * 1000000LL;
> ++ tmp2 = ((s64)scale * 1000000LL) + scale2;
> + *val = div64_s64(tmp, tmp2) + schan_off;
> + return IIO_VAL_INT;
> + default:
> 3: 2efa970bad26 ! 4: 95ec184759f6 iio: afe: rescale: fix accuracy for small fractional scales
> @@ drivers/iio/afe/iio-rescale.c: int rescale_process_scale(struct rescale *rescale
> + return IIO_VAL_INT_PLUS_NANO;
> case IIO_VAL_INT_PLUS_NANO:
> case IIO_VAL_INT_PLUS_MICRO:
> - mult = scale_type == IIO_VAL_INT_PLUS_NANO ? GIGA : MEGA;
> + mult = scale_type == IIO_VAL_INT_PLUS_NANO ? 1000000000L : 1000000L;
> 4: 201037c0ead8 = 5: 2e1d41ef69d9 iio: afe: rescale: reduce risk of integer overflow
> 5: a0037cc3ee90 < -: ------------ iio: afe: rescale: make use of units.h
> 6: f8d47728f482 ! 6: 0b6c029dea1d iio: test: add basic tests for the iio-rescale driver
> @@ drivers/iio/test/iio-test-rescale.c (new)
> +
> +#include <linux/gcd.h>
> +#include <linux/overflow.h>
> -+#include <linux/units.h>
> +
> +#include <linux/iio/afe/rescale.h>
> +#include <linux/iio/iio.h>
> @@ drivers/iio/test/iio-test-rescale.c (new)
> + if (tmp < 0)
> + tmp2 *= -1;
> +
> -+ *nano = (s64)tmp * GIGA + tmp2;
> ++ *nano = (s64)tmp * 1000000000UL + tmp2;
> +
> + return ret;
> +}
> @@ drivers/iio/test/iio-test-rescale.c (new)
> + return -EINVAL;
> + }
> +
> -+ err = MEGA * abs(exp - real);
> ++ err = 1000000UL * abs(exp - real);
> +
> + return (int)div64_u64(err, abs(exp));
> +}
> 7: a04685586340 ! 7: 951ea44d0f5c iio: afe: rescale: add RTD temperature sensor support
> @@ drivers/iio/afe/iio-rescale.c: static int rescale_voltage_divider_props(struct d
> + return ret;
> + }
> +
> -+ tmp = r0 * iexc * alpha / MEGA;
> -+ factor = gcd(tmp, MEGA);
> -+ rescale->numerator = MEGA / factor;
> ++ tmp = r0 * iexc * alpha / 1000000;
> ++ factor = gcd(tmp, 1000000);
> ++ rescale->numerator = 1000000 / factor;
> + rescale->denominator = tmp / factor;
> +
> -+ rescale->offset = -1 * ((r0 * iexc) / KILO);
> ++ rescale->offset = -1 * ((r0 * iexc) / 1000);
> +
> + return 0;
> +}
> 8: e3b716aaee50 ! 8: 56516fdc67bf iio: afe: rescale: add temperature transducers
> @@ drivers/iio/afe/iio-rescale.c: static int rescale_temp_sense_rtd_props(struct de
> + return ret;
> + }
> +
> -+ rescale->numerator = MEGA;
> ++ rescale->numerator = 1000000;
> + rescale->denominator = alpha * sense;
> +
> + rescale->offset = div_s64((s64)offset * rescale->denominator,
> 9: 22ae1458eb8b = 9: 8c409050990b dt-bindings: iio: afe: add bindings for temperature-sense-rtd
> 10: 33825ad452d6 = 10: bb39296590f3 dt-bindings: iio: afe: add bindings for temperature transducers
>
> base-commit: cd717ac6f69db4953ca701c6220c7cb58e17f35a
Powered by blists - more mailing lists