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(structdma_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