Re: [RFC PATCH] fsldma: Add DMA_SLAVE support

[Dan Williams <dan.j.williams@intel.com>]

Ira Snyder wrote:
> I can do something similar by calling device_prep_dma_memcpy() lots of
> times. Once for each page in the scatterlist.
> 
> This has the advantage of not changing the DMAEngine API.
> 
> This has the disadvantage of not being a single transaction. The DMA
> controller will get an interrupt after each memcpy() transaction, clean
> up descriptors, etc.
>

Why would it need an interrupt per memcpy?  There is a 
DMA_PREP_INTERRUPT flag to gate interrupt generation to the last 
descriptor.  This is how async_tx delays callbacks until the last 
operation in a chain has completed.  Also, I am not currently seeing how 
your implementation achieves a single hardware transaction.  It still 
calls fsl_dma_alloc_descriptor() per address pair?

The api currently allows a call to ->prep_* to generate multiple 
internal descriptors for a single input address (i.e. memcpy in the case 
where the transfer length exceeds the hardware maximum).  The slave api 
allows for transfers from a scatterlist to a slave context.  I think 
what is bothering me is that the fsldma slave implementation is passing 
a list of sideband addresses rather than a constant address context like 
the existing dw_dmac, so it is different.  However, I can now see that 
trying to enforce uniformity in this area is counterproductive.  The 
DMA_SLAVE interface will always have irreconcilable differences across 
architectures.

> It also has the disadvantage of not being able to change the
> controller-specific features I'm using: external start. This feature
> lets the "3rd device" control the DMA controller. It looks like the
> atmel-mci driver has a similar feature. The DMAEngine API has no way to
> expose this type of feature except through DMA_SLAVE.

Yeah, an example of an architecture specific quirk that has no chance of 
being uniformly handled in a generic api.

> If it is just the 3 helper routines that are the issue with this patch,
> I have no problem dropping them and letting each user re-create them
> themselves.

I think this makes the most sense at this point.  We can reserve 
judgement on the approach until the next fsldma-slave user arrives and 
tries to use this implementation for their device.  Can we move the 
header file under arch/powerpc/include?

[..]
> A single-transaction scatterlist-to-scatterlist copy would be nice.
> 
> One of the major advantages of working with the DMA controller is that
> it automatically handles scatter/gather. Almost all DMA controllers have
> the feature, but it is totally missing from the high-level API.

The engines I am familiar with (ioatdma and iop-adma) still need a 
hardware descriptor per address pair I do not see how fsldma does this 
any more automatically than those engines?  I could see having a helper 
routine that does the mapping and iterating, but in the end the driver 
still sees multiple calls to ->prep (unless of course you are doing this 
in a DMA_SLAVE context, then anything goes).

Thanks,
Dan