[PATCH 1/5] mmc: msm_sdcc: Fix possible circular locking dependency warning

[PATCH 1/5] mmc: msm_sdcc: Fix possible circular locking dependency warning

[Sahitya Tummala]

In the context of request processing thread, data mover lock is
acquired after the host lock.  In another context, in the completion
handler of data mover the locks are acquired in the reverse order,
resulting in possible circular lock dependency warning. Hence,
schedule a tasklet to process the dma completion so as to avoid
nested locks.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
---
 drivers/mmc/host/msm_sdcc.c |   49 +++++++++++++++++++++++++++++--------------
 drivers/mmc/host/msm_sdcc.h |    3 ++
 2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/msm_sdcc.c b/drivers/mmc/host/msm_sdcc.c
index 1290d14..b147971 100644
--- a/drivers/mmc/host/msm_sdcc.c
+++ b/drivers/mmc/host/msm_sdcc.c
@@ -189,42 +189,40 @@ msmsdcc_dma_exec_func(struct msm_dmov_cmd *cmd)
 }
 
 static void
-msmsdcc_dma_complete_func(struct msm_dmov_cmd *cmd,
-			  unsigned int result,
-			  struct msm_dmov_errdata *err)
+msmsdcc_dma_complete_tlet(unsigned long data)
 {
-	struct msmsdcc_dma_data	*dma_data =
-		container_of(cmd, struct msmsdcc_dma_data, hdr);
-	struct msmsdcc_host	*host = dma_data->host;
+	struct msmsdcc_host *host = (struct msmsdcc_host *)data;
 	unsigned long		flags;
 	struct mmc_request	*mrq;
+	struct msm_dmov_errdata err;
 
 	spin_lock_irqsave(&host->lock, flags);
 	host->dma.active = 0;
 
+	err = host->dma.err;
 	mrq = host->curr.mrq;
 	BUG_ON(!mrq);
 	WARN_ON(!mrq->data);
 
-	if (!(result & DMOV_RSLT_VALID)) {
+	if (!(host->dma.result & DMOV_RSLT_VALID)) {
 		pr_err("msmsdcc: Invalid DataMover result\n");
 		goto out;
 	}
 
-	if (result & DMOV_RSLT_DONE) {
+	if (host->dma.result & DMOV_RSLT_DONE) {
 		host->curr.data_xfered = host->curr.xfer_size;
 	} else {
 		/* Error or flush  */
-		if (result & DMOV_RSLT_ERROR)
+		if (host->dma.result & DMOV_RSLT_ERROR)
 			pr_err("%s: DMA error (0x%.8x)\n",
-			       mmc_hostname(host->mmc), result);
-		if (result & DMOV_RSLT_FLUSH)
+			       mmc_hostname(host->mmc), host->dma.result);
+		if (host->dma.result & DMOV_RSLT_FLUSH)
 			pr_err("%s: DMA channel flushed (0x%.8x)\n",
-			       mmc_hostname(host->mmc), result);
-		if (err)
-			pr_err("Flush data: %.8x %.8x %.8x %.8x %.8x %.8x\n",
-			       err->flush[0], err->flush[1], err->flush[2],
-			       err->flush[3], err->flush[4], err->flush[5]);
+			       mmc_hostname(host->mmc), host->dma.result);
+
+		pr_err("Flush data: %.8x %.8x %.8x %.8x %.8x %.8x\n",
+		       err.flush[0], err.flush[1], err.flush[2],
+		       err.flush[3], err.flush[4], err.flush[5]);
 		if (!mrq->data->error)
 			mrq->data->error = -EIO;
 	}
@@ -273,6 +271,22 @@ out:
 	return;
 }
 
