[PATCH 0/8] iio: more timestamp alignment

[PATCH 0/8] iio: more timestamp alignment

[David Lechner]

Wile reviewing [1], I noticed a few more cases where we can use
aligned_s64 or need __aligned(8) on data structures used with
iio_push_to_buffers_with_timestamp().

[1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
David Lechner (8):
      iio: adc: dln2-adc: use aligned_s64 for timestamp
      iio: adc: mt6360-adc: use aligned_s64 for timestamp
      iio: addac: ad74413r: use aligned_s64 for timestamp
      iio: chemical: pms7003: use aligned_s64 for timestamp
      iio: chemical: sps30: use aligned_s64 for timestamp
      iio: imu: adis16550: align buffers for timestamp
      iio: imu: inv_mpu6050: align buffer for timestamp
      iio: pressure: mprls0025pa: use aligned_s64 for timestamp

 drivers/iio/accel/bmc150-accel.h           | 2 +-
 drivers/iio/adc/dln2-adc.c                 | 2 +-
 drivers/iio/adc/mt6360-adc.c               | 4 ++--
 drivers/iio/addac/ad74413r.c               | 5 +++--
 drivers/iio/chemical/pms7003.c             | 5 +++--
 drivers/iio/chemical/sps30.c               | 2 +-
 drivers/iio/imu/adis16550.c                | 2 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
 drivers/iio/pressure/mprls0025pa.h         | 2 +-
 9 files changed, 14 insertions(+), 12 deletions(-)
---
base-commit: 3159d40a2ca0ae14e69e1cae8b12f04c933d0445
change-id: 20250416-iio-more-timestamp-alignment-6c6c6a87ebda

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

[PATCH 1/8] iio: adc: dln2-adc: use aligned_s64 for timestamp

[David Lechner]

Follow the pattern of other drivers and use aligned_s64 for the
timestamp. This will ensure the struct itself it also 8-byte aligned.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/dln2-adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c
index a1e48a756a7b519105393f77c4aebde1f2f85d50..359e26e3f5bcfe16d723f621bdfc01df2dfcf6a9 100644
--- a/drivers/iio/adc/dln2-adc.c
+++ b/drivers/iio/adc/dln2-adc.c
@@ -466,7 +466,7 @@ static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct {
 		__le16 values[DLN2_ADC_MAX_CHANNELS];
-		int64_t timestamp_space;
+		aligned_s64 timestamp_space;
 	} data;
 	struct dln2_adc_get_all_vals dev_data;
 	struct dln2_adc *dln2 = iio_priv(indio_dev);

-- 
2.43.0

[PATCH 2/8] iio: adc: mt6360-adc: use aligned_s64 for timestamp

[David Lechner]

Follow the pattern of other drivers and use aligned_s64 for the
timestamp. This will ensure that the timestamp is correctly aligned on
all architectures. It also ensures that the struct itself it also 8-byte
aligned so we can drop the explicit __aligned(8) attribute.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/mt6360-adc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c
index 4eb2455d6ffacb8f09a404df4490b5a11e49660d..f8e98b6fa7e923c6b73bedf9ca1c466e7a9c3c47 100644
--- a/drivers/iio/adc/mt6360-adc.c
+++ b/drivers/iio/adc/mt6360-adc.c
@@ -263,8 +263,8 @@ static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p)
 	struct mt6360_adc_data *mad = iio_priv(indio_dev);
 	struct {
 		u16 values[MT6360_CHAN_MAX];
-		int64_t timestamp;
-	} data __aligned(8);
+		aligned_s64 timestamp;
+	} data;
 	int i = 0, bit, val, ret;
 
 	memset(&data, 0, sizeof(data));

-- 
2.43.0

[PATCH 3/8] iio: addac: ad74413r: use aligned_s64 for timestamp

[David Lechner]

Follow the pattern of other drivers and use aligned_s64 for the
timestamp. Technically there was no issue here since
AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX == 16 and IIO_DMA_MINALIGN
is always a multiple of 8. But best to conform in case someone copies
this to new code and then tweaks something.

Also move the unaligned.h header while touching this since it was the
only one not in alphabetical order.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/addac/ad74413r.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index f0929616ab899cb374f00869787321eed4ccde16..a0bb1dbcb7ad9d02337d0990e5a3f90be7eaa4ac 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -4,7 +4,6 @@
  * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
  */
 
-#include <linux/unaligned.h>
 #include <linux/bitfield.h>
 #include <linux/cleanup.h>
 #include <linux/crc8.h>
@@ -24,6 +23,8 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
 
 #include <dt-bindings/iio/addac/adi,ad74413r.h>
 
@@ -84,7 +85,7 @@ struct ad74413r_state {
 	 */
 	struct {
 		u8 rx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX];
-		s64 timestamp;
+		aligned_s64 timestamp;
 	} adc_samples_buf __aligned(IIO_DMA_MINALIGN);
 
 	u8	adc_samples_tx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX];

-- 
2.43.0

[PATCH 4/8] iio: chemical: pms7003: use aligned_s64 for timestamp

[David Lechner]

Follow the pattern of other drivers and use aligned_s64 for the
timestamp. This will ensure that the timestamp is correctly aligned on
all architectures.

Also move the unaligned.h header while touching this since it was the
only one not in alphabetical order.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/chemical/pms7003.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/chemical/pms7003.c b/drivers/iio/chemical/pms7003.c
index d0bd94912e0a3492641acd955adbc2184f4a11b3..e05ce1f12065c65d14b66ab86e291fab47805dec 100644
--- a/drivers/iio/chemical/pms7003.c
+++ b/drivers/iio/chemical/pms7003.c
@@ -5,7 +5,6 @@
  * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
  */
 
-#include <linux/unaligned.h>
 #include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/errno.h>
@@ -19,6 +18,8 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/serdev.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
 
 #define PMS7003_DRIVER_NAME "pms7003"
 
@@ -76,7 +77,7 @@ struct pms7003_state {
 	/* Used to construct scan to push to the IIO buffer */
 	struct {
 		u16 data[3]; /* PM1, PM2P5, PM10 */
-		s64 ts;
+		aligned_s64 ts;
 	} scan;
 };
 

-- 
2.43.0

[PATCH 5/8] iio: chemical: sps30: use aligned_s64 for timestamp

[David Lechner]

Follow the pattern of other drivers and use aligned_s64 for the
timestamp. This will ensure that the timestamp is correctly aligned on
all architectures.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/chemical/sps30.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
index 6f4f2ba2c09d5e691df13bc11ca9e3a910d98dc8..a7888146188d09ddbf376b398ee24dab7f0e2611 100644
--- a/drivers/iio/chemical/sps30.c
+++ b/drivers/iio/chemical/sps30.c
@@ -108,7 +108,7 @@ static irqreturn_t sps30_trigger_handler(int irq, void *p)
 	int ret;
 	struct {
 		s32 data[4]; /* PM1, PM2P5, PM4, PM10 */
-		s64 ts;
+		aligned_s64 ts;
 	} scan;
 
 	mutex_lock(&state->lock);

