Thread (30 messages) 30 messages, 7 authors, 2025-09-27

Re: [PATCH v2 6/8] iio: adc: ad4030: Add SPI offload support

From: Jonathan Cameron <jic23@kernel.org>
Date: 2025-09-20 09:43:03
Also in: linux-devicetree, linux-iio, linux-spi, lkml

On Thu, 18 Sep 2025 14:39:10 -0300
Marcelo Schmitt [off-list ref] wrote:
AD4030 and similar ADCs can capture data at sample rates up to 2 mega
samples per second (MSPS). Not all SPI controllers are able to achieve such
high throughputs and even when the controller is fast enough to run
transfers at the required speed, it may be costly to the CPU to handle
transfer data at such high sample rates. Add SPI offload support for AD4030
and similar ADCs to enable data capture at maximum sample rates.

Co-developed-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Co-developed-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Co-developed-by: Trevor Gamblin <tgamblin@baylibre.com>
Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
Co-developed-by: Axel Haslam <redacted>
Signed-off-by: Axel Haslam <redacted>
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Most of the code for SPI offload support is based on work from Sergiu Cuciurean,
Nuno Sa, Axel Haslam, and Trevor Gamblin. Thus, this patch comes with many
co-developed-by tags. I also draw inspiration from other drivers supporting SPI
offload, many of them written by David Lechner.

Change log v1 -> v2
- Dropped all clock-modes and DDR related stuff for now as those will require
  further changes to the SPI subsystem or to SPI controller drivers.
- Update the modes register with proper output data mode bits when sample
  averaging (oversampling_ratio) is set.
- Lock on device state mutex before updating oversampling and sampling frequency.
- Made sampling_frequency shared by all channels.
- Better checking the requested sampling frequency is valid.
- Adjusted to SPI offload data capture preparation and stop procedures.
- Error out if try to get/set sampling frequency without offload trigger.
- Depend on PWM so build always succeed.
- Drop unmatched/unbalanced call to iio_device_release_direct().
- No longer shadowing error codes.

Suggestions to v1 that I did not comply to:
[SPI]
quoted
I would be tempted to put the loop check here [in drivers/spi/spi-offload-trigger-pwm.c]:

	offload_offset_ns = periodic->offset_ns;

	do {
		wf.offset_ns = offload_offset_ns;
		ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
		if (ret)
			return ret;
		offload_offset_ns += 10;

	} while (wf.offset_ns < periodic->offset_ns);

	wf.duty_offset_ns = periodic->offset_ns;

instead of in the ADC driver so that all future callers don't have to
repeat this.  
Not sure implementing the PWM trigger phase approximation/rounding/setup within
spi-offload-trigger-pwm is actually desirable. The PWM phase
approximation/rounding/setup done in AD4030 iterates over the configuration of a
second PWM (the PWM connected to the CNV pin). I haven't seen any other device
that would use such double PWM setup schema so pushing an additional argument to
spi_offload_trigger_pwm_validate() doesn't seem worth it.

[IIO]
quoted
Why using slower speed for offload?  
Looks like it's the same max speed for both register access and data sample.
So, just reusing the existing define for the max transfer speed.

 drivers/iio/adc/Kconfig  |   3 +
 drivers/iio/adc/ad4030.c | 485 +++++++++++++++++++++++++++++++++++----
 2 files changed, 445 insertions(+), 43 deletions(-)
Hi Marcelo

Just one thing I noticed today.  If nothing else comes up I can fix that
up whilst applying.  However, this will benefit from review from others
+ the IIO tree is effectively closed for this cycle so we have lots of time
to tidy up any remaining stuff.

Thanks,

Jonathan
 
quoted hunk ↗ jump to hunk
diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index aa0e27321869..52805c779934 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
+static int ad4030_offload_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad4030_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_write(st->regmap, AD4030_REG_EXIT_CFG_MODE, BIT(0));
+	if (ret)
+		return ret;
+
+	ad4030_prepare_offload_msg(indio_dev);
+	st->offload_msg.offload = st->offload;
+	ret = spi_optimize_message(st->spi, &st->offload_msg);
+	if (ret)
+		goto out_reset_mode;
+
+	ret = pwm_set_waveform_might_sleep(st->cnv_trigger, &st->cnv_wf, false);
+	if (ret)
+		goto out_unoptimize;
+
+	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
+					 &st->offload_trigger_config);
+	if (ret)
+		goto out_pwm_disable;
+
+	return 0;
+
+out_pwm_disable:
+	pwm_disable(st->cnv_trigger);
+out_unoptimize:
+	spi_unoptimize_message(&st->offload_msg);
+out_reset_mode:
+	/* reenter register configuration mode */
+	ret = ad4030_enter_config_mode(st);
This blows away the original error.  I'd do something like
	if (ad40303_enter_config_mode(st))
		dev_err(...)

	return ret;
so we preserve whatever went wrong first.

+	if (ret)
+		dev_err(&st->spi->dev,
+			"couldn't reenter register configuration mode\n");
+	return ret;
+}

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help