+static void
+msmsdcc_dma_complete_func(struct msm_dmov_cmd *cmd,
+			  unsigned int result,
+			  struct msm_dmov_errdata *err)
+{
+	struct msmsdcc_dma_data	*dma_data =
+		container_of(cmd, struct msmsdcc_dma_data, hdr);
+	struct msmsdcc_host *host = dma_data->host;
+
+	dma_data->result = result;
+	if (err)
+		memcpy(&dma_data->err, err, sizeof(struct msm_dmov_errdata));
+
+	tasklet_schedule(&host->dma_tlet);
+}
+
 static int validate_dma(struct msmsdcc_host *host, struct mmc_data *data)
 {
 	if (host->dma.channel == -1)
@@ -1118,6 +1132,9 @@ msmsdcc_probe(struct platform_device *pdev)
 	host->dmares = dmares;
 	spin_lock_init(&host->lock);
 
+	tasklet_init(&host->dma_tlet, msmsdcc_dma_complete_tlet,
+			(unsigned long)host);
+
 	/*
 	 * Setup DMA
 	 */
diff --git a/drivers/mmc/host/msm_sdcc.h b/drivers/mmc/host/msm_sdcc.h
index ff2b0f7..996990d 100644
--- a/drivers/mmc/host/msm_sdcc.h
+++ b/drivers/mmc/host/msm_sdcc.h
@@ -172,6 +172,8 @@ struct msmsdcc_dma_data {
 	struct msmsdcc_host		*host;
 	int				busy; /* Set if DM is busy */
 	int				active;
+	unsigned int			result;
+	struct msm_dmov_errdata		err;
 };
 
 struct msmsdcc_pio_data {
@@ -235,6 +237,7 @@ struct msmsdcc_host {
 	int			cmdpoll;
 	struct msmsdcc_stats	stats;
 
+	struct tasklet_struct	dma_tlet;
 	/* Command parameters */
 	unsigned int		cmd_timeout;
 	unsigned int		cmd_pio_irqmask;
-- 
1.7.1

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 2/5] mmc: msm_sdcc: Add prog done interrupt support

[Sahitya Tummala]

Enable prog done interrupt for stop command(CMD12) that is sent
after a multi-block write(CMD25). The PROG_DONE bit is set when
the card has finished its programming and is ready for next data.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
---
 drivers/mmc/host/msm_sdcc.c |   28 +++++++++++++++++++++++++---
 drivers/mmc/host/msm_sdcc.h |    4 +++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/msm_sdcc.c b/drivers/mmc/host/msm_sdcc.c
index b147971..b64775c 100644
--- a/drivers/mmc/host/msm_sdcc.c
+++ b/drivers/mmc/host/msm_sdcc.c
@@ -438,6 +438,11 @@ msmsdcc_start_command_deferred(struct msmsdcc_host *host,
 	      (cmd->opcode == 53))
 		*c |= MCI_CSPM_DATCMD;
 
+	if (host->prog_scan && (cmd->opcode == 12)) {
+		*c |= MCI_CPSM_PROGENA;
+		host->prog_enable = 1;
+	}
+
 	if (cmd == cmd->mrq->stop)
 		*c |= MCI_CSPM_MCIABORT;
 
@@ -508,6 +513,8 @@ msmsdcc_start_data(struct msmsdcc_host *host, struct mmc_data *data,
 			host->cmd_c = c;
 		}
 		msm_dmov_enqueue_cmd(host->dma.channel, &host->dma.hdr);
+		if (data->flags & MMC_DATA_WRITE)
+			host->prog_scan = 1;
 	} else {
 		msmsdcc_writel(host, timeout, MMCIDATATIMER);
 
@@ -718,8 +725,23 @@ static void msmsdcc_do_cmdirq(struct msmsdcc_host *host, uint32_t status)
 		else if (host->curr.data) { /* Non DMA */
 			msmsdcc_stop_data(host);
 			msmsdcc_request_end(host, cmd->mrq);
-		} else /* host->data == NULL */
-			msmsdcc_request_end(host, cmd->mrq);
+		} else { /* host->data == NULL */
+			if (!cmd->error && host->prog_enable) {
+				if (status & MCI_PROGDONE) {
+					host->prog_scan = 0;
+					host->prog_enable = 0;
+					msmsdcc_request_end(host, cmd->mrq);
+				} else {
+					host->curr.cmd = cmd;
+				}
+			} else {
+				if (host->prog_enable) {
+					host->prog_scan = 0;
+					host->prog_enable = 0;
+				}
+				msmsdcc_request_end(host, cmd->mrq);
+			}
+		}
 	} else if (cmd->data)
 		if (!(cmd->data->flags & MMC_DATA_READ))
 			msmsdcc_start_data(host, cmd->data,
@@ -733,7 +755,7 @@ msmsdcc_handle_irq_data(struct msmsdcc_host *host, u32 status,
 	struct mmc_data *data = host->curr.data;
 
 	if (status & (MCI_CMDSENT | MCI_CMDRESPEND | MCI_CMDCRCFAIL |
-	              MCI_CMDTIMEOUT) && host->curr.cmd) {
+		MCI_CMDTIMEOUT | MCI_PROGDONE) && host->curr.cmd) {
 		msmsdcc_do_cmdirq(host, status);
 	}
 
diff --git a/drivers/mmc/host/msm_sdcc.h b/drivers/mmc/host/msm_sdcc.h
index 996990d..ac79ed9 100644
--- a/drivers/mmc/host/msm_sdcc.h
+++ b/drivers/mmc/host/msm_sdcc.h
@@ -138,7 +138,7 @@
 #define MCI_IRQENABLE	\
 	(MCI_CMDCRCFAILMASK|MCI_DATACRCFAILMASK|MCI_CMDTIMEOUTMASK|	\
 	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
-	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATAENDMASK)
+	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATAENDMASK|MCI_PROGDONEMASK)
 
 /*
  * The size of the FIFO in bytes.
@@ -245,6 +245,8 @@ struct msmsdcc_host {
 	struct mmc_command	*cmd_cmd;
 	u32			cmd_c;
 
+	unsigned int prog_scan;
+	unsigned int prog_enable;
 };
 
 #endif
-- 
1.7.1

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 3/5] mmc: msm_sdcc: Reset SDCC in case of data transfer errors

[Sahitya Tummala]

Reset SDCC in case of data transfer errors to ensure that
any left over data in FIFOs is flushed out.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
---
 drivers/mmc/host/msm_sdcc.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/msm_sdcc.c b/drivers/mmc/host/msm_sdcc.c
index b64775c..b5cca8c 100644
--- a/drivers/mmc/host/msm_sdcc.c
+++ b/drivers/mmc/host/msm_sdcc.c
@@ -44,6 +44,7 @@
 #include <mach/mmc.h>
 #include <mach/msm_iomap.h>
 #include <mach/dma.h>
+#include <mach/clk.h>
 
 #include "msm_sdcc.h"
 
@@ -126,6 +127,40 @@ static void
 msmsdcc_start_command(struct msmsdcc_host *host, struct mmc_command *cmd,
 		      u32 c);
 
+static void msmsdcc_reset_and_restore(struct msmsdcc_host *host)
+{
+	u32	mci_clk = 0;
+	u32	mci_mask0 = 0;
+	int	ret = 0;
+
+	/* Save the controller state */
+	mci_clk = readl(host->base + MMCICLOCK);
+	mci_mask0 = readl(host->base + MMCIMASK0);
+
+	/* Reset the controller */
+	ret = clk_reset(host->clk, CLK_RESET_ASSERT);
+	if (ret)
+		pr_err("%s: Clock assert failed at %u Hz with err %d\n",
+				mmc_hostname(host->mmc), host->clk_rate, ret);
+
+	ret = clk_reset(host->clk, CLK_RESET_DEASSERT);
+	if (ret)
+		pr_err("%s: Clock deassert failed at %u Hz with err %d\n",
+				mmc_hostname(host->mmc), host->clk_rate, ret);
+
+	pr_info("%s: Controller has been re-initialiazed\n",
+			mmc_hostname(host->mmc));
+
+	/* Restore the contoller state */
+	writel(host->pwr, host->base + MMCIPOWER);
+	writel(mci_clk, host->base + MMCICLOCK);
+	writel(mci_mask0, host->base + MMCIMASK0);
+	ret = clk_set_rate(host->clk, host->clk_rate);
+	if (ret)
+		pr_err("%s: Failed to set clk rate %u Hz (%d)\n",
+				mmc_hostname(host->mmc), host->clk_rate, ret);
+}
+
 static void
 msmsdcc_request_end(struct msmsdcc_host *host, struct mmc_request *mrq)
 {
@@ -223,6 +258,8 @@ msmsdcc_dma_complete_tlet(unsigned long data)
 		pr_err("Flush data: %.8x %.8x %.8x %.8x %.8x %.8x\n",
 		       err.flush[0], err.flush[1], err.flush[2],
 		       err.flush[3], err.flush[4], err.flush[5]);
+
+		msmsdcc_reset_and_restore(host);
 		if (!mrq->data->error)
 			mrq->data->error = -EIO;
 	}
