iio: adc: ad4000: Add SPI offload support

FPGA HDL projects can include a PWM generator in addition to SPI-Engine. The PWM IP is used to trigger SPI-Engine offload modules that in turn set SPI-Engine to execute transfers to poll data from the ADC. That allows data to be read at the maximum sample rates. Also, it is possible to set a specific sample rate by setting the proper PWM duty cycle and related state parameters, thus allowing an adjustable ADC sample rate when a PWM (offload trigger) is used in combination with SPI-Engine.

Add support for SPI offload.

Signed-off-by Marcelo Schmitt <[email protected]>

---

drivers/iio/adc/ad4000.c +1 -0

 #include <linux/units.h>
 #include <linux/util_macros.h>
 #include <linux/iio/iio.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/buffer-dmaengine.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 

+/*
+ * When SPI offload is configured, transfers are executed withouth CPU

+ * intervention so no soft timestamp can be recorded when transfers run.
+ * Because of that, the macros that set timestamp channel are only used when
+ * transfers are not offloaded.
+ */

drivers/iio/adc/ad4000.c +18 -2

 		/*
 		 * In "3-wire mode", the ADC SDI line must be kept high when

@@ -796,9 +1096,26 @@ static int ad4000_probe(struct spi_device *spi)
 		if (ret < 0)
 			return ret;
 
+		if (st->using_offload) {
+			indio_dev->channels = &chip->reg_access_offload_chan_spec;
+			indio_dev->num_channels = 1;
+			ret = ad4000_prepare_offload_message(st, indio_dev->channels);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to optimize SPI msg\n");
+		} else {
+			indio_dev->channels = chip->reg_access_chan_spec;
+			indio_dev->num_channels = ARRAY_SIZE(chip->reg_access_chan_spec);
+		}
+
+		/*
+		 * Call ad4000_prepare_3wire_mode_message() so single-shot read
+		 * SPI messages are always initialized.
+		 */
 		ret = ad4000_prepare_3wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
-			return ret;
+			return dev_err_probe(dev, ret,
+					     "Failed to optimize SPI msg\n");
 
 		ret = ad4000_config(st);
 		if (ret < 0)

@@ -806,19 +1123,47 @@ static int ad4000_probe(struct spi_device *spi)
 
 		break;
 	case AD4000_SDI_VIO:
-		indio_dev->info = &ad4000_info;

drivers/iio/adc/ad4000.c +17 -1

+				ret = spi_setup(spi);
+				if (ret < 0)
+					return ret;
+			}
+
+			ret = ad4000_prepare_offload_message(st, indio_dev->channels);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to optimize SPI msg\n");
+		} else {
+			indio_dev->info = &ad4000_info;
+			indio_dev->channels = chip->chan_spec;
+			indio_dev->num_channels = ARRAY_SIZE(chip->chan_spec);
+		}
+
 		ret = ad4000_prepare_3wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
-			return ret;
+			return dev_err_probe(dev, ret,
+					     "Failed to optimize SPI msg\n");
 
 		break;

Quoted 12 lines · Marcelo Schmitt

FPGA HDL projects can include a PWM generator in addition to SPI-Engine. The PWM IP is used to trigger SPI-Engine offload modules that in turn set SPI-Engine to execute transfers to poll data from the ADC. That allows data to be read at the maximum sample rates. Also, it is possible to set a specific sample rate by setting the proper PWM duty cycle and related state parameters, thus allowing an adjustable ADC sample rate when a PWM (offload trigger) is used in combination with SPI-Engine.

Add support for SPI offload.

Signed-off-by Marcelo Schmitt <[email protected]>

---

Quoted 3 lines

#include <linux/regulator/consumer.h>
#include <linux/spi/spi.h>
+#include <linux/spi/offload/consumer.h>

Alphabetical order?

Quoted 10 lines

#include <linux/units.h>
#include <linux/util_macros.h>
#include <linux/iio/iio.h>

#include <linux/iio/buffer.h>
+#include <linux/iio/buffer-dmaengine.h>
#include <linux/iio/triggered_buffer.h>
#include <linux/iio/trigger_consumer.h>

...

Quoted 3 lines

+/*
+ * When SPI offload is configured, transfers are executed withouth CPU

s/withouth/without/

Quoted 4 lines

+ * intervention so no soft timestamp can be recorded when transfers run.
+ * Because of that, the macros that set timestamp channel are only used when
+ * transfers are not offloaded.
+ */

drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1087

Quoted 9 lines

drivers/iio/adc/ad4000.c +4 -1

@@ -784,7 +1081,10 @@ static int ad4000_probe(struct spi_device *spi)
 	switch (st->sdi_pin) {
 	case AD4000_SDI_MOSI:
 		indio_dev->info = &ad4000_reg_access_info;
-		indio_dev->channels = chip->reg_access_chan_spec;
+
+		/* Set CNV/CS high time for when turbo mode is used */
+		spi->cs_inactive.value = st->time_spec->t_quiet1_ns;
+		spi->cs_inactive.unit = SPI_DELAY_UNIT_NSECS;

This code path later calls ad4000_prepare_3wire_mode_message() which sets:

	xfers[0].cs_change_delay.value = st->time_spec->t_conv_ns;

Which contradicts/overrides this.

Quoted 27 lines

drivers/iio/adc/ad4000.c +18 -1

+		if (st->using_offload) {
+			indio_dev->channels = &chip->reg_access_offload_chan_spec;
+			indio_dev->num_channels = 1;
+			ret = ad4000_prepare_offload_message(st, indio_dev->channels);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to optimize SPI msg\n");
+		} else {
+			indio_dev->channels = chip->reg_access_chan_spec;
+			indio_dev->num_channels = ARRAY_SIZE(chip->reg_access_chan_spec);
+		}
+
+		/*
+		 * Call ad4000_prepare_3wire_mode_message() so single-shot read
+		 * SPI messages are always initialized.
+		 */
 		ret = ad4000_prepare_3wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
-			return ret;
+			return dev_err_probe(dev, ret,
+					     "Failed to optimize SPI msg\n");
 
 		ret = ad4000_config(st);
 		if (ret < 0)

@@ -806,19 +1123,47 @@ static int ad4000_probe(struct spi_device *spi)
 

drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1134

Quoted 13 lines

drivers/iio/adc/ad4000.c +9 -2

 		break;
 	case AD4000_SDI_VIO:
-		indio_dev->info = &ad4000_info;
-		indio_dev->channels = chip->chan_spec;
+		if (st->using_offload) {
+			indio_dev->info = &ad4000_offload_info;
+			indio_dev->channels = &chip->offload_chan_spec;
+			indio_dev->num_channels = 1;
+
+			/* Set CNV/CS high time for when turbo mode is not used */
+			if (!st->cnv_gpio) {
+				spi->cs_inactive.value = st->time_spec->t_conv_ns;
+				spi->cs_inactive.unit = SPI_DELAY_UNIT_NSECS;

I'm still not sold on this. We know it has no effect with AXI SPI Engine and it is writing over something that usually comes from DT. It is misleading.

And the non-offload case already does:

	xfers[0].cs_change_delay.value = st->time_spec->t_conv_ns;

which actually does work with the AXI SPI Engine. So why not be consistent and do it the same way for the offload case?

It also seems safe to omit this altogether in the offload case and assume that the max sample rate will also ensure that the miniumum time for CS deasserted is respected.

Quoted 22 lines

drivers/iio/adc/ad4000.c +17 -1

+				ret = spi_setup(spi);
+				if (ret < 0)
+					return ret;
+			}
+
+			ret = ad4000_prepare_offload_message(st, indio_dev->channels);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to optimize SPI msg\n");
+		} else {
+			indio_dev->info = &ad4000_info;
+			indio_dev->channels = chip->chan_spec;
+			indio_dev->num_channels = ARRAY_SIZE(chip->chan_spec);
+		}
+
 		ret = ad4000_prepare_3wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
-			return ret;
+			return dev_err_probe(dev, ret,
+					     "Failed to optimize SPI msg\n");
 

Review static int ad4000_probe(struct spi_device *spi) · line 1155

Quoted 1 line

(quoted diff) +0 -0

 		break;

Quoted 7 lines

Quoted 7 lines

drivers/iio/adc/ad4000.c +4 -2

 	case AD4000_SDI_VIO:
-		indio_dev->info = &ad4000_info;
-		indio_dev->channels = chip->chan_spec;
+		if (st->using_offload) {
+			indio_dev->info = &ad4000_offload_info;
+			indio_dev->channels = &chip->offload_chan_spec;
+			indio_dev->num_channels = 1;

Quoted 8 lines

drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1134

Quoted 5 lines

drivers/iio/adc/ad4000.c +5 -0

+
+			/* Set CNV/CS high time for when turbo mode is not used */
+			if (!st->cnv_gpio) {
+				spi->cs_inactive.value = st->time_spec->t_conv_ns;
+				spi->cs_inactive.unit = SPI_DELAY_UNIT_NSECS;

I'm still not sold on this. We know it has no effect with AXI SPI Engine and it is writing over something that usually comes from DT. It is misleading.

I thought it was okay to set cs_inactive and call spi_setup() from the field doc in include/linux/spi/spi.h.

set_cs_timing() method is for SPI controllers that supports configuring CS timing.

This hook allows SPI client drivers to request SPI controllers to configure specific CS timing through spi_set_cs_timing() after spi_setup().

Would it be better to set spi-cs-inactive-delay-ns in ADC dt node? Or it still doesn't look like a proper use of cs_inactive?

Quoted 7 lines

And the non-offload case already does:

	xfers[0].cs_change_delay.value = st->time_spec->t_conv_ns;

which actually does work with the AXI SPI Engine. So why not be consistent and do it the same way for the offload case?

One of the points in using bits_per_word in spi transfers was to reach high frequency sample rate, right? I think it makes sense to use them for SPI offload transfers. But we were also trying to set a proper CNV/CS dealy so that ADC conversion could complete properly before starting requesting the data. That also sound reasonable to me. But, spi_transfer struct doesn't provide a good way of setting a CS inactive delay if only one transfer is executed. If we use cs_change_delay, we would then be running two transfers, no?

Plus, the ADC
would be doing two conversions (one after CS deasert of previous message and one after CS deassert at the end of the first transfer) while we only read one of them.

The offload message preparation would look like what we had in v2:

static int ad4000_prepare_offload_turbo_message(struct ad4000_state *st,
const struct iio_chan_spec *chan)

{
	struct spi_transfer *xfers = st->offload_xfers;

	/* Dummy transfer to guarantee enough CS high time. */
	xfers[0].cs_change = 1;
	xfers[0].cs_change_delay.value = st->time_spec->t_quiet1_ns;
	xfers[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;

	xfers[1].bits_per_word = chan->scan_type.realbits;
	xfers[1].len = chan->scan_type.realbits > 16 ? 4 : 2;
	xfers[1].delay.value = st->time_spec->t_quiet2_ns;
	xfers[1].delay.unit = SPI_DELAY_UNIT_NSECS;
	xfers[1].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;

	spi_message_init_with_transfers(&st->offload_msg, xfers, 2);
	st->offload_msg.offload = st->offload;

	return devm_spi_optimize_message(&st->spi->dev, st->spi, &st->offload_msg);
}

Are we worried about a few clock cycles in between transfers but not worried about running an entire dummy transfer?

Plus, I've tested the single-transfer offload message version with ADAQ4003 on CoraZ7 and verified the results were correct.
FWIW, I put a copy of the dts I used for the tests at the end of this email.

Quoted 1 line

Quoted 3 lines

It also seems safe to omit this altogether in the offload case and assume that the max sample rate will also ensure that the miniumum time for CS deasserted is respected.

If we can assume that, then I think that's another reason why we don't need a dummy transfer to set CS high delay.

Commit <c3948d090080> ("iio: adc: ad4000: Add support for PulSAR devices"), extended the ad4000 driver supports many single-channel PulSAR devices.

Update IIO ad4000 documentation with the extra list of supported devices.

Signed-off-by Marcelo Schmitt <[email protected]>

---

Quoted 7 lines · Marcelo Schmitt

Commit <c3948d090080> ("iio: adc: ad4000: Add support for PulSAR devices"), extended the ad4000 driver supports many single-channel PulSAR devices.

Update IIO ad4000 documentation with the extra list of supported devices.

Signed-off-by Marcelo Schmitt <[email protected]>

---

Don't forget to pick up tags. I already gave my:

Reviewed-by David Lechner <[email protected]>

Complement ad4000 IIO driver documentation with considerations about ``_scale_available`` attribute and table of typical channel attributes.

Signed-off-by Marcelo Schmitt <[email protected]>

---

When SPI offloading is supported, the IIO device provides different sysfs interfaces to allow using the adjusting the sample rate. Document SPI offload support for AD4000 and similar devices.

Signed-off-by Marcelo Schmitt <[email protected]>

---