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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240112124509.000001b8@Huawei.com>
Date: Fri, 12 Jan 2024 12:45:09 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: David Lechner <dlechner@...libre.com>
CC: Mark Brown <broonie@...nel.org>, Jonathan Cameron <jic23@...nel.org>, Rob
 Herring <robh+dt@...nel.org>, Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
	Michael Hennerich <michael.hennerich@...log.com>, Nuno Sá <nuno.sa@...log.com>, Frank Rowand
	<frowand.list@...il.com>, Thierry Reding <thierry.reding@...il.com>, Uwe
 Kleine-König <u.kleine-koenig@...gutronix.de>, "Jonathan
 Corbet" <corbet@....net>, <linux-spi@...r.kernel.org>,
	<linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-doc@...r.kernel.org>, <linux-pwm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 12/13] iio: offload: add new PWM triggered DMA buffer
 driver

On Wed, 10 Jan 2024 13:49:53 -0600
David Lechner <dlechner@...libre.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@...libre.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.

> +}