Re: [PATCH 3/4] iio: buffer: cache largest scan element size

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, 2026-03-02 at 09:35 -0600, David Lechner wrote:
> On 3/2/26 6:16 AM, Nuno Sá wrote:
> > On Sun, 2026-03-01 at 14:24 -0600, David Lechner wrote:
> > > Cache the largest scan element size of elements enabled in a scan
> > > buffer. This will be used later to ensure proper alignment of the
> > > timestamp element in the scan buffer.
> > > 
> > > The new field could not be placed in struct iio_dev_opaque because we
> > > will need to access it in a static inline function later, so we make it
> > > __private instead. It is only intended to be used by core IIO code.
> > > 
> > > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > > ---
> > >  drivers/iio/industrialio-buffer.c | 14 +++++++++++---
> > >  include/linux/iio/iio.h           |  3 +++
> > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> > > index 71dfc81cb9e5..83e9392f949f 100644
> > > --- a/drivers/iio/industrialio-buffer.c
> > > +++ b/drivers/iio/industrialio-buffer.c
> > > @@ -765,7 +765,8 @@ static int iio_storage_bytes_for_timestamp(struct iio_dev *indio_dev)
> > >  
> > >  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
> > >  				  const unsigned long *mask, bool timestamp,
> > > -				  unsigned int *scan_bytes)
> > > +				  unsigned int *scan_bytes,
> > > +				  unsigned int *largest_element_size)
> > >  {
> > >  	unsigned int bytes = 0;
> > >  	int length, i, largest = 0;
> > > @@ -793,6 +794,9 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
> > >  
> > >  	*scan_bytes = ALIGN(bytes, largest);
> > >  
> > > +	if (largest_element_size)
> > > +		*largest_element_size = largest;
> > 
> > I might be missing something but it seems we now have two paths:
> > 
> > 1. Go with 32 bytes
> > 2. Go with 24 bytes (natural alignment)
> 
> Hmm... You are right, but we are safe for now because the only repeat is
> a quaternion, which has repeat of 4, so we don't have a case with e.g.
> 24 bytes right now.
> 
> I can do a follow-up to future-proof iio_storage_bytes_for_si() to
> handle repeat = 3 (or any other less likely value).
> 
> Without the repeat, the size is based on .storagebits, and .storagebits /
> BITS_PER_BYTE is always a power of 2, so no problem there.
> 
> > 
> > ABI was not clear so I'm not sure if we do want to enforce/treat repeated values as one single
> > element
> 
> It has always been this way and we had some recent breakage because I didn't
> know about this. This patch series is a direct result of that [1] because we
> identified more bugs when analyzing it.

Hmm true. If we shrink the buffer, userspace expecting timestamps at bit 24 will not work.


- Nuno Sá

> 
> [1]: https://lore.kernel.org/linux-iio/bug-221077-217253@https.bugzilla.kernel.org%2F/
> 
> > If so, nothing to change. But if not, we could re-think the approach and save some bytes. 
> 
> It's too late and breaks usespace as we have seen.
> 
> > Marginal savings though so If having the smaller buffer is not straight enough I would be ok
> > with
> > the simplicity tradeoff.
> > 
> > - Nuno Sá
> >