The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data
@ 2026-05-10  8:25 Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 1/5] iio: accel: adxl372: Add timestamp " Md Shofiqul Islam
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Md Shofiqul Islam @ 2026-05-10  8:25 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Md Shofiqul Islam

Five ADXL-family accelerometer drivers (ADXL313, ADXL345, ADXL367,
ADXL372, ADXL380) push buffered samples using iio_push_to_buffers(),
which does not attach a hardware timestamp to the scan data.
Userspace consumers therefore receive samples with no timing
information.

This series adds timestamp support uniformly across the family:

  - A scan buffer struct with an aligned_s64 ts field is added to each
    driver's state struct.  The struct layout ensures the timestamp
    field sits at an 8-byte aligned offset as required by
    iio_push_to_buffers_with_timestamp().

  - In the FIFO push loop, FIFO data is copied into scan.channels via
    memcpy(), then iio_push_to_buffers_with_timestamp() is called with
    a single timestamp captured once per interrupt with
    iio_get_time_ns().  Using one timestamp per IRQ is consistent with
    the existing approach in the same handlers for event timestamps.

  - For ADXL367, the helper adxl367_push_fifo_data() gains a s64 ts
    parameter so the timestamp captured in the IRQ handler is passed
    through instead of calling iio_get_time_ns() a second time.

  - For ADXL372 and ADXL380, where the IRQ handler already called
    iio_get_time_ns() for the event push, the same captured timestamp
    is now also passed to the FIFO push, removing the duplicate call.

The ADXL313 and ADXL345 drivers always scan all three axes together
(available_scan_masks contains only the full X|Y|Z mask), so their
scan buffer layout is fixed.  The ADXL367, ADXL372, and ADXL380
drivers support variable scan masks; fifo_set_size tracks the number
of enabled channels per sample set and is used as the memcpy length.

Md Shofiqul Islam (5):
  iio: accel: adxl372: Add timestamp to FIFO data
  iio: accel: adxl380: Add timestamp to FIFO data
  iio: accel: adxl367: Add timestamp to FIFO data
  iio: accel: adxl313: Add timestamp to FIFO data
  iio: accel: adxl345: Add timestamp to FIFO data

 drivers/iio/accel/adxl313.h      |  4 ++++
 drivers/iio/accel/adxl313_core.c |  8 ++++++--
 drivers/iio/accel/adxl345_core.c | 12 ++++++++++--
 drivers/iio/accel/adxl367.c      | 17 +++++++++++++----
 drivers/iio/accel/adxl372.c      | 13 +++++++++++--
 drivers/iio/accel/adxl380.c      | 15 ++++++++++++---
 6 files changed, 56 insertions(+), 13 deletions(-)

-- 
2.54.0.windows.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] iio: accel: adxl372: Add timestamp to FIFO data
  2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
@ 2026-05-10  8:25 ` Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 2/5] iio: accel: adxl380: " Md Shofiqul Islam
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Md Shofiqul Islam @ 2026-05-10  8:25 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Md Shofiqul Islam

The driver pushes FIFO samples using iio_push_to_buffers() which does
not attach a hardware timestamp to the data.  Add a scan buffer struct
with an aligned_s64 timestamp field to the driver state, capture a
single timestamp per IRQ with iio_get_time_ns(), and switch the FIFO
push loop to iio_push_to_buffers_with_timestamp().  The same timestamp
is reused for the event push call in the same handler, replacing the
duplicate iio_get_time_ns() invocation there.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/iio/accel/adxl372.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 545a21e5a3..521e8313b1 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -367,6 +367,10 @@ struct adxl372_state {
 	u16				watermark;
 	__be16				fifo_buf[ADXL372_FIFO_SIZE];
 	bool				peak_fifo_mode_en;
+	struct {
+		__be16 channels[3];
+		aligned_s64 ts;
+	} scan;
 	struct mutex			threshold_m; /* lock for threshold */
 };
 
@@ -703,13 +707,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 +739,10 @@ 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]);
+			memcpy(st->scan.channels, &st->fifo_buf[i],
+			       st->fifo_set_size * sizeof(__be16));
+			iio_push_to_buffers_with_timestamp(indio_dev,
+							   &st->scan, ts);
 		}
 	}
 err:
-- 
2.54.0.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] iio: accel: adxl380: Add timestamp to FIFO data
  2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 1/5] iio: accel: adxl372: Add timestamp " Md Shofiqul Islam
@ 2026-05-10  8:25 ` Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 3/5] iio: accel: adxl367: " Md Shofiqul Islam
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Md Shofiqul Islam @ 2026-05-10  8:25 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Md Shofiqul Islam

