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

[PATCH] 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/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/dma-slave-api.txt

diff --git a/Documentation/dma-slave-api.txt b/Documentation/dma-slave-api.txt
new file mode 100644
index 0000000..7498807
--- /dev/null
+++ b/Documentation/dma-slave-api.txt
@@ -0,0 +1,74 @@
+		     Slave DMA API Guide
+		     ===================
+
+		 Vinod Koul <vinod dot koul at intel.com>
+
+This 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 and not async tx. For
+the async tx usage of DMA Engine please see
+Documentation/crypto/async-tx-api.txt
+
+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:
+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(s) and wait for callback notification when slave API
+is 3 above returns, the non NULL value would imply a "descriptor" for the
+transaction. These transaction(s) would need to be submitted which pushes them
+into queue in DMA driver. If DMA is idle then the first descriptor submit will
+be pushed to DMA and subsequent ones will be queued. 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.
+
+Additional usage notes
+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] 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/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/dma-slave-api.txt

diff --git a/Documentation/dma-slave-api.txt b/Documentation/dma-slave-api.txt
new file mode 100644
index 0000000..7498807
--- /dev/null
+++ b/Documentation/dma-slave-api.txt
@@ -0,0 +1,74 @@
+		     Slave DMA API Guide
+		     ===================
+
+		 Vinod Koul <vinod dot koul@intel.com>
+
+This 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 and not async tx. For
+the async tx usage of DMA Engine please see
+Documentation/crypto/async-tx-api.txt
+
+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:
+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(s) and wait for callback notification when slave API
+is 3 above returns, the non NULL value would imply a "descriptor" for the
+transaction. These transaction(s) would need to be submitted which pushes them
+into queue in DMA driver. If DMA is idle then the first descriptor submit will
+be pushed to DMA and subsequent ones will be queued. 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.
+
+Additional usage notes
+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

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Per Forlin]

Hi Vinod,

On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
> From: Vinod Koul <vinod.koul@intel.com>
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  Documentation/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
>  1 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/dma-slave-api.txt
I suggest putting this in subsection of dmaengine.txt instead.
dmaengine.txt would be the natural place to look at.

>
> diff --git a/Documentation/dma-slave-api.txt b/Documentation/dma-slave-api.txt
> new file mode 100644
> index 0000000..7498807
> --- /dev/null
> +++ b/Documentation/dma-slave-api.txt
> @@ -0,0 +1,74 @@
> +                    Slave DMA API Guide
> +                    ===================
> +
> +                Vinod Koul <vinod dot koul at intel.com>
> +
> +This 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 and not async tx. For
> +the async tx usage of DMA Engine please see
> +Documentation/crypto/async-tx-api.txt
> +
> +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:
> +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(s) and wait for callback notification when slave API
> +is 3 above returns, the non NULL value would imply a "descriptor" for the
> +transaction. These transaction(s) would need to be submitted which pushes them
> +into queue in DMA driver. If DMA is idle then the first descriptor submit will
> +be pushed to DMA and subsequent ones will be queued. 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.
> +
Does submit really start the transfer as well? My interpretation of
submit is that is only adds desc to a pending queue. When calling
issue_pending all these descs will be schedule for DMA transfer. Calls
to submit after this point will added to the pending queue again and
not be issued until calling issue_pending once more.

Thanks for your initiative to document the slave parts of dmaegine.
Per

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

[Per Forlin]

Hi Vinod,

On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
> From: Vinod Koul <vinod.koul@intel.com>
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
> ?Documentation/dma-slave-api.txt | ? 74 +++++++++++++++++++++++++++++++++++++++
> ?1 files changed, 74 insertions(+), 0 deletions(-)
> ?create mode 100644 Documentation/dma-slave-api.txt
I suggest putting this in subsection of dmaengine.txt instead.
dmaengine.txt would be the natural place to look at.

>
> diff --git a/Documentation/dma-slave-api.txt b/Documentation/dma-slave-api.txt
> new file mode 100644
> index 0000000..7498807
> --- /dev/null
> +++ b/Documentation/dma-slave-api.txt
> @@ -0,0 +1,74 @@
> + ? ? ? ? ? ? ? ? ? ?Slave DMA API Guide
> + ? ? ? ? ? ? ? ? ? ?===================
> +
> + ? ? ? ? ? ? ? ?Vinod Koul <vinod dot koul@intel.com>
> +
> +This 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 and not async tx. For
> +the async tx usage of DMA Engine please see
> +Documentation/crypto/async-tx-api.txt
> +
> +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:
> +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(s) and wait for callback notification when slave API
> +is 3 above returns, the non NULL value would imply a "descriptor" for the
> +transaction. These transaction(s) would need to be submitted which pushes them
> +into queue in DMA driver. If DMA is idle then the first descriptor submit will
> +be pushed to DMA and subsequent ones will be queued. 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.
> +
Does submit really start the transfer as well? My interpretation of
submit is that is only adds desc to a pending queue. When calling
issue_pending all these descs will be schedule for DMA transfer. Calls
to submit after this point will added to the pending queue again and
not be issued until calling issue_pending once more.