-- 
2.43.0

[PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[David Lechner]

Align the buffers used with iio_push_to_buffers_with_timestamp() to
ensure the s64 timestamp is aligned to 8 bytes.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/accel/bmc150-accel.h | 2 +-
 drivers/iio/imu/adis16550.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel.h b/drivers/iio/accel/bmc150-accel.h
index 7a7baf52e5955b4cdaef86aeacf479459b76fe94..0079dc99b2c3fba927f73bb3ee8bdc0ea049833e 100644
--- a/drivers/iio/accel/bmc150-accel.h
+++ b/drivers/iio/accel/bmc150-accel.h
@@ -63,7 +63,7 @@ struct bmc150_accel_data {
 	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
 	struct mutex mutex;
 	u8 fifo_mode, watermark;
-	s16 buffer[8];
+	s16 buffer[8] __aligned(8);
 	/*
 	 * Ensure there is sufficient space and correct alignment for
 	 * the timestamp if enabled
diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
index b14ea8937c7f5a2123e4097dc5b8260492044d1b..28f0dbd0226cbea67bc6c87d892f7812f21e9304 100644
--- a/drivers/iio/imu/adis16550.c
+++ b/drivers/iio/imu/adis16550.c
@@ -836,7 +836,7 @@ static irqreturn_t adis16550_trigger_handler(int irq, void *p)
 	u16 dummy;
 	bool valid;
 	struct iio_poll_func *pf = p;
-	__be32 data[ADIS16550_MAX_SCAN_DATA];
+	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct adis16550 *st = iio_priv(indio_dev);
 	struct adis *adis = iio_device_get_drvdata(indio_dev);

-- 
2.43.0

[PATCH 7/8] iio: imu: inv_mpu6050: align buffer for timestamp

[David Lechner]

Align the buffer used with iio_push_to_buffers_with_timestamp() to
ensure the s64 timestamp is aligned to 8 bytes.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 3d3b27f28c9d1c94aba93678261ce0d63099e1dc..273196e647a2b5a4860e18cfa34a088c773540e4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -50,7 +50,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 	u16 fifo_count;
 	u32 fifo_period;
 	s64 timestamp;
-	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
+	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE] __aligned(8);
 	size_t i, nb;
 
 	mutex_lock(&st->lock);

-- 
2.43.0

[PATCH 8/8] iio: pressure: mprls0025pa: use aligned_s64 for timestamp

[David Lechner]

Follow the pattern of other drivers and use aligned_s64 for the
timestamp. This will ensure the struct itself it also 8-byte aligned.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/pressure/mprls0025pa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index 9d5c30afa9d69a6a606662aa7906a76347329cef..9fe9eb35e79d992b2a576e5d0af71113c6c47400 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -41,7 +41,7 @@ struct mpr_ops;
  */
 struct mpr_chan {
 	s32 pres;
-	s64 ts;
+	aligned_s64 ts;
 };
 
 enum mpr_func_id {

-- 
2.43.0

Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[Andy Shevchenko]

On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:
> Align the buffers used with iio_push_to_buffers_with_timestamp() to
> ensure the s64 timestamp is aligned to 8 bytes.
> 
>  drivers/iio/accel/bmc150-accel.h | 2 +-
>  drivers/iio/imu/adis16550.c      | 2 +-

Looks like a stray squash of the two independent commits.

...

>  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
>  	struct mutex mutex;
>  	u8 fifo_mode, watermark;
> -	s16 buffer[8];
> +	s16 buffer[8] __aligned(8);

As for the code, would it be possible to convert to actually use a sturcture
rather than an array?

...

>  	struct iio_poll_func *pf = p;
> -	__be32 data[ADIS16550_MAX_SCAN_DATA];
> +	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct adis16550 *st = iio_priv(indio_dev);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 7/8] iio: imu: inv_mpu6050: align buffer for timestamp

[Andy Shevchenko]

On Thu, Apr 17, 2025 at 11:52:39AM -0500, David Lechner wrote:
> Align the buffer used with iio_push_to_buffers_with_timestamp() to
> ensure the s64 timestamp is aligned to 8 bytes.

Same question as per previous patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/8] iio: more timestamp alignment

[Andy Shevchenko]

On Thu, Apr 17, 2025 at 11:52:32AM -0500, David Lechner wrote:
> Wile reviewing [1], I noticed a few more cases where we can use
> aligned_s64 or need __aligned(8) on data structures used with
> iio_push_to_buffers_with_timestamp().
> 
> [1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/


Link: URL [1] :-)

This will help to maintainer with b4 as it manages tags.

> Signed-off-by: David Lechner <dlechner@baylibre.com>

Reviewed-by: Andy Shevchenko <andy@kernel.org>
for non-commented patches.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[David Lechner]

On 4/17/25 11:59 AM, Andy Shevchenko wrote:
> On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:
>> Align the buffers used with iio_push_to_buffers_with_timestamp() to
>> ensure the s64 timestamp is aligned to 8 bytes.
>>
>>  drivers/iio/accel/bmc150-accel.h | 2 +-
>>  drivers/iio/imu/adis16550.c      | 2 +-
> 
> Looks like a stray squash of the two independent commits.

Oops, sure enough.

> 
> ...
> 
>>  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
>>  	struct mutex mutex;
>>  	u8 fifo_mode, watermark;
>> -	s16 buffer[8];
>> +	s16 buffer[8] __aligned(8);
> 
> As for the code, would it be possible to convert to actually use a sturcture
> rather than an array?

I do personally prefer the struct pattern, but there are very many other drivers
using this buffer pattern that I was not tempted to try to start converting them.

> 
> ...
> 
>>  	struct iio_poll_func *pf = p;
>> -	__be32 data[ADIS16550_MAX_SCAN_DATA];
>> +	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
>>  	struct iio_dev *indio_dev = pf->indio_dev;
>>  	struct adis16550 *st = iio_priv(indio_dev);
> 
> Ditto.
> 

Re: [PATCH 0/8] iio: more timestamp alignment

[David Lechner]

On 4/17/25 12:01 PM, Andy Shevchenko wrote:
> On Thu, Apr 17, 2025 at 11:52:32AM -0500, David Lechner wrote:
>> Wile reviewing [1], I noticed a few more cases where we can use
>> aligned_s64 or need __aligned(8) on data structures used with
>> iio_push_to_buffers_with_timestamp().
>>
>> [1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/
> 
> 
> Link: URL [1] :-)
> 
> This will help to maintainer with b4 as it manages tags.

In this case, I don't want b4 to add this Link: to all patches, it is just
context for the cover letter and not so useful long-term.

> 
>> Signed-off-by: David Lechner <dlechner@baylibre.com>
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> for non-commented patches.
> 

Re: [PATCH 0/8] iio: more timestamp alignment

[Andy Shevchenko]

On Thu, Apr 17, 2025 at 12:16:53PM -0500, David Lechner wrote:
> On 4/17/25 12:01 PM, Andy Shevchenko wrote:
> > On Thu, Apr 17, 2025 at 11:52:32AM -0500, David Lechner wrote:
> >> Wile reviewing [1], I noticed a few more cases where we can use
> >> aligned_s64 or need __aligned(8) on data structures used with
> >> iio_push_to_buffers_with_timestamp().
> >>
> >> [1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/
> > 
> > 
> > Link: URL [1] :-)
> > 
> > This will help to maintainer with b4 as it manages tags.
> 
> In this case, I don't want b4 to add this Link: to all patches, it is just
> context for the cover letter and not so useful long-term.

Then tell it to not do that.
OTOH I dunno if it has a special filters for cover letter.

> > Reviewed-by: Andy Shevchenko <andy@kernel.org>
> > for non-commented patches.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/8] iio: adc: dln2-adc: use aligned_s64 for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 11:52:33 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure the struct itself it also 8-byte aligned.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/adc/dln2-adc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c
> index a1e48a756a7b519105393f77c4aebde1f2f85d50..359e26e3f5bcfe16d723f621bdfc01df2dfcf6a9 100644
> --- a/drivers/iio/adc/dln2-adc.c
> +++ b/drivers/iio/adc/dln2-adc.c
> @@ -466,7 +466,7 @@ static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct {
>  		__le16 values[DLN2_ADC_MAX_CHANNELS];
> -		int64_t timestamp_space;
> +		aligned_s64 timestamp_space;
Bug :( So needs a fixes tag ideally.  Fine to just reply with one
(or I might go digging if I get time).

>  	} data;
>  	struct dln2_adc_get_all_vals dev_data;
>  	struct dln2_adc *dln2 = iio_priv(indio_dev);
> 

Re: [PATCH 0/8] iio: more timestamp alignment

[Jonathan Cameron]

On Thu, 17 Apr 2025 11:52:32 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Wile reviewing [1], I noticed a few more cases where we can use
> aligned_s64 or need __aligned(8) on data structures used with
> iio_push_to_buffers_with_timestamp().
> 
> [1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>

I have a bunch of these in a 'too hard before sending that patch
pile' as well.  Hopefully you've covered many of those :) There
were definitely more bugs than I expected to see when I started that
effort!  I thought we'd tracked all these down long ago :(

Jonathan

> ---
> David Lechner (8):
>       iio: adc: dln2-adc: use aligned_s64 for timestamp
>       iio: adc: mt6360-adc: use aligned_s64 for timestamp
>       iio: addac: ad74413r: use aligned_s64 for timestamp
>       iio: chemical: pms7003: use aligned_s64 for timestamp
>       iio: chemical: sps30: use aligned_s64 for timestamp
>       iio: imu: adis16550: align buffers for timestamp
>       iio: imu: inv_mpu6050: align buffer for timestamp
>       iio: pressure: mprls0025pa: use aligned_s64 for timestamp
> 
>  drivers/iio/accel/bmc150-accel.h           | 2 +-
>  drivers/iio/adc/dln2-adc.c                 | 2 +-
>  drivers/iio/adc/mt6360-adc.c               | 4 ++--
>  drivers/iio/addac/ad74413r.c               | 5 +++--
>  drivers/iio/chemical/pms7003.c             | 5 +++--
>  drivers/iio/chemical/sps30.c               | 2 +-
>  drivers/iio/imu/adis16550.c                | 2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
>  drivers/iio/pressure/mprls0025pa.h         | 2 +-
>  9 files changed, 14 insertions(+), 12 deletions(-)
> ---
> base-commit: 3159d40a2ca0ae14e69e1cae8b12f04c933d0445
> change-id: 20250416-iio-more-timestamp-alignment-6c6c6a87ebda
> 
> Best regards,

Re: [PATCH 4/8] iio: chemical: pms7003: use aligned_s64 for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 11:52:36 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure that the timestamp is correctly aligned on
> all architectures.
> 
> Also move the unaligned.h header while touching this since it was the
> only one not in alphabetical order.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/chemical/pms7003.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/chemical/pms7003.c b/drivers/iio/chemical/pms7003.c
> index d0bd94912e0a3492641acd955adbc2184f4a11b3..e05ce1f12065c65d14b66ab86e291fab47805dec 100644
> --- a/drivers/iio/chemical/pms7003.c
> +++ b/drivers/iio/chemical/pms7003.c
> @@ -5,7 +5,6 @@
>   * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
>   */
>  
> -#include <linux/unaligned.h>
>  #include <linux/completion.h>
>  #include <linux/device.h>
>  #include <linux/errno.h>
> @@ -19,6 +18,8 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/serdev.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
>  
>  #define PMS7003_DRIVER_NAME "pms7003"
>  
> @@ -76,7 +77,7 @@ struct pms7003_state {
>  	/* Used to construct scan to push to the IIO buffer */
>  	struct {
>  		u16 data[3]; /* PM1, PM2P5, PM10 */
> -		s64 ts;
> +		aligned_s64 ts;

Bug I think..  So another one that really needs a fixes tag.
For all these we might be lucky with padding on the allocations
but we shouldn't really rely on that.

>  	} scan;
>  };
>  
> 

Re: [PATCH 5/8] iio: chemical: sps30: use aligned_s64 for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 11:52:37 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure that the timestamp is correctly aligned on
> all architectures.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/chemical/sps30.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> index 6f4f2ba2c09d5e691df13bc11ca9e3a910d98dc8..a7888146188d09ddbf376b398ee24dab7f0e2611 100644
> --- a/drivers/iio/chemical/sps30.c
> +++ b/drivers/iio/chemical/sps30.c
> @@ -108,7 +108,7 @@ static irqreturn_t sps30_trigger_handler(int irq, void *p)
>  	int ret;
>  	struct {
>  		s32 data[4]; /* PM1, PM2P5, PM4, PM10 */
> -		s64 ts;
> +		aligned_s64 ts;
Definitely a bug as we have no idea what is next on the stack
so fixes tag needed.

>  	} scan;
>  
>  	mutex_lock(&state->lock);
> 

Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 12:07:37 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/17/25 11:59 AM, Andy Shevchenko wrote:
> > On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:  
> >> Align the buffers used with iio_push_to_buffers_with_timestamp() to
> >> ensure the s64 timestamp is aligned to 8 bytes.
> >>
> >>  drivers/iio/accel/bmc150-accel.h | 2 +-
> >>  drivers/iio/imu/adis16550.c      | 2 +-  
> > 
> > Looks like a stray squash of the two independent commits.  
> 
> Oops, sure enough.
> 
> > 
> > ...
> >   
> >>  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> >>  	struct mutex mutex;
> >>  	u8 fifo_mode, watermark;
> >> -	s16 buffer[8];
> >> +	s16 buffer[8] __aligned(8);  
> > 
> > As for the code, would it be possible to convert to actually use a sturcture
> > rather than an array?  
> 
> I do personally prefer the struct pattern, but there are very many other drivers
> using this buffer pattern that I was not tempted to try to start converting them.

For drivers like this one where there is no room for the timestamp
to sit earlier for minimal channels I think it is worth that conversion
if we are touching them anyway. 

Jonathan


> 
> > 
> > ...
> >   
> >>  	struct iio_poll_func *pf = p;
> >> -	__be32 data[ADIS16550_MAX_SCAN_DATA];
> >> +	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
This one is more complex as you need to take the
available scan masks into account to figure out that it
always has enough channels enabled to ensure the timestamp
ends up in the 3rd 64 byte position.

We get 7 channels for each of the available scan masks.
So fine, but hard to see that, so this one I'd be less tempted
to change.


> >>  	struct iio_dev *indio_dev = pf->indio_dev;
> >>  	struct adis16550 *st = iio_priv(indio_dev);  
> > 
> > Ditto.
> >   
> 
> 

Re: [PATCH 7/8] iio: imu: inv_mpu6050: align buffer for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 20:00:05 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Thu, Apr 17, 2025 at 11:52:39AM -0500, David Lechner wrote:
> > Align the buffer used with iio_push_to_buffers_with_timestamp() to
> > ensure the s64 timestamp is aligned to 8 bytes.  
> 
> Same question as per previous patch.
> 
In this case I don't think we know the position of the timestamp
so a structure would be misleading.

The comment above the define certainly suggests it is variable..

/*
 * Maximum of 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8.
 * May be less if fewer channels are enabled, as long as the timestamp
 * remains 8 byte aligned
 */
#define INV_MPU6050_OUTPUT_DATA_SIZE         32

Re: [PATCH 0/8] iio: more timestamp alignment

[Jonathan Cameron]

On Thu, 17 Apr 2025 20:01:13 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Thu, Apr 17, 2025 at 11:52:32AM -0500, David Lechner wrote:
> > Wile reviewing [1], I noticed a few more cases where we can use
> > aligned_s64 or need __aligned(8) on data structures used with
> > iio_push_to_buffers_with_timestamp().
> > 
> > [1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/  
> 
> 
> Link: URL [1] :-)
In a cover letter? Not sure we care.  In general agree though.
> 
> This will help to maintainer with b4 as it manages tags.
> 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>  
Also no need to sign off on cover letters as they don't end up in git.
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> for non-commented patches.
> 

Re: [PATCH 8/8] iio: pressure: mprls0025pa: use aligned_s64 for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 11:52:40 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure the struct itself it also 8-byte aligned.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/pressure/mprls0025pa.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
> index 9d5c30afa9d69a6a606662aa7906a76347329cef..9fe9eb35e79d992b2a576e5d0af71113c6c47400 100644
> --- a/drivers/iio/pressure/mprls0025pa.h
> +++ b/drivers/iio/pressure/mprls0025pa.h
> @@ -41,7 +41,7 @@ struct mpr_ops;
>   */
>  struct mpr_chan {
>  	s32 pres;
> -	s64 ts;
> +	aligned_s64 ts;
>  };
Whilst you are here, no point in there being a named type for this.
Would you mind just pushing it into the struct mpr_data definition.

Might be a bug (I can't be bothered to work out the structure padding
to see if we end up with a gap after this) so fixes tag appropriate for
this one I think.

>  
>  enum mpr_func_id {
> 

Re: [PATCH 0/8] iio: more timestamp alignment

[Andy Shevchenko]

On Thu, Apr 17, 2025 at 06:47:16PM +0100, Jonathan Cameron wrote:
> On Thu, 17 Apr 2025 20:01:13 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> > On Thu, Apr 17, 2025 at 11:52:32AM -0500, David Lechner wrote:

...

> > > Signed-off-by: David Lechner <dlechner@baylibre.com>  
> Also no need to sign off on cover letters as they don't end up in git.

It depends if you use `b4 shazam` or not. I like that feature.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[David Lechner]

On 4/17/25 12:44 PM, Jonathan Cameron wrote:
> On Thu, 17 Apr 2025 12:07:37 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On 4/17/25 11:59 AM, Andy Shevchenko wrote:
>>> On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:  
>>>> Align the buffers used with iio_push_to_buffers_with_timestamp() to
>>>> ensure the s64 timestamp is aligned to 8 bytes.
>>>>
>>>>  drivers/iio/accel/bmc150-accel.h | 2 +-
>>>>  drivers/iio/imu/adis16550.c      | 2 +-  
>>>
>>> Looks like a stray squash of the two independent commits.  
>>
>> Oops, sure enough.
>>
>>>
>>> ...
>>>   
>>>>  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
>>>>  	struct mutex mutex;
>>>>  	u8 fifo_mode, watermark;
>>>> -	s16 buffer[8];
>>>> +	s16 buffer[8] __aligned(8);  
>>>
>>> As for the code, would it be possible to convert to actually use a sturcture
>>> rather than an array?  
>>
>> I do personally prefer the struct pattern, but there are very many other drivers
>> using this buffer pattern that I was not tempted to try to start converting them.
> 
> For drivers like this one where there is no room for the timestamp
> to sit earlier for minimal channels I think it is worth that conversion
> if we are touching them anyway. 
> 
> Jonathan
> 
There is actually a lot more wrong in this driver, so will save that for a
separate series.

Re: [PATCH 4/8] iio: chemical: pms7003: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 18:35 +0100, Jonathan Cameron wrote:
> On Thu, 17 Apr 2025 11:52:36 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > Follow the pattern of other drivers and use aligned_s64 for the
> > timestamp. This will ensure that the timestamp is correctly aligned on
> > all architectures.
> > 
> > Also move the unaligned.h header while touching this since it was the
> > only one not in alphabetical order.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> >  drivers/iio/chemical/pms7003.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/chemical/pms7003.c b/drivers/iio/chemical/pms7003.c
> > index
> > d0bd94912e0a3492641acd955adbc2184f4a11b3..e05ce1f12065c65d14b66ab86e291fab47805de
> > c 100644
> > --- a/drivers/iio/chemical/pms7003.c
> > +++ b/drivers/iio/chemical/pms7003.c
> > @@ -5,7 +5,6 @@
> >   * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
> >   */
> >  
> > -#include <linux/unaligned.h>
> >  #include <linux/completion.h>
> >  #include <linux/device.h>
> >  #include <linux/errno.h>
> > @@ -19,6 +18,8 @@
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> >  #include <linux/serdev.h>
> > +#include <linux/types.h>
> > +#include <linux/unaligned.h>
> >  
> >  #define PMS7003_DRIVER_NAME "pms7003"
> >  
> > @@ -76,7 +77,7 @@ struct pms7003_state {
> >  	/* Used to construct scan to push to the IIO buffer */
> >  	struct {
> >  		u16 data[3]; /* PM1, PM2P5, PM10 */
> > -		s64 ts;
> > +		aligned_s64 ts;
> 
> Bug I think..  So another one that really needs a fixes tag.
> For all these we might be lucky with padding on the allocations
> but we shouldn't really rely on that.

Agreed... We're likely not that lucky for x86-32

- Nuno Sá
> 
> >  	} scan;
> >  };
> >  
> > 
> 

Re: [PATCH 2/8] iio: adc: mt6360-adc: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure that the timestamp is correctly aligned on
> all architectures. It also ensures that the struct itself it also 8-byte
> aligned so we can drop the explicit __aligned(8) attribute.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/mt6360-adc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c
> index
> 4eb2455d6ffacb8f09a404df4490b5a11e49660d..f8e98b6fa7e923c6b73bedf9ca1c466e7a9c3c47
> 100644
> --- a/drivers/iio/adc/mt6360-adc.c
> +++ b/drivers/iio/adc/mt6360-adc.c
> @@ -263,8 +263,8 @@ static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p)
>  	struct mt6360_adc_data *mad = iio_priv(indio_dev);
>  	struct {
>  		u16 values[MT6360_CHAN_MAX];
> -		int64_t timestamp;
> -	} data __aligned(8);
> +		aligned_s64 timestamp;
> +	} data;
>  	int i = 0, bit, val, ret;
>  
>  	memset(&data, 0, sizeof(data));
> 