The driver pushes FIFO samples using iio_push_to_buffers() which does
not attach a hardware timestamp to the data.  Add a scan buffer struct
with an aligned_s64 timestamp field to the driver state, capture a
single timestamp per IRQ with iio_get_time_ns(), and switch the FIFO
push loop to iio_push_to_buffers_with_timestamp().  The same timestamp
is reused for the event push call in the same handler, replacing the
duplicate iio_get_time_ns() invocation there.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/iio/accel/adxl380.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/adxl380.c b/drivers/iio/accel/adxl380.c
index e7bb32fbc4..b8d0d15b00 100644
--- a/drivers/iio/accel/adxl380.c
+++ b/drivers/iio/accel/adxl380.c
@@ -224,6 +224,10 @@ struct adxl380_state {
 	int hpf_tbl[7][2];
 
 	__be16 fifo_buf[ADXL380_FIFO_SAMPLES] __aligned(IIO_DMA_MINALIGN);
+	struct {
+		__be16 channels[4];
+		aligned_s64 ts;
+	} scan;
 };
 
 bool adxl380_readable_noinc_reg(struct device *dev, unsigned int reg)
@@ -948,6 +952,7 @@ static irqreturn_t adxl380_irq_handler(int irq, void  *p)
 	struct adxl380_state *st = iio_priv(indio_dev);
 	u8 status0, status1;
 	u16 fifo_entries;
+	s64 ts;
 	int i;
 	int ret;
 
@@ -957,7 +962,8 @@ static irqreturn_t adxl380_irq_handler(int irq, void  *p)
 	if (ret)
 		return IRQ_HANDLED;
 
-	adxl380_push_event(indio_dev, iio_get_time_ns(indio_dev), status1);
+	ts = iio_get_time_ns(indio_dev);
+	adxl380_push_event(indio_dev, ts, status1);
 
 	if (!FIELD_GET(ADXL380_STATUS_0_FIFO_WM_MSK, status0))
 		return IRQ_HANDLED;
@@ -971,8 +977,11 @@ static irqreturn_t adxl380_irq_handler(int irq, void  *p)
 				sizeof(*st->fifo_buf) * fifo_entries);
 	if (ret)
 		return IRQ_HANDLED;
-	for (i = 0; i < fifo_entries; i += st->fifo_set_size)
-		iio_push_to_buffers(indio_dev, &st->fifo_buf[i]);
+	for (i = 0; i < fifo_entries; i += st->fifo_set_size) {
+		memcpy(st->scan.channels, &st->fifo_buf[i],
+		       st->fifo_set_size * sizeof(__be16));
+		iio_push_to_buffers_with_timestamp(indio_dev, &st->scan, ts);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
2.54.0.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] iio: accel: adxl367: Add timestamp to FIFO data
  2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 1/5] iio: accel: adxl372: Add timestamp " Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 2/5] iio: accel: adxl380: " Md Shofiqul Islam
