mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] iio: pressure: dps310: fix overflow in pressure compensation
@ 2026-10-01 12:16 Rupesh Majhi
  2026-10-02  9:25 ` Andy Shevchenko
  2026-10-11 13:14 ` Jonathan Cameron
  0 siblings, 2 replies; 3+ messages in thread
From: Rupesh Majhi @ 2026-10-01 12:16 UTC (permalink / raw)
  To: Andy Shevchenko, David Lechner, Eddie James, Jonathan Cameron,
	Nuno Sá
  Cc: linux-iio, linux-kernel, Rupesh Majhi, Sashiko, stable

p^3 * c30 and t * p^2 * c21 overflow s64 at most oversampling ratios,
and pressure comes out about 150 Pa low on a DPS310 here.

Use Horner form, dividing by scale factor at each step. Overflow then
needs a raw value far outside sensor range and returns -ERANGE.

Checked against datasheet formula at all eight ratios.

Fixes: d711a3c7dc82 ("iio: dps310: Add pressure sensing capability")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260921183132.233136-1-zoone.rupert%40gmail.com
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
---
v3: min_t() line left as in mainline (Andy), comment cut to one line
v2: (s64)BIT(20) instead of BIT_ULL(), min() instead of min_t() (Andy)
    https://lore.kernel.org/linux-iio/20260930085819.886339-1-zoone.rupert@gmail.com/
v1: https://lore.kernel.org/linux-iio/20260928101803.547005-1-zoone.rupert@gmail.com/

Based on iio/testing, since the buffer series reworked this function.
Can send a version for fixes-togreg if you prefer.

 drivers/iio/pressure/dps310.c | 89 ++++++++++++++---------------------
 1 file changed, 35 insertions(+), 54 deletions(-)

diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index 85df58ac1809..2682615b69bb 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -19,6 +19,7 @@
 #include <linux/limits.h>
 #include <linux/math64.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/regmap.h>
 #include <linux/unaligned.h>
 
@@ -660,72 +661,52 @@ static int dps310_write_raw(struct iio_dev *iio,
 	}
 }
 
+/* Fixed point one for the compensation fractions */
+#define DPS310_CALC_ONE		((s64)BIT(20))
+
+/* *acc = (*acc + coef) * raw / k, with coef scaled to match *acc */
+static bool dps310_horner_step(s64 *acc, s32 coef, s64 raw, s64 k)
+{
+	s64 prod;
+
+	if (check_mul_overflow(*acc + coef * DPS310_CALC_ONE, raw, &prod))
+		return false;
+
+	*acc = div64_s64(prod, k);
+
+	return true;
+}
+
 static int dps310_calculate_pressure(struct dps310_data *data, int *val)
 	__must_hold(&data->lock)
 {
-	int i;
+	s64 p = data->pressure_raw;
+	s64 t = data->temp_raw;
+	s64 prs = 0;
+	s64 tmp = 0;
+	s64 pressure;
+	int kp, kt;
 	int rc;
-	int kpi;
-	int kti;
-	s64 rem = 0ULL;
-	s64 pressure = 0ULL;
-	s64 p;
-	s64 t;
-	s64 denoms[7];
-	s64 nums[7];
-	s64 rems[7];
-	s64 kp;
-	s64 kt;
 
-	rc = dps310_get_pres_k(data, &kpi);
+	rc = dps310_get_pres_k(data, &kp);
 	if (rc)
 		return rc;
 
-	rc = dps310_get_temp_k(data, &kti);
+	rc = dps310_get_temp_k(data, &kt);
 	if (rc)
 		return rc;
 
-	kp = (s64)kpi;
-	kt = (s64)kti;
-
-	p = (s64)data->pressure_raw;
-	t = (s64)data->temp_raw;
-
-	/* Section 4.9.1 of the DPS310 spec; algebra'd to avoid underflow */
-	nums[0] = (s64)data->c00;
-	denoms[0] = 1LL;
-	nums[1] = p * (s64)data->c10;
-	denoms[1] = kp;
-	nums[2] = p * p * (s64)data->c20;
-	denoms[2] = kp * kp;
-	nums[3] = p * p * p * (s64)data->c30;
-	denoms[3] = kp * kp * kp;
-	nums[4] = t * (s64)data->c01;
-	denoms[4] = kt;
-	nums[5] = t * p * (s64)data->c11;
-	denoms[5] = kp * kt;
-	nums[6] = t * p * p * (s64)data->c21;
-	denoms[6] = kp * kp * kt;
-
-	/* Kernel lacks a div64_s64_rem function; denoms are all positive */
-	for (i = 0; i < 7; ++i) {
-		u64 irem;
-
-		if (nums[i] < 0LL) {
-			pressure -= div64_u64_rem(-nums[i], denoms[i], &irem);
-			rems[i] = -irem;
-		} else {
-			pressure += div64_u64_rem(nums[i], denoms[i], &irem);
-			rems[i] = (s64)irem;
-		}
-	}
-
-	/* Increase precision and calculate the remainder sum */
-	for (i = 0; i < 7; ++i)
-		rem += div64_s64((s64)rems[i] * 1000000000LL, denoms[i]);
+	/* Section 4.9.1 of the DPS310 spec, Horner form to avoid overflow */
+	if (!dps310_horner_step(&prs, data->c30, p, kp) ||
+	    !dps310_horner_step(&prs, data->c20, p, kp) ||
+	    !dps310_horner_step(&prs, data->c10, p, kp) ||
+	    !dps310_horner_step(&tmp, data->c21, p, kp) ||
+	    !dps310_horner_step(&tmp, data->c11, p, kp) ||
+	    !dps310_horner_step(&tmp, data->c01, t, kt))
+		return -ERANGE;
 
-	pressure += div_s64(rem, 1000000000LL);
-	if (pressure < 0LL)
+	pressure = data->c00 + div64_s64(prs + tmp, DPS310_CALC_ONE);
+	if (pressure < 0)
 		return -ERANGE;
 
 	*val = (int)min_t(s64, pressure, INT_MAX);

base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] iio: pressure: dps310: fix overflow in pressure compensation
  2026-10-01 12:16 [PATCH v3] iio: pressure: dps310: fix overflow in pressure compensation Rupesh Majhi
