linux-kernel - Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DCIAU0EO8E0K.3JIXZ2XICP3YC@gmail.com>
Date: Tue, 02 Sep 2025 20:04:53 +0800
From: "Javier Carrasco" <javier.carrasco.cruz@...il.com>
To: <dimitri.fedrau@...bherr.com>, "Li peiyu" <579lpy@...il.com>, "Jonathan
 Cameron" <jic23@...nel.org>, "David Lechner" <dlechner@...libre.com>,
 Nuno Sá <nuno.sa@...log.com>, "Andy Shevchenko"
 <andy@...nel.org>, "Dimitri Fedrau" <dima.fedrau@...il.com>
Cc: "Jonathan Cameron" <Jonathan.Cameron@...wei.com>,
 <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Chris Lesiak"
 <chris.lesiak@...orbio.com>
Subject: Re: [PATCH v2 2/2] iio: humditiy: hdc3020: fix units for thresholds
 and hysteresis

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@...bherr.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@...orbio.com>
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@...bherr.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