iio: adc: ad4000: Add SPI offload support

When SPI bits_per_word is not set, SPI transfers default 8-bit word size and ADC data gets stored in big-endian format in memory. Because of that, the IIO driver requests ADC data to be rearranged from BE to CPU endianness. However, with bits_per_word set to the number of ADC precision bits, transfers use larger word sizes that get stored in 'in-memory wordsizes' and can be read in CPU endianness.

Use proper bits_per_word size for SPI transfers thus saving the driver from requesting endianness conversions. With that, shifting the buffer data is also no longer needed. This change has no impact on IIO device functionality.

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

---

Quoted 12 lines · Marcelo Schmitt

When SPI bits_per_word is not set, SPI transfers default 8-bit word size and ADC data gets stored in big-endian format in memory. Because of that, the IIO driver requests ADC data to be rearranged from BE to CPU endianness. However, with bits_per_word set to the number of ADC precision bits, transfers use larger word sizes that get stored in 'in-memory wordsizes' and can be read in CPU endianness.

Use proper bits_per_word size for SPI transfers thus saving the driver from requesting endianness conversions. With that, shifting the buffer data is also no longer needed. This change has no impact on IIO device functionality.

Quoted 2 lines

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

---

This is a breaking change. Some SPI controllers, like RPi can only do 8-bit transfers, so this driver would stop working on those platforms. Also, if anyone made software already that depended on the big-endian ordering without checking the scan_type attribute, it would break that software.

I would leave this as-is (drop this patch) and just make it:

.endianness = _offl ? IIO_CPU : IIO_BE,

in the next patch.

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

---

Instead of changing bits_per_word according to buffer endianness, I set bits_per_word for other SPI transfers and updated IIO channels to always use CPU endianness (the first patch in the series). With that, bits_per_word no longer needs be updated according to buffer endianness or, in other words, buffer endianness is no longer related to bits_per_word.

 drivers/iio/adc/Kconfig  |   7 +-
 drivers/iio/adc/ad4000.c | 491 ++++++++++++++++++++++++++++++++-------
 2 files changed, 419 insertions(+), 79 deletions(-)

drivers/iio/adc/Kconfig +0 -0

index b7ae6e0ae0df..1cfa3a32f3a7 100644
@@ -25,10 +25,15 @@ config AD4000
 	tristate "Analog Devices AD4000 ADC Driver"
 	depends on SPI
 	select IIO_BUFFER

drivers/iio/adc/Kconfig +0 -0

 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4000.

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

index 19bc021e1b5d..b3f4d215cad4 100644

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

+
 #define AD4000_READ_COMMAND	0x54
 #define AD4000_WRITE_COMMAND	0x14
 

@@ -32,10 +35,15 @@
 /* AD4000 Configuration Register programmable bits */

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

+
+#define __AD4000_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access, _offl)\
 {										\
 	.type = IIO_VOLTAGE,							\
 	.indexed = 1,								\

@@ -43,8 +51,9 @@
 	.channel = 0,								\
 	.channel2 = 1,								\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
-			      BIT(IIO_CHAN_INFO_SCALE),				\
-	.info_mask_separate_available = _reg_access ? BIT(IIO_CHAN_INFO_SCALE) : 0,\
+			      BIT(IIO_CHAN_INFO_SCALE) |			\
+			      (_offl ? BIT(IIO_CHAN_INFO_SAMP_FREQ) : 0),	\
+	.info_mask_separate_available = (_reg_access ? BIT(IIO_CHAN_INFO_SCALE) : 0),\
 	.scan_index = 0,							\
 	.scan_type = {								\
 		.sign = _sign,							\

@@ -54,25 +63,28 @@
 	},									\
 }

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

 {										\
-	AD4000_DIFF_CHANNEL(_sign, _real_bits, _reg_access),			\
+	AD4000_DIFF_CHANNEL(_sign, _real_bits, _reg_access, _offl),		\
 	IIO_CHAN_SOFT_TIMESTAMP(1),						\
 }
 
-#define __AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)\
+#define __AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _storage_bits,		\
+				     _reg_access, _offl)			\
 {										\

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

 	.scan_index = 0,							\
 	.scan_type = {								\
 		.sign = _sign,							\

@@ -82,13 +94,14 @@
 	},									\
 }

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

 {										\
-	AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _reg_access),		\
+	AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _reg_access, _offl),	\
 	IIO_CHAN_SOFT_TIMESTAMP(1),						\
 }

};

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

