[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Venkatraman S]

See previous post http://patchwork.kernel.org/patch/82907/
Rebased to 2.6.34-rc1 and fixed comment alignment.

CC: Adrian Hunter <adrian.hunter@nokia.com>
CC: Madhusudhan C <madhu.cr@ti.com>
CC: Tony Lindgren <tony@atomide.com>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 83f0aff..ea2a082 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1046,8 +1046,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)

 	if (end_cmd || ((status & CC) && host->cmd))
 		omap_hsmmc_cmd_done(host, host->cmd);
-	if ((end_trans || (status & TC)) && host->mrq)
+	if ((end_trans || (status & TC)) && host->mrq) {
 		omap_hsmmc_xfer_done(host, data);
+		if (host->dma_ch != -1) {
+			omap_free_dma(host->dma_ch);
+			host->dma_ch = -1;
+			/*
+			 * Callback: run in interrupt context.
+			 * mutex_unlock will throw a kernel warning if used.
+			 */
+			up(&host->sem);
+		}
+	}

 	spin_unlock(&host->irq_lock);

@@ -1267,13 +1277,6 @@ static void omap_hsmmc_dma_cb(int lch, u16
ch_status, void *data)
 		return;
 	}

-	omap_free_dma(host->dma_ch);
-	host->dma_ch = -1;
-	/*
-	 * DMA Callback: run in interrupt context.
-	 * mutex_unlock will throw a kernel warning if used.
-	 */
-	up(&host->sem);
 }

 /*
-- 
1.6.3.3

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Shilimkar, Santosh]

> -----Original Message-----
> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of
> Venkatraman S
> Sent: Wednesday, March 10, 2010 7:42 PM
> To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-mmc at vger.kernel.org
> Cc: Adrian Hunter; Chikkature Rajashekar, Madhusudhan; Tony Lindgren
> Subject: [PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback
> 
> See previous post http://patchwork.kernel.org/patch/82907/
> Rebased to 2.6.34-rc1 and fixed comment alignment.
>
Why did you dropped change log in all your patches? You should retain that and
just change the patch version in the subject.
 
> CC: Adrian Hunter <adrian.hunter@nokia.com>
> CC: Madhusudhan C <madhu.cr@ti.com>
> CC: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 83f0aff..ea2a082 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1046,8 +1046,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
> 
>  	if (end_cmd || ((status & CC) && host->cmd))
>  		omap_hsmmc_cmd_done(host, host->cmd);
> -	if ((end_trans || (status & TC)) && host->mrq)
> +	if ((end_trans || (status & TC)) && host->mrq) {
>  		omap_hsmmc_xfer_done(host, data);
> +		if (host->dma_ch != -1) {
> +			omap_free_dma(host->dma_ch);
> +			host->dma_ch = -1;
> +			/*
> +			 * Callback: run in interrupt context.
> +			 * mutex_unlock will throw a kernel warning if used.
> +			 */
> +			up(&host->sem);
> +		}
> +	}
> 
>  	spin_unlock(&host->irq_lock);
> 
> @@ -1267,13 +1277,6 @@ static void omap_hsmmc_dma_cb(int lch, u16
> ch_status, void *data)
>  		return;
>  	}
> 
> -	omap_free_dma(host->dma_ch);
> -	host->dma_ch = -1;
> -	/*
> -	 * DMA Callback: run in interrupt context.
> -	 * mutex_unlock will throw a kernel warning if used.
> -	 */
> -	up(&host->sem);
>  }
> 
>  /*
> --
> 1.6.3.3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Venkatraman S]

Shilimkar, Santosh wrote:
>> -----Original Message-----
>> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of
>> Venkatraman S
>> Sent: Wednesday, March 10, 2010 7:42 PM
>> To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-mmc at vger.kernel.org
>> Cc: Adrian Hunter; Chikkature Rajashekar, Madhusudhan; Tony Lindgren
>> Subject: [PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback
>>
>> See previous post http://patchwork.kernel.org/patch/82907/
>> Rebased to 2.6.34-rc1 and fixed comment alignment.
>>
> Why did you dropped change log in all your patches? You should retain that and
> just change the patch version in the subject.
>

My bad. I just edited the merge commit to point to the previous
threads and posted it.
Should I post them again ?

Thanks and regards,
Venkat.

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Kevin Hilman]

Venkatraman S <svenkatr@ti.com> writes:

> Shilimkar, Santosh wrote:
>>> -----Original Message-----
>>> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of
>>> Venkatraman S
>>> Sent: Wednesday, March 10, 2010 7:42 PM
>>> To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-mmc at vger.kernel.org
>>> Cc: Adrian Hunter; Chikkature Rajashekar, Madhusudhan; Tony Lindgren
>>> Subject: [PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback
>>>
>>> See previous post http://patchwork.kernel.org/patch/82907/
>>> Rebased to 2.6.34-rc1 and fixed comment alignment.
>>>
>> Why did you dropped change log in all your patches? You should retain that and
>> just change the patch version in the subject.
>>
>
> My bad. I just edited the merge commit to point to the previous
> threads and posted it.
> Should I post them again ?
>

Yes.  

As Santosh pointed out, keep the changelog. and add any comments about
how this differs from previous versions after the '---' in the
changelog.

Keep in mind that the changelog will be stored in the git history and
should be useful without having to reference previous versions etc.

Anything useful to the reviewers but not important for the changelog
should go after the '---' since that will be ignored by git when
applying.

Kevin

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Chris Ball]

Hi,

Looks like the discussion on this patch stopped: do we think there's a
race worth fixing here?

On Wed, Mar 10, 2010 at 07:41:38PM +0530, Venkatraman S wrote:
> See previous post http://patchwork.kernel.org/patch/82907/
> Rebased to 2.6.34-rc1 and fixed comment alignment.
> 
> CC: Adrian Hunter <adrian.hunter@nokia.com>
> CC: Madhusudhan C <madhu.cr@ti.com>
> CC: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 83f0aff..ea2a082 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1046,8 +1046,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
> 
>  	if (end_cmd || ((status & CC) && host->cmd))
>  		omap_hsmmc_cmd_done(host, host->cmd);
> -	if ((end_trans || (status & TC)) && host->mrq)
> +	if ((end_trans || (status & TC)) && host->mrq) {
>  		omap_hsmmc_xfer_done(host, data);
> +		if (host->dma_ch != -1) {
> +			omap_free_dma(host->dma_ch);
> +			host->dma_ch = -1;
> +			/*
> +			 * Callback: run in interrupt context.
> +			 * mutex_unlock will throw a kernel warning if used.
> +			 */
> +			up(&host->sem);
> +		}
> +	}
> 
>  	spin_unlock(&host->irq_lock);
> 
> @@ -1267,13 +1277,6 @@ static void omap_hsmmc_dma_cb(int lch, u16
> ch_status, void *data)
>  		return;
>  	}
> 
> -	omap_free_dma(host->dma_ch);
> -	host->dma_ch = -1;
> -	/*
> -	 * DMA Callback: run in interrupt context.
> -	 * mutex_unlock will throw a kernel warning if used.
> -	 */
> -	up(&host->sem);
>  }
> 
>  /*
> -- 
> 1.6.3.3

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Venkatraman S]

It is observed that sometimes the MMC Transfer Complete (TC) is not
received when DMA completion callback is processed (or takes too long
to process)
This patch moves the releasing of DMA channel from dma callback to the
mmc transfer complete handler.
This is observed consistently when descriptor loading is used (see the
rest of the series)

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 83f0aff..06337f6 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1046,8 +1046,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)

 	if (end_cmd || ((status & CC) && host->cmd))
 		omap_hsmmc_cmd_done(host, host->cmd);
-	if ((end_trans || (status & TC)) && host->mrq)
+	if ((end_trans || (status & TC)) && host->mrq) {
 		omap_hsmmc_xfer_done(host, data);
+		if (host->dma_ch != -1) {
+			omap_free_dma(host->dma_ch);
+			host->dma_ch = -1;
+		/*
+		 * Callback: run in interrupt context.
+		 * mutex_unlock will throw a kernel warning if used.
+		 */
+			up(&host->sem);
+		}
+	}

 	spin_unlock(&host->irq_lock);

