[PATCH V3 1/2] of: Add generic device tree DMA helpers

[Guennadi Liakhovetski]

On Fri, 6 Jul 2012, Russell King - ARM Linux wrote:

> On Fri, Jul 06, 2012 at 01:36:32PM +0200, Guennadi Liakhovetski wrote:
> > I like this idea, but why don't we extend it to also cover the non-DT 
> > case? I.e., why don't we add the above callback (call it "match" or 
> > "filter" or anything else) to dmaengine operations and inside (the 
> > extended) dma_request_channel(), instead of calling the filter function, 
> > passed as a parameter, we loop over all registered DMAC devices and call 
> > their filter callbacks, until one of them returns true? In fact, it goes 
> > back to my earlier proposal from 
> > http://thread.gmane.org/gmane.linux.kernel/1246957
> > which I, possibly, failed to explain properly. So, the transformation 
> > chain from today's API would be (all code is approximate):
> > 
> > (today)
> > 
> > <client driver>
> > 	dma_request_channel(mask, filter, filter_arg);
> > 
> > <dmaengine_core>
> > 	for_each_channel() {
> > 		ret = (*filter)(chan, filter_arg);
> > 		if (ret) {
> > 			ret = chan->device->device_alloc_chan_resources(chan);
> > 			if (!ret)
> > 				return chan;
> > 			else
> > 				return NULL;
> > 		}
> > 	}
> > 
> > (can be transformed to)
> > 
> > <client driver>
> > 	dma_request_channel(mask, filter_arg);
> > 
> > <dmaengine_core>
> > 	for_each_channel() {
> > 		ret = chan->device->filter(chan, filter_arg);
> > 		if (ret) {
> > 			<same as above>
> > 		}
> > 	}
> > 
> > (which further could be simplified to)
> > 
> > <client driver>
> > 	dma_request_channel(mask, filter_arg);
> > 
> > <dmaengine_core>
> > 	for_each_channel() {
> > 		ret = chan->device->device_alloc_chan_resources(chan, filter_arg);
> 
> This looks like you're re-purposing a perfectly good API for something that
> it wasn't designed to do, namely providing a channel selection mechanism,
> rather than "allocating channel resources".  The hint is in the bloody
> name, please take a look sometime!
> 
> If you want to call into the DMA engine driver for channel selection,
> then create an _explicit_ API for it.  Don't bugger around with existing
> APIs.

Sure, it's better to create a new call-back.

> Moreover, the *big* problem that your proposal above creates is this.
> What _type_ is filter_arg?  If it's void *, then your suggestion is
> nonsense, even if you associate it with a size argument.  You have no
> idea what the bytestream that would be passed via that pointer means,
> even if the size just happens to look like it's what you expect.

Right, thanks to you and Arnd, who has pointed this out too.

Then it seems to me, that we need to introduce a notion of a mux device. 
We could of course try to just use some strings instead, arrays of 
acceptable DMA devices and channels, and most likely we would even be able 
to find such an approach, that would work for all existing configurations. 
But it still wouldn't be really fully generic, right? Whereas creating a 
mux driver we really could cover all possible cases. DMA clients in this 
case would always be hard wired to only one pin of such a mux, the DMA 
device on the other side also only has to care about its physical 
channels. The dmaengine core would then query the mux driver, where it can 
route specific client request lines?

A separate mux is needed, because several clients can have their DMA 
handshake lines routed to several DMAC instances, so, this muxing 
functionality can neither reside on the client nor on the CMAC.

Is this a right direction now? Shall I try to prototype such a DMA mux 
API?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/