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

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 01 Mar 2026 14:24:52 -0600
David Lechner <dlechner@baylibre.com> 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.
Why can't we just stash the start of the timestamp? It's computed
a couple of lines up (or at least it's supposed to be computed there!)

	if (timestamp) {
		length = iio_storage_bytes_for_timestamp(indio_dev);
		if (length < 0)
			return length;

		bytes = ALIGN(bytes, length);

Or is that wrong currently?  If it is we need to fix that logic.

J
> 
> 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;
> +
>  	return 0;
>  }
>  
> @@ -848,7 +852,7 @@ static int iio_buffer_update_bytes_per_datum(struct iio_dev *indio_dev,
>  		return 0;
>  
>  	ret = iio_compute_scan_bytes(indio_dev, buffer->scan_mask,
> -				     buffer->scan_timestamp, &bytes);
> +				     buffer->scan_timestamp, &bytes, NULL);
>  	if (ret)
>  		return ret;
>  
> @@ -892,6 +896,7 @@ struct iio_device_config {
>  	unsigned int watermark;
>  	const unsigned long *scan_mask;
>  	unsigned int scan_bytes;
> +	unsigned int largest_scan_element_size;
>  	bool scan_timestamp;
>  };
>  
> @@ -997,7 +1002,8 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>  	}
>  
>  	ret = iio_compute_scan_bytes(indio_dev, scan_mask, scan_timestamp,
> -				     &config->scan_bytes);
> +				     &config->scan_bytes,
> +				     &config->largest_scan_element_size);
>  	if (ret)
>  		return ret;
>  
> @@ -1155,6 +1161,8 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
>  	indio_dev->active_scan_mask = config->scan_mask;
>  	ACCESS_PRIVATE(indio_dev, scan_timestamp) = config->scan_timestamp;
>  	indio_dev->scan_bytes = config->scan_bytes;
> +	ACCESS_PRIVATE(indio_dev, largest_scan_element_size) =
> +		config->largest_scan_element_size;
>  	iio_dev_opaque->currentmode = config->mode;
>  
>  	iio_update_demux(indio_dev);
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a9ecff191bd9..85bcb5f8ae15 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -584,6 +584,8 @@ struct iio_buffer_setup_ops {
>   *			and owner
>   * @buffer:		[DRIVER] any buffer present
>   * @scan_bytes:		[INTERN] num bytes captured to be fed to buffer demux
> + * @largest_scan_element_size: [INTERN] cache of the largest scan element size
> + *			       among the channels selected in the scan mask
>   * @available_scan_masks: [DRIVER] optional array of allowed bitmasks. Sort the
>   *			   array in order of preference, the most preferred
>   *			   masks first.
> @@ -610,6 +612,7 @@ struct iio_dev {
>  
>  	struct iio_buffer		*buffer;
>  	int				scan_bytes;
> +	unsigned int			__private largest_scan_element_size;
>  
>  	const unsigned long		*available_scan_masks;
>  	unsigned int			__private masklength;
>