public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
@ 2026-03-14 21:12 David Lechner
  2026-03-16 11:26 ` Andy Shevchenko
  2026-03-21 21:05 ` Jonathan Cameron
  0 siblings, 2 replies; 7+ messages in thread
From: David Lechner @ 2026-03-14 21:12 UTC (permalink / raw)
  To: Jonathan Cameron, Nuno Sá, Andy Shevchenko
  Cc: Dmitry Torokhov, linux-iio, linux-kernel, David Lechner

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>


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

* Re: [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
  2026-03-14 21:12 [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned() David Lechner
@ 2026-03-16 11:26 ` Andy Shevchenko
  2026-03-16 14:58   ` David Lechner
  2026-03-21 21:05 ` Jonathan Cameron
  1 sibling, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-03-16 11:26 UTC (permalink / raw)
  To: David Lechner
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Dmitry Torokhov,
	linux-iio, linux-kernel

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



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

* Re: [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
  2026-03-16 11:26 ` Andy Shevchenko
@ 2026-03-16 14:58   ` David Lechner
  0 siblings, 0 replies; 7+ messages in thread
From: David Lechner @ 2026-03-16 14:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Dmitry Torokhov,
	linux-iio, linux-kernel

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));
> 


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

* Re: [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
  2026-03-14 21:12 [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned() David Lechner
  2026-03-16 11:26 ` Andy Shevchenko
@ 2026-03-21 21:05 ` Jonathan Cameron
  2026-03-22 22:30   ` Dmitry Torokhov
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-03-21 21:05 UTC (permalink / raw)
  To: David Lechner
  Cc: Nuno Sá, Andy Shevchenko, Dmitry Torokhov, linux-iio,
	linux-kernel

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>
> 


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

* Re: [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
  2026-03-21 21:05 ` Jonathan Cameron
@ 2026-03-22 22:30   ` Dmitry Torokhov
  2026-03-25 14:56     ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2026-03-22 22:30 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

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

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

* Re: [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
  2026-03-22 22:30   ` Dmitry Torokhov
@ 2026-03-25 14:56     ` Dmitry Torokhov
  2026-03-25 20:39       ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2026-03-25 14:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

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

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

* Re: [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned()
  2026-03-25 14:56     ` Dmitry Torokhov
@ 2026-03-25 20:39       ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-03-25 20:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

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


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

end of thread, other threads:[~2026-03-25 20:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-14 21:12 [PATCH v2] iio: adc: ti-ads7950: use iio_push_to_buffers_with_ts_unaligned() David Lechner
2026-03-16 11:26 ` Andy Shevchenko
2026-03-16 14:58   ` David Lechner
2026-03-21 21:05 ` Jonathan Cameron
2026-03-22 22:30   ` Dmitry Torokhov
2026-03-25 14:56     ` Dmitry Torokhov
2026-03-25 20:39       ` Jonathan Cameron

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