[PATCH/RFC 0/4] dma: Add multiplexer call-back for channel allocation

[PATCH/RFC 0/4] dma: Add multiplexer call-back for channel allocation

[Guennadi Liakhovetski]

Hi all

As recently [1] proposed, a DMA channel multiplexer can be user to 
flexibly allocate DMA channels without a use of a DMAC-specific filter 
function. This patch set presents a possible implementation. Of the 3 
patches in this series the first one is the actual proposal, the other two 
are an illustration, using the shdma dmaengine and the sh_mmcif mmc 
drivers on the sh7372 mackerel board.

This is an RFC, please, comment. One extension, that seems to make sense 
is an addition of a multiplexer-configure call-back to actually configure 
the routing. It isn't needed in the shdma case, so, I didn't want to add 
it without real users. Something along the lines of

	struct dma_chan *(*route_chan)(struct dma_chan *, struct device *,
				enum dma_transfer_direction, const char *);

perhaps?

Thanks
Guennadi

[1] http://thread.gmane.org/gmane.linux.ports.arm.omap/75828/focusv295
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH/RFC 1/4] dma: add a function to request a DMA channel for a specific client

[Guennadi Liakhovetski]

Add a new channel-request function, that uses a device pointer and a
transfer direction to locate the required client DMA request line. Devices,
that require DMA channels exceeding the traditional Tx / Rx scheme, can use
the optional "name" parameter.

This new function uses a new DMA multiplexer interface. The new DMA request
line routing concept declares, that client DMA request lines are always
statically connected to a DMA provider. If a client is directly connected
to a DMAC, such a statical connection results in a 1-to-1 relationship
between client DMA request lines and DMAC DMA channels. A more flexible
configuration includes a multiplexer between clients and DMAC instances. In
these cases it is the multiplexer, that controls the routing. The initial
DMA multiplexer API consists of only one call-back: .request_chan(), that
finds the next free (physical or virtual) DMA channel, that can be routed
to the requested slave DMA interface. The actual routing, if required,
should be performed as a part of the dmaengine_slave_config() processing.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/dma/dmaengine.c   |   73 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dmaengine.h |    9 +++++
 2 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 2397f6f..4fb927c 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -542,6 +542,79 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
 }
 EXPORT_SYMBOL_GPL(__dma_request_channel);
 
+/**
+ * dma_request_slave_channel() - try to allocate an exclusive channel for device
+ * @dev:	client device, for which a channel should be allocated
+ * @direction:	transfer direction
+ * @name:	optional name of the channel, used, when the direction is not
+ *		sufficient to uniquely identify the DMA request
+ */
+struct dma_chan *dma_request_slave_channel(struct device *dev,
+		enum dma_transfer_direction direction, const char *name)
+{
+	struct dma_device *device, *_d;
+	struct dma_chan *chan = NULL;
+	dma_cap_mask_t mask;
+	int err;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	/* Find a channel */
+	mutex_lock(&dma_list_mutex);
+	list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
+		if (!device->mux || !device->mux->request_chan)
+			continue;
+
+		if (!dma_device_satisfies_mask(device, mask)) {
+			pr_info("%s: wrong capabilities\n", __func__);
+			continue;
+		}
+
+		/* ensure that all channels are either private or public. */
+		if (!dma_has_cap(DMA_PRIVATE, device->cap_mask)) {
+			bool public = false;
+			list_for_each_entry(chan, &device->channels, device_node) {
+				/* some channels are already publicly allocated */
+				if (chan->client_count) {
+					public = true;
+					break;
+				}
+			}
+			if (public)
+				continue;
+		}
+
+		chan = device->mux->request_chan(device, dev, direction, name);
+		if (chan) {
+			dma_cap_set(DMA_PRIVATE, device->cap_mask);
+			device->privatecnt++;
+			err = dma_chan_get(chan);
+			if (!err)
+				break;
+			if (err = -ENODEV) {
+				pr_debug("%s: %s module removed\n", __func__,
+					 dma_chan_name(chan));
+				list_del_rcu(&device->global_node);
+			} else
+				pr_debug("%s: failed to get %s: (%d)\n",
+					 __func__, dma_chan_name(chan), err);
+
+			if (--device->privatecnt = 0)
+				dma_cap_clear(DMA_PRIVATE, device->cap_mask);
+
+			chan = NULL;
+		}
+	}
+	mutex_unlock(&dma_list_mutex);
+
+	pr_debug("%s: %s (%s)\n", __func__, chan ? "success" : "fail",
+		 chan ? dma_chan_name(chan) : NULL);
+
+	return chan;
+}
+EXPORT_SYMBOL_GPL(dma_request_slave_channel);
+
 void dma_release_channel(struct dma_chan *chan)
 {
 	mutex_lock(&dma_list_mutex);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index ccec62f..19b96d9 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -482,6 +482,11 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr
 }
 #endif
 
