[PATCH v2] iio: accel: adxl372: Add timestamp to FIFO data

Md Shofiqul Islam posted 1 patch 1 week, 1 day ago
drivers/iio/accel/adxl372.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
[PATCH v2] iio: accel: adxl372: Add timestamp to FIFO data
Posted by Md Shofiqul Islam 1 week, 1 day ago
The driver pushes FIFO samples using iio_push_to_buffers() which does
not attach a hardware timestamp to the data.  Capture a single timestamp
per IRQ with iio_get_time_ns() and reuse it for both the event push and
the FIFO sample loop.

Use IIO_DECLARE_BUFFER_WITH_TS() for fifo_buf to guarantee the required
s64 alignment, and switch the FIFO push loop to
iio_push_to_buffers_with_ts() which accepts an explicit data size and
timestamp without requiring a separate scan struct or memcpy.

Add IIO_CHAN_SOFT_TIMESTAMP(3) to the channel spec so the IIO core
enables scan_timestamp and includes the timestamp slot in the buffer.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
Changes in v2:
  - Use IIO_DECLARE_BUFFER_WITH_TS() + iio_push_to_buffers_with_ts() instead
    of a separate scan struct + memcpy, per review from David Lechner
  - Add IIO_CHAN_SOFT_TIMESTAMP(3) to channel spec (missing in v1)
 drivers/iio/accel/adxl372.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index e375d068a..266e80e27 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -344,6 +344,7 @@ static const struct iio_chan_spec adxl372_channels[] = {
 	ADXL372_ACCEL_CHANNEL(0, ADXL372_X_DATA_H, X),
 	ADXL372_ACCEL_CHANNEL(1, ADXL372_Y_DATA_H, Y),
 	ADXL372_ACCEL_CHANNEL(2, ADXL372_Z_DATA_H, Z),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 struct adxl372_state {
@@ -365,7 +366,7 @@ struct adxl372_state {
 	u8				fifo_set_size;
 	unsigned long			int1_bitmask;
 	u16				watermark;
-	__be16				fifo_buf[ADXL372_FIFO_SIZE];
+	IIO_DECLARE_BUFFER_WITH_TS(__be16, fifo_buf, ADXL372_FIFO_SIZE);
 	bool				peak_fifo_mode_en;
 	struct mutex			threshold_m; /* lock for threshold */
 };
@@ -703,13 +704,15 @@ static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
 	struct adxl372_state *st = iio_priv(indio_dev);
 	u8 status1, status2;
 	u16 fifo_entries;
+	s64 ts;
 	int i, ret;
 
 	ret = adxl372_get_status(st, &status1, &status2, &fifo_entries);
 	if (ret < 0)
 		goto err;
 
-	adxl372_push_event(indio_dev, iio_get_time_ns(indio_dev), status2);
+	ts = iio_get_time_ns(indio_dev);
+	adxl372_push_event(indio_dev, ts, status2);
 
 	if (st->fifo_mode != ADXL372_FIFO_BYPASSED &&
 	    ADXL372_STATUS_1_FIFO_FULL(status1)) {
@@ -733,7 +736,9 @@ static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
 			/* filter peak detection data */
 			if (st->peak_fifo_mode_en)
 				adxl372_arrange_axis_data(st, &st->fifo_buf[i]);
-			iio_push_to_buffers(indio_dev, &st->fifo_buf[i]);
+			iio_push_to_buffers_with_ts(indio_dev, &st->fifo_buf[i],
+						    st->fifo_set_size * sizeof(__be16),
+						    ts);
 		}
 	}
 err:
-- 
2.51.1
Re: [PATCH v2] iio: accel: adxl372: Add timestamp to FIFO data
Posted by Stepan Ionichev 1 week ago
On Sat, May 16, 2026, Md Shofiqul Islam wrote:
> +			iio_push_to_buffers_with_ts(indio_dev, &st->fifo_buf[i],
> +						    st->fifo_set_size * sizeof(__be16),
> +						    ts);

On top of David's note about the size argument, the bigger issue is
that iio_push_to_buffers_with_ts() writes the timestamp into the
buffer it was given, at offset (scan_bytes - sizeof(s64)) from the
passed pointer. For the 3-channel scan here scan_bytes is 16, so the
s64 lands at &fifo_buf[i + 4]. With fifo_set_size == 3 that overlaps
the next iteration's samples in the same array, so the following
push reads partially overwritten data.

iio_push_to_buffers_with_ts_unaligned() copies into an internal
aligned bounce buffer instead and does not touch the caller's data,
which fits this looped FIFO-drain pattern. Alternatively, the v1
shape with a small local scan struct and a memcpy per iteration
works too.

Stepan
Re: [PATCH v2] iio: accel: adxl372: Add timestamp to FIFO data
Posted by David Lechner 1 week, 1 day ago
On 5/16/26 4:19 PM, Md Shofiqul Islam wrote:
> The driver pushes FIFO samples using iio_push_to_buffers() which does
> not attach a hardware timestamp to the data.  Capture a single timestamp
> per IRQ with iio_get_time_ns() and reuse it for both the event push and
> the FIFO sample loop.
> 
> Use IIO_DECLARE_BUFFER_WITH_TS() for fifo_buf to guarantee the required
> s64 alignment, and switch the FIFO push loop to
> iio_push_to_buffers_with_ts() which accepts an explicit data size and
> timestamp without requiring a separate scan struct or memcpy.
> 
> Add IIO_CHAN_SOFT_TIMESTAMP(3) to the channel spec so the IIO core
> enables scan_timestamp and includes the timestamp slot in the buffer.
> 
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> ---
> Changes in v2:
>   - Use IIO_DECLARE_BUFFER_WITH_TS() + iio_push_to_buffers_with_ts() instead
>     of a separate scan struct + memcpy, per review from David Lechner
>   - Add IIO_CHAN_SOFT_TIMESTAMP(3) to channel spec (missing in v1)
>  drivers/iio/accel/adxl372.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index e375d068a..266e80e27 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -344,6 +344,7 @@ static const struct iio_chan_spec adxl372_channels[] = {
>  	ADXL372_ACCEL_CHANNEL(0, ADXL372_X_DATA_H, X),
>  	ADXL372_ACCEL_CHANNEL(1, ADXL372_Y_DATA_H, Y),
>  	ADXL372_ACCEL_CHANNEL(2, ADXL372_Z_DATA_H, Z),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
>  struct adxl372_state {
> @@ -365,7 +366,7 @@ struct adxl372_state {
>  	u8				fifo_set_size;
>  	unsigned long			int1_bitmask;
>  	u16				watermark;
> -	__be16				fifo_buf[ADXL372_FIFO_SIZE];
> +	IIO_DECLARE_BUFFER_WITH_TS(__be16, fifo_buf, ADXL372_FIFO_SIZE);
>  	bool				peak_fifo_mode_en;
>  	struct mutex			threshold_m; /* lock for threshold */
>  };
> @@ -703,13 +704,15 @@ static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
>  	struct adxl372_state *st = iio_priv(indio_dev);
>  	u8 status1, status2;
>  	u16 fifo_entries;
> +	s64 ts;
>  	int i, ret;
>  
>  	ret = adxl372_get_status(st, &status1, &status2, &fifo_entries);
>  	if (ret < 0)
>  		goto err;
>  
> -	adxl372_push_event(indio_dev, iio_get_time_ns(indio_dev), status2);
> +	ts = iio_get_time_ns(indio_dev);
> +	adxl372_push_event(indio_dev, ts, status2);
>  
>  	if (st->fifo_mode != ADXL372_FIFO_BYPASSED &&
>  	    ADXL372_STATUS_1_FIFO_FULL(status1)) {
> @@ -733,7 +736,9 @@ static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
>  			/* filter peak detection data */
>  			if (st->peak_fifo_mode_en)
>  				adxl372_arrange_axis_data(st, &st->fifo_buf[i]);
> -			iio_push_to_buffers(indio_dev, &st->fifo_buf[i]);
> +			iio_push_to_buffers_with_ts(indio_dev, &st->fifo_buf[i],
> +						    st->fifo_set_size * sizeof(__be16),

Should just be sizeof(st->fifo_buf), otherwise it doesn't
actually protect against buffer overflow.

> +						    ts);
>  		}
>  	}
>  err:
Re: [PATCH v2] iio: accel: adxl372: Add timestamp to FIFO data
Posted by David Lechner 1 week, 1 day ago
On 5/16/26 4:19 PM, Md Shofiqul Islam wrote:
> The driver pushes FIFO samples using iio_push_to_buffers() which does
> not attach a hardware timestamp to the data.  Capture a single timestamp
> per IRQ with iio_get_time_ns() and reuse it for both the event push and
> the FIFO sample loop.
> 
> Use IIO_DECLARE_BUFFER_WITH_TS() for fifo_buf to guarantee the required
> s64 alignment, and switch the FIFO push loop to
> iio_push_to_buffers_with_ts() which accepts an explicit data size and
> timestamp without requiring a separate scan struct or memcpy.
> 
> Add IIO_CHAN_SOFT_TIMESTAMP(3) to the channel spec so the IIO core
> enables scan_timestamp and includes the timestamp slot in the buffer.
> 
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> ---
> Changes in v2:
>   - Use IIO_DECLARE_BUFFER_WITH_TS() + iio_push_to_buffers_with_ts() instead
>     of a separate scan struct + memcpy, per review from David Lechner
>   - Add IIO_CHAN_SOFT_TIMESTAMP(3) to channel spec (missing in v1)

There was other discussion on v1 that said adding a timestamp channel
to FIFO data is not practical since we are receiving multiple data points
all at one time. The timestamp will not be accurate other than the last
data point in each burst.

Did you respond to that?

If there is a good reason for it, the commit message should explain that.