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

[David Lechner]

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

[1]: https://lore.kernel.org/linux-iio/bug-221077-217253@xxxxxxxxxxxxxxxxxxxxxxxxx%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á
>