+};
+
+static int ad4000_spi_offload_setup(struct iio_dev *indio_dev,
+				    struct ad4000_state *st)
+{
+	struct spi_device *spi = st->spi;
+	struct device *dev = &spi->dev;
+	struct dma_chan *rx_dma;
+	int ret;
+
+	st->offload_trigger = devm_spi_offload_trigger_get(dev, st->offload,
+							   SPI_OFFLOAD_TRIGGER_PERIODIC);
+	if (IS_ERR(st->offload_trigger))
+		return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
+				     "Failed to get offload trigger\n");
+
+	ret = ad4000_set_sampling_freq(st, st->max_rate_hz);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to set sampling frequency\n");
+
+	rx_dma = devm_spi_offload_rx_stream_request_dma_chan(dev, st->offload);
+	if (IS_ERR(rx_dma))
+		return dev_err_probe(dev, PTR_ERR(rx_dma),
+				     "Failed to get offload RX DMA\n");
+
+	ret = devm_iio_dmaengine_buffer_setup_with_handle(dev, indio_dev, rx_dma,
+							  IIO_BUFFER_DIRECTION_IN);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to setup DMA buffer\n");
+
+	return 0;
+}
+
+/*
+ * This executes a data sample transfer when using SPI offloading for when the
+ * device connections are in "3-wire" mode, selected when the adi,sdi-pin device
+ * tree property is absent. In this connection mode, the ADC SDI pin is
+ * connected to MOSI or to VIO and ADC CNV pin is connected to a SPI controller
+ * CS (it can't be connected to a GPIO).
+ *
+ * In order to achieve the maximum sample rate, we only do one transfer per
+ * SPI offload trigger. Because the ADC output has a one sample latency (delay)
+ * when the device is wired in "3-wire" mode and only one transfer per sample is
+ * being made in turbo mode, the first data sample is not valid because it
+ * contains the output of an earlier conversion result. We also set transfer
+ * `bits_per_word` to achieve higher throughput by using the minimum number of
+ * SCLK cycles. Also, a delay is added to make sure we meet the minimum quiet
+ * time before releasing the CS line. Plus the CS change delay is set to ensure
+ * that we meet the minimum quiet time before asserting CS again.
+ *
+ * This timing is only valid if turbo mode is enabled (reading during conversion).
+ */

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

+
+	xfers[1].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+	xfers[1].bits_per_word = chan->scan_type.realbits;
+	xfers[1].len = chan->scan_type.realbits > 16 ? 4 : 2;
+	xfers[1].delay.value = AD4000_TQUIET2_NS;
+	xfers[1].delay.unit = SPI_DELAY_UNIT_NSECS;
+
+	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);
+}
+

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

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

Quoted 4 lines

Add support for SPI offload.

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

---

I'm surprised I'm not on the CC for this series. scripts/get_maintainer.pl should have picked me up due to K: spi_offload which matches this patch.

Quoted 6 lines

Instead of changing bits_per_word according to buffer endianness, I set bits_per_word for other SPI transfers and updated IIO channels to always use CPU endianness (the first patch in the series). With that, bits_per_word no longer needs be updated according to buffer endianness or, in other words, buffer endianness is no longer related to bits_per_word.

As mentioned in my reply to the previous patch, this is a breaking change, so probably not the best choice.

Quoted 19 lines

 drivers/iio/adc/Kconfig  |   7 +-
 drivers/iio/adc/ad4000.c | 491 ++++++++++++++++++++++++++++++++-------
 2 files changed, 419 insertions(+), 79 deletions(-)

drivers/iio/adc/Kconfig +2 -0

index b7ae6e0ae0df..1cfa3a32f3a7 100644
@@ -25,10 +25,15 @@ config AD4000
 	tristate "Analog Devices AD4000 ADC Driver"
 	depends on SPI
 	select IIO_BUFFER
+	select IIO_BUFFER_DMAENGINE
 	select IIO_TRIGGERED_BUFFER
+	select SPI_OFFLOAD
 	help
 	  Say yes here to build support for Analog Devices AD4000 high speed

drivers/iio/adc/Kconfig config AD4000 · line 36

Quoted 5 lines

drivers/iio/adc/Kconfig +4 -1

-	  SPI analog to digital converters (ADC).
+	  SPI analog to digital converters (ADC). If intended to use with
+	  SPI offloading support, it is recommended to enable
+	  CONFIG_SPI_AXI_SPI_ENGINE, CONFIG_PWM_AXI_PWMGEN, and
+	  CONFIG_SPI_OFFLOAD_TRIGGER_PWM.

I guess this is fine here, but people might be more likely to see it on the ADI wiki where the HDL project that has these requirements is described.

Quoted 11 lines

drivers/iio/adc/Kconfig +0 -0

 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4000.

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

index 19bc021e1b5d..b3f4d215cad4 100644
@@ -21,9 +21,12 @@
 #include <linux/iio/iio.h>
 

drivers/iio/adc/ad4000.c line 28

Quoted 6 lines

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

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

Why not before #include <linux/spi/spi.h> ?

drivers/iio/adc/ad4000.c line 44

Quoted 5 lines

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

-#define __AD4000_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)	\
+#define AD4000_TQUIET1_NS		190
+#define AD4000_TQUIET2_NS		60
+#define AD4000_TCONV_NS			320

Does this actually work for every single chip? For example, the AD7983 datasheet give the max as 500 ns for this timing parameter. Might need to put this in the chip_info to account for differences.

