Re: [PATCH 3/3] iio: adc: ad7768-1: add support for SPI offload

[David Lechner <dlechner@baylibre.com>]

On 1/31/26 7:35 PM, Jonathan Santos wrote:
> The AD7768-1 family supports sampling rates up to 1 MSPS, which exceeds
> the capabilities of conventional triggered buffer operations due to SPI
> transaction overhead and interrupt latency.
> 
> Add SPI offload support to enable hardware-accelerated data acquisition
> that bypasses software SPI transactions using continuous data streaming.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

...

>  struct ad7768_state {
>  	struct spi_device *spi;
> +	struct spi_offload *offload;
> +	struct spi_offload_trigger *offload_trigger;
>  	struct regmap *regmap;
>  	struct regmap *regmap24;
>  	int vref_uv;
> @@ -306,8 +325,11 @@ struct ad7768_state {
>  	struct gpio_desc *gpio_reset;
>  	const char *labels[AD7768_MAX_CHANNELS];
>  	struct gpio_chip gpiochip;
> +	struct spi_transfer offload_xfer;
> +	struct spi_message offload_msg;
>  	const struct ad7768_chip_info *chip;
>  	bool en_spi_sync;
> +	bool offload_en;

offload_en makes it sound like the offload could be enabled or disabled
at runtime, but that doesn't seem to be the case. I would just check if
offload != NULL rather than add a new field. Or change this to has_offload.

>  	struct mutex pga_lock; /* protect device internal state (PGA) */
>  	/*
>  	 * DMA (thus cache coherency maintenance) may require the
> @@ -1139,6 +1161,10 @@ static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
>  {
>  	struct ad7768_state *st = iio_priv(indio_dev);
>  
> +	if (st->offload_en)
> +		return st->oversampling_ratio == 8 ?
> +			AD7768_SCAN_TYPE_OFFLOAD_HIGH_SPEED : AD7768_SCAN_TYPE_OFFLOAD_NORMAL;
> +
>  	return st->oversampling_ratio == 8 ?
>  	       AD7768_SCAN_TYPE_HIGH_SPEED : AD7768_SCAN_TYPE_NORMAL;
>  }
> @@ -1320,7 +1346,7 @@ static irqreturn_t ad7768_interrupt(int irq, void *dev_id)
>  	struct iio_dev *indio_dev = dev_id;
>  	struct ad7768_state *st = iio_priv(indio_dev);
>  
> -	if (iio_buffer_enabled(indio_dev))
> +	if (iio_buffer_enabled(indio_dev) && !st->offload_en)

I would expect that the interrupt is just disabled when doing a buffered
read with SPI offload. No sense in wasting CPU time. So no special handling
should be needed here.

>  		iio_trigger_poll(st->trig);
>  	else
>  		complete(&st->completion);
> @@ -1357,6 +1383,72 @@ static const struct iio_buffer_setup_ops ad7768_buffer_ops = {
>  	.predisable = &ad7768_buffer_predisable,
>  };
>  
> +static int ad7768_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	struct spi_offload_trigger_config config = {
> +		.type = SPI_OFFLOAD_TRIGGER_DATA_READY,
> +	};
> +	const struct iio_scan_type *scan_type;
> +	int ret;
> +
> +	scan_type = iio_get_current_scan_type(indio_dev, &indio_dev->channels[0]);
> +	if (IS_ERR(scan_type))
> +		return PTR_ERR(scan_type);
> +
> +	st->offload_xfer.len = roundup_pow_of_two(BITS_TO_BYTES(scan_type->realbits));

We have spi_bpw_to_bytes() for this.

> +	st->offload_xfer.bits_per_word = scan_type->realbits;
> +	st->offload_xfer.offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +
> +	/*
> +	 * Write a 1 to the LSB of the INTERFACE_FORMAT register to enter
> +	 * continuous read mode. Subsequent data reads do not require an
> +	 * initial 8-bit write to query the ADC_DATA register.
> +	 */
> +	ret =  regmap_write(st->regmap, AD7768_REG_INTERFACE_FORMAT, 0x01);
> +	if (ret)
> +		return ret;
> +
> +	spi_message_init_with_transfers(&st->offload_msg, &st->offload_xfer, 1);
> +	st->offload_msg.offload = st->offload;
> +
> +	ret = spi_optimize_message(st->spi, &st->offload_msg);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "failed to prepare offload, err: %d\n", ret);

Do we need to do someting to undo AD7768_REG_INTERFACE_FORMAT write if this fails?

> +		return ret;
> +	}
> +
> +	ret = spi_offload_trigger_enable(st->offload,
> +					 st->offload_trigger,
> +					 &config);
> +	if (ret) {
> +		spi_unoptimize_message(&st->offload_msg);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +

...

> +static bool ad7768_offload_trigger_match(struct spi_offload_trigger *trigger,
> +					 enum spi_offload_trigger_type type,
> +					 u64 *args, u32 nargs)
> +{
> +	if (type != SPI_OFFLOAD_TRIGGER_DATA_READY)
> +		return false;
> +
> +	/* Requires 1 arg to indicate the trigger output signal */

The DT bindings allow 1 or 2 args, so we should also allow nargs == 2 even (the
2nd arg is just ignored).

> +	if (nargs != 1 || args[0] != AD7768_TRIGGER_SOURCE_DRDY)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int ad7768_offload_trigger_request(struct spi_offload_trigger *trigger,
> +					  enum spi_offload_trigger_type type,
> +					  u64 *args, u32 nargs)
> +{
> +	/* Should already be validated by match, but just in case. */
> +	if (nargs != 1)

As above.

> +		return -EINVAL;
> +
> +	return 0;
> +}
> +