public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: ti-ads7950: use spi_optimize_message()
@ 2026-04-11 22:13 David Lechner
  2026-04-14  9:54 ` Andy Shevchenko
  0 siblings, 1 reply; 5+ messages in thread
From: David Lechner @ 2026-04-11 22:13 UTC (permalink / raw)
  To: Jonathan Cameron, Nuno Sá, Andy Shevchenko
  Cc: linux-iio, linux-kernel, David Lechner

Use spi_optimize_message() to reduce CPU usage during buffered reads.

On hardware with support for SPI_CS_WORD, this reduced the CPU usage
of the threaded interrupt by about 5%. On hardware without support, this
should reduce CPU usage even more since it won't have to split the SPI
transfers each time the interrupt handler is called.

The update_scan_mode callback hand to be moved to the buffer preenable
callback since the SPI transfer mode can't be changed after
spi_optimize_message() has been called. (The buffer postenable callback
can't be used because it happens after the trigger is enabled, so the
SPI message needs to be optimized before that.)

The indent of ti_ads7950_read_raw is changed since there is no longer
anything else in the struct to align with since we removed
ti_ads7950_update_scan_mode.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ti-ads7950.c | 65 +++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 882b280d9e0b..39a074bce6d5 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -267,31 +267,6 @@ static const struct ti_ads7950_chip_info ti_ads7961_chip_info = {
 	.num_channels	= ARRAY_SIZE(ti_ads7961_channels),
 };
 
-/*
- * ti_ads7950_update_scan_mode() setup the spi transfer buffer for the new
- * scan mask
- */
-static int ti_ads7950_update_scan_mode(struct iio_dev *indio_dev,
-				       const unsigned long *active_scan_mask)
-{
-	struct ti_ads7950_state *st = iio_priv(indio_dev);
-	int i, cmd, len;
-
-	len = 0;
-	for_each_set_bit(i, active_scan_mask, indio_dev->num_channels) {
-		cmd = TI_ADS7950_MAN_CMD(TI_ADS7950_CR_CHAN(i));
-		st->tx_buf[len++] = cmd;
-	}
-
-	/* Data for the 1st channel is not returned until the 3rd transfer */
-	st->tx_buf[len++] = 0;
-	st->tx_buf[len++] = 0;
-
-	st->ring_xfer.len = len * 2;
-
-	return 0;
-}
-
 static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -375,8 +350,42 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
 }
 
 static const struct iio_info ti_ads7950_info = {
-	.read_raw		= &ti_ads7950_read_raw,
-	.update_scan_mode	= ti_ads7950_update_scan_mode,
+	.read_raw = &ti_ads7950_read_raw,
+};
+
+static int ti_ads7950_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ti_ads7950_state *st = iio_priv(indio_dev);
+	u32 len = 0;
+	u32 i;
+	u16 cmd;
+
+	for_each_set_bit(i, indio_dev->active_scan_mask, indio_dev->num_channels) {
+		cmd = TI_ADS7950_MAN_CMD(TI_ADS7950_CR_CHAN(i));
+		st->tx_buf[len++] = cmd;
+	}
+
+	/* Data for the 1st channel is not returned until the 3rd transfer */
+	st->tx_buf[len++] = 0;
+	st->tx_buf[len++] = 0;
+
+	st->ring_xfer.len = len * 2;
+
+	return spi_optimize_message(st->spi, &st->ring_msg);
+}
+
+static int ti_ads7950_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ti_ads7950_state *st = iio_priv(indio_dev);
+
+	spi_unoptimize_message(&st->ring_msg);
+
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops ti_ads7950_buffer_setup_ops = {
+	.preenable = ti_ads7950_buffer_preenable,
+	.postdisable = ti_ads7950_buffer_postdisable,
 };
 
 static int ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
@@ -573,7 +582,7 @@ static int ti_ads7950_probe(struct spi_device *spi)
 
 	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
 					      &ti_ads7950_trigger_handler,
-					      NULL);
+					      &ti_ads7950_buffer_setup_ops);
 	if (ret)
 		return dev_err_probe(&spi->dev, ret,
 				     "Failed to setup triggered buffer\n");

---
base-commit: 66672af7a095d89f082c5327f3b15bc2f93d558e
change-id: 20260411-iio-adc-ti-ads7950-spi-optimize-msg-6924969dc473
prerequisite-message-id: <20260405-ti-ads7950-facelift-v5-0-1f980ed3cf9e@gmail.com>
prerequisite-patch-id: 0606a4fc48f3fe7bf5f9156c93f434050898c309
prerequisite-patch-id: 1bb68480bcba4198e8b18b3ca7d8a0980fdd4bdd
prerequisite-patch-id: 551390239268e710bdb14ae2629e287fa751cb62
prerequisite-patch-id: 1a5f202fc1b16f2aee0368cccda2a521a9c8af19

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


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

* Re: [PATCH] iio: adc: ti-ads7950: use spi_optimize_message()
  2026-04-11 22:13 [PATCH] iio: adc: ti-ads7950: use spi_optimize_message() David Lechner
@ 2026-04-14  9:54 ` Andy Shevchenko
  2026-04-14 13:29   ` David Lechner
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2026-04-14  9:54 UTC (permalink / raw)
  To: David Lechner
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On Sat, Apr 11, 2026 at 05:13:33PM -0500, David Lechner wrote:
> Use spi_optimize_message() to reduce CPU usage during buffered reads.
> 
> On hardware with support for SPI_CS_WORD, this reduced the CPU usage
> of the threaded interrupt by about 5%. On hardware without support, this
> should reduce CPU usage even more since it won't have to split the SPI
> transfers each time the interrupt handler is called.
> 
> The update_scan_mode callback hand to be moved to the buffer preenable
> callback since the SPI transfer mode can't be changed after
> spi_optimize_message() has been called. (The buffer postenable callback
> can't be used because it happens after the trigger is enabled, so the
> SPI message needs to be optimized before that.)
> 
> The indent of ti_ads7950_read_raw is changed since there is no longer
> anything else in the struct to align with since we removed
> ti_ads7950_update_scan_mode.

Some of the func() are mentioned w/o parentheses and I got lost which one is
which. Also callbacks usually mentioned as .callback() (with a leading dot).

The second paragraph doesn't tell me clearly if there is a behaviour change
from user perspective.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH] iio: adc: ti-ads7950: use spi_optimize_message()
  2026-04-14  9:54 ` Andy Shevchenko
