Re: [PATCH 2/2] iio: temperature: tmp117: add support for HIGH and LOW alerts

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 11 Oct 2026 10:57:34 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 10/05, Thomas Bonnefille wrote:
> > The TMP117/116/119 chips allow the user to set an alert if the
> > temperature reaches a HIGH or LOW limit.
> > 
> > Add support for setting the high and low alert limits.
> > An IIO event is generated when the temperature crosses either limit
> > thanks to an interrupt on the Alert pin.
> > 
> > Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> > ---  
> ...
> > +static int tmp117_read_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 tmp117_data *data = iio_priv(indio_dev);
> > +	s32 ret;
> > +	u8 reg;
> > +
> > +	switch (dir) {
> > +	case IIO_EV_DIR_RISING:
> > +		reg = TMP117_REG_HIGH_LIM;
> > +		break;
> > +	case IIO_EV_DIR_FALLING:
> > +		reg = TMP117_REG_LOW_LIM;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (info != IIO_EV_INFO_VALUE || type != IIO_EV_TYPE_THRESH)
> > +		return -EINVAL;
> > +	ret = i2c_smbus_read_word_swapped(data->client, reg);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/*
> > +	 * Report the threshold in millidegrees Celsius as an exact fraction:
> > +	 * value = raw * TMP117_LIMIT_LSB_NUM / TMP117_LIMIT_LSB_DEN.
> > +	 */  
> The IIO temperature channel provides single-read data in (raw) device units
> (i.e. not scaled to mili degrees Celsius). Now, in contrast with the single-read
> interface, the event configuration being proposed reads/writes data in milli
> degrees Celsius. And, since the event ABI doesn't yet provide clear indication
> about IIO event data unit types, users might get confused when using the
> different interfaces.
> 
> There are two main options for avoiding/mitigating the potential interface mismatch:
> (1) provide/get raw device units in tmp117_read/write_thresh().

This one is my strong preference.

> 
> (2) add IIO_CHAN_INFO_PROCESSED to the IIO channel mask and implement the
> handling for the channel _input interface to provide single-read data in
> milli degrees Celsius units. Though, this option will also need an updated
> IIO event interface. Probably something like [1] or [2]. I'll respin on that
> next week. The event specs would be updated to something like
> 
> 
> +static const struct iio_event_spec tmp11x_obj_event[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.unit = IIO_EV_UNIT_PROCESSED,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	},
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.unit = IIO_EV_UNIT_PROCESSED,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	},
> +};
> +
> 
> 
> [1]: https://lore.kernel.org/linux-iio/cover.1768759292.git.marcelo.schmitt1@gmail.com/
> [2]: https://lore.kernel.org/linux-iio/e359fa003ebc61c8b0e37ba9810a04c70e5a300e.1791437050.git.marcelo.schmitt@analog.com/
> 
> With best regards,
> Marcelo
> 
> > +	*val = sign_extend32(ret, 15) * TMP117_LIMIT_LSB_NUM;
> > +	*val2 = TMP117_LIMIT_LSB_DEN;
> > +
> > +	return IIO_VAL_FRACTIONAL;
> > +}
> > +
> > +static int tmp117_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 tmp117_data *data = iio_priv(indio_dev);
> > +	s64 tmp;
> > +	u8 reg;
> > +
> > +	switch (dir) {
> > +	case IIO_EV_DIR_RISING:
> > +		reg = TMP117_REG_HIGH_LIM;
> > +		break;
> > +	case IIO_EV_DIR_FALLING:
> > +		reg = TMP117_REG_LOW_LIM;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (info != IIO_EV_INFO_VALUE || type != IIO_EV_TYPE_THRESH)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * The threshold is passed in millidegrees Celsius as
> > +	 * value = val + val2 / 1000000 (val2 holds the fractional part, with
> > +	 * the sign taken from val). Convert it back with the same LSB ratio:
> > +	 * raw = value / (TMP117_LIMIT_LSB_NUM / TMP117_LIMIT_LSB_DEN).
> > +	 */
> > +	tmp = (val < 0) ? (s64)val * 1000000 - val2
> > +			: (s64)val * 1000000 + val2;
> > +	tmp = DIV_ROUND_CLOSEST(tmp * TMP117_LIMIT_LSB_DEN,
> > +				(s64)TMP117_LIMIT_LSB_NUM * 1000000);
> > +	tmp = clamp_t(s64, tmp, S16_MIN, S16_MAX);
> > +
> > +	return i2c_smbus_write_word_swapped(data->client,
> > +					    reg, tmp);
> > +}
> > +