Re: [PATCH 3/8] iio: addac: ad74413r: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. Technically there was no issue here since
> AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX == 16 and IIO_DMA_MINALIGN
> is always a multiple of 8. But best to conform in case someone copies
> this to new code and then tweaks something.
> 
> Also move the unaligned.h header while touching this since it was the
> only one not in alphabetical order.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/addac/ad74413r.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index
> f0929616ab899cb374f00869787321eed4ccde16..a0bb1dbcb7ad9d02337d0990e5a3f90be7eaa4ac
> 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -4,7 +4,6 @@
>   * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
>   */
>  
> -#include <linux/unaligned.h>
>  #include <linux/bitfield.h>
>  #include <linux/cleanup.h>
>  #include <linux/crc8.h>
> @@ -24,6 +23,8 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
>  
>  #include <dt-bindings/iio/addac/adi,ad74413r.h>
>  
> @@ -84,7 +85,7 @@ struct ad74413r_state {
>  	 */
>  	struct {
>  		u8 rx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX];
> -		s64 timestamp;
> +		aligned_s64 timestamp;
>  	} adc_samples_buf __aligned(IIO_DMA_MINALIGN);
>  
>  	u8	adc_samples_tx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX];
> 

Re: [PATCH 1/8] iio: adc: dln2-adc: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure the struct itself it also 8-byte aligned.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

