linux-kernel - Re: [PATCH v2] iio: humidity: hdc3020: add threshold events support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <194ed4ce-bb2e-404e-a716-ec2e9876c740@gmail.com>
Date: Sun, 4 Feb 2024 12:26:53 +0100
From: Javier Carrasco <javier.carrasco.cruz@...il.com>
To: Dimitri Fedrau <dima.fedrau@...il.com>
Cc: Li peiyu <579lpy@...il.com>, Jonathan Cameron <jic23@...nel.org>,
 Lars-Peter Clausen <lars@...afoo.de>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: humidity: hdc3020: add threshold events support

Hi Dimitri,

On 04.02.24 11:37, Dimitri Fedrau wrote:
>  
> +static int hdc3020_write_thresh(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *chan,
> +				enum iio_event_type type,
> +				enum iio_event_direction dir,
> +				enum iio_event_info info,
> +				int val, int val2)
> +{
> +	struct hdc3020_data *data = iio_priv(indio_dev);
> +	u16 *thresh;
> +	u8 buf[5];
> +	int ret;
> +
I tested your patch right now and I noticed that you are only writing
integer values, which means that for example 19.9999 turns into 18.9169.
It seems that you are not using val2 (the decimal part).

Is there any reason for that? The displayed thresholds are decimal, though.
> +	/* Supported temperature range is from –40 to 125 degree celsius */
> +	if (val < HDC3020_MIN_TEMP || val > HDC3020_MAX_TEMP)
> +		return -EINVAL;
> +
> +	/* Select threshold and associated register */
> +	if (info == IIO_EV_INFO_VALUE) {
> +		if (dir == IIO_EV_DIR_RISING) {
> +			thresh = &data->t_rh_thresh_high;
> +			memcpy(buf, HDC3020_S_T_RH_THRESH_HIGH, 2);
> +		} else {
> +			thresh = &data->t_rh_thresh_low;
> +			memcpy(buf, HDC3020_S_T_RH_THRESH_LOW, 2);
> +		}
> +	} else {
> +		if (dir == IIO_EV_DIR_RISING) {
> +			thresh = &data->t_rh_thresh_high_clr;
> +			memcpy(buf, HDC3020_S_T_RH_THRESH_HIGH_CLR, 2);
> +		} else {
> +			thresh = &data->t_rh_thresh_low_clr;
> +			memcpy(buf, HDC3020_S_T_RH_THRESH_LOW_CLR, 2);
> +		}
> +	}
> +
> +	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);
> +		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;
> +}

Best regards,
Javier Carrasco