linux-kernel - Re: [PATCH RFC v3 7/9] iio: buffer-dmaengine: generalize requesting DMA channel

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: <20240727144303.4a8604cb@jic23-huawei>
Date: Sat, 27 Jul 2024 14:43:03 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Nuno Sá <nuno.sa@...log.com>, Michael
 Hennerich <Michael.Hennerich@...log.com>, Lars-Peter Clausen
 <lars@...afoo.de>, David Jander <david@...tonic.nl>, Martin Sperl
 <kernel@...tin.sperl.org>, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH RFC v3 7/9] iio: buffer-dmaengine: generalize requesting
 DMA channel

On Mon, 22 Jul 2024 16:57:14 -0500
David Lechner <dlechner@...libre.com> wrote:

> This patch generalizes the iio_dmaengine_buffer_setup_ext() functions
> by passing the pointer to the DMA channel as an argument rather than
> the channel name. This will allow future callers of the function to
> use other methods to get the DMA channel pointer.
> 
> This aims to keep it as easy to use as possible by stealing ownership
> of the dma_chan pointer from the caller. This way, dma_request_chan()
> can be called inline in the function call without any extra error
> handling.

That's odd enough to be a likely source of future bugs. Doesn't seem
necessary to me. Just have the extra handling in the few places it's needed.

Or add a wrapper for this case where you just provide the
channel name as was done before this patch.

> 
> Signed-off-by: David Lechner <dlechner@...libre.com>
> 
> v3 changes:
> * This is a new patch in v3.


...

> @@ -277,22 +282,26 @@ static void __devm_iio_dmaengine_buffer_free(void *buffer)
>   * devm_iio_dmaengine_buffer_setup_ext() - Setup a DMA buffer for an IIO device
>   * @dev: Parent device for the buffer
>   * @indio_dev: IIO device to which to attach this buffer.
> - * @channel: DMA channel name, typically "rx".
> + * @chan: DMA channel.
>   * @dir: Direction of buffer (in or out)
>   *
>   * This allocates a new IIO buffer with devm_iio_dmaengine_buffer_alloc()
>   * and attaches it to an IIO device with iio_device_attach_buffer().
>   * It also appends the INDIO_BUFFER_HARDWARE mode to the supported modes of the
>   * IIO device.
> + *
> + * This "steals" the @chan pointer, so the caller must not call
> + * dma_release_channel() on it. @chan is also checked for error, so callers
> + * can pass the result of dma_request_chan() directly.
>   */
>  int devm_iio_dmaengine_buffer_setup_ext(struct device *dev,
>  					struct iio_dev *indio_dev,
> -					const char *channel,
> +					struct dma_chan *chan,
>  					enum iio_buffer_direction dir)
>  {
>  	struct iio_buffer *buffer;
>  
> -	buffer = iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel, dir);
> +	buffer = iio_dmaengine_buffer_setup_ext(dev, indio_dev, chan, dir);
>  	if (IS_ERR(buffer))
>  		return PTR_ERR(buffer);
>