@ 2026-10-02  9:25 ` Andy Shevchenko
  2026-10-11 13:14 ` Jonathan Cameron
  1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-10-02  9:25 UTC (permalink / raw)
  To: Rupesh Majhi
  Cc: Andy Shevchenko, David Lechner, Eddie James, Jonathan Cameron,
	Nuno Sá,
	linux-iio, linux-kernel, Sashiko, stable

On Thu, Oct 01, 2026 at 03:16:16PM +0300, Rupesh Majhi wrote:
> p^3 * c30 and t * p^2 * c21 overflow s64 at most oversampling ratios,
> and pressure comes out about 150 Pa low on a DPS310 here.
> 
> Use Horner form, dividing by scale factor at each step. Overflow then
> needs a raw value far outside sensor range and returns -ERANGE.
> 
> Checked against datasheet formula at all eight ratios.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

...

> +	/* Section 4.9.1 of the DPS310 spec, Horner form to avoid overflow */
> +	if (!dps310_horner_step(&prs, data->c30, p, kp) ||
> +	    !dps310_horner_step(&prs, data->c20, p, kp) ||
> +	    !dps310_horner_step(&prs, data->c10, p, kp) ||
> +	    !dps310_horner_step(&tmp, data->c21, p, kp) ||
> +	    !dps310_horner_step(&tmp, data->c11, p, kp) ||
> +	    !dps310_horner_step(&tmp, data->c01, t, kt))
> +		return -ERANGE;

Sorry for nit-picking, but since the above checks for overflow, perhaps makes
sense to use actually -EOVERFLOW here.

> -	pressure += div_s64(rem, 1000000000LL);
> -	if (pressure < 0LL)
> +	pressure = data->c00 + div64_s64(prs + tmp, DPS310_CALC_ONE);
> +	if (pressure < 0)
>  		return -ERANGE;

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] iio: pressure: dps310: fix overflow in pressure compensation
  2026-10-01 12:16 [PATCH v3] iio: pressure: dps310: fix overflow in pressure compensation Rupesh Majhi
  2026-10-02  9:25 ` Andy Shevchenko
@ 2026-10-11 13:14 ` Jonathan Cameron
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-10-11 13:14 UTC (permalink / raw)
  To: Rupesh Majhi
  Cc: Andy Shevchenko, David Lechner, Eddie James, Nuno Sá,
	linux-iio, linux-kernel, Sashiko, stable

On Thu,  1 Oct 2026 15:16:16 +0300
Rupesh Majhi <zoone.rupert@gmail.com> wrote:

> p^3 * c30 and t * p^2 * c21 overflow s64 at most oversampling ratios,
> and pressure comes out about 150 Pa low on a DPS310 here.
> 
> Use Horner form, dividing by scale factor at each step. Overflow then
> needs a raw value far outside sensor range and returns -ERANGE.
> 
> Checked against datasheet formula at all eight ratios.
> 
> Fixes: d711a3c7dc82 ("iio: dps310: Add pressure sensing capability")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260921183132.233136-1-zoone.rupert%40gmail.com
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
I haven't checked the maths closely but assume you have that well tested
etc.  

> ---
> v3: min_t() line left as in mainline (Andy), comment cut to one line
> v2: (s64)BIT(20) instead of BIT_ULL(), min() instead of min_t() (Andy)
>     https://lore.kernel.org/linux-iio/20260930085819.886339-1-zoone.rupert@gmail.com/
> v1: https://lore.kernel.org/linux-iio/20260928101803.547005-1-zoone.rupert@gmail.com/
> 
> Based on iio/testing, since the buffer series reworked this function.
> Can send a version for fixes-togreg if you prefer.

This is fine.  Ideal would be to then chase the merge upstream of this patch
with a backported version - but always up to the author on whether they do that
work or not.

