[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250308161717.3842cbdf@jic23-huawei>
Date: Sat, 8 Mar 2025 16:17:17 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Angelo Dureghello <adureghello@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
<Michael.Hennerich@...log.com>, Nuno Sá <nuno.sa@...log.com>,
David Lechner <dlechner@...libre.com>, Jonathan Corbet <corbet@....net>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, Julien Stephan <jstephan@...libre.com>
Subject: Re: [PATCH v3 1/2] iio: ad7380: add support for SPI offload
On Tue, 04 Mar 2025 16:25:44 +0100
Angelo Dureghello <adureghello@...libre.com> 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@...libre.com>
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@baylibre.com/
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
Powered by blists - more mailing lists