All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Javier Carrasco" <javier.carrasco.cruz@gmail.com>
To: dimitri.fedrau@liebherr.com, "Li peiyu" <579lpy@gmail.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Dimitri Fedrau" <dima.fedrau@gmail.com>
Cc: "Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
	<linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	"Chris Lesiak" <chris.lesiak@licorbio.com>
Subject: Re: [PATCH v3 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis
Date: Wed, 15 Oct 2025 16:29:49 +0800	[thread overview]
Message-ID: <DDIR6RWVMIRK.2DQB98BRS180Z@gmail.com> (raw)
In-Reply-To: <20251013-hdc3020-units-fix-v3-2-b21fab32b882@liebherr.com>

On Mon Oct 13, 2025 at 4:12 PM 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 | 67 ++++++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
> index 8aa567d9aded9cab461f1f905b6b5ada721ba2f0..5a978801fb09ca49c449028d046a91e0e67c38ea 100644
> --- a/drivers/iio/humidity/hdc3020.c
> +++ b/drivers/iio/humidity/hdc3020.c
> @@ -72,6 +72,9 @@
>  #define HDC3020_MAX_TEMP_HYST_MICRO	164748607
>  #define HDC3020_MAX_HUM_MICRO		99220264
>  
> +/* Divide 65535 from the datasheet by 5 to avoid overflows */
> +#define HDC3020_THRESH_FRACTION		(65535 / 5)
> +
>  struct hdc3020_data {
>  	struct i2c_client *client;
>  	struct gpio_desc *reset_gpio;
> @@ -376,15 +379,17 @@ 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 explicit formula in the datasheet and
> +	 * additionally scale by HDC3020_THRESH_FRACTION to avoid precision loss
> +	 * when calculating threshold and hysteresis values. Result is degree
> +	 * celsius scaled by HDC3020_THRESH_FRACTION.

I see that Andy asked you to include the formula, and you accepted that
suggestion, but the formula is still missing. I also think it should be
added to the comment because that would clarify where this kind of
magic numbers in the return value come from.

>  	 */
>  	temp = FIELD_GET(HDC3020_THRESH_TEMP_MASK, thresh) <<
>  	       HDC3020_THRESH_TEMP_TRUNC_SHIFT;
>  
> -	return -2949075 + (175 * temp);
> +	return -589815 + (35 * temp);
>  }

With that modification,

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

  reply	other threads:[~2025-10-15  8:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13  8:12 [PATCH v3 0/2] iio: humditiy: hdc3020: fix units Dimitri Fedrau
2025-10-13  8:12 ` Dimitri Fedrau via B4 Relay
2025-10-13  8:12 ` [PATCH v3 1/2] iio: humditiy: hdc3020: fix units for temperature and humidity measurement Dimitri Fedrau
2025-10-13  8:12   ` Dimitri Fedrau via B4 Relay
2025-10-13  8:12 ` [PATCH v3 2/2] iio: humditiy: hdc3020: fix units for thresholds and hysteresis Dimitri Fedrau
2025-10-13  8:12   ` Dimitri Fedrau via B4 Relay
2025-10-15  8:29   ` Javier Carrasco [this message]
2025-10-15  9:45   ` Andy Shevchenko
2025-10-15 14:40     ` Dimitri Fedrau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DDIR6RWVMIRK.2DQB98BRS180Z@gmail.com \
    --to=javier.carrasco.cruz@gmail.com \
    --cc=579lpy@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andy@kernel.org \
    --cc=chris.lesiak@licorbio.com \
    --cc=dima.fedrau@gmail.com \
    --cc=dimitri.fedrau@liebherr.com \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.