Re: [PATCH 12/13] iio: offload: add new PWM triggered DMA buffer driver

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 10 Jan 2024 13:49:53 -0600
David Lechner <dlechner@baylibre.com> wrote:

> This adds a new driver for handling SPI offloading using a PWM as the
> trigger and DMA for the received data. This will be used by ADCs in
> conjunction with SPI controllers with offloading support to be able
> to sample at high rates without CPU intervention.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Ah. So if I read this right, the trigger only exists to provide somewhere
to hang the frequency control?   Not sure it's worth the complexity for that.


Do we expect a given offload engine to allow a bunch of 'standard' triggers?
If that's the case then I don't mind them being exposed as triggers. We do
that for a few other devices where we need to pick between a bunch of
different internal signals and it works fine.

> +
> +static int iio_pwm_triggered_dma_buffer_probe(struct platform_device *pdev)
> +{
> +	struct iio_pwm_triggered_dma_buffer *st;
> +	struct auxiliary_device *adev;
> +	int ret;
> +
> +	st = devm_kzalloc(&pdev->dev, sizeof(*st), GFP_KERNEL);
> +	if (!st)
> +		return -ENOMEM;
> +
> +	st->pwm = devm_pwm_get(&pdev->dev, NULL);
> +	if (IS_ERR(st->pwm))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(st->pwm),
> +				     "failed to get PWM\n");
> +
> +	st->hw.buffer = devm_iio_dmaengine_buffer_alloc(&pdev->dev, "rx");
> +	if (IS_ERR(st->hw.buffer))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(st->hw.buffer),
> +				     "failed to allocate buffer\n");
> +
> +	st->hw.trig = devm_iio_trigger_alloc(&pdev->dev, "%s-%s-pwm-trigger",
> +					     dev_name(pdev->dev.parent),
> +					     dev_name(&pdev->dev));
> +	if (!st->hw.trig)
> +		return -ENOMEM;
> +
> +	st->hw.trig->ops = &iio_pwm_triggered_dma_buffer_ops;
> +	st->hw.trig->dev.parent = &pdev->dev;
> +	st->hw.trig->dev.groups = iio_pwm_triggered_dma_buffer_groups;
> +	iio_trigger_set_drvdata(st->hw.trig, st);
> +
> +	/* start with a reasonable default value */
> +	ret = axi_spi_engine_offload_set_samp_freq(st, 1000);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to set sampling frequency\n");
> +
> +	ret = devm_iio_trigger_register(&pdev->dev, st->hw.trig);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to register trigger\n");
> +
> +	adev = &st->hw.adev;
> +	adev->name = "triggered-buffer";
> +	adev->dev.parent = &pdev->dev;
> +	adev->dev.release = iio_pwm_triggered_dma_buffer_adev_release;
> +	adev->id = 0;
> +
> +	ret = auxiliary_device_init(adev);
> +	if (ret)
> +		return ret;
> +
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(&pdev->dev,
> +			iio_pwm_triggered_dma_buffer_unregister_adev, adev);
Split this an register the uninit and delete as separate callbacks to so we
can clearly see what each is doing.

> +}