On Tue, 04 Mar 2025 16:25:44 +0100 Angelo Dureghello <adureghello@xxxxxxxxxxxx> wrote: > Add support for SPI offload to the ad7380 driver. SPI offload allows > sampling data at the max sample rate (2MSPS with one SDO line). > > This is developed and tested against the ADI example FPGA design for > this family of ADCs [1]. > > [1]: http://analogdevicesinc.github.io/hdl/projects/ad738x_fmc/index.html > > Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx> Hi Angelo A couple of trivial comments inline. This has crossed with Julien adding an extra device to the supported set. I could have guessed what the necessary changes were but probably better for you to do it and check for any problems. https://lore.kernel.org/all/20250226-ad7380-add-adaq4381-4-support-v1-1-f350ab872d37@xxxxxxxxxxxx/ Jonathan > --- > drivers/iio/adc/Kconfig | 2 + > drivers/iio/adc/ad7380.c | 509 +++++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 475 insertions(+), 36 deletions(-) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 27413516216cb3f83cf1d995b9ffc22bf01776a4..c528f4632c0ef6782269d8afa89c17d2046d28a3 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -218,7 +218,9 @@ config AD7298 > config AD7380 > tristate "Analog Devices AD7380 ADC driver" > depends on SPI_MASTER > + select SPI_OFFLOAD > select IIO_BUFFER > + select IIO_BUFFER_DMAENGINE > select IIO_TRIGGER > select IIO_TRIGGERED_BUFFER > help > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c > index f232ad1a49634baeedc655916bc7a967604a1206..39a5e55fa7e8a6706e15750d07fa4b0fda7175eb 100644 > --- a/drivers/iio/adc/ad7380.c > +++ b/drivers/iio/adc/ad7380.c > @@ -15,6 +15,9 @@ > * ad7386/7/8-4 : https://www.analog.com/media/en/technical-documentation/data-sheets/ad7386-4-7387-4-7388-4.pdf > * adaq4370-4 : https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4370-4.pdf > * adaq4380-4 : https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4380-4.pdf > + * > + * HDL ad738x_fmc: https://analogdevicesinc.github.io/hdl/projects/ad738x_fmc/index.html > + * Pet dislike of mine. No lines at ends of comment blocks with nothing on them. The */ provides any necessary space. > */ > > #include <linux/align.h> > @@ -29,11 +32,13 @@ > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > +#include <linux/spi/offload/consumer.h> > #include <linux/spi/spi.h> > #include <linux/units.h> > #include <linux/util_macros.h> > > #include <linux/iio/buffer.h> > +#include <linux/iio/buffer-dmaengine.h> > #include <linux/iio/events.h> > #include <linux/iio/iio.h> > #include <linux/iio/trigger_consumer.h> > @@ -92,6 +97,12 @@ > #define AD7380_NUM_SDO_LINES 1 > #define AD7380_DEFAULT_GAIN_MILLI 1000 > > +/* > + * Using SPI offload, storagebits is always 32, so can't be used to compute struct > + * spi_transfer.len. Using realbits instead. > + */ > +#define AD7380_SPI_BYTES(scan_type) ((scan_type)->realbits > 16 ? 4 : 2) > + > struct ad7380_timing_specs { > const unsigned int t_csh_ns; /* CS minimum high time */ > }; > @@ -99,6 +110,7 @@ struct ad7380_timing_specs { > struct ad7380_chip_info { > const char *name; > const struct iio_chan_spec *channels; > + const struct iio_chan_spec *offload_channels; > unsigned int num_channels; > unsigned int num_simult_channels; > bool has_hardware_gain; > @@ -111,6 +123,7 @@ struct ad7380_chip_info { > unsigned int num_vcm_supplies; > const unsigned long *available_scan_masks; > const struct ad7380_timing_specs *timing_specs; > + u32 max_conversion_rate_hz; > }; > > static const struct iio_event_spec ad7380_events[] = { > @@ -216,6 +229,91 @@ static const struct iio_scan_type ad7380_scan_type_16_u[] = { > }, > }; > > +/* > + * Defining here scan types for offload mode, since with current available HDL > + * only a value of 32 for storagebits is supported. > + */ > + > +/* Extended scan types for 12-bit unsigned chips, offload support. */ > +static const struct iio_scan_type ad7380_scan_type_12_u_offload[] = { > + [AD7380_SCAN_TYPE_NORMAL] = { > + .sign = 'u', > + .realbits = 12, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + [AD7380_SCAN_TYPE_RESOLUTION_BOOST] = { > + .sign = 'u', > + .realbits = 14, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > +}; > + > +/* Extended scan types for 14-bit signed chips, offload support. */ > +static const struct iio_scan_type ad7380_scan_type_14_s_offload[] = { > + [AD7380_SCAN_TYPE_NORMAL] = { > + .sign = 's', > + .realbits = 14, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + [AD7380_SCAN_TYPE_RESOLUTION_BOOST] = { > + .sign = 's', > + .realbits = 16, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > +}; > + > +/* Extended scan types for 14-bit unsigned chips, offload support. */ > +static const struct iio_scan_type ad7380_scan_type_14_u_offload[] = { > + [AD7380_SCAN_TYPE_NORMAL] = { > + .sign = 'u', > + .realbits = 14, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + [AD7380_SCAN_TYPE_RESOLUTION_BOOST] = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > +}; > + > +/* Extended scan types for 16-bit signed_chips, offload support. */ > +static const struct iio_scan_type ad7380_scan_type_16_s_offload[] = { > + [AD7380_SCAN_TYPE_NORMAL] = { > + .sign = 's', > + .realbits = 16, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + [AD7380_SCAN_TYPE_RESOLUTION_BOOST] = { > + .sign = 's', > + .realbits = 18, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > +}; > + > +/* Extended scan types for 16-bit unsigned chips, offload support. */ > +static const struct iio_scan_type ad7380_scan_type_16_u_offload[] = { > + [AD7380_SCAN_TYPE_NORMAL] = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + [AD7380_SCAN_TYPE_RESOLUTION_BOOST] = { > + .sign = 'u', > + .realbits = 18, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > +}; You could have perhaps used a macro for these to reduce repetition but it is perhaps slightly more readable without doing that. Jonathan