[PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Andy Gross]

This patch adds APIs that allow for BAM hardware flags to be set per
descriptor.  Each one of the new flags informs the attached peripheral of a
special behavior that is required.

The EOT flag requests that the peripheral assert an end of transaction interrupt
when that descriptor is complete.  It also results in special signaling protocol
that is used between the attached peripheral and the core using the DMA
controller.

The NWD flag requests that the peripheral wait until the data has been fully
processed before signaling an interrupt.

The CMD flag informs the peripheral that the descriptor payload contains
command descriptors and not data descriptors.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 drivers/dma/qcom_bam_dma.c       |   48 ++++++++++++++++++++++++++++++++++++--
 include/linux/dma/qcom_bam_dma.h |   23 ++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/dma/qcom_bam_dma.h

diff --git a/drivers/dma/qcom_bam_dma.c b/drivers/dma/qcom_bam_dma.c
index 02f7fef..f6c8ef1 100644
--- a/drivers/dma/qcom_bam_dma.c
+++ b/drivers/dma/qcom_bam_dma.c
@@ -61,12 +61,18 @@ struct bam_desc_hw {
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
+#define DESC_FLAG_NWD BIT(12)
+#define DESC_FLAG_CMD BIT(11)
 
 struct bam_async_desc {
 	struct virt_dma_desc vd;
 
 	u32 num_desc;
 	u32 xfer_len;
+
+	/* transaction flags, EOT|EOB|NWD|CMD */
+	u16 flags;
+
 	struct bam_desc_hw *curr_desc;
 
 	enum dma_transfer_direction dir;
@@ -800,6 +806,34 @@ static void bam_apply_new_config(struct bam_chan *bchan,
 	bchan->reconfigure = 0;
 }
 
+void qcom_bam_set_desc_eot(struct dma_async_tx_descriptor *txd)
+{
+	struct bam_async_desc *async_desc = container_of(txd,
+			struct bam_async_desc, vd.tx);
+
+	async_desc->flags |= DESC_FLAG_EOT;
+}
+EXPORT_SYMBOL(qcom_bam_set_desc_eot);
+
+void qcom_bam_set_desc_cmd(struct dma_async_tx_descriptor *txd)
+{
+	struct bam_async_desc *async_desc = container_of(txd,
+			struct bam_async_desc, vd.tx);
+
+	async_desc->flags |= DESC_FLAG_CMD;
+}
+EXPORT_SYMBOL(qcom_bam_set_desc_cmd);
+
+void qcom_bam_set_desc_nwd(struct dma_async_tx_descriptor *txd)
+{
+	struct bam_async_desc *async_desc = container_of(txd,
+			struct bam_async_desc, vd.tx);
+
+	async_desc->flags |= DESC_FLAG_NWD;
+}
+EXPORT_SYMBOL(qcom_bam_set_desc_nwd);
+
+
 /**
  * bam_start_dma - start next transaction
  * @bchan - bam dma channel
@@ -812,6 +846,7 @@ static void bam_start_dma(struct bam_chan *bchan)
 	struct bam_desc_hw *desc;
 	struct bam_desc_hw *fifo = PTR_ALIGN(bchan->fifo_virt,
 					sizeof(struct bam_desc_hw));
+	int i;
 
 	lockdep_assert_held(&bchan->vc.lock);
 
@@ -838,8 +873,17 @@ static void bam_start_dma(struct bam_chan *bchan)
 	else
 		async_desc->xfer_len = async_desc->num_desc;
 
-	/* set INT on last descriptor */
-	desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_INT;
+	/* set command descriptor flag, if applicable */
+	if (async_desc->flags & DESC_FLAG_CMD)
+		for (i = 0; i < async_desc->xfer_len; i++)
+			desc[i].flags |= DESC_FLAG_CMD;
+
+	/* set EOT or INT based on flag settings and if final transaction */
+	if (async_desc->flags & DESC_FLAG_EOT &&
+		async_desc->num_desc == async_desc->xfer_len)
+		desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_EOT;
+	else
+		desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_INT;
 
 	if (bchan->tail + async_desc->xfer_len > MAX_DESCRIPTORS) {
 		u32 partial = MAX_DESCRIPTORS - bchan->tail;
diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h
new file mode 100644
index 0000000..65a371eb
--- /dev/null
+++ b/include/linux/dma/qcom_bam_dma.h
@@ -0,0 +1,23 @@
+#ifndef _QCOM_BAM_DMA_H_
+#define _QCOM_BAM_DMA_H_
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/dmaengine.h>
+
+void qcom_bam_set_desc_cmd(struct dma_async_tx_descriptor *);
+void qcom_bam_set_desc_eot(struct dma_async_tx_descriptor *);
+void qcom_bam_set_desc_nwd(struct dma_async_tx_descriptor *);
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Vinod Koul]

On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote:
> This patch adds APIs that allow for BAM hardware flags to be set per
> descriptor.  Each one of the new flags informs the attached peripheral of a
> special behavior that is required.
> 
> The EOT flag requests that the peripheral assert an end of transaction interrupt
> when that descriptor is complete.  It also results in special signaling protocol
> that is used between the attached peripheral and the core using the DMA
> controller.
DMA_PREP_INTERRUPT ??

> 
> The NWD flag requests that the peripheral wait until the data has been fully
> processed before signaling an interrupt.
interrupt for transaction complete or DMA request?

-- 
~Vinod

> 
> The CMD flag informs the peripheral that the descriptor payload contains
> command descriptors and not data descriptors.
> 
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> ---
>  drivers/dma/qcom_bam_dma.c       |   48 ++++++++++++++++++++++++++++++++++++--
>  include/linux/dma/qcom_bam_dma.h |   23 ++++++++++++++++++
>  2 files changed, 69 insertions(+), 2 deletions(-)
>  create mode 100644 include/linux/dma/qcom_bam_dma.h
> 
> diff --git a/drivers/dma/qcom_bam_dma.c b/drivers/dma/qcom_bam_dma.c
> index 02f7fef..f6c8ef1 100644
> --- a/drivers/dma/qcom_bam_dma.c
> +++ b/drivers/dma/qcom_bam_dma.c
> @@ -61,12 +61,18 @@ struct bam_desc_hw {
>  #define DESC_FLAG_INT BIT(15)
>  #define DESC_FLAG_EOT BIT(14)
>  #define DESC_FLAG_EOB BIT(13)
> +#define DESC_FLAG_NWD BIT(12)
> +#define DESC_FLAG_CMD BIT(11)
>  
>  struct bam_async_desc {
>  	struct virt_dma_desc vd;
>  
>  	u32 num_desc;
>  	u32 xfer_len;
> +
> +	/* transaction flags, EOT|EOB|NWD|CMD */
> +	u16 flags;
> +
>  	struct bam_desc_hw *curr_desc;
>  
>  	enum dma_transfer_direction dir;
> @@ -800,6 +806,34 @@ static void bam_apply_new_config(struct bam_chan *bchan,
>  	bchan->reconfigure = 0;
>  }
>  
> +void qcom_bam_set_desc_eot(struct dma_async_tx_descriptor *txd)
> +{
> +	struct bam_async_desc *async_desc = container_of(txd,
> +			struct bam_async_desc, vd.tx);
> +
> +	async_desc->flags |= DESC_FLAG_EOT;
> +}
> +EXPORT_SYMBOL(qcom_bam_set_desc_eot);
> +
> +void qcom_bam_set_desc_cmd(struct dma_async_tx_descriptor *txd)
> +{
> +	struct bam_async_desc *async_desc = container_of(txd,
> +			struct bam_async_desc, vd.tx);
> +
> +	async_desc->flags |= DESC_FLAG_CMD;
> +}
> +EXPORT_SYMBOL(qcom_bam_set_desc_cmd);
> +
> +void qcom_bam_set_desc_nwd(struct dma_async_tx_descriptor *txd)
> +{
> +	struct bam_async_desc *async_desc = container_of(txd,
> +			struct bam_async_desc, vd.tx);
> +
> +	async_desc->flags |= DESC_FLAG_NWD;
> +}
> +EXPORT_SYMBOL(qcom_bam_set_desc_nwd);
> +
> +
>  /**
>   * bam_start_dma - start next transaction
>   * @bchan - bam dma channel
> @@ -812,6 +846,7 @@ static void bam_start_dma(struct bam_chan *bchan)
>  	struct bam_desc_hw *desc;
>  	struct bam_desc_hw *fifo = PTR_ALIGN(bchan->fifo_virt,
>  					sizeof(struct bam_desc_hw));
> +	int i;
>  
>  	lockdep_assert_held(&bchan->vc.lock);
>  
> @@ -838,8 +873,17 @@ static void bam_start_dma(struct bam_chan *bchan)
>  	else
>  		async_desc->xfer_len = async_desc->num_desc;
>  
> -	/* set INT on last descriptor */
> -	desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_INT;
> +	/* set command descriptor flag, if applicable */
> +	if (async_desc->flags & DESC_FLAG_CMD)
> +		for (i = 0; i < async_desc->xfer_len; i++)
> +			desc[i].flags |= DESC_FLAG_CMD;
> +
> +	/* set EOT or INT based on flag settings and if final transaction */
> +	if (async_desc->flags & DESC_FLAG_EOT &&
> +		async_desc->num_desc == async_desc->xfer_len)
> +		desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_EOT;
> +	else
> +		desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_INT;
>  
>  	if (bchan->tail + async_desc->xfer_len > MAX_DESCRIPTORS) {
>  		u32 partial = MAX_DESCRIPTORS - bchan->tail;
> diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h
> new file mode 100644
> index 0000000..65a371eb
> --- /dev/null
> +++ b/include/linux/dma/qcom_bam_dma.h
> @@ -0,0 +1,23 @@
> +#ifndef _QCOM_BAM_DMA_H_
> +#define _QCOM_BAM_DMA_H_
> +/*
> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/dmaengine.h>
> +
> +void qcom_bam_set_desc_cmd(struct dma_async_tx_descriptor *);
> +void qcom_bam_set_desc_eot(struct dma_async_tx_descriptor *);
> +void qcom_bam_set_desc_nwd(struct dma_async_tx_descriptor *);
> +
> +#endif
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Andy Gross]

On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote:
> On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote:
> > This patch adds APIs that allow for BAM hardware flags to be set per
> > descriptor.  Each one of the new flags informs the attached peripheral of a
> > special behavior that is required.
> > 
> > The EOT flag requests that the peripheral assert an end of transaction interrupt
> > when that descriptor is complete.  It also results in special signaling protocol
> > that is used between the attached peripheral and the core using the DMA
> > controller.
> DMA_PREP_INTERRUPT ??

I have 3 different IRQs that can be asserted based on the bit I set in the
hardware descriptor.  The normal IRQ is the INT bit.  However, in some cases the
peripheral protocol requires the use of the EOT or EOB interrupt instead.  The
DMA_PREP_INTERRUPT would only work if I had only 2 choices.

> 
> > 
> > The NWD flag requests that the peripheral wait until the data has been fully
> > processed before signaling an interrupt.
> interrupt for transaction complete or DMA request?

This is a special signaling mechanism that holds off the DMA interrupt until the
peripheral actually acks that the data has been processed completely.  This is
required in many cases by the peripheral.  One example is the SPI controller.
At the end of a transaction you are supposed to set the NWD so that the chip
select is de-asserted.

<snip>

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Andy Gross]

On Fri, May 02, 2014 at 01:08:27PM -0500, Andy Gross wrote:
> On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote:
> > On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote:
> > > This patch adds APIs that allow for BAM hardware flags to be set per
> > > descriptor.  Each one of the new flags informs the attached peripheral of a
> > > special behavior that is required.
> > > 
> > > The EOT flag requests that the peripheral assert an end of transaction interrupt
> > > when that descriptor is complete.  It also results in special signaling protocol
> > > that is used between the attached peripheral and the core using the DMA
> > > controller.
> > DMA_PREP_INTERRUPT ??
> 
> I have 3 different IRQs that can be asserted based on the bit I set in the
> hardware descriptor.  The normal IRQ is the INT bit.  However, in some cases the
> peripheral protocol requires the use of the EOT or EOB interrupt instead.  The
> DMA_PREP_INTERRUPT would only work if I had only 2 choices.

Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT
flag.  However, I might get in a bind later if I need to support the EOB (end of
block) interrupt.

> 
> > 
> > > 
> > > The NWD flag requests that the peripheral wait until the data has been fully
> > > processed before signaling an interrupt.
> > interrupt for transaction complete or DMA request?
> 
> This is a special signaling mechanism that holds off the DMA interrupt until the
> peripheral actually acks that the data has been processed completely.  This is
> required in many cases by the peripheral.  One example is the SPI controller.
> At the end of a transaction you are supposed to set the NWD so that the chip
> select is de-asserted.

I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK?  or maybe the
DMA_PREP_FENCE?  I don't generally like overloading the flags and slightly
twisting their intent.  Could we add a flag to denote device ACK?

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Srinivas Kandagatla]

Hi Andy,

On 15/05/14 18:32, Andy Gross wrote:
> On Fri, May 02, 2014 at 01:08:27PM -0500, Andy Gross wrote:
>> On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote:
>>> On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote:
>>>> This patch adds APIs that allow for BAM hardware flags to be set per
>>>> descriptor.  Each one of the new flags informs the attached peripheral of a
>>>> special behavior that is required.
>>>>
>>>> The EOT flag requests that the peripheral assert an end of transaction interrupt
>>>> when that descriptor is complete.  It also results in special signaling protocol
>>>> that is used between the attached peripheral and the core using the DMA
>>>> controller.
>>> DMA_PREP_INTERRUPT ??
>>
>> I have 3 different IRQs that can be asserted based on the bit I set in the
>> hardware descriptor.  The normal IRQ is the INT bit.  However, in some cases the
>> peripheral protocol requires the use of the EOT or EOB interrupt instead.  The
>> DMA_PREP_INTERRUPT would only work if I had only 2 choices.
>
> Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT
> flag.  However, I might get in a bind later if I need to support the EOB (end of
> block) interrupt.

This is good start, mapping EOT to DMA_PREP_INTERRUPT seems to be much 
appropriate. This will also provide mmci driver with much more generic 
interface than the bam specific function calls.

Only way forward is to tie up this descriptor specific flags to more 
generic flags. Having specific callbacks would introduce limitations in 
using generic device drivers.

Not really sure how we can map EOB/NWD flags without really defining new 
flags. Mapping to other generic flags might be totally confusing to 
people using/interpreting those flags. Needs more discussion on this.



--srini
>
>>
>>>
>>>>
>>>> The NWD flag requests that the peripheral wait until the data has been fully
>>>> processed before signaling an interrupt.
>>> interrupt for transaction complete or DMA request?
>>
>> This is a special signaling mechanism that holds off the DMA interrupt until the
>> peripheral actually acks that the data has been processed completely.  This is
>> required in many cases by the peripheral.  One example is the SPI controller.
>> At the end of a transaction you are supposed to set the NWD so that the chip
>> select is de-asserted.
>
> I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK?  or maybe the
> DMA_PREP_FENCE?  I don't generally like overloading the flags and slightly
> twisting their intent.  Could we add a flag to denote device ACK?
>

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Vinod Koul]

On Thu, May 15, 2014 at 12:32:06PM -0500, Andy Gross wrote:
> On Fri, May 02, 2014 at 01:08:27PM -0500, Andy Gross wrote:
> > On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote:
> > > On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote:
> > > > This patch adds APIs that allow for BAM hardware flags to be set per
> > > > descriptor.  Each one of the new flags informs the attached peripheral of a
> > > > special behavior that is required.
> > > > 
> > > > The EOT flag requests that the peripheral assert an end of transaction interrupt
> > > > when that descriptor is complete.  It also results in special signaling protocol
> > > > that is used between the attached peripheral and the core using the DMA
> > > > controller.
> > > DMA_PREP_INTERRUPT ??
> > 
> > I have 3 different IRQs that can be asserted based on the bit I set in the
> > hardware descriptor.  The normal IRQ is the INT bit.  However, in some cases the
> > peripheral protocol requires the use of the EOT or EOB interrupt instead.  The
> > DMA_PREP_INTERRUPT would only work if I had only 2 choices.
> 
> Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT
> flag.  However, I might get in a bind later if I need to support the EOB (end of
> block) interrupt.
Sorry for delay in this.

I think it would make sense to use DMA_PREP_INTERRUPT for EOB interrupt. The EOT
should always be enabled for the cases where it is applicable instead of nomral
irq.

That should genrically ocvery your cases, or did we miss anything here
> 
> > 
> > > 
> > > > 
> > > > The NWD flag requests that the peripheral wait until the data has been fully
> > > > processed before signaling an interrupt.
> > > interrupt for transaction complete or DMA request?
> > 
> > This is a special signaling mechanism that holds off the DMA interrupt until the
> > peripheral actually acks that the data has been processed completely.  This is
> > required in many cases by the peripheral.  One example is the SPI controller.
> > At the end of a transaction you are supposed to set the NWD so that the chip
> > select is de-asserted.
> 
> I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK?  or maybe the
> DMA_PREP_FENCE?  I don't generally like overloading the flags and slightly
> twisting their intent.  Could we add a flag to denote device ACK?
Nope lets not override these...

-- 
~Vinod

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Andy Gross]

On Thu, May 22, 2014 at 11:40:49AM +0530, Vinod Koul wrote:

<snip>

> > > I have 3 different IRQs that can be asserted based on the bit I set in the
> > > hardware descriptor.  The normal IRQ is the INT bit.  However, in some cases the
> > > peripheral protocol requires the use of the EOT or EOB interrupt instead.  The
> > > DMA_PREP_INTERRUPT would only work if I had only 2 choices.
> > 
> > Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT
> > flag.  However, I might get in a bind later if I need to support the EOB (end of
> > block) interrupt.
> Sorry for delay in this.
> 
> I think it would make sense to use DMA_PREP_INTERRUPT for EOB interrupt. The EOT
> should always be enabled for the cases where it is applicable instead of nomral
> irq.
> 
> That should genrically ocvery your cases, or did we miss anything here

The EOT is not used for every transaction.  It is part of a handshaking
protocol with the attached peripheral, much like the NWD (notify when done).  As
near as I can tell today, no peripheral depends on the EOB, so we could drop it
for now until it is needed and cross this bridge when we need to.

> > 
> > > 
> > > > 
> > > > > 
> > > > > The NWD flag requests that the peripheral wait until the data has been fully
> > > > > processed before signaling an interrupt.
> > > > interrupt for transaction complete or DMA request?
> > > 
> > > This is a special signaling mechanism that holds off the DMA interrupt until the
> > > peripheral actually acks that the data has been processed completely.  This is
> > > required in many cases by the peripheral.  One example is the SPI controller.
> > > At the end of a transaction you are supposed to set the NWD so that the chip
> > > select is de-asserted.
> > 
> > I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK?  or maybe the
> > DMA_PREP_FENCE?  I don't generally like overloading the flags and slightly
> > twisting their intent.  Could we add a flag to denote device ACK?
> Nope lets not override these...

Then I need to add a flag.  Something like DMA_PREP_DEVICE_ACK that denotes that the
attached device needs to ACK the transfer.

Also, one thing I forgot.  For crypto and some of the other blocks, we have
something called command descriptors that can be transferred to the blocks to
provide programming or direction.  The DMA controller has a flag that tells the
attached peripheral that the incoming DMA contains command descriptors.  The dma
descriptors still point to a memory region to transfer, but the contents are
interpreted differently.  This is yet another flag in the descriptor flag
section.  I'd need another flag for this as well.  Something like DMA_PREP_CMD.

We are actively working on upstreaming our crypto block and this will be
required for that to work properly.

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Srinivas Kandagatla]

On 22/05/14 16:09, Andy Gross wrote:
> On Thu, May 22, 2014 at 11:40:49AM +0530, Vinod Koul wrote:
>
> <snip>
>
>>>> I have 3 different IRQs that can be asserted based on the bit I set in the
>>>> hardware descriptor.  The normal IRQ is the INT bit.  However, in some cases the
>>>> peripheral protocol requires the use of the EOT or EOB interrupt instead.  The
>>>> DMA_PREP_INTERRUPT would only work if I had only 2 choices.
>>>
>>> Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT
>>> flag.  However, I might get in a bind later if I need to support the EOB (end of
>>> block) interrupt.
>> Sorry for delay in this.
>>
>> I think it would make sense to use DMA_PREP_INTERRUPT for EOB interrupt. The EOT
>> should always be enabled for the cases where it is applicable instead of nomral
>> irq.
>>
>> That should genrically ocvery your cases, or did we miss anything here
>
> The EOT is not used for every transaction.  It is part of a handshaking
> protocol with the attached peripheral, much like the NWD (notify when done).  As
> near as I can tell today, no peripheral depends on the EOB, so we could drop it
> for now until it is needed and cross this bridge when we need to.

As EOT behaviour is totally dependent on the attached peripheral(or 
channel), Can't we make this specific to channel by passing additional 
flags in the DT dma channel descriptors? This will be better abstraction 
for drivers as well.

I know that EOT flag is part of descriptor but still some channels 
*must* have EOT to run there state-machine correctly. So making it 
optional for those channels might be wrong.

Are there any use cases for particular *channel* where EOT requirement 
changes dynamically?



--srini

>
>>>
>>>>
>>>>>
>>>>>>
>>>>>> The NWD flag requests that the peripheral wait until the data has been fully
>>>>>> processed before signaling an interrupt.
>>>>> interrupt for transaction complete or DMA request?
>>>>
>>>> This is a special signaling mechanism that holds off the DMA interrupt until the
>>>> peripheral actually acks that the data has been processed completely.  This is
>>>> required in many cases by the peripheral.  One example is the SPI controller.
>>>> At the end of a transaction you are supposed to set the NWD so that the chip
>>>> select is de-asserted.
>>>
>>> I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK?  or maybe the
>>> DMA_PREP_FENCE?  I don't generally like overloading the flags and slightly
>>> twisting their intent.  Could we add a flag to denote device ACK?
>> Nope lets not override these...
>
> Then I need to add a flag.  Something like DMA_PREP_DEVICE_ACK that denotes that the
> attached device needs to ACK the transfer.
>
> Also, one thing I forgot.  For crypto and some of the other blocks, we have
> something called command descriptors that can be transferred to the blocks to
> provide programming or direction.  The DMA controller has a flag that tells the
> attached peripheral that the incoming DMA contains command descriptors.  The dma
> descriptors still point to a memory region to transfer, but the contents are
> interpreted differently.  This is yet another flag in the descriptor flag
> section.  I'd need another flag for this as well.  Something like DMA_PREP_CMD.
>
> We are actively working on upstreaming our crypto block and this will be
> required for that to work properly.
>

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Andy Gross]

On Thu, May 22, 2014 at 04:27:05PM +0100, Srinivas Kandagatla wrote:

<snip>

> >
> >The EOT is not used for every transaction.  It is part of a handshaking
> >protocol with the attached peripheral, much like the NWD (notify when done).  As
> >near as I can tell today, no peripheral depends on the EOB, so we could drop it
> >for now until it is needed and cross this bridge when we need to.
> 
> As EOT behaviour is totally dependent on the attached peripheral(or
> channel), Can't we make this specific to channel by passing
> additional flags in the DT dma channel descriptors? This will be
> better abstraction for drivers as well.

Even for channels where you want to use EOT, you don't use it for every
transaction.  So a global channel flag isn't going to work.  This is the same
for NWD.  It is a per descriptor choice.

> 
> I know that EOT flag is part of descriptor but still some channels
> *must* have EOT to run there state-machine correctly. So making it
> optional for those channels might be wrong.
> 
> Are there any use cases for particular *channel* where EOT
> requirement changes dynamically?

I2C is one example.  You place EOT on the last transaction that makes up a
write/read transaction.  You may have multiple descriptors to send data, but the
last one has EOT.  And for read transactions, you place NWD on the last read
transaction.

<snip>

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] dmaengine: qcom_bam_dma: Add descriptor flag APIs

[Srinivas Kandagatla]

On 22/05/14 16:32, Andy Gross wrote:
> On Thu, May 22, 2014 at 04:27:05PM +0100, Srinivas Kandagatla wrote:
>
> <snip>
>
>>>
>>> The EOT is not used for every transaction.  It is part of a handshaking
>>> protocol with the attached peripheral, much like the NWD (notify when done).  As
>>> near as I can tell today, no peripheral depends on the EOB, so we could drop it
>>> for now until it is needed and cross this bridge when we need to.
>>
>> As EOT behaviour is totally dependent on the attached peripheral(or
>> channel), Can't we make this specific to channel by passing
>> additional flags in the DT dma channel descriptors? This will be
>> better abstraction for drivers as well.
>
> Even for channels where you want to use EOT, you don't use it for every
> transaction.  So a global channel flag isn't going to work.  This is the same
> for NWD.  It is a per descriptor choice.

Thanks Andy for explaining, I got it now.
>
>>
>> I know that EOT flag is part of descriptor but still some channels
>> *must* have EOT to run there state-machine correctly. So making it
>> optional for those channels might be wrong.
>>
>> Are there any use cases for particular *channel* where EOT
>> requirement changes dynamically?
>
> I2C is one example.  You place EOT on the last transaction that makes up a
> write/read transaction.  You may have multiple descriptors to send data, but the
> last one has EOT.  And for read transactions, you place NWD on the last read
> transaction.
>
> <snip>
>