[PATCH 0/2] iio: adc: ti-adc161s626: fix scan buffer handling

[PATCH 0/2] iio: adc: ti-adc161s626: fix scan buffer handling

[David Lechner]

While searching for odd-looking casts to int *, I found there was a bug
with that here and some deeper bugs as well.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
David Lechner (2):
      iio: adc: ti-adc161s626: fix buffer read on big-endian
      iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

 drivers/iio/adc/ti-adc161s626.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)
---
base-commit: ff0843ceb1fb11a6b73e0e77b932ef7967aecd4b
change-id: 20260314-iio-adc-ti-adc161s626-fix-scan-buf-e1d5b983a58f

Best regards,
--  
David Lechner <dlechner@baylibre.com>

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

[David Lechner]

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.

Fixes: 4d671b71beef ("iio: adc: ti-adc161s626: add support for TI 1-channel differential ADCs")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ti-adc161s626.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index 28aa6b80160c..1d427548e0b3 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -70,8 +70,6 @@ struct ti_adc_data {
 
 	u8 read_size;
 	u8 shift;
-
-	u8 buffer[16] __aligned(IIO_DMA_MINALIGN);
 };
 
 static int ti_adc_read_measurement(struct ti_adc_data *data,
@@ -114,14 +112,18 @@ 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;
-
-	ret = ti_adc_read_measurement(data, &indio_dev->channels[0],
-				     (int *) &data->buffer);
-	if (!ret)
-		iio_push_to_buffers_with_timestamp(indio_dev,
-					data->buffer,
-					iio_get_time_ns(indio_dev));
+	int ret, val;
+	struct {
+		s16 data;
+		aligned_s64 timestamp;
+	} scan = { };
+
+	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));
+	}
 
 	iio_trigger_notify_done(indio_dev->trig);
 

-- 
2.43.0

[PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[David Lechner]

Add a DMA-safe buffer and use it for spi_read() instead of a stack
memory. All SPI buffers must be DMA-safe.

Since we only need up to 3 bytes, we just use a u8[] instead of __be16
and __be32 and change the conversion functions appropriately.

Fixes: 4d671b71beef ("iio: adc: ti-adc161s626: add support for TI 1-channel differential ADCs")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ti-adc161s626.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index 1d427548e0b3..6416d6a7ada0 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -70,6 +70,7 @@ struct ti_adc_data {
 
 	u8 read_size;
 	u8 shift;
+	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
 };
 
 static int ti_adc_read_measurement(struct ti_adc_data *data,
@@ -78,26 +79,20 @@ static int ti_adc_read_measurement(struct ti_adc_data *data,
 	int ret;
 
 	switch (data->read_size) {
-	case 2: {
-		__be16 buf;
-
-		ret = spi_read(data->spi, (void *) &buf, 2);
+	case 2:
+		ret = spi_read(data->spi, data->buf, 2);
 		if (ret)
 			return ret;
 
-		*val = be16_to_cpu(buf);
+		*val = get_unaligned_be16(data->buf);
 		break;
-	}
-	case 3: {
-		__be32 buf;
-
-		ret = spi_read(data->spi, (void *) &buf, 3);
+	case 3:
+		ret = spi_read(data->spi, data->buf, 3);
 		if (ret)
 			return ret;
 
-		*val = be32_to_cpu(buf) >> 8;
+		*val = get_unaligned_be24(data->buf);
 		break;
-	}
 	default:
 		return -EINVAL;
 	}

-- 
2.43.0

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

[Andy Shevchenko]

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.

...

>  	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);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Andy Shevchenko]

On Sat, Mar 14, 2026 at 06:13:32PM -0500, David Lechner wrote:
> Add a DMA-safe buffer and use it for spi_read() instead of a stack
> memory. All SPI buffers must be DMA-safe.
> 
> Since we only need up to 3 bytes, we just use a u8[] instead of __be16
> and __be32 and change the conversion functions appropriately.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Jonathan Cameron]