+struct dma_multiplexer {
+	struct dma_chan *(*request_chan)(struct dma_device *, struct device *,
+				enum dma_transfer_direction, const char *);
+};
+
 /**
  * struct dma_tx_state - filled in to report the status of
  * a transfer.
@@ -553,6 +558,8 @@ struct dma_device {
 	int dev_id;
 	struct device *dev;
 
+	const struct dma_multiplexer *mux;
+
 	int (*device_alloc_chan_resources)(struct dma_chan *chan);
 	void (*device_free_chan_resources)(struct dma_chan *chan);
 
@@ -994,6 +1001,8 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
 struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
 struct dma_chan *net_dma_find_channel(void);
 #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
+struct dma_chan *dma_request_slave_channel(struct device *dev,
+		enum dma_transfer_direction direction, const char *name);
 
 /* --- Helper iov-locking functions --- */
 
-- 
1.7.2.5

[PATCH/RFC 2/4] dma: shdma: add explicit support for client DMA channel multiplexing

[Guennadi Liakhovetski]

Add support for the new dmaengine multiplexer API. In shdma case the
multiplexer is built-in into the DMAC, so, the multiplexer .request_chan()
method only has to check, whether this client is supported by the specific
DMAC instance. The routing is performed inside the .device_control()
method.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/dma/sh/shdma-base.c |   28 +++++++++++++++++++++++++++-
 drivers/dma/sh/shdma.c      |   22 ++++++++++++++++++++++
 include/linux/sh_dma.h      |    3 +++
 include/linux/shdma-base.h  |    4 ++++
 4 files changed, 56 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
index 0c34c73..76e4854 100644
--- a/drivers/dma/sh/shdma-base.c
+++ b/drivers/dma/sh/shdma-base.c
@@ -808,6 +808,30 @@ static irqreturn_t chan_irqt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static struct dma_chan *shdma_mux_request_chan(struct dma_device *dmadev,
+		struct device *dev, enum dma_transfer_direction direction,
+		const char *name)
+{
+	struct shdma_dev *sdev = to_shdma_dev(dmadev);
+	const struct shdma_ops *ops = sdev->ops;
+	struct shdma_chan *schan;
+	int i;
+
+	if (!ops->slave_supported(sdev, dev, direction, name))
+		return NULL;
+
+	/* The DMAC supports this client, pick up the first free channel */
+	shdma_for_each_chan(schan, sdev, i)
+		if (!schan->dma_chan.client_count)
+			return &schan->dma_chan;
+
+	return NULL;
+}
+
+static const struct dma_multiplexer shdma_mux = {
+	.request_chan = shdma_mux_request_chan,
+};
+
 int shdma_request_irq(struct shdma_chan *schan, int irq,
 			   unsigned long flags, const char *name)
 {
@@ -880,7 +904,8 @@ int shdma_init(struct device *dev, struct shdma_dev *sdev,
 	    !sdev->ops->slave_addr ||
 	    !sdev->ops->channel_busy ||
 	    !sdev->ops->halt_channel ||
-	    !sdev->ops->desc_completed)
+	    !sdev->ops->desc_completed ||
+	    !sdev->ops->slave_supported)
 		return -EINVAL;
 
 	sdev->schan = kcalloc(chan_num, sizeof(*sdev->schan), GFP_KERNEL);
