[PATCH v2 0/2] dmaengine: add slave documentation

[PATCH v2 0/2] dmaengine: add slave documentation

[Koul, Vinod]

From: Vinod Koul <vinod.koul@intel.com>

Version 2:
Added a TODO file for tracking future work
Merged the slave documentation updates to dmaengine.txt
Resolved commenst recieved on v1

Vinod Koul (2):
  dmaengine: Add API documentation for slave dma usage
  dmaengine: add TODO items for future work on dma drivers

 Documentation/dmaengine.txt |   97 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/dma/TODO            |   12 +++++
 2 files changed, 108 insertions(+), 1 deletions(-)
 create mode 100644 drivers/dma/TODO

[PATCH 1/2] dmaengine: Add API documentation for slave dma usage

[Koul, Vinod]

From: Vinod Koul <vinod.koul@intel.com>

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 Documentation/dmaengine.txt |   97 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 96 insertions(+), 1 deletions(-)

diff --git a/Documentation/dmaengine.txt b/Documentation/dmaengine.txt
index 0c1c2f6..5a0cb1e 100644
--- a/Documentation/dmaengine.txt
+++ b/Documentation/dmaengine.txt
@@ -1 +1,96 @@
-See Documentation/crypto/async-tx-api.txt
+			DMA Engine API Guide
+			====================
+
+		 Vinod Koul <vinod dot koul@intel.com>
+
+NOTE: For DMA Engine usage in async_tx please see:
+	Documentation/crypto/async-tx-api.txt
+
+
+Below is a guide to device driver writers on how to use the Slave-DMA API of the
+DMA Engine. This is applicable only for slave DMA usage only.
+
+The slave DMA usage consists of following steps
+1. Allocate a DMA slave channel
+2. Set slave and controller specific parameters
+3. Get a descriptor for transaction
+4. Submit the transaction and wait for callback notification
+
+1. Allocate a DMA slave channel
+Channel allocation is slightly different in the slave DMA context, client
+drivers typically need a channel from a particular DMA controller only and even
+in some cases a specific channel is desired. To request a channel
+dma_request_channel() API is used.
+
+Interface:
+struct dma_chan *dma_request_channel(dma_cap_mask_t mask,
+		dma_filter_fn filter_fn,
+		void *filter_param);
+where dma_filter_fn is defined as:
+typedef bool (*dma_filter_fn)(struct dma_chan *chan, void *filter_param);
+
+When the optional 'filter_fn' parameter is set to NULL dma_request_channel
+simply returns the first channel that satisfies the capability mask.  Otherwise,
+when the mask parameter is insufficient for specifying the necessary channel,
+the filter_fn routine can be used to disposition the available channels in the
+system. The filter_fn routine is called once for each free channel in the
+system.  Upon seeing a suitable channel filter_fn returns DMA_ACK which flags
+that channel to be the return value from dma_request_channel.  A channel
+allocated via this interface is exclusive to the caller, until
+dma_release_channel() is called.
+
+2. Set slave and controller specific parameters
+Next step is always to pass some specific information to the DMA driver. Most of
+the generic information which a slave DMA can use is in struct dma_slave_config.
+It allows the clients to specify DMA direction, DMA addresses, bus widths, DMA
+burst lengths etc. If some DMA controllers have more parameters to be sent then
+they should try to embed struct dma_slave_config in their controller specific
+structure. That gives flexibility to client to pass more parameters, if
+required.
+
+Interface:
+int dmaengine_slave_config(struct dma_chan *chan,
+					  struct dma_slave_config *config)
+
+3. Get a descriptor for transaction
+For slave usage the various modes of slave transfers supported by the
+DMA-engine are:
+slave_sg	- DMA a list of scatter gather buffers from/to a peripheral
+dma_cyclic	- Perform a cyclic DMA operation from/to a peripheral till the
+		  operation is explicitly stopped.
+The non NULL return of this transfer API represents a "descriptor" for the given
+transaction.
+
+Interface:
+struct dma_async_tx_descriptor *(*chan->device->device_prep_dma_sg)(
+		struct dma_chan *chan,
+		struct scatterlist *dst_sg, unsigned int dst_nents,
+		struct scatterlist *src_sg, unsigned int src_nents,
+		unsigned long flags);
+struct dma_async_tx_descriptor *(*chan->device->device_prep_dma_cyclic)(
+		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
+		size_t period_len, enum dma_data_direction direction);
+
+4. Submit the transaction and wait for callback notification
+To schedule the transaction to be scheduled by dma device, the "descriptor"
+returned in above (3) needs to be submitted.
+To tell the dma driver that a transaction is ready to be serviced, the
+descriptor->submit() callback needs to be invoked. This chains the descriptor to
+the pending queue.
+The transactions in the pending queue can be activated by calling the
+issue_pending API. If channel is idle then the first transaction in queue is
+started and subsequent ones queued up.
+On completion of the DMA operation the next in queue is submitted and a tasklet
+triggered. The tasklet would then call the client driver completion callback
+routine for notification, if set.
+Interface:
+void dma_async_issue_pending(struct dma_chan *chan);
+
+==============================================================================
+
+Additional usage notes for dma driver writers
+1/ Although DMA engine specifies that completion callback routines cannot submit
+any new operations, but typically for slave DMA subsequent transaction may not
+be available for submit prior to callback routine being called. This requirement
+is not a requirement for DMA-slave devices. But they should take care to drop
+the spin-lock they might be holding before calling the callback routine
-- 
1.7.0.4

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Koul, Vinod]

