Re: [PATCH] fsldma: dev_err 1 too early.

[Dan Williams]

On Wed, 2009-02-25 at 05:47 -0700, Roel Kluin wrote:
> If unsuccessful, i reaches 101 after the loop, so 100 is still success.
> 
> Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx>
> ---
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 70126a6..444c4e4 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -168,7 +168,7 @@ static void dma_halt(struct fsl_dma_chan *fsl_chan)
>  
>  	while (!dma_is_idle(fsl_chan) && (i++ < 100))
>  		udelay(10);
> -	if (i >= 100 && !dma_is_idle(fsl_chan))
> +	if (i > 100 && !dma_is_idle(fsl_chan))
>  		dev_err(fsl_chan->dev, "DMA halt timeout!\n");
>  }
>  

Thanks for the report.  I prefer a more canonical loop.  Objections?

--
Dan

<-----
fsldma: fix off by one in dma_halt

From: Dan Williams <dan.j.williams@xxxxxxxxx>

Prevent dev_err from firing even if we successfully detected 'dma-idle'
before the full 1ms timeout has elapsed.

Reported-by: Roel Kluin <roel.kluin@xxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 drivers/dma/fsldma.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 70126a6..86d6da4 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -158,7 +158,8 @@ static void dma_start(struct fsl_dma_chan *fsl_chan)
 
 static void dma_halt(struct fsl_dma_chan *fsl_chan)
 {
-	int i = 0;
+	int i;
+
 	DMA_OUT(fsl_chan, &fsl_chan->reg_base->mr,
 		DMA_IN(fsl_chan, &fsl_chan->reg_base->mr, 32) | FSL_DMA_MR_CA,
 		32);
@@ -166,8 +167,11 @@ static void dma_halt(struct fsl_dma_chan *fsl_chan)
 		DMA_IN(fsl_chan, &fsl_chan->reg_base->mr, 32) & ~(FSL_DMA_MR_CS
 		| FSL_DMA_MR_EMS_EN | FSL_DMA_MR_CA), 32);
 
-	while (!dma_is_idle(fsl_chan) && (i++ < 100))
+	for (i = 0; i < 100; i++) {
+		if (dma_is_idle(fsl_chan))
+			break;
 		udelay(10);
+	}
 	if (i >= 100 && !dma_is_idle(fsl_chan))
 		dev_err(fsl_chan->dev, "DMA halt timeout!\n");
 }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/