Re: [PATCH 1/4] iio: introduce IIO_DECLARE_BUFFER_WITH_TS

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

On Fri, 2025-04-18 at 17:58 -0500, David Lechner wrote:
> Add a new macro to help with the common case of declaring a buffer that
> is safe to use with iio_push_to_buffers_with_ts(). This is not trivial
> to do correctly because of the alignment requirements of the timestamp.
> This will make it easier for both authors and reviewers.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  include/linux/iio/iio.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index
> 638cf2420fbd85cf2924d09d061df601d1d4bb2a..f523b046c627037c449170b7490ce2a351c6
> b9c0 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -7,6 +7,7 @@
>  #ifndef _INDUSTRIAL_IO_H_
>  #define _INDUSTRIAL_IO_H_
>  
> +#include <linux/align.h>
>  #include <linux/device.h>
>  #include <linux/cdev.h>
>  #include <linux/compiler_types.h>
> @@ -770,6 +771,21 @@ static inline void *iio_device_get_drvdata(const struct
> iio_dev *indio_dev)
>  	return dev_get_drvdata(&indio_dev->dev);
>  }
>  
> +/**
> + * IIO_DECLARE_BUFFER_WITH_TS() - Declare a buffer with timestamp
> + * @type: element type of the buffer
> + * @name: identifier name of the buffer
> + * @count: number of elements in the buffer
> + *
> + * Declares a buffer that is safe to use with iio_push_to_buffer_with_ts().
> In
> + * addition to allocating enough space for @count elements of @type, it also
> + * allocates space for a s64 timestamp at the end of the buffer and ensures
> + * proper alignment of the timestamp.
> + */
> +#define IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
> +	type name[ALIGN((count), sizeof(s64) / sizeof(type)) \
> +		  + sizeof(s64)/ sizeof(type)] __aligned(sizeof(s64))
> +

I tend to agree with Andy on the two alignments thing. Independent of that I
guess this is also not intended for stack allocations? If so, we should maybe be
clear about that in the docs... If we do intend using stack allocations we could
improve it by making sure the build fails if "count" is not a constant
expression (likely it will already but I think we should make the intent clear).

- Nuno Sá

>  /*
>   * Used to ensure the iio_priv() structure is aligned to allow that structure
>   * to in turn include IIO_DMA_MINALIGN'd elements such as buffers which