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

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Guennadi Liakhovetski
Date: Tuesday, December 2, 2008 - 8:52 am

Hi Dan,

I think, there is a problem with your dma_request_channel() / 
private_candidate() implementation: your current version only tries one 
channel from a dma device list, which matched capabilities. If this 
channel is not accepted by the client, you do not try other channels from 
this device and just go to the next one...

Another problem I encountered with my framebuffer is the initialisation 
order. You initialise dmaengine per subsys_initcall(), whereas the only 
way to guarantee the order:

dmaengine
dma-device driver
framebuffer

when they are all linked into the kernel was to switch dmaengine to 
arch_initcall, put the dma driver under subsys_initcall, and the 
framebuffer under a normal module_init / device_initcall.

Below is a naive patch, adding two more arguments to private_candidate() 
is not very elegant, so, this is not an official submission. Feel free to 
propose a better fix. But if you like, I can make two patches out of this 
- separating the initialisation priority.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

dmaengine: scan all channels if one is rejected, initialise earlier.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 94dcc64..677b0c8 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -404,10 +404,12 @@ static void dma_channel_rebalance(void)
 		}
 }
 
-static struct dma_chan *private_candidate(dma_cap_mask_t *mask, struct dma_device *dev)
+static struct dma_chan *private_candidate(dma_cap_mask_t *mask,
+		struct dma_device *dev, dma_filter_fn fn, void *fn_param)
 {
 	struct dma_chan *chan;
 	struct dma_chan *ret = NULL;
+	bool ack;
 
 	/* devices with multiple channels need special handling as we need to
 	 * ensure that all channels are either private or public.
@@ -430,8 +432,18 @@ static struct dma_chan *private_candidate(dma_cap_mask_t *mask, struct dma_devic
 				 __func__, dev_name(&chan->dev));
 			continue;
 		}
-		ret = chan;
-		break;
+
+		if (fn)
+			ack = fn(chan, fn_param);
+		else
+			ack = true;
+
+		if (ack) {
+			ret = chan;
+			break;
+		} else
+			pr_debug("%s: %s filter said false\n",
+				 __func__, dev_name(&chan->dev));
 	}
 
 	return ret;
@@ -447,42 +459,33 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
 {
 	struct dma_device *device, *_d;
 	struct dma_chan *chan = NULL;
-	bool ack;
 	int err;
 
 	/* Find a channel */
 	mutex_lock(&dma_list_mutex);
 	list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
-		chan = private_candidate(mask, device);
+		chan = private_candidate(mask, device, fn, fn_param);
 		if (!chan)
 			continue;
 
-		if (fn)
-			ack = fn(chan, fn_param);
+		/* Found a suitable channel, try to grab, prep, and
+		 * return it.  We first set DMA_PRIVATE to disable
+		 * balance_ref_count as this channel will not be
+		 * published in the general-purpose allocator
+		 */
+		dma_cap_set(DMA_PRIVATE, device->cap_mask);
+		err = dma_chan_get(chan);
+
+		if (err == -ENODEV) {
+			pr_debug("%s: %s module removed\n", __func__,
+				 dev_name(&chan->dev));
+			list_del_rcu(&device->global_node);
+		} else if (err)
+			pr_err("dmaengine: failed to get %s: (%d)",
+			       dev_name(&chan->dev), err);
 		else
-			ack = true;
-
-		if (ack) {
-			/* Found a suitable channel, try to grab, prep, and
-			 * return it.  We first set DMA_PRIVATE to disable
-			 * balance_ref_count as this channel will not be
-			 * published in the general-purpose allocator
-			 */
-			dma_cap_set(DMA_PRIVATE, device->cap_mask);
-			err = dma_chan_get(chan);
+			break;
 
-			if (err == -ENODEV) {
-				pr_debug("%s: %s module removed\n", __func__,
-					 dev_name(&chan->dev));
-				list_del_rcu(&device->global_node);
-			} else if (err)
-				pr_err("dmaengine: failed to get %s: (%d)",
-				       dev_name(&chan->dev), err);
-			else
-				break;
-		} else
-			pr_debug("%s: %s filter said false\n",
-				 __func__, dev_name(&chan->dev));
 		chan = NULL;
 	}
 	mutex_unlock(&dma_list_mutex);
