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.0On 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
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. >
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. > > > >
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.
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
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á
© 2016 - 2026 Red Hat, Inc.