@@ -901,6 +926,7 @@ int shdma_init(struct device *dev, struct shdma_dev *sdev,
 	dma_dev->device_prep_slave_sg = shdma_prep_slave_sg;
 	dma_dev->device_control = shdma_control;
 
+	dma_dev->mux = &shdma_mux;
 	dma_dev->dev = dev;
 
 	return 0;
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index 027c9be..ef3c018 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -424,6 +424,27 @@ static bool sh_dmae_desc_completed(struct shdma_chan *schan,
 		 (sh_desc->hw.sar + sh_desc->hw.tcr) = sar_buf);
 }
 
+static bool sh_dmae_slave_supported(struct shdma_dev *sdev, struct device *dev,
+		enum dma_transfer_direction direction, const char *name)
+{
+	struct sh_dmae_device *shdev = container_of(sdev, struct sh_dmae_device,
+						    shdma_dev);
+	struct sh_dmae_pdata *pdata = shdev->pdata;
+	const struct sh_dmae_slave_config *cfg;
+	int i;
+
+	if (!dev)
+		return false;
+
+	for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
+		if (cfg->dev_id && !strcmp(cfg->dev_id, dev_name(dev)) &&
+		    cfg->direction = direction &&
+		    (!name || (cfg->name && !strcmp(name, cfg->name))))
+			return true;
+
+	return false;
+}
+
 static bool sh_dmae_nmi_notify(struct sh_dmae_device *shdev)
 {
 	/* Fast path out if NMIF is not asserted for this controller */
@@ -632,6 +653,7 @@ static const struct shdma_ops sh_dmae_shdma_ops = {
 	.start_xfer = sh_dmae_start_xfer,
 	.embedded_desc = sh_dmae_embedded_desc,
 	.chan_irq = sh_dmae_chan_irq,
+	.slave_supported = sh_dmae_slave_supported,
 };
 
 static int __devinit sh_dmae_probe(struct platform_device *pdev)
diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
index b64d6be..86be8ab 100644
--- a/include/linux/sh_dma.h
+++ b/include/linux/sh_dma.h
@@ -31,6 +31,9 @@ struct sh_dmae_slave_config {
 	dma_addr_t	addr;
 	u32		chcr;
 	char		mid_rid;
+	enum dma_transfer_direction direction;
+	const char	*name;
+	const char	*dev_id;
 };
 
 struct sh_dmae_channel {
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index 93f9821..3769518 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -70,6 +70,8 @@ struct shdma_chan {
 	enum shdma_pm_state pm_state;
 };
 
+struct shdma_dev;
+
 /**
  * struct shdma_ops - simple DMA driver operations
  * desc_completed:	return true, if this is the descriptor, that just has
@@ -98,6 +100,8 @@ struct shdma_ops {
 	void (*start_xfer)(struct shdma_chan *, struct shdma_desc *);
 	struct shdma_desc *(*embedded_desc)(void *, int);
 	bool (*chan_irq)(struct shdma_chan *, int);
+	bool (*slave_supported)(struct shdma_dev *, struct device *,
+				enum dma_transfer_direction, const char *);
 };
 
 struct shdma_dev {
-- 
1.7.2.5

[PATCH/RFC 3/4] ARM: shmobile: extend MMCIF DMA client configuration entries

[Guennadi Liakhovetski]

To prepare for MMCIF driver transition to the dma_request_slave_channel()
API the MMCIF DMA client configuration entries in the slave configuration
table have to be extended with device ID and DMA transfer direction fields.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

This is just an example. Eventually all DMA client configurations on all 
affected platforms should be converted.

 arch/arm/mach-shmobile/setup-sh7372.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
index 7873159..f583e03 100644
--- a/arch/arm/mach-shmobile/setup-sh7372.c
+++ b/arch/arm/mach-shmobile/setup-sh7372.c
@@ -452,11 +452,15 @@ static const struct sh_dmae_slave_config sh7372_dmae_slaves[] = {
 		.addr		= 0xe6bd0034,
 		.chcr		= CHCR_TX(XMIT_SZ_32BIT),
 		.mid_rid	= 0xd1,
+		.dev_id		= "sh_mmcif.0",
+		.direction	= DMA_MEM_TO_DEV,
 	}, {
 		.slave_id	= SHDMA_SLAVE_MMCIF_RX,
 		.addr		= 0xe6bd0034,
 		.chcr		= CHCR_RX(XMIT_SZ_32BIT),
 		.mid_rid	= 0xd2,
+		.dev_id		= "sh_mmcif.0",
+		.direction	= DMA_DEV_TO_MEM,
 	},
 };
 
-- 
1.7.2.5

[PATCH/RFC 4/4] mmc: sh_mmcif: switch to using dma_request_slave_channel()

[Guennadi Liakhovetski]

Use dma_request_slave_channel() to obtain DMA channels, which eliminates
the need for a filter function.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

This shall only be applied after all MMCIF users, using DMA, have been 
converted

 drivers/mmc/host/sh_mmcif.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 0f07d28..cb71410 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -386,8 +386,7 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
-	host->chan_tx = dma_request_channel(mask, shdma_chan_filter,
-					    (void *)pdata->slave_id_tx);
+	host->chan_tx = dma_request_slave_channel(&host->pd->dev, DMA_MEM_TO_DEV, NULL);
 	dev_dbg(&host->pd->dev, "%s: TX: got channel %p\n", __func__,
 		host->chan_tx);
 
@@ -402,8 +401,7 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 	if (ret < 0)
 		goto ecfgtx;
 
-	host->chan_rx = dma_request_channel(mask, shdma_chan_filter,
-					    (void *)pdata->slave_id_rx);
+	host->chan_rx = dma_request_slave_channel(&host->pd->dev, DMA_DEV_TO_MEM, NULL);
 	dev_dbg(&host->pd->dev, "%s: RX: got channel %p\n", __func__,
 		host->chan_rx);
 
-- 
1.7.2.5

Re: [PATCH/RFC 1/4] dma: add a function to request a DMA channel for a specific client

[Vinod Koul]

On Tue, 2012-07-17 at 12:53 +0200, Guennadi Liakhovetski wrote:
> Add a new channel-request function, that uses a device pointer and a
> transfer direction to locate the required client DMA request line. Devices,
> that require DMA channels exceeding the traditional Tx / Rx scheme, can use
> the optional "name" parameter.
> 
> This new function uses a new DMA multiplexer interface. The new DMA request
> line routing concept declares, that client DMA request lines are always
> statically connected to a DMA provider. If a client is directly connected
> to a DMAC, such a statical connection results in a 1-to-1 relationship
> between client DMA request lines and DMAC DMA channels. A more flexible
> configuration includes a multiplexer between clients and DMAC instances. In
> these cases it is the multiplexer, that controls the routing. The initial
> DMA multiplexer API consists of only one call-back: .request_chan(), that
> finds the next free (physical or virtual) DMA channel, that can be routed
> to the requested slave DMA interface. The actual routing, if required,
> should be performed as a part of the dmaengine_slave_config() processing.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  drivers/dma/dmaengine.c   |   73 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/dmaengine.h |    9 +++++
>  2 files changed, 82 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 2397f6f..4fb927c 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -542,6 +542,79 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
>  }
>  EXPORT_SYMBOL_GPL(__dma_request_channel);
>  
> +/**
> + * dma_request_slave_channel() - try to allocate an exclusive channel for device
> + * @dev:	client device, for which a channel should be allocated
> + * @direction:	transfer direction
> + * @name:	optional name of the channel, used, when the direction is not
> + *		sufficient to uniquely identify the DMA request
> + */
> +struct dma_chan *dma_request_slave_channel(struct device *dev,
> +		enum dma_transfer_direction direction, const char *name)
> +{
> +	struct dma_device *device, *_d;
> +	struct dma_chan *chan = NULL;
> +	dma_cap_mask_t mask;
> +	int err;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +
> +	/* Find a channel */
> +	mutex_lock(&dma_list_mutex);
> +	list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
> +		if (!device->mux || !device->mux->request_chan)
> +			continue;
> +
> +		if (!dma_device_satisfies_mask(device, mask)) {
> +			pr_info("%s: wrong capabilities\n", __func__);
> +			continue;
> +		}
> +
> +		/* ensure that all channels are either private or public. */
> +		if (!dma_has_cap(DMA_PRIVATE, device->cap_mask)) {
> +			bool public = false;
> +			list_for_each_entry(chan, &device->channels, device_node) {
> +				/* some channels are already publicly allocated */
> +				if (chan->client_count) {
> +					public = true;
> +					break;
> +				}
> +			}
> +			if (public)
> +				continue;
> +		}
> +
> +		chan = device->mux->request_chan(device, dev, direction, name);
NAK

1. We dont want dmacs to have anything to do with filtering and
allocating. That is role of dmaengien and we need to add stuff there
only.
2. Even if we start using optional name argument here we still don't
know out of 4 channels dmac supports which one to allocate.

> +		if (chan) {
> +			dma_cap_set(DMA_PRIVATE, device->cap_mask);
> +			device->privatecnt++;
> +			err = dma_chan_get(chan);
> +			if (!err)
> +				break;
> +			if (err = -ENODEV) {
> +				pr_debug("%s: %s module removed\n", __func__,
> +					 dma_chan_name(chan));
> +				list_del_rcu(&device->global_node);
> +			} else
> +				pr_debug("%s: failed to get %s: (%d)\n",
> +					 __func__, dma_chan_name(chan), err);
> +
> +			if (--device->privatecnt = 0)
> +				dma_cap_clear(DMA_PRIVATE, device->cap_mask);
> +
> +			chan = NULL;
> +		}
> +	}
> +	mutex_unlock(&dma_list_mutex);
> +
> +	pr_debug("%s: %s (%s)\n", __func__, chan ? "success" : "fail",
> +		 chan ? dma_chan_name(chan) : NULL);
> +
> +	return chan;
> +}
> +EXPORT_SYMBOL_GPL(dma_request_slave_channel);
> +
>  void dma_release_channel(struct dma_chan *chan)
>  {
>  	mutex_lock(&dma_list_mutex);
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index ccec62f..19b96d9 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -482,6 +482,11 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr
>  }
>  #endif
>  
> +struct dma_multiplexer {
> +	struct dma_chan *(*request_chan)(struct dma_device *, struct device *,
> +				enum dma_transfer_direction, const char *);
> +};
> +
>  /**
>   * struct dma_tx_state - filled in to report the status of
>   * a transfer.
> @@ -553,6 +558,8 @@ struct dma_device {
>  	int dev_id;
>  	struct device *dev;
>  
> +	const struct dma_multiplexer *mux;
> +
>  	int (*device_alloc_chan_resources)(struct dma_chan *chan);
>  	void (*device_free_chan_resources)(struct dma_chan *chan);
>  
> @@ -994,6 +1001,8 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
>  struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
>  struct dma_chan *net_dma_find_channel(void);
>  #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> +struct dma_chan *dma_request_slave_channel(struct device *dev,
> +		enum dma_transfer_direction direction, const char *name);
>  
>  /* --- Helper iov-locking functions --- */
>  


-- 
~Vinod

Re: [PATCH/RFC 1/4] dma: add a function to request a DMA channel for a specific client

[Guennadi Liakhovetski]

Hi Vinod

Thanks for your review and sorry for a delayed reply.

On Thu, 26 Jul 2012, Vinod Koul wrote:

> On Tue, 2012-07-17 at 12:53 +0200, Guennadi Liakhovetski wrote:
> > Add a new channel-request function, that uses a device pointer and a
> > transfer direction to locate the required client DMA request line. Devices,
> > that require DMA channels exceeding the traditional Tx / Rx scheme, can use
> > the optional "name" parameter.
> > 
> > This new function uses a new DMA multiplexer interface. The new DMA request
> > line routing concept declares, that client DMA request lines are always
> > statically connected to a DMA provider. If a client is directly connected
> > to a DMAC, such a statical connection results in a 1-to-1 relationship
> > between client DMA request lines and DMAC DMA channels. A more flexible
> > configuration includes a multiplexer between clients and DMAC instances. In
> > these cases it is the multiplexer, that controls the routing. The initial
> > DMA multiplexer API consists of only one call-back: .request_chan(), that
> > finds the next free (physical or virtual) DMA channel, that can be routed
> > to the requested slave DMA interface. The actual routing, if required,
> > should be performed as a part of the dmaengine_slave_config() processing.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  drivers/dma/dmaengine.c   |   73 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/dmaengine.h |    9 +++++
> >  2 files changed, 82 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index 2397f6f..4fb927c 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -542,6 +542,79 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
> >  }
> >  EXPORT_SYMBOL_GPL(__dma_request_channel);
> >  
> > +/**
> > + * dma_request_slave_channel() - try to allocate an exclusive channel for device
> > + * @dev:	client device, for which a channel should be allocated
> > + * @direction:	transfer direction
> > + * @name:	optional name of the channel, used, when the direction is not
> > + *		sufficient to uniquely identify the DMA request
> > + */
> > +struct dma_chan *dma_request_slave_channel(struct device *dev,
> > +		enum dma_transfer_direction direction, const char *name)
> > +{
> > +	struct dma_device *device, *_d;
> > +	struct dma_chan *chan = NULL;
> > +	dma_cap_mask_t mask;
> > +	int err;
> > +
> > +	dma_cap_zero(mask);
> > +	dma_cap_set(DMA_SLAVE, mask);
> > +
> > +	/* Find a channel */
> > +	mutex_lock(&dma_list_mutex);
> > +	list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
> > +		if (!device->mux || !device->mux->request_chan)
> > +			continue;
> > +
> > +		if (!dma_device_satisfies_mask(device, mask)) {
> > +			pr_info("%s: wrong capabilities\n", __func__);
> > +			continue;
> > +		}
> > +
> > +		/* ensure that all channels are either private or public. */
> > +		if (!dma_has_cap(DMA_PRIVATE, device->cap_mask)) {
> > +			bool public = false;
> > +			list_for_each_entry(chan, &device->channels, device_node) {
> > +				/* some channels are already publicly allocated */
> > +				if (chan->client_count) {
> > +					public = true;
> > +					break;
> > +				}
> > +			}
> > +			if (public)
> > +				continue;
> > +		}
> > +
> > +		chan = device->mux->request_chan(device, dev, direction, name);
> NAK
> 
> 1. We dont want dmacs to have anything to do with filtering and
> allocating. That is role of dmaengien and we need to add stuff there
> only.
> 2. Even if we start using optional name argument here we still don't
> know out of 4 channels dmac supports which one to allocate.

I don't think we can be generic enough in the dmaengine core to identify 
which channels are suitable for all slave requests. This is why I proposed 
a concept of a DMA channel multiplexer driver, and this is exactly what 
this callback is. I tried to explain this in the patch description above. 
I thought we discussed this earlier in this thread and agreed, that a 
multiplexer API is the way to go with channel allocation. I think, I 
mentioned in one of my mails, that in the beginning I wanted to implement 
multiplexers as a proper (platform) device, but at least one problem with 
this is, that at least on my hardware the multiplexer and the DMAC share 
registers, and an MFD seems an overkill for this, but we can discuss this. 
I don't know, whether there are systems, where multiplexers are really 
separate hardware blocks, if there are, we can think of a way to handle 
those nicely too.

Thanks
Guennadi

> 
> > +		if (chan) {
> > +			dma_cap_set(DMA_PRIVATE, device->cap_mask);
> > +			device->privatecnt++;
> > +			err = dma_chan_get(chan);
> > +			if (!err)
> > +				break;
> > +			if (err = -ENODEV) {
> > +				pr_debug("%s: %s module removed\n", __func__,
> > +					 dma_chan_name(chan));
> > +				list_del_rcu(&device->global_node);
> > +			} else
> > +				pr_debug("%s: failed to get %s: (%d)\n",
> > +					 __func__, dma_chan_name(chan), err);
> > +
> > +			if (--device->privatecnt = 0)
> > +				dma_cap_clear(DMA_PRIVATE, device->cap_mask);
> > +
> > +			chan = NULL;
> > +		}
> > +	}
> > +	mutex_unlock(&dma_list_mutex);
> > +
> > +	pr_debug("%s: %s (%s)\n", __func__, chan ? "success" : "fail",
> > +		 chan ? dma_chan_name(chan) : NULL);
> > +
> > +	return chan;
> > +}
> > +EXPORT_SYMBOL_GPL(dma_request_slave_channel);
> > +
> >  void dma_release_channel(struct dma_chan *chan)
> >  {
> >  	mutex_lock(&dma_list_mutex);
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index ccec62f..19b96d9 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -482,6 +482,11 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr
> >  }
> >  #endif
> >  
> > +struct dma_multiplexer {
> > +	struct dma_chan *(*request_chan)(struct dma_device *, struct device *,
> > +				enum dma_transfer_direction, const char *);
> > +};
> > +
> >  /**
> >   * struct dma_tx_state - filled in to report the status of
> >   * a transfer.
> > @@ -553,6 +558,8 @@ struct dma_device {
> >  	int dev_id;
> >  	struct device *dev;
> >  
> > +	const struct dma_multiplexer *mux;
> > +
> >  	int (*device_alloc_chan_resources)(struct dma_chan *chan);
> >  	void (*device_free_chan_resources)(struct dma_chan *chan);
> >  
> > @@ -994,6 +1001,8 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
> >  struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
> >  struct dma_chan *net_dma_find_channel(void);
> >  #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> > +struct dma_chan *dma_request_slave_channel(struct device *dev,
> > +		enum dma_transfer_direction direction, const char *name);
> >  
> >  /* --- Helper iov-locking functions --- */
> >  
> 
> 
> -- 
> ~Vinod
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RFC 1/4] dma: add a function to request a DMA channel for a specific client

