[PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT.

[PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT.

[Zhang Wei]

The device->device_prep_dma_interrupt function is used by
DMA_INTERRUPT capability, not DMA_ZERO_SUM.

Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
---
 drivers/dma/dmaengine.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 2996523..8db0e7f 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -357,7 +357,7 @@ int dma_async_device_register(struct dma_device *device)
 		!device->device_prep_dma_zero_sum);
 	BUG_ON(dma_has_cap(DMA_MEMSET, device->cap_mask) &&
 		!device->device_prep_dma_memset);
-	BUG_ON(dma_has_cap(DMA_ZERO_SUM, device->cap_mask) &&
+	BUG_ON(dma_has_cap(DMA_INTERRUPT, device->cap_mask) &&
 		!device->device_prep_dma_interrupt);
 
 	BUG_ON(!device->device_alloc_chan_resources);
-- 
1.5.4

[PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

[Zhang Wei]

This is a bug that I assigned DMA_INTERRUPT capability to fsldma
but missing device_prep_dma_interrupt function. For a bug in
dmaengine.c the driver passed BUG_ON() checking. The patch fixes it.

Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
---
 drivers/dma/fsldma.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 5dfedf3..5428f81 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -406,6 +406,32 @@ static void fsl_dma_free_chan_resources(struct dma_chan *chan)
 	dma_pool_destroy(fsl_chan->desc_pool);
 }
 
+static struct dma_async_tx_descriptor *fsl_dma_prep_interrupt(
+						struct dma_chan *chan)
+{
+	struct fsl_dma_chan *fsl_chan;
+	struct fsl_desc_sw *new;
+
+	if (!chan)
+		return NULL;
+
+	fsl_chan = to_fsl_chan(chan);
+
+	new = fsl_dma_alloc_descriptor(fsl_chan);
+	if (!new) {
+		dev_err(fsl_chan->dev, "No free memory for link descriptor\n");
+		return NULL;
+	}
+
+	new->async_tx.cookie = -EBUSY;
+	new->async_tx.ack = 0;
+
+	/* Set End-of-link to the last link descriptor of new list*/
+	set_ld_eol(fsl_chan, new);
+
+	return &new->async_tx;
+}
+
 static struct dma_async_tx_descriptor *fsl_dma_prep_memcpy(
 	struct dma_chan *chan, dma_addr_t dma_dest, dma_addr_t dma_src,
 	size_t len, unsigned long flags)
@@ -1020,6 +1046,7 @@ static int __devinit of_fsl_dma_probe(struct of_device *dev,
 	dma_cap_set(DMA_INTERRUPT, fdev->common.cap_mask);
 	fdev->common.device_alloc_chan_resources = fsl_dma_alloc_chan_resources;
 	fdev->common.device_free_chan_resources = fsl_dma_free_chan_resources;
+	fdev->common.device_prep_dma_interrupt = fsl_dma_prep_interrupt;
 	fdev->common.device_prep_dma_memcpy = fsl_dma_prep_memcpy;
 	fdev->common.device_is_tx_complete = fsl_dma_is_complete;
 	fdev->common.device_issue_pending = fsl_dma_memcpy_issue_pending;
-- 
1.5.4

Re: [PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

[Dan Williams]

On Mon, Mar 10, 2008 at 8:25 PM, Zhang Wei <wei.zhang@freescale.com> wrote:
> This is a bug that I assigned DMA_INTERRUPT capability to fsldma
>  but missing device_prep_dma_interrupt function. For a bug in
>  dmaengine.c the driver passed BUG_ON() checking. The patch fixes it.
>
>  Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
>  ---
>   drivers/dma/fsldma.c |   27 +++++++++++++++++++++++++++
>   1 files changed, 27 insertions(+), 0 deletions(-)
>
>  diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
>  index 5dfedf3..5428f81 100644
>  --- a/drivers/dma/fsldma.c
>  +++ b/drivers/dma/fsldma.c
>  @@ -406,6 +406,32 @@ static void fsl_dma_free_chan_resources(struct dma_chan *chan)
>         dma_pool_destroy(fsl_chan->desc_pool);
>   }
>
>  +static struct dma_async_tx_descriptor *fsl_dma_prep_interrupt(
>  +                                               struct dma_chan *chan)
>  +{
>  +       struct fsl_dma_chan *fsl_chan;
>  +       struct fsl_desc_sw *new;
>  +
>  +       if (!chan)
>  +               return NULL;
>  +
>  +       fsl_chan = to_fsl_chan(chan);
>  +
>  +       new = fsl_dma_alloc_descriptor(fsl_chan);
>  +       if (!new) {
>  +               dev_err(fsl_chan->dev, "No free memory for link descriptor\n");
>  +               return NULL;
>  +       }
>  +
>  +       new->async_tx.cookie = -EBUSY;
>  +       new->async_tx.ack = 0;
>  +
>  +       /* Set End-of-link to the last link descriptor of new list*/
>  +       set_ld_eol(fsl_chan, new);

Question, is 'set_ld_eol' safe to call on descriptors that may not be
the last in the list?  For example what about the following sequence:

/* prepare two descriptors */
tx1 = fsl_dma_prep_interrupt(chan);
tx2 = fsl_dma_prep_memcpy(chan);

/* submit out of order */
tx2->tx_submit(tx2);
tx1->tx_submit(tx1);

This is only a concern if you plan to support channel switching at
some point.  For example, switching from a memcpy channel to an xor
channel.

Thanks,
Dan

RE: [PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

[Zhang Wei]

Hi, Dan, 

> -----Original Message-----
> From: dan.j.williams@gmail.com 
> >  +
> >  +       /* Set End-of-link to the last link descriptor of 
> new list*/
> >  +       set_ld_eol(fsl_chan, new);
> 
> Question, is 'set_ld_eol' safe to call on descriptors that may not be
> the last in the list?  For example what about the following sequence:

set_ld_eol() is a safe function, which is only for preparing the tx descriptors
lists. When adding prepared tx descriptior list to DMA channel tx list,
the function append_ld_queue() should be called.

> 
> /* prepare two descriptors */
> tx1 = fsl_dma_prep_interrupt(chan);
> tx2 = fsl_dma_prep_memcpy(chan);
> 
> /* submit out of order */
> tx2->tx_submit(tx2);
> tx1->tx_submit(tx1);
> 
> This is only a concern if you plan to support channel switching at
> some point.  For example, switching from a memcpy channel to an xor
> channel.
> 
It's no problem. :) In fact, I've added out of order testing codes in fsl_dma_self_test()
 function.

But I have a question about device_prep_dma_interrupt(), which is no way to assign
 dest and src address. Is it a null tx action dma_async_tx_descriptor except to trigger
 an interrupt?

Thanks!
Wei.

Re: [PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

[Dan Williams]

On Tue, Mar 11, 2008 at 7:28 PM, Zhang Wei <Wei.Zhang@freescale.com> wrote:
[..]
>  But I have a question about device_prep_dma_interrupt(), which is no way to assign
>   dest and src address. Is it a null tx action dma_async_tx_descriptor except to trigger
>   an interrupt?
>

Yes, it is for sequences where we are scheduling a chain of operations
of indeterminate length and want a callback after they all complete.
See ops_run_biofill() in drivers/md/raid5.c for an example.

>  Thanks!
>  Wei.

--
Dan

RE: [PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT.

[Sosnowski, Maciej]

> ---------- Original message ----------
> From: Zhang Wei <wei.zhang@freescale.com>
> Date: Mar 11, 2008 4:25 AM
> Subject: [PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability
> DMA_INTERRUPT.
> To: dan.j.williams@intel.com
> Cc: linux-kernel@vger.kernel.org, Zhang Wei <wei.zhang@freescale.com>
> 
> 
> The device->device_prep_dma_interrupt function is used by
> DMA_INTERRUPT capability, not DMA_ZERO_SUM.
> 
> Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
> ---
>  drivers/dma/dmaengine.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 2996523..8db0e7f 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -357,7 +357,7 @@ int dma_async_device_register(struct dma_device
*device)
>                !device->device_prep_dma_zero_sum);
>        BUG_ON(dma_has_cap(DMA_MEMSET, device->cap_mask) &&
>                !device->device_prep_dma_memset);
> -       BUG_ON(dma_has_cap(DMA_ZERO_SUM, device->cap_mask) &&
> +       BUG_ON(dma_has_cap(DMA_INTERRUPT, device->cap_mask) &&
>                !device->device_prep_dma_interrupt);
> 
>        BUG_ON(!device->device_alloc_chan_resources);
> --
> 1.5.4

Acked-by: Maciej Sosnowski <maciej.sosnowski@intel.com>
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.