@ 2026-05-10  8:25 ` Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 4/5] iio: accel: adxl313: " Md Shofiqul Islam
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Md Shofiqul Islam @ 2026-05-10  8:25 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Md Shofiqul Islam

The driver pushes FIFO samples using iio_push_to_buffers() which does
not attach a hardware timestamp to the data.  Add a scan buffer struct
with an aligned_s64 timestamp field to the driver state, capture a
single timestamp per IRQ with iio_get_time_ns() in the IRQ handler, and
pass it into adxl367_push_fifo_data() (whose signature gains a s64 ts
parameter) to switch the FIFO push loop to
iio_push_to_buffers_with_timestamp().

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/iio/accel/adxl367.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
index 0c04b2bb7e..2852f2fdfa 100644
--- a/drivers/iio/accel/adxl367.c
+++ b/drivers/iio/accel/adxl367.c
@@ -179,6 +179,10 @@ struct adxl367_state {
 
 	__be16		fifo_buf[ADXL367_FIFO_SIZE] __aligned(IIO_DMA_MINALIGN);
 	__be16		sample_buf;
+	struct {
+		__be16 channels[4];
+		aligned_s64 ts;
+	} scan;
 	u8		act_threshold_buf[2];
 	u8		inact_time_buf[2];
 	u8		status_buf[3];
@@ -779,7 +783,7 @@ static bool adxl367_push_event(struct iio_dev *indio_dev, u8 status)
 }
 
 static bool adxl367_push_fifo_data(struct iio_dev *indio_dev, u8 status,
-				   u16 fifo_entries)
+				   u16 fifo_entries, s64 ts)
 {
 	struct adxl367_state *st = iio_priv(indio_dev);
 	int ret;
@@ -796,8 +800,11 @@ static bool adxl367_push_fifo_data(struct iio_dev *indio_dev, u8 status,
 		return true;
 	}
 
-	for (i = 0; i < fifo_entries; i += st->fifo_set_size)
-		iio_push_to_buffers(indio_dev, &st->fifo_buf[i]);
+	for (i = 0; i < fifo_entries; i += st->fifo_set_size) {
+		memcpy(st->scan.channels, &st->fifo_buf[i],
+		       st->fifo_set_size * sizeof(__be16));
+		iio_push_to_buffers_with_timestamp(indio_dev, &st->scan, ts);
+	}
 
 	return true;
 }
@@ -809,14 +816,16 @@ static irqreturn_t adxl367_irq_handler(int irq, void *private)
 	u16 fifo_entries;
 	bool handled;
 	u8 status;
+	s64 ts;
 	int ret;
 
 	ret = adxl367_get_status(st, &status, &fifo_entries);
 	if (ret)
 		return IRQ_NONE;
 
+	ts = iio_get_time_ns(indio_dev);
 	handled = adxl367_push_event(indio_dev, status);
-	handled |= adxl367_push_fifo_data(indio_dev, status, fifo_entries);
+	handled |= adxl367_push_fifo_data(indio_dev, status, fifo_entries, ts);
 
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
-- 
2.54.0.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] iio: accel: adxl313: Add timestamp to FIFO data
  2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
                   ` (2 preceding siblings ...)
  2026-05-10  8:25 ` [PATCH 3/5] iio: accel: adxl367: " Md Shofiqul Islam
@ 2026-05-10  8:25 ` Md Shofiqul Islam
  2026-05-10  8:25 ` [PATCH 5/5] iio: accel: adxl345: " Md Shofiqul Islam
  2026-05-10 12:58 ` [PATCH 0/5] iio: accel: adxl3xx: Add timestamps " Andy Shevchenko
  5 siblings, 0 replies; 8+ messages in thread
From: Md Shofiqul Islam @ 2026-05-10  8:25 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Md Shofiqul Islam