[Vinod Koul]

On Thu, 2012-08-02 at 11:11 +0200, Guennadi Liakhovetski wrote:
> > > +           chan = device->mux->request_chan(device, dev, direction, name);
> > NAK
> > 
> > 1. We dont want dmacs to have anything to do with filtering and
> > allocating. That is role of dmaengien and we need to add stuff there
> > only.
> > 2. Even if we start using optional name argument here we still don't
> > know out of 4 channels dmac supports which one to allocate.
> 
> I don't think we can be generic enough in the dmaengine core to identify 
> which channels are suitable for all slave requests. This is why I proposed 
> a concept of a DMA channel multiplexer driver, and this is exactly what 
> this callback is. I tried to explain this in the patch description above. 
> I thought we discussed this earlier in this thread and agreed, that a 
> multiplexer API is the way to go with channel allocation. I think, I 
> mentioned in one of my mails, that in the beginning I wanted to implement 
> multiplexers as a proper (platform) device, but at least one problem with 
> this is, that at least on my hardware the multiplexer and the DMAC share 
> registers, and an MFD seems an overkill for this, but we can discuss this. 
> I don't know, whether there are systems, where multiplexers are really 
> separate hardware blocks, if there are, we can think of a way to handle 
> those nicely too.
I would disagree.

As I have said in past, we need the mapping information to be available
with dmaengine for slave channels even before any slave allocation
occurs. This way dmaengine knows what to do with a channel.
I am planning to add the support in dmanegine for this...

