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..f523b046c627037c449170b7490ce2a351c6b9c0 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))
+
 /*
  * 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

-- 
2.43.0

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

On Fri, Apr 18, 2025 at 05:58:32PM -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.

...

> +#define IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
> +	type name[ALIGN((count), sizeof(s64) / sizeof(type)) \
> +		  + sizeof(s64)/ sizeof(type)] __aligned(sizeof(s64))

We have two alignments in a row in most of the cases, I would think that the
proper one is for DMA and this should not be used at all, it actually might be
a bug in bmp280.

-- 
With Best Regards,
Andy Shevchenko

On Fri, Apr 18, 2025 at 05:58:32PM -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.

...

> +/**
> + * 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))

Missing space and I would rather to see [...] on the same line independently on
the size as it will give better impression on what's going on here.

-- 
With Best Regards,
Andy Shevchenko

On 4/19/25 11:33 AM, Andy Shevchenko wrote:
> On Fri, Apr 18, 2025 at 05:58:32PM -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.
> 
> ...
> 
>> +/**
>> + * 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))
> 
> Missing space

Sorry, but my eyes can't find any missing space. Can you be more specific?

> and I would rather to see [...] on the same line independently on> the size as it will give better impression on what's going on here.
> 

As long as Jonathan doesn't mind the long line. :-)

Mon, Apr 21, 2025 at 05:40:41PM -0500, David Lechner kirjoitti:
> On 4/19/25 11:33 AM, Andy Shevchenko wrote:
> > On Fri, Apr 18, 2025 at 05:58:32PM -0500, David Lechner wrote:

...

> >> +/**
> >> + * 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))
> > 
> > Missing space
> 
> Sorry, but my eyes can't find any missing space. Can you be more specific?

As far as I can see it's missed after sizeof(s64)
Also I don't like to see the leading operators (like +, -, *, &).

> > and I would rather to see [...] on the same line independently on> the size as it will give better impression on what's going on here.
> 
> As long as Jonathan doesn't mind the long line. :-)

-- 
With Best Regards,
Andy Shevchenko

On 4/22/25 1:36 AM, Andy Shevchenko wrote:
> Mon, Apr 21, 2025 at 05:40:41PM -0500, David Lechner kirjoitti:
>> On 4/19/25 11:33 AM, Andy Shevchenko wrote:
>>> On Fri, Apr 18, 2025 at 05:58:32PM -0500, David Lechner wrote:
> 
> ...
> 
>>>> +/**
>>>> + * 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))
>>>
>>> Missing space
>>
>> Sorry, but my eyes can't find any missing space. Can you be more specific?
> 
> As far as I can see it's missed after sizeof(s64)
> Also I don't like to see the leading operators (like +, -, *, &).

:facepalm: yes, I see it now.

> 
>>> and I would rather to see [...] on the same line independently on> the size as it will give better impression on what's going on here.
>>
>> As long as Jonathan doesn't mind the long line. :-)
>