With the fixes tag:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/dln2-adc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c
> index
> a1e48a756a7b519105393f77c4aebde1f2f85d50..359e26e3f5bcfe16d723f621bdfc01df2dfcf6a9
> 100644
> --- a/drivers/iio/adc/dln2-adc.c
> +++ b/drivers/iio/adc/dln2-adc.c
> @@ -466,7 +466,7 @@ static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct {
>  		__le16 values[DLN2_ADC_MAX_CHANNELS];
> -		int64_t timestamp_space;
> +		aligned_s64 timestamp_space;
>  	} data;
>  	struct dln2_adc_get_all_vals dev_data;
>  	struct dln2_adc *dln2 = iio_priv(indio_dev);
> 

Re: [PATCH 4/8] iio: chemical: pms7003: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure that the timestamp is correctly aligned on
> all architectures.
> 
> Also move the unaligned.h header while touching this since it was the
> only one not in alphabetical order.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

With the tag:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/chemical/pms7003.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/chemical/pms7003.c b/drivers/iio/chemical/pms7003.c
> index
> d0bd94912e0a3492641acd955adbc2184f4a11b3..e05ce1f12065c65d14b66ab86e291fab47805dec
> 100644
> --- a/drivers/iio/chemical/pms7003.c
> +++ b/drivers/iio/chemical/pms7003.c
> @@ -5,7 +5,6 @@
>   * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
>   */
>  
> -#include <linux/unaligned.h>
>  #include <linux/completion.h>
>  #include <linux/device.h>
>  #include <linux/errno.h>
> @@ -19,6 +18,8 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/serdev.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
>  
>  #define PMS7003_DRIVER_NAME "pms7003"
>  
> @@ -76,7 +77,7 @@ struct pms7003_state {
>  	/* Used to construct scan to push to the IIO buffer */
>  	struct {
>  		u16 data[3]; /* PM1, PM2P5, PM10 */
> -		s64 ts;
> +		aligned_s64 ts;
>  	} scan;
>  };
>  
> 

