Messages in this thread | | | Date | Mon, 15 Dec 2008 16:55:06 -0700 | From | "Dan Williams" <> | Subject | Re: [PATCH 07/13] dmaengine: introduce dma_request_channel and private channels |
| |
On Fri, Dec 12, 2008 at 7:29 AM, Sosnowski, Maciej <maciej.sosnowski@intel.com> wrote: >> 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.
>> +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.
>> + >> + 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
| |