The driver pushes FIFO samples using iio_push_to_buffers() which does
not attach a hardware timestamp to the data.  Add a scan buffer struct
with an aligned_s64 timestamp field to struct adxl313_data (in
adxl313.h), capture a timestamp with iio_get_time_ns() at the start of
adxl313_fifo_push(), and switch the push loop to
iio_push_to_buffers_with_timestamp().  The ADXL313 always scans all
three axes together so the scan buffer layout is fixed.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/iio/accel/adxl313.h      | 4 ++++
 drivers/iio/accel/adxl313_core.c | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index 75ef54b60f..ea5792fae2 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -94,6 +94,10 @@ struct adxl313_data {
 	u8 watermark;
 	__le16		transf_buf __aligned(IIO_DMA_MINALIGN);
 	__le16		fifo_buf[ADXL313_NUM_AXIS * ADXL313_FIFO_SIZE + 1];
+	struct {
+		__le16 channels[ADXL313_NUM_AXIS];
+		aligned_s64 ts;
+	} scan;
 };
 
 struct adxl313_chip_info {
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index bcc11dabdf..bd45b90c42 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -1013,6 +1013,7 @@ static const struct iio_buffer_setup_ops adxl313_buffer_ops = {
 static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
+	s64 ts = iio_get_time_ns(indio_dev);
 	unsigned int i;
 	int ret;
 
@@ -1020,8 +1021,11 @@ static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
 	if (ret)
 		return ret;
 
-	for (i = 0; i < ADXL313_NUM_AXIS * samples; i += ADXL313_NUM_AXIS)
-		iio_push_to_buffers(indio_dev, &data->fifo_buf[i]);
+	for (i = 0; i < ADXL313_NUM_AXIS * samples; i += ADXL313_NUM_AXIS) {
+		memcpy(data->scan.channels, &data->fifo_buf[i],
+		       sizeof(data->scan.channels));
+		iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, ts);
+	}
 
 	return 0;
 }
-- 
2.54.0.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] iio: accel: adxl345: Add timestamp to FIFO data
  2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
                   ` (3 preceding siblings ...)
  2026-05-10  8:25 ` [PATCH 4/5] iio: accel: adxl313: " Md Shofiqul Islam
@ 2026-05-10  8:25 ` Md Shofiqul Islam
  2026-05-10 12:58 ` [PATCH 0/5] iio: accel: adxl3xx: Add timestamps " Andy Shevchenko
  5 siblings, 0 replies; 8+ messages in thread
From: Md Shofiqul Islam @ 2026-05-10  8:25 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Md Shofiqul Islam

