Re: [PATCH/RFC] dmaengine: add a slave parameter to __dma_request_channel()

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

Hi Vinod

Thanks for your review.

On Tue, 6 Mar 2012, Vinod Koul wrote:

> On Fri, 2012-03-02 at 14:21 +0100, Guennadi Liakhovetski wrote:
> > Hi Vinod
> > 
> > On Wed, 1 Feb 2012, Guennadi Liakhovetski wrote:
> sorry I thought I had replied, but looks like it got missed!
> > 
> > > When performing slame DMA some dmaengine drivers need additional data from
> typo		  ^^^^^^^^^
> > > client drivers to find out, whether they can support that specific client
> > > and to configure the DMA channel for it. This additional data has to be
> > > supplied by client drivers during channel allocation, i.e., with the
> > > __dma_request_channel() function. This patch adds a new
> > > struct dma_slave_desc with some basic data in it, further this struct can
> > > be embedded in hardware-specific types to supply any auxiliary
> > > configuration.
> counter arguing shouldn't the client drivers find out of the channel
> requested is capable or not, that can be alternate approach as well.
> That way people implement this in the filer functions and find if this
> is the channel we need rather than dmac finding out if it can service
> the client or not.

How shall clients find this out? This is system- and DMAC-specific, this 
has nothing to do with the client functionality. The proposed approach is:

* a client driver (MMC, USB, anything else) is capable to use DMA uses the 
standard dmaengine API to transfer the data

* if the platform, where it's running, is supplying any auxiliary data, 
that it has to pass to the DMAC driver, it can do so, without getting 
involved in the details, just passing a pointer

* the most natural location to do this is IMHO when requesting a DMA 
channel

Now, on sh-mobile platforms you can realistically have around 5 DMAC 
instances with 2 or 6 channels each, of which, say, 3 controllers are 
suitable for MMC and 2 are not. How shall the filter function find this 
out? Call some ugly platform callback? Traverse some platform-specific 
lists? Or use a fixed channel, thus significantly reducing flexibility? 
Sorry, none of these options seems very attractive to me.

> Frankly I prefer former model, as that way dmacs will present channel
> capabilities, and clients can use as they deem fit.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

[snip]

> > > @@ -948,7 +954,9 @@ int dma_async_device_register(struct dma_device *device);
> > >  void dma_async_device_unregister(struct dma_device *device);
> > >  void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
> > >  struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
> > > -#define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> > > +#define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y, NULL)
> > > +#define dma_request_slave_channel(mask, x, y, id) __dma_request_channel(&(mask), \
> > > +									x, y, id)
> > >  
> > >  /* --- Helper iov-locking functions --- */
> > >  
> So what are we supposed to do with the slave argument. Is the
> expectation that dmacs will parse the parameters in dma_slave_desc and
> based on these return the status of .device_alloc_chan_resources?

Exactly. If the channel, handed in to the DMAC driver by the dmaengine 
core for allocation / configuration is suitable for this slave, actions 
will be performed, otherwise an error will be returned.

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