linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] iio: humditiy: hdc3020: fix units
@ 2025-09-01 17:51 Dimitri Fedrau via B4 Relay
  2025-09-01 17:51 ` [PATCH v2 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement Dimitri Fedrau via B4 Relay
  2025-09-01 17:51 ` [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis Dimitri Fedrau via B4 Relay
  0 siblings, 2 replies; 9+ messages in thread
From: Dimitri Fedrau via B4 Relay @ 2025-09-01 17:51 UTC (permalink / raw)
  To: Javier Carrasco, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Dimitri Fedrau
  Cc: Jonathan Cameron, linux-iio, linux-kernel, Dimitri Fedrau,
	Chris Lesiak

Fix units to milli degree celsius and milli percent for temperature
respectively relative humidity measurements and thresholds.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
Changes in v2:
- Added explanation what is wrong at the moment into commit msg
- Added define HDC3020_THRESH_FRACTION and comment at the beginning of the
  code.
- Use MILLI for instead of hardcoded 1000.
- Link to v1: https://lore.kernel.org/r/20250821-hdc3020-units-fix-v1-0-6ab0bc353c5e@liebherr.com

---
Dimitri Fedrau (2):
      iio: humditiy: hdc3020: fix units for temperature and humidity measurement
      iio: humditiy: hdc3020: fix units for thresholds and hysteresis

 drivers/iio/humidity/hdc3020.c | 73 +++++++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 30 deletions(-)
---
base-commit: 43c0f6456f801181a80b73d95def0e0fd134e1cc
change-id: 20250820-hdc3020-units-fix-91edbb8ccd07

Best regards,
-- 
Dimitri Fedrau <dimitri.fedrau@liebherr.com>



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

* [PATCH v2 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement
  2025-09-01 17:51 [PATCH v2 0/2] iio: humditiy: hdc3020: fix units Dimitri Fedrau via B4 Relay
@ 2025-09-01 17:51 ` Dimitri Fedrau via B4 Relay
  2025-09-02 12:05   ` Javier Carrasco
  2025-09-01 17:51 ` [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis Dimitri Fedrau via B4 Relay
  1 sibling, 1 reply; 9+ messages in thread
From: Dimitri Fedrau via B4 Relay @ 2025-09-01 17:51 UTC (permalink / raw)
  To: Javier Carrasco, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Dimitri Fedrau
  Cc: Jonathan Cameron, linux-iio, linux-kernel, Dimitri Fedrau,
	Chris Lesiak

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

According to the ABI the units after application of scale and offset are
milli degrees for temperature measurements and milli percent for relative
humidity measurements. Currently the resulting units are degree celsius for
temperature measurements and percent for relative humidity measurements.
Change scale factor to fix this issue.

Fixes: c9180b8e39be ("iio: humidity: Add driver for ti HDC302x humidity sensors")
Reported-by: Chris Lesiak <chris.lesiak@licorbio.com>
Suggested-by: Chris Lesiak <chris.lesiak@licorbio.com>
Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/iio/humidity/hdc3020.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
index ffb25596d3a8bad01d1f84a9a972561266f65d76..8aa567d9aded9cab461f1f905b6b5ada721ba2f0 100644
--- a/drivers/iio/humidity/hdc3020.c
+++ b/drivers/iio/humidity/hdc3020.c
@@ -301,9 +301,9 @@ static int hdc3020_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		*val2 = 65536;
 		if (chan->type == IIO_TEMP)
-			*val = 175;
+			*val = 175 * MILLI;
 		else
-			*val = 100;
+			*val = 100 * MILLI;
 		return IIO_VAL_FRACTIONAL;
 
 	case IIO_CHAN_INFO_OFFSET:

-- 
2.39.5



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

* [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
  2025-09-01 17:51 [PATCH v2 0/2] iio: humditiy: hdc3020: fix units Dimitri Fedrau via B4 Relay
  2025-09-01 17:51 ` [PATCH v2 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement Dimitri Fedrau via B4 Relay
@ 2025-09-01 17:51 ` Dimitri Fedrau via B4 Relay
  2025-09-02 12:04   ` Javier Carrasco
  2025-09-02 12:47   ` Andy Shevchenko
  1 sibling, 2 replies; 9+ messages in thread
From: Dimitri Fedrau via B4 Relay @ 2025-09-01 17:51 UTC (permalink / raw)
  To: Javier Carrasco, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Dimitri Fedrau
  Cc: Jonathan Cameron, linux-iio, linux-kernel, Dimitri Fedrau,
	Chris Lesiak

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

According to the ABI the units after application of scale and offset are
milli degree celsius for temperature thresholds and milli percent for
relative humidity thresholds. Currently the resulting units are degree
celsius for temperature thresholds and hysteresis and percent for relative
humidity thresholds and hysteresis. Change scale factor to fix this issue.

Fixes: 3ad0e7e5f0cb ("iio: humidity: hdc3020: add threshold events support")
Reported-by: Chris Lesiak <chris.lesiak@licorbio.com>
Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/iio/humidity/hdc3020.c | 69 +++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
index 8aa567d9aded9cab461f1f905b6b5ada721ba2f0..deb19e0291f7f820c5f8760fbf0682379ab5f34b 100644
--- a/drivers/iio/humidity/hdc3020.c
+++ b/drivers/iio/humidity/hdc3020.c
@@ -72,6 +72,13 @@
 #define HDC3020_MAX_TEMP_HYST_MICRO	164748607
 #define HDC3020_MAX_HUM_MICRO		99220264
 
+/*
+ * In order to avoid overflows when returning thresholds and hysteresis values,
+ * the fraction of these is set to 13107, also the datasheet refers to 65535
+ * this is not enough to prevent overflows. Dividing this value by 5 fixes this.
+ */
+#define HDC3020_THRESH_FRACTION		(65535 / 5)
+
 struct hdc3020_data {
 	struct i2c_client *client;
 	struct gpio_desc *reset_gpio;
@@ -376,15 +383,16 @@ static int hdc3020_thresh_get_temp(u16 thresh)
 	int temp;
 
 	/*
-	 * Get the temperature threshold from 9 LSBs, shift them to get
-	 * the truncated temperature threshold representation and
-	 * calculate the threshold according to the formula in the
-	 * datasheet. Result is degree celsius scaled by 65535.
+	 * Get the temperature threshold from 9 LSBs, shift them to get the
+	 * truncated temperature threshold representation and calculate the
+	 * threshold according to the formula in the datasheet and additionally
+	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
+	 * calculating threshold and hysteresis values.
 	 */
 	temp = FIELD_GET(HDC3020_THRESH_TEMP_MASK, thresh) <<
 	       HDC3020_THRESH_TEMP_TRUNC_SHIFT;
 
-	return -2949075 + (175 * temp);
+	return -589815 + (35 * temp);
 }
 
 static int hdc3020_thresh_get_hum(u16 thresh)
@@ -394,13 +402,14 @@ static int hdc3020_thresh_get_hum(u16 thresh)
 	/*
 	 * Get the humidity threshold from 7 MSBs, shift them to get the
 	 * truncated humidity threshold representation and calculate the
-	 * threshold according to the formula in the datasheet. Result is
-	 * percent scaled by 65535.
+	 * threshold according to the formula in the datasheet and additionally
+	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
+	 * calculating threshold and hysteresis values.
 	 */
 	hum = FIELD_GET(HDC3020_THRESH_HUM_MASK, thresh) <<
 	      HDC3020_THRESH_HUM_TRUNC_SHIFT;
 
-	return hum * 100;
+	return hum * 20;
 }
 
 static u16 hdc3020_thresh_set_temp(int s_temp, u16 curr_thresh)
@@ -455,8 +464,8 @@ int hdc3020_thresh_clr(s64 s_thresh, s64 s_hyst, enum iio_event_direction dir)
 	else
 		s_clr = s_thresh + s_hyst;
 
-	/* Divide by 65535 to get units of micro */
-	return div_s64(s_clr, 65535);
+	/* Divide by HDC3020_THRESH_FRACTION to get units of micro */
+	return div_s64(s_clr, HDC3020_THRESH_FRACTION);
 }
 
 static int _hdc3020_write_thresh(struct hdc3020_data *data, u16 reg, u16 val)
@@ -507,7 +516,7 @@ static int hdc3020_write_thresh(struct iio_dev *indio_dev,
 
 	clr = ret;
 	/* Scale value to include decimal part into calculations */
-	s_val = (val < 0) ? (val * 1000000 - val2) : (val * 1000000 + val2);
+	s_val = (val < 0) ? (val * 1000 - val2) : (val * 1000 + val2);
 	switch (chan->type) {
 	case IIO_TEMP:
 		switch (info) {
@@ -523,7 +532,8 @@ static int hdc3020_write_thresh(struct iio_dev *indio_dev,
 			/* Calculate old hysteresis */
 			s_thresh = (s64)hdc3020_thresh_get_temp(thresh) * 1000000;
 			s_clr = (s64)hdc3020_thresh_get_temp(clr) * 1000000;
-			s_hyst = div_s64(abs(s_thresh - s_clr), 65535);
+			s_hyst = div_s64(abs(s_thresh - s_clr),
+					 HDC3020_THRESH_FRACTION);
 			/* Set new threshold */
 			thresh = reg_val;
 			/* Set old hysteresis */
@@ -532,16 +542,17 @@ static int hdc3020_write_thresh(struct iio_dev *indio_dev,
 		case IIO_EV_INFO_HYSTERESIS:
 			/*
 			 * Function hdc3020_thresh_get_temp returns temperature
-			 * in degree celsius scaled by 65535. Scale by 1000000
-			 * to be able to subtract scaled hysteresis value.
+			 * in degree celsius scaled by HDC3020_THRESH_FRACTION.
+			 * Scale by 1000000 to be able to subtract scaled
+			 * hysteresis value.
 			 */
 			s_thresh = (s64)hdc3020_thresh_get_temp(thresh) * 1000000;
 			/*
 			 * Units of s_val are in micro degree celsius, scale by
-			 * 65535 to get same units as s_thresh.
+			 * HDC3020_THRESH_FRACTION to get same units as s_thresh.
 			 */
 			s_val = min(abs(s_val), HDC3020_MAX_TEMP_HYST_MICRO);
-			s_hyst = (s64)s_val * 65535;
+			s_hyst = (s64)s_val * HDC3020_THRESH_FRACTION;
 			s_clr = hdc3020_thresh_clr(s_thresh, s_hyst, dir);
 			s_clr = max(s_clr, HDC3020_MIN_TEMP_MICRO);
 			s_clr = min(s_clr, HDC3020_MAX_TEMP_MICRO);
@@ -565,7 +576,8 @@ static int hdc3020_write_thresh(struct iio_dev *indio_dev,
 			/* Calculate old hysteresis */
 			s_thresh = (s64)hdc3020_thresh_get_hum(thresh) * 1000000;
 			s_clr = (s64)hdc3020_thresh_get_hum(clr) * 1000000;
-			s_hyst = div_s64(abs(s_thresh - s_clr), 65535);
+			s_hyst = div_s64(abs(s_thresh - s_clr),
+					 HDC3020_THRESH_FRACTION);
 			/* Set new threshold */
 			thresh = reg_val;
 			/* Try to set old hysteresis */
@@ -574,15 +586,16 @@ static int hdc3020_write_thresh(struct iio_dev *indio_dev,
 		case IIO_EV_INFO_HYSTERESIS:
 			/*
 			 * Function hdc3020_thresh_get_hum returns relative
-			 * humidity in percent scaled by 65535. Scale by 1000000
-			 * to be able to subtract scaled hysteresis value.
+			 * humidity in percent scaled by HDC3020_THRESH_FRACTION.
+			 * Scale by 1000000 to be able to subtract scaled
+			 * hysteresis value.
 			 */
 			s_thresh = (s64)hdc3020_thresh_get_hum(thresh) * 1000000;
 			/*
-			 * Units of s_val are in micro percent, scale by 65535
-			 * to get same units as s_thresh.
+			 * Units of s_val are in micro percent, scale by
+			 * HDC3020_THRESH_FRACTION to get same units as s_thresh.
 			 */
-			s_hyst = (s64)s_val * 65535;
+			s_hyst = (s64)s_val * HDC3020_THRESH_FRACTION;
 			s_clr = hdc3020_thresh_clr(s_thresh, s_hyst, dir);
 			s_clr = max(s_clr, 0);
 			s_clr = min(s_clr, HDC3020_MAX_HUM_MICRO);
@@ -630,7 +643,7 @@ static int hdc3020_read_thresh(struct iio_dev *indio_dev,
 		thresh = hdc3020_thresh_get_temp(ret);
 		switch (info) {
 		case IIO_EV_INFO_VALUE:
-			*val = thresh;
+			*val = thresh * MILLI;
 			break;
 		case IIO_EV_INFO_HYSTERESIS:
 			ret = hdc3020_read_be16(data, reg_clr);
@@ -638,18 +651,18 @@ static int hdc3020_read_thresh(struct iio_dev *indio_dev,
 				return ret;
 
 			clr = hdc3020_thresh_get_temp(ret);
-			*val = abs(thresh - clr);
+			*val = abs(thresh - clr) * MILLI;
 			break;
 		default:
 			return -EOPNOTSUPP;
 		}
-		*val2 = 65535;
+		*val2 = HDC3020_THRESH_FRACTION;
 		return IIO_VAL_FRACTIONAL;
 	case IIO_HUMIDITYRELATIVE:
 		thresh = hdc3020_thresh_get_hum(ret);
 		switch (info) {
 		case IIO_EV_INFO_VALUE:
-			*val = thresh;
+			*val = thresh * MILLI;
 			break;
 		case IIO_EV_INFO_HYSTERESIS:
 			ret = hdc3020_read_be16(data, reg_clr);
@@ -657,12 +670,12 @@ static int hdc3020_read_thresh(struct iio_dev *indio_dev,
 				return ret;
 
 			clr = hdc3020_thresh_get_hum(ret);
-			*val = abs(thresh - clr);
+			*val = abs(thresh - clr) * MILLI;
 			break;
 		default:
 			return -EOPNOTSUPP;
 		}
-		*val2 = 65535;
+		*val2 = HDC3020_THRESH_FRACTION;
 		return IIO_VAL_FRACTIONAL;
 	default:
 		return -EOPNOTSUPP;

-- 
2.39.5



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

* Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
  2025-09-01 17:51 ` [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis Dimitri Fedrau via B4 Relay
@ 2025-09-02 12:04   ` Javier Carrasco
  2025-09-02 12:51     ` Dimitri Fedrau
  2025-09-02 12:47   ` Andy Shevchenko
  1 sibling, 1 reply; 9+ messages in thread
From: Javier Carrasco @ 2025-09-02 12:04 UTC (permalink / raw)
  To: dimitri.fedrau, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Dimitri Fedrau
  Cc: Jonathan Cameron, linux-iio, linux-kernel, Chris Lesiak

Hi Dimitri, thank you for your patch. A few comments inline.

On Tue Sep 2, 2025 at 1:51 AM CST, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
>
> According to the ABI the units after application of scale and offset are
> milli degree celsius for temperature thresholds and milli percent for
> relative humidity thresholds. Currently the resulting units are degree
> celsius for temperature thresholds and hysteresis and percent for relative
> humidity thresholds and hysteresis. Change scale factor to fix this issue.
>
> Fixes: 3ad0e7e5f0cb ("iio: humidity: hdc3020: add threshold events support")
> Reported-by: Chris Lesiak <chris.lesiak@licorbio.com>
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  drivers/iio/humidity/hdc3020.c | 69 +++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
> index 8aa567d9aded9cab461f1f905b6b5ada721ba2f0..deb19e0291f7f820c5f8760fbf0682379ab5f34b 100644
> --- a/drivers/iio/humidity/hdc3020.c
> +++ b/drivers/iio/humidity/hdc3020.c
> @@ -72,6 +72,13 @@
>  #define HDC3020_MAX_TEMP_HYST_MICRO	164748607
>  #define HDC3020_MAX_HUM_MICRO		99220264
>  
> +/*
> + * In order to avoid overflows when returning thresholds and hysteresis values,
> + * the fraction of these is set to 13107, also the datasheet refers to 65535
> + * this is not enough to prevent overflows. Dividing this value by 5 fixes this.
> + */
> +#define HDC3020_THRESH_FRACTION		(65535 / 5)
> +

I think the comment is a bit too verbose. If you explain why the value
has to be divided by 5, you probably don't need to tell the result of
the division. Something as simple as "divide 65535 from the datasheet by
5 to avoid overflows" could do.

>  struct hdc3020_data {
>  	struct i2c_client *client;
>  	struct gpio_desc *reset_gpio;
> @@ -376,15 +383,16 @@ static int hdc3020_thresh_get_temp(u16 thresh)
>  	int temp;
>  
>  	/*
> -	 * Get the temperature threshold from 9 LSBs, shift them to get
> -	 * the truncated temperature threshold representation and
> -	 * calculate the threshold according to the formula in the
> -	 * datasheet. Result is degree celsius scaled by 65535.
> +	 * Get the temperature threshold from 9 LSBs, shift them to get the
> +	 * truncated temperature threshold representation and calculate the
> +	 * threshold according to the formula in the datasheet and additionally
> +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> +	 * calculating threshold and hysteresis values.

If I got it right, you are actually dividing by 5 and not by
HDC3020_THRESH_FRACTION.

>  	 */
>  	temp = FIELD_GET(HDC3020_THRESH_TEMP_MASK, thresh) <<
>  	       HDC3020_THRESH_TEMP_TRUNC_SHIFT;
>  
> -	return -2949075 + (175 * temp);
> +	return -589815 + (35 * temp);
>  }
>  
>  static int hdc3020_thresh_get_hum(u16 thresh)
> @@ -394,13 +402,14 @@ static int hdc3020_thresh_get_hum(u16 thresh)
>  	/*
>  	 * Get the humidity threshold from 7 MSBs, shift them to get the
>  	 * truncated humidity threshold representation and calculate the
> -	 * threshold according to the formula in the datasheet. Result is
> -	 * percent scaled by 65535.
> +	 * threshold according to the formula in the datasheet and additionally
> +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> +	 * calculating threshold and hysteresis values.

Same here?

>  	 */
>  	hum = FIELD_GET(HDC3020_THRESH_HUM_MASK, thresh) <<
>  	      HDC3020_THRESH_HUM_TRUNC_SHIFT;
>  
> -	return hum * 100;
> +	return hum * 20;
>  }

Best regards,
Javier Carrasco

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

* Re: [PATCH v2 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement
  2025-09-01 17:51 ` [PATCH v2 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement Dimitri Fedrau via B4 Relay
@ 2025-09-02 12:05   ` Javier Carrasco
  0 siblings, 0 replies; 9+ messages in thread
From: Javier Carrasco @ 2025-09-02 12:05 UTC (permalink / raw)
  To: dimitri.fedrau, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Dimitri Fedrau
  Cc: Jonathan Cameron, linux-iio, linux-kernel, Chris Lesiak

On Tue Sep 2, 2025 at 1:51 AM CST, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
>
> According to the ABI the units after application of scale and offset are
> milli degrees for temperature measurements and milli percent for relative
> humidity measurements. Currently the resulting units are degree celsius for
> temperature measurements and percent for relative humidity measurements.
> Change scale factor to fix this issue.
>
> Fixes: c9180b8e39be ("iio: humidity: Add driver for ti HDC302x humidity sensors")
> Reported-by: Chris Lesiak <chris.lesiak@licorbio.com>
> Suggested-by: Chris Lesiak <chris.lesiak@licorbio.com>
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  drivers/iio/humidity/hdc3020.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
> index ffb25596d3a8bad01d1f84a9a972561266f65d76..8aa567d9aded9cab461f1f905b6b5ada721ba2f0 100644
> --- a/drivers/iio/humidity/hdc3020.c
> +++ b/drivers/iio/humidity/hdc3020.c
> @@ -301,9 +301,9 @@ static int hdc3020_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		*val2 = 65536;
>  		if (chan->type == IIO_TEMP)
> -			*val = 175;
> +			*val = 175 * MILLI;
>  		else
> -			*val = 100;
> +			*val = 100 * MILLI;
>  		return IIO_VAL_FRACTIONAL;
>  
>  	case IIO_CHAN_INFO_OFFSET:

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

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

* Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
  2025-09-01 17:51 ` [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis Dimitri Fedrau via B4 Relay
  2025-09-02 12:04   ` Javier Carrasco
@ 2025-09-02 12:47   ` Andy Shevchenko
  2025-09-02 13:54     ` Dimitri Fedrau
  1 sibling, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2025-09-02 12:47 UTC (permalink / raw)
  To: dimitri.fedrau
  Cc: Javier Carrasco, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Dimitri Fedrau, Jonathan Cameron,
	linux-iio, linux-kernel, Chris Lesiak

On Mon, Sep 01, 2025 at 07:51:59PM +0200, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> According to the ABI the units after application of scale and offset are
> milli degree celsius for temperature thresholds and milli percent for
> relative humidity thresholds. Currently the resulting units are degree
> celsius for temperature thresholds and hysteresis and percent for relative
> humidity thresholds and hysteresis. Change scale factor to fix this issue.

...

>  	/*
> -	 * Get the temperature threshold from 9 LSBs, shift them to get
> -	 * the truncated temperature threshold representation and
> -	 * calculate the threshold according to the formula in the
> -	 * datasheet. Result is degree celsius scaled by 65535.
> +	 * Get the temperature threshold from 9 LSBs, shift them to get the
> +	 * truncated temperature threshold representation and calculate the
> +	 * threshold according to the formula in the datasheet and additionally

Replace "formula in the datasheet" by explicit formula

> +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> +	 * calculating threshold and hysteresis values.
>  	 */
>  	temp = FIELD_GET(HDC3020_THRESH_TEMP_MASK, thresh) <<
>  	       HDC3020_THRESH_TEMP_TRUNC_SHIFT;
>  
> -	return -2949075 + (175 * temp);
> +	return -589815 + (35 * temp);

TBH, I prefer to have the proper units be mentioned in the comment along with

	return -2949075 / 5 + ((175 / 5) * temp);

5 itself can be a definition

#define ..._PRE_SCALE	5

and used everywhere.

...

>  	/*
>  	 * Get the humidity threshold from 7 MSBs, shift them to get the
>  	 * truncated humidity threshold representation and calculate the
> -	 * threshold according to the formula in the datasheet. Result is
> -	 * percent scaled by 65535.
> +	 * threshold according to the formula in the datasheet and additionally
> +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> +	 * calculating threshold and hysteresis values.
>  	 */

Ditto. "percent scaled by ..." is much better to understand.

>  	hum = FIELD_GET(HDC3020_THRESH_HUM_MASK, thresh) <<
>  	      HDC3020_THRESH_HUM_TRUNC_SHIFT;
>  
> -	return hum * 100;
> +	return hum * 20;
>  }

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
  2025-09-02 12:04   ` Javier Carrasco
@ 2025-09-02 12:51     ` Dimitri Fedrau
  0 siblings, 0 replies; 9+ messages in thread
From: Dimitri Fedrau @ 2025-09-02 12:51 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: dimitri.fedrau, Li peiyu, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Jonathan Cameron, linux-iio,
	linux-kernel, Chris Lesiak

Am Tue, Sep 02, 2025 at 08:04:53PM +0800 schrieb Javier Carrasco:
> Hi Dimitri, thank you for your patch. A few comments inline.
> 
> On Tue Sep 2, 2025 at 1:51 AM CST, Dimitri Fedrau via B4 Relay wrote:
> > From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> >
> > According to the ABI the units after application of scale and offset are
> > milli degree celsius for temperature thresholds and milli percent for
> > relative humidity thresholds. Currently the resulting units are degree
> > celsius for temperature thresholds and hysteresis and percent for relative
> > humidity thresholds and hysteresis. Change scale factor to fix this issue.
> >
> > Fixes: 3ad0e7e5f0cb ("iio: humidity: hdc3020: add threshold events support")
> > Reported-by: Chris Lesiak <chris.lesiak@licorbio.com>
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  drivers/iio/humidity/hdc3020.c | 69 +++++++++++++++++++++++++-----------------
> >  1 file changed, 41 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
> > index 8aa567d9aded9cab461f1f905b6b5ada721ba2f0..deb19e0291f7f820c5f8760fbf0682379ab5f34b 100644
> > --- a/drivers/iio/humidity/hdc3020.c
> > +++ b/drivers/iio/humidity/hdc3020.c
> > @@ -72,6 +72,13 @@
> >  #define HDC3020_MAX_TEMP_HYST_MICRO	164748607
> >  #define HDC3020_MAX_HUM_MICRO		99220264
> >  
> > +/*
> > + * In order to avoid overflows when returning thresholds and hysteresis values,
> > + * the fraction of these is set to 13107, also the datasheet refers to 65535
> > + * this is not enough to prevent overflows. Dividing this value by 5 fixes this.
> > + */
> > +#define HDC3020_THRESH_FRACTION		(65535 / 5)
> > +
> 
> I think the comment is a bit too verbose. If you explain why the value
> has to be divided by 5, you probably don't need to tell the result of
> the division. Something as simple as "divide 65535 from the datasheet by
> 5 to avoid overflows" could do.
>

Ok, will change the comment as you suggested.

> >  struct hdc3020_data {
> >  	struct i2c_client *client;
> >  	struct gpio_desc *reset_gpio;
> > @@ -376,15 +383,16 @@ static int hdc3020_thresh_get_temp(u16 thresh)
> >  	int temp;
> >  
> >  	/*
> > -	 * Get the temperature threshold from 9 LSBs, shift them to get
> > -	 * the truncated temperature threshold representation and
> > -	 * calculate the threshold according to the formula in the
> > -	 * datasheet. Result is degree celsius scaled by 65535.
> > +	 * Get the temperature threshold from 9 LSBs, shift them to get the
> > +	 * truncated temperature threshold representation and calculate the
> > +	 * threshold according to the formula in the datasheet and additionally
> > +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> > +	 * calculating threshold and hysteresis values.
> 
> If I got it right, you are actually dividing by 5 and not by
> HDC3020_THRESH_FRACTION.

I do scale it by HDC3020_THRESH_FRACTION.

Formula from the datasheet is:
T( deggree celsius) = -45 + (175 * temp) / 65535

Before patching it was(scaled by 65535):

T(degree celsius) * 65535 = -2949075 + (175 * temp)

After patching(scaled by 13107):

T(degree celsius) * 13107 = -589815 + (35 * temp)

I need to scale the return value to avoid precision loss when
calculating threshold and hysteresis values.

> 
> >  	 */
> >  	temp = FIELD_GET(HDC3020_THRESH_TEMP_MASK, thresh) <<
> >  	       HDC3020_THRESH_TEMP_TRUNC_SHIFT;
> >  
> > -	return -2949075 + (175 * temp);
> > +	return -589815 + (35 * temp);
> >  }
> >  
> >  static int hdc3020_thresh_get_hum(u16 thresh)
> > @@ -394,13 +402,14 @@ static int hdc3020_thresh_get_hum(u16 thresh)
> >  	/*
> >  	 * Get the humidity threshold from 7 MSBs, shift them to get the
> >  	 * truncated humidity threshold representation and calculate the
> > -	 * threshold according to the formula in the datasheet. Result is
> > -	 * percent scaled by 65535.
> > +	 * threshold according to the formula in the datasheet and additionally
> > +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> > +	 * calculating threshold and hysteresis values.
> 
> Same here?
> 

See above.

> >  	 */
> >  	hum = FIELD_GET(HDC3020_THRESH_HUM_MASK, thresh) <<
> >  	      HDC3020_THRESH_HUM_TRUNC_SHIFT;
> >  
> > -	return hum * 100;
> > +	return hum * 20;
> >  }
>

Best regards,
Dimitri Fedrau

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

* Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
  2025-09-02 12:47   ` Andy Shevchenko
@ 2025-09-02 13:54     ` Dimitri Fedrau
  2025-09-02 14:22       ` Andy Shevchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Dimitri Fedrau @ 2025-09-02 13:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: dimitri.fedrau, Javier Carrasco, Li peiyu, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Jonathan Cameron,
	linux-iio, linux-kernel, Chris Lesiak

Hi Andy,

Am Tue, Sep 02, 2025 at 03:47:52PM +0300 schrieb Andy Shevchenko:
> On Mon, Sep 01, 2025 at 07:51:59PM +0200, Dimitri Fedrau via B4 Relay wrote:
> > From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > 
> > According to the ABI the units after application of scale and offset are
> > milli degree celsius for temperature thresholds and milli percent for
> > relative humidity thresholds. Currently the resulting units are degree
> > celsius for temperature thresholds and hysteresis and percent for relative
> > humidity thresholds and hysteresis. Change scale factor to fix this issue.
> 
> ...
> 
> >  	/*
> > -	 * Get the temperature threshold from 9 LSBs, shift them to get
> > -	 * the truncated temperature threshold representation and
> > -	 * calculate the threshold according to the formula in the
> > -	 * datasheet. Result is degree celsius scaled by 65535.
> > +	 * Get the temperature threshold from 9 LSBs, shift them to get the
> > +	 * truncated temperature threshold representation and calculate the
> > +	 * threshold according to the formula in the datasheet and additionally
> 
> Replace "formula in the datasheet" by explicit formula
> 

Ok.
> > +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> > +	 * calculating threshold and hysteresis values.
> >  	 */
> >  	temp = FIELD_GET(HDC3020_THRESH_TEMP_MASK, thresh) <<
> >  	       HDC3020_THRESH_TEMP_TRUNC_SHIFT;
> >  
> > -	return -2949075 + (175 * temp);
> > +	return -589815 + (35 * temp);
> 
> TBH, I prefer to have the proper units be mentioned in the comment along with
> 
> 	return -2949075 / 5 + ((175 / 5) * temp);
> 

You are right, will add the units again.

> 5 itself can be a definition
> 
> #define ..._PRE_SCALE	5
> 
> and used everywhere.
> 
> ...
> 

The explicit formula in the datasheet:

T(degree celsius) = -45 + (175 * temp) / 65535

The formula before the patch:

T(degree celsius) * 65525 = -2949075 + (175 * temp)

Adding the PRE_SCALE into the formula doesn't improve readability from
my perspective. I would prefer to just scale the result as it has been
done before.

> >  	/*
> >  	 * Get the humidity threshold from 7 MSBs, shift them to get the
> >  	 * truncated humidity threshold representation and calculate the
> > -	 * threshold according to the formula in the datasheet. Result is
> > -	 * percent scaled by 65535.
> > +	 * threshold according to the formula in the datasheet and additionally
> > +	 * scale by HDC3020_THRESH_FRACTION to avoid precision loss when
> > +	 * calculating threshold and hysteresis values.
> >  	 */
> 
> Ditto. "percent scaled by ..." is much better to understand.
> 

Ok.
> >  	hum = FIELD_GET(HDC3020_THRESH_HUM_MASK, thresh) <<
> >  	      HDC3020_THRESH_HUM_TRUNC_SHIFT;
> >  
> > -	return hum * 100;
> > +	return hum * 20;
> >  }

Best regards,
Dimitri Fedrau

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

* Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
  2025-09-02 13:54     ` Dimitri Fedrau
@ 2025-09-02 14:22       ` Andy Shevchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2025-09-02 14:22 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: dimitri.fedrau, Javier Carrasco, Li peiyu, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Jonathan Cameron,
	linux-iio, linux-kernel, Chris Lesiak

On Tue, Sep 02, 2025 at 03:54:04PM +0200, Dimitri Fedrau wrote:
> Am Tue, Sep 02, 2025 at 03:47:52PM +0300 schrieb Andy Shevchenko:
> > On Mon, Sep 01, 2025 at 07:51:59PM +0200, Dimitri Fedrau via B4 Relay wrote:

...

> The explicit formula in the datasheet:
> 
> T(degree celsius) = -45 + (175 * temp) / 65535
> 
> The formula before the patch:
> 
> T(degree celsius) * 65525 = -2949075 + (175 * temp)

Then embed it into the comment.

> Adding the PRE_SCALE into the formula doesn't improve readability from
> my perspective. I would prefer to just scale the result as it has been
> done before.

Up to you, it was just an idea. But in any case that 5 should be explicit (and
not hidden in the precalculated values).

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2025-09-02 14:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 17:51 [PATCH v2 0/2] iio: humditiy: hdc3020: fix units Dimitri Fedrau via B4 Relay
2025-09-01 17:51 ` [PATCH v2 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement Dimitri Fedrau via B4 Relay
2025-09-02 12:05   ` Javier Carrasco
2025-09-01 17:51 ` [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis Dimitri Fedrau via B4 Relay
2025-09-02 12:04   ` Javier Carrasco
2025-09-02 12:51     ` Dimitri Fedrau
2025-09-02 12:47   ` Andy Shevchenko
2025-09-02 13:54     ` Dimitri Fedrau
2025-09-02 14:22       ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).