Re: [PATCH 5/8] iio: chemical: sps30: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure that the timestamp is correctly aligned on
> all architectures.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

ditto

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/chemical/sps30.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> index
> 6f4f2ba2c09d5e691df13bc11ca9e3a910d98dc8..a7888146188d09ddbf376b398ee24dab7f0e2611
> 100644
> --- a/drivers/iio/chemical/sps30.c
> +++ b/drivers/iio/chemical/sps30.c
> @@ -108,7 +108,7 @@ static irqreturn_t sps30_trigger_handler(int irq, void *p)
>  	int ret;
>  	struct {
>  		s32 data[4]; /* PM1, PM2P5, PM4, PM10 */
> -		s64 ts;
> +		aligned_s64 ts;
>  	} scan;
>  
>  	mutex_lock(&state->lock);
> 

Re: [PATCH 8/8] iio: pressure: mprls0025pa: use aligned_s64 for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> Follow the pattern of other drivers and use aligned_s64 for the
> timestamp. This will ensure the struct itself it also 8-byte aligned.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

ditto

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/pressure/mprls0025pa.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/pressure/mprls0025pa.h
> b/drivers/iio/pressure/mprls0025pa.h
> index
> 9d5c30afa9d69a6a606662aa7906a76347329cef..9fe9eb35e79d992b2a576e5d0af71113c6c47400
> 100644
> --- a/drivers/iio/pressure/mprls0025pa.h
> +++ b/drivers/iio/pressure/mprls0025pa.h
> @@ -41,7 +41,7 @@ struct mpr_ops;
>   */
>  struct mpr_chan {
>  	s32 pres;
> -	s64 ts;
> +	aligned_s64 ts;
>  };
>  
>  enum mpr_func_id {
> 

Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[Nuno Sá]

On Thu, 2025-04-17 at 15:48 -0500, David Lechner wrote:
> On 4/17/25 12:44 PM, Jonathan Cameron wrote:
> > On Thu, 17 Apr 2025 12:07:37 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> > 
> > > On 4/17/25 11:59 AM, Andy Shevchenko wrote:
> > > > On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:  
> > > > > Align the buffers used with iio_push_to_buffers_with_timestamp() to
> > > > > ensure the s64 timestamp is aligned to 8 bytes.
> > > > > 
> > > > >  drivers/iio/accel/bmc150-accel.h | 2 +-
> > > > >  drivers/iio/imu/adis16550.c      | 2 +-  
> > > > 
> > > > Looks like a stray squash of the two independent commits.  
> > > 
> > > Oops, sure enough.
> > > 
> > > > 
> > > > ...
> > > >   
> > > > >  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> > > > >  	struct mutex mutex;
> > > > >  	u8 fifo_mode, watermark;
> > > > > -	s16 buffer[8];
> > > > > +	s16 buffer[8] __aligned(8);  
> > > > 
> > > > As for the code, would it be possible to convert to actually use a sturcture
> > > > rather than an array?  
> > > 
> > > I do personally prefer the struct pattern, but there are very many other
> > > drivers
> > > using this buffer pattern that I was not tempted to try to start converting
> > > them.
> > 
> > For drivers like this one where there is no room for the timestamp
> > to sit earlier for minimal channels I think it is worth that conversion
> > if we are touching them anyway. 
> > 
> > Jonathan
> > 
> There is actually a lot more wrong in this driver, so will save that for a
> separate series.
> 

I think one of them is actually leaking some memory into userspace...

- Nuno Sá

Re: [PATCH 7/8] iio: imu: inv_mpu6050: align buffer for timestamp

[Jean-Baptiste Maneyrol]

On Thu, 17 Apr 2025 19:46:00, Jonathan Cameron wrote:
> On Thu, 17 Apr 2025 20:00:05 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> 
> > On Thu, Apr 17, 2025 at 11:52:39AM -0500, David Lechner wrote:
> > > Align the buffer used with iio_push_to_buffers_with_timestamp() to
> > > ensure the s64 timestamp is aligned to 8 bytes.  
> > 
> > Same question as per previous patch.
> > 
> In this case I don't think we know the position of the timestamp
> so a structure would be misleading.
> 
> The comment above the define certainly suggests it is variable..

I confirm timestamp position is changing depending on channels enabled. It
can be at address 8, 16 or 24.

If there is only 1 sensor enabled (6 bytes of data), timestamp is at address
8. 2 sensors (12 bytes of data), timestamp will be at address 16. 3 sensors
for MPU-9xxx (19 bytes of data), timestamp will be at address 24.

If the buffer is aligned on 8 bytes, it will always work without any problem.

> 
> /*
>  * Maximum of 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8.
>  * May be less if fewer channels are enabled, as long as the timestamp
>  * remains 8 byte aligned
>  */
> #define INV_MPU6050_OUTPUT_DATA_SIZE         32

Thanks,
JB

Re: [PATCH 1/8] iio: adc: dln2-adc: use aligned_s64 for timestamp

[Jonathan Cameron]

On Fri, 18 Apr 2025 09:58:04 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> > Follow the pattern of other drivers and use aligned_s64 for the
> > timestamp. This will ensure the struct itself it also 8-byte aligned.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---  
> 
> With the fixes tag:
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
This one was in my series... Admittedly without a fixes tag.  :(

I've added a tag to that patch given I'd not pushed it out yet.
Fixes: 7c0299e879dd ("iio: adc: Add support for DLN2 ADC")

> 
> >  drivers/iio/adc/dln2-adc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c
> > index
> > a1e48a756a7b519105393f77c4aebde1f2f85d50..359e26e3f5bcfe16d723f621bdfc01df2dfcf6a9
> > 100644
> > --- a/drivers/iio/adc/dln2-adc.c
> > +++ b/drivers/iio/adc/dln2-adc.c
> > @@ -466,7 +466,7 @@ static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
> >  	struct iio_dev *indio_dev = pf->indio_dev;
> >  	struct {
> >  		__le16 values[DLN2_ADC_MAX_CHANNELS];
> > -		int64_t timestamp_space;
> > +		aligned_s64 timestamp_space;
> >  	} data;
> >  	struct dln2_adc_get_all_vals dev_data;
> >  	struct dln2_adc *dln2 = iio_priv(indio_dev);
> >   
> 

Re: [PATCH 2/8] iio: adc: mt6360-adc: use aligned_s64 for timestamp

[Jonathan Cameron]

On Fri, 18 Apr 2025 09:57:09 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> > Follow the pattern of other drivers and use aligned_s64 for the
> > timestamp. This will ensure that the timestamp is correctly aligned on
> > all architectures. It also ensures that the struct itself it also 8-byte
> > aligned so we can drop the explicit __aligned(8) attribute.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Applied.
> 
> >  drivers/iio/adc/mt6360-adc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c
> > index
> > 4eb2455d6ffacb8f09a404df4490b5a11e49660d..f8e98b6fa7e923c6b73bedf9ca1c466e7a9c3c47
> > 100644
> > --- a/drivers/iio/adc/mt6360-adc.c
> > +++ b/drivers/iio/adc/mt6360-adc.c
> > @@ -263,8 +263,8 @@ static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p)
> >  	struct mt6360_adc_data *mad = iio_priv(indio_dev);
> >  	struct {
> >  		u16 values[MT6360_CHAN_MAX];
> > -		int64_t timestamp;
> > -	} data __aligned(8);
> > +		aligned_s64 timestamp;
> > +	} data;
> >  	int i = 0, bit, val, ret;
> >  
> >  	memset(&data, 0, sizeof(data));
> >   
> 

