drivers/iio/adc/ti-ads7950.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access
when writing the timestamp in the rx_buf.
The previous implementation would have been fine on architectures that
support 4-byte alignment of 64-bit integers but could cause issues on
architectures that require 8-byte alignment.
Fixes: 902c4b2446d4 ("iio: adc: New driver for TI ADS7950 chips")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
Since we were looking at this driver, I was going to convert this to use
IIO_DECLARE_DMA_BUFFER_WITH_TS() but then I noticed that we actually
have an unaligned access problem with the timestamp since we are
ignoring the first two elements in the rx_buf when pushing the data to
the buffer.
Unfortunately, this will cause a merge conflict with the series Dmitry
is working on. I don't think there is any rush to get this backported
since no one has reported a crash from unaligned access. Since fixes
should go before improvements, we could apply this to iio/togreg then
Dmitry can rebase his series on top of it.
---
Changes in v2:
- use sizeof(*st->rx_buf)
- Link to v1: https://patch.msgid.link/20260307-iio-adc-ti-ads7950-declare-dma-buffer-v1-1-79b1309338df@baylibre.com
---
drivers/iio/adc/ti-ads7950.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index cdc624889559..418452aaca81 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -47,8 +47,6 @@
#define TI_ADS7950_MAX_CHAN 16
#define TI_ADS7950_NUM_GPIOS 4
-#define TI_ADS7950_TIMESTAMP_SIZE (sizeof(int64_t) / sizeof(__be16))
-
/* val = value, dec = left shift, bits = number of bits of the mask */
#define TI_ADS7950_EXTRACT(val, dec, bits) \
(((val) >> (dec)) & ((1 << (bits)) - 1))
@@ -105,8 +103,7 @@ struct ti_ads7950_state {
* DMA (thus cache coherency maintenance) may require the
* transfer buffers to live in their own cache lines.
*/
- u16 rx_buf[TI_ADS7950_MAX_CHAN + 2 + TI_ADS7950_TIMESTAMP_SIZE]
- __aligned(IIO_DMA_MINALIGN);
+ u16 rx_buf[TI_ADS7950_MAX_CHAN + 2] __aligned(IIO_DMA_MINALIGN);
u16 tx_buf[TI_ADS7950_MAX_CHAN + 2];
u16 single_tx;
u16 single_rx;
@@ -313,8 +310,10 @@ static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
if (ret < 0)
goto out;
- iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2],
- iio_get_time_ns(indio_dev));
+ iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
+ sizeof(*st->rx_buf) *
+ TI_ADS7950_MAX_CHAN,
+ iio_get_time_ns(indio_dev));
out:
mutex_unlock(&st->slock);
---
base-commit: 79a86a6cc3669416a21fef32d0767d39ba84b3aa
change-id: 20260307-iio-adc-ti-ads7950-declare-dma-buffer-9d5269ffa8bb
Best regards,
--
David Lechner <dlechner@baylibre.com>On Sat, 14 Mar 2026 16:12:24 -0500
David Lechner <dlechner@baylibre.com> wrote:
> Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access
> when writing the timestamp in the rx_buf.
>
> The previous implementation would have been fine on architectures that
> support 4-byte alignment of 64-bit integers but could cause issues on
> architectures that require 8-byte alignment.
>
> Fixes: 902c4b2446d4 ("iio: adc: New driver for TI ADS7950 chips")
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> Since we were looking at this driver, I was going to convert this to use
> IIO_DECLARE_DMA_BUFFER_WITH_TS() but then I noticed that we actually
> have an unaligned access problem with the timestamp since we are
> ignoring the first two elements in the rx_buf when pushing the data to
> the buffer.
>
> Unfortunately, this will cause a merge conflict with the series Dmitry
> is working on. I don't think there is any rush to get this backported
> since no one has reported a crash from unaligned access. Since fixes
> should go before improvements, we could apply this to iio/togreg then
> Dmitry can rebase his series on top of it.
I've done that. Applied to the togreg branch of iio.git pushed out as
testing etc. I've also marked it for stable so it gets picked up in the long
run.
Thanks,
Jonathan
> ---
> Changes in v2:
> - use sizeof(*st->rx_buf)
> - Link to v1: https://patch.msgid.link/20260307-iio-adc-ti-ads7950-declare-dma-buffer-v1-1-79b1309338df@baylibre.com
> ---
> drivers/iio/adc/ti-ads7950.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index cdc624889559..418452aaca81 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -47,8 +47,6 @@
> #define TI_ADS7950_MAX_CHAN 16
> #define TI_ADS7950_NUM_GPIOS 4
>
> -#define TI_ADS7950_TIMESTAMP_SIZE (sizeof(int64_t) / sizeof(__be16))
> -
> /* val = value, dec = left shift, bits = number of bits of the mask */
> #define TI_ADS7950_EXTRACT(val, dec, bits) \
> (((val) >> (dec)) & ((1 << (bits)) - 1))
> @@ -105,8 +103,7 @@ struct ti_ads7950_state {
> * DMA (thus cache coherency maintenance) may require the
> * transfer buffers to live in their own cache lines.
> */
> - u16 rx_buf[TI_ADS7950_MAX_CHAN + 2 + TI_ADS7950_TIMESTAMP_SIZE]
> - __aligned(IIO_DMA_MINALIGN);
> + u16 rx_buf[TI_ADS7950_MAX_CHAN + 2] __aligned(IIO_DMA_MINALIGN);
> u16 tx_buf[TI_ADS7950_MAX_CHAN + 2];
> u16 single_tx;
> u16 single_rx;
> @@ -313,8 +310,10 @@ static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
> if (ret < 0)
> goto out;
>
> - iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2],
> - iio_get_time_ns(indio_dev));
> + iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
> + sizeof(*st->rx_buf) *
> + TI_ADS7950_MAX_CHAN,
> + iio_get_time_ns(indio_dev));
>
> out:
> mutex_unlock(&st->slock);
>
> ---
> base-commit: 79a86a6cc3669416a21fef32d0767d39ba84b3aa
> change-id: 20260307-iio-adc-ti-ads7950-declare-dma-buffer-9d5269ffa8bb
>
> Best regards,
> --
> David Lechner <dlechner@baylibre.com>
>
On Sat, Mar 21, 2026 at 09:05:39PM +0000, Jonathan Cameron wrote:
> On Sat, 14 Mar 2026 16:12:24 -0500
> David Lechner <dlechner@baylibre.com> wrote:
>
> > Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access
> > when writing the timestamp in the rx_buf.
> >
> > The previous implementation would have been fine on architectures that
> > support 4-byte alignment of 64-bit integers but could cause issues on
> > architectures that require 8-byte alignment.
> >
> > Fixes: 902c4b2446d4 ("iio: adc: New driver for TI ADS7950 chips")
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> > Since we were looking at this driver, I was going to convert this to use
> > IIO_DECLARE_DMA_BUFFER_WITH_TS() but then I noticed that we actually
> > have an unaligned access problem with the timestamp since we are
> > ignoring the first two elements in the rx_buf when pushing the data to
> > the buffer.
> >
> > Unfortunately, this will cause a merge conflict with the series Dmitry
> > is working on. I don't think there is any rush to get this backported
> > since no one has reported a crash from unaligned access. Since fixes
> > should go before improvements, we could apply this to iio/togreg then
> > Dmitry can rebase his series on top of it.
> I've done that. Applied to the togreg branch of iio.git pushed out as
> testing etc. I've also marked it for stable so it gets picked up in the long
> run.
Cool, I'll rebase and send out my patches as soon as this hits
linux-next.
Thanks.
--
Dmitry
On Sun, Mar 22, 2026 at 03:30:21PM -0700, Dmitry Torokhov wrote:
> On Sat, Mar 21, 2026 at 09:05:39PM +0000, Jonathan Cameron wrote:
> > On Sat, 14 Mar 2026 16:12:24 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >
> > > Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access
> > > when writing the timestamp in the rx_buf.
> > >
> > > The previous implementation would have been fine on architectures that
> > > support 4-byte alignment of 64-bit integers but could cause issues on
> > > architectures that require 8-byte alignment.
> > >
> > > Fixes: 902c4b2446d4 ("iio: adc: New driver for TI ADS7950 chips")
> > > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > > ---
> > > Since we were looking at this driver, I was going to convert this to use
> > > IIO_DECLARE_DMA_BUFFER_WITH_TS() but then I noticed that we actually
> > > have an unaligned access problem with the timestamp since we are
> > > ignoring the first two elements in the rx_buf when pushing the data to
> > > the buffer.
> > >
> > > Unfortunately, this will cause a merge conflict with the series Dmitry
> > > is working on. I don't think there is any rush to get this backported
> > > since no one has reported a crash from unaligned access. Since fixes
> > > should go before improvements, we could apply this to iio/togreg then
> > > Dmitry can rebase his series on top of it.
> > I've done that. Applied to the togreg branch of iio.git pushed out as
> > testing etc. I've also marked it for stable so it gets picked up in the long
> > run.
>
> Cool, I'll rebase and send out my patches as soon as this hits
> linux-next.
Hm, I only see it in "testing", not in "togreg"...
--
Dmitry
On Wed, 25 Mar 2026 07:56:31 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Sun, Mar 22, 2026 at 03:30:21PM -0700, Dmitry Torokhov wrote:
> > On Sat, Mar 21, 2026 at 09:05:39PM +0000, Jonathan Cameron wrote:
> > > On Sat, 14 Mar 2026 16:12:24 -0500
> > > David Lechner <dlechner@baylibre.com> wrote:
> > >
> > > > Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access
> > > > when writing the timestamp in the rx_buf.
> > > >
> > > > The previous implementation would have been fine on architectures that
> > > > support 4-byte alignment of 64-bit integers but could cause issues on
> > > > architectures that require 8-byte alignment.
> > > >
> > > > Fixes: 902c4b2446d4 ("iio: adc: New driver for TI ADS7950 chips")
> > > > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > > > ---
> > > > Since we were looking at this driver, I was going to convert this to use
> > > > IIO_DECLARE_DMA_BUFFER_WITH_TS() but then I noticed that we actually
> > > > have an unaligned access problem with the timestamp since we are
> > > > ignoring the first two elements in the rx_buf when pushing the data to
> > > > the buffer.
> > > >
> > > > Unfortunately, this will cause a merge conflict with the series Dmitry
> > > > is working on. I don't think there is any rush to get this backported
> > > > since no one has reported a crash from unaligned access. Since fixes
> > > > should go before improvements, we could apply this to iio/togreg then
> > > > Dmitry can rebase his series on top of it.
> > > I've done that. Applied to the togreg branch of iio.git pushed out as
> > > testing etc. I've also marked it for stable so it gets picked up in the long
> > > run.
> >
> > Cool, I'll rebase and send out my patches as soon as this hits
> > linux-next.
>
> Hm, I only see it in "testing", not in "togreg"...
>
Should be there now. I was waiting for build bots to take a first look.
They were clean so now pushed out for picking up by linux-next.
J
On Sat, Mar 14, 2026 at 04:12:24PM -0500, David Lechner wrote: > Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access > when writing the timestamp in the rx_buf. > > The previous implementation would have been fine on architectures that > support 4-byte alignment of 64-bit integers but could cause issues on > architectures that require 8-byte alignment. ... > + u16 rx_buf[TI_ADS7950_MAX_CHAN + 2] __aligned(IIO_DMA_MINALIGN); > u16 tx_buf[TI_ADS7950_MAX_CHAN + 2]; ... > - iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2], > - iio_get_time_ns(indio_dev)); > + iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2], > + sizeof(*st->rx_buf) * > + TI_ADS7950_MAX_CHAN, Hmm... Wouldn't this benefit from array_size() macro? > + iio_get_time_ns(indio_dev)); -- With Best Regards, Andy Shevchenko
On 3/16/26 6:26 AM, Andy Shevchenko wrote: > On Sat, Mar 14, 2026 at 04:12:24PM -0500, David Lechner wrote: >> Use iio_push_to_buffers_with_ts_unaligned() to avoid unaligned access >> when writing the timestamp in the rx_buf. >> >> The previous implementation would have been fine on architectures that >> support 4-byte alignment of 64-bit integers but could cause issues on >> architectures that require 8-byte alignment. > > ... > >> + u16 rx_buf[TI_ADS7950_MAX_CHAN + 2] __aligned(IIO_DMA_MINALIGN); >> u16 tx_buf[TI_ADS7950_MAX_CHAN + 2]; > > ... > >> - iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2], >> - iio_get_time_ns(indio_dev)); >> + iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2], >> + sizeof(*st->rx_buf) * >> + TI_ADS7950_MAX_CHAN, > > Hmm... Wouldn't this benefit from array_size() macro? I don't think so. This is a compile-time constant. array_size() is a function that handles overflow, and there is no risk of overflow since it is a constant. > >> + iio_get_time_ns(indio_dev)); >
© 2016 - 2026 Red Hat, Inc.