@@ -1267,13 +1277,6 @@ static void omap_hsmmc_dma_cb(int lch, u16
ch_status, void *data)
 		return;
 	}

-	omap_free_dma(host->dma_ch);
-	host->dma_ch = -1;
-	/*
-	 * DMA Callback: run in interrupt context.
-	 * mutex_unlock will throw a kernel warning if used.
-	 */
-	up(&host->sem);
 }

 /*
-- 
1.6.3.3

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Sergei Shtylyov]

Hello.

Venkatraman S wrote:

> It is observed that sometimes the MMC Transfer Complete (TC) is not
> received when DMA completion callback is processed (or takes too long
> to process)
> This patch moves the releasing of DMA channel from dma callback to the
> mmc transfer complete handler.
> This is observed consistently when descriptor loading is used (see the
> rest of the series)
>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 83f0aff..06337f6 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1046,8 +1046,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
>
>  	if (end_cmd || ((status & CC) && host->cmd))
>  		omap_hsmmc_cmd_done(host, host->cmd);
> -	if ((end_trans || (status & TC)) && host->mrq)
> +	if ((end_trans || (status & TC)) && host->mrq) {
>  		omap_hsmmc_xfer_done(host, data);
> +		if (host->dma_ch != -1) {
> +			omap_free_dma(host->dma_ch);
> +			host->dma_ch = -1;
> +		/*
> +		 * Callback: run in interrupt context.
> +		 * mutex_unlock will throw a kernel warning if used.
> +		 */
>   

   The comment should be aligned with the rest of the code.