Re: [PATCH 3/8] iio: addac: ad74413r: use aligned_s64 for timestamp

[Jonathan Cameron]

On Fri, 18 Apr 2025 09:57:38 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2025-04-17 at 11:52 -0500, David Lechner wrote:
> > Follow the pattern of other drivers and use aligned_s64 for the
> > timestamp. Technically there was no issue here since
> > AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX == 16 and IIO_DMA_MINALIGN
> > is always a multiple of 8. But best to conform in case someone copies
> > this to new code and then tweaks something.
> > 
> > Also move the unaligned.h header while touching this since it was the
> > only one not in alphabetical order.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Applied.
> 
> >  drivers/iio/addac/ad74413r.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> > index
> > f0929616ab899cb374f00869787321eed4ccde16..a0bb1dbcb7ad9d02337d0990e5a3f90be7eaa4ac
> > 100644
> > --- a/drivers/iio/addac/ad74413r.c
> > +++ b/drivers/iio/addac/ad74413r.c
> > @@ -4,7 +4,6 @@
> >   * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
> >   */
> >  
> > -#include <linux/unaligned.h>
> >  #include <linux/bitfield.h>
> >  #include <linux/cleanup.h>
> >  #include <linux/crc8.h>
> > @@ -24,6 +23,8 @@
> >  #include <linux/regmap.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/spi/spi.h>
> > +#include <linux/types.h>
> > +#include <linux/unaligned.h>
> >  
> >  #include <dt-bindings/iio/addac/adi,ad74413r.h>
> >  
> > @@ -84,7 +85,7 @@ struct ad74413r_state {
> >  	 */
> >  	struct {
> >  		u8 rx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX];
> > -		s64 timestamp;
> > +		aligned_s64 timestamp;
> >  	} adc_samples_buf __aligned(IIO_DMA_MINALIGN);
> >  
> >  	u8	adc_samples_tx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX];
> >   
> 

