Skip to contentSkip to conversation

mmc: rtsx: add new cmd type handle and modify error handle

PATCH Superseded [email protected] 2 participants 6 messages Mar 25, 2014 – Mar 26, 2014 MMC, SD and SDIO

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover mmc: rtsx: add new cmd type handle and modify error handle

From: Micky Ching <[email protected]>

Add new command type(R1 without CRC) handle, without this patch mmc card initialize will be failed.

Using a more careful handle in request timeout, this would improve error recover capability. Debug info is printed using non DMA mode, this would help print more accurately for DMA command failed. Smatch warning was removed.

Micky Ching (2):
mmc: rtsx: add R1-no-CRC mmc command type handle
mmc: rtsx: modify error handle and remove smatch warnings

 drivers/mmc/host/rtsx_pci_sdmmc.c |  121 ++++++++++++++++++++-----------------
 1 file changed, 67 insertions(+), 54 deletions(-)

--
1.7.9.5

#1 lore raw

PATCH 1/2 mmc: rtsx: add R1-no-CRC mmc command type handle

From: Micky Ching <[email protected]>

commit a27fbf2f067b0cd6f172c8b696b9a44c58bfaa7a
<mmc: add ignorance case for CMD13 CRC error>
produced a cmd.flags unhandled in realtek pci host driver. This will make MMC card failed initialize, this patch is used to handle the new cmd.flags condition and MMC card can be used.

---
 drivers/mmc/host/rtsx_pci_sdmmc.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/mmc/host/rtsx_pci_sdmmc.c +3 -0
index 5fb994f..0d8904a 100644
@@ -346,6 +346,9 @@ static void sd_send_cmd(struct realtek_pci_sdmmc *host, struct mmc_command *cmd)
 	case MMC_RSP_R1:
 		rsp_type = SD_RSP_TYPE_R1;
 		break;
+	case MMC_RSP_R1 & ~MMC_RSP_CRC:
+		rsp_type = SD_RSP_TYPE_R1 | SD_NO_CHECK_CRC7;
+		break;
 	case MMC_RSP_R1B:
 		rsp_type = SD_RSP_TYPE_R1b;
 		break;
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 2/2 mmc: rtsx: modify error handle and remove smatch warnings

In reply to [email protected]

From: Micky Ching <[email protected]>

Using non-DMA dump-regs, which would be more exactly for DMA transfer failed.

More careful handle when cmd/data timeout, add stop(CMD12) cmd before go to finish request when multi-rw timeout.

Remove some static checher warings.
on commit: <mmc: rtsx: add support for pre_req and post_req> drivers/mmc/host/rtsx_pci_sdmmc.c:194 sd_finish_request()
error: we previously assumed 'mrq' could be null (see line 158) drivers/mmc/host/rtsx_pci_sdmmc.c:504 sd_get_rsp()
error: we previously assumed 'cmd' could be null (see line 434) drivers/mmc/host/rtsx_pci_sdmmc.c:525 sd_pre_dma_transfer()
warn: we tested 'next' before and it was 'false'

---
 drivers/mmc/host/rtsx_pci_sdmmc.c |  118 ++++++++++++++++++++-----------------
 1 file changed, 64 insertions(+), 54 deletions(-)
drivers/mmc/host/rtsx_pci_sdmmc.c +64 -54
index 0d8904a..d00a94e 100644
@@ -81,25 +81,23 @@ static inline void sd_clear_error(struct realtek_pci_sdmmc *host)
 }
 
 #ifdef DEBUG
+static inline void sd_print_reg(struct realtek_pci_sdmmc *host, u16 reg)
+{
+	u8 val;
+	if (rtsx_pci_read_register(host->pcr, reg, &val) < 0)
+		dev_dbg(sdmmc_dev(host), "read 0x%04x failed\n", reg);
+	else
+		dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", reg, val);
+}
+
 static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
 {
-	struct rtsx_pcr *pcr = host->pcr;
 	u16 i;
-	u8 *ptr;
-
-	/* Print SD host internal registers */
-	rtsx_pci_init_cmd(pcr);
-	for (i = 0xFDA0; i <= 0xFDAE; i++)
-		rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
-	for (i = 0xFD52; i <= 0xFD69; i++)
-		rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
-	rtsx_pci_send_cmd(pcr, 100);
 
-	ptr = rtsx_pci_get_cmd_data(pcr);
 	for (i = 0xFDA0; i <= 0xFDAE; i++)
-		dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
+		sd_print_reg(host, i);
 	for (i = 0xFD52; i <= 0xFD69; i++)
-		dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
+		sd_print_reg(host, i);
 }
 #else
 #define sd_print_debug_regs(host)
@@ -125,19 +123,27 @@ static void sd_request_timeout(unsigned long host_addr)
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (!host->mrq) {
-		dev_err(sdmmc_dev(host), "error: no request exist\n");
-		goto out;
+		dev_err(sdmmc_dev(host), "error: request not exist\n");
+		spin_unlock_irqrestore(&host->lock, flags);
+		return;
 	}
 
-	if (host->cmd)
+	if (host->cmd && host->data)
+		dev_err(sdmmc_dev(host), "error: cmd and data conflict\n");
+
+	if (host->cmd) {
 		host->cmd->error = -ETIMEDOUT;
-	if (host->data)
-		host->data->error = -ETIMEDOUT;
+		dev_dbg(sdmmc_dev(host), "timeout for cmd %d\n",
+			host->cmd->opcode);
+		tasklet_schedule(&host->cmd_tasklet);
+	}
 
-	dev_dbg(sdmmc_dev(host), "timeout for request\n");
+	if (host->data) {
+		host->data->error = -ETIMEDOUT;
+		dev_dbg(sdmmc_dev(host), "timeout for data transfer\n");
+		tasklet_schedule(&host->data_tasklet);
+	}
 
-out:
-	tasklet_schedule(&host->finish_tasklet);
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
@@ -157,7 +163,8 @@ static void sd_finish_request(unsigned long host_addr)
 	mrq = host->mrq;
 	if (!mrq) {
 		dev_err(sdmmc_dev(host), "error: no request need finish\n");
-		goto out;
+		spin_unlock_irqrestore(&host->lock, flags);
+		return;
 	}
 
 	cmd = mrq->cmd;
@@ -167,11 +174,6 @@ static void sd_finish_request(unsigned long host_addr)
 		(mrq->stop && mrq->stop->error) ||
 		(cmd && cmd->error) || (data && data->error);
 
-	if (any_error) {
-		rtsx_pci_stop_cmd(pcr);
-		sd_clear_error(host);
-	}
-
 	if (data) {
 		if (any_error)
 			data->bytes_xfered = 0;
@@ -188,7 +190,6 @@ static void sd_finish_request(unsigned long host_addr)
 	host->cmd = NULL;
 	host->data = NULL;
 
-out:
 	spin_unlock_irqrestore(&host->lock, flags);
 	mutex_unlock(&pcr->pcr_mutex);
 	mmc_request_done(host->mmc, mrq);
@@ -373,8 +374,11 @@ static void sd_send_cmd(struct realtek_pci_sdmmc *host, struct mmc_command *cmd)
 	if (cmd->opcode == SD_SWITCH_VOLTAGE) {
 		err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
 				0xFF, SD_CLK_TOGGLE_EN);
-		if (err < 0)
+		if (err < 0) {
+			rtsx_pci_write_register(pcr, SD_BUS_STAT,
+				SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
 			goto out;
+		}
 	}
 
 	rtsx_pci_init_cmd(pcr);
@@ -436,7 +440,8 @@ static void sd_get_rsp(unsigned long host_addr)
 
 	if (!cmd) {
 		dev_err(sdmmc_dev(host), "error: cmd not exist\n");
-		goto out;
+		spin_unlock_irqrestore(&host->lock, flags);
+		return;
 	}
 
 	spin_lock(&pcr->lock);
@@ -446,16 +451,18 @@ static void sd_get_rsp(unsigned long host_addr)
 		err = -EINVAL;
 	spin_unlock(&pcr->lock);
 
-	if (err < 0)
+	if (err < 0) {
+		rtsx_pci_stop_cmd(host->pcr);
+		sd_print_debug_regs(host);
+		sd_clear_error(host);
 		goto out;
+	}
 
 	rsp_type = host->rsp_type;
 	stat_idx = host->rsp_len;
 
-	if (rsp_type == SD_RSP_TYPE_R0) {
-		err = 0;
+	if (rsp_type == SD_RSP_TYPE_R0)
 		goto out;
-	}
 
 	/* Eliminate returned value of CHECK_REG_CMD */
 	ptr = rtsx_pci_get_cmd_data(pcr) + 1;
@@ -498,14 +505,19 @@ static void sd_get_rsp(unsigned long host_addr)
 		goto out;
 
 	if (cmd->data) {
-		sd_start_multi_rw(host, host->mrq);
+		err = sd_start_multi_rw(host, host->mrq);
+		if (err) {
+			cmd->data->error = err;
+			dev_err(sdmmc_dev(host),
+				"error: start data transfer failed\n");
+			tasklet_schedule(&host->data_tasklet);
+		}
 		spin_unlock_irqrestore(&host->lock, flags);
 		return;
 	}
 
 out:
 	cmd->error = err;
-
 	tasklet_schedule(&host->finish_tasklet);
 	spin_unlock_irqrestore(&host->lock, flags);
 }
