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

[Dimitri Fedrau <dima.fedrau@gmail.com>]

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