@ 2026-04-14 13:29   ` David Lechner
  2026-04-14 16:31     ` Andy Shevchenko
  0 siblings, 1 reply; 5+ messages in thread
From: David Lechner @ 2026-04-14 13:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On 4/14/26 4:54 AM, Andy Shevchenko wrote:
> On Sat, Apr 11, 2026 at 05:13:33PM -0500, David Lechner wrote:
>> Use spi_optimize_message() to reduce CPU usage during buffered reads.
>>
>> On hardware with support for SPI_CS_WORD, this reduced the CPU usage
>> of the threaded interrupt by about 5%. On hardware without support, this
>> should reduce CPU usage even more since it won't have to split the SPI
>> transfers each time the interrupt handler is called.
>>
>> The update_scan_mode callback hand to be moved to the buffer preenable

s/hand/had/

>> callback since the SPI transfer mode can't be changed after
>> spi_optimize_message() has been called. (The buffer postenable callback
>> can't be used because it happens after the trigger is enabled, so the
>> SPI message needs to be optimized before that.)
>>
>> The indent of ti_ads7950_read_raw is changed since there is no longer
>> anything else in the struct to align with since we removed
>> ti_ads7950_update_scan_mode.
> 
> Some of the func() are mentioned w/o parentheses and I got lost which one is
> which. Also callbacks usually mentioned as .callback() (with a leading dot).

I didn't put () in the last paragraph because I was talking about the function
pointer, not the function. I guess I missed update_scan_mode() though.

> 
> The second paragraph doesn't tell me clearly if there is a behaviour change
> from user perspective.
> 

It is not clear that the difference the user can notice is that there are
some CPU cycles freed up for other tasks?

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