The problem with this solution here is again how the hell will a dmac
know what to do with alloc request. It doesn't and any current way it
does is just a hack and nothing else. This also prevenrts us form
building genric dmac driver which should only know how to deal with dmac
only and nothing else.

-- 
~Vinod

Re: [PATCH/RFC 1/4] dma: add a function to request a DMA channel for a specific client

[Guennadi Liakhovetski]

On Fri, 3 Aug 2012, Vinod Koul wrote:

> On Thu, 2012-08-02 at 11:11 +0200, Guennadi Liakhovetski wrote:
> > > > +           chan = device->mux->request_chan(device, dev, direction, name);
> > > NAK
> > > 
> > > 1. We dont want dmacs to have anything to do with filtering and
> > > allocating. That is role of dmaengien and we need to add stuff there
> > > only.
> > > 2. Even if we start using optional name argument here we still don't
> > > know out of 4 channels dmac supports which one to allocate.
> > 
> > I don't think we can be generic enough in the dmaengine core to identify 
> > which channels are suitable for all slave requests. This is why I proposed 
> > a concept of a DMA channel multiplexer driver, and this is exactly what 
> > this callback is. I tried to explain this in the patch description above. 
> > I thought we discussed this earlier in this thread and agreed, that a 
> > multiplexer API is the way to go with channel allocation. I think, I 
> > mentioned in one of my mails, that in the beginning I wanted to implement 
> > multiplexers as a proper (platform) device, but at least one problem with 
> > this is, that at least on my hardware the multiplexer and the DMAC share 
> > registers, and an MFD seems an overkill for this, but we can discuss this. 
> > I don't know, whether there are systems, where multiplexers are really 
> > separate hardware blocks, if there are, we can think of a way to handle 
> > those nicely too.
> I would disagree.
> 
> As I have said in past, we need the mapping information to be available
> with dmaengine for slave channels even before any slave allocation
> occurs. This way dmaengine knows what to do with a channel.