Re: [PATCH 4/8] iio: chemical: pms7003: use aligned_s64 for timestamp

[Jonathan Cameron]

On Fri, 18 Apr 2025 09:51:37 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2025-04-17 at 18:35 +0100, Jonathan Cameron wrote:
> > On Thu, 17 Apr 2025 11:52:36 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> > > Follow the pattern of other drivers and use aligned_s64 for the
> > > timestamp. This will ensure that the timestamp is correctly aligned on
> > > all architectures.
> > > 
> > > Also move the unaligned.h header while touching this since it was the
> > > only one not in alphabetical order.
> > > 
> > > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > > ---
> > >  drivers/iio/chemical/pms7003.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iio/chemical/pms7003.c b/drivers/iio/chemical/pms7003.c
> > > index
> > > d0bd94912e0a3492641acd955adbc2184f4a11b3..e05ce1f12065c65d14b66ab86e291fab47805de
> > > c 100644
> > > --- a/drivers/iio/chemical/pms7003.c
> > > +++ b/drivers/iio/chemical/pms7003.c
> > > @@ -5,7 +5,6 @@
> > >   * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
> > >   */
> > >  
> > > -#include <linux/unaligned.h>
> > >  #include <linux/completion.h>
> > >  #include <linux/device.h>
> > >  #include <linux/errno.h>
> > > @@ -19,6 +18,8 @@
> > >  #include <linux/module.h>
> > >  #include <linux/mutex.h>
> > >  #include <linux/serdev.h>
> > > +#include <linux/types.h>
> > > +#include <linux/unaligned.h>
> > >  
> > >  #define PMS7003_DRIVER_NAME "pms7003"
> > >  
> > > @@ -76,7 +77,7 @@ struct pms7003_state {
> > >  	/* Used to construct scan to push to the IIO buffer */
> > >  	struct {
> > >  		u16 data[3]; /* PM1, PM2P5, PM10 */
> > > -		s64 ts;
> > > +		aligned_s64 ts;  
> > 
> > Bug I think..  So another one that really needs a fixes tag.
> > For all these we might be lucky with padding on the allocations
> > but we shouldn't really rely on that.  
> 
> Agreed... We're likely not that lucky for x86-32
Applied with
Fixes: 13e945631c2f ("iio:chemical:pms7003: Fix timestamp alignment and prevent data leak.")
and +CC stable.
> 
> - Nuno Sá
> >   
> > >  	} scan;
> > >  };
> > >  
> > >   
> >   
> 
> 

Re: [PATCH 5/8] iio: chemical: sps30: use aligned_s64 for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 18:36:21 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Thu, 17 Apr 2025 11:52:37 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > Follow the pattern of other drivers and use aligned_s64 for the
> > timestamp. This will ensure that the timestamp is correctly aligned on
> > all architectures.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> >  drivers/iio/chemical/sps30.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> > index 6f4f2ba2c09d5e691df13bc11ca9e3a910d98dc8..a7888146188d09ddbf376b398ee24dab7f0e2611 100644
> > --- a/drivers/iio/chemical/sps30.c
> > +++ b/drivers/iio/chemical/sps30.c
> > @@ -108,7 +108,7 @@ static irqreturn_t sps30_trigger_handler(int irq, void *p)
> >  	int ret;
> >  	struct {
> >  		s32 data[4]; /* PM1, PM2P5, PM4, PM10 */
> > -		s64 ts;
> > +		aligned_s64 ts;  
> Definitely a bug as we have no idea what is next on the stack
> so fixes tag needed.
Applied with:
Fixes: a5bf6fdd19c3 ("iio:chemical:sps30: Fix timestamp alignment")


> 
> >  	} scan;
> >  
> >  	mutex_lock(&state->lock);
> >   
> 

Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[Jonathan Cameron]

On Thu, 17 Apr 2025 15:48:44 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/17/25 12:44 PM, Jonathan Cameron wrote:
> > On Thu, 17 Apr 2025 12:07:37 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> >> On 4/17/25 11:59 AM, Andy Shevchenko wrote:  
> >>> On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:    
> >>>> Align the buffers used with iio_push_to_buffers_with_timestamp() to
> >>>> ensure the s64 timestamp is aligned to 8 bytes.
> >>>>
> >>>>  drivers/iio/accel/bmc150-accel.h | 2 +-
> >>>>  drivers/iio/imu/adis16550.c      | 2 +-    
> >>>
> >>> Looks like a stray squash of the two independent commits.    
> >>
> >> Oops, sure enough.
> >>  
> >>>
> >>> ...
> >>>     
> >>>>  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> >>>>  	struct mutex mutex;
> >>>>  	u8 fifo_mode, watermark;
> >>>> -	s16 buffer[8];
> >>>> +	s16 buffer[8] __aligned(8);    
> >>>
> >>> As for the code, would it be possible to convert to actually use a sturcture
> >>> rather than an array?    
> >>
> >> I do personally prefer the struct pattern, but there are very many other drivers
> >> using this buffer pattern that I was not tempted to try to start converting them.  
> > 
> > For drivers like this one where there is no room for the timestamp
> > to sit earlier for minimal channels I think it is worth that conversion
> > if we are touching them anyway. 
> > 
> > Jonathan
> >   
> There is actually a lot more wrong in this driver, so will save that for a
> separate series.
> 
ok.  That is probably fair enough.
I'll not pick this up though given the smashing of 2 patches.

So this one will need a v2.

Jonathan

Re: [PATCH 0/8] iio: more timestamp alignment

[Jonathan Cameron]

On Thu, 17 Apr 2025 21:05:40 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Thu, Apr 17, 2025 at 06:47:16PM +0100, Jonathan Cameron wrote:
> > On Thu, 17 Apr 2025 20:01:13 +0300
> > Andy Shevchenko <andy@kernel.org> wrote:  
> > > On Thu, Apr 17, 2025 at 11:52:32AM -0500, David Lechner wrote:  
> 
> ...
> 
> > > > Signed-off-by: David Lechner <dlechner@baylibre.com>    
> > Also no need to sign off on cover letters as they don't end up in git.  
> 
> It depends if you use `b4 shazam` or not. I like that feature.
> 
Definitely don't add what you did here though in a reply!

That adds: 

    + Link: URL [1] :-)