Thanks for your initiative to document the slave parts of dmaegine.
Per

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Koul, Vinod]

On Tue, 2011-05-24 at 17:40 +0200, Per Forlin wrote:
> Hi Vinod,
> 
> On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
> > From: Vinod Koul <vinod.koul@intel.com>
> >
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  Documentation/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 74 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/dma-slave-api.txt
> I suggest putting this in subsection of dmaengine.txt instead.
> dmaengine.txt would be the natural place to look at.
Agreed, that would make it easier for people to find

> > +4. Submit the transaction(s) and wait for callback notification when slave API
> > +is 3 above returns, the non NULL value would imply a "descriptor" for the
> > +transaction. These transaction(s) would need to be submitted which pushes them
> > +into queue in DMA driver. If DMA is idle then the first descriptor submit will
> > +be pushed to DMA and subsequent ones will be queued. 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.
> > +
> Does submit really start the transfer as well? My interpretation of
> submit is that is only adds desc to a pending queue. When calling
> issue_pending all these descs will be schedule for DMA transfer. Calls
> to submit after this point will added to the pending queue again and
> not be issued until calling issue_pending once more.
For slave dma devices, submit() is used to start the transaction if the
channel is idle. If its already doing a transaction then it will queue
it up and submit once cureent excuting one is completed. It is not
required to call issue_pending once more.
I am not sure if this is true for non slave usage, Dan would that be
correct for you as well?

-- 
~Vinod

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

[Koul, Vinod]

On Tue, 2011-05-24 at 17:40 +0200, Per Forlin wrote:
> Hi Vinod,
> 
> On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
> > From: Vinod Koul <vinod.koul@intel.com>
> >
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  Documentation/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 74 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/dma-slave-api.txt
> I suggest putting this in subsection of dmaengine.txt instead.
> dmaengine.txt would be the natural place to look at.
Agreed, that would make it easier for people to find

> > +4. Submit the transaction(s) and wait for callback notification when slave API
> > +is 3 above returns, the non NULL value would imply a "descriptor" for the
> > +transaction. These transaction(s) would need to be submitted which pushes them
> > +into queue in DMA driver. If DMA is idle then the first descriptor submit will
> > +be pushed to DMA and subsequent ones will be queued. 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.
> > +
> Does submit really start the transfer as well? My interpretation of
> submit is that is only adds desc to a pending queue. When calling
> issue_pending all these descs will be schedule for DMA transfer. Calls
> to submit after this point will added to the pending queue again and
> not be issued until calling issue_pending once more.
For slave dma devices, submit() is used to start the transaction if the
channel is idle. If its already doing a transaction then it will queue
it up and submit once cureent excuting one is completed. It is not
required to call issue_pending once more.
I am not sure if this is true for non slave usage, Dan would that be
correct for you as well?

-- 
~Vinod

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Russell King - ARM Linux]

On Tue, May 24, 2011 at 05:34:17PM +0530, Koul, Vinod wrote:
> +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.

Shouldn't this be "dma_request_channel" ?

> +
> +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);

This should have some description as to what contexts it can be called.

> +
> +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.
> +

This doesn't give the interface for this.

Presumably there's section 3 missing here too for this interface:

> +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);

This should have some description as to what contexts it can be called.

> +
> +4. Submit the transaction(s) and wait for callback notification when slave API
> +is 3 above returns, the non NULL value would imply a "descriptor" for the

"when the slave API is 3 above returns" doesn't make sense - could you
re-phrase?

> +transaction. These transaction(s) would need to be submitted which pushes them
> +into queue in DMA driver. If DMA is idle then the first descriptor submit will
> +be pushed to DMA and subsequent ones will be queued. 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.
> +
> +Additional usage notes
> +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

spurious | character.

> +the spin-lock they might be holding before calling the callback routine

Is this document aimed at driver writers using the DMA engine slave API,
or DMA engine writers implemting the DMA engine slave API?  Most of the
document appears to be aimed at the users of the API except for this
last paragraph, and so could be confusing to users reading this document.

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

[Russell King - ARM Linux]

On Tue, May 24, 2011 at 05:34:17PM +0530, Koul, Vinod wrote:
> +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.

Shouldn't this be "dma_request_channel" ?

> +
> +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);

This should have some description as to what contexts it can be called.

> +
> +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.
> +

This doesn't give the interface for this.

Presumably there's section 3 missing here too for this interface:

> +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);

This should have some description as to what contexts it can be called.

> +
> +4. Submit the transaction(s) and wait for callback notification when slave API
> +is 3 above returns, the non NULL value would imply a "descriptor" for the

"when the slave API is 3 above returns" doesn't make sense - could you
re-phrase?

> +transaction. These transaction(s) would need to be submitted which pushes them
> +into queue in DMA driver. If DMA is idle then the first descriptor submit will
> +be pushed to DMA and subsequent ones will be queued. 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.
> +
> +Additional usage notes
> +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

spurious | character.

> +the spin-lock they might be holding before calling the callback routine

Is this document aimed at driver writers using the DMA engine slave API,
or DMA engine writers implemting the DMA engine slave API?  Most of the
document appears to be aimed at the users of the API except for this
last paragraph, and so could be confusing to users reading this document.

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Dan Williams]

On Tue, May 24, 2011 at 9:06 AM, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Tue, 2011-05-24 at 17:40 +0200, Per Forlin wrote:
>> Hi Vinod,
>>
>> On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> > From: Vinod Koul <vinod.koul@intel.com>
>> >
>> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
>> > ---
>> >  Documentation/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
>> >  1 files changed, 74 insertions(+), 0 deletions(-)
>> >  create mode 100644 Documentation/dma-slave-api.txt
>> I suggest putting this in subsection of dmaengine.txt instead.
>> dmaengine.txt would be the natural place to look at.
> Agreed, that would make it easier for people to find
>
>> > +4. Submit the transaction(s) and wait for callback notification when slave API
>> > +is 3 above returns, the non NULL value would imply a "descriptor" for the
>> > +transaction. These transaction(s) would need to be submitted which pushes them
>> > +into queue in DMA driver. If DMA is idle then the first descriptor submit will
>> > +be pushed to DMA and subsequent ones will be queued. 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.
>> > +
>> Does submit really start the transfer as well? My interpretation of
>> submit is that is only adds desc to a pending queue. When calling
>> issue_pending all these descs will be schedule for DMA transfer. Calls
>> to submit after this point will added to the pending queue again and
>> not be issued until calling issue_pending once more.
> For slave dma devices, submit() is used to start the transaction if the
> channel is idle. If its already doing a transaction then it will queue
> it up and submit once cureent excuting one is completed. It is not
> required to call issue_pending once more.
> I am not sure if this is true for non slave usage, Dan would that be
> correct for you as well?

No, ->submit() is just an "add this descriptor to the chain"
operation, and ->issue_pending() is always required to make sure the
everything submitted previously is actively executing.  This was a
holdover from the very first dmaengine implementation where, for
efficiency reasons, it could save mmio writes by batching the issuing
of requests.

--
Dan

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

[Dan Williams]

On Tue, May 24, 2011 at 9:06 AM, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Tue, 2011-05-24 at 17:40 +0200, Per Forlin wrote:
>> Hi Vinod,
>>
>> On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> > From: Vinod Koul <vinod.koul@intel.com>
>> >
>> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
>> > ---
>> > ?Documentation/dma-slave-api.txt | ? 74 +++++++++++++++++++++++++++++++++++++++
>> > ?1 files changed, 74 insertions(+), 0 deletions(-)
>> > ?create mode 100644 Documentation/dma-slave-api.txt
>> I suggest putting this in subsection of dmaengine.txt instead.
>> dmaengine.txt would be the natural place to look at.
> Agreed, that would make it easier for people to find
>
>> > +4. Submit the transaction(s) and wait for callback notification when slave API
>> > +is 3 above returns, the non NULL value would imply a "descriptor" for the
>> > +transaction. These transaction(s) would need to be submitted which pushes them
>> > +into queue in DMA driver. If DMA is idle then the first descriptor submit will
>> > +be pushed to DMA and subsequent ones will be queued. 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.
>> > +
>> Does submit really start the transfer as well? My interpretation of
>> submit is that is only adds desc to a pending queue. When calling
>> issue_pending all these descs will be schedule for DMA transfer. Calls
>> to submit after this point will added to the pending queue again and
>> not be issued until calling issue_pending once more.
> For slave dma devices, submit() is used to start the transaction if the
> channel is idle. If its already doing a transaction then it will queue
> it up and submit once cureent excuting one is completed. It is not
> required to call issue_pending once more.
> I am not sure if this is true for non slave usage, Dan would that be
> correct for you as well?

No, ->submit() is just an "add this descriptor to the chain"
operation, and ->issue_pending() is always required to make sure the
everything submitted previously is actively executing.  This was a
holdover from the very first dmaengine implementation where, for
efficiency reasons, it could save mmio writes by batching the issuing
of requests.

--
Dan

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Per Forlin]

On 24 May 2011 23:03, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, May 24, 2011 at 9:06 AM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> On Tue, 2011-05-24 at 17:40 +0200, Per Forlin wrote:
>>> Hi Vinod,
>>>
>>> On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>>> > From: Vinod Koul <vinod.koul@intel.com>
>>> >
>>> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
>>> > ---
>>> >  Documentation/dma-slave-api.txt |   74 +++++++++++++++++++++++++++++++++++++++
>>> >  1 files changed, 74 insertions(+), 0 deletions(-)
>>> >  create mode 100644 Documentation/dma-slave-api.txt
>>> I suggest putting this in subsection of dmaengine.txt instead.
>>> dmaengine.txt would be the natural place to look at.
>> Agreed, that would make it easier for people to find
>>
>>> > +4. Submit the transaction(s) and wait for callback notification when slave API
>>> > +is 3 above returns, the non NULL value would imply a "descriptor" for the
>>> > +transaction. These transaction(s) would need to be submitted which pushes them
>>> > +into queue in DMA driver. If DMA is idle then the first descriptor submit will
>>> > +be pushed to DMA and subsequent ones will be queued. 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.
>>> > +
>>> Does submit really start the transfer as well? My interpretation of
>>> submit is that is only adds desc to a pending queue. When calling
>>> issue_pending all these descs will be schedule for DMA transfer. Calls
>>> to submit after this point will added to the pending queue again and
>>> not be issued until calling issue_pending once more.
>> For slave dma devices, submit() is used to start the transaction if the
>> channel is idle. If its already doing a transaction then it will queue
>> it up and submit once cureent excuting one is completed. It is not
>> required to call issue_pending once more.
>> I am not sure if this is true for non slave usage, Dan would that be
>> correct for you as well?
>
> No, ->submit() is just an "add this descriptor to the chain"
> operation, and ->issue_pending() is always required to make sure the
> everything submitted previously is actively executing.  This was a
> holdover from the very first dmaengine implementation where, for
> efficiency reasons, it could save mmio writes by batching the issuing
> of requests.
Thanks Dan for clarifying.
Vinod, I propose that the submit and issue_pending works the same for
SLAVE and none SLAVE channels. The API should have the same definition
independent of DMA_CAP. Please enlighten me if there are things I have
foreseen on this matter.

Thanks,
Per

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

[Per Forlin]

On 24 May 2011 23:03, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, May 24, 2011 at 9:06 AM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> On Tue, 2011-05-24 at 17:40 +0200, Per Forlin wrote:
>>> Hi Vinod,
>>>
>>> On Tue, May 24, 2011 at 2:04 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>>> > From: Vinod Koul <vinod.koul@intel.com>
>>> >
>>> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
>>> > ---
>>> > ?Documentation/dma-slave-api.txt | ? 74 +++++++++++++++++++++++++++++++++++++++
>>> > ?1 files changed, 74 insertions(+), 0 deletions(-)
>>> > ?create mode 100644 Documentation/dma-slave-api.txt
>>> I suggest putting this in subsection of dmaengine.txt instead.
>>> dmaengine.txt would be the natural place to look at.
>> Agreed, that would make it easier for people to find
>>
>>> > +4. Submit the transaction(s) and wait for callback notification when slave API
>>> > +is 3 above returns, the non NULL value would imply a "descriptor" for the
>>> > +transaction. These transaction(s) would need to be submitted which pushes them
>>> > +into queue in DMA driver. If DMA is idle then the first descriptor submit will
>>> > +be pushed to DMA and subsequent ones will be queued. 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.
>>> > +
>>> Does submit really start the transfer as well? My interpretation of
>>> submit is that is only adds desc to a pending queue. When calling
>>> issue_pending all these descs will be schedule for DMA transfer. Calls
>>> to submit after this point will added to the pending queue again and
>>> not be issued until calling issue_pending once more.
>> For slave dma devices, submit() is used to start the transaction if the
>> channel is idle. If its already doing a transaction then it will queue
>> it up and submit once cureent excuting one is completed. It is not
>> required to call issue_pending once more.
>> I am not sure if this is true for non slave usage, Dan would that be
>> correct for you as well?
>
> No, ->submit() is just an "add this descriptor to the chain"
> operation, and ->issue_pending() is always required to make sure the
> everything submitted previously is actively executing. ?This was a
> holdover from the very first dmaengine implementation where, for
> efficiency reasons, it could save mmio writes by batching the issuing
> of requests.
Thanks Dan for clarifying.
Vinod, I propose that the submit and issue_pending works the same for
SLAVE and none SLAVE channels. The API should have the same definition
independent of DMA_CAP. Please enlighten me if there are things I have
foreseen on this matter.

Thanks,
Per

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Koul, Vinod]

On Wed, 2011-05-25 at 09:47 +0200, Per Forlin wrote:
> On 24 May 2011 23:03, Dan Williams <dan.j.williams@intel.com> wrote:

> >>> Does submit really start the transfer as well? My interpretation of
> >>> submit is that is only adds desc to a pending queue. When calling
> >>> issue_pending all these descs will be schedule for DMA transfer. Calls
> >>> to submit after this point will added to the pending queue again and
> >>> not be issued until calling issue_pending once more.
> >> For slave dma devices, submit() is used to start the transaction if the
> >> channel is idle. If its already doing a transaction then it will queue
> >> it up and submit once cureent excuting one is completed. It is not
> >> required to call issue_pending once more.
> >> I am not sure if this is true for non slave usage, Dan would that be
> >> correct for you as well?
> >
> > No, ->submit() is just an "add this descriptor to the chain"
> > operation, and ->issue_pending() is always required to make sure the
> > everything submitted previously is actively executing.  This was a
> > holdover from the very first dmaengine implementation where, for
> > efficiency reasons, it could save mmio writes by batching the issuing
> > of requests.
> Thanks Dan for clarifying.
> Vinod, I propose that the submit and issue_pending works the same for
> SLAVE and none SLAVE channels. The API should have the same definition
> independent of DMA_CAP. Please enlighten me if there are things I have
> foreseen on this matter.
I am okay with making this same for the slave devices as well.

That would also require to change few drivers which start the txn in
submit()


-- 
~Vinod

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

[Koul, Vinod]

On Wed, 2011-05-25 at 09:47 +0200, Per Forlin wrote:
> On 24 May 2011 23:03, Dan Williams <dan.j.williams@intel.com> wrote:

> >>> Does submit really start the transfer as well? My interpretation of
> >>> submit is that is only adds desc to a pending queue. When calling
> >>> issue_pending all these descs will be schedule for DMA transfer. Calls
> >>> to submit after this point will added to the pending queue again and
> >>> not be issued until calling issue_pending once more.
> >> For slave dma devices, submit() is used to start the transaction if the
> >> channel is idle. If its already doing a transaction then it will queue
> >> it up and submit once cureent excuting one is completed. It is not
> >> required to call issue_pending once more.
> >> I am not sure if this is true for non slave usage, Dan would that be
> >> correct for you as well?
> >
> > No, ->submit() is just an "add this descriptor to the chain"
> > operation, and ->issue_pending() is always required to make sure the
> > everything submitted previously is actively executing.  This was a
> > holdover from the very first dmaengine implementation where, for
> > efficiency reasons, it could save mmio writes by batching the issuing
> > of requests.
> Thanks Dan for clarifying.
> Vinod, I propose that the submit and issue_pending works the same for
> SLAVE and none SLAVE channels. The API should have the same definition
> independent of DMA_CAP. Please enlighten me if there are things I have
> foreseen on this matter.
I am okay with making this same for the slave devices as well.

That would also require to change few drivers which start the txn in
submit()


-- 
~Vinod

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Koul, Vinod]

On Tue, 2011-05-24 at 18:48 +0100, Russell King - ARM Linux wrote:
> On Tue, May 24, 2011 at 05:34:17PM +0530, Koul, Vinod wrote:
> 
> Is this document aimed at driver writers using the DMA engine slave API,
> or DMA engine writers implemting the DMA engine slave API?  Most of the
> document appears to be aimed at the users of the API except for this
> last paragraph, and so could be confusing to users reading this document.
Thanks for the comments :)

Its aimed at users, but then thought to add the last one as its often
asked and causes confusion, will explicitly state this...

-- 
~Vinod

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

[Koul, Vinod]

On Tue, 2011-05-24 at 18:48 +0100, Russell King - ARM Linux wrote:
> On Tue, May 24, 2011 at 05:34:17PM +0530, Koul, Vinod wrote:
> 
> Is this document aimed at driver writers using the DMA engine slave API,
> or DMA engine writers implemting the DMA engine slave API?  Most of the
> document appears to be aimed at the users of the API except for this
> last paragraph, and so could be confusing to users reading this document.
Thanks for the comments :)

Its aimed at users, but then thought to add the last one as its often
asked and causes confusion, will explicitly state this...

-- 
~Vinod

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Koul, Vinod]

On Wed, 2011-05-25 at 11:47 +0200, Per Forlin wrote:
> On 25 May 2011 10:26, Koul, Vinod <vinod.koul@intel.com> wrote:
> > I am okay with making this same for the slave devices as well.
> >
> > That would also require to change few drivers which start the txn in
> > submit()
> Yes,
> The trivial part is to move enable from submit to issue_pending. I had
> a quick look and it looks like the following drivers need to be
> changed.
> mpc512x_dma.c
> imx-dma.c
> imx-sdma.c
> mxs-dma.c
> 
> The other part is more complex. To make submit only add descs to a
> queue that will be pushed down to the DMA only after issue_pending. It
> is not as trivial to identify which of the drivers support this or
> not.
> I still think it make sense to fix the documentation first and then
> fix the drivers. Keep a list inside the dmaengine.txt of drivers that
> need to be fixed in order to comply with the API. Over time drivers
> will be fixed and removed from the list. When we have agreed upon the
> API (we may not be there yet) I am willingly to make a draft of such a
> list.
Yes this sounds right, I will send updated version and now drivers would
need to call issue_pending.

I know dw_dmac, intel_mid_dma and ste_dma40 would need changes as well.

Was thinking to add TODO in drivers/dma with these and any other changes
we feel are required moving forward.
We need dw_dmac to be moved to new slave interface and remove old slave
pointer mechanism when all are moved. 

-- 
~Vinod

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

[Koul, Vinod]

On Wed, 2011-05-25 at 11:47 +0200, Per Forlin wrote:
> On 25 May 2011 10:26, Koul, Vinod <vinod.koul@intel.com> wrote:
> > I am okay with making this same for the slave devices as well.
> >
> > That would also require to change few drivers which start the txn in
> > submit()
> Yes,
> The trivial part is to move enable from submit to issue_pending. I had
> a quick look and it looks like the following drivers need to be
> changed.
> mpc512x_dma.c
> imx-dma.c
> imx-sdma.c
> mxs-dma.c
> 
> The other part is more complex. To make submit only add descs to a
> queue that will be pushed down to the DMA only after issue_pending. It
> is not as trivial to identify which of the drivers support this or
> not.
> I still think it make sense to fix the documentation first and then
> fix the drivers. Keep a list inside the dmaengine.txt of drivers that
> need to be fixed in order to comply with the API. Over time drivers
> will be fixed and removed from the list. When we have agreed upon the
> API (we may not be there yet) I am willingly to make a draft of such a
> list.
Yes this sounds right, I will send updated version and now drivers would
need to call issue_pending.

I know dw_dmac, intel_mid_dma and ste_dma40 would need changes as well.

Was thinking to add TODO in drivers/dma with these and any other changes
we feel are required moving forward.
We need dw_dmac to be moved to new slave interface and remove old slave
pointer mechanism when all are moved. 

-- 
~Vinod

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Per Forlin]

On 25 May 2011 10:26, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Wed, 2011-05-25 at 09:47 +0200, Per Forlin wrote:
>> On 24 May 2011 23:03, Dan Williams <dan.j.williams@intel.com> wrote:
>
>> >>> Does submit really start the transfer as well? My interpretation of
>> >>> submit is that is only adds desc to a pending queue. When calling
>> >>> issue_pending all these descs will be schedule for DMA transfer. Calls
>> >>> to submit after this point will added to the pending queue again and
>> >>> not be issued until calling issue_pending once more.
>> >> For slave dma devices, submit() is used to start the transaction if the
>> >> channel is idle. If its already doing a transaction then it will queue
>> >> it up and submit once cureent excuting one is completed. It is not
>> >> required to call issue_pending once more.
>> >> I am not sure if this is true for non slave usage, Dan would that be
>> >> correct for you as well?
>> >
>> > No, ->submit() is just an "add this descriptor to the chain"
>> > operation, and ->issue_pending() is always required to make sure the
>> > everything submitted previously is actively executing.  This was a
>> > holdover from the very first dmaengine implementation where, for
>> > efficiency reasons, it could save mmio writes by batching the issuing
>> > of requests.
>> Thanks Dan for clarifying.
>> Vinod, I propose that the submit and issue_pending works the same for
>> SLAVE and none SLAVE channels. The API should have the same definition
>> independent of DMA_CAP. Please enlighten me if there are things I have
>> foreseen on this matter.
> I am okay with making this same for the slave devices as well.
>
> That would also require to change few drivers which start the txn in
> submit()
Yes,
The trivial part is to move enable from submit to issue_pending. I had
a quick look and it looks like the following drivers need to be
changed.
mpc512x_dma.c
imx-dma.c
imx-sdma.c
mxs-dma.c

The other part is more complex. To make submit only add descs to a
queue that will be pushed down to the DMA only after issue_pending. It
is not as trivial to identify which of the drivers support this or
not.
I still think it make sense to fix the documentation first and then
fix the drivers. Keep a list inside the dmaengine.txt of drivers that
need to be fixed in order to comply with the API. Over time drivers
will be fixed and removed from the list. When we have agreed upon the
API (we may not be there yet) I am willingly to make a draft of such a
list.

/Per

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

[Per Forlin]

On 25 May 2011 10:26, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Wed, 2011-05-25 at 09:47 +0200, Per Forlin wrote:
>> On 24 May 2011 23:03, Dan Williams <dan.j.williams@intel.com> wrote:
>
>> >>> Does submit really start the transfer as well? My interpretation of
>> >>> submit is that is only adds desc to a pending queue. When calling
>> >>> issue_pending all these descs will be schedule for DMA transfer. Calls
>> >>> to submit after this point will added to the pending queue again and
>> >>> not be issued until calling issue_pending once more.
>> >> For slave dma devices, submit() is used to start the transaction if the
>> >> channel is idle. If its already doing a transaction then it will queue
>> >> it up and submit once cureent excuting one is completed. It is not
>> >> required to call issue_pending once more.
>> >> I am not sure if this is true for non slave usage, Dan would that be
>> >> correct for you as well?
>> >
>> > No, ->submit() is just an "add this descriptor to the chain"
>> > operation, and ->issue_pending() is always required to make sure the
>> > everything submitted previously is actively executing. ?This was a
>> > holdover from the very first dmaengine implementation where, for
>> > efficiency reasons, it could save mmio writes by batching the issuing
>> > of requests.
>> Thanks Dan for clarifying.
>> Vinod, I propose that the submit and issue_pending works the same for
>> SLAVE and none SLAVE channels. The API should have the same definition
>> independent of DMA_CAP. Please enlighten me if there are things I have
>> foreseen on this matter.
> I am okay with making this same for the slave devices as well.
>
> That would also require to change few drivers which start the txn in
> submit()
Yes,
The trivial part is to move enable from submit to issue_pending. I had
a quick look and it looks like the following drivers need to be
changed.
mpc512x_dma.c
imx-dma.c
imx-sdma.c
mxs-dma.c

The other part is more complex. To make submit only add descs to a
queue that will be pushed down to the DMA only after issue_pending. It
is not as trivial to identify which of the drivers support this or
not.
I still think it make sense to fix the documentation first and then
fix the drivers. Keep a list inside the dmaengine.txt of drivers that
need to be fixed in order to comply with the API. Over time drivers
will be fixed and removed from the list. When we have agreed upon the
API (we may not be there yet) I am willingly to make a draft of such a
list.

/Per

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Per Forlin]

On 25 May 2011 11:34, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Wed, 2011-05-25 at 11:47 +0200, Per Forlin wrote:
>> On 25 May 2011 10:26, Koul, Vinod <vinod.koul@intel.com> wrote:
>> > I am okay with making this same for the slave devices as well.
>> >
>> > That would also require to change few drivers which start the txn in
>> > submit()
>> Yes,
>> The trivial part is to move enable from submit to issue_pending. I had
>> a quick look and it looks like the following drivers need to be
>> changed.
>> mpc512x_dma.c
>> imx-dma.c
>> imx-sdma.c
>> mxs-dma.c
>>
>> The other part is more complex. To make submit only add descs to a
>> queue that will be pushed down to the DMA only after issue_pending. It
>> is not as trivial to identify which of the drivers support this or
>> not.
>> I still think it make sense to fix the documentation first and then
>> fix the drivers. Keep a list inside the dmaengine.txt of drivers that
>> need to be fixed in order to comply with the API. Over time drivers
>> will be fixed and removed from the list. When we have agreed upon the
>> API (we may not be there yet) I am willingly to make a draft of such a
>> list.
> Yes this sounds right, I will send updated version and now drivers would
> need to call issue_pending.
>
> I know dw_dmac, intel_mid_dma and ste_dma40 would need changes as well.
>
I think ste_dma40 treats issue_pending and submit correctly but it may
be other things not aligned. I need to double check this since it was
a long time since I did any work in this driver. I can update
ste_dma40 where it need changes.

> Was thinking to add TODO in drivers/dma with these and any other changes
> we feel are required moving forward.
> We need dw_dmac to be moved to new slave interface and remove old slave
> pointer mechanism when all are moved.
>
I agree, TODO is a better place for it.

I would like to ask a question on usage of DMA_CTRL_ACK while we're at it.
In ste_dma40 I added support to reuse descriptors if DMA_CTRL_ACK is
clear. The purpose of DMA_CTRL_ACK is to set up dependency but is it
also a valid to reuse it?
I mean submit the same descriptor again and again without having to
call prepare_slave_sg.
Example:
desc = prepare_slave_sg()
submit(desc)
issue_pending(desc)

wait_for_callback()

submit(desc)
issue_pending(desc)

/Per

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

[Per Forlin]

On 25 May 2011 11:34, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Wed, 2011-05-25 at 11:47 +0200, Per Forlin wrote:
>> On 25 May 2011 10:26, Koul, Vinod <vinod.koul@intel.com> wrote:
>> > I am okay with making this same for the slave devices as well.
>> >
>> > That would also require to change few drivers which start the txn in
>> > submit()
>> Yes,
>> The trivial part is to move enable from submit to issue_pending. I had
>> a quick look and it looks like the following drivers need to be
>> changed.
>> mpc512x_dma.c
>> imx-dma.c
>> imx-sdma.c
>> mxs-dma.c
>>
>> The other part is more complex. To make submit only add descs to a
>> queue that will be pushed down to the DMA only after issue_pending. It
>> is not as trivial to identify which of the drivers support this or
>> not.
>> I still think it make sense to fix the documentation first and then
>> fix the drivers. Keep a list inside the dmaengine.txt of drivers that
>> need to be fixed in order to comply with the API. Over time drivers
>> will be fixed and removed from the list. When we have agreed upon the
>> API (we may not be there yet) I am willingly to make a draft of such a
>> list.
> Yes this sounds right, I will send updated version and now drivers would
> need to call issue_pending.
>
> I know dw_dmac, intel_mid_dma and ste_dma40 would need changes as well.
>
I think ste_dma40 treats issue_pending and submit correctly but it may
be other things not aligned. I need to double check this since it was
a long time since I did any work in this driver. I can update
ste_dma40 where it need changes.

> Was thinking to add TODO in drivers/dma with these and any other changes
> we feel are required moving forward.
> We need dw_dmac to be moved to new slave interface and remove old slave
> pointer mechanism when all are moved.
>
I agree, TODO is a better place for it.

I would like to ask a question on usage of DMA_CTRL_ACK while we're at it.
In ste_dma40 I added support to reuse descriptors if DMA_CTRL_ACK is
clear. The purpose of DMA_CTRL_ACK is to set up dependency but is it
also a valid to reuse it?
I mean submit the same descriptor again and again without having to
call prepare_slave_sg.
Example:
desc = prepare_slave_sg()
submit(desc)
issue_pending(desc)

wait_for_callback()

submit(desc)
issue_pending(desc)

/Per

Re: [PATCH] dmaengine: Add API documentation for slave dma usage

[Koul, Vinod]

On Wed, 2011-05-25 at 16:21 +0530, Per Forlin wrote:
> On 25 May 2011 11:34, Koul, Vinod <vinod.koul@intel.com> wrote:

> I would like to ask a question on usage of DMA_CTRL_ACK while we're at it.
> In ste_dma40 I added support to reuse descriptors if DMA_CTRL_ACK is
> clear. The purpose of DMA_CTRL_ACK is to set up dependency but is it
> also a valid to reuse it?
> I mean submit the same descriptor again and again without having to
> call prepare_slave_sg.
> Example:
> desc = prepare_slave_sg()
> submit(desc)
> issue_pending(desc)
> 
> wait_for_callback()
> 
> submit(desc)
> issue_pending(desc)
> 
I think your question is answered in dmaengine.h. Quoting here:
* @DMA_CTRL_ACK - if clear, the descriptor cannot be reused until the 
* client acknowledges receipt, i.e. has has a chance to establish any
* dependency chains

The DMA driver recycles the descriptors which it returns for the
transfer APIs. The DMA_CTRL_ACK flag would tell driver explicitly not to
reuse this descriptor until the client clears this flag.
Since the descriptor is still valid after callback notification it can
be reused by client in the way you described.

Again, Dan can you please confirm if my interpretation is correct.


-- 
~Vinod

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

[Koul, Vinod]

On Wed, 2011-05-25 at 16:21 +0530, Per Forlin wrote:
> On 25 May 2011 11:34, Koul, Vinod <vinod.koul@intel.com> wrote:

> I would like to ask a question on usage of DMA_CTRL_ACK while we're at it.
> In ste_dma40 I added support to reuse descriptors if DMA_CTRL_ACK is
> clear. The purpose of DMA_CTRL_ACK is to set up dependency but is it
> also a valid to reuse it?
> I mean submit the same descriptor again and again without having to
> call prepare_slave_sg.
> Example:
> desc = prepare_slave_sg()
> submit(desc)
> issue_pending(desc)
> 
> wait_for_callback()
> 
> submit(desc)
> issue_pending(desc)
> 
I think your question is answered in dmaengine.h. Quoting here:
* @DMA_CTRL_ACK - if clear, the descriptor cannot be reused until the 
* client acknowledges receipt, i.e. has has a chance to establish any
* dependency chains

The DMA driver recycles the descriptors which it returns for the
transfer APIs. The DMA_CTRL_ACK flag would tell driver explicitly not to
reuse this descriptor until the client clears this flag.
Since the descriptor is still valid after callback notification it can
be reused by client in the way you described.

Again, Dan can you please confirm if my interpretation is correct.


-- 
~Vinod