On Fri, 29 Aug 2025 21:42:50 -0300
Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx> wrote:
> AD4030 and similar ADCs can capture data at sample rates up to 2 mega
> samples per second (MSPS). Not all SPI controllers are able to achieve
> such high throughputs and even when the controller is fast enough to run
> transfers at the required speed, it may be costly to the CPU to handle
> transfer data at such high sample rates. Add SPI offload support for
> AD4030 and similar ADCs so to enable ADC data capture at maximum sample
> rates.
>
> Cc: Sergiu Cuciurean <sergiu.cuciurean@xxxxxxxxxx>
> Cc: Nuno Sa <nuno.sa@xxxxxxxxxx>
> Cc: Trevor Gamblin <tgamblin@xxxxxxxxxxxx>
> Cc: Axel Haslam <ahaslam@xxxxxxxxxxxx>
> Cc: David Lechner <dlechner@xxxxxxxxxxxx>
> Co-developed-by: Sergiu Cuciurean <sergiu.cuciurean@xxxxxxxxxx>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@xxxxxxxxxx>
> Co-developed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> Co-developed-by: Trevor Gamblin <tgamblin@xxxxxxxxxxxx>
> Signed-off-by: Trevor Gamblin <tgamblin@xxxxxxxxxxxx>
> Co-developed-by: Axel Haslam <ahaslam@xxxxxxxxxxxx>
> Signed-off-by: Axel Haslam <ahaslam@xxxxxxxxxxxx>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx>
> ---
> Most of the code in this patch is based on work from Sergiu Cuciurean, Nuno Sa,
> Axel Haslam, and Trevor Gamblin, hence the many co-developed-by tags. I also
> draw inspiration from other drivers supporting SPI offload, many of them written
> by David Lechner.
A few things inline.
> +
> +static int ad4030_set_sampling_freq(struct iio_dev *indio_dev, unsigned int freq)
> +{
> + struct ad4030_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + if (PTR_ERR_OR_ZERO(st->offload))
> + return -EINVAL;
> +
> + if (!freq || freq > st->chip->max_sample_rate_hz)
> + return -EINVAL;
> +
> + ret = __ad4030_set_sampling_freq(st, freq);
> + iio_device_release_direct(indio_dev);
Where is the claim?
> +
> + return ret;
> +}
>
> static int ad4030_update_scan_mode(struct iio_dev *indio_dev,
> @@ -903,6 +1038,67 @@ static const struct iio_buffer_setup_ops ad4030_buffer_setup_ops = {
> .validate_scan_mask = ad4030_validate_scan_mask,
> };
>
> +static int ad4030_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct ad4030_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_write(st->regmap, AD4030_REG_EXIT_CFG_MODE, BIT(0));
> + if (ret)
> + return ret;
> +
> + st->offload_msg.offload = st->offload;
> + ret = spi_optimize_message(st->spi, &st->offload_msg);
> + if (ret < 0)
> + goto out_reset_mode;
> +
> + ret = pwm_set_waveform_might_sleep(st->conv_trigger, &st->conv_wf, false);
> + if (ret)
> + goto out_unoptimize;
> +
> + ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
> + &st->offload_trigger_config);
> + if (ret)
> + goto out_pwm_disable;
Blank line here.
> + return 0;
Blank line here.
> +out_pwm_disable:
> + pwm_disable(st->conv_trigger);
> +out_unoptimize:
> + spi_unoptimize_message(&st->offload_msg);
> +out_reset_mode:
> + /* reenter register configuration mode */
> + ret = ad4030_enter_config_mode(st);
> + if (ret)
> + dev_warn(&st->spi->dev,
> + "couldn't reenter register configuration mode\n");
> + return ret;
> +}
> +
> +static void ad4030_prepare_offload_msg(struct ad4030_state *st)
> +{
> + u8 data_width = st->chip->precision_bits;
> + u8 offload_bpw;
> +
> + if (st->lane_mode == AD4030_LANE_MD_INTERLEAVED)
> + /*
> + * This means all channels on 1 lane.
> + */
Single line comment looks like enough here.
> + offload_bpw = data_width * st->chip->num_voltage_inputs;
> + else
> + offload_bpw = data_width;
> +
> + st->offload_xfer.speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED;
> + st->offload_xfer.bits_per_word = offload_bpw;
> + st->offload_xfer.len = roundup_pow_of_two(BITS_TO_BYTES(offload_bpw));
> + st->offload_xfer.offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> + spi_message_init_with_transfers(&st->offload_msg, &st->offload_xfer, 1);
> +}
> @@ -1103,6 +1393,20 @@ static const struct iio_scan_type ad4030_24_scan_types[] = {
> .shift = 2,
> .endianness = IIO_BE,
> },
> + [AD4030_OFFLOAD_SCAN_TYPE_NORMAL] = {
> + .sign = 's',
> + .storagebits = 32,
> + .realbits = 24,
> + .shift = 0,
> + .endianness = IIO_CPU,
> + },
> + [AD4030_OFFLOAD_SCAN_TYPE_AVG] = {
> + .sign = 's',
> + .storagebits = 32,
> + .realbits = 30,
> + .shift = 2,
> + .endianness = IIO_CPU,
> + },
> };
>
> static const struct iio_scan_type ad4030_16_scan_types[] = {
> @@ -1119,7 +1423,21 @@ static const struct iio_scan_type ad4030_16_scan_types[] = {
> .realbits = 30,
> .shift = 2,
> .endianness = IIO_BE,
> - }
> + },
> + [AD4030_OFFLOAD_SCAN_TYPE_NORMAL] = {
> + .sign = 's',
> + .storagebits = 32,
> + .realbits = 16,
> + .shift = 0,
> + .endianness = IIO_CPU,
> + },
> + [AD4030_OFFLOAD_SCAN_TYPE_AVG] = {
> + .sign = 's',
> + .storagebits = 32,
> + .realbits = 30,
> + .shift = 2,
> + .endianness = IIO_CPU,
> + },
> };
>
> static const struct ad4030_chip_info ad4030_24_chip_info = {
> @@ -1130,10 +1448,15 @@ static const struct ad4030_chip_info ad4030_24_chip_info = {
> AD4030_CHAN_CMO(1, 0),
> IIO_CHAN_SOFT_TIMESTAMP(2),
> },
> + .offload_channels = {
> + AD4030_OFFLOAD_CHAN_DIFF(0, ad4030_24_scan_types),
This array still has the non offload cases. Do they make sense?
> + AD4030_CHAN_CMO(1, 0),
> + },
> .grade = AD4030_REG_CHIP_GRADE_AD4030_24_GRADE,
> .precision_bits = 24,
> .num_voltage_inputs = 1,
> .tcyc_ns = AD4030_TCYC_ADJUSTED_NS,
> + .max_sample_rate_hz = 2 * MEGA,
> };