* Re: [PATCH] iio: adc: ti-ads7950: use spi_optimize_message()
  2026-04-14 13:29   ` David Lechner
@ 2026-04-14 16:31     ` Andy Shevchenko
  2026-04-14 17:52       ` David Lechner
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2026-04-14 16:31 UTC (permalink / raw)
  To: David Lechner
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On Tue, Apr 14, 2026 at 08:29:29AM -0500, David Lechner wrote:
> On 4/14/26 4:54 AM, Andy Shevchenko wrote:
> > On Sat, Apr 11, 2026 at 05:13:33PM -0500, David Lechner wrote:
> >> Use spi_optimize_message() to reduce CPU usage during buffered reads.
> >>
> >> On hardware with support for SPI_CS_WORD, this reduced the CPU usage
> >> of the threaded interrupt by about 5%. On hardware without support, this
> >> should reduce CPU usage even more since it won't have to split the SPI
> >> transfers each time the interrupt handler is called.
> >>
> >> The update_scan_mode callback hand to be moved to the buffer preenable
> 
> s/hand/had/
> 
> >> callback since the SPI transfer mode can't be changed after
> >> spi_optimize_message() has been called. (The buffer postenable callback
> >> can't be used because it happens after the trigger is enabled, so the
> >> SPI message needs to be optimized before that.)
> >>
> >> The indent of ti_ads7950_read_raw is changed since there is no longer
> >> anything else in the struct to align with since we removed
> >> ti_ads7950_update_scan_mode.
> > 
> > Some of the func() are mentioned w/o parentheses and I got lost which one is
> > which. Also callbacks usually mentioned as .callback() (with a leading dot).
> 
> I didn't put () in the last paragraph because I was talking about the function
> pointer, not the function. I guess I missed update_scan_mode() though.

Then probably spell it as "pointer to func()" ?

> > The second paragraph doesn't tell me clearly if there is a behaviour change
> > from user perspective.
> 
> It is not clear that the difference the user can notice is that there are
> some CPU cycles freed up for other tasks?

He-he :-)

I asked more about possible behaviour changes in ABI (note, that [significant]
delays moved around might affect ABI if some user's program times out due to
that). But I think it's not the case here.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH] iio: adc: ti-ads7950: use spi_optimize_message()
  2026-04-14 16:31     ` Andy Shevchenko
@ 2026-04-14 17:52       ` David Lechner
  0 siblings, 0 replies; 5+ messages in thread
From: David Lechner @ 2026-04-14 17:52 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On 4/14/26 11:31 AM, Andy Shevchenko wrote:
> On Tue, Apr 14, 2026 at 08:29:29AM -0500, David Lechner wrote:
>> On 4/14/26 4:54 AM, Andy Shevchenko wrote:
>>> On Sat, Apr 11, 2026 at 05:13:33PM -0500, David Lechner wrote:
>>>> Use spi_optimize_message() to reduce CPU usage during buffered reads.
>>>>
>>>> On hardware with support for SPI_CS_WORD, this reduced the CPU usage
>>>> of the threaded interrupt by about 5%. On hardware without support, this
>>>> should reduce CPU usage even more since it won't have to split the SPI
>>>> transfers each time the interrupt handler is called.
>>>>
>>>> The update_scan_mode callback hand to be moved to the buffer preenable
>>
>> s/hand/had/
>>
>>>> callback since the SPI transfer mode can't be changed after
>>>> spi_optimize_message() has been called. (The buffer postenable callback
>>>> can't be used because it happens after the trigger is enabled, so the
>>>> SPI message needs to be optimized before that.)
>>>>
>>>> The indent of ti_ads7950_read_raw is changed since there is no longer
>>>> anything else in the struct to align with since we removed
>>>> ti_ads7950_update_scan_mode.
>>>
>>> Some of the func() are mentioned w/o parentheses and I got lost which one is
>>> which. Also callbacks usually mentioned as .callback() (with a leading dot).
>>
>> I didn't put () in the last paragraph because I was talking about the function
>> pointer, not the function. I guess I missed update_scan_mode() though.
> 
> Then probably spell it as "pointer to func()" ?
> 
>>> The second paragraph doesn't tell me clearly if there is a behaviour change
>>> from user perspective.
>>
>> It is not clear that the difference the user can notice is that there are
>> some CPU cycles freed up for other tasks?
> 
> He-he :-)
> 
> I asked more about possible behaviour changes in ABI (note, that [significant]
> delays moved around might affect ABI if some user's program times out due to
> that). But I think it's not the case here.
> 

No, I don't expect any other changes either.

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

end of thread, other threads:[~2026-04-14 17:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-11 22:13 [PATCH] iio: adc: ti-ads7950: use spi_optimize_message() David Lechner
2026-04-14  9:54 ` Andy Shevchenko
2026-04-14 13:29   ` David Lechner
2026-04-14 16:31     ` Andy Shevchenko
2026-04-14 17:52       ` David Lechner

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