From: Vinod Koul <vinod.koul@intel.com>

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/dma/TODO |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
 create mode 100644 drivers/dma/TODO

diff --git a/drivers/dma/TODO b/drivers/dma/TODO
new file mode 100644
index 0000000..b67d184
--- /dev/null
+++ b/drivers/dma/TODO
@@ -0,0 +1,12 @@
+TODO for slave dma
+
+1. Move remaining drivers to use new slave interface
+2. Remove old slave pointer machansim
+3. Make issue_pending to start the transaction in below drivers
+	- mpc512x_dma
+	- imx-dma
+	- imx-sdma
+	- mxs-dma.c
+	- dw_dmac
+	- intel_mid_dma
+	- ste_dma40
-- 
1.7.0.4

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Per Forlin]

On 25 May 2011 13:39, Koul, Vinod <vinod.koul@intel.com> wrote:
> From: Vinod Koul <vinod.koul@intel.com>
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
> ?drivers/dma/TODO | ? 12 ++++++++++++
> ?1 files changed, 12 insertions(+), 0 deletions(-)
> ?create mode 100644 drivers/dma/TODO
>
> diff --git a/drivers/dma/TODO b/drivers/dma/TODO
> new file mode 100644
> index 0000000..b67d184
> --- /dev/null
> +++ b/drivers/dma/TODO
> @@ -0,0 +1,12 @@
> +TODO for slave dma
> +
> +1. Move remaining drivers to use new slave interface
> +2. Remove old slave pointer machansim
> +3. Make issue_pending to start the transaction in below drivers
> + ? ? ? - mpc512x_dma
> + ? ? ? - imx-dma
> + ? ? ? - imx-sdma
> + ? ? ? - mxs-dma.c
> + ? ? ? - dw_dmac
> + ? ? ? - intel_mid_dma
> + ? ? ? - ste_dma40
ste_dma40 already does this. At least is does so in 2.6.39. I may not
be up to date of what is merged for 2.6.40.

/Per

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Koul, Vinod]

