Re: [PATCH v6 17/17] iio: dac: ad5791: Add offload support

[Jonathan Cameron <jic23@xxxxxxxxxx>]

On Wed, 11 Dec 2024 14:54:54 -0600
David Lechner <dlechner@xxxxxxxxxxxx> wrote:

> From: Axel Haslam <ahaslam@xxxxxxxxxxxx>
> 
> Add SPI offload support to stream TX buffers using DMA.
> This allows loading samples to the DAC with a rate of 1 MSPS.
> 
> Signed-off-by: Axel Haslam <ahaslam@xxxxxxxxxxxx>
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
> ---
> 
> v6 changes: new patch in v6

Nice.   A few formatting micro comments inline.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

For merging this ultimately I'm kind of assuming Mark Brown (or I) will
spin an immutable branch with the SPI parts and then I'll pull that into the
IIO tree and apply patch 8 onwards on top.

Before that point we need DT folk and Mark to be happy of course.

Thanks,

Jonathan


> @@ -299,6 +336,24 @@ static const struct ad5791_chip_info _name##_chip_info = {		\
>  			},						\
>  			.ext_info = ad5791_ext_info,			\
>  	},								\
> +	.channel_offload = {						\
> +			.type = IIO_VOLTAGE,				\

Reduce indent by 1 tab for these.

> +			.output = 1,					\
> +			.indexed = 1,					\
> +			.address = AD5791_ADDR_DAC0,			\
> +			.channel = 0,					\
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +			.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_OFFSET),		\
> +			.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> +			.scan_type = {					\
> +				.sign = 'u',				\
> +				.realbits = (bits),			\
> +				.storagebits = 32,			\
> +				.shift = (_shift),			\
> +			},						\
> +			.ext_info = ad5791_ext_info,			\
> +	},								\
>  }
>  
>  AD5791_DEFINE_CHIP_INFO(ad5760, 16, 4, ad5780_get_lin_comp);
> @@ -322,16 +377,95 @@ static int ad5791_write_raw(struct iio_dev *indio_dev,
>  
>  		return ad5791_spi_write(st, chan->address, val);
>  
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (val < 0 || val2 < 0)

Given you ignore val2, is val2 != 0 more appropriate?

> +			return -EINVAL;
> +		return ad5791_set_sample_freq(st, val);
>  	default:
>  		return -EINVAL;
>  	}
>  }

>  static int ad5791_probe(struct spi_device *spi)
>  {
>  	const struct ad5791_platform_data *pdata = dev_get_platdata(&spi->dev);
> @@ -416,6 +550,21 @@ static int ad5791_probe(struct spi_device *spi)
>  	indio_dev->channels = &st->chip_info->channel;
>  	indio_dev->num_channels = 1;
>  	indio_dev->name = st->chip_info->name;
> +
> +	st->offload = devm_spi_offload_get(&spi->dev, spi, &ad5791_offload_config);
> +	ret = PTR_ERR_OR_ZERO(st->offload);
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(&spi->dev, ret, "failed to get offload\n");
> +
> +	if (ret != -ENODEV) {
> +		indio_dev->channels = &st->chip_info->channel_offload;
> +		indio_dev->setup_ops = &ad5791_buffer_setup_ops;
> +		ret =  ad5791_offload_setup(indio_dev);

bonus space after =

> +		if (ret)
> +			return dev_err_probe(&spi->dev, ret,
> +					     "fail to setup offload\n");
> +	}
> +
>  	return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>  
> @@ -452,3 +601,4 @@ module_spi_driver(ad5791_driver);
>  MODULE_AUTHOR("Michael Hennerich <michael.hennerich@xxxxxxxxxx>");
>  MODULE_DESCRIPTION("Analog Devices AD5760/AD5780/AD5781/AD5790/AD5791 DAC");
>  MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS("IIO_DMAENGINE_BUFFER");
>