| Message ID | 20210703180131.4036589-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (ntc_thermistor): Use library interpolation | expand |
On Sat, 3 Jul 2021 20:01:31 +0200 Linus Walleij <linus.walleij@linaro.org> wrote: > The kernel has a helper function for linear interpolation so > use it. It incidentally makes the code easier to read as well. > > Cc: Peter Rosin <peda@axentia.se> > Cc: Chris Lesiak <chris.lesiak@licor.com> > Cc: linux-iio@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Potential precision problem because the multiplication by 1000 is now done post division whereas before it was before it? I'm only eyeballing the code so no idea if it makes a practical difference. > --- > drivers/hwmon/ntc_thermistor.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c > index 8587189c7f15..61bd0e074ec9 100644 > --- a/drivers/hwmon/ntc_thermistor.c > +++ b/drivers/hwmon/ntc_thermistor.c > @@ -14,6 +14,7 @@ > #include <linux/err.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/fixp-arith.h> > > #include <linux/platform_data/ntc_thermistor.h> > > @@ -557,10 +558,12 @@ static int get_temp_mc(struct ntc_data *data, unsigned int ohm) > /* Unable to use linear approximation */ > temp = data->comp[low].temp_c * 1000; > } else { > - temp = data->comp[low].temp_c * 1000 + > - ((data->comp[high].temp_c - data->comp[low].temp_c) * > - 1000 * ((int)ohm - (int)data->comp[low].ohm)) / > - ((int)data->comp[high].ohm - (int)data->comp[low].ohm); > + temp = fixp_linear_interpolate(data->comp[low].ohm, > + data->comp[low].temp_c, > + data->comp[high].ohm, > + data->comp[high].temp_c, > + ohm); > + temp *= 1000; > } > return temp; > }
On 7/4/21 9:14 AM, Jonathan Cameron wrote: > On Sat, 3 Jul 2021 20:01:31 +0200 > Linus Walleij <linus.walleij@linaro.org> wrote: > >> The kernel has a helper function for linear interpolation so >> use it. It incidentally makes the code easier to read as well. >> >> Cc: Peter Rosin <peda@axentia.se> >> Cc: Chris Lesiak <chris.lesiak@licor.com> >> Cc: linux-iio@vger.kernel.org >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Potential precision problem because the multiplication by 1000 is now done > post division whereas before it was before it? > > I'm only eyeballing the code so no idea if it makes a practical difference. > I think so, since the code now only reports temperatures in full degrees C which wasn't the case earlier. That should be easy to fix though by passing data->comp[low].temp_c * 1000 and data->comp[high].temp_c * 1000 as parameters to fixp_linear_interpolate(). Also, the if/else in get_temp_mc() seems now unnecessary since fixp_linear_interpolate() effectively does the same checks. Guenter >> --- >> drivers/hwmon/ntc_thermistor.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c >> index 8587189c7f15..61bd0e074ec9 100644 >> --- a/drivers/hwmon/ntc_thermistor.c >> +++ b/drivers/hwmon/ntc_thermistor.c >> @@ -14,6 +14,7 @@ >> #include <linux/err.h> >> #include <linux/of.h> >> #include <linux/of_device.h> >> +#include <linux/fixp-arith.h> >> >> #include <linux/platform_data/ntc_thermistor.h> >> >> @@ -557,10 +558,12 @@ static int get_temp_mc(struct ntc_data *data, unsigned int ohm) >> /* Unable to use linear approximation */ >> temp = data->comp[low].temp_c * 1000; >> } else { >> - temp = data->comp[low].temp_c * 1000 + >> - ((data->comp[high].temp_c - data->comp[low].temp_c) * >> - 1000 * ((int)ohm - (int)data->comp[low].ohm)) / >> - ((int)data->comp[high].ohm - (int)data->comp[low].ohm); >> + temp = fixp_linear_interpolate(data->comp[low].ohm, >> + data->comp[low].temp_c, >> + data->comp[high].ohm, >> + data->comp[high].temp_c, >> + ohm); >> + temp *= 1000; >> } >> return temp; >> } >
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c index 8587189c7f15..61bd0e074ec9 100644 --- a/drivers/hwmon/ntc_thermistor.c +++ b/drivers/hwmon/ntc_thermistor.c @@ -14,6 +14,7 @@ #include <linux/err.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/fixp-arith.h> #include <linux/platform_data/ntc_thermistor.h> @@ -557,10 +558,12 @@ static int get_temp_mc(struct ntc_data *data, unsigned int ohm) /* Unable to use linear approximation */ temp = data->comp[low].temp_c * 1000; } else { - temp = data->comp[low].temp_c * 1000 + - ((data->comp[high].temp_c - data->comp[low].temp_c) * - 1000 * ((int)ohm - (int)data->comp[low].ohm)) / - ((int)data->comp[high].ohm - (int)data->comp[low].ohm); + temp = fixp_linear_interpolate(data->comp[low].ohm, + data->comp[low].temp_c, + data->comp[high].ohm, + data->comp[high].temp_c, + ohm); + temp *= 1000; } return temp; }
The kernel has a helper function for linear interpolation so use it. It incidentally makes the code easier to read as well. Cc: Peter Rosin <peda@axentia.se> Cc: Chris Lesiak <chris.lesiak@licor.com> Cc: linux-iio@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/hwmon/ntc_thermistor.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)