diff mbox series

[RFC,v3,7/9] iio: buffer-dmaengine: generalize requesting DMA channel

Message ID 20240722-dlech-mainline-spi-engine-offload-2-v3-7-7420e45df69b@baylibre.com (mailing list archive)
State Changes Requested
Headers show
Series spi: axi-spi-engine: add offload support | expand

Commit Message

David Lechner July 22, 2024, 9:57 p.m. UTC
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.

Signed-off-by: David Lechner <dlechner@baylibre.com>

v3 changes:
* This is a new patch in v3.
---
 drivers/iio/buffer/industrialio-buffer-dmaengine.c | 39 +++++++++++++---------
 drivers/iio/dac/adi-axi-dac.c                      |  3 +-
 include/linux/iio/buffer-dmaengine.h               | 11 +++---
 3 files changed, 33 insertions(+), 20 deletions(-)

Comments

Jonathan Cameron July 27, 2024, 1:43 p.m. UTC | #1
On Mon, 22 Jul 2024 16:57:14 -0500
David Lechner <dlechner@baylibre.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@baylibre.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);
>
diff mbox series

Patch

diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 918f6f8d65b6..44c3b4fe0643 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -163,32 +163,34 @@  static const struct iio_dev_attr *iio_dmaengine_buffer_attrs[] = {
 /**
  * iio_dmaengine_buffer_alloc() - Allocate new buffer which uses DMAengine
  * @dev: Parent device for the buffer
- * @channel: DMA channel name, typically "rx".
+ * @chan: DMA channel.
  *
  * This allocates a new IIO buffer which internally uses the DMAengine framework
  * to perform its transfers. The parent device will be used to request the DMA
  * channel.
  *
+ * 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.
+ *
  * Once done using the buffer iio_dmaengine_buffer_free() should be used to
  * release it.
  */
 static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
-	const char *channel)
+						     struct dma_chan *chan)
 {
 	struct dmaengine_buffer *dmaengine_buffer;
 	unsigned int width, src_width, dest_width;
 	struct dma_slave_caps caps;
-	struct dma_chan *chan;
 	int ret;
 
-	dmaengine_buffer = kzalloc(sizeof(*dmaengine_buffer), GFP_KERNEL);
-	if (!dmaengine_buffer)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(chan))
+		return ERR_CAST(chan);
 
-	chan = dma_request_chan(dev, channel);
-	if (IS_ERR(chan)) {
-		ret = PTR_ERR(chan);
-		goto err_free;
+	dmaengine_buffer = kzalloc(sizeof(*dmaengine_buffer), GFP_KERNEL);
+	if (!dmaengine_buffer) {
+		ret = -ENOMEM;
+		goto err_release;
 	}
 
 	ret = dma_get_slave_caps(chan, &caps);
@@ -221,6 +223,9 @@  static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
 
 err_free:
 	kfree(dmaengine_buffer);
+err_release:
+	dma_release_channel(chan);
+
 	return ERR_PTR(ret);
 }
 
@@ -244,13 +249,13 @@  EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_free, IIO_DMAENGINE_BUFFER);
 
 struct iio_buffer *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;
 	int ret;
 
-	buffer = iio_dmaengine_buffer_alloc(dev, channel);
+	buffer = iio_dmaengine_buffer_alloc(dev, chan);
 	if (IS_ERR(buffer))
 		return ERR_CAST(buffer);
 
@@ -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);
 
diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index 880d83a014a1..7e6225920e49 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -124,7 +124,8 @@  static struct iio_buffer *axi_dac_request_buffer(struct iio_backend *back,
 	if (device_property_read_string(st->dev, "dma-names", &dma_name))
 		dma_name = "tx";
 
-	return iio_dmaengine_buffer_setup_ext(st->dev, indio_dev, dma_name,
+	return iio_dmaengine_buffer_setup_ext(st->dev, indio_dev,
+					      dma_request_chan(st->dev, dma_name),
 					      IIO_BUFFER_DIRECTION_OUT);
 }
 
diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
index 81d9a19aeb91..a80397c3b198 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -7,6 +7,7 @@ 
 #ifndef __IIO_DMAENGINE_H__
 #define __IIO_DMAENGINE_H__
 
+#include <linux/dmaengine.h>
 #include <linux/iio/buffer.h>
 
 struct iio_dev;
@@ -15,20 +16,22 @@  struct device;
 void iio_dmaengine_buffer_free(struct iio_buffer *buffer);
 struct iio_buffer *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);
 
 #define iio_dmaengine_buffer_setup(dev, indio_dev, channel)	\
-	iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel,	\
+	iio_dmaengine_buffer_setup_ext(dev, indio_dev,		\
+				       dma_request_chan(dev, channel),\
 				       IIO_BUFFER_DIRECTION_IN)
 
 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);
 
 #define devm_iio_dmaengine_buffer_setup(dev, indio_dev, channel)	\
-	devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel,	\
+	devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev,		\
+					    dma_request_chan(dev, channel), \
 					    IIO_BUFFER_DIRECTION_IN)
 
 #endif