On Sat, 14 Mar 2026 18:13:32 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Add a DMA-safe buffer and use it for spi_read() instead of a stack
> memory. All SPI buffers must be DMA-safe.
> 
> Since we only need up to 3 bytes, we just use a u8[] instead of __be16
> and __be32 and change the conversion functions appropriately.
> 
> Fixes: 4d671b71beef ("iio: adc: ti-adc161s626: add support for TI 1-channel differential ADCs")
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/adc/ti-adc161s626.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
> index 1d427548e0b3..6416d6a7ada0 100644
> --- a/drivers/iio/adc/ti-adc161s626.c
> +++ b/drivers/iio/adc/ti-adc161s626.c
> @@ -70,6 +70,7 @@ struct ti_adc_data {
>  
>  	u8 read_size;
>  	u8 shift;
> +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
On this. There is new generic infrastructure for marking these.
https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/

Would look like
	__dma_from_device_group_begin();
	u8 buf[3];
	__dma_from_device_group_end();

Do you think we should adopt them rather than doing our own thing?
Slowly though I don't want the noise of a mass conversion.

As normal, advantage of standard infrastructure is cutting down
in subsystem specific magic.

I 'think' result is the same (though it also forces the trailing padding if anything
comes after this and needs it).


>  };
>  
>  static int ti_adc_read_measurement(struct ti_adc_data *data,
> @@ -78,26 +79,20 @@ static int ti_adc_read_measurement(struct ti_adc_data *data,
>  	int ret;
>  
>  	switch (data->read_size) {
> -	case 2: {
> -		__be16 buf;
> -
> -		ret = spi_read(data->spi, (void *) &buf, 2);
> +	case 2:
> +		ret = spi_read(data->spi, data->buf, 2);
>  		if (ret)
>  			return ret;
>  
> -		*val = be16_to_cpu(buf);
> +		*val = get_unaligned_be16(data->buf);
>  		break;
> -	}
> -	case 3: {
> -		__be32 buf;
> -
> -		ret = spi_read(data->spi, (void *) &buf, 3);
> +	case 3:
> +		ret = spi_read(data->spi, data->buf, 3);
>  		if (ret)
>  			return ret;
>  
> -		*val = be32_to_cpu(buf) >> 8;
> +		*val = get_unaligned_be24(data->buf);
>  		break;
> -	}
>  	default:
>  		return -EINVAL;
>  	}
> 

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Andy Shevchenko]

On Mon, Mar 16, 2026 at 06:31:17PM +0000, Jonathan Cameron wrote:
> On Sat, 14 Mar 2026 18:13:32 -0500
> David Lechner <dlechner@baylibre.com> wrote:

...

> >  	u8 shift;
> > +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
> On this. There is new generic infrastructure for marking these.
> https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
> https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/
> 
> Would look like
> 	__dma_from_device_group_begin();
> 	u8 buf[3];
> 	__dma_from_device_group_end();
> 
> Do you think we should adopt them rather than doing our own thing?
> Slowly though I don't want the noise of a mass conversion.
> 
> As normal, advantage of standard infrastructure is cutting down
> in subsystem specific magic.
> 
> I 'think' result is the same (though it also forces the trailing padding if anything
> comes after this and needs it).

As I read it it will be an equivalent to

	u8 shift; __aligned(IIO_DMA_MINALIGN);
	u8 buf[3] __aligned(IIO_DMA_MINALIGN);


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[David Lechner]

On 3/16/26 1:31 PM, Jonathan Cameron wrote:
> On Sat, 14 Mar 2026 18:13:32 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> Add a DMA-safe buffer and use it for spi_read() instead of a stack
>> memory. All SPI buffers must be DMA-safe.
>>

...

>> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
>> index 1d427548e0b3..6416d6a7ada0 100644
>> --- a/drivers/iio/adc/ti-adc161s626.c
>> +++ b/drivers/iio/adc/ti-adc161s626.c
>> @@ -70,6 +70,7 @@ struct ti_adc_data {
>>  
>>  	u8 read_size;
>>  	u8 shift;
>> +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
> On this. There is new generic infrastructure for marking these.
> https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
> https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/
> 
> Would look like
> 	__dma_from_device_group_begin();
> 	u8 buf[3];
> 	__dma_from_device_group_end();
> 
> Do you think we should adopt them rather than doing our own thing?

I have mixed thoughts on this.

Pros:
* This would make it more obvious it should be at the end of the struct
  but doesn't hurt if it isn't.
Cons:
* It is more verbose.
* There doesn't seem to be __dma_to_device_group_begin(), so it isn't
  clear what we should do for tx buffers.

> Slowly though I don't want the noise of a mass conversion.
> 
> As normal, advantage of standard infrastructure is cutting down
> in subsystem specific magic.
> 
> I 'think' result is the same (though it also forces the trailing padding if anything
> comes after this and needs it).

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[David Lechner]

On 3/16/26 2:53 PM, Andy Shevchenko wrote:
> On Mon, Mar 16, 2026 at 06:31:17PM +0000, Jonathan Cameron wrote:
>> On Sat, 14 Mar 2026 18:13:32 -0500
>> David Lechner <dlechner@baylibre.com> wrote:
> 
> ...
> 
>>>  	u8 shift;
>>> +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
>> On this. There is new generic infrastructure for marking these.
>> https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
>> https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/
>>
>> Would look like
>> 	__dma_from_device_group_begin();
>> 	u8 buf[3];
>> 	__dma_from_device_group_end();
>>
>> Do you think we should adopt them rather than doing our own thing?
>> Slowly though I don't want the noise of a mass conversion.
>>
>> As normal, advantage of standard infrastructure is cutting down
>> in subsystem specific magic.
>>
>> I 'think' result is the same (though it also forces the trailing padding if anything
>> comes after this and needs it).
> 
> As I read it it will be an equivalent to
> 
> 	u8 shift; __aligned(IIO_DMA_MINALIGN);
> 	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
> 
> 

It will be:

	u8 shift;
	__u8 __cacheline_group_begin__[0] __aligned(ARCH_DMA_MINALIGN);
	u8 buf[3];
	__u8 __cacheline_group_end__[0] __aligned(ARCH_DMA_MINALIGN);

Note that ARCH_DMA_MINALIGN is not always the same as IIO_DMA_MINALIGN.
IIO_DMA_MINALIGN has a minimum of 8 bytes to account for timestamp
alignment.

I wonder if this would add an extra ARCH_DMA_MINALIGN bytes to
the struct. Or if the compiler is smart enough to see that it has
0 size on the last array and have a special case for that.

And even if the 0 is handled, if someone added a new field after this,
I expect the struct would grow by ARCH_DMA_MINALIGN rather than sizeof(field)
bytes.

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Jonathan Cameron]

On Mon, 16 Mar 2026 21:53:30 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Mar 16, 2026 at 06:31:17PM +0000, Jonathan Cameron wrote:
> > On Sat, 14 Mar 2026 18:13:32 -0500
> > David Lechner <dlechner@baylibre.com> wrote:  
> 
> ...
> 
> > >  	u8 shift;
> > > +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);  
> > On this. There is new generic infrastructure for marking these.
> > https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
> > https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/
> > 
> > Would look like
> > 	__dma_from_device_group_begin();
> > 	u8 buf[3];
> > 	__dma_from_device_group_end();
> > 
> > Do you think we should adopt them rather than doing our own thing?
> > Slowly though I don't want the noise of a mass conversion.
> > 
> > As normal, advantage of standard infrastructure is cutting down
> > in subsystem specific magic.
> > 
> > I 'think' result is the same (though it also forces the trailing padding if anything
> > comes after this and needs it).  
> 
> As I read it it will be an equivalent to
> 
> 	u8 shift; __aligned(IIO_DMA_MINALIGN);
> 	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
I think it ends up as
	u8 shift; //unmoved
	__u8 something[0] __aligned(IIO_DMA_MINALIGN);
	u8 buf[3];
	__u8 something_else[0] __aligned(IIO_DMA_MINALING);