Also AD7983 doesn't have any quite time in the datasheet, so those might not be needed in some cases?

drivers/iio/adc/ad4000.c line 71

Quoted 2 lines

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

-#define AD4000_DIFF_CHANNELS(_sign, _real_bits, _reg_access)			\
+#define AD4000_DIFF_CHANNELS(_sign, _real_bits, _reg_access, _offl)		\

We always pass 0 to _offl here, so we can leave out this parameter and just hard-code the 0 below. We will never have a case with offload + soft timestamp.

Quoted 8 lines

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

-#define __AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)\
+#define __AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _storage_bits,		\
+				     _reg_access, _offl)			\
 {										\
 	.type = IIO_VOLTAGE,							\
 	.indexed = 1,								\
 	.channel = 0,								\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\

drivers/iio/adc/ad4000.c line 87

Quoted 6 lines

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

 			      BIT(IIO_CHAN_INFO_SCALE) |			\
-			      BIT(IIO_CHAN_INFO_OFFSET),			\
-	.info_mask_separate_available = _reg_access ? BIT(IIO_CHAN_INFO_SCALE) : 0,\
+			      BIT(IIO_CHAN_INFO_OFFSET) |			\
+			      (_offl ? BIT(IIO_CHAN_INFO_SAMP_FREQ) : 0),	\
+	.info_mask_separate_available = (_reg_access ? BIT(IIO_CHAN_INFO_SCALE) : 0),\

Unrelated change (adding braces)?

Also, could be nice to have sampling_frequency_available so that userspace can discover the max sample rate.

drivers/iio/adc/ad4000.c line 102

Quoted 2 lines

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

-#define AD4000_PSEUDO_DIFF_CHANNELS(_sign, _real_bits, _reg_access)		\
+#define AD4000_PSEUDO_DIFF_CHANNELS(_sign, _real_bits, _reg_access, _offl)	\

ditto about hard-coding _offl here

drivers/iio/adc/ad4000.c struct ad4000_chip_info { · line 460

Quoted 7 lines

static const struct ad4000_chip_info ad7983_chip_info = {

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

 	.dev_name = "ad7983",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0, 0),
+	.offload_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 0, 1),
 	.time_spec = &ad7983_t_spec,
+	.max_rate_hz = 1330 * KILO,

This is actually 752 ns period when 750 is allowed. Could make it 1 * MEGA + 333
* KILO + 333 or 1333333.

drivers/iio/adc/ad4000.c struct ad4000_chip_info { · line 468

Quoted 10 lines

};

static const struct ad4000_chip_info ad7984_chip_info = {

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

 	.dev_name = "ad7984",
-	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0, 0),
+	.offload_chan_spec = AD4000_DIFF_CHANNEL('s', 18, 0, 1),
 	.time_spec = &ad7983_t_spec,
+	.max_rate_hz = 1330 * KILO,

ditto

drivers/iio/adc/ad4000.c static const struct iio_info ad4000_reg_access_info = { · line 831

Quoted 3 lines

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

+static const struct iio_buffer_setup_ops ad4000_offload_buffer_setup_ops = {
+	.postenable = &ad4000_offload_buffer_postenable,
+	.postdisable = &ad4000_offload_buffer_postdisable,

Needs to be predisable to correctly balance postenable.

Quoted 4 lines

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

+static int ad4000_prepare_offload_turbo_message(struct ad4000_state *st,
+						const struct iio_chan_spec *chan)
+{
+	struct spi_transfer *xfers = st->offload_xfers;

drivers/iio/adc/ad4000.c static const struct iio_info ad4000_reg_access_info = { · line 894

Quoted 6 lines

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

+
+	/* Have to do a short CS toggle to trigger conversion. */
+	xfers[0].cs_change = 1;
+	xfers[0].cs_change_delay.value = AD4000_TQUIET1_NS;
+	xfers[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
+	xfers[0].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;

There is no data transferred in this xfer, so we don't need to set this flag.

Quoted 6 lines

drivers/iio/adc/ad4000.c +3 -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;

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

Quoted 4 lines

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

+			indio_dev->num_channels = 1;
+
+			spi->cs_hold.value = AD4000_TCONV_NS;
+			spi->cs_hold.unit = SPI_DELAY_UNIT_NSECS;

Modifying spi is suspicious. Normally, this would come from the devicetree. And this isn't used by all SPI drivers - notably the AXI SPI Engine ignores it. Better to just handle this in individual spi xfer structs.

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

Quoted 10 lines

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

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

nit: add extra blank line here

Quoted 8 lines

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

 		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 14 lines · David Lechner

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

I'm surprised I'm not on the CC for this series. scripts/get_maintainer.pl should have picked me up due to K: spi_offload which matches this patch.

get_maintainers doesn't list you if run over ad4000.c.
If run over the patch file, get_maintainers lists many developers. Some of them don't seem to be related to this series, so I cropped the list to avoid churn. Will CC you on the next versions.

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

---

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

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

---

+Differential ADCs
+-----------------

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

---