1. Patchew
  2. linux
  3. View series

[PATCH] iio: health: max30102: Add timestamp to FIFO data

Md Shofiqul Islam posted 1 patch 4 weeks ago 
drivers/iio/health/max30102.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
[PATCH] iio: health: max30102: Add timestamp to FIFO data
Posted by Md Shofiqul Islam 4 weeks ago 
The I2C DMA read buffer was not aligned to IIO_DMA_MINALIGN, which
can cause issues on platforms with strict DMA alignment requirements.
Fix this by annotating the buffer field with __aligned(IIO_DMA_MINALIGN).

Replace the plain processed_buffer[] array with a packed scan struct
containing the channel data and an aligned_s64 timestamp field. This
satisfies the alignment requirement for iio_push_to_buffers_with_timestamp()
and ensures correct layout when the IIO core appends the timestamp.

Add IIO_CHAN_SOFT_TIMESTAMP entries to both max30102_channels[] and
max30105_channels[] so the IIO core knows to expect a timestamp slot in
the scan buffer.

Capture iio_get_time_ns() per FIFO sample read and pass it to
iio_push_to_buffers_with_timestamp() so userspace consumers receive
timing information alongside the intensity data.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/iio/health/max30102.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/health/max30102.c b/drivers/iio/health/max30102.c
index 47da44efd68b..447c4de8ff4b 100644
--- a/drivers/iio/health/max30102.c
+++ b/drivers/iio/health/max30102.c
@@ -24,6 +24,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/kfifo_buf.h>
+#include <linux/iio/trigger_consumer.h>
 
 #define MAX30102_DRV_NAME	"max30102"
 #define MAX30102_PART_NUMBER	0x15
@@ -106,8 +107,11 @@ struct max30102_data {
 	struct regmap *regmap;
 	enum max30102_chip_id chip_id;
 
-	u8 buffer[12];
-	__be32 processed_buffer[3]; /* 3 x 18-bit (padded to 32-bits) */
+	u8 buffer[12] __aligned(IIO_DMA_MINALIGN);
+	struct {
+		__be32 channels[3]; /* 3 x 18-bit (padded to 32-bits) */
+		aligned_s64 ts;
+	} scan;
 };
 
 static const struct regmap_config max30102_regmap_config = {
@@ -152,6 +156,7 @@ static const struct iio_chan_spec max30102_channels[] = {
 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 		.scan_index = -1,
 	},
+	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
 static const struct iio_chan_spec max30105_channels[] = {
@@ -164,6 +169,7 @@ static const struct iio_chan_spec max30105_channels[] = {
 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 		.scan_index = -1,
 	},
+	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 static int max30102_set_power(struct max30102_data *data, bool en)
@@ -253,7 +259,7 @@ static inline int max30102_fifo_count(struct max30102_data *data)
 }
 
 #define MAX30102_COPY_DATA(i) \
-	memcpy(&data->processed_buffer[(i)], \
+	memcpy(&data->scan.channels[(i)], \
 	       &buffer[(i) * MAX30102_REG_FIFO_DATA_BYTES], \
 	       MAX30102_REG_FIFO_DATA_BYTES)
 
@@ -298,11 +304,13 @@ static irqreturn_t max30102_interrupt_handler(int irq, void *private)
 	mutex_lock(&data->lock);
 
 	while (cnt || (cnt = max30102_fifo_count(data)) > 0) {
+		s64 ts = iio_get_time_ns(indio_dev);
+
 		ret = max30102_read_measurement(data, measurements);
 		if (ret)
 			break;
 
-		iio_push_to_buffers(data->indio_dev, data->processed_buffer);
+		iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, ts);
 		cnt--;
 	}
 
-- 
2.54.0.windows.1
Re: [PATCH] iio: health: max30102: Add timestamp to FIFO data
Posted by Jonathan Cameron 3 weeks, 6 days ago 
On Thu, 14 May 2026 23:03:09 +0300
Md Shofiqul Islam <shofiqtest@gmail.com> wrote:

There are various things going on here - that patch title
isn't enough.


> The I2C DMA read buffer was not aligned to IIO_DMA_MINALIGN, which
> can cause issues on platforms with strict DMA alignment requirements.
> Fix this by annotating the buffer field with __aligned(IIO_DMA_MINALIGN).

Nope. Look into the requirements of i2c (basically it doesn't have
any as it always bounces the data unless you go to considerable effort
to tell it to not do so!)

I've kind of lost track but didn't I explain why timestamps and fifos
are an awful lot more complex than this!  This is an absolute non starter.
Also the max30102 is a serious weird device that runs with a custom
userspace. You can't get pulse readings without some nasty (not particularly
public) maths.