I think, this depends on our interpretation of what struct dma_chan 
represents and who has control over it. If we think, that this is a 
purely software object, which at allocation time has absolutely no 
hardware association attached to it, then yes, only the dmaengine core 
should manage them at least at that time. If, however, we define, that 
struct dma_chan _can_ get hardware association already at allocation time 
in the context of DMAC's .alloc_chan_resources() function, then it's 
the DMAC that has to manage them.

And, I think, there is one important aspect to this, that tells us, that 
those channels cannot be absolutely hardware-agnostic: they are attached 
to DMAC instances. So, they intrinsically have some hardware association.

So, you have to take into account hardware limitations already at 
allocation time. You can do this by either asking the DMAC, whether a 
certain allocation can be performed (what we currently do with filters, 
and what this patch proposed to do with the multiplexer callback), or you 
can require some static information from DMACs, like a mapping that you 
propose.

Currently both approaches seem possible, however, a static mapping seems 
less flexible to me, I don't know whether it can describe all available 
hardware configurations. We shouldn't forget, that those mappings can be 
many-to-many, as in the sh-dma example. On sh-dma it would suffice and be 
optimal enough to have a pointer to a slave list in struct dma_chan. Would 
this also suit everyone else? Is this what you're thinking about?

Thanks
Guennadi

> I am planning to add the support in dmanegine for this...
> 
> The problem with this solution here is again how the hell will a dmac
> know what to do with alloc request. It doesn't and any current way it
> does is just a hack and nothing else. This also prevenrts us form
> building genric dmac driver which should only know how to deal with dmac
> only and nothing else.
> 
> -- 
> ~Vinod
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/