@@ -912,6 +915,4 @@ static int __init dma_bus_init(void)
 	mutex_init(&dma_list_mutex);
 	return class_register(&dma_devclass);
 }
-subsys_initcall(dma_bus_init);
-
-
+arch_initcall(dma_bus_init);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 00/13] dmaengine redux, Dan Williams, (Fri Nov 14, 2:34 pm)
[PATCH 02/13] dmaengine: remove dependency on async_tx, Dan Williams, (Fri Nov 14, 2:34 pm)
[PATCH 06/13] net_dma: convert to dma_find_channel, Dan Williams, (Fri Nov 14, 2:34 pm)
[PATCH 08/13] dmatest: convert to dma_request_channel, Dan Williams, (Fri Nov 14, 2:34 pm)
[PATCH 12/13] dmaengine: remove 'bigref' infrastructure, Dan Williams, (Fri Nov 14, 2:35 pm)
[PATCH 13/13] dmaengine: kill enum dma_state_client, Dan Williams, (Fri Nov 14, 2:35 pm)
Re: [PATCH 02/13] dmaengine: remove dependency on async_tx, Andrew Morton, (Fri Nov 14, 11:02 pm)
Re: [PATCH 08/13] dmatest: convert to dma_request_channel, Andrew Morton, (Fri Nov 14, 11:17 pm)
Re: [PATCH 08/13] dmatest: convert to dma_request_channel, Dan Williams, (Tue Nov 18, 11:24 am)
Re: [PATCH 08/13] dmatest: convert to dma_request_channel, Andrew Morton, (Tue Nov 18, 1:58 pm)
Re: [PATCH 07/13] dmaengine: introduce dma_request_channel ..., Guennadi Liakhovetski, (Tue Dec 2, 8:52 am)
Re: [PATCH 07/13] dmaengine: introduce dma_request_channel ..., Guennadi Liakhovetski, (Tue Dec 2, 10:27 am)
Re: [PATCH 07/13] dmaengine: introduce dma_request_channel ..., Guennadi Liakhovetski, (Tue Dec 2, 2:28 pm)
Re: [PATCH 03/13] dmaengine: up-level reference counting t ..., Guennadi Liakhovetski, (Thu Dec 4, 9:56 am)
Re: [PATCH 03/13] dmaengine: up-level reference counting t ..., Guennadi Liakhovetski, (Thu Dec 4, 12:28 pm)
RE: [PATCH 00/13] dmaengine redux, Sosnowski, Maciej, (Fri Dec 12, 7:27 am)
RE: [PATCH 03/13] dmaengine: up-level reference counting t ..., Sosnowski, Maciej, (Fri Dec 12, 7:28 am)
RE: [PATCH 07/13] dmaengine: introduce dma_request_channel ..., Sosnowski, Maciej, (Fri Dec 12, 7:29 am)
RE: [PATCH 11/13] dmaengine: kill struct dma_client and su ..., Sosnowski, Maciej, (Fri Dec 12, 7:29 am)
RE: [PATCH 03/13] dmaengine: up-level reference counting t ..., Sosnowski, Maciej, (Thu Dec 18, 7:26 am)
RE: [PATCH 07/13] dmaengine: introduce dma_request_channel ..., Sosnowski, Maciej, (Thu Dec 18, 7:33 am)
RE: [PATCH 11/13] dmaengine: kill struct dma_client and su ..., Sosnowski, Maciej, (Thu Dec 18, 7:34 am)
Re: [PATCH 07/13] dmaengine: introduce dma_request_channel ..., Guennadi Liakhovetski, (Fri Jan 30, 4:27 pm)