* [PATCH v6 1/7] iio: make IIO_DMA_MINALIGN minimum of 8 bytes
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-07 20:42 ` [PATCH v6 2/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros David Lechner
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel
Add a condition to ensure that IIO_DMA_MINALIGN is at least 8 bytes.
On some 32-bit architectures, IIO_DMA_MINALIGN is 4. In many cases,
drivers are using this alignment for buffers that include a 64-bit
timestamp that is used with iio_push_to_buffers_with_ts(), which expects
the timestamp to be aligned to 8 bytes. To handle this, we can just make
IIO_DMA_MINALIGN at least 8 bytes.
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
v6 changes:
- Removed the #if and use MAX instead since apparently clang raises an
error that __alignof__ isn't defined when used in a #if condition.
---
include/linux/iio/iio.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 638cf2420fbd85cf2924d09d061df601d1d4bb2a..a574f22398e45cad1ea741d20d302f88756a1b13 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -10,6 +10,7 @@
#include <linux/device.h>
#include <linux/cdev.h>
#include <linux/compiler_types.h>
+#include <linux/minmax.h>
#include <linux/slab.h>
#include <linux/iio/types.h>
/* IIO TODO LIST */
@@ -775,8 +776,14 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev)
* to in turn include IIO_DMA_MINALIGN'd elements such as buffers which
* must not share cachelines with the rest of the structure, thus making
* them safe for use with non-coherent DMA.
+ *
+ * A number of drivers also use this on buffers that include a 64-bit timestamp
+ * that is used with iio_push_to_buffer_with_ts(). Therefore, in the case where
+ * DMA alignment is not sufficient for proper timestamp alignment, we align to
+ * 8 bytes instead.
*/
-#define IIO_DMA_MINALIGN ARCH_DMA_MINALIGN
+#define IIO_DMA_MINALIGN MAX(ARCH_DMA_MINALIGN, sizeof(s64))
+
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.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 2/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
2025-05-07 20:42 ` [PATCH v6 1/7] iio: make IIO_DMA_MINALIGN minimum of 8 bytes David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-07 20:42 ` [PATCH v6 3/7] iio: adc: ad4695: use IIO_DECLARE_DMA_BUFFER_WITH_TS David Lechner
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel
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().
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
v5 changes:
* Revert back to __align(IIO_DMA_MINALIGN) as IIO_DMA_MINALIGN now has
a minimum of 8 bytes.
v4 changes:
* Drop the static_asserts(). Some 32-bit arches were triggering one, so
we had to address the problem instead of hoping that it didn't exist.
The other made a multi-statement macro, which isn't the best practice
and didn't have a way to make a really helpful error message. The
condition we were trying to catch is still caught by -Wvla.
* Changed __align(IIO_DMA_MINALIGN) to __align(MAX(IIO_DMA_MINALIGN,
sizeof(s64))). As the build-bots found, there are some 32-bit arches
where IIO_DMA_MINALIGN is 4, but we still need 8-byte alignment for
the timestamp.
v3 changes:
* Use leading double-underscore for "private" macro to match "private"
functions that do the same.
* Use static_assert() from linux/build_bug.h instead of _Static_assert()
* Fix incorrectly using sizeof(IIO_DMA_MINALIGN).
* Add check that count argument is constant. (Note, I didn't include a
message in this static assert because it already gives a reasonable
message.)
/home/david/work/bl/linux/drivers/iio/accel/sca3300.c:482:51: error: expression in static assertion is not constant
482 | IIO_DECLARE_BUFFER_WITH_TS(s16, channels, val);
| ^~~
v2 changes:
* Add 2nd macro for DMA alignment
---
include/linux/iio/iio.h | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a574f22398e45cad1ea741d20d302f88756a1b13..d11668f14a3e17654fcf17a4e853d4b493205019 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>
@@ -784,6 +785,37 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev)
*/
#define IIO_DMA_MINALIGN MAX(ARCH_DMA_MINALIGN, sizeof(s64))
+#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)
+
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.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 3/7] iio: adc: ad4695: use IIO_DECLARE_DMA_BUFFER_WITH_TS
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
2025-05-07 20:42 ` [PATCH v6 1/7] iio: make IIO_DMA_MINALIGN minimum of 8 bytes David Lechner
2025-05-07 20:42 ` [PATCH v6 2/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-07 20:42 ` [PATCH v6 4/7] iio: adc: ad4695: rename AD4695_MAX_VIN_CHANNELS David Lechner
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel, Trevor Gamblin
Use IIO_DECLARE_DMA_BUFFER_WITH_TS() to declare the buffer that gets
used with iio_push_to_buffers_with_ts(). This makes the code a bit
easier to read and understand.
Reviewed-by: Trevor Gamblin <tgamblin@baylibre.com>
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
drivers/iio/adc/ad4695.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index 0c633d43e480d5404074e9fa35f1d330b448f0a2..992abf6c63b51dee222caf624e172455fb9b9900 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -160,8 +160,7 @@ struct ad4695_state {
struct spi_transfer buf_read_xfer[AD4695_MAX_CHANNELS * 2 + 3];
struct spi_message buf_read_msg;
/* Raw conversion data received. */
- u16 buf[ALIGN((AD4695_MAX_CHANNELS + 1) * sizeof(u16),
- sizeof(s64)) + sizeof(s64)] __aligned(IIO_DMA_MINALIGN);
+ IIO_DECLARE_DMA_BUFFER_WITH_TS(u16, buf, AD4695_MAX_CHANNELS + 1);
u16 raw_data;
/* Commands to send for single conversion. */
u16 cnv_cmd;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 4/7] iio: adc: ad4695: rename AD4695_MAX_VIN_CHANNELS
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
` (2 preceding siblings ...)
2025-05-07 20:42 ` [PATCH v6 3/7] iio: adc: ad4695: use IIO_DECLARE_DMA_BUFFER_WITH_TS David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-07 20:42 ` [PATCH v6 5/7] iio: adc: ad7380: use IIO_DECLARE_DMA_BUFFER_WITH_TS David Lechner
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel, Trevor Gamblin
Rename AD4695_MAX_CHANNELS to AD4695_MAX_VIN_CHANNELS. It has been a
point of confusion that this macro is only the voltage input channels
and not all channels.
Reviewed-by: Trevor Gamblin <tgamblin@baylibre.com>
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
drivers/iio/adc/ad4695.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index 992abf6c63b51dee222caf624e172455fb9b9900..cda419638d9a88debb3501d05a513b17a4ecde95 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -105,7 +105,7 @@
#define AD4695_REG_ACCESS_SCLK_HZ (10 * MEGA)
/* Max number of voltage input channels. */
-#define AD4695_MAX_CHANNELS 16
+#define AD4695_MAX_VIN_CHANNELS 16
enum ad4695_in_pair {
AD4695_IN_PAIR_REFGND,
@@ -143,8 +143,8 @@ struct ad4695_state {
/* offload also requires separate gpio to manually control CNV */
struct gpio_desc *cnv_gpio;
/* voltages channels plus temperature and timestamp */
- struct iio_chan_spec iio_chan[AD4695_MAX_CHANNELS + 2];
- struct ad4695_channel_config channels_cfg[AD4695_MAX_CHANNELS];
+ struct iio_chan_spec iio_chan[AD4695_MAX_VIN_CHANNELS + 2];
+ struct ad4695_channel_config channels_cfg[AD4695_MAX_VIN_CHANNELS];
const struct ad4695_chip_info *chip_info;
int sample_freq_range[3];
/* Reference voltage. */
@@ -157,10 +157,10 @@ struct ad4695_state {
* to control CS and add a delay between the last SCLK and next
* CNV rising edges.
*/
- struct spi_transfer buf_read_xfer[AD4695_MAX_CHANNELS * 2 + 3];
+ struct spi_transfer buf_read_xfer[AD4695_MAX_VIN_CHANNELS * 2 + 3];
struct spi_message buf_read_msg;
/* Raw conversion data received. */
- IIO_DECLARE_DMA_BUFFER_WITH_TS(u16, buf, AD4695_MAX_CHANNELS + 1);
+ IIO_DECLARE_DMA_BUFFER_WITH_TS(u16, buf, AD4695_MAX_VIN_CHANNELS + 1);
u16 raw_data;
/* Commands to send for single conversion. */
u16 cnv_cmd;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 5/7] iio: adc: ad7380: use IIO_DECLARE_DMA_BUFFER_WITH_TS
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
` (3 preceding siblings ...)
2025-05-07 20:42 ` [PATCH v6 4/7] iio: adc: ad4695: rename AD4695_MAX_VIN_CHANNELS David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-07 20:42 ` [PATCH v6 6/7] iio: accel: sca3300: use IIO_DECLARE_BUFFER_WITH_TS David Lechner
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel
Use IIO_DECLARE_DMA_BUFFER_WITH_TS() to declare the buffer that gets
used with iio_push_to_buffers_with_ts(). This makes the code a bit
easier to read and understand.
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
As discussed in v1, this one stays u8 because it is used with both 16
and 32-bit word sizes.
v3 changes:
* Use IIO_DECLARE_DMA_BUFFER_WITH_TS() and drop __align()
v2 changes:
* None (but I messed up and there was supposed to be a change).
---
drivers/iio/adc/ad7380.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index f93e6c67766aa89b18c1a7dec02ae8912f65261c..ed5e43c8c84cfcc9c4ce1866659a05787c1d6f5e 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -909,8 +909,7 @@ struct ad7380_state {
* Make the buffer large enough for MAX_NUM_CHANNELS 32-bit samples and
* one 64-bit aligned 64-bit timestamp.
*/
- u8 scan_data[ALIGN(MAX_NUM_CHANNELS * sizeof(u32), sizeof(s64))
- + sizeof(s64)] __aligned(IIO_DMA_MINALIGN);
+ IIO_DECLARE_DMA_BUFFER_WITH_TS(u8, scan_data, MAX_NUM_CHANNELS * sizeof(u32));
/* buffers for reading/writing registers */
u16 tx;
u16 rx;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 6/7] iio: accel: sca3300: use IIO_DECLARE_BUFFER_WITH_TS
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
` (4 preceding siblings ...)
2025-05-07 20:42 ` [PATCH v6 5/7] iio: adc: ad7380: use IIO_DECLARE_DMA_BUFFER_WITH_TS David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-07 20:42 ` [PATCH v6 7/7] iio: adc: at91-sama5d2: " David Lechner
2025-05-08 19:09 ` [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS Jonathan Cameron
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel
Use IIO_DECLARE_BUFFER_WITH_TS() to declare the buffer that gets used
with iio_push_to_buffers_with_ts(). This makes the code a bit easier to
read and understand.
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
This is an alternative to [1]. Also, this serves as a test to see if we
can get a rule of thumb to decide how much is too much to put on the
stack vs. needing to put the buffer in a static struct. SCA3300_SCAN_MAX
is 7, so this add a bit over 64 bytes to the stack, make the stack now
roughly double what it was before.
[1]: https://lore.kernel.org/linux-iio/20250418-iio-prefer-aligned_s64-timestamp-v1-1-4c6080710516@baylibre.com/
---
drivers/iio/accel/sca3300.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index 1132bbaba75bcca525fac2f3e19f63546380fd4f..67416a406e2f43e4e417210410904d44c93111d2 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -58,15 +58,6 @@ enum sca3300_scan_indexes {
SCA3300_SCAN_MAX
};
-/*
- * Buffer size max case:
- * Three accel channels, two bytes per channel.
- * Temperature channel, two bytes.
- * Three incli channels, two bytes per channel.
- * Timestamp channel, eight bytes.
- */
-#define SCA3300_MAX_BUFFER_SIZE (ALIGN(sizeof(s16) * SCA3300_SCAN_MAX, sizeof(s64)) + sizeof(s64))
-
#define SCA3300_ACCEL_CHANNEL(index, reg, axis) { \
.type = IIO_ACCEL, \
.address = reg, \
@@ -193,9 +184,6 @@ struct sca3300_chip_info {
* @spi: SPI device structure
* @lock: Data buffer lock
* @chip: Sensor chip specific information
- * @buffer: Triggered buffer:
- * -SCA3300: 4 channel 16-bit data + 64-bit timestamp
- * -SCL3300: 7 channel 16-bit data + 64-bit timestamp
* @txbuf: Transmit buffer
* @rxbuf: Receive buffer
*/
@@ -203,7 +191,6 @@ struct sca3300_data {
struct spi_device *spi;
struct mutex lock;
const struct sca3300_chip_info *chip;
- u8 buffer[SCA3300_MAX_BUFFER_SIZE] __aligned(sizeof(s64));
u8 txbuf[4] __aligned(IIO_DMA_MINALIGN);
u8 rxbuf[4];
};
@@ -492,7 +479,7 @@ static irqreturn_t sca3300_trigger_handler(int irq, void *p)
struct iio_dev *indio_dev = pf->indio_dev;
struct sca3300_data *data = iio_priv(indio_dev);
int bit, ret, val, i = 0;
- s16 *channels = (s16 *)data->buffer;
+ IIO_DECLARE_BUFFER_WITH_TS(s16, channels, SCA3300_SCAN_MAX);
iio_for_each_active_channel(indio_dev, bit) {
ret = sca3300_read_reg(data, indio_dev->channels[bit].address, &val);
@@ -505,8 +492,7 @@ static irqreturn_t sca3300_trigger_handler(int irq, void *p)
channels[i++] = val;
}
- iio_push_to_buffers_with_ts(indio_dev, data->buffer,
- sizeof(data->buffer),
+ iio_push_to_buffers_with_ts(indio_dev, channels, sizeof(channels),
iio_get_time_ns(indio_dev));
out:
iio_trigger_notify_done(indio_dev->trig);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 7/7] iio: adc: at91-sama5d2: use IIO_DECLARE_BUFFER_WITH_TS
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
` (5 preceding siblings ...)
2025-05-07 20:42 ` [PATCH v6 6/7] iio: accel: sca3300: use IIO_DECLARE_BUFFER_WITH_TS David Lechner
@ 2025-05-07 20:42 ` David Lechner
2025-05-08 19:09 ` [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS Jonathan Cameron
7 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-05-07 20:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-iio, linux-kernel, linux-arm-kernel
Use IIO_DECLARE_BUFFER_WITH_TS() to declare the buffer that gets used
with iio_push_to_buffers_with_ts(). This makes the code a bit easier to
read and understand.
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
v5 changes:
* Fixed missing space at end of comment.
This is an alternative to [1].
[1]: https://lore.kernel.org/linux-iio/20250418-iio-prefer-aligned_s64-timestamp-v1-2-4c6080710516@baylibre.com/
---
drivers/iio/adc/at91-sama5d2_adc.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 414610afcb2c4128a63cf76767803c32cb01ac5e..c3450246730e08cdacc975ed19f46044dc76848f 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -586,15 +586,6 @@ struct at91_adc_temp {
u16 saved_oversampling;
};
-/*
- * Buffer size requirements:
- * No channels * bytes_per_channel(2) + timestamp bytes (8)
- * Divided by 2 because we need half words.
- * We assume 32 channels for now, has to be increased if needed.
- * Nobody minds a buffer being too big.
- */
-#define AT91_BUFFER_MAX_HWORDS ((32 * 2 + 8) / 2)
-
struct at91_adc_state {
void __iomem *base;
int irq;
@@ -616,8 +607,8 @@ struct at91_adc_state {
struct at91_adc_temp temp_st;
struct iio_dev *indio_dev;
struct device *dev;
- /* Ensure naturally aligned timestamp */
- u16 buffer[AT91_BUFFER_MAX_HWORDS] __aligned(8);
+ /* We assume 32 channels for now, has to be increased if needed. */
+ IIO_DECLARE_BUFFER_WITH_TS(u16, buffer, 32);
/*
* lock to prevent concurrent 'single conversion' requests through
* sysfs.
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS
2025-05-07 20:42 [PATCH v6 0/7] iio: introduce IIO_DECLARE_BUFFER_WITH_TS David Lechner
` (6 preceding siblings ...)
2025-05-07 20:42 ` [PATCH v6 7/7] iio: adc: at91-sama5d2: " David Lechner
@ 2025-05-08 19:09 ` Jonathan Cameron
7 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2025-05-08 19:09 UTC (permalink / raw)
To: David Lechner
Cc: Nuno Sá, Andy Shevchenko, Lars-Peter Clausen,
Michael Hennerich, Eugen Hristev, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, linux-iio, linux-kernel,
linux-arm-kernel, Trevor Gamblin
On Wed, 07 May 2025 15:42:39 -0500
David Lechner <dlechner@baylibre.com> wrote:
> Creating a buffer of the proper size and correct alignment for use with
> iio_push_to_buffers_with_ts() is commonly used and not easy to get
> right (as seen by a number of recent fixes on the mailing list).
>
> In general, we prefer to use this pattern for creating such buffers:
>
> struct {
> u16 data[2];
> aligned_s64 timestamp;
> } buffer;
>
> However, there are many cases where a driver may have a large number of
> channels that can be optionally enabled or disabled in a scan or the
> driver might support a range of chips that have different numbers of
> channels or different storage sizes for the data. In these cases, the
> timestamp may not always be at the same place relative to the data. To
> handle these, we allocate a buffer large enough for the largest possible
> case and don't care exactly where the timestamp ends up in the buffer.
>
> For these cases, we propose to introduce new macros to make it easier
> it easier for both the authors to get it right and for readers of the
> code to not have to do all of the math to verify that it is correct.
>
> I have just included a few examples of drivers that can make use of this
> new macro, but there are dozens more.
>
Seems better. Applied and pushed out as testing to see if any other
compiler versions don't like it!
J
> ---
> Changes in v6:
> - Rework IIO_DMA_MINALIGN to work around clang compiler issue.
> - Link to v5: https://lore.kernel.org/r/20250505-iio-introduce-iio_declare_buffer_with_ts-v5-0-814b72b1cae3@baylibre.com
>
> Changes in v5:
> - Add new patch to set minimum alignment to 8 for IIO_DMA_MINALIGN.
> - Adjust IIO_DECLARE_DMA_BUFFER_WITH_TS() macro for above change.
> - Drop one ad4695 patch that was already applied.
> - Link to v4: https://lore.kernel.org/r/20250428-iio-introduce-iio_declare_buffer_with_ts-v4-0-6f7f6126f1cb@baylibre.com
>
> Changes in v4:
> - Dropped static_assert()s from the first patch.
> - Handle case when IIO_DMA_MINALIGN < sizeof(timestamp).
> - Added one more patch for ad4695 to rename a confusing macro.
> - Link to v3: https://lore.kernel.org/r/20250425-iio-introduce-iio_declare_buffer_with_ts-v3-0-f12df1bff248@baylibre.com
>
> Changes in v3:
> - Fixed a few mistakes, style issues and incorporate other feedback (see
> individual commit message changelogs for details).
> - Link to v2: https://lore.kernel.org/r/20250422-iio-introduce-iio_declare_buffer_with_ts-v2-0-3fd36475c706@baylibre.com
>
> Changes in v2:
> - Add 2nd macro for case where we need DMA alignment.
> - Add new patch for ad4695 to convert buffer from u8 to u16 before
> making use of the new macro.
> - Drop the bmp280 patch since it was determined to have a better
> alternative not using these macros.
> - Add a few more examples to show the non-DMA case, both in a struct and
> stack allocated.
> - Link to v1: https://lore.kernel.org/r/20250418-iio-introduce-iio_declare_buffer_with_ts-v1-0-ee0c62a33a0f@baylibre.com
>
> ---
> David Lechner (7):
> iio: make IIO_DMA_MINALIGN minimum of 8 bytes
> iio: introduce IIO_DECLARE_BUFFER_WITH_TS macros
> iio: adc: ad4695: use IIO_DECLARE_DMA_BUFFER_WITH_TS
> iio: adc: ad4695: rename AD4695_MAX_VIN_CHANNELS
> iio: adc: ad7380: use IIO_DECLARE_DMA_BUFFER_WITH_TS
> iio: accel: sca3300: use IIO_DECLARE_BUFFER_WITH_TS
> iio: adc: at91-sama5d2: use IIO_DECLARE_BUFFER_WITH_TS
>
> drivers/iio/accel/sca3300.c | 18 ++---------------
> drivers/iio/adc/ad4695.c | 11 +++++-----
> drivers/iio/adc/ad7380.c | 3 +--
> drivers/iio/adc/at91-sama5d2_adc.c | 13 ++----------
> include/linux/iio/iio.h | 41 +++++++++++++++++++++++++++++++++++++-
> 5 files changed, 50 insertions(+), 36 deletions(-)
> ---
> base-commit: 7e9a82ab5b861d3c33c99a22c1245a5b262ee502
> change-id: 20250418-iio-introduce-iio_declare_buffer_with_ts-2f8773f7dad6
>
> Best regards,
^ permalink raw reply [flat|nested] 9+ messages in thread