iio: adc: ad4000: Add SPI offload support
Marcelo Schmitt
maintainercover iio: adc: ad4000: Add SPI offload support
This patch series extends the ad4000 driver to support SPI offloading. In addition to that, ad4000 IIO documentation is expanded to:
- list PulSAR parts supported by the ad4000 driver.
- describe some characteristics of AD4000 IIO device.
- describe changes when SPI offload is being used.
The proposed changes were tested with AD7687 and ADAQ4003 on CoraZ7 setup running Linux built from IIO testing branch.
Marcelo Schmitt (4):
iio: adc: ad4000: Add support for SPI offload
Documentation: iio: ad4000: Add new supported parts
Documentation: iio: ad4000: Add IIO Device characteristics section
Documentation: iio: ad4000: Describe offload support
Documentation/iio/ad4000.rst | 81 +++++- drivers/iio/adc/Kconfig | 7 +- drivers/iio/adc/ad4000.c | 519 +++++++++++++++++++++++++++++------ 3 files changed, 517 insertions(+), 90 deletions(-)
base-commit: af94f401e26f686f7391ce79b38a6129417c22dc
prerequisite-patch-id: 3d517eef53a799adba5922815fe684b913e36773
Signature · 1 line
2.47.2
Marcelo Schmitt
maintainerPATCH 1/4 iio: adc: ad4000: Add support for SPI offload
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.
--- drivers/iio/adc/Kconfig | 7 +- drivers/iio/adc/ad4000.c | 519 ++++++++++++++++++++++++++++++++------- 2 files changed, 437 insertions(+), 89 deletions(-)
drivers/iio/adc/Kconfig +6 -1
@@ -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 - 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. To compile this driver as a module, choose M here: the module will be called ad4000.
drivers/iio/adc/ad4000.c +431 -88
865 lines not on this page · raw message
Signature · 1 line
2.47.2
Re: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload
Hi Marcelo,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Marcelo-Schmitt/iio-adc-ad4000-Add-support-for-SPI-offload/20250315-012316 base: af94f401e26f686f7391ce79b38a6129417c22dc patch link: https://lore.kernel.org/r/301fc83a961c4a2ef2ac980d0baa83d9d89a88c5.1741970538.git.marcelo.schmitt%40analog.com patch subject: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload config: parisc-randconfig-r072-20250316 (https://download.01.org/0day-ci/archive/20250316/[email protected]/config) compiler: hppa-linux-gcc (GCC) 14.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]> | Reported-by: Dan Carpenter <[email protected]> | Closes: https://lore.kernel.org/r/[email protected]/
New smatch warnings:drivers/iio/adc/ad4000.c:862 ad4000_spi_offload_setup() warn: passing zero to 'PTR_ERR'
vim +/PTR_ERR +862 drivers/iio/adc/ad4000.c
e74205e82803041 Marcelo Schmitt 2025-03-14 846 static int ad4000_spi_offload_setup(struct iio_dev *indio_dev,
e74205e82803041 Marcelo Schmitt 2025-03-14 847 struct ad4000_state *st)
e74205e82803041 Marcelo Schmitt 2025-03-14 848 {
e74205e82803041 Marcelo Schmitt 2025-03-14 849 struct spi_device *spi = st->spi;
e74205e82803041 Marcelo Schmitt 2025-03-14 850 struct device *dev = &spi->dev;
e74205e82803041 Marcelo Schmitt 2025-03-14 851 struct dma_chan *rx_dma;
e74205e82803041 Marcelo Schmitt 2025-03-14 852 int ret;
e74205e82803041 Marcelo Schmitt 2025-03-14 853
e74205e82803041 Marcelo Schmitt 2025-03-14 854 st->offload_trigger = devm_spi_offload_trigger_get(dev, st->offload,
e74205e82803041 Marcelo Schmitt 2025-03-14 855 SPI_OFFLOAD_TRIGGER_PERIODIC);
e74205e82803041 Marcelo Schmitt 2025-03-14 856 if (IS_ERR(st->offload_trigger))
e74205e82803041 Marcelo Schmitt 2025-03-14 857 return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
e74205e82803041 Marcelo Schmitt 2025-03-14 858 "Failed to get offload trigger\n");
e74205e82803041 Marcelo Schmitt 2025-03-14 859
e74205e82803041 Marcelo Schmitt 2025-03-14 860 ret = ad4000_set_sampling_freq(st, st->max_rate_hz);
e74205e82803041 Marcelo Schmitt 2025-03-14 861 if (ret)e74205e82803041 Marcelo Schmitt 2025-03-14 @862 return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
s/PTR_ERR(st->offload_trigger)/ret/
e74205e82803041 Marcelo Schmitt 2025-03-14 863 "Failed to set sampling frequency\n"); e74205e82803041 Marcelo Schmitt 2025-03-14 864 e74205e82803041 Marcelo Schmitt 2025-03-14 865 rx_dma = devm_spi_offload_rx_stream_request_dma_chan(dev, st->offload); e74205e82803041 Marcelo Schmitt 2025-03-14 866 if (IS_ERR(rx_dma)) e74205e82803041 Marcelo Schmitt 2025-03-14 867 return dev_err_probe(dev, PTR_ERR(rx_dma), e74205e82803041 Marcelo Schmitt 2025-03-14 868 "Failed to get offload RX DMA\n"); e74205e82803041 Marcelo Schmitt 2025-03-14 869 e74205e82803041 Marcelo Schmitt 2025-03-14 870 ret = devm_iio_dmaengine_buffer_setup_with_handle(dev, indio_dev, rx_dma, e74205e82803041 Marcelo Schmitt 2025-03-14 871 IIO_BUFFER_DIRECTION_IN); e74205e82803041 Marcelo Schmitt 2025-03-14 872 if (ret) e74205e82803041 Marcelo Schmitt 2025-03-14 873 return dev_err_probe(dev, ret, "Failed to setup DMA buffer\n"); e74205e82803041 Marcelo Schmitt 2025-03-14 874 e74205e82803041 Marcelo Schmitt 2025-03-14 875 return 0; e74205e82803041 Marcelo Schmitt 2025-03-14 876 }
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Jonathan Cameron
maintainerRe: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload
In reply to Marcelo SchmittQuoted 11 lines · On Fri, 14 Mar 2025 14:18:39 -0300 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]>
A few minor things inline.
Jonathan
drivers/iio/adc/ad4000.c static const struct iio_info ad4000_reg_access_info = { · line 837
Quoted 11 lines
drivers/iio/adc/ad4000.c +6
-0+ +static int ad4000_offload_buffer_postdisable (struct iio_dev *indio_dev) +{ +struct ad4000_state *st =iio_priv (indio_dev); + +spi_offload_trigger_disable (st->offload, st->offload_trigger);
Trivial. Prefer a blank line before a 'simple return' like this one.
Quoted 5 lines
drivers/iio/adc/ad4000.c +3
-0+return 0 ; +} +
drivers/iio/adc/ad4000.c static const struct iio_info ad4000_reg_access_info = { · line 887
Quoted 10 lines
drivers/iio/adc/ad4000.c +10
-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. This has the effect that the first sample data is not +* valid because it is reading the previous conversion result. We also use
As below. Should mention what happens with that invalid sample.
Quoted 11 lines
+ *
bits_per_wordto ensure the minimum of SCLK cycles are used. And 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 +4
-0+static int ad4000_prepare_offload_turbo_message (struct ad4000_state *st, +const struct iio_chan_spec *chan) +{ +
drivers/iio/adc/ad4000.c static const struct iio_info ad4000_reg_access_info = { · line 927
Quoted 10 lines
drivers/iio/adc/ad4000.c +10
-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 set to "high". In this connection mode, the ADC SDI pin is +* connected 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. This has the effect that the first sample data is not +* valid because it is reading the previous conversion result. We also use
Say what happens to that invalid sample. Is it dropped or provided to userspace as if it were valid? (I hope dropped!)
Quoted 10 lines
+ *
bits_per_wordto ensure the minimum of SCLK cycles are used. And 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
+ * conversion time before asserting CS again.
+ *
+ * This timing is only valid if turbo mode is disabled (reading during acquisition).
+ */drivers/iio/adc/ad4000.c +3
-0+static int ad4000_prepare_offload_message (struct ad4000_state *st, +const struct iio_chan_spec *chan) +
...
drivers/iio/adc/ad4000.c static int ad4000_prepare_3wire_mode_message(struct ad4000_state *st, · line 979
Quoted 10 lines
/*
* This executes a data sample transfer for when the device connections are
* in "3-wire" mode, selected when the adi,sdi-pin device tree property isdrivers/iio/adc/ad4000.c +2
-1@@ -689,7 +975,9 @@ static int ad4000_prepare_3wire_mode_message(struct ad4000_state *st, xfers[0 ].cs_change_delay.unit =SPI_DELAY_UNIT_NSECS ; xfers[1 ].rx_buf = &st->scan.data; - xfers[1 ].len =BITS_TO_BYTES (chan->scan_type.storagebits); + xfers[1 ].len = chan->scan_type.realbits >16 ?4 :2 ; +if (chan->scan_type.endianness !=IIO_BE )
As below. Endianness being related to bits_per_word is odd. So needs explanation.
drivers/iio/adc/ad4000.c static int ad4000_prepare_4wire_mode_message(struct ad4000_state *st, · line 1011
Quoted 11 lines
drivers/iio/adc/ad4000.c +3
-1+ xfers[1 ].bits_per_word = chan->scan_type.realbits; xfers[1 ].delay.value = st->time_spec->t_quiet2_ns; xfers[1 ].delay.unit =SPI_DELAY_UNIT_NSECS ;@@ -719,7 +1007,9 @@ static int ad4000_prepare_4wire_mode_message(struct ad4000_state *st, xfers[0 ].delay.unit =SPI_DELAY_UNIT_NSECS ; xfers[1 ].rx_buf = &st->scan.data; - xfers[1 ].len =BITS_TO_BYTES (chan->scan_type.storagebits); + xfers[1 ].len = chan->scan_type.realbits >16 ?4 :2 ; +if (chan->scan_type.endianness !=IIO_BE )
This is odd enough to require a comment. Why is endianness relevant?
Quoted 5 lines
drivers/iio/adc/ad4000.c +1
-0+ xfers[1 ].bits_per_word = chan->scan_type.realbits;@@ -733,6 +1023,9 @@ static int ad4000_config(struct ad4000_state *st)if (device_property_present (&st->spi->dev,"adi,high-z-input" )) reg_val |=FIELD_PREP (AD4000_CFG_HIGHZ ,1 );
drivers/iio/adc/ad4000.c static int ad4000_config(struct ad4000_state *st)
Quoted 10 lines
drivers/iio/adc/ad4000.c +5
-0+if (st->using_offload) + reg_val |=FIELD_PREP (AD4000_CFG_TURBO ,1 ); +return ad4000_write_reg (st, reg_val); } +if (st->using_offload) { + indio_dev->channels = &chip->reg_access_offload_chan_spec;
Set num_channels here
drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1119
Quoted 6 lines
drivers/iio/adc/ad4000.c +6
-0+ ret =ad4000_prepare_offload_turbo_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;
and here as well. I think you can use ARRAY_SIZE() to make the connection even more obvious.
Quoted 15 lines
drivers/iio/adc/ad4000.c +8
-1+ } + +/* +* 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 )
drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1139
Quoted 8 lines
drivers/iio/adc/ad4000.c +3
-2@@ -806,19 +1134,36 @@ static int ad4000_probe(struct spi_device *spi)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;
here as well.
drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1149
Quoted 10 lines
drivers/iio/adc/ad4000.c +10
-0+ + spi->cs_hold.value =AD4000_TCONV_NS ; + spi->cs_hold.unit =SPI_DELAY_UNIT_NSECS ; + 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;
Also set size here. Obviously this means a little duplication but still good to keep them together.
drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1181
Quoted 12 lines
drivers/iio/adc/ad4000.c +5
-1+ }case AD4000_SDI_GND :@@ -830,7 +1175,10 @@ static int ad4000_probe(struct spi_device *spi) } indio_dev->name = chip->dev_name; - indio_dev->num_channels =2 ; +if (st->using_offload) + indio_dev->num_channels =1 ; +else + indio_dev->num_channels =2 ;
Move this up to where you set channels so that the array and size are set together.
Quoted 21 lines
drivers/iio/adc/ad4000.c +1
-6ret =devm_mutex_init (dev, &st->lock);if (ret)@@ -853,12 +1201,6 @@ static int ad4000_probe(struct spi_device *spi)ad4000_fill_scale_tbl (st, &indio_dev->channels[0 ]); - ret =devm_iio_triggered_buffer_setup (dev, indio_dev, - &iio_pollfunc_store_time, - &ad4000_trigger_handler,NULL ); -if (ret) -return ret; -return devm_iio_device_register (dev, indio_dev); }@@ -947,3 +1289,4 @@ module_spi_driver(ad4000_driver);MODULE_AUTHOR ("Marcelo Schmitt <[email protected]>" );MODULE_DESCRIPTION ("Analog Devices AD4000 ADC driver" );MODULE_LICENSE ("GPL" ); +MODULE_IMPORT_NS ("IIO_DMAENGINE_BUFFER" );
Re: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload
...
Quoted 5 lines
Quoted 4 lines
drivers/iio/adc/ad4000.c +0
-0
Quoted 9 lines
drivers/iio/adc/ad4000.c static const struct iio_info ad4000_reg_access_info = { · line 837
Quoted 6 lines
drivers/iio/adc/ad4000.c +6
-0+ +static int ad4000_offload_buffer_postdisable (struct iio_dev *indio_dev) +{ +struct ad4000_state *st =iio_priv (indio_dev); + +spi_offload_trigger_disable (st->offload, st->offload_trigger);Trivial. Prefer a blank line before a 'simple return' like this one.
Ack
Quoted 4 lines
Quoted 2 lines
drivers/iio/adc/ad4000.c +2
-0+return 0 ; +}
...
Quoted 11 lines
Quoted 10 lines
+/*
+ * 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 set to "high". In this connection mode, the ADC SDI pin is
+ * connected 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. This has the effect that the first sample data is not
+ * valid because it is reading the previous conversion result. We also use
Quoted 2 lines
Say what happens to that invalid sample. Is it dropped or provided to userspace as if it were valid? (I hope dropped!)
TL;DR: The invalid sample goes into the buffer as a valid one.
In AD4000 '3-wire' mode, data capture has a latency (delay) of one sample.
The ADC begins sampling data N at CNV rising edge
| +-- CNV (usually SPI CS) is brought low to begin reading the data
| | +-- Data N + 1 that will be read
| | | on the next transfer starts
v v v being sampled at end of transfer N. ___ ____
CNV _____/ \________________________________/ \_____
_ _ _
SCLK ______________/ \___/ \_ ... ___/ \_______________
___ ___ ___
SDO _____________/___\_/___\ ... __/___\_______________
^
|
Data from conversion N is output from here onA better drawing can be found in datasheet page 29, Figure 57.
https://www.analog.com/media/en/technical-documentation/data-sheets/ADAQ4003.pdf
In sum, we're always reading a conversion that started at the end of the previous SPI transfer or, in other words, the data comes out with a latency (delay) of one read.
Datasheet somehow mentions that by saying
When turbo mode is enabled, the conversion result read on SDO corresponds to the result of the previous conversion.
I think I can do a dummy SPI transfer on buffer preenable so at least the first data is not invalid. Would that be better?
Quoted 12 lines
Quoted 11 lines
+ *
bits_per_wordto ensure the minimum of SCLK cycles are used. And 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
+ * conversion time before asserting CS again.
+ *
+ * This timing is only valid if turbo mode is disabled (reading during acquisition).
+ */drivers/iio/adc/ad4000.c +3
-0+static int ad4000_prepare_offload_message (struct ad4000_state *st, +const struct iio_chan_spec *chan) +
...
Quoted 7 lines
Quoted 7 lines
drivers/iio/adc/ad4000.c +1
-0+ xfers[1 ].bits_per_word = chan->scan_type.realbits; xfers[1 ].delay.value = st->time_spec->t_quiet2_ns; xfers[1 ].delay.unit =SPI_DELAY_UNIT_NSECS ;@@ -719,7 +1007,9 @@ static int ad4000_prepare_4wire_mode_message(struct ad4000_state *st, xfers[0 ].delay.unit =SPI_DELAY_UNIT_NSECS ;
Quoted 6 lines
drivers/iio/adc/ad4000.c static int ad4000_prepare_3wire_mode_message(struct ad4000_state *st, · line 979
Quoted 4 lines
drivers/iio/adc/ad4000.c +2
-1xfers[1 ].rx_buf = &st->scan.data; - xfers[1 ].len =BITS_TO_BYTES (chan->scan_type.storagebits); + xfers[1 ].len = chan->scan_type.realbits >16 ?4 :2 ; +if (chan->scan_type.endianness !=IIO_BE )This is odd enough to require a comment. Why is endianness relevant?
When using SPI offloading (at least with PULSAR-ADC HDL project [1]), ADC data is read by SPI controller and pushed to DMA memory in CPU endianness. I don't know exactly where data gets rearranged in the data path (whether SPI-Engine, the DMA controller, or something else rearranges ADC data into CPU endianess). But I know, from testing with these ADCs and HDL project, that data is correct when read in CPU endianness because it converts back to expected mV values. When IIO buffers were set to IIO_BE and SPI offloading is used, data just looked weird and didn't convert to expected values in mV.
[1]: https://analogdevicesinc.github.io/hdl/projects/pulsar_adc/index.html
Other IIO drivers also set IIO_CPU buffer endianness when using offload support, e.g. ad7944, ad7380.
They only say buffer would use 32 storagebits when using SPI offload.
https://lore.kernel.org/linux-iio/20250207-dlech-mainline-spi-engine-offload-2-v8-10-e48a489be48c@baylibre.com/
https://lore.kernel.org/linux-iio/20250220-wip-bl-spi-offload-ad7380-v1-1-838aa873e62a@baylibre.com/#t
I also didn't expect to find out HDL support for 16-bit data width was removed. We used to have a build parameter for 16-bit precision ADCs.
https://github.com/analogdevicesinc/hdl/commit/b2dc91b30dae891b6319d88e083f26e726f43ba0#diff-1117c2618353232e5f22aa6a12e8ae976757fa897b3425f470a12123cae26535L13
Would something like 'because SPI offloading leads to data being pushed to memory in CPU endianness' be a reasonable comment?
...
Quoted 4 lines
drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1113
Quoted 3 lines
drivers/iio/adc/ad4000.c +2
-0+if (st->using_offload) { + indio_dev->channels = &chip->reg_access_offload_chan_spec;Set
num_channelshere
Ack for all these
Quoted 1 line
Quoted 1 line
drivers/iio/adc/ad4000.c +1
-0+ ret =ad4000_prepare_offload_turbo_message (st, indio_dev->channels);
...
Quoted 3 lines
Also set size here. Obviously this means a little duplication but still good to keep them together.
Okay, sounds reasonable.
Quoted 8 lines
Quoted 8 lines
drivers/iio/adc/ad4000.c +1
-0+ }case AD4000_SDI_GND :@@ -830,7 +1175,10 @@ static int ad4000_probe(struct spi_device *spi) }
Quoted 9 lines
drivers/iio/adc/ad4000.c static int ad4000_probe(struct spi_device *spi) · line 1181
Quoted 6 lines
drivers/iio/adc/ad4000.c +4
-1indio_dev->name = chip->dev_name; - indio_dev->num_channels =2 ; +if (st->using_offload) + indio_dev->num_channels =1 ; +else + indio_dev->num_channels =2 ;Move this up to where you set channels so that the array and size are set together.
Acknowledged as all the above cases.
Thanks,
Marcelo
David Lechner
reviewerRe: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload
Quoted 41 lines · Marcelo Schmitt
...
Quoted 11 lines
Quoted 10 lines
+/*
+ * 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 set to "high". In this connection mode, the ADC SDI pin is
+ * connected 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. This has the effect that the first sample data is not
+ * valid because it is reading the previous conversion result. We also useQuoted 2 lines
Say what happens to that invalid sample. Is it dropped or provided to userspace as if it were valid? (I hope dropped!)
TL;DR: The invalid sample goes into the buffer as a valid one.
In AD4000 '3-wire' mode, data capture has a latency (delay) of one sample.
The ADC begins sampling data N at CNV rising edge
| +-- CNV (usually SPI CS) is brought low to begin reading the data | | +-- Data N + 1 that will be read | | | on the next transfer starts v v v being sampled at end of transfer N.___ ____
CNV _____/ \________________________________/ \_____
_ _ _
SCLK ______________/ \___/ \_ ... ___/ \__________________ ___ ___ SDO _____________/___\_/___\ ... __/___\_______________ ^ | Data from conversion N is output from here onA better drawing can be found in datasheet page 29, Figure 57.
https://www.analog.com/media/en/technical-documentation/data-sheets/ADAQ4003.pdfIn sum, we're always reading a conversion that started at the end of the previous SPI transfer or, in other words, the data comes out with a latency (delay) of one read.
Quoted 6 lines
Datasheet somehow mentions that by saying
When turbo mode is enabled, the conversion result read on SDO corresponds to the result of the previous conversion.I think I can do a dummy SPI transfer on buffer preenable so at least the first data is not invalid. Would that be better?
Not really. There will be a relatively long delay between that conversion trigger and when the sample is read. So the data might be slightly less stale in that case, but still not particularly useful, e.g. if you are doing any kind of signal processing that expects equal time between all samples.
On similar chips, like ad7944, we just documented that the first sample does not contain valid data and needs to be discarded.
Quoted 37 lines
Quoted 13 lines
Quoted 12 lines
+ *
bits_per_wordto ensure the minimum of SCLK cycles are used. And 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
+ * conversion time before asserting CS again.
+ *
+ * This timing is only valid if turbo mode is disabled (reading during acquisition).
+ */drivers/iio/adc/ad4000.c +3
-0+static int ad4000_prepare_offload_message (struct ad4000_state *st, +const struct iio_chan_spec *chan) +...
Quoted 7 lines
Quoted 7 lines
drivers/iio/adc/ad4000.c +1
-0+ xfers[1 ].bits_per_word = chan->scan_type.realbits; xfers[1 ].delay.value = st->time_spec->t_quiet2_ns; xfers[1 ].delay.unit =SPI_DELAY_UNIT_NSECS ;@@ -719,7 +1007,9 @@ static int ad4000_prepare_4wire_mode_message(struct ad4000_state *st, xfers[0 ].delay.unit =SPI_DELAY_UNIT_NSECS ;Quoted 6 lines
drivers/iio/adc/ad4000.c static int ad4000_prepare_3wire_mode_message(struct ad4000_state *st, · line 979
Quoted 4 lines
drivers/iio/adc/ad4000.c +2
-1xfers[1 ].rx_buf = &st->scan.data; - xfers[1 ].len =BITS_TO_BYTES (chan->scan_type.storagebits); + xfers[1 ].len = chan->scan_type.realbits >16 ?4 :2 ; +if (chan->scan_type.endianness !=IIO_BE )This is odd enough to require a comment. Why is endianness relevant?
When using SPI offloading (at least with PULSAR-ADC HDL project [1]), ADC data is read by SPI controller and pushed to DMA memory in CPU endianness. I don't know exactly where data gets rearranged in the data path (whether SPI-Engine, the DMA controller, or something else rearranges ADC data into CPU endianess). But I know, from testing with these ADCs and HDL project, that data is correct when read in CPU endianness because it converts back to expected mV values. When IIO buffers were set to
IIO_BEand SPI offloading is used, data just looked weird and didn't convert to expected values in mV.
Quoted 4 lines
[1]: https://analogdevicesinc.github.io/hdl/projects/pulsar_adc/index.html
Other IIO drivers also set
IIO_CPUbuffer endianness when using offload support, e.g. ad7944, ad7380.
These drivers also use IIO_CPU for the non-SPI offload case though.
Quoted 8 lines
They only say buffer would use 32 storagebits when using SPI offload.
https://lore.kernel.org/linux-iio/20250207-dlech-mainline-spi-engine-offload-2-v8-10-e48a489be48c@baylibre.com/
https://lore.kernel.org/linux-iio/20250220-wip-bl-spi-offload-ad7380-v1-1-838aa873e62a@baylibre.com/#tI also didn't expect to find out HDL support for 16-bit data width was removed. We used to have a build parameter for 16-bit precision ADCs.
https://github.com/analogdevicesinc/hdl/commit/b2dc91b30dae891b6319d88e083f26e726f43ba0#diff-1117c2618353232e5f22aa6a12e8ae976757fa897b3425f470a12123cae26535L13
A while back the HDL engineers mentioned to us that they wanted to standardize on 32-bit data words everywhere. While not the most efficient use of memory, having fewer options does make things simpler across the entire software stack.
Quoted 3 lines
Would something like 'because SPI offloading leads to data being pushed to memory in CPU endianness' be a reasonable comment?
Another way to say it is that SPI offload reads data in complete words and not in separate 8-bit xfers (bits_per_word = realbits vs. bits_per_word = 8).
Re: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload
Hi, comments inline.
Quoted 51 lines · David Lechner
Quoted 41 lines · Marcelo Schmitt
...
Quoted 11 lines
Quoted 10 lines
+/*
+ * 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 set to "high". In this connection mode, the ADC SDI pin is
+ * connected 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. This has the effect that the first sample data is not
+ * valid because it is reading the previous conversion result. We also useQuoted 2 lines
Say what happens to that invalid sample. Is it dropped or provided to userspace as if it were valid? (I hope dropped!)
TL;DR: The invalid sample goes into the buffer as a valid one.
In AD4000 '3-wire' mode, data capture has a latency (delay) of one sample.
The ADC begins sampling data N at CNV rising edge
| +-- CNV (usually SPI CS) is brought low to begin reading the data | | +-- Data N + 1 that will be read | | | on the next transfer starts v v v being sampled at end of transfer N.___ ____
CNV _____/ \________________________________/ \_____
_ _ _
SCLK ______________/ \___/ \_ ... ___/ \__________________ ___ ___ SDO _____________/___\_/___\ ... __/___\_______________ ^ | Data from conversion N is output from here onA better drawing can be found in datasheet page 29, Figure 57.
https://www.analog.com/media/en/technical-documentation/data-sheets/ADAQ4003.pdfIn sum, we're always reading a conversion that started at the end of the previous SPI transfer or, in other words, the data comes out with a latency (delay) of one read.
Quoted 6 lines
Datasheet somehow mentions that by saying
When turbo mode is enabled, the conversion result read on SDO corresponds to the result of the previous conversion.I think I can do a dummy SPI transfer on buffer preenable so at least the first data is not invalid. Would that be better?
Quoted 8 lines
Not really. There will be a relatively long delay between that conversion trigger and when the sample is read. So the data might be slightly less stale in that case, but still not particularly useful, e.g. if you are doing any kind of signal processing that expects equal time between all samples.
On similar chips, like ad7944, we just documented that the first sample does not contain valid data and needs to be discarded.
Okay, I'll assume that to be acceptable and do the same for this one.
...
Quoted 8 lines
Quoted 3 lines
I also didn't expect to find out HDL support for 16-bit data width was removed. We used to have a build parameter for 16-bit precision ADCs.
https://github.com/analogdevicesinc/hdl/commit/b2dc91b30dae891b6319d88e083f26e726f43ba0#diff-1117c2618353232e5f22aa6a12e8ae976757fa897b3425f470a12123cae26535L13A while back the HDL engineers mentioned to us that they wanted to standardize on 32-bit data words everywhere. While not the most efficient use of memory, having fewer options does make things simpler across the entire software stack.
Ack
Quoted 7 lines
Quoted 3 lines
Would something like 'because SPI offloading leads to data being pushed to memory in CPU endianness' be a reasonable comment?
Another way to say it is that SPI offload reads data in complete words and not in separate 8-bit xfers (
bits_per_word= realbits vs.bits_per_word= 8).
Ah sure, I recall the effect of setting .bits_per_word now. Will add a comment explaining why the difference in endianness.
Thanks,
Marcelo
Jonathan Cameron
maintainerRe: [PATCH v1 1/4] iio: adc: ad4000: Add support for SPI offload
Quoted 59 lines · On Mon, 17 Mar 2025 14:21:13 -0300 Marcelo Schmitt…
Hi, comments inline.
Quoted 56 lines · David Lechner
Quoted 41 lines · Marcelo Schmitt
...
Quoted 11 lines
Quoted 10 lines
+/*
+ * 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 set to "high". In this connection mode, the ADC SDI pin is
+ * connected 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. This has the effect that the first sample data is not
+ * valid because it is reading the previous conversion result. We also useQuoted 2 lines
Say what happens to that invalid sample. Is it dropped or provided to userspace as if it were valid? (I hope dropped!)
TL;DR: The invalid sample goes into the buffer as a valid one.
In AD4000 '3-wire' mode, data capture has a latency (delay) of one sample.
The ADC begins sampling data N at CNV rising edge
| +-- CNV (usually SPI CS) is brought low to begin reading the data | | +-- Data N + 1 that will be read | | | on the next transfer starts v v v being sampled at end of transfer N.___ ____
CNV _____/ \________________________________/ \_____
_ _ _
SCLK ______________/ \___/ \_ ... ___/ \__________________ ___ ___ SDO _____________/___\_/___\ ... __/___\_______________ ^ | Data from conversion N is output from here onA better drawing can be found in datasheet page 29, Figure 57.
https://www.analog.com/media/en/technical-documentation/data-sheets/ADAQ4003.pdfIn sum, we're always reading a conversion that started at the end of the previous SPI transfer or, in other words, the data comes out with a latency (delay) of one read.
Quoted 6 lines
Datasheet somehow mentions that by saying
When turbo mode is enabled, the conversion result read on SDO corresponds to the result of the previous conversion.I think I can do a dummy SPI transfer on buffer preenable so at least the first data is not invalid. Would that be better?
Not really. There will be a relatively long delay between that conversion trigger and when the sample is read. So the data might be slightly less stale in that case, but still not particularly useful, e.g. if you are doing any kind of signal processing that expects equal time between all samples.
Quoted 4 lines
Quoted 3 lines
On similar chips, like ad7944, we just documented that the first sample does not contain valid data and needs to be discarded.
Okay, I'll assume that to be acceptable and do the same for this one.
Hmm. Doesn't really pass the useability 'smell' test but I guess if there is precedence and clear docs it should be fine.
Quoted 14 lines
...
Quoted 8 lines
Quoted 3 lines
I also didn't expect to find out HDL support for 16-bit data width was removed. We used to have a build parameter for 16-bit precision ADCs.
https://github.com/analogdevicesinc/hdl/commit/b2dc91b30dae891b6319d88e083f26e726f43ba0#diff-1117c2618353232e5f22aa6a12e8ae976757fa897b3425f470a12123cae26535L13A while back the HDL engineers mentioned to us that they wanted to standardize on 32-bit data words everywhere. While not the most efficient use of memory, having fewer options does make things simpler across the entire software stack.
Ack
Quoted 1 line
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Would something like 'because SPI offloading leads to data being pushed to memory in CPU endianness' be a reasonable comment?
Another way to say it is that SPI offload reads data in complete words and not in separate 8-bit xfers (
bits_per_word= realbits vs.bits_per_word= 8).Ah sure, I recall the effect of setting .bits_per_word now. Will add a comment explaining why the difference in endianness.
Great
Quoted 3 lines
Thanks,
Marcelo
Marcelo Schmitt
maintainerPATCH 2/4 Documentation: iio: ad4000: Add new supported parts
In reply to Marcelo SchmittCommit <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.
--- Documentation/iio/ad4000.rst | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
Documentation/iio/ad4000.rst +16 -1
@@ -4,7 +4,7 @@ AD4000 driver ============= -Device driver for Analog Devices Inc. AD4000 series of ADCs. +Device driver for Analog Devices Inc. AD4000 series of ADCs and similar devices. Supported devices =================
@@ -25,6 +25,21 @@ Supported devices * `AD4022 <https://www.analog.com/AD4022>`_ * `ADAQ4001 <https://www.analog.com/ADAQ4001>`_ * `ADAQ4003 <https://www.analog.com/ADAQ4003>`_ +* `AD7685 <https://www.analog.com/AD7685>`_ +* `AD7686 <https://www.analog.com/AD7686>`_ +* `AD7687 <https://www.analog.com/AD7687>`_ +* `AD7688 <https://www.analog.com/AD7688>`_ +* `AD7690 <https://www.analog.com/AD7690>`_ +* `AD7691 <https://www.analog.com/AD7691>`_ +* `AD7693 <https://www.analog.com/AD7693>`_ +* `AD7942 <https://www.analog.com/AD7942>`_ +* `AD7946 <https://www.analog.com/AD7946>`_ +* `AD7980 <https://www.analog.com/AD7980>`_ +* `AD7982 <https://www.analog.com/AD7982>`_ +* `AD7983 <https://www.analog.com/AD7983>`_ +* `AD7984 <https://www.analog.com/AD7984>`_ +* `AD7988-1 <https://www.analog.com/AD7988-1>`_ +* `AD7988-5 <https://www.analog.com/AD7988-5>`_ Wiring connections ------------------
Signature · 1 line
2.47.2
Marcelo Schmitt
maintainerPATCH 3/4 Documentation: iio: ad4000: Add IIO Device characteristics section
In reply to Marcelo SchmittComplement ad4000 IIO driver documentation with considerations about ``_scale_available`` attribute and table of typical channel attributes.
--- Documentation/iio/ad4000.rst | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
Documentation/iio/ad4000.rst +47 -0
@@ -144,3 +144,50 @@ Set ``adi,sdi-pin`` to ``"cs"`` to select this mode. ^ | | +--------------------| SCLK | +-------------+ + +IIO Device characteristics +========================== + +The AD4000 series driver supports differential and pseudo-differential ADCs. + +The span compression feature available in AD4000 series devices can be +enabled/disabled by changing the ``_scale_available`` attribute of the voltage +channel. Note that span compression configuration requires writing to AD4000 +configuration register, which is only possible when the ADC is wired in 3-wire +turbo mode, and the SPI controller is ``SPI_MOSI_IDLE_HIGH`` capable. If those +conditions are not met, no ``_scale_available`` attribute is provided. + +Besides that, differential and pseudo-differential voltage channels present +slightly different sysfs interfaces. + +Pseudo-differential ADCs +------------------------ + +Typical voltage channel attributes of a pseudo-differential AD4000 series device: + ++-------------------------------------------+------------------------------------------+ +| Voltage Channel Attributes | Description | ++===========================================+==========================================+ +| ``in_voltage0_raw`` | Raw ADC output code. | ++-------------------------------------------+------------------------------------------+ +| ``in_voltage0_offset`` | Offset to convert raw value to mV. | ++-------------------------------------------+------------------------------------------+ +| ``in_voltage0_scale`` | Scale factor to convert raw value to mV. | ++-------------------------------------------+------------------------------------------+ +| ``in_voltage0_scale_available`` | Toggles input span compression | ++-------------------------------------------+------------------------------------------+ + +Differential ADCs +----------------- + +Typical voltage channel attributes of a differential AD4000 series device: + ++-------------------------------------------+------------------------------------------+ +| Voltage Channel Attributes | Description | ++===========================================+==========================================+ +| ``in_voltage0-voltage1_raw`` | Raw ADC output code. | ++-------------------------------------------+------------------------------------------+ +| ``in_voltage0-voltage1_scale`` | Scale factor to convert raw value to mV. | ++-------------------------------------------+------------------------------------------+ +| ``in_voltage0-voltage1_scale_available`` | Toggles input span compression | ++-------------------------------------------+------------------------------------------+
Signature · 1 line
2.47.2
Marcelo Schmitt
maintainerPATCH 4/4 Documentation: iio: ad4000: Describe offload support
In reply to Marcelo SchmittWhen 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.
--- Documentation/iio/ad4000.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Documentation/iio/ad4000.rst +17 -0
@@ -191,3 +191,20 @@ Typical voltage channel attributes of a differential AD4000 series device: +-------------------------------------------+------------------------------------------+ | ``in_voltage0-voltage1_scale_available`` | Toggles input span compression | +-------------------------------------------+------------------------------------------+ + +SPI offload support +------------------- + +To be able to achieve the maximum sample rate, the driver can be used with the +`AXI SPI Engine`_ to provide SPI offload support. + +.. _AXI SPI Engine: http://analogdevicesinc.github.io/hdl/projects/pulsar_adc/index.html + +When set for SPI offload support, the IIO device will provide different +interfaces. + +* Either ``in_voltage0_sampling_frequency`` or + ``in_voltage0-voltage1_sampling_frequency`` file is provided to allow setting + the sample rate. +* IIO trigger device is not provided (no ``trigger`` directory). +* ``timestamp`` channel is not provided.
Signature · 1 line
2.47.2
Jonathan Cameron
maintainerRe: [PATCH v1 4/4] Documentation: iio: ad4000: Describe offload support
Quoted 14 lines · On Fri, 14 Mar 2025 14:19:33 -0300 Marcelo Schmitt…
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]>--- Documentation/iio/ad4000.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)Documentation/iio/ad4000.rst +0
-0
Documentation/iio/ad4000.rst Typical voltage channel attributes of a differential AD4000 series device: · line 199
Quoted 9 lines
Documentation/iio/ad4000.rst +6
-0@@ -191,3 +191,20 @@ Typical voltage channel attributes of a differential AD4000 series device: +-------------------------------------------+------------------------------------------+ | ``in_voltage0-voltage1_scale_available`` | Toggles input span compression | +-------------------------------------------+------------------------------------------+ + +SPI offload support +------------------- + +To be able to achieve the maximum sample rate, the driver can be used with the +`AXI SPI Engine`_ to provide SPI offload support.
Can we make this more generic?
"the driver can be used with SPI offload engine, such as the ...
I'm rather hoping we see others in the long run as useful little things!
Quoted 11 lines
Documentation/iio/ad4000.rst +11
-0+ +.. _AXI SPI Engine: http://analogdevicesinc.github.io/hdl/projects/pulsar_adc/index.html + +When set for SPI offload support, the IIO device will provide different +interfaces. + +* Either ``in_voltage0_sampling_frequency`` or + ``in_voltage0-voltage1_sampling_frequency`` file is provided to allow setting + the sample rate. +* IIO trigger device is not provided (no ``trigger`` directory). +* ``timestamp`` channel is not provided.
Re: [PATCH v1 4/4] Documentation: iio: ad4000: Describe offload support
Quoted 17 lines · Jonathan Cameron
Quoted 14 lines · On Fri, 14 Mar 2025 14:19:33 -0300 Marcelo Schmitt…
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]>--- Documentation/iio/ad4000.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)Documentation/iio/ad4000.rst +0
-0@@ -191,3 +191,20 @@ Typical voltage channel attributes of a differential AD4000 series device:
...
Quoted 10 lines
Documentation/iio/ad4000.rst Typical voltage channel attributes of a differential AD4000 series device: · line 199
Quoted 5 lines
Documentation/iio/ad4000.rst +5
-0+SPI offload support +------------------- + +To be able to achieve the maximum sample rate, the driver can be used with the +`AXI SPI Engine`_ to provide SPI offload support.Can we make this more generic?
"the driver can be used with SPI offload engine, such as the ...I'm rather hoping we see others in the long run as useful little things!
Okay, will do.
Quoted 3 lines
Quoted 3 lines
Documentation/iio/ad4000.rst +3
-0+ +.. _AXI SPI Engine: http://analogdevicesinc.github.io/hdl/projects/pulsar_adc/index.html +