On Wed, 2011-05-25 at 14:39 +0200, Per Forlin wrote:
> On 25 May 2011 13:39, Koul, Vinod <vinod.koul@intel.com> wrote:
> > From: Vinod Koul <vinod.koul@intel.com>
> >
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  drivers/dma/TODO |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/dma/TODO
> >
> > diff --git a/drivers/dma/TODO b/drivers/dma/TODO
> > new file mode 100644
> > index 0000000..b67d184
> > --- /dev/null
> > +++ b/drivers/dma/TODO
> > @@ -0,0 +1,12 @@
> > +TODO for slave dma
> > +
> > +1. Move remaining drivers to use new slave interface
> > +2. Remove old slave pointer machansim
> > +3. Make issue_pending to start the transaction in below drivers
> > +       - mpc512x_dma
> > +       - imx-dma
> > +       - imx-sdma
> > +       - mxs-dma.c
> > +       - dw_dmac
> > +       - intel_mid_dma
> > +       - ste_dma40
> ste_dma40 already does this. At least is does so in 2.6.39. I may not
> be up to date of what is merged for 2.6.40.
Yes, I was supposed to drop this, but forgot before sending.
I will update this before I apply if no other updates are required


-- 
~Vinod

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Per Forlin]

On 25 May 2011 14:39, Per Forlin <per.forlin@linaro.org> wrote:
> On 25 May 2011 13:39, Koul, Vinod <vinod.koul@intel.com> wrote:
>> From: Vinod Koul <vinod.koul@intel.com>
>>
>> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
>> ---
>> ?drivers/dma/TODO | ? 12 ++++++++++++
>> ?1 files changed, 12 insertions(+), 0 deletions(-)
>> ?create mode 100644 drivers/dma/TODO
>>
>> diff --git a/drivers/dma/TODO b/drivers/dma/TODO
>> new file mode 100644
>> index 0000000..b67d184
>> --- /dev/null
>> +++ b/drivers/dma/TODO
>> @@ -0,0 +1,12 @@
>> +TODO for slave dma
>> +
>> +1. Move remaining drivers to use new slave interface
>> +2. Remove old slave pointer machansim
>> +3. Make issue_pending to start the transaction in below drivers
>> + ? ? ? - mpc512x_dma
>> + ? ? ? - imx-dma
>> + ? ? ? - imx-sdma
>> + ? ? ? - mxs-dma.c
>> + ? ? ? - dw_dmac
>> + ? ? ? - intel_mid_dma
>> + ? ? ? - ste_dma40
> ste_dma40 already does this. At least is does so in 2.6.39. I may not
> be up to date of what is merged for 2.6.40.
>
I looked at the code once more and ste_dam40 should be on this list :)
ste_dm40 only treats issue_pending accordingly if the DMA driver is in
idle. If the dma is active the desc:s submitted from submit() will be
started later on from dma-irq-function. The dma-irq-function simply
checks if there are pending desc are start them if any. I will fix a
patch for this.

/Per

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Linus Walleij]

2011/5/25 Koul, Vinod <vinod.koul@intel.com>:

> From: Vinod Koul <vinod.koul@intel.com>
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
> ?drivers/dma/TODO | ? 12 ++++++++++++
> ?1 files changed, 12 insertions(+), 0 deletions(-)
> ?create mode 100644 drivers/dma/TODO
>
> diff --git a/drivers/dma/TODO b/drivers/dma/TODO
> new file mode 100644
> index 0000000..b67d184
> --- /dev/null
> +++ b/drivers/dma/TODO
> @@ -0,0 +1,12 @@
> +TODO for slave dma
> +
> +1. Move remaining drivers to use new slave interface

Here is the hitlist:
- find arch/arm/ |grep -e "dma.c$"
- arch/arm/mach-s3c64xx/dma.c need to be merged into
  drivers/dma/amba-pl08x.c - make adjustments for local
  deviations

Or were you mainly talking about the drivers already in
drivers/dma?

Linus Walleij

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Koul, Vinod]

On Wed, 2011-05-25 at 14:47 +0200, Linus Walleij wrote:
> 2011/5/25 Koul, Vinod <vinod.koul@intel.com>:
> 
> > From: Vinod Koul <vinod.koul@intel.com>
> >
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  drivers/dma/TODO |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/dma/TODO
> >
> > diff --git a/drivers/dma/TODO b/drivers/dma/TODO
> > new file mode 100644
> > index 0000000..b67d184
> > --- /dev/null
> > +++ b/drivers/dma/TODO
> > @@ -0,0 +1,12 @@
> > +TODO for slave dma
> > +
> > +1. Move remaining drivers to use new slave interface
> 
> Here is the hitlist:
> - find arch/arm/ |grep -e "dma.c$"
> - arch/arm/mach-s3c64xx/dma.c need to be merged into
>   drivers/dma/amba-pl08x.c - make adjustments for local
>   deviations
> 
> Or were you mainly talking about the drivers already in
> drivers/dma?
I was talking about current drivers still using old pointer mechanism
for passing slave info.

