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

[Javier Carrasco <javier.carrasco.cruz@gmail.com>]

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
>> <dima.fedrau-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   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