Applied, to the testing branch of iio.git - though this is more about
the other patches you have on list potentially making things more complex
if I took it through the fixes-togreg branch.

Thanks,

Jonathan

> 
>  drivers/iio/pressure/dps310.c | 89 ++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index 85df58ac1809..2682615b69bb 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c
> @@ -19,6 +19,7 @@
>  #include <linux/limits.h>
>  #include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/overflow.h>
>  #include <linux/regmap.h>
>  #include <linux/unaligned.h>
>  
> @@ -660,72 +661,52 @@ static int dps310_write_raw(struct iio_dev *iio,
>  	}
>  }
>  
> +/* Fixed point one for the compensation fractions */
> +#define DPS310_CALC_ONE		((s64)BIT(20))
> +
> +/* *acc = (*acc + coef) * raw / k, with coef scaled to match *acc */
> +static bool dps310_horner_step(s64 *acc, s32 coef, s64 raw, s64 k)
> +{
> +	s64 prod;
> +
> +	if (check_mul_overflow(*acc + coef * DPS310_CALC_ONE, raw, &prod))
> +		return false;
> +
> +	*acc = div64_s64(prod, k);
> +
> +	return true;
> +}
> +
>  static int dps310_calculate_pressure(struct dps310_data *data, int *val)
>  	__must_hold(&data->lock)
>  {
> -	int i;
> +	s64 p = data->pressure_raw;
> +	s64 t = data->temp_raw;
> +	s64 prs = 0;
> +	s64 tmp = 0;
> +	s64 pressure;
> +	int kp, kt;
>  	int rc;
> -	int kpi;
> -	int kti;
> -	s64 rem = 0ULL;
> -	s64 pressure = 0ULL;
> -	s64 p;
> -	s64 t;
> -	s64 denoms[7];
> -	s64 nums[7];
> -	s64 rems[7];
> -	s64 kp;
> -	s64 kt;
>  
> -	rc = dps310_get_pres_k(data, &kpi);
> +	rc = dps310_get_pres_k(data, &kp);
>  	if (rc)
>  		return rc;
>  
> -	rc = dps310_get_temp_k(data, &kti);
> +	rc = dps310_get_temp_k(data, &kt);
>  	if (rc)
>  		return rc;
>  
> -	kp = (s64)kpi;
> -	kt = (s64)kti;
> -
> -	p = (s64)data->pressure_raw;
> -	t = (s64)data->temp_raw;
> -
> -	/* Section 4.9.1 of the DPS310 spec; algebra'd to avoid underflow */
> -	nums[0] = (s64)data->c00;
> -	denoms[0] = 1LL;
> -	nums[1] = p * (s64)data->c10;
> -	denoms[1] = kp;
> -	nums[2] = p * p * (s64)data->c20;
> -	denoms[2] = kp * kp;
> -	nums[3] = p * p * p * (s64)data->c30;
> -	denoms[3] = kp * kp * kp;
> -	nums[4] = t * (s64)data->c01;
> -	denoms[4] = kt;
> -	nums[5] = t * p * (s64)data->c11;
> -	denoms[5] = kp * kt;
> -	nums[6] = t * p * p * (s64)data->c21;
> -	denoms[6] = kp * kp * kt;
> -
> -	/* Kernel lacks a div64_s64_rem function; denoms are all positive */
> -	for (i = 0; i < 7; ++i) {
> -		u64 irem;
> -
> -		if (nums[i] < 0LL) {
> -			pressure -= div64_u64_rem(-nums[i], denoms[i], &irem);
> -			rems[i] = -irem;
> -		} else {
> -			pressure += div64_u64_rem(nums[i], denoms[i], &irem);
> -			rems[i] = (s64)irem;
> -		}
> -	}
> -
> -	/* Increase precision and calculate the remainder sum */
> -	for (i = 0; i < 7; ++i)
> -		rem += div64_s64((s64)rems[i] * 1000000000LL, denoms[i]);
> +	/* Section 4.9.1 of the DPS310 spec, Horner form to avoid overflow */
> +	if (!dps310_horner_step(&prs, data->c30, p, kp) ||
> +	    !dps310_horner_step(&prs, data->c20, p, kp) ||
> +	    !dps310_horner_step(&prs, data->c10, p, kp) ||
> +	    !dps310_horner_step(&tmp, data->c21, p, kp) ||
> +	    !dps310_horner_step(&tmp, data->c11, p, kp) ||
> +	    !dps310_horner_step(&tmp, data->c01, t, kt))
> +		return -ERANGE;
>  
> -	pressure += div_s64(rem, 1000000000LL);
> -	if (pressure < 0LL)
> +	pressure = data->c00 + div64_s64(prs + tmp, DPS310_CALC_ONE);
> +	if (pressure < 0)
>  		return -ERANGE;
>  
>  	*val = (int)min_t(s64, pressure, INT_MAX);
> 
> base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-11 13:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01 12:16 [PATCH v3] iio: pressure: dps310: fix overflow in pressure compensation Rupesh Majhi
2026-10-02  9:25 ` Andy Shevchenko
2026-10-11 13:14 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®