Re: [PATCH 1/2] iio: adc: ti-adc161s626: fix buffer read on big-endian

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 16 Mar 2026 15:56:26 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Sat, Mar 14, 2026 at 06:13:31PM -0500, David Lechner wrote:
> > Rework ti_adc_trigger_handler() to properly handle data on big-endian
> > architectures. The scan data format is 16-bit CPU-endian, so we can't
> > cast it to a int * on big-endian and expect it to work. Instead, we
> > introduce a local int variable to read the data into, and then copy it
> > to the buffer.
> > 
> > Since the buffer isn't passed to any SPI functions, we don't need it to
> > be DMA-safe. So we can drop it from the driver data struct and just
> > use stack memory for the scan data.
> > 
> > Since there is only one data value (plus timestamp), we don't need an
> > array and can just declare a struct with the correct data type instead.
> > 
> > Also fix alignment of iio_get_time_ns() to ( while we are touching this.  
> 
> ...

I agree with Andy's comments, so applied with diff below.
Please sanity check this. Applied to the fixes-togreg branch of iio.git and marked
for stable.

Thanks,

J

diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index 1d427548e0b3..42968d96572b 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -112,19 +112,20 @@ static irqreturn_t ti_adc_trigger_handler(int irq, void *private)
        struct iio_poll_func *pf = private;
        struct iio_dev *indio_dev = pf->indio_dev;
        struct ti_adc_data *data = iio_priv(indio_dev);
-       int ret, val;
        struct {
                s16 data;
                aligned_s64 timestamp;
        } scan = { };
+       int ret, val;
 
        ret = ti_adc_read_measurement(data, &indio_dev->channels[0], &val);
-       if (!ret) {
-               scan.data = val;
-               iio_push_to_buffers_with_timestamp(indio_dev, &scan,
-                                                  iio_get_time_ns(indio_dev));
-       }
+       if (ret)
+               goto exit_notify_done;
+
+       scan.data = val;
+       iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
 
+ exit_notify_done:
        iio_trigger_notify_done(indio_dev->trig);
 
        return IRQ_HANDLED;

> 
> >  	struct iio_poll_func *pf = private;
> >  	struct iio_dev *indio_dev = pf->indio_dev;
> >  	struct ti_adc_data *data = iio_priv(indio_dev);  
> 
> > +	int ret, val;  
> 
> Can this be put after the 'scan'?
> 
> > +	struct {
> > +		s16 data;
> > +		aligned_s64 timestamp;
> > +	} scan = { };
> > +
> > +	ret = ti_adc_read_measurement(data, &indio_dev->channels[0], &val);  
> 
> > +	if (!ret) {  
> 
> Personally I prefer goto approach over unusual pattern and TAB indentation.
> 
> > +		scan.data = val;
> > +		iio_push_to_buffers_with_timestamp(indio_dev, &scan,
> > +						   iio_get_time_ns(indio_dev));
> > +	}
> >  
> > +	ret = ti_adc_read_measurement(data, &indio_dev->channels[0], &val);  
> 
> 	if (ret)
> 		goto exit_notify_done;
> 
> 	scan.data = val;
> 	// Also can we consider moving towards new API?
> 	iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
> 
> exit_notify_done:
> 
> >  	iio_trigger_notify_done(indio_dev->trig);  
>