On Fri, Dec 12, 2008 at 7:29 AM, Sosnowski, Maciej
[off-list ref] wrote:

quoted

      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

ok.

quoted

+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?

This allows unused channels to be claimed by dma_request_channel().
It is not dangerous as long as ->client_count is zero.

quoted

+
+     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).

Yes, changed.

Thanks,
Dan