2024-02-03 10:06:27

by Javier Carrasco

[permalink] [raw]
Subject: Re: [PATCH] iio: humidity: hdc3020: add threshold events support

On 03.02.24 10:58, Christophe JAILLET wrote:
> Le 03/02/2024 à 10:05, Dimitri Fedrau a écrit :
>> Add threshold events support for temperature and relative humidity. To
>> enable them the higher and lower threshold registers must be programmed
>> and the higher threshold must be greater then or equal to the lower
>> threshold. Otherwise the event is disabled. Invalid hysteresis values
>> are ignored by the device. There is no further configuration possible.
>>
>> Tested by setting thresholds/hysteresis and turning the heater on/off.
>> Used iio_event_monitor in tools/iio to catch events while constantly
>> displaying temperature and humidity values.
>> Threshold and hysteresis values are cached in the driver, used i2c-tools
>> to read the threshold and hysteresis values from the device and make
>> sure cached values are consistent to values written to the device.
>>
>> Based on Fix:
>> a69eeaad093d "iio: humidity: hdc3020: fix temperature offset" in branch
>> fixes-togreg
>>
>> Signed-off-by: Dimitri Fedrau
>> <[email protected]>
>> ---
>> drivers/iio/humidity/hdc3020.c | 339 +++++++++++++++++++++++++++++++++
>> 1 file changed, 339 insertions(+)

..

>> + guard(mutex)(&data->lock);
>> + switch (chan->type) {
>> + case IIO_TEMP:
>> + /*
>> + * Store truncated temperature threshold into 9 LSBs while
>> + * keeping the old humidity threshold in the 7 MSBs.
>> + */
>> + val = (((val + 45) * 65535 / 175) >> HDC3020_THRESH_TEMP_SHIFT);
>
> Why 175?
> If the span is -40/+120, I guess it should be 160 and if it is -45/+120,
> 165. No?
>
> Maybe something like:
> #define MIN_TEMP -45 (or -40)
> #define MAX_TEMP 120
> in order to avoid hard coded constant?
>

the 45 and 175 values come from the conversion formula provided in the
datasheet (page 13), even though the sensor range is from –40°C to 125°C.

>> + val &= HDC3020_THRESH_TEMP_MASK;
>> + val |= (*thresh & HDC3020_THRESH_HUM_MASK);
>> + break;
>> + case IIO_HUMIDITYRELATIVE:
>> + /*
>> + * Store truncated humidity threshold into 7 MSBs while
>> + * keeping the old temperature threshold in the 9 LSBs.
>> + */
>> + val = ((val * 65535 / 100) & HDC3020_THRESH_HUM_MASK);
>> + val |= (*thresh & HDC3020_THRESH_TEMP_MASK);
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + put_unaligned_be16(val, &buf[2]);
>> + buf[4] = crc8(hdc3020_crc8_table, buf + 2, 2, CRC8_INIT_VALUE);
>> + ret = hdc3020_write_bytes(data, buf, 5);
>> + if (ret)
>> + return ret;
>> +
>> + /* Update threshold */
>> + *thresh = val;
>> +
>> + return 0;
>> +}
>
> CJ
>

Best regards,
Javier Carrasco


2024-02-03 14:54:16

by Dimitri Fedrau

[permalink] [raw]
Subject: Re: [PATCH] iio: humidity: hdc3020: add threshold events support

Am Sat, Feb 03, 2024 at 11:06:02AM +0100 schrieb Javier Carrasco:
> On 03.02.24 10:58, Christophe JAILLET wrote:
> > Le 03/02/2024 à 10:05, Dimitri Fedrau a écrit :
> >> [...]
> >> drivers/iio/humidity/hdc3020.c | 339 +++++++++++++++++++++++++++++++++
> >> 1 file changed, 339 insertions(+)
>
> ...
>
> >> + guard(mutex)(&data->lock);
> >> + switch (chan->type) {
> >> + case IIO_TEMP:
> >> + /*
> >> + * Store truncated temperature threshold into 9 LSBs while
> >> + * keeping the old humidity threshold in the 7 MSBs.
> >> + */
> >> + val = (((val + 45) * 65535 / 175) >> HDC3020_THRESH_TEMP_SHIFT);
> >
> > Why 175?
> > If the span is -40/+120, I guess it should be 160 and if it is -45/+120,
> > 165. No?
> >
> > Maybe something like:
> > #define MIN_TEMP -45 (or -40)
> > #define MAX_TEMP 120
> > in order to avoid hard coded constant?
> >
>
> the 45 and 175 values come from the conversion formula provided in the
> datasheet (page 13), even though the sensor range is from –40°C to 125°C.
>
Will add following constants:
#define MIN_TEMP -40
#define MAX_TEMP 125

It's the supported temperature range by the chip as Javier already
explained. Thanks for finding this.
> >> [...]

Best regards,
Dimitri