Re: [PATCH v2] iio: temperature: tmp117: fix calibbias cache update on I2C write failure

On Sun, 23 Aug 2026 05:23:00 +0100
Salah Triki <[email protected]> wrote:

> The calibbias cache (data->calibbias) was updated before the I2C
> write to TMP117_REG_TEMP_OFFSET was known to succeed. If the write
> failed, the function correctly returned an error, but the driver's
> internal cache had already been updated to the new value.
> 
> This causes the cache and the actual hardware register to go out of
> sync: a subsequent write of the same value would be silently
> skipped by the early "if (off == data->calibbias) return 0;" check,
> since the cache matches even though the register was never
> successfully updated.
> 
> Update data->calibbias only after confirming the I2C write
> succeeded, so the cache always reflects the actual state of the
> device.
> 
> Found by code inspection.
> 
> Fixes: df041e737a38 ("iio: temperature: add driver support for ti tmp117")
> Signed-off-by: Salah Triki <[email protected]>

Unlike some of your other fixes this one to me is low urgency and not necessary
to backport. So applied to the testing branch of iio.git (material for next merge window)

Whilst here a bit more info on why I'm rapidly applying some of your fixes and
sitting on others.  I tend to take into account

1) Complexity of the fix - reluctant to rush on the hard ones!  This also applies more
   to core fixes where the blast radius is higher if we get it wrong,
2) Activity on the driver - if it is recent and the author is still around or others
   have been active (real changes to that driver, not tree wide stuff) then chances
   of getting additional specific review is good.
3) Manufacturer specific - generally if we have frequent contributions from a given
   manufacturer, they are also pretty good about reviewing fixes for other drivers
   supporting their hardware - even more so if someone working for them wrote the
   driver
4) Already had some review on earlier versions.
5) Phase of the moon.  I won't claim any great consistency on this ;)  Sometimes I work
   forwards in time through my backlog, sometimes backwards, sometimes based on size of
   patch etc.

Jonathan

> ---
> Changes since v2:
> - Fixed formatting around ret assignment and check (Joshua Crofts).
> - Mentioned in commit description that the issue was found by code inspection (David Lechner).
> 
>  drivers/iio/temperature/tmp117.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/temperature/tmp117.c b/drivers/iio/temperature/tmp117.c
> index 6bc18616ad15..a07028d560ba 100644
> --- a/drivers/iio/temperature/tmp117.c
> +++ b/drivers/iio/temperature/tmp117.c
> @@ -95,15 +95,20 @@ static int tmp117_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec
>  {
>  	struct tmp117_data *data = iio_priv(indio_dev);
>  	s16 off;
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		off = clamp_t(int, val, S16_MIN, S16_MAX);
>  		if (off == data->calibbias)
>  			return 0;
> +
> +		ret = i2c_smbus_write_word_swapped(data->client, TMP117_REG_TEMP_OFFSET, off);
> +		if (ret)
> +			return ret;
> +
>  		data->calibbias = off;
> -		return i2c_smbus_write_word_swapped(data->client,
> -						TMP117_REG_TEMP_OFFSET, off);
> +		return 0;
>  
>  	default:
>  		return -EINVAL;