Generally don't add features to a driver you aren't using - they
end up as dead code.

Jonathan


> 
> Replace the plain processed_buffer[] array with a packed scan struct
> containing the channel data and an aligned_s64 timestamp field. This
> satisfies the alignment requirement for iio_push_to_buffers_with_timestamp()
> and ensures correct layout when the IIO core appends the timestamp.
> 
> Add IIO_CHAN_SOFT_TIMESTAMP entries to both max30102_channels[] and
> max30105_channels[] so the IIO core knows to expect a timestamp slot in
> the scan buffer.
> 
> Capture iio_get_time_ns() per FIFO sample read and pass it to
> iio_push_to_buffers_with_timestamp() so userspace consumers receive
> timing information alongside the intensity data.
> 
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> ---
>  drivers/iio/health/max30102.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/health/max30102.c b/drivers/iio/health/max30102.c
> index 47da44efd68b..447c4de8ff4b 100644
> --- a/drivers/iio/health/max30102.c
> +++ b/drivers/iio/health/max30102.c
> @@ -24,6 +24,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/kfifo_buf.h>
> +#include <linux/iio/trigger_consumer.h>
>  
>  #define MAX30102_DRV_NAME	"max30102"
>  #define MAX30102_PART_NUMBER	0x15
> @@ -106,8 +107,11 @@ struct max30102_data {
>  	struct regmap *regmap;
>  	enum max30102_chip_id chip_id;
>  
> -	u8 buffer[12];
> -	__be32 processed_buffer[3]; /* 3 x 18-bit (padded to 32-bits) */
> +	u8 buffer[12] __aligned(IIO_DMA_MINALIGN);
> +	struct {
> +		__be32 channels[3]; /* 3 x 18-bit (padded to 32-bits) */
> +		aligned_s64 ts;

Where does the timestamp end up if only 1 or 2 channels are enabled?
(or indeed the device only has 2 channels).
The answer to that question will tell you why (if we were doing this)
it should be an array.


> +	} scan;
>  };

> @@ -298,11 +304,13 @@ static irqreturn_t max30102_interrupt_handler(int irq, void *private)
>  	mutex_lock(&data->lock);
>  
>  	while (cnt || (cnt = max30102_fifo_count(data)) > 0) {
> +		s64 ts = iio_get_time_ns(indio_dev);

So all the samples get timestamps near each other. Nope. They should
have the timestamps that reflect when they enter the fifo. That's hard.

> +
>  		ret = max30102_read_measurement(data, measurements);
>  		if (ret)
>  			break;
>  
> -		iio_push_to_buffers(data->indio_dev, data->processed_buffer);
> +		iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, ts);
>  		cnt--;
>  	}
>
Re: [PATCH] iio: health: max30102: Add timestamp to FIFO data
Posted by Md Shofiqul Islam 3 weeks, 5 days ago 
On Fri, 15 May 2026 16:04:34 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> Generally don't add features to a driver you aren't using - they
> end up as dead code.

Understood. Dropping this patch. The scan_index = -1 design and the
per-sample timestamp problem both make this a non-starter without a
much more substantial rework.

Thank you for the review.

Md Shofiqul Islam

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.