Skip to contentSkip to conversation

mmc: msm_sdcc: Fix possible circular locking dependency warning

PATCH Superseded Sahitya Tummala 2 participants 10 messages Dec 7, 2010 – Dec 8, 2010

Subsystems (2)
Arm Qualcomm mailing list MMC, SD and SDIO

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Sahitya Tummala

author
#0 lore raw

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

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 <[email protected]>
---
 drivers/mmc/host/msm_sdcc.c |   49 +++++++++++++++++++++++++++++--------------
 drivers/mmc/host/msm_sdcc.h |    3 ++
 2 files changed, 36 insertions(+), 16 deletions(-)
drivers/mmc/host/msm_sdcc.c +33 -16
index 1290d14..b147971 100644
@@ -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
 	 */
drivers/mmc/host/msm_sdcc.h +3 -0
index ff2b0f7..996990d 100644
@@ -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;
Signature · 5 lines
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.
author
#1 lore raw

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

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 <[email protected]>
---
 drivers/mmc/host/msm_sdcc.c |   28 +++++++++++++++++++++++++---
 drivers/mmc/host/msm_sdcc.h |    4 +++-
 2 files changed, 28 insertions(+), 4 deletions(-)
drivers/mmc/host/msm_sdcc.c +25 -3
index b147971..b64775c 100644
@@ -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);
 	}
drivers/mmc/host/msm_sdcc.h +3 -1
index 996990d..ac79ed9 100644
@@ -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
Signature · 5 lines
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.
#2 lore raw

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

Quoted 3 lines · 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.

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

drivers/mmc/host/msm_sdcc.c msmsdcc_handle_irq_data(struct msmsdcc_host *host, u32 status, · line 758

Quoted 3 lines
drivers/mmc/host/msm_sdcc.c +1 -1
 	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.

Quoted 17 lines
drivers/mmc/host/msm_sdcc.c +0 -0
 		msmsdcc_do_cmdirq(host, status);
 	}
drivers/mmc/host/msm_sdcc.h +1 -1
index 996990d..ac79ed9 100644
@@ -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.

drivers/mmc/host/msm_sdcc.h struct msmsdcc_host { · line 249

Quoted 5 lines
drivers/mmc/host/msm_sdcc.h +2 -0
@@ -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

Signature · 2 lines
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
author
#3 lore raw

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

Hi Daniel,

Quoted 6 lines · Daniel Walker
Quoted 3 lines · 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.

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.

Quoted 5 lines

drivers/mmc/host/msm_sdcc.c msmsdcc_handle_irq_data(struct msmsdcc_host *host, u32 status, · line 758

Quoted 3 lines
drivers/mmc/host/msm_sdcc.c +1 -1
 	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.

Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/msm_sdcc.c +0 -0
 		msmsdcc_do_cmdirq(host, status);
 	}
drivers/mmc/host/msm_sdcc.h +1 -1
index 996990d..ac79ed9 100644
@@ -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;
Quoted 5 lines

drivers/mmc/host/msm_sdcc.h struct msmsdcc_host { · line 249

Quoted 2 lines
drivers/mmc/host/msm_sdcc.h +2 -0
+	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.

author
#4 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/mmc/host/msm_sdcc.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
drivers/mmc/host/msm_sdcc.c +39 -0
index b64775c..b5cca8c 100644
@@ -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)
Signature · 5 lines
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.
#5 lore raw

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

Quoted 2 lines · Sahitya Tummala

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

Signature · 2 lines
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
author
#6 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/mmc/host/msm_sdcc.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
drivers/mmc/host/msm_sdcc.c +6 -2
index b5cca8c..355d799 100644
@@ -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;
Signature · 5 lines
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.
author
#7 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/mmc/host/msm_sdcc.c |   15 ++++-----------
 drivers/mmc/host/msm_sdcc.h |    1 -
 2 files changed, 4 insertions(+), 12 deletions(-)
drivers/mmc/host/msm_sdcc.c +4 -11
index 355d799..23b6f57 100644
@@ -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)
drivers/mmc/host/msm_sdcc.h +0 -1
index ac79ed9..9a14144 100644
@@ -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;
 };
Signature · 5 lines
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.
#8 lore raw
In reply to Sahitya Tummala
Quoted 6 lines · 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.

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

Daniel

Signature · 2 lines
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
author
#9 lore raw

Hi Daniel,

Quoted 2 lines · Daniel Walker

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.