Thread (52 messages) 52 messages, 6 authors, 2009-02-06

RE: [PATCH 07/13] dmaengine: introduce dma_request_channel and private channels

From: Sosnowski, Maciej <hidden>
Date: 2008-12-12 14:29:47
Also in: lkml

Williams, Dan J wrote:
quoted hunk ↗ jump to hunk
This interface is primarily for device-to-memory clients which need to
search for dma channels with platform-specific characteristics.  The
prototype is:

struct dma_chan *dma_request_channel(dma_cap_mask_t mask,
                                     dma_filter_fn filter_fn,
                                     void *filter_param);

When the optional 'filter_fn' parameter is set to NULL
dma_request_channel simply returns the first channel that satisfies
the 
capability mask.  Otherwise, when the mask parameter is insufficient
for 
specifying the necessary channel, the filter_fn routine can be used to
disposition the available channels in the system. The filter_fn
routine 
is called once for each free channel in the system.  Upon seeing a
suitable channel filter_fn returns DMA_ACK which flags that channel to
be the return value from dma_request_channel.  A channel allocated via
this interface is exclusive to the caller, until dma_release_channel()
is called.

To ensure that all channels are not consumed by the general-purpose
allocator the DMA_PRIVATE capability is provided to exclude a
dma_device 
from general-purpose (memory-to-memory) consideration.

Signed-off-by: Dan Williams <redacted>
---

 drivers/dma/dmaengine.c   |  161
 ++++++++++++++++++++++++++++++++++++++++-----
 include/linux/dmaengine.h |   16 ++++ 2 files changed, 159
insertions(+), 18 deletions(-) 
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index ec483cc..46fd5fa 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -190,7 +190,7 @@ static int dma_chan_get(struct dma_chan *chan)
 			chan->client_count = 0;
 			module_put(owner);
 			err = -ENOMEM;
-		} else
+		} else if (!dma_has_cap(DMA_PRIVATE, chan->device->cap_mask))
 			balance_ref_count(chan);
 	}
@@ -221,6 +221,8 @@ static void dma_client_chan_alloc(struct
dma_client *client) 

 	/* Find a channel */
 	list_for_each_entry(device, &dma_device_list, global_node) {
+		if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
+			continue;
 		/* Does the client require a specific DMA controller? */
 		if (client->slave && client->slave->dma_dev
 				&& client->slave->dma_dev != device->dev)
@@ -313,6 +315,7 @@ static int __init dma_channel_table_init(void)
 	 * channel_table
 	 */
 	clear_bit(DMA_INTERRUPT, dma_cap_mask_all.bits);
+	clear_bit(DMA_PRIVATE, dma_cap_mask_all.bits);
 	clear_bit(DMA_SLAVE, dma_cap_mask_all.bits);
The comment above should be updated according to this change:
-/* 'interrupt' and 'slave' are channel capabilities, but are not
+/* 'interrupt', 'private' and 'slave' are channel capabilities, but are not
+static struct dma_chan *private_candidate(dma_cap_mask_t *mask,
struct dma_device *dev) +{
+	struct dma_chan *chan;
+	struct dma_chan *ret = NULL;
+
+	/* devices with multiple channels need special handling as we need
to +	 * ensure that all channels are either private or public.
+	 */
+	if (dev->chancnt > 1 && !dma_has_cap(DMA_PRIVATE, dev->cap_mask))
+		list_for_each_entry(chan, &dev->channels, device_node) {
+			/* some channels are already publicly allocated */
+			if (chan->client_count)
+				return NULL;
+		}
Isn't it a dangerous approach to let clients consume for their exclusive usage channels
meant for general-purpose ("pubilc" ones)?
Why not to limit private_candidate to devices with DMA_PRIVATE capability only?
+
+	list_for_each_entry(chan, &dev->channels, device_node) {
+		if (!__dma_chan_satisfies_mask(chan, mask)) {
+			pr_debug("%s: %s wrong capabilities\n",
+				 __func__, dev_name(&chan->dev));
+			continue;
+		}
As capabilities are per device, this check could be performed just once
before list_for_each_entry(chan, &dev->channels, device_node).

Regards,
Maciej
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help