@@ -525,7 +537,7 @@ static int sd_pre_dma_transfer(struct realtek_pci_sdmmc *host,
 		data->host_cookie = 0;
 	}
 
-	if (next || (!next && data->host_cookie != host->next_data.cookie))
+	if (next || data->host_cookie != host->next_data.cookie)
 		sg_count = rtsx_pci_dma_map_sg(pcr,
 				data->sg, data->sg_len, read);
 	else
@@ -580,7 +592,6 @@ static int sd_start_multi_rw(struct realtek_pci_sdmmc *host,
 	int uhs = mmc_card_uhs(card);
 	int read = data->flags & MMC_DATA_READ;
 	u8 cfg2, trans_mode;
-	int err;
 	size_t data_len = data->blksz * data->blocks;
 
 	if (host->data)
@@ -641,12 +652,7 @@ static int sd_start_multi_rw(struct realtek_pci_sdmmc *host,
 	mod_timer(&host->timer, jiffies + 10 * HZ);
 	rtsx_pci_send_cmd_no_wait(pcr);
 
-	err = rtsx_pci_dma_transfer(pcr, data->sg, host->sg_count, read);
-	if (err < 0) {
-		data->error = err;
-		tasklet_schedule(&host->finish_tasklet);
-	}
-	return 0;
+	return rtsx_pci_dma_transfer(pcr, data->sg, host->sg_count, read);
 }
 
 static void sd_finish_multi_rw(unsigned long host_addr)
@@ -660,8 +666,9 @@ static void sd_finish_multi_rw(unsigned long host_addr)
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (!host->data) {
-		dev_err(sdmmc_dev(host), "error: no data exist\n");
-		goto out;
+		dev_err(sdmmc_dev(host), "error: data not exist\n");
+		spin_unlock_irqrestore(&host->lock, flags);
+		return;
 	}
 
 	data = host->data;
@@ -672,19 +679,22 @@ static void sd_finish_multi_rw(unsigned long host_addr)
 	else if (pcr->trans_result != TRANS_RESULT_OK)
 		err = -EINVAL;
 
-	if (err < 0) {
+	if (err < 0)
 		data->error = err;
-		goto out;
+
+	if (data->error) {
+		rtsx_pci_stop_cmd(host->pcr);
+		sd_print_debug_regs(host);
+		sd_clear_error(host);
+		dev_dbg(sdmmc_dev(host), "data transfer failed %d\n",
+			data->error);
 	}
 
-	if (!host->mrq->sbc && data->stop) {
+	if (!host->mrq->sbc && data->stop)
 		sd_send_cmd(host, data->stop);
-		spin_unlock_irqrestore(&host->lock, flags);
-		return;
-	}
+	else
+		tasklet_schedule(&host->finish_tasklet);
 
-out:
-	tasklet_schedule(&host->finish_tasklet);
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
Signature · 1 line
1.7.9.5
#3 lore raw
In reply to [email protected]
Quoted 9 lines · On 25 March 2014 10:47, <[email protected]> wrote:

From: Micky Ching <[email protected]>

Add new command type(R1 without CRC) handle, without this patch mmc card initialize will be failed.

Using a more careful handle in request timeout, this would improve error recover capability. Debug info is printed using non DMA mode, this would help print more accurately for DMA command failed. Smatch warning was removed.

Hi Micky,

Seems like this patchset has been around for some time, sorry for the delay.

I don't have any objections to it getting merged, but you need to run checkpatch of your patches. Patch2 has a warning you should fix.

Kind regards
Ulf Hansson

Quoted 10 lines

Micky Ching (2):
mmc: rtsx: add R1-no-CRC mmc command type handle
mmc: rtsx: modify error handle and remove smatch warnings

 drivers/mmc/host/rtsx_pci_sdmmc.c |  121 ++++++++++++++++++++-----------------
 1 file changed, 67 insertions(+), 54 deletions(-)

--
1.7.9.5

author
#4 lore raw

Re: [PATCH 0/2] mmc: rtsx: add new cmd type handle and modify errorhandle

Hi Ulf,

Quoted 10 lines · Ulf Hansson
Quoted 9 lines · On 25 March 2014 10:47, <[email protected]> wrote:

From: Micky Ching <[email protected]>

Add new command type(R1 without CRC) handle, without this patch mmc card initialize will be failed.

Using a more careful handle in request timeout, this would improve error recover capability. Debug info is printed using non DMA mode, this would help print more accurately for DMA command failed. Smatch warning was removed.

Quoted 6 lines

Hi Micky,

Seems like this patchset has been around for some time, sorry for the delay.

I don't have any objections to it getting merged, but you need to run checkpatch of your patches. Patch2 has a warning you should fix.

Patch2 is based on <mmc: rtsx: fix bug and support nonblocking request>
see: https://lkml.org/lkml/2014/2/17/270
checkpatch.pl seems ok, is there any thing I need to check?

Best Regards.
micky.

Quoted 15 lines

Kind regards
Ulf Hansson

Quoted 9 lines

Micky Ching (2):
mmc: rtsx: add R1-no-CRC mmc command type handle
mmc: rtsx: modify error handle and remove smatch warnings

  drivers/mmc/host/rtsx_pci_sdmmc.c |  121 ++++++++++++++++++++-----------------
  1 file changed, 67 insertions(+), 54 deletions(-)

--
1.7.9.5

.

#5 lore raw

Re: [PATCH 0/2] mmc: rtsx: add new cmd type handle and modify errorhandle

Quoted 21 lines · micky

Hi Ulf,

Quoted 18 lines · Ulf Hansson
Quoted 10 lines · On 25 March 2014 10:47, <[email protected]> wrote:

From: Micky Ching <[email protected]>

Add new command type(R1 without CRC) handle, without this patch mmc card initialize will be failed.

Using a more careful handle in request timeout, this would improve error recover capability. Debug info is printed using non DMA mode, this would help print more accurately for DMA command failed. Smatch warning was removed.

Hi Micky,

Seems like this patchset has been around for some time, sorry for the delay.

Quoted 6 lines
Quoted 2 lines

I don't have any objections to it getting merged, but you need to run checkpatch of your patches. Patch2 has a warning you should fix.

Patch2 is based on <mmc: rtsx: fix bug and support nonblocking request>
see: https://lkml.org/lkml/2014/2/17/270
checkpatch.pl seems ok, is there any thing I need to check?

It seems like the checkpatch script has been recently patched. When I ran the script for your patch I used the latest from Stephen Rothwell's linux-next tree. Please do the same.

Kind regards
Ulf Hansson

Quoted 22 lines

Best Regards.
micky.

Quoted 18 lines

Kind regards
Ulf Hansson

Quoted 10 lines

Micky Ching (2):
mmc: rtsx: add R1-no-CRC mmc command type handle
mmc: rtsx: modify error handle and remove smatch warnings

  drivers/mmc/host/rtsx_pci_sdmmc.c |  121
++++++++++++++++++++-----------------
  1 file changed, 67 insertions(+), 54 deletions(-)

--
1.7.9.5

.