And yes this should be in list too, and this is where I may need your
help :) I will try to compile the list for drivers which should be moved

-- 
~Vinod

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Russell King - ARM Linux]

On Wed, May 25, 2011 at 05:09:30PM +0530, Koul, Vinod wrote:
> +1. Move remaining drivers to use new slave interface
> +2. Remove old slave pointer machansim
> +3. Make issue_pending to start the transaction in below drivers
> +	- mpc512x_dma
> +	- imx-dma
> +	- imx-sdma
> +	- mxs-dma.c
> +	- dw_dmac
> +	- intel_mid_dma
> +	- ste_dma40

I'd suggest adding some more to this:

4. Remove dma_slave_config's dma direction.

It's pointless that dma_slave_config carries the DMA direction (to/from
device) and the prepare function does too.  It leads to DMA engine drivers
having to verify that the two match, and DMA engine users having to issue
two calls every time they change direction.

Instead, lets specify that dma_slave_config carries the _device_ side
parameters, which are selected according to the direction given to the
prepare function.  The memory-side parameters should be selected by the
DMA engine driver according to its knowledge of the system.

This is sensible as M2M transfers don't allow configuration and therefore
already have to select these parameters internally.

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Koul, Vinod]

On Thu, 2011-05-26 at 13:06 +0530, Russell King - ARM Linux wrote:
> On Wed, May 25, 2011 at 05:09:30PM +0530, Koul, Vinod wrote:
> > +1. Move remaining drivers to use new slave interface
> > +2. Remove old slave pointer machansim
> > +3. Make issue_pending to start the transaction in below drivers
> > +	- mpc512x_dma
> > +	- imx-dma
> > +	- imx-sdma
> > +	- mxs-dma.c
> > +	- dw_dmac
> > +	- intel_mid_dma
> > +	- ste_dma40
> 
> I'd suggest adding some more to this:
> 
> 4. Remove dma_slave_config's dma direction.
> 
> It's pointless that dma_slave_config carries the DMA direction (to/from
> device) and the prepare function does too.  It leads to DMA engine drivers
> having to verify that the two match, and DMA engine users having to issue
> two calls every time they change direction.
> 
> Instead, lets specify that dma_slave_config carries the _device_ side
> parameters, which are selected according to the direction given to the
> prepare function.  The memory-side parameters should be selected by the
> DMA engine driver according to its knowledge of the system.
> 
> This is sensible as M2M transfers don't allow configuration and therefore
> already have to select these parameters internally.
Sure, I am adding this as well and applying both patches

Thanks
-- 
~Vinod

[PATCH 2/2] dmaengine: add TODO items for future work on dma drivers

[Russell King - ARM Linux]

On Thu, May 26, 2011 at 05:55:30PM +0530, Koul, Vinod wrote:
> > I'd suggest adding some more to this:
> > 
> > 4. Remove dma_slave_config's dma direction.
> > 
> > It's pointless that dma_slave_config carries the DMA direction (to/from
> > device) and the prepare function does too.  It leads to DMA engine drivers
> > having to verify that the two match, and DMA engine users having to issue
> > two calls every time they change direction.
> > 
> > Instead, lets specify that dma_slave_config carries the _device_ side
> > parameters, which are selected according to the direction given to the
> > prepare function.  The memory-side parameters should be selected by the
> > DMA engine driver according to its knowledge of the system.
> > 
> > This is sensible as M2M transfers don't allow configuration and therefore
> > already have to select these parameters internally.
> Sure, I am adding this as well and applying both patches

Thanks.  In which case I'll mention that I already have patches to make
the PL08x driver do this.