Subject: Re: [RFC PATCH] fsldma: Add DMA_SLAVE support

From: Dan Williams <[email-masked]>
Date: Wed, 17 Jun 2009 10:17:54 -0700

Ira Snyder wrote:
>> Using EXPORT_SYMBOL would defeat the purpose of conforming to the
>> dmaengine api which should allow other subsystems to generically
>> discover an fsldma resource.
>>
> 
> Any driver would still use dma_request_channel(), etc. to get access to
> a DMA channel. AFAICT, DMA_SLAVE is intended for doing something
> completely hardware-specific with the DMA controller.

Yes.  Specifically DMA_SLAVE operations imply a pre-established context 
with a 3rd device (the other 2 devices being system memory and the dma 
channel), as compared to plain memcpy.  The assumption is that a dma 
device with this capability may not be shared with other requesters 
until the context is torn down.

[..]
> What I've implemented is this: (sorry about the poor drawing)
> 
> scatterlist           fsl_dma_hw_addr
> +--------+            +-------+
> |  DATA  | -------->  | DEST1 |
> |  DATA  | ----+      +-------+
> |  DATA  |     |
> |  DATA  |     |      +-------+
> |  DATA  |     +--->  | DEST2 |
> |  DATA  |            +-------+
> +--------+
>                           .
>                           .
>                           .
> 
> Of course, the reverse works as well. You can copy from a list of
> hardware address/length pairs to a scatterlist.
> 
> So, using my implementation of the DMA_SLAVE feature, you can take a big
> chunk of data (which is organized into a scatterlist) and DMA it
> directly to a set of hardware addresses, all in a single, unbroken
> transaction.

Could the same effect be achieved by calling ->prep_slave_sg multiple 
times?  Once you need to add dma-driver specific helper routines you are 
effectively extending the dmaengine interface in an fsldma specific 
fashion.  Can we reduce this to just the existing primitives?  If it 
turns out that this is untenable can we extend dmaengine to make this a 
generic capability?  My preference is the former.

> I've inlined the driver for the FPGA programmer below. I don't think it
> is appropriate to push into mainline, since it will only work for our
> board, and nothing else.

If we find that we need to extend the dmaengine interface we will need 
an in-tree user.  In my opinion, as long as it passes the Voyager test 
[1] then I do not see why it should be barred from upstream.

[..]
> He also used platform data to get the register addresses. I'm unaware of
> a way to put arbitrary platform data into the OF tree used on PowerPC.
> 

Anybody else on ppc-dev know if this is possible??

> I didn't want to force other users to implement the allocation routines
> for the struct fsl_dma_hw_addr themselves, so I provided routines to do
> so.

It depends on how many users of this feature there ends up being, 
pushing this into each driver that needs it would not be too horrible 
especially if it just the three straightforward routines 
(fsl_dma_slave_append, fsl_dma_slave_free, and fsl_dma_slave_alloc).

So, the three options in order of preference are:
1/ arrange to call ->prep multiple times to handle each dma address
2/ push the functionality down into the individual drivers that need it
3/ up level the functionality into dmaengine to make it generically 
available

Thanks,
Dan

[1]: The Voyager test refers to James Bottomley's maintenance of the 
Voyager x86-sub-architecture.  If at least one person cares about a 
feature, is willing to maintain the code, and it does not impose a 
maintenance burden on other parts of the tree then it can go upstream.