Who doesn't like forcing alignment on 0 sized things.

I assume someone who is more familiar with the C spec than I am
established that works.


> 
> 

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Jonathan Cameron]

On Sat, 21 Mar 2026 14:27:10 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 3/16/26 1:31 PM, Jonathan Cameron wrote:
> > On Sat, 14 Mar 2026 18:13:32 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> >> Add a DMA-safe buffer and use it for spi_read() instead of a stack
> >> memory. All SPI buffers must be DMA-safe.
> >>  
> 
> ...
> 
> >> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
> >> index 1d427548e0b3..6416d6a7ada0 100644
> >> --- a/drivers/iio/adc/ti-adc161s626.c
> >> +++ b/drivers/iio/adc/ti-adc161s626.c
> >> @@ -70,6 +70,7 @@ struct ti_adc_data {
> >>  
> >>  	u8 read_size;
> >>  	u8 shift;
> >> +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);  
> > On this. There is new generic infrastructure for marking these.
> > https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
> > https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/
> > 
> > Would look like
> > 	__dma_from_device_group_begin();
> > 	u8 buf[3];
> > 	__dma_from_device_group_end();
> > 
> > Do you think we should adopt them rather than doing our own thing?  
> 
> I have mixed thoughts on this.
> 
> Pros:
> * This would make it more obvious it should be at the end of the struct
>   but doesn't hurt if it isn't.
> Cons:
> * It is more verbose.
> * There doesn't seem to be __dma_to_device_group_begin(), so it isn't
>   clear what we should do for tx buffers.

Agreed. Should add that.  Though it would then imply that we should
treat them differently. Hopefully my understanding that we don't need
tx and rx to live in their own cachelines isn't wrong (at least
for all the stuff we care about)!

Let's let this sit for a cycle or two and see how adoption of those
new toys goes.  Longer term I'd generally like to get rid of as 
much that makes us special as possible.

> 
> > Slowly though I don't want the noise of a mass conversion.
> > 
> > As normal, advantage of standard infrastructure is cutting down
> > in subsystem specific magic.
> > 
> > I 'think' result is the same (though it also forces the trailing padding if anything
> > comes after this and needs it).  
> 

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

[Jonathan Cameron]

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);  
> 

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Jonathan Cameron]

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

> On Sat, Mar 14, 2026 at 06:13:32PM -0500, David Lechner wrote:
> > Add a DMA-safe buffer and use it for spi_read() instead of a stack
> > memory. All SPI buffers must be DMA-safe.
> > 
> > Since we only need up to 3 bytes, we just use a u8[] instead of __be16
> > and __be32 and change the conversion functions appropriately.  
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 

Applied and marked for stable with the following as amazingly enough I was
running a config where it didn't have this via another path so the build
failed.

Thanks,

J

diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index 833a636a29bb..be1cc2e77862 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/spi/spi.h>
+#include <linux/unaligned.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/buffer.h>

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Jonathan Cameron]

On Sat, 21 Mar 2026 14:40:35 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 3/16/26 2:53 PM, Andy Shevchenko wrote:
> > On Mon, Mar 16, 2026 at 06:31:17PM +0000, Jonathan Cameron wrote:  
> >> On Sat, 14 Mar 2026 18:13:32 -0500
> >> David Lechner <dlechner@baylibre.com> wrote:  
> > 
> > ...
> >   
> >>>  	u8 shift;
> >>> +	u8 buf[3] __aligned(IIO_DMA_MINALIGN);  
> >> On this. There is new generic infrastructure for marking these.
> >> https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/dma-mapping.h#L720
> >> https://lore.kernel.org/all/01ea88055ded4d70cac70ba557680fd5fa7d9ff5.1767601130.git.mst@redhat.com/
> >>
> >> Would look like
> >> 	__dma_from_device_group_begin();
> >> 	u8 buf[3];
> >> 	__dma_from_device_group_end();
> >>
> >> Do you think we should adopt them rather than doing our own thing?
> >> Slowly though I don't want the noise of a mass conversion.
> >>
> >> As normal, advantage of standard infrastructure is cutting down
> >> in subsystem specific magic.
> >>
> >> I 'think' result is the same (though it also forces the trailing padding if anything
> >> comes after this and needs it).  
> > 
> > As I read it it will be an equivalent to
> > 
> > 	u8 shift; __aligned(IIO_DMA_MINALIGN);
> > 	u8 buf[3] __aligned(IIO_DMA_MINALIGN);
> > 
> >   
> 
> It will be:
> 
> 	u8 shift;
> 	__u8 __cacheline_group_begin__[0] __aligned(ARCH_DMA_MINALIGN);
> 	u8 buf[3];
> 	__u8 __cacheline_group_end__[0] __aligned(ARCH_DMA_MINALIGN);
> 
> Note that ARCH_DMA_MINALIGN is not always the same as IIO_DMA_MINALIGN.
> IIO_DMA_MINALIGN has a minimum of 8 bytes to account for timestamp
> alignment.

Good point. All the sensible arches have min 8 anyway but who knows..

> 
> I wonder if this would add an extra ARCH_DMA_MINALIGN bytes to
> the struct. Or if the compiler is smart enough to see that it has
> 0 size on the last array and have a special case for that.
> 
My gut feeling is that it will be fine.  If you have a [0] element in
a flex array (the old way of doing it that recently got ripped out of
the kernel) then the sizeof() the structure never included anything for
that.  I don't see why aligning it should matter.

> And even if the 0 is handled, if someone added a new field after this,
> I expect the struct would grow by ARCH_DMA_MINALIGN rather than sizeof(field)
> bytes.

Yes, the structure always has to be a multiple of the item with
the largest alignment so anything after that forcing align will bloat
by whole ARCH_DMA_MINALIGN.


> 

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[kernel test robot]

Hi David,

kernel test robot noticed the following build errors:

[auto build test ERROR on ff0843ceb1fb11a6b73e0e77b932ef7967aecd4b]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Lechner/iio-adc-ti-adc161s626-fix-buffer-read-on-big-endian/20260315-092711
base:   ff0843ceb1fb11a6b73e0e77b932ef7967aecd4b
patch link:    https://lore.kernel.org/r/20260314-iio-adc-ti-adc161s626-fix-scan-buf-v1-2-56243b11e87b%40baylibre.com
patch subject: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20260327/202603271734.pnWuSmNr-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260327/202603271734.pnWuSmNr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603271734.pnWuSmNr-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iio/adc/ti-adc161s626.c: In function 'ti_adc_read_measurement':
>> drivers/iio/adc/ti-adc161s626.c:87:24: error: implicit declaration of function 'get_unaligned_be16' [-Wimplicit-function-declaration]
      87 |                 *val = get_unaligned_be16(data->buf);
         |                        ^~~~~~~~~~~~~~~~~~
>> drivers/iio/adc/ti-adc161s626.c:94:24: error: implicit declaration of function 'get_unaligned_be24' [-Wimplicit-function-declaration]
      94 |                 *val = get_unaligned_be24(data->buf);
         |                        ^~~~~~~~~~~~~~~~~~


vim +/get_unaligned_be16 +87 drivers/iio/adc/ti-adc161s626.c

    75	
    76	static int ti_adc_read_measurement(struct ti_adc_data *data,
    77					   struct iio_chan_spec const *chan, int *val)
    78	{
    79		int ret;
    80	
    81		switch (data->read_size) {
    82		case 2:
    83			ret = spi_read(data->spi, data->buf, 2);
    84			if (ret)
    85				return ret;
    86	
  > 87			*val = get_unaligned_be16(data->buf);
    88			break;
    89		case 3:
    90			ret = spi_read(data->spi, data->buf, 3);
    91			if (ret)
    92				return ret;
    93	
  > 94			*val = get_unaligned_be24(data->buf);
    95			break;
    96		default:
    97			return -EINVAL;
    98		}
    99	
   100		*val = sign_extend32(*val >> data->shift, chan->scan_type.realbits - 1);
   101	
   102		return 0;
   103	}
   104	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 05:13:16PM +0800, kernel test robot wrote:
> Hi David,
> 
> kernel test robot noticed the following build errors:


> All errors (new ones prefixed by >>):
> 
>    drivers/iio/adc/ti-adc161s626.c: In function 'ti_adc_read_measurement':
> >> drivers/iio/adc/ti-adc161s626.c:87:24: error: implicit declaration of function 'get_unaligned_be16' [-Wimplicit-function-declaration]
>       87 |                 *val = get_unaligned_be16(data->buf);
>          |                        ^~~~~~~~~~~~~~~~~~
> >> drivers/iio/adc/ti-adc161s626.c:94:24: error: implicit declaration of function 'get_unaligned_be24' [-Wimplicit-function-declaration]
>       94 |                 *val = get_unaligned_be24(data->buf);
>          |                        ^~~~~~~~~~~~~~~~~~

+ linux/unaligned.h should fix this.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: adc: ti-adc161s626: use DMA-safe memory for spi_read()

[David Lechner]

On 3/27/26 4:44 AM, Andy Shevchenko wrote:
> On Fri, Mar 27, 2026 at 05:13:16PM +0800, kernel test robot wrote:
>> Hi David,
>>
>> kernel test robot noticed the following build errors:
> 
> 
>> All errors (new ones prefixed by >>):
>>
>>    drivers/iio/adc/ti-adc161s626.c: In function 'ti_adc_read_measurement':
>>>> drivers/iio/adc/ti-adc161s626.c:87:24: error: implicit declaration of function 'get_unaligned_be16' [-Wimplicit-function-declaration]
>>       87 |                 *val = get_unaligned_be16(data->buf);
>>          |                        ^~~~~~~~~~~~~~~~~~
>>>> drivers/iio/adc/ti-adc161s626.c:94:24: error: implicit declaration of function 'get_unaligned_be24' [-Wimplicit-function-declaration]
>>       94 |                 *val = get_unaligned_be24(data->buf);
>>          |                        ^~~~~~~~~~~~~~~~~~
> 
> + linux/unaligned.h should fix this.
> 

Jonathan caught this already [1], so should be fixed already [2].

[1]: https://lore.kernel.org/linux-iio/20260321204506.4b5df02a@jic23-huawei/
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=fixes-togreg&id=768461517a28d80fe81ea4d5d03a90cd184ea6ad