> +			up(&host->sem);
> +		}
> +	}
>
>  	spin_unlock(&host->irq_lock);

WBR, Sergei

[PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc callback

[Madhusudhan]

> -----Original Message-----
> From: linux-mmc-owner at vger.kernel.org [mailto:linux-mmc-
> owner at vger.kernel.org] On Behalf Of Venkatraman S
> Sent: Monday, March 01, 2010 5:26 AM
> To: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linux-omap at vger.kernel.org
> Subject: [PATCH 01/03] omap hsmmc: prevent race between dma and hsmmc
> callback
> 
> It is observed that sometimes the MMC Transfer Complete (TC) is not
> received when DMA completion callback is processed (or takes too long
> to process)
> This patch moves the releasing of DMA channel from dma callback to the
> mmc transfer complete handler.
> This is observed consistently when descriptor loading is used (see the
> rest of the series)
> 
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 83f0aff..06337f6 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1046,8 +1046,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> *dev_id)
> 
>  	if (end_cmd || ((status & CC) && host->cmd))
>  		omap_hsmmc_cmd_done(host, host->cmd);
> -	if ((end_trans || (status & TC)) && host->mrq)
> +	if ((end_trans || (status & TC)) && host->mrq) {
>  		omap_hsmmc_xfer_done(host, data);
> +		if (host->dma_ch != -1) {
> +			omap_free_dma(host->dma_ch);
> +			host->dma_ch = -1;

Why is this necessary? The core is notified of xfer complete only when TC is
received which is good enough.

> +		/*
> +		 * Callback: run in interrupt context.
> +		 * mutex_unlock will throw a kernel warning if used.
> +		 */
> +			up(&host->sem);
> +		}
> +	}
> 
>  	spin_unlock(&host->irq_lock);
> 
> @@ -1267,13 +1277,6 @@ static void omap_hsmmc_dma_cb(int lch, u16
> ch_status, void *data)
>  		return;
>  	}
> 
> -	omap_free_dma(host->dma_ch);
> -	host->dma_ch = -1;
> -	/*
> -	 * DMA Callback: run in interrupt context.
> -	 * mutex_unlock will throw a kernel warning if used.
> -	 */
> -	up(&host->sem);
>  }
> 
>  /*
> --
> 1.6.3.3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html