Re: [PATCH] iio: temperature: tmp006: Fix endianness byte swap in trigger handler

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 7 Sep 2026 16:47:33 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Mon,  7 Sep 2026 09:22:15 +0100
> Salah Triki <salah.triki@gmail.com> wrote:
> 
> > In tmp006_trigger_handler(), i2c_smbus_read_word_data() is used to read
> > the VOBJECT and TAMBIENT registers. However, the TMP006 stores register
> > values in Big Endian byte order, while i2c_smbus_read_word_data() assumes
> > Little Endian.
> > 
> > This causes byte-swapped values to be pushed to the IIO buffer on Little
> > Endian architectures, leading to corrupted sensor readings when using
> > triggered buffers.
> > 
> > Fix this by switching to i2c_smbus_read_word_swapped() inside the trigger
> > handler.
> > 
> > Fixes: 91f75ccf9f03 ("iio: temperature: tmp006: add triggered buffer support")
> > Assisted-by: LLM
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > ---
> >  drivers/iio/temperature/tmp006.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/temperature/tmp006.c b/drivers/iio/temperature/tmp006.c
> > index d9f6449ec0d8..f3b3de0bf221 100644
> > --- a/drivers/iio/temperature/tmp006.c
> > +++ b/drivers/iio/temperature/tmp006.c
> > @@ -256,12 +256,12 @@ static irqreturn_t tmp006_trigger_handler(int irq, void *p)
> >  	} scan = { };
> >  	s32 ret;
> >  
> > -	ret = i2c_smbus_read_word_data(data->client, TMP006_VOBJECT);
> > +	ret = i2c_smbus_read_word_swapped(data->client, TMP006_VOBJECT);  
> 
> Sashiko has something to say:
> 
> Does this code introduce a regression where sensor readings are corrupted for
> userspace clients reading from the triggered IIO buffer on Little Endian
> architectures?
> 
> Before this patch, i2c_smbus_read_word_data() returned a byte-swapped word.
> When this was assigned to the s16 array in tmp006_trigger_handler() on a Little
> Endian CPU, the memory layout became [msb, lsb]. This implicitly matched the
> IIO_BE (Big Endian) endianness declared in the tmp006_channels array, so it
> worked correctly.
> 
> By switching to i2c_smbus_read_word_swapped(), the value is now in native CPU
> endianness [lsb, msb]. However, the channel specification in tmp006_channels
> still specifies IIO_BE:
> drivers/iio/temperature/tmp006.c:tmp006_channels[] {
>     ...
>         .scan_type = {
>             .endianness = IIO_BE,
>         },
>     ...
> }
> 
> Because the ABI still advertises the data as Big Endian, userspace will parse
> the little-endian data as big-endian.
> 
> Should the channel specification be updated from IIO_BE to IIO_CPU, or should
> scan.channels be declared as __be16 and populated using cpu_to_be16() to
> preserve the existing Big Endian ABI?
> 
Nice catch for sashiko.  It is absolutely correct.  Thanks for
highlighting this one!

Jonathan