Re: [PATCH 7/8] iio: imu: inv_mpu6050: align buffer for timestamp

[Jonathan Cameron]

On Fri, 18 Apr 2025 11:26:39 +0000
Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:

> On Thu, 17 Apr 2025 19:46:00, Jonathan Cameron wrote:
> > On Thu, 17 Apr 2025 20:00:05 +0300
> > Andy Shevchenko <andy@kernel.org> wrote:
> >   
> > > On Thu, Apr 17, 2025 at 11:52:39AM -0500, David Lechner wrote:  
> > > > Align the buffer used with iio_push_to_buffers_with_timestamp() to
> > > > ensure the s64 timestamp is aligned to 8 bytes.    
> > > 
> > > Same question as per previous patch.
> > >   
> > In this case I don't think we know the position of the timestamp
> > so a structure would be misleading.
> > 
> > The comment above the define certainly suggests it is variable..  
> 
> I confirm timestamp position is changing depending on channels enabled. It
> can be at address 8, 16 or 24.
> 
> If there is only 1 sensor enabled (6 bytes of data), timestamp is at address
> 8. 2 sensors (12 bytes of data), timestamp will be at address 16. 3 sensors
> for MPU-9xxx (19 bytes of data), timestamp will be at address 24.
> 
> If the buffer is aligned on 8 bytes, it will always work without any problem.
> 
> > 
> > /*
> >  * Maximum of 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8.
> >  * May be less if fewer channels are enabled, as long as the timestamp
> >  * remains 8 byte aligned
> >  */
> > #define INV_MPU6050_OUTPUT_DATA_SIZE         32  
> 
> Thanks,
> JB

I applied this one as it stands with fixes tag and +CC stable.

Fixes: 0829edc43e0a ("iio: imu: inv_mpu6050: read the full fifo when processing data")

I thought about seeing if all the cases that are fixes are separable enough
to take through togreg-fixes whilst the with_ts() series goes through togreg
in parallel.  I might see if that is doable easily.

Jonathan

Re: [PATCH 7/8] iio: imu: inv_mpu6050: align buffer for timestamp

[Jonathan Cameron]

On Fri, 18 Apr 2025 16:02:38 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 18 Apr 2025 11:26:39 +0000
> Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:
> 
> > On Thu, 17 Apr 2025 19:46:00, Jonathan Cameron wrote:  
> > > On Thu, 17 Apr 2025 20:00:05 +0300
> > > Andy Shevchenko <andy@kernel.org> wrote:
> > >     
> > > > On Thu, Apr 17, 2025 at 11:52:39AM -0500, David Lechner wrote:    
> > > > > Align the buffer used with iio_push_to_buffers_with_timestamp() to
> > > > > ensure the s64 timestamp is aligned to 8 bytes.      
> > > > 
> > > > Same question as per previous patch.
> > > >     
> > > In this case I don't think we know the position of the timestamp
> > > so a structure would be misleading.
> > > 
> > > The comment above the define certainly suggests it is variable..    
> > 
> > I confirm timestamp position is changing depending on channels enabled. It
> > can be at address 8, 16 or 24.
> > 
> > If there is only 1 sensor enabled (6 bytes of data), timestamp is at address
> > 8. 2 sensors (12 bytes of data), timestamp will be at address 16. 3 sensors
> > for MPU-9xxx (19 bytes of data), timestamp will be at address 24.
> > 
> > If the buffer is aligned on 8 bytes, it will always work without any problem.
> >   
> > > 
> > > /*
> > >  * Maximum of 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8.
> > >  * May be less if fewer channels are enabled, as long as the timestamp
> > >  * remains 8 byte aligned
> > >  */
> > > #define INV_MPU6050_OUTPUT_DATA_SIZE         32    
> > 
> > Thanks,
> > JB  
> 
> I applied this one as it stands with fixes tag and +CC stable.
> 
> Fixes: 0829edc43e0a ("iio: imu: inv_mpu6050: read the full fifo when processing data")
> 
> I thought about seeing if all the cases that are fixes are separable enough
> to take through togreg-fixes whilst the with_ts() series goes through togreg
> in parallel.  I might see if that is doable easily.
> 
I have done so and it seems fine as we didn't rename anything...

52d349884738 (HEAD -> fixes-togreg) iio: adc: ad7266: Fix potential timestamp alignment issue.
ffbc26bc91c1 iio: adc: ad7768-1: Fix insufficient alignment of timestamp.
5097eaae98e5 iio: adc: dln2: Use aligned_s64 for timestamp
1bb942287e05 iio: accel: adxl355: Make timestamp 64-bit aligned using aligned_s64
f79aeb6c631b iio: temp: maxim-thermocouple: Fix potential lack of DMA safe buffer.
6ffa69867405 iio: chemical: pms7003: use aligned_s64 for timestamp
bb49d940344b iio: chemical: sps30: use aligned_s64 for timestamp

Now on the fixes-togreg branch of iio.git.
> Jonathan

Re: [PATCH 0/8] iio: more timestamp alignment

[Jonathan Cameron]

On Thu, 17 Apr 2025 11:52:32 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Wile reviewing [1], I noticed a few more cases where we can use
> aligned_s64 or need __aligned(8) on data structures used with
> iio_push_to_buffers_with_timestamp().
> 
> [1]: https://lore.kernel.org/linux-iio/20250413103443.2420727-1-jic23@kernel.org/
I picked up via one or other branch all but 1 (as that was already fixed), 6 and 8

Jonathan

> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> David Lechner (8):
>       iio: adc: dln2-adc: use aligned_s64 for timestamp
>       iio: adc: mt6360-adc: use aligned_s64 for timestamp
>       iio: addac: ad74413r: use aligned_s64 for timestamp
>       iio: chemical: pms7003: use aligned_s64 for timestamp
>       iio: chemical: sps30: use aligned_s64 for timestamp
>       iio: imu: adis16550: align buffers for timestamp
>       iio: imu: inv_mpu6050: align buffer for timestamp
>       iio: pressure: mprls0025pa: use aligned_s64 for timestamp
> 
>  drivers/iio/accel/bmc150-accel.h           | 2 +-
>  drivers/iio/adc/dln2-adc.c                 | 2 +-
>  drivers/iio/adc/mt6360-adc.c               | 4 ++--
>  drivers/iio/addac/ad74413r.c               | 5 +++--
>  drivers/iio/chemical/pms7003.c             | 5 +++--
>  drivers/iio/chemical/sps30.c               | 2 +-
>  drivers/iio/imu/adis16550.c                | 2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
>  drivers/iio/pressure/mprls0025pa.h         | 2 +-
>  9 files changed, 14 insertions(+), 12 deletions(-)
> ---
> base-commit: 3159d40a2ca0ae14e69e1cae8b12f04c933d0445
> change-id: 20250416-iio-more-timestamp-alignment-6c6c6a87ebda
> 
> Best regards,