@@ -723,6 +760,7 @@ static void msmsdcc_do_cmdirq(struct msmsdcc_host *host, uint32_t status)
 			msm_dmov_stop_cmd(host->dma.channel,
 					  &host->dma.hdr, 0);
 		else if (host->curr.data) { /* Non DMA */
+			msmsdcc_reset_and_restore(host);
 			msmsdcc_stop_data(host);
 			msmsdcc_request_end(host, cmd->mrq);
 		} else { /* host->data == NULL */
@@ -771,6 +809,7 @@ msmsdcc_handle_irq_data(struct msmsdcc_host *host, u32 status,
 			msm_dmov_stop_cmd(host->dma.channel,
 					  &host->dma.hdr, 0);
 		else {
+			msmsdcc_reset_and_restore(host);
 			if (host->curr.data)
 				msmsdcc_stop_data(host);
 			if (!data->stop)
-- 
1.7.1

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 4/5] mmc: msm_sdcc: Fix bug in PIO mode when data size is not word aligned

[Sahitya Tummala]

The current code for PIO doesn't transfer whole data when data size
is not in multiple of 4 bytes. The last few bytes are not written to
the card resulting in no DATAEND interrupt from SDCC. This patch
allows data transfer for non-aligned data size in PIO mode.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
---
 drivers/mmc/host/msm_sdcc.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/msm_sdcc.c b/drivers/mmc/host/msm_sdcc.c
index b5cca8c..355d799 100644
--- a/drivers/mmc/host/msm_sdcc.c
+++ b/drivers/mmc/host/msm_sdcc.c
@@ -613,6 +613,9 @@ msmsdcc_pio_read(struct msmsdcc_host *host, char *buffer, unsigned int remain)
 	uint32_t	*ptr = (uint32_t *) buffer;
 	int		count = 0;
 
+	if (remain % 4)
+		remain = ((remain >> 2) + 1) << 2;
+
 	while (msmsdcc_readl(host, MMCISTATUS) & MCI_RXDATAAVLBL) {
 		*ptr = msmsdcc_readl(host, MMCIFIFO + (count % MCI_FIFOSIZE));
 		ptr++;
@@ -633,13 +636,14 @@ msmsdcc_pio_write(struct msmsdcc_host *host, char *buffer,
 	char *ptr = buffer;
 
 	do {
-		unsigned int count, maxcnt;
+		unsigned int count, maxcnt, sz;
 
 		maxcnt = status & MCI_TXFIFOEMPTY ? MCI_FIFOSIZE :
 						    MCI_FIFOHALFSIZE;
 		count = min(remain, maxcnt);
 
-		writesl(base + MMCIFIFO, ptr, count >> 2);
+		sz = count % 4 ? (count >> 2) + 1 : (count >> 2);
+		writesl(base + MMCIFIFO, ptr, sz);
 		ptr += count;
 		remain -= count;
 
-- 
1.7.1

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 5/5] mmc: msm_sdcc: Check for only DATA_END interrupt to end a request

[Sahitya Tummala]

The current code checks for both DATA_END and DATA_BLK_END bits in
MCI_STATUS register and ends a request only if both are set at a time.
The hardware doesn't always set DATA_BLK_END when DATA_END is set.
But DATA_END status itself is sufficient condition from hardware that
data transfer is done and hence, check for only DATA_END interrupt in
software to end a request.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
---
 drivers/mmc/host/msm_sdcc.c |   15 ++++-----------
 drivers/mmc/host/msm_sdcc.h |    1 -
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/msm_sdcc.c b/drivers/mmc/host/msm_sdcc.c
index 355d799..23b6f57 100644
--- a/drivers/mmc/host/msm_sdcc.c
+++ b/drivers/mmc/host/msm_sdcc.c
@@ -190,7 +190,7 @@ static void
 msmsdcc_stop_data(struct msmsdcc_host *host)
 {
 	host->curr.data = NULL;
-	host->curr.got_dataend = host->curr.got_datablkend = 0;
+	host->curr.got_dataend = 0;
 }
 
 uint32_t msmsdcc_fifo_addr(struct msmsdcc_host *host)
@@ -277,8 +277,7 @@ msmsdcc_dma_complete_tlet(unsigned long data)
 	host->dma.sg = NULL;
 	host->dma.busy = 0;
 
-	if ((host->curr.got_dataend && host->curr.got_datablkend)
-	     || mrq->data->error) {
+	if (host->curr.got_dataend || mrq->data->error) {
 
 		/*
 		 * If we've already gotten our DATAEND / DATABLKEND
@@ -506,7 +505,6 @@ msmsdcc_start_data(struct msmsdcc_host *host, struct mmc_data *data,
 	host->curr.xfer_remain = host->curr.xfer_size;
 	host->curr.data_xfered = 0;
 	host->curr.got_dataend = 0;
-	host->curr.got_datablkend = 0;
 
 	memset(&host->pio, 0, sizeof(host->pio));
 
@@ -827,14 +825,10 @@ msmsdcc_handle_irq_data(struct msmsdcc_host *host, u32 status,
 	if (!host->curr.got_dataend && (status & MCI_DATAEND))
 		host->curr.got_dataend = 1;
 
-	if (!host->curr.got_datablkend && (status & MCI_DATABLOCKEND))
-		host->curr.got_datablkend = 1;
-
 	/*
 	 * If DMA is still in progress, we complete via the completion handler
 	 */
-	if (host->curr.got_dataend && host->curr.got_datablkend &&
-	    !host->dma.busy) {
+	if (host->curr.got_dataend && !host->dma.busy) {
 		/*
 		 * There appears to be an issue in the controller where
 		 * if you request a small block transfer (< fifo size),
@@ -871,8 +865,7 @@ msmsdcc_irq(int irq, void *dev_id)
 
 	do {
 		status = msmsdcc_readl(host, MMCISTATUS);
-		status &= (msmsdcc_readl(host, MMCIMASK0) |
-					      MCI_DATABLOCKENDMASK);
+		status &= msmsdcc_readl(host, MMCIMASK0);
 		msmsdcc_writel(host, status, MMCICLEAR);
 
 		if (status & MCI_SDIOINTR)
diff --git a/drivers/mmc/host/msm_sdcc.h b/drivers/mmc/host/msm_sdcc.h
index ac79ed9..9a14144 100644
--- a/drivers/mmc/host/msm_sdcc.h
+++ b/drivers/mmc/host/msm_sdcc.h
@@ -190,7 +190,6 @@ struct msmsdcc_curr_req {
 	unsigned int		xfer_remain;	/* Bytes remaining to send */
 	unsigned int		data_xfered;	/* Bytes acked by BLKEND irq */
 	int			got_dataend;
-	int			got_datablkend;
 	int			user_pages;
 };
 
-- 
1.7.1

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 1/5] mmc: msm_sdcc: Fix possible circular locking dependency warning

[Daniel Walker]

On Tue, 2010-12-07 at 16:25 +0530, Sahitya Tummala wrote:
> In the context of request processing thread, data mover lock is
> acquired after the host lock.  In another context, in the completion
> handler of data mover the locks are acquired in the reverse order,
> resulting in possible circular lock dependency warning. Hence,
> schedule a tasklet to process the dma completion so as to avoid
> nested locks.

Can you provide and example of the warning ? Not in the commit, but in
this thread.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 2/5] mmc: msm_sdcc: Add prog done interrupt support

[Daniel Walker]

On Tue, 2010-12-07 at 16:25 +0530, Sahitya Tummala wrote:
> Enable prog done interrupt for stop command(CMD12) that is sent
> after a multi-block write(CMD25). The PROG_DONE bit is set when
> the card has finished its programming and is ready for next data.

What's the benefit ? It looks like there is one, but please describe it.

>  	if (status & (MCI_CMDSENT | MCI_CMDRESPEND | MCI_CMDCRCFAIL |
> -	              MCI_CMDTIMEOUT) && host->curr.cmd) {
> +		MCI_CMDTIMEOUT | MCI_PROGDONE) && host->curr.cmd) {

Add a line break here instead of weird spacing.

>  		msmsdcc_do_cmdirq(host, status);
>  	}
>  
> diff --git a/drivers/mmc/host/msm_sdcc.h b/drivers/mmc/host/msm_sdcc.h
> index 996990d..ac79ed9 100644
> --- a/drivers/mmc/host/msm_sdcc.h
> +++ b/drivers/mmc/host/msm_sdcc.h
> @@ -138,7 +138,7 @@
>  #define MCI_IRQENABLE	\
>  	(MCI_CMDCRCFAILMASK|MCI_DATACRCFAILMASK|MCI_CMDTIMEOUTMASK|	\
>  	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
> -	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATAENDMASK)
> +	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATAENDMASK|MCI_PROGDONEMASK)
>  
>  /*
>   * The size of the FIFO in bytes.
> @@ -245,6 +245,8 @@ struct msmsdcc_host {
>  	struct mmc_command	*cmd_cmd;
>  	u32			cmd_c;
>  
> +	unsigned int prog_scan;
> +	unsigned int prog_enable;

These need to be bool, and you need to use true and false instead of 1
and 0.

Daniel
-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 3/5] mmc: msm_sdcc: Reset SDCC in case of data transfer errors

[Daniel Walker]

On Tue, 2010-12-07 at 16:25 +0530, Sahitya Tummala wrote:
> Reset SDCC in case of data transfer errors to ensure that
> any left over data in FIFOs is flushed out.

Does this cause a problem which you have observed? If so please describe
it in the commit text.

Daniel
-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 1/5] mmc: msm_sdcc: Fix possible circular locking dependency warning

[Sahitya Tummala]

Hi Daniel,

On Tue, 2010-12-07 at 10:15 -0800, Daniel Walker wrote:

> Can you provide and example of the warning ? Not in the commit, but in
> this thread.

This patch fixes the following warning - 

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.37-rc2-g7559620 #58
-------------------------------------------------------
init/1 is trying to acquire lock:
 (&(&host->lock)->rlock){-.....}, at: [<c019efe8>]
msmsdcc_dma_complete_func+0x20/0x26c

but task is already holding lock:
 (msm_dmov_lock){-.....}, at: [<c002a2b4>] msm_datamover_irq_handler
+0x14/0x594

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (msm_dmov_lock){-.....}:
       [<c0061618>] lock_acquire+0x88/0x9c
       [<c02189cc>] _raw_spin_lock_irqsave+0x48/0x5c
       [<c002a884>] msm_dmov_enqueue_cmd+0x14/0x1ac
       [<c019ee94>] msmsdcc_start_data+0x3a4/0x4d8
       [<c019f584>] msmsdcc_request+0x15c/0x20c
       [<c01960f0>] mmc_wait_for_req+0x148/0x164
       [<c01994b0>] mmc_app_sd_status+0xec/0x10c
       [<c01987c8>] mmc_sd_setup_card+0xc4/0x304
       [<c0198f30>] mmc_sd_init_card+0x130/0x1d4
       [<c01990e4>] mmc_attach_sd+0x110/0x198
       [<c0195f2c>] mmc_rescan+0x298/0x314
       [<c0049c18>] process_one_work+0x254/0x408
       [<c004a1fc>] worker_thread+0x1b8/0x2d8
       [<c004ef0c>] kthread+0x84/0x8c
       [<c0022910>] kernel_thread_exit+0x0/0x8

-> #0 (&(&host->lock)->rlock){-.....}:
       [<c0060e70>] __lock_acquire+0x1118/0x1838
       [<c0061618>] lock_acquire+0x88/0x9c
       [<c02189cc>] _raw_spin_lock_irqsave+0x48/0x5c
       [<c019efe8>] msmsdcc_dma_complete_func+0x20/0x26c
       [<c002a47c>] msm_datamover_irq_handler+0x1dc/0x594
       [<c006b3c8>] handle_IRQ_event+0x24/0xe8
       [<c006d50c>] handle_level_irq+0xc0/0x140
       [<c0021074>] asm_do_IRQ+0x74/0x98
       [<c0021af0>] __irq_svc+0x50/0x98
       [<c0061624>] lock_acquire+0x94/0x9c
       [<c0217ee0>] down_read+0x48/0x5c
       [<c00473e0>] sys_newuname+0x10/0x78
       [<c0021f00>] ret_fast_syscall+0x0/0x3c

other info that might help us debug this:

2 locks held by init/1:
 #0:  (uts_sem){.+.+..}, at: [<c00473e0>] sys_newuname+0x10/0x78
 #1:  (msm_dmov_lock){-.....}, at: [<c002a2b4>]
msm_datamover_irq_handler+0x14/0x594

stack backtrace:
[<c002638c>] (unwind_backtrace+0x0/0xec) from [<c005f834>]
(print_circular_bug+0xc8/0xe0)
[<c005f834>] (print_circular_bug+0xc8/0xe0) from [<c0060e70>]
(__lock_acquire+0x1118/0x1838)
[<c0060e70>] (__lock_acquire+0x1118/0x1838) from [<c0061618>]
(lock_acquire+0x88/0x9c)
[<c0061618>] (lock_acquire+0x88/0x9c) from [<c02189cc>]
(_raw_spin_lock_irqsave+0x48/0x5c)
[<c02189cc>] (_raw_spin_lock_irqsave+0x48/0x5c) from [<c019efe8>]
(msmsdcc_dma_complete_func+0x20/0x26c)
[<c019efe8>] (msmsdcc_dma_complete_func+0x20/0x26c) from [<c002a47c>]
(msm_datamover_irq_handler+0x1dc/0x594)
[<c002a47c>] (msm_datamover_irq_handler+0x1dc/0x594) from [<c006b3c8>]
(handle_IRQ_event+0x24/0xe8)
[<c006b3c8>] (handle_IRQ_event+0x24/0xe8) from [<c006d50c>]
(handle_level_irq+0xc0/0x140)
[<c006d50c>] (handle_level_irq+0xc0/0x140) from [<c0021074>] (asm_do_IRQ
+0x74/0x98)
[<c0021074>] (asm_do_IRQ+0x74/0x98) from [<c0021af0>] (__irq_svc
+0x50/0x98)
Exception stack(0xc7823ef8 to 0xc7823f40)
3ee0:                                                       00000001
000000db
3f00: c08158e0 c7821240 00000000 60000013 c7822000 00000000 00000000
00000001
3f20: 00000000 c02e7360 c7821658 c7823f40 c006157c c0061624 80000013
ffffffff
[<c0021af0>] (__irq_svc+0x50/0x98) from [<c0061624>] (lock_acquire
+0x94/0x9c)
[<c0061624>] (lock_acquire+0x94/0x9c) from [<c0217ee0>] (down_read
+0x48/0x5c)
[<c0217ee0>] (down_read+0x48/0x5c) from [<c00473e0>] (sys_newuname
+0x10/0x78)
[<c00473e0>] (sys_newuname+0x10/0x78) from [<c0021f00>]
(ret_fast_syscall+0x0/0x3c)
mmc0: host does not support reading read-only switch. assuming
write-enable.
mmc0: new high speed SDHC card at address 0007
mmcblk0: mmc0:0007 SD8GB 7.63 GiB
 mmcblk0: p1

Thanks,
Sahitya.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

Re: [PATCH 2/5] mmc: msm_sdcc: Add prog done interrupt support

[Sahitya Tummala]

Hi Daniel,

On Tue, 2010-12-07 at 10:27 -0800, Daniel Walker wrote:
> On Tue, 2010-12-07 at 16:25 +0530, Sahitya Tummala wrote:
> > Enable prog done interrupt for stop command(CMD12) that is sent
> > after a multi-block write(CMD25). The PROG_DONE bit is set when
> > the card has finished its programming and is ready for next data.
> 
> What's the benefit ? It looks like there is one, but please describe it.

After every write request the card will be polled for ready status
using CMD13. For a multi-block write(CMD25) before sending CMD13,
stop command (CMD12) will be sent.  If we enable prog done interrupt
for CMD12, then CMD13 polling can be avoided. The prog done interrupt
means that the card is done with its programming and is ready for
next request. I will this to commit text also.

> >  	if (status & (MCI_CMDSENT | MCI_CMDRESPEND | MCI_CMDCRCFAIL |
> > -	              MCI_CMDTIMEOUT) && host->curr.cmd) {
> > +		MCI_CMDTIMEOUT | MCI_PROGDONE) && host->curr.cmd) {
> 
> Add a line break here instead of weird spacing.

Will fix it in V2.

> >  		msmsdcc_do_cmdirq(host, status);
> >  	}
> >  
> > diff --git a/drivers/mmc/host/msm_sdcc.h b/drivers/mmc/host/msm_sdcc.h
> > index 996990d..ac79ed9 100644
> > --- a/drivers/mmc/host/msm_sdcc.h
> > +++ b/drivers/mmc/host/msm_sdcc.h
> > @@ -138,7 +138,7 @@
> >  #define MCI_IRQENABLE	\
> >  	(MCI_CMDCRCFAILMASK|MCI_DATACRCFAILMASK|MCI_CMDTIMEOUTMASK|	\
> >  	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
> > -	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATAENDMASK)
> > +	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATAENDMASK|MCI_PROGDONEMASK)
> >  
> >  /*
> >   * The size of the FIFO in bytes.
> > @@ -245,6 +245,8 @@ struct msmsdcc_host {
> >  	struct mmc_command	*cmd_cmd;
> >  	u32			cmd_c;
> >  
> > +	unsigned int prog_scan;
> > +	unsigned int prog_enable;
> 
> These need to be bool, and you need to use true and false instead of 1
> and 0.

Will fix it in V2.

Thanks,
Sahitya.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.