The driver pushes FIFO samples using iio_push_to_buffers() which does
not attach a hardware timestamp to the data.  Add a scan buffer struct
with an aligned_s64 timestamp field to struct adxl345_state, capture a
timestamp with iio_get_time_ns() at the start of adxl345_fifo_push(),
and switch the push loop to iio_push_to_buffers_with_timestamp().  The
ADXL345 always scans all three axes together so the scan buffer layout
is fixed.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 6c9080d88c..2b121bed89 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -204,6 +204,10 @@ struct adxl345_state {
 	u32 tap_window_us;
 
 	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
+	struct {
+		__le16 channels[ADXL345_DIRS];
+		aligned_s64 ts;
+	} scan;
 };
 
 static const struct iio_event_spec adxl345_events[] = {
@@ -1657,6 +1661,7 @@ static int adxl345_fifo_push(struct iio_dev *indio_dev,
 			     int samples)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
+	s64 ts = iio_get_time_ns(indio_dev);
 	int i, ret;
 
 	if (samples <= 0)
@@ -1666,8 +1671,11 @@ static int adxl345_fifo_push(struct iio_dev *indio_dev,
 	if (ret)
 		return ret;
 
-	for (i = 0; i < ADXL345_DIRS * samples; i += ADXL345_DIRS)
-		iio_push_to_buffers(indio_dev, &st->fifo_buf[i]);
+	for (i = 0; i < ADXL345_DIRS * samples; i += ADXL345_DIRS) {
+		memcpy(st->scan.channels, &st->fifo_buf[i],
+		       sizeof(st->scan.channels));
+		iio_push_to_buffers_with_timestamp(indio_dev, &st->scan, ts);
+	}
 
 	return 0;
 }
-- 
2.54.0.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data
  2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
                   ` (4 preceding siblings ...)
  2026-05-10  8:25 ` [PATCH 5/5] iio: accel: adxl345: " Md Shofiqul Islam
@ 2026-05-10 12:58 ` Andy Shevchenko
       [not found]   ` <CAOTCDVsUF1qaYgSGa4hcDYOqmMpkK2G+HYeQ=ShQ1oEbM8nGJQ@mail.gmail.com>
  5 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2026-05-10 12:58 UTC (permalink / raw)
  To: Md Shofiqul Islam
  Cc: jic23, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
	linux-iio, linux-kernel

On Sun, May 10, 2026 at 11:25:51AM +0300, Md Shofiqul Islam wrote:
> Five ADXL-family accelerometer drivers (ADXL313, ADXL345, ADXL367,
> ADXL372, ADXL380) push buffered samples using iio_push_to_buffers(),
> which does not attach a hardware timestamp to the scan data.
> Userspace consumers therefore receive samples with no timing
> information.
> 
> This series adds timestamp support uniformly across the family:
> 
>   - A scan buffer struct with an aligned_s64 ts field is added to each
>     driver's state struct.  The struct layout ensures the timestamp
>     field sits at an 8-byte aligned offset as required by
>     iio_push_to_buffers_with_timestamp().
> 
>   - In the FIFO push loop, FIFO data is copied into scan.channels via
>     memcpy(), then iio_push_to_buffers_with_timestamp() is called with
>     a single timestamp captured once per interrupt with
>     iio_get_time_ns().  Using one timestamp per IRQ is consistent with
>     the existing approach in the same handlers for event timestamps.
> 
>   - For ADXL367, the helper adxl367_push_fifo_data() gains a s64 ts
>     parameter so the timestamp captured in the IRQ handler is passed
>     through instead of calling iio_get_time_ns() a second time.
> 
>   - For ADXL372 and ADXL380, where the IRQ handler already called
>     iio_get_time_ns() for the event push, the same captured timestamp
>     is now also passed to the FIFO push, removing the duplicate call.
> 
> The ADXL313 and ADXL345 drivers always scan all three axes together
> (available_scan_masks contains only the full X|Y|Z mask), so their
> scan buffer layout is fixed.  The ADXL367, ADXL372, and ADXL380
> drivers support variable scan masks; fifo_set_size tracks the number
> of enabled channels per sample set and is used as the memcpy length.

This is sensitive change. Do we have any confirmation that this
- does work as expected on real HW and platforms that use these devices
- does not break any ABI

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data
       [not found]   ` <CAOTCDVsUF1qaYgSGa4hcDYOqmMpkK2G+HYeQ=ShQ1oEbM8nGJQ@mail.gmail.com>
@ 2026-05-11 14:41     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-11 14:41 UTC (permalink / raw)
  To: Md Shofiqul Islam
  Cc: Andy Shevchenko, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
	linux-iio, linux-kernel

On Sun, 10 May 2026 23:02:00 +0300
Md Shofiqul Islam <shofiqtest@gmail.com> wrote:

> On the ABI question: none of the five drivers in this series define
> IIO_CHAN_SOFT_TIMESTAMP in their channel arrays, and their
> available_scan_masks do not include a timestamp bit.
> As a result, indio_dev->scan_timestamp is never set for any of them, which
> means iio_push_to_buffers_with_timestamp() writes no timestamp and the call
> reduces to iio_push_to_buffers().
> The scan buffer content seen by userspace is byte-for-byte identical before
> and after this series. No ABI is changed.
> 
> That said, this also means the series does not actually deliver timestamps
> to userspace. To do that, a follow-on patch adding
> IIO_CHAN_SOFT_TIMESTAMP(N) to each driver's channel array is required.
> That would be an intentional, explicit ABI addition. Should I include that
> in a v2 of this series?
> 
Yes if you take this forward. However, so far I'm giving this a hard no.

> On hardware testing: I do not have any of the ADXL313/345/367/372/380
> devices. Given that the patches are currently a no-op for userspace the
> risk of a silent regression is low, but real-hardware confirmation is
> needed before this can be considered complete.
> If any maintainer or user of these parts on the list can run a test, I
> would welcome that.
> 
> I also verified that the scan struct alignment is correct for all five
> drivers: the aligned_s64 ts field sits at offset 8 in each case, satisfying
> the 8-byte alignment required by iio_push_to_buffers_with_timestamp().

It's no where near this simple.  Think about how a fifo is working.
You are adding a timestamp based on when the last element that triggered
the fifo watershed was added.  That is wrong for all the other data pulled
off the fifo which should have earlier timestamps.

It is possible to approximate the correct timestamps but it's complex.
See the support form the invensense IMUs which may not even be appropriate
to port to these drivers.

You definitely don't want to try getting timestamps working for a fifo
equipped part without real hardware so you can plot them over time and
verify there are no inconsistencies of bugs in your timestamp approximation
algorithm.

Note the invensense one has been rewritten several times - that should give
you an idea how hard this is to do.

Thanks,

Jonathan

> 
> Thanks
> Shofiq
> 
> On Sun, May 10, 2026 at 3:58 PM Andy Shevchenko <andriy.shevchenko@intel.com>
> wrote:
> 
> > On Sun, May 10, 2026 at 11:25:51AM +0300, Md Shofiqul Islam wrote:  
> > > Five ADXL-family accelerometer drivers (ADXL313, ADXL345, ADXL367,
> > > ADXL372, ADXL380) push buffered samples using iio_push_to_buffers(),
> > > which does not attach a hardware timestamp to the scan data.
> > > Userspace consumers therefore receive samples with no timing
> > > information.
> > >
> > > This series adds timestamp support uniformly across the family:
> > >
> > >   - A scan buffer struct with an aligned_s64 ts field is added to each
> > >     driver's state struct.  The struct layout ensures the timestamp
> > >     field sits at an 8-byte aligned offset as required by
> > >     iio_push_to_buffers_with_timestamp().
> > >
> > >   - In the FIFO push loop, FIFO data is copied into scan.channels via
> > >     memcpy(), then iio_push_to_buffers_with_timestamp() is called with
> > >     a single timestamp captured once per interrupt with
> > >     iio_get_time_ns().  Using one timestamp per IRQ is consistent with
> > >     the existing approach in the same handlers for event timestamps.
> > >
> > >   - For ADXL367, the helper adxl367_push_fifo_data() gains a s64 ts
> > >     parameter so the timestamp captured in the IRQ handler is passed
> > >     through instead of calling iio_get_time_ns() a second time.
> > >
> > >   - For ADXL372 and ADXL380, where the IRQ handler already called
> > >     iio_get_time_ns() for the event push, the same captured timestamp
> > >     is now also passed to the FIFO push, removing the duplicate call.
> > >
> > > The ADXL313 and ADXL345 drivers always scan all three axes together
> > > (available_scan_masks contains only the full X|Y|Z mask), so their
> > > scan buffer layout is fixed.  The ADXL367, ADXL372, and ADXL380
> > > drivers support variable scan masks; fifo_set_size tracks the number
> > > of enabled channels per sample set and is used as the memcpy length.  
> >
> > This is sensitive change. Do we have any confirmation that this
> > - does work as expected on real HW and platforms that use these devices
> > - does not break any ABI
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
> >  


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-11 14:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10  8:25 [PATCH 0/5] iio: accel: adxl3xx: Add timestamps to FIFO data Md Shofiqul Islam
2026-05-10  8:25 ` [PATCH 1/5] iio: accel: adxl372: Add timestamp " Md Shofiqul Islam
2026-05-10  8:25 ` [PATCH 2/5] iio: accel: adxl380: " Md Shofiqul Islam
2026-05-10  8:25 ` [PATCH 3/5] iio: accel: adxl367: " Md Shofiqul Islam
2026-05-10  8:25 ` [PATCH 4/5] iio: accel: adxl313: " Md Shofiqul Islam
2026-05-10  8:25 ` [PATCH 5/5] iio: accel: adxl345: " Md Shofiqul Islam
2026-05-10 12:58 ` [PATCH 0/5] iio: accel: adxl3xx: Add timestamps " Andy Shevchenko
     [not found]   ` <CAOTCDVsUF1qaYgSGa4hcDYOqmMpkK2G+HYeQ=ShQ1oEbM8nGJQ@mail.gmail.com>
2026-05-11 14:41     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox