[PATCH 0/6] A few fixes on imx-sdma

[PATCH 0/6] A few fixes on imx-sdma

[Shawn Guo]

The patch set is to address the concerns discussed on thread below.
And it's been tested on mx51 babbage board with sound driver.

"A few questions on imx-sdma"
http://article.gmane.org/gmane.linux.ports.arm.kernel/103001/

Shawn Guo (6)
 drivers/dma/imx-sdma.c |   31 +++++++++++++++++++------------
 1 files changed, 19 insertions(+), 12 deletions(-)

 [PATCH 1/6] dmaengine: imx-sdma: remove IMX_DMA_SG_LOOP handling in sdma_prep_slave_sg()
 [PATCH 2/6] dmaengine: imx-sdma: set sdmac->status to DMA_ERROR in err_out of sdma_prep_slave_sg()
 [PATCH 3/6] dmaengine: imx-sdma: return sdmac->status in sdma_tx_status()
 [PATCH 4/6] dmaengine: imxs-sdma: correct sdmac->status in sdma_handle_channel_loop()
 [PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()
 [PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

Regards,
Shawn

[PATCH 1/6] dmaengine: imx-sdma: remove IMX_DMA_SG_LOOP handling in sdma_prep_slave_sg()

[Shawn Guo]

This is a leftover from the time that the driver did not have
sdma_prep_dma_cyclic callback and implemented sound dma as a looped
sg chain.  And it can be removed now.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/dma/imx-sdma.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 5456480..45413b7 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -931,12 +931,6 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 
 		param = BD_DONE | BD_EXTD | BD_CONT;
 
-		if (sdmac->flags & IMX_DMA_SG_LOOP) {
-			param |= BD_INTR;
-			if (i + 1 == sg_len)
-				param |= BD_WRAP;
-		}
-
 		if (i + 1 == sg_len)
 			param |= BD_INTR;
 
-- 
1.7.1

[PATCH 2/6] dmaengine: imx-sdma: set sdmac->status to DMA_ERROR in err_out of sdma_prep_slave_sg()

[Shawn Guo]

sdma_prep_dma_cyclic() sets sdmac->status to DMA_ERROR in err_out,
and sdma_prep_slave_sg() needs to do the same.  Otherwise,
sdmac->status stays at DMA_IN_PROGRESS, which will make the function
return immediately next time it gets called.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/dma/imx-sdma.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 45413b7..2aa5131 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -947,6 +947,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 
 	return &sdmac->desc;
 err_out:
+	sdmac->status = DMA_ERROR;
 	return NULL;
 }
 
-- 
1.7.1

[PATCH 3/6] dmaengine: imx-sdma: return sdmac->status in sdma_tx_status()

[Shawn Guo]

The sdmac->status was designed to reflect the status of the tx,
so simply return it in sdma_tx_status().  Then dma client can call
dma_async_is_tx_complete() to know the status of the tx.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/dma/imx-sdma.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 2aa5131..ccdafa4 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1061,14 +1061,12 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
 {
 	struct sdma_channel *sdmac = to_sdma_chan(chan);
 	dma_cookie_t last_used;
-	enum dma_status ret;
 
 	last_used = chan->cookie;
 
-	ret = dma_async_is_complete(cookie, sdmac->last_completed, last_used);
 	dma_set_tx_state(txstate, sdmac->last_completed, last_used, 0);
 
-	return ret;
+	return sdmac->status;
 }
 
 static void sdma_issue_pending(struct dma_chan *chan)
-- 
1.7.1

[PATCH 4/6] dmaengine: imxs-sdma: correct sdmac->status in sdma_handle_channel_loop()

[Shawn Guo]

sdma_handle_channel_loop() is the handler of cyclic tx.  One period
success does not really mean the success of the tx.  Instead of
DMA_SUCCESS, DMA_IN_PROGRESS should be the one to tell.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/dma/imx-sdma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index ccdafa4..e5af547 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -449,7 +449,7 @@ static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
 		if (bd->mode.status & BD_RROR)
 			sdmac->status = DMA_ERROR;
 		else
-			sdmac->status = DMA_SUCCESS;
+			sdmac->status = DMA_IN_PROGRESS;
 
 		bd->mode.status |= BD_DONE;
 		sdmac->buf_tail++;
-- 
1.7.1

[PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

[Shawn Guo]

As per the reference manual, bit "L" should be set while bit "C"
should be cleared for the last buffer descriptor in the non-cyclic
chain, so that sdma can stop trying to find the next BD and end
the transfer.

In case of sdma_prep_slave_sg(), BD_LAST needs to be set and BD_CONT
be cleared for the last BD.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/dma/imx-sdma.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e5af547..fa63ace 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -931,8 +931,11 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 
 		param = BD_DONE | BD_EXTD | BD_CONT;
 
-		if (i + 1 == sg_len)
+		if (i + 1 == sg_len) {
 			param |= BD_INTR;
+			param |= BD_LAST;
+			param &= ~BD_CONT;
+		}
 
 		dev_dbg(sdma->dev, "entry %d: count: %d dma: 0x%08x %s%s\n",
 				i, count, sg->dma_address,
-- 
1.7.1

[PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

[Shawn Guo]

When STOP register bit gets set, SDMA hardware will immediately stop
the channel once the current burst other than buffer descriptor
transfer is done.

* Change sdma_disable_channel() to only set STOP register bit after
  polling the current BD done, so that the current BD transfer
  corruption could be avoided.

* Increment buf_tail in mxc_sdma_handle_channel_normal() as well as
  sdma_handle_channel_loop(), since buf_tail now is used in
  sdma_disable_channel() which could be called in both sg and cyclic
  cases.

* Return DMA_SUCCESS other than DMA_ERROR in sdma_disable_channel().

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
 drivers/dma/imx-sdma.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index fa63ace..ae87287 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -481,6 +481,8 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
 	else
 		sdmac->status = DMA_SUCCESS;
 
+	sdmac->buf_tail++;
+
 	if (sdmac->desc.callback)
 		sdmac->desc.callback(sdmac->desc.callback_param);
 	sdmac->last_completed = sdmac->desc.cookie;
@@ -655,9 +657,14 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
 {
 	struct sdma_engine *sdma = sdmac->sdma;
 	int channel = sdmac->channel;
+	struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
+
+	while (bd->mode.status & BD_DONE)
+		cpu_relax();
 
 	__raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
-	sdmac->status = DMA_ERROR;
+
+	sdmac->status = DMA_SUCCESS;
 }
 
 static int sdma_config_channel(struct sdma_channel *sdmac)
-- 
1.7.1

[PATCH 0/6] A few fixes on imx-sdma

[Shawn Guo]

On Thu, Jan 20, 2011 at 05:50:34AM +0800, Shawn Guo wrote:
> The patch set is to address the concerns discussed on thread below.
> And it's been tested on mx51 babbage board with sound driver.
> 
> "A few questions on imx-sdma"
> http://article.gmane.org/gmane.linux.ports.arm.kernel/103001/
> 
> Shawn Guo (6)
>  drivers/dma/imx-sdma.c |   31 +++++++++++++++++++------------
>  1 files changed, 19 insertions(+), 12 deletions(-)
> 
>  [PATCH 1/6] dmaengine: imx-sdma: remove IMX_DMA_SG_LOOP handling in sdma_prep_slave_sg()
>  [PATCH 2/6] dmaengine: imx-sdma: set sdmac->status to DMA_ERROR in err_out of sdma_prep_slave_sg()
>  [PATCH 3/6] dmaengine: imx-sdma: return sdmac->status in sdma_tx_status()
>  [PATCH 4/6] dmaengine: imxs-sdma: correct sdmac->status in sdma_handle_channel_loop()
			  ^^^^
This will be fixed in v2.

>  [PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()
>  [PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer
> 

-- 
Regards,
Shawn

[PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

[Sascha Hauer]

On Thu, Jan 20, 2011 at 06:43:13PM +0800, Shawn Guo wrote:
> On Thu, Jan 20, 2011 at 05:50:40AM +0800, Shawn Guo wrote:
> > When STOP register bit gets set, SDMA hardware will immediately stop
> > the channel once the current burst other than buffer descriptor
> > transfer is done.
> > 
> > * Change sdma_disable_channel() to only set STOP register bit after
> >   polling the current BD done, so that the current BD transfer
> >   corruption could be avoided.
> > 
> > * Increment buf_tail in mxc_sdma_handle_channel_normal() as well as
> >   sdma_handle_channel_loop(), since buf_tail now is used in
> >   sdma_disable_channel() which could be called in both sg and cyclic
> >   cases.
> > 
> > * Return DMA_SUCCESS other than DMA_ERROR in sdma_disable_channel().
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> >  drivers/dma/imx-sdma.c |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index fa63ace..ae87287 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -481,6 +481,8 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
> >  	else
> >  		sdmac->status = DMA_SUCCESS;
> >  
> > +	sdmac->buf_tail++;
> > +
> >  	if (sdmac->desc.callback)
> >  		sdmac->desc.callback(sdmac->desc.callback_param);
> >  	sdmac->last_completed = sdmac->desc.cookie;
> > @@ -655,9 +657,14 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> >  {
> >  	struct sdma_engine *sdma = sdmac->sdma;
> >  	int channel = sdmac->channel;
> > +	struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > +
> > +	while (bd->mode.status & BD_DONE)
> > +		cpu_relax();
> >  
> >  	__raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > -	sdmac->status = DMA_ERROR;
> > +
> > +	sdmac->status = DMA_SUCCESS;
> >  }
> 
> Sorry. The patch lost the change as below.  Will pick it up in v2.
> 
> @@ -658,11 +658,15 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
>         struct sdma_engine *sdma = sdmac->sdma;
>         int channel = sdmac->channel;
>         struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> +       u32 stat = __raw_readl(sdma->regs + SDMA_H_STATSTOP);
> 
> -       while (bd->mode.status & BD_DONE)
> -               cpu_relax();
> +       /* stop the channel if it's running */
> +       if (stat & (1 << channel)) {
> +               while (bd->mode.status & BD_DONE)
> +                       cpu_relax();
> 
> -       __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> +               __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> +       }
> 
>         sdmac->status = DMA_SUCCESS;

NAK.

The patches has several problems.

- buf_tail is only used for cyclic transfers, so in case of non cyclic
  transfers you poll for an arbitrary descriptor being ready.

- Even in cyclic transfers when buf_tail points to the correct
  descriptor the hardware will immediatly start the next descriptor
  before you have a chance to set the STATSTOP bit. So you probably
  will corrupt the next descriptor instead of the current one
  which makes this patch useless.

- buf_tail is increased in the interrupt handler, so if you
  happen to disable the channel after the bd is done but before
  the interrupt handler has increased buf_tail you poll for the
  wrong bd being ready which again makes this patch useless.

If in non cyclic transfers we want to disable a channel we have our
reasons (device error or timeout) and then the data is corrupted anyway,
so no reason to poll for a descriptor getting done. Even worse, in case
of an device error the descriptor might not get ready at all and
we will poll for ever in the loop above.
Cyclic transfers are designed for audio and disabling a channel
basically means pausing the stream. When the SDMA engine does not
support pausing the stream in hardware, we have to live with the
fact that we pick up the stream on the next descriptor resulting
in some bytes missing in the resulting audio stream.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

[Sascha Hauer]

On Thu, Jan 20, 2011 at 05:50:39AM +0800, Shawn Guo wrote:
> As per the reference manual, bit "L" should be set while bit "C"
> should be cleared for the last buffer descriptor in the non-cyclic
> chain, so that sdma can stop trying to find the next BD and end
> the transfer.
> 
> In case of sdma_prep_slave_sg(), BD_LAST needs to be set and BD_CONT
> be cleared for the last BD.

Did you test this patch?

Sascha

> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
>  drivers/dma/imx-sdma.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index e5af547..fa63ace 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -931,8 +931,11 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
>  
>  		param = BD_DONE | BD_EXTD | BD_CONT;
>  
> -		if (i + 1 == sg_len)
> +		if (i + 1 == sg_len) {
>  			param |= BD_INTR;
> +			param |= BD_LAST;
> +			param &= ~BD_CONT;
> +		}
>  
>  		dev_dbg(sdma->dev, "entry %d: count: %d dma: 0x%08x %s%s\n",
>  				i, count, sg->dma_address,
> -- 
> 1.7.1
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

[Shawn Guo]

On Thu, Jan 20, 2011 at 05:50:40AM +0800, Shawn Guo wrote:
> When STOP register bit gets set, SDMA hardware will immediately stop
> the channel once the current burst other than buffer descriptor
> transfer is done.
> 
> * Change sdma_disable_channel() to only set STOP register bit after
>   polling the current BD done, so that the current BD transfer
>   corruption could be avoided.
> 
> * Increment buf_tail in mxc_sdma_handle_channel_normal() as well as
>   sdma_handle_channel_loop(), since buf_tail now is used in
>   sdma_disable_channel() which could be called in both sg and cyclic
>   cases.
> 
> * Return DMA_SUCCESS other than DMA_ERROR in sdma_disable_channel().
> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
>  drivers/dma/imx-sdma.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index fa63ace..ae87287 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -481,6 +481,8 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
>  	else
>  		sdmac->status = DMA_SUCCESS;
>  
> +	sdmac->buf_tail++;
> +
>  	if (sdmac->desc.callback)
>  		sdmac->desc.callback(sdmac->desc.callback_param);
>  	sdmac->last_completed = sdmac->desc.cookie;
> @@ -655,9 +657,14 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
>  {
>  	struct sdma_engine *sdma = sdmac->sdma;
>  	int channel = sdmac->channel;
> +	struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> +
> +	while (bd->mode.status & BD_DONE)
> +		cpu_relax();
>  
>  	__raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> -	sdmac->status = DMA_ERROR;
> +
> +	sdmac->status = DMA_SUCCESS;
>  }

Sorry. The patch lost the change as below.  Will pick it up in v2.

@@ -658,11 +658,15 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
        struct sdma_engine *sdma = sdmac->sdma;
        int channel = sdmac->channel;
        struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
+       u32 stat = __raw_readl(sdma->regs + SDMA_H_STATSTOP);

-       while (bd->mode.status & BD_DONE)
-               cpu_relax();
+       /* stop the channel if it's running */
+       if (stat & (1 << channel)) {
+               while (bd->mode.status & BD_DONE)
+                       cpu_relax();

-       __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
+               __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
+       }

        sdmac->status = DMA_SUCCESS;
 }

>  
>  static int sdma_config_channel(struct sdma_channel *sdmac)
> -- 
> 1.7.1
> 

-- 
Regards,
Shawn

[PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

[Sascha Hauer]

On Fri, Jan 21, 2011 at 11:55:19PM +0800, Shawn Guo wrote:
> Hi Sascha,
> 
> On Thu, Jan 20, 2011 at 11:27:01AM +0100, Sascha Hauer wrote:
> > On Thu, Jan 20, 2011 at 06:43:13PM +0800, Shawn Guo wrote:
> > > On Thu, Jan 20, 2011 at 05:50:40AM +0800, Shawn Guo wrote:
> > > > When STOP register bit gets set, SDMA hardware will immediately stop
> > > > the channel once the current burst other than buffer descriptor
> > > > transfer is done.
> > > > 
> > > > * Change sdma_disable_channel() to only set STOP register bit after
> > > >   polling the current BD done, so that the current BD transfer
> > > >   corruption could be avoided.
> > > > 
> > > > * Increment buf_tail in mxc_sdma_handle_channel_normal() as well as
> > > >   sdma_handle_channel_loop(), since buf_tail now is used in
> > > >   sdma_disable_channel() which could be called in both sg and cyclic
> > > >   cases.
> 
> As the commit message tells, buf_tail is now handled by non-cyclic too.
> 
> > > > 
> > > > * Return DMA_SUCCESS other than DMA_ERROR in sdma_disable_channel().
> > > > 
> > > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > > ---
> > > >  drivers/dma/imx-sdma.c |    9 ++++++++-
> > > >  1 files changed, 8 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > > > index fa63ace..ae87287 100644
> > > > --- a/drivers/dma/imx-sdma.c
> > > > +++ b/drivers/dma/imx-sdma.c
> > > > @@ -481,6 +481,8 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
> > > >  	else
> > > >  		sdmac->status = DMA_SUCCESS;
> > > >  
> > > > +	sdmac->buf_tail++;
> > > > +
> 
> ditto
> 
> > > >  	if (sdmac->desc.callback)
> > > >  		sdmac->desc.callback(sdmac->desc.callback_param);
> > > >  	sdmac->last_completed = sdmac->desc.cookie;
> > > > @@ -655,9 +657,14 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> > > >  {
> > > >  	struct sdma_engine *sdma = sdmac->sdma;
> > > >  	int channel = sdmac->channel;
> > > > +	struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > > > +
> > > > +	while (bd->mode.status & BD_DONE)
> > > > +		cpu_relax();
> > > >  
> > > >  	__raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > > -	sdmac->status = DMA_ERROR;
> > > > +
> > > > +	sdmac->status = DMA_SUCCESS;
> > > >  }
> > > 
> > > Sorry. The patch lost the change as below.  Will pick it up in v2.
> > > 
> > > @@ -658,11 +658,15 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> > >         struct sdma_engine *sdma = sdmac->sdma;
> > >         int channel = sdmac->channel;
> > >         struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > > +       u32 stat = __raw_readl(sdma->regs + SDMA_H_STATSTOP);
> > > 
> > > -       while (bd->mode.status & BD_DONE)
> > > -               cpu_relax();
> > > +       /* stop the channel if it's running */
> > > +       if (stat & (1 << channel)) {
> > > +               while (bd->mode.status & BD_DONE)
> > > +                       cpu_relax();
> > > 
> > > -       __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > +               __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > +       }
> > > 
> > >         sdmac->status = DMA_SUCCESS;
> > 
> > NAK.
> > 
> > The patches has several problems.
> > 
> > - buf_tail is only used for cyclic transfers, so in case of non cyclic
> >   transfers you poll for an arbitrary descriptor being ready.
> > 
> See above.
> 
> > - Even in cyclic transfers when buf_tail points to the correct
> >   descriptor the hardware will immediatly start the next descriptor
> >   before you have a chance to set the STATSTOP bit. So you probably
> >   will corrupt the next descriptor instead of the current one
> >   which makes this patch useless.
> > 
> You are right that the buf_tail will not stop immediately until next
> time it gets looped on.  But in any case, polling BD_DONE will not
> corrupt any descriptor.
> 
> > - buf_tail is increased in the interrupt handler, so if you
> >   happen to disable the channel after the bd is done but before
> >   the interrupt handler has increased buf_tail you poll for the
> >   wrong bd being ready which again makes this patch useless.
> > 
> > If in non cyclic transfers we want to disable a channel we have our
> > reasons (device error or timeout) and then the data is corrupted anyway,
> > so no reason to poll for a descriptor getting done. Even worse, in case
> > of an device error the descriptor might not get ready at all and
> Hmm, we should polling (BD_DONE | BD_ERROR).
> 
> > we will poll for ever in the loop above.
> > Cyclic transfers are designed for audio and disabling a channel
> > basically means pausing the stream. When the SDMA engine does not
> I thought pausing channel is different from disabling channel, or we
> do not need DMA_PAUSE and DMA_TERMINATE_ALL for dma_ctrl_cmd.

We currently do not handle DMA_PAUSE and in case of DMA_TERMINATE_ALL we
do not care about the data anyway. So just disabling the channel is the
best we can do.

> 
> > support pausing the stream in hardware, we have to live with the
> What about on the fly setting bit "L" of the next descriptor that's
> not been running?  So that SDMA can stop the channel when it gets
> this descriptor done.

I think that would be possible, but this should be implemented as DMA_PAUSE
and not as DMA_TERMINATE_ALL.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

[Sascha Hauer]

On Fri, Jan 21, 2011 at 11:15:39PM +0800, Shawn Guo wrote:
> Hi Sascha,
> 
> On Thu, Jan 20, 2011 at 11:28:08AM +0100, Sascha Hauer wrote:
> > On Thu, Jan 20, 2011 at 05:50:39AM +0800, Shawn Guo wrote:
> > > As per the reference manual, bit "L" should be set while bit "C"
> > > should be cleared for the last buffer descriptor in the non-cyclic
> > > chain, so that sdma can stop trying to find the next BD and end
> > > the transfer.
> > > 
> > > In case of sdma_prep_slave_sg(), BD_LAST needs to be set and BD_CONT
> > > be cleared for the last BD.
> > 
> > Did you test this patch?
> > 
> Would you suggest any driver on mx51 using this api?  So that I can
> test it on my babbage board.  Or you can help test it with mx27/31
> mmc driver?  I do not have them on my end.  Thanks.

I can only suggest the imx/imx-sdma-sound-mx353ds-mx51_babbage
branch, but this only uses cyclic transfers so you can't use it
to test this patch.
I asked because I could add an ack to this patch if it has been
tested. Without testing I'd like delaying it until I found some
time testing it.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

[Shawn Guo]

Hi Sascha,

On Fri, Jan 21, 2011 at 11:24:24AM +0100, Sascha Hauer wrote:
> On Fri, Jan 21, 2011 at 11:15:39PM +0800, Shawn Guo wrote:
> > Hi Sascha,
> > 
> > On Thu, Jan 20, 2011 at 11:28:08AM +0100, Sascha Hauer wrote:
> > > On Thu, Jan 20, 2011 at 05:50:39AM +0800, Shawn Guo wrote:
> > > > As per the reference manual, bit "L" should be set while bit "C"
> > > > should be cleared for the last buffer descriptor in the non-cyclic
> > > > chain, so that sdma can stop trying to find the next BD and end
> > > > the transfer.
> > > > 
> > > > In case of sdma_prep_slave_sg(), BD_LAST needs to be set and BD_CONT
> > > > be cleared for the last BD.
> > > 
> > > Did you test this patch?
> > > 
> > Would you suggest any driver on mx51 using this api?  So that I can
> > test it on my babbage board.  Or you can help test it with mx27/31
> > mmc driver?  I do not have them on my end.  Thanks.
> 
> I can only suggest the imx/imx-sdma-sound-mx353ds-mx51_babbage
> branch, but this only uses cyclic transfers so you can't use it
> to test this patch.
Yes, I tested the patch set against imx/imx-sdma-sound-mx353ds-mx51_babbage.
But the only use of sdma_prep_slave_sg I know is mxcmmc.c that you
patched recently.  And this driver is for mx27/31 which I do not have
the setup.

> I asked because I could add an ack to this patch if it has been
> tested. Without testing I'd like delaying it until I found some
> time testing it.
Reasonable.  Thanks.

-- 
Regards,
Shawn

[PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

[Shawn Guo]

Hi Sascha,

On Fri, Jan 21, 2011 at 10:45:40AM +0100, Sascha Hauer wrote:
> On Fri, Jan 21, 2011 at 11:55:19PM +0800, Shawn Guo wrote:
> > Hi Sascha,
> > 
> > On Thu, Jan 20, 2011 at 11:27:01AM +0100, Sascha Hauer wrote:
> > > On Thu, Jan 20, 2011 at 06:43:13PM +0800, Shawn Guo wrote:
> > > > On Thu, Jan 20, 2011 at 05:50:40AM +0800, Shawn Guo wrote:
> > > > > When STOP register bit gets set, SDMA hardware will immediately stop
> > > > > the channel once the current burst other than buffer descriptor
> > > > > transfer is done.
> > > > > 
> > > > > * Change sdma_disable_channel() to only set STOP register bit after
> > > > >   polling the current BD done, so that the current BD transfer
> > > > >   corruption could be avoided.
> > > > > 
> > > > > * Increment buf_tail in mxc_sdma_handle_channel_normal() as well as
> > > > >   sdma_handle_channel_loop(), since buf_tail now is used in
> > > > >   sdma_disable_channel() which could be called in both sg and cyclic
> > > > >   cases.
> > 
> > As the commit message tells, buf_tail is now handled by non-cyclic too.
> > 
> > > > > 
> > > > > * Return DMA_SUCCESS other than DMA_ERROR in sdma_disable_channel().
> > > > > 
> > > > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > > > ---
> > > > >  drivers/dma/imx-sdma.c |    9 ++++++++-
> > > > >  1 files changed, 8 insertions(+), 1 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > > > > index fa63ace..ae87287 100644
> > > > > --- a/drivers/dma/imx-sdma.c
> > > > > +++ b/drivers/dma/imx-sdma.c
> > > > > @@ -481,6 +481,8 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
> > > > >  	else
> > > > >  		sdmac->status = DMA_SUCCESS;
> > > > >  
> > > > > +	sdmac->buf_tail++;
> > > > > +
> > 
> > ditto
> > 
> > > > >  	if (sdmac->desc.callback)
> > > > >  		sdmac->desc.callback(sdmac->desc.callback_param);
> > > > >  	sdmac->last_completed = sdmac->desc.cookie;
> > > > > @@ -655,9 +657,14 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> > > > >  {
> > > > >  	struct sdma_engine *sdma = sdmac->sdma;
> > > > >  	int channel = sdmac->channel;
> > > > > +	struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > > > > +
> > > > > +	while (bd->mode.status & BD_DONE)
> > > > > +		cpu_relax();
> > > > >  
> > > > >  	__raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > > > -	sdmac->status = DMA_ERROR;
> > > > > +
> > > > > +	sdmac->status = DMA_SUCCESS;
> > > > >  }
> > > > 
> > > > Sorry. The patch lost the change as below.  Will pick it up in v2.
> > > > 
> > > > @@ -658,11 +658,15 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> > > >         struct sdma_engine *sdma = sdmac->sdma;
> > > >         int channel = sdmac->channel;
> > > >         struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > > > +       u32 stat = __raw_readl(sdma->regs + SDMA_H_STATSTOP);
> > > > 
> > > > -       while (bd->mode.status & BD_DONE)
> > > > -               cpu_relax();
> > > > +       /* stop the channel if it's running */
> > > > +       if (stat & (1 << channel)) {
> > > > +               while (bd->mode.status & BD_DONE)
> > > > +                       cpu_relax();
> > > > 
> > > > -       __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > > +               __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > > +       }
> > > > 
> > > >         sdmac->status = DMA_SUCCESS;
> > > 
> > > NAK.
> > > 
> > > The patches has several problems.
> > > 
> > > - buf_tail is only used for cyclic transfers, so in case of non cyclic
> > >   transfers you poll for an arbitrary descriptor being ready.
> > > 
> > See above.
> > 
> > > - Even in cyclic transfers when buf_tail points to the correct
> > >   descriptor the hardware will immediatly start the next descriptor
> > >   before you have a chance to set the STATSTOP bit. So you probably
> > >   will corrupt the next descriptor instead of the current one
> > >   which makes this patch useless.
> > > 
> > You are right that the buf_tail will not stop immediately until next
> > time it gets looped on.  But in any case, polling BD_DONE will not
> > corrupt any descriptor.
> > 
> > > - buf_tail is increased in the interrupt handler, so if you
> > >   happen to disable the channel after the bd is done but before
> > >   the interrupt handler has increased buf_tail you poll for the
> > >   wrong bd being ready which again makes this patch useless.
> > > 
> > > If in non cyclic transfers we want to disable a channel we have our
> > > reasons (device error or timeout) and then the data is corrupted anyway,
> > > so no reason to poll for a descriptor getting done. Even worse, in case
> > > of an device error the descriptor might not get ready at all and
> > Hmm, we should polling (BD_DONE | BD_ERROR).
> > 
> > > we will poll for ever in the loop above.
> > > Cyclic transfers are designed for audio and disabling a channel
> > > basically means pausing the stream. When the SDMA engine does not
> > I thought pausing channel is different from disabling channel, or we
> > do not need DMA_PAUSE and DMA_TERMINATE_ALL for dma_ctrl_cmd.
> 
> We currently do not handle DMA_PAUSE and in case of DMA_TERMINATE_ALL we
> do not care about the data anyway. So just disabling the channel is the
> best we can do.
> 
OK.  We can drop this patch ...

> > 
> > > support pausing the stream in hardware, we have to live with the
> > What about on the fly setting bit "L" of the next descriptor that's
> > not been running?  So that SDMA can stop the channel when it gets
> > this descriptor done.
> 
> I think that would be possible, but this should be implemented as DMA_PAUSE
> and not as DMA_TERMINATE_ALL.
and I will try to add DMA_PAUSE support with a new patch later.

-- 
Regards,
Shawn

[PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

[Shawn Guo]

Hi Sascha,

On Thu, Jan 20, 2011 at 11:28:08AM +0100, Sascha Hauer wrote:
> On Thu, Jan 20, 2011 at 05:50:39AM +0800, Shawn Guo wrote:
> > As per the reference manual, bit "L" should be set while bit "C"
> > should be cleared for the last buffer descriptor in the non-cyclic
> > chain, so that sdma can stop trying to find the next BD and end
> > the transfer.
> > 
> > In case of sdma_prep_slave_sg(), BD_LAST needs to be set and BD_CONT
> > be cleared for the last BD.
> 
> Did you test this patch?
> 
Would you suggest any driver on mx51 using this api?  So that I can
test it on my babbage board.  Or you can help test it with mx27/31
mmc driver?  I do not have them on my end.  Thanks.

-- 
Regards,
Shawn
> 
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> >  drivers/dma/imx-sdma.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index e5af547..fa63ace 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -931,8 +931,11 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
> >  
> >  		param = BD_DONE | BD_EXTD | BD_CONT;
> >  
> > -		if (i + 1 == sg_len)
> > +		if (i + 1 == sg_len) {
> >  			param |= BD_INTR;
> > +			param |= BD_LAST;
> > +			param &= ~BD_CONT;
> > +		}
> >  
> >  		dev_dbg(sdma->dev, "entry %d: count: %d dma: 0x%08x %s%s\n",
> >  				i, count, sg->dma_address,
> > -- 
> > 1.7.1
> > 
> > 
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

[PATCH 6/6] dmaengine: imx-sdma: avoid stopping channel in the middle of a BD transfer

[Shawn Guo]

Hi Sascha,

On Thu, Jan 20, 2011 at 11:27:01AM +0100, Sascha Hauer wrote:
> On Thu, Jan 20, 2011 at 06:43:13PM +0800, Shawn Guo wrote:
> > On Thu, Jan 20, 2011 at 05:50:40AM +0800, Shawn Guo wrote:
> > > When STOP register bit gets set, SDMA hardware will immediately stop
> > > the channel once the current burst other than buffer descriptor
> > > transfer is done.
> > > 
> > > * Change sdma_disable_channel() to only set STOP register bit after
> > >   polling the current BD done, so that the current BD transfer
> > >   corruption could be avoided.
> > > 
> > > * Increment buf_tail in mxc_sdma_handle_channel_normal() as well as
> > >   sdma_handle_channel_loop(), since buf_tail now is used in
> > >   sdma_disable_channel() which could be called in both sg and cyclic
> > >   cases.

As the commit message tells, buf_tail is now handled by non-cyclic too.

> > > 
> > > * Return DMA_SUCCESS other than DMA_ERROR in sdma_disable_channel().
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > ---
> > >  drivers/dma/imx-sdma.c |    9 ++++++++-
> > >  1 files changed, 8 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > > index fa63ace..ae87287 100644
> > > --- a/drivers/dma/imx-sdma.c
> > > +++ b/drivers/dma/imx-sdma.c
> > > @@ -481,6 +481,8 @@ static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
> > >  	else
> > >  		sdmac->status = DMA_SUCCESS;
> > >  
> > > +	sdmac->buf_tail++;
> > > +

ditto

> > >  	if (sdmac->desc.callback)
> > >  		sdmac->desc.callback(sdmac->desc.callback_param);
> > >  	sdmac->last_completed = sdmac->desc.cookie;
> > > @@ -655,9 +657,14 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> > >  {
> > >  	struct sdma_engine *sdma = sdmac->sdma;
> > >  	int channel = sdmac->channel;
> > > +	struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > > +
> > > +	while (bd->mode.status & BD_DONE)
> > > +		cpu_relax();
> > >  
> > >  	__raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > > -	sdmac->status = DMA_ERROR;
> > > +
> > > +	sdmac->status = DMA_SUCCESS;
> > >  }
> > 
> > Sorry. The patch lost the change as below.  Will pick it up in v2.
> > 
> > @@ -658,11 +658,15 @@ static void sdma_disable_channel(struct sdma_channel *sdmac)
> >         struct sdma_engine *sdma = sdmac->sdma;
> >         int channel = sdmac->channel;
> >         struct sdma_buffer_descriptor *bd = &sdmac->bd[sdmac->buf_tail];
> > +       u32 stat = __raw_readl(sdma->regs + SDMA_H_STATSTOP);
> > 
> > -       while (bd->mode.status & BD_DONE)
> > -               cpu_relax();
> > +       /* stop the channel if it's running */
> > +       if (stat & (1 << channel)) {
> > +               while (bd->mode.status & BD_DONE)
> > +                       cpu_relax();
> > 
> > -       __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > +               __raw_writel(1 << channel, sdma->regs + SDMA_H_STATSTOP);
> > +       }
> > 
> >         sdmac->status = DMA_SUCCESS;
> 
> NAK.
> 
> The patches has several problems.
> 
> - buf_tail is only used for cyclic transfers, so in case of non cyclic
>   transfers you poll for an arbitrary descriptor being ready.
> 
See above.

> - Even in cyclic transfers when buf_tail points to the correct
>   descriptor the hardware will immediatly start the next descriptor
>   before you have a chance to set the STATSTOP bit. So you probably
>   will corrupt the next descriptor instead of the current one
>   which makes this patch useless.
> 
You are right that the buf_tail will not stop immediately until next
time it gets looped on.  But in any case, polling BD_DONE will not
corrupt any descriptor.

> - buf_tail is increased in the interrupt handler, so if you
>   happen to disable the channel after the bd is done but before
>   the interrupt handler has increased buf_tail you poll for the
>   wrong bd being ready which again makes this patch useless.
> 
> If in non cyclic transfers we want to disable a channel we have our
> reasons (device error or timeout) and then the data is corrupted anyway,
> so no reason to poll for a descriptor getting done. Even worse, in case
> of an device error the descriptor might not get ready at all and
Hmm, we should polling (BD_DONE | BD_ERROR).

> we will poll for ever in the loop above.
> Cyclic transfers are designed for audio and disabling a channel
> basically means pausing the stream. When the SDMA engine does not
I thought pausing channel is different from disabling channel, or we
do not need DMA_PAUSE and DMA_TERMINATE_ALL for dma_ctrl_cmd.

> support pausing the stream in hardware, we have to live with the
What about on the fly setting bit "L" of the next descriptor that's
not been running?  So that SDMA can stop the channel when it gets
this descriptor done.

> fact that we pick up the stream on the next descriptor resulting
> in some bytes missing in the resulting audio stream.
> 

-- 
Regards,
Shawn

[PATCH 0/6] A few fixes on imx-sdma

[Sascha Hauer]

On Thu, Jan 20, 2011 at 05:50:34AM +0800, Shawn Guo wrote:
> The patch set is to address the concerns discussed on thread below.
> And it's been tested on mx51 babbage board with sound driver.
> 
> "A few questions on imx-sdma"
> http://article.gmane.org/gmane.linux.ports.arm.kernel/103001/
> 
> Shawn Guo (6)
>  drivers/dma/imx-sdma.c |   31 +++++++++++++++++++------------
>  1 files changed, 19 insertions(+), 12 deletions(-)
> 
>  [PATCH 1/6] dmaengine: imx-sdma: remove IMX_DMA_SG_LOOP handling in sdma_prep_slave_sg()
>  [PATCH 2/6] dmaengine: imx-sdma: set sdmac->status to DMA_ERROR in err_out of sdma_prep_slave_sg()
>  [PATCH 3/6] dmaengine: imx-sdma: return sdmac->status in sdma_tx_status()
>  [PATCH 4/6] dmaengine: imxs-sdma: correct sdmac->status in sdma_handle_channel_loop()
>  [PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

Patches 1/6 - 5/6:

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

Dan,

For your convenience I've set up a branch containing Shawns patches and
the ones just posted here:


The following changes since commit 1bae4ce27c9c90344f23c65ea6966c50ffeae2f5:

  Linux 2.6.38-rc2 (2011-01-21 19:01:34 -0800)

are available in the git repository at:
  git://git.pengutronix.de/git/imx/linux-2.6 dmaengine

Sascha Hauer (11):
      dmaengine i.MX sdma: set maximum segment size for our device
      dmaengine i.MX sdma: check sg entries for valid addresses and lengths
      dmaengine i.MX SDMA: do not initialize chan_id field
      dmaengine i.MX SDMA: initialize dma capabilities outside channel loop
      dmaengine i.MX SDMA: reserve channel 0 by not registering it
      dmaengine i.MX dma: set maximum segment size for our device
      dmaengine i.MX dma: check sg entries for valid addresses and lengths
      dmaengine i.MX DMA: do not initialize chan_id field
      dmaengine i.MX dma: initialize dma capabilities outside channel loop
      Merge branch 'dmaengine-sdma' into dmaengine
      Merge branch 'dmaengine-shawn' into dmaengine

Shawn Guo (5):
      dmaengine: imx-sdma: remove IMX_DMA_SG_LOOP handling in sdma_prep_slave_sg()
      dmaengine: imx-sdma: set sdmac->status to DMA_ERROR in err_out of sdma_prep_slave_sg()
      dmaengine: imx-sdma: return sdmac->status in sdma_tx_status()
      dmaengine: imx-sdma: correct sdmac->status in sdma_handle_channel_loop()
      dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()

 drivers/dma/imx-dma.c  |   26 ++++++++++++++--
 drivers/dma/imx-sdma.c |   76 ++++++++++++++++++++++++++----------------------
 2 files changed, 63 insertions(+), 39 deletions(-)


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 0/6] A few fixes on imx-sdma

[Dan Williams]

On Mon, Jan 31, 2011 at 4:03 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jan 20, 2011 at 05:50:34AM +0800, Shawn Guo wrote:
>> The patch set is to address the concerns discussed on thread below.
>> And it's been tested on mx51 babbage board with sound driver.
>>
>> "A few questions on imx-sdma"
>> http://article.gmane.org/gmane.linux.ports.arm.kernel/103001/
>>
>> Shawn Guo (6)
>> ?drivers/dma/imx-sdma.c | ? 31 +++++++++++++++++++------------
>> ?1 files changed, 19 insertions(+), 12 deletions(-)
>>
>> ?[PATCH 1/6] dmaengine: imx-sdma: remove IMX_DMA_SG_LOOP handling in sdma_prep_slave_sg()
>> ?[PATCH 2/6] dmaengine: imx-sdma: set sdmac->status to DMA_ERROR in err_out of sdma_prep_slave_sg()
>> ?[PATCH 3/6] dmaengine: imx-sdma: return sdmac->status in sdma_tx_status()
>> ?[PATCH 4/6] dmaengine: imxs-sdma: correct sdmac->status in sdma_handle_channel_loop()
>> ?[PATCH 5/6] dmaengine: imx-sdma: fix up param for the last BD in sdma_prep_slave_sg()
>
> Patches 1/6 - 5/6:
>
> Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Dan,
>
> For your convenience I've set up a branch containing Shawns patches and
> the ones just posted here:

Thanks, I'll take this.  One nit please try to avoid
one-line/subject-only commit messages.

--
Dan