Re: [PATCH v3 6/8] iio: adc: Add offload support for ad4052

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 10 Jun 2025 09:34:39 +0200
Jorge Marques <jorge.marques@analog.com> wrote:

> Support SPI offload with appropriate FPGA firmware. Since the SPI-Engine
> offload module always sends 32-bit data to the DMA engine, the
> scantype.storagebytes is set to 32-bit and the SPI transfer length is
> based on the scantype.realbits. This combination allows to optimize the
> SPI to transfer only 2 or 3 bytes (depending on the granularity and
> mode), while the number of samples are computed correctly by tools on
> top of the iio scantype.
> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Minor comments inline.  I think they are all follow up from comments on
earlier patches that apply here as well.

> ---
>  drivers/iio/adc/ad4052.c | 244 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 242 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4052.c b/drivers/iio/adc/ad4052.c
> index 842f5972a1c58701addf5243e7b87da9c26c773f..7d32dc4701ddb0204b5505a650ce7caafc2cb5ed 100644
> --- a/drivers/iio/adc/ad4052.c
> +++ b/drivers/iio/adc/ad4052.c
> @@ -11,6 +11,8 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/buffer-dmaengine.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/interrupt.h>
> @@ -23,6 +25,8 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
> +#include <linux/spi/offload/consumer.h>
> +#include <linux/spi/offload/provider.h>
>  #include <linux/string.h>
>  #include <linux/types.h>
>  #include <linux/units.h>
> @@ -111,6 +115,7 @@ enum ad4052_interrupt_en {
>  
>  struct ad4052_chip_info {
>  	const struct iio_chan_spec channels[1];
> +	const struct iio_chan_spec offload_channels[1];

If there is only ever one of these drop the array.


>  
> +static int ad4052_update_xfer_offload(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan)
> +{
> +	struct ad4052_state *st = iio_priv(indio_dev);
> +	const struct iio_scan_type *scan_type;
> +	struct spi_transfer *xfer = &st->offload_xfer;
> +
> +	scan_type = iio_get_current_scan_type(indio_dev, chan);
> +	if (IS_ERR(scan_type))
> +		return PTR_ERR(scan_type);
> +
> +	xfer->bits_per_word = scan_type->realbits;
> +	xfer->offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +	xfer->len = scan_type->realbits == 24 ? 4 : 2;

Same question on length vs bits_per_word applies here as in the earlier
patch.

> +	xfer->speed_hz = AD4052_SPI_MAX_ADC_XFER_SPEED(st->vio_uv);
> +
> +	spi_message_init_with_transfers(&st->offload_msg, &st->offload_xfer, 1);
> +	st->offload_msg.offload = st->offload;
> +
> +	return 0;
> +}
> +
>  static int ad4052_set_oversampling_ratio(struct iio_dev *indio_dev,
>  					 const struct iio_chan_spec *chan,
>  					 unsigned int val)
> @@ -838,6 +873,87 @@ static int ad4052_write_raw(struct iio_dev *indio_dev,
>  	return ret;
>  }

>  static int __ad4052_validate_trigger_sources(struct of_phandle_args *trigger_sources)
>  {
>  	switch (trigger_sources->args[1]) {
> +	case AD4052_TRIGGER_PIN_GP0:
> +		return trigger_sources->args[0] == AD4052_TRIGGER_EVENT_EITHER_THRESH ?
> +		       0 : -EINVAL;
>  	case AD4052_TRIGGER_PIN_GP1:
>  		return trigger_sources->args[0] == AD4052_TRIGGER_EVENT_DATA_READY ?
>  		       0 : -EINVAL;
> @@ -903,14 +1092,45 @@ static int ad4052_validate_trigger_sources(struct iio_dev *indio_dev)
>  	int ret;
>  
>  	np = st->spi->dev.of_node;
> +	for (u8 i = 0; i < 2; i++) {
> +		ret = of_parse_phandle_with_args(np, "trigger-sources",
> +						 "#trigger-source-cells", i,
> +						 &trigger_sources);
> +		if (ret)
> +			return ret;
> +
> +		ret = __ad4052_validate_trigger_sources(&trigger_sources);
> +		of_node_put(trigger_sources.np);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return ret;

I think this is always 0. So return 0; preferred to make that explicit.

> +}
>