Received-SPF: pass (google.com: domain of linux-kernel+bounces-51019-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <b789eb01-5b5d-4513-88d2-1f14e95969d9@gmail.com>
Date: Sat, 3 Feb 2024 11:06:02 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] iio: humidity: hdc3020: add threshold events support
Content-Language: en-US
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>, dima.fedrau@gmail.com
Cc: 579lpy@gmail.com, jic23@kernel.org, lars@metafoo.de,
 linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240203090530.53374-1-dima.fedrau@gmail.com>
 <236dc9db-8525-413e-a77b-54df198c86f5@wanadoo.fr>
From: Javier Carrasco <javier.carrasco.cruz@gmail.com>
In-Reply-To: <236dc9db-8525-413e-a77b-54df198c86f5@wanadoo.fr>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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