Add new macros 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.
To avoid double __align() attributes in cases where we also need DMA
alignment, add a 2nd variant IIO_DECLARE_DMA_BUFFER_WITH_TS.
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
include/linux/iio/iio.h | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 638cf2420fbd85cf2924d09d061df601d1d4bb2a..4dd811e3530e228a6fadbd80cfb2f5068c3d6a9a 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>
@@ -777,6 +778,41 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev)
* them safe for use with non-coherent DMA.
*/
#define IIO_DMA_MINALIGN ARCH_DMA_MINALIGN
+
+#define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
+ type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / sizeof(type)]
+
+/**
+ * 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) \
+ _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(sizeof(s64))
+
+/**
+ * IIO_DECLARE_DMA_BUFFER_WITH_TS() - Declare a DMA-aligned buffer with timestamp
+ * @type: element type of the buffer
+ * @name: identifier name of the buffer
+ * @count: number of elements in the buffer
+ *
+ * Same as IIO_DECLARE_BUFFER_WITH_TS(), but is uses __aligned(IIO_DMA_MINALIGN)
+ * to ensure that the buffer doesn't share cachelines with anything that comes
+ * before it in a struct. This should not be used for stack-allocated buffers
+ * as stack memory cannot generally be used for DMA.
+ */
+#define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \
+ _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(IIO_DMA_MINALIGN)
+
+_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
+ "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment");
+
struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv);
/* The information at the returned address is guaranteed to be cacheline aligned */
--
2.43.0Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on aff301f37e220970c2f301b5c65a8bfedf52058e]
url: https://github.com/intel-lab-lkp/linux/commits/David-Lechner/iio-introduce-IIO_DECLARE_BUFFER_WITH_TS-macros/20250423-061049
base: aff301f37e220970c2f301b5c65a8bfedf52058e
patch link: https://lore.kernel.org/r/20250422-iio-introduce-iio_declare_buffer_with_ts-v2-1-3fd36475c706%40baylibre.com
patch subject: [PATCH v2 1/6] iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros
config: riscv-randconfig-002-20250424 (https://download.01.org/0day-ci/archive/20250424/202504240137.4mJaj0GN-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250424/202504240137.4mJaj0GN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504240137.4mJaj0GN-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:804:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
804 | insb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:104:53: note: expanded from macro 'insb'
104 | #define insb(addr, buffer, count) __insb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:812:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
812 | insw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
105 | #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:820:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
820 | insl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:829:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
829 | outsb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:838:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
838 | outsw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:847:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
847 | outsl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
1175 | return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-buffer.c:29:
>> include/linux/iio/iio.h:813:16: error: static assertion failed due to requirement 'sizeof (__alignof(unsigned long long)) % sizeof(long long) == 0': macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/iio/iio.h:813:55: note: expression evaluates to '4 == 0'
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
7 warnings and 1 error generated.
--
In file included from drivers/iio/industrialio-sw-device.c:14:
In file included from include/linux/iio/sw_device.h:13:
>> include/linux/iio/iio.h:813:16: error: static assertion failed due to requirement 'sizeof (__alignof(unsigned long long)) % sizeof(long long) == 0': macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/iio/iio.h:813:55: note: expression evaluates to '4 == 0'
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
1 error generated.
--
In file included from drivers/iio/industrialio-triggered-event.c:9:
>> include/linux/iio/iio.h:813:16: error: static assertion failed due to requirement 'sizeof (__alignof(unsigned long long)) % sizeof(long long) == 0': macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/iio/iio.h:813:55: note: expression evaluates to '4 == 0'
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:804:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
804 | insb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:104:53: note: expanded from macro 'insb'
104 | #define insb(addr, buffer, count) __insb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:812:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
812 | insw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
105 | #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:820:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
820 | insl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:829:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
829 | outsb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:838:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
838 | outsw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:847:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
847 | outsl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/iio/industrialio-triggered-event.c:10:
In file included from include/linux/iio/triggered_event.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
vim +813 include/linux/iio/iio.h
781
782 #define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
783 type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / sizeof(type)]
784
785 /**
786 * IIO_DECLARE_BUFFER_WITH_TS() - Declare a buffer with timestamp
787 * @type: element type of the buffer
788 * @name: identifier name of the buffer
789 * @count: number of elements in the buffer
790 *
791 * Declares a buffer that is safe to use with iio_push_to_buffer_with_ts(). In
792 * addition to allocating enough space for @count elements of @type, it also
793 * allocates space for a s64 timestamp at the end of the buffer and ensures
794 * proper alignment of the timestamp.
795 */
796 #define IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
797 _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(sizeof(s64))
798
799 /**
800 * IIO_DECLARE_DMA_BUFFER_WITH_TS() - Declare a DMA-aligned buffer with timestamp
801 * @type: element type of the buffer
802 * @name: identifier name of the buffer
803 * @count: number of elements in the buffer
804 *
805 * Same as IIO_DECLARE_BUFFER_WITH_TS(), but is uses __aligned(IIO_DMA_MINALIGN)
806 * to ensure that the buffer doesn't share cachelines with anything that comes
807 * before it in a struct. This should not be used for stack-allocated buffers
808 * as stack memory cannot generally be used for DMA.
809 */
810 #define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \
811 _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(IIO_DMA_MINALIGN)
812
> 813 _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
814 "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment");
815
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on aff301f37e220970c2f301b5c65a8bfedf52058e]
url: https://github.com/intel-lab-lkp/linux/commits/David-Lechner/iio-introduce-IIO_DECLARE_BUFFER_WITH_TS-macros/20250423-061049
base: aff301f37e220970c2f301b5c65a8bfedf52058e
patch link: https://lore.kernel.org/r/20250422-iio-introduce-iio_declare_buffer_with_ts-v2-1-3fd36475c706%40baylibre.com
patch subject: [PATCH v2 1/6] iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros
config: sh-randconfig-001-20250424 (https://download.01.org/0day-ci/archive/20250424/202504240112.hZy9LpvD-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250424/202504240112.hZy9LpvD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504240112.hZy9LpvD-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/iio/industrialio-buffer.c:29:
>> include/linux/iio/iio.h:813:1: error: static assertion failed: "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment"
813 | _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
| ^~~~~~~~~~~~~~
vim +813 include/linux/iio/iio.h
781
782 #define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
783 type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / sizeof(type)]
784
785 /**
786 * IIO_DECLARE_BUFFER_WITH_TS() - Declare a buffer with timestamp
787 * @type: element type of the buffer
788 * @name: identifier name of the buffer
789 * @count: number of elements in the buffer
790 *
791 * Declares a buffer that is safe to use with iio_push_to_buffer_with_ts(). In
792 * addition to allocating enough space for @count elements of @type, it also
793 * allocates space for a s64 timestamp at the end of the buffer and ensures
794 * proper alignment of the timestamp.
795 */
796 #define IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
797 _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(sizeof(s64))
798
799 /**
800 * IIO_DECLARE_DMA_BUFFER_WITH_TS() - Declare a DMA-aligned buffer with timestamp
801 * @type: element type of the buffer
802 * @name: identifier name of the buffer
803 * @count: number of elements in the buffer
804 *
805 * Same as IIO_DECLARE_BUFFER_WITH_TS(), but is uses __aligned(IIO_DMA_MINALIGN)
806 * to ensure that the buffer doesn't share cachelines with anything that comes
807 * before it in a struct. This should not be used for stack-allocated buffers
808 * as stack memory cannot generally be used for DMA.
809 */
810 #define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \
811 _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(IIO_DMA_MINALIGN)
812
> 813 _Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0,
814 "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment");
815
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi David, Nice patch, I really think these will be very helpful... Just one comment bellow On Tue, 2025-04-22 at 17:07 -0500, David Lechner wrote: > Add new macros 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. > > To avoid double __align() attributes in cases where we also need DMA > alignment, add a 2nd variant IIO_DECLARE_DMA_BUFFER_WITH_TS. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > include/linux/iio/iio.h | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > index > 638cf2420fbd85cf2924d09d061df601d1d4bb2a..4dd811e3530e228a6fadbd80cfb2f5068c3d > 6a9a 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> > @@ -777,6 +778,41 @@ static inline void *iio_device_get_drvdata(const struct > iio_dev *indio_dev) > * them safe for use with non-coherent DMA. > */ > #define IIO_DMA_MINALIGN ARCH_DMA_MINALIGN > + > +#define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \ > + type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / > sizeof(type)] > + > +/** > + * 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) \ > + _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(sizeof(s64)) > + > +/** > + * IIO_DECLARE_DMA_BUFFER_WITH_TS() - Declare a DMA-aligned buffer with > timestamp > + * @type: element type of the buffer > + * @name: identifier name of the buffer > + * @count: number of elements in the buffer > + * > + * Same as IIO_DECLARE_BUFFER_WITH_TS(), but is uses > __aligned(IIO_DMA_MINALIGN) > + * to ensure that the buffer doesn't share cachelines with anything that > comes > + * before it in a struct. This should not be used for stack-allocated buffers > + * as stack memory cannot generally be used for DMA. > + */ > +#define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \ > + _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) > __aligned(IIO_DMA_MINALIGN) > + > +_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0, > + "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp > alignment"); > I wonder about the usefulness of the above assert... AFAICT, the default alignment is 8 bytes and I could not find any arch defining ARCH_DMA_MINALIGN smaller than that (would be very odd to have a cacheline smaller than that these days). For bigger values, nowadays they are all power of 2 and I would be surprised otherwise. But the more important question to me is what if the above assert fails? Will we not allow IIO or some drivers to be used in that architecture? It can become a very "painful" situation (assuming these macros get widely used). So, IMHO, either we assume the above can happen and rework the macros to make it work for that hypotetical case or we assume the above is always true and drop the assert. TBH, I think it would be a fair assumption... On top of that the assertion is wrong: sizeof(IIO_DMA_MINALIGN) != IIO_DMA_MINALIGN :) On the other hand, as I mentioned in V1, I think that an assertion or BUILD_BUG_ON_MSG for making sure 'count' is a compile time constant expression would be helpful. Sure, we'll get -Wvla but some developers might still ignore the warning and send patches with these arrays. So, it would be neater if we fail to build and force them to fix their code. - Nuno Sá
On 4/23/25 4:18 AM, Nuno Sá wrote: > Hi David, > > Nice patch, I really think these will be very helpful... Just one comment bellow > > On Tue, 2025-04-22 at 17:07 -0500, David Lechner wrote: >> Add new macros 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. >> >> To avoid double __align() attributes in cases where we also need DMA >> alignment, add a 2nd variant IIO_DECLARE_DMA_BUFFER_WITH_TS. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> >> --- >> include/linux/iio/iio.h | 36 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h >> index >> 638cf2420fbd85cf2924d09d061df601d1d4bb2a..4dd811e3530e228a6fadbd80cfb2f5068c3d >> 6a9a 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> >> @@ -777,6 +778,41 @@ static inline void *iio_device_get_drvdata(const struct >> iio_dev *indio_dev) >> * them safe for use with non-coherent DMA. >> */ >> #define IIO_DMA_MINALIGN ARCH_DMA_MINALIGN >> + >> +#define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \ >> + type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / >> sizeof(type)] >> + >> +/** >> + * 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) \ >> + _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(sizeof(s64)) >> + >> +/** >> + * IIO_DECLARE_DMA_BUFFER_WITH_TS() - Declare a DMA-aligned buffer with >> timestamp >> + * @type: element type of the buffer >> + * @name: identifier name of the buffer >> + * @count: number of elements in the buffer >> + * >> + * Same as IIO_DECLARE_BUFFER_WITH_TS(), but is uses >> __aligned(IIO_DMA_MINALIGN) >> + * to ensure that the buffer doesn't share cachelines with anything that >> comes >> + * before it in a struct. This should not be used for stack-allocated buffers >> + * as stack memory cannot generally be used for DMA. >> + */ >> +#define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \ >> + _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) >> __aligned(IIO_DMA_MINALIGN) >> + >> +_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0, >> + "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp >> alignment"); >> > > I wonder about the usefulness of the above assert... AFAICT, the default Jonathan seemed minorly concerned that a strange new architecture might have IIO_DMA_MINALIGN is < 8 some day, so I threw it in there. But agree, it seems highly unlikely to actually happen. > alignment is 8 bytes and I could not find any arch defining ARCH_DMA_MINALIGN > smaller than that (would be very odd to have a cacheline smaller than that these > days). For bigger values, nowadays they are all power of 2 and I would be > surprised otherwise. But the more important question to me is what if the above > assert fails? Will we not allow IIO or some drivers to be used in that > architecture? It can become a very "painful" situation (assuming these macros > get widely used). So, IMHO, either we assume the above can happen and rework the > macros to make it work for that hypotetical case or we assume the above is > always true and drop the assert. TBH, I think it would be a fair assumption... > > On top of that the assertion is wrong: > > sizeof(IIO_DMA_MINALIGN) != IIO_DMA_MINALIGN :) Doh! > > On the other hand, as I mentioned in V1, I think that an assertion or > BUILD_BUG_ON_MSG for making sure 'count' is a compile time constant expression > would be helpful. Sure, we'll get -Wvla but some developers might still ignore > the warning and send patches with these arrays. So, it would be neater if we > fail to build and force them to fix their code. BUILD_BUG_ON_MSG() won't work because it expands to a do/while loop which won't work in static struct declarations. But I can try to see if we can come up with something that works.
On Wed, 23 Apr 2025 09:51:25 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 4/23/25 4:18 AM, Nuno Sá wrote: > > Hi David, > > > > Nice patch, I really think these will be very helpful... Just one comment bellow > > > > On Tue, 2025-04-22 at 17:07 -0500, David Lechner wrote: > >> Add new macros 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. > >> > >> To avoid double __align() attributes in cases where we also need DMA > >> alignment, add a 2nd variant IIO_DECLARE_DMA_BUFFER_WITH_TS. > >> > >> Signed-off-by: David Lechner <dlechner@baylibre.com> > >> --- > >> include/linux/iio/iio.h | 36 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 36 insertions(+) > >> > >> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > >> index > >> 638cf2420fbd85cf2924d09d061df601d1d4bb2a..4dd811e3530e228a6fadbd80cfb2f5068c3d > >> 6a9a 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> > >> @@ -777,6 +778,41 @@ static inline void *iio_device_get_drvdata(const struct > >> iio_dev *indio_dev) > >> * them safe for use with non-coherent DMA. > >> */ > >> #define IIO_DMA_MINALIGN ARCH_DMA_MINALIGN > >> + > >> +#define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \ > >> + type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / > >> sizeof(type)] > >> + > >> +/** > >> + * 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) \ > >> + _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(sizeof(s64)) > >> + > >> +/** > >> + * IIO_DECLARE_DMA_BUFFER_WITH_TS() - Declare a DMA-aligned buffer with > >> timestamp > >> + * @type: element type of the buffer > >> + * @name: identifier name of the buffer > >> + * @count: number of elements in the buffer > >> + * > >> + * Same as IIO_DECLARE_BUFFER_WITH_TS(), but is uses > >> __aligned(IIO_DMA_MINALIGN) > >> + * to ensure that the buffer doesn't share cachelines with anything that > >> comes > >> + * before it in a struct. This should not be used for stack-allocated buffers > >> + * as stack memory cannot generally be used for DMA. > >> + */ > >> +#define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \ > >> + _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) > >> __aligned(IIO_DMA_MINALIGN) > >> + > >> +_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0, > >> + "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp > >> alignment"); > >> > > > > I wonder about the usefulness of the above assert... AFAICT, the default > > Jonathan seemed minorly concerned that a strange new architecture might have > IIO_DMA_MINALIGN is < 8 some day, so I threw it in there. But agree, it seems > highly unlikely to actually happen. Yeah, it's unlikely. Architectures using small sizes is not about cacheline length any more but rather than they guarantee that the system will work fine irrespective of the cacheline length. (e.g. x86_64 where the min align has been 8 for a long time - possibly always? and cachelines are generally 64 bytes) It seems very unlikely anyone will care about smaller than that so such a macro is really just paranoia! The ARCH_DMA_MINALIGN fallback is sizeof(unsigned long long). Basically I want the assert so I don't have to pay attention to weird new architectures. I'm not that fussed though if it is hard to do for some reason. Jonathan
On Wed, Apr 23, 2025 at 5:51 PM David Lechner <dlechner@baylibre.com> wrote: > On 4/23/25 4:18 AM, Nuno Sá wrote: > > On Tue, 2025-04-22 at 17:07 -0500, David Lechner wrote: ... > > On the other hand, as I mentioned in V1, I think that an assertion or > > BUILD_BUG_ON_MSG for making sure 'count' is a compile time constant expression > > would be helpful. Sure, we'll get -Wvla but some developers might still ignore > > the warning and send patches with these arrays. So, it would be neater if we > > fail to build and force them to fix their code. > BUILD_BUG_ON_MSG() won't work because it expands to a do/while loop which won't > work in static struct declarations. But I can try to see if we can come up with > something that works. I guess Nuno is okay with static_assert() and TBH nowadays the BUILD_BUG() as is most likely historical. -- With Best Regards, Andy Shevchenko
On Wed, 2025-04-23 at 18:43 +0300, Andy Shevchenko wrote: > On Wed, Apr 23, 2025 at 5:51 PM David Lechner <dlechner@baylibre.com> wrote: > > On 4/23/25 4:18 AM, Nuno Sá wrote: > > > On Tue, 2025-04-22 at 17:07 -0500, David Lechner wrote: > > ... > > > > On the other hand, as I mentioned in V1, I think that an assertion or > > > BUILD_BUG_ON_MSG for making sure 'count' is a compile time constant > > > expression > > > would be helpful. Sure, we'll get -Wvla but some developers might still > > > ignore > > > the warning and send patches with these arrays. So, it would be neater if > > > we > > > fail to build and force them to fix their code. > > > BUILD_BUG_ON_MSG() won't work because it expands to a do/while loop which > > won't > > work in static struct declarations. But I can try to see if we can come up > > with > > something that works. > > I guess Nuno is okay with static_assert() and TBH nowadays the > BUILD_BUG() as is most likely historical. Yes... "...I think that an __assertion__ or BUILD_BUG_ON_MSG..." - Nuno Sá
On Wed, Apr 23, 2025 at 1:08 AM David Lechner <dlechner@baylibre.com> wrote: > > Add new macros 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. > > To avoid double __align() attributes in cases where we also need DMA > alignment, add a 2nd variant IIO_DECLARE_DMA_BUFFER_WITH_TS. ... > +#define _IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \ > + type name[ALIGN((count), sizeof(s64) / sizeof(type)) + sizeof(s64) / sizeof(type)] Single leading underscore seems to me not so usual, I saw people use double underscores to make sure that it will be visible that it's an internal one (kinda). ... > +_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0, Why not static_assert() ? Because of the message? But static_assert() supports messages AFAICS. > + "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment"); -- With Best Regards, Andy Shevchenko
On 4/22/25 5:30 PM, Andy Shevchenko wrote: > On Wed, Apr 23, 2025 at 1:08 AM David Lechner <dlechner@baylibre.com> wrote: >> ... >> +_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0, > > Why not static_assert() ? Because of the message? But static_assert() > supports messages AFAICS. > >> + "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment"); > I just knew that was standard C. But I support BUILD_BUG_ON_MSG or static_assert would work just as well here.
On Wed, Apr 23, 2025 at 1:37 AM David Lechner <dlechner@baylibre.com> wrote: > On 4/22/25 5:30 PM, Andy Shevchenko wrote: > > On Wed, Apr 23, 2025 at 1:08 AM David Lechner <dlechner@baylibre.com> wrote: ... > >> +_Static_assert(sizeof(IIO_DMA_MINALIGN) % sizeof(s64) == 0, > > > > Why not static_assert() ? Because of the message? But static_assert() > > supports messages AFAICS. > > > >> + "macros above assume that IIO_DMA_MINALIGN also ensures s64 timestamp alignment"); > > I just knew that was standard C. But I support BUILD_BUG_ON_MSG or static_assert > would work just as well here. In the kernel we use u8, for example, however in the standard it's uint8_t :-) Same with many compiler attributes and wrappers on top of the compiler things. According to v6.14 codebase the only one driver uses _Static_assert() for that (there are many in tools/ and more in BPF, with a few headers where it's fine (esp. in UAPI where no static_assert() available). -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.