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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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@xxxxxxxxxx/
> >>
> >> 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.


>