Skip to contentSkip to conversation

set timeout control reg for such SDHCI host

PATCH In review Chuanxiao Dong 2 participants 5 messages Dec 2, 2010 – Dec 5, 2010

Subsystems (2)
MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver
lore
Share & tools
lore

Chuanxiao Dong

author
#0 lore raw

PATCH v3 2/3 set timeout control reg for such SDHCI host

Quoted 1 line

From b1cc696f0fbb0c48f024359977624a862ece93f4 Mon Sep 17 00:00:00 2001

From: Chuanxiao Dong <[email protected]>
Date: Thu, 2 Dec 2010 15:53:37 +0800
Subject: [PATCH 2/3] set timeout control reg for SDHCI host when sending erase cmd

Since if erasing needs longer time than the timeout host can wait, host will generate a timeout interrupt and this will make erasing failed. So before starting erasing, set the timeout control register value to be the maximum one which is long enough for SDHCI host to wait.

Signed-off-by Chuanxiao Dong <[email protected]>
---
 drivers/mmc/core/core.c  |    4 ++++
 drivers/mmc/host/sdhci.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)
drivers/mmc/core/core.c +4 -0
index 9fe16c2..fbac265 100644
@@ -170,6 +170,9 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 
 	mrq->cmd->error = 0;
 	mrq->cmd->mrq = mrq;
+	if (mrq->cmd->opcode != MMC_ERASE)
+		mrq->cmd->erase_timeout = 0;
+
 	if (mrq->data) {
 		BUG_ON(mrq->data->blksz > host->max_blk_size);
 		BUG_ON(mrq->data->blocks > host->max_blk_count);

@@ -190,6 +193,7 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 			mrq->data->stop = mrq->stop;
 			mrq->stop->error = 0;
 			mrq->stop->mrq = mrq;
+			mrq->stop->erase_timeout = 0;
 		}
 	}
 	mmc_host_clk_ungate(host);
drivers/mmc/host/sdhci.c +6 -0
index b4448a9..de9685d 100644
@@ -949,6 +949,12 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	if (cmd->data)
 		flags |= SDHCI_CMD_DATA;
 
+	if (host->quirks & SDHCI_QUIRK_FORCE_ERASE_SINGLE) {
+		/* Set the timeout to be the maximum value */
+		if (cmd->erase_timeout)
+			sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
+	}
+
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
 }
Signature · 1 line
1.6.6.1
#1 lore raw

drivers/mmc/host/sdhci.c static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd) · line 958

Quoted 7 lines · Chuanxiao Dong
drivers/mmc/host/sdhci.c +6 -0
+	if (host->quirks & SDHCI_QUIRK_FORCE_ERASE_SINGLE) {
+		/* Set the timeout to be the maximum value */
+		if (cmd->erase_timeout)
+			sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
+	}
+
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);

Hmm, this looks like another argument for Philip's idea to always use the maximum timeout value and skip the quirks related to it?

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#2 lore raw
Quoted 14 lines · Chuanxiao Dong
Quoted 9 lines
drivers/mmc/host/sdhci.c +6 -0
+	if (host->quirks & SDHCI_QUIRK_FORCE_ERASE_SINGLE) {
+		/* Set the timeout to be the maximum value */
+		if (cmd->erase_timeout)
+			sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
+	}
+
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags),

SDHCI_COMMAND);

Hmm, this looks like another argument for Philip's idea to always use the maximum timeout value and skip the quirks related to it?

Yes, if always using the maximum timeout value is OK for other command, the patch2 can be removed I think.
The new added quirk in the serials patches is used to set the limitation of request queue, not only just to set the timeout control reg. Even the timeout value was set to be 0xE (the maximum value), erasing too many sectors can still be failed since the timeout time was still not longer enough. So the count of erased sectors passed down by request queue should be reduced by using this quirk.

#3 lore raw
Quoted 15 lines · Chuanxiao
Quoted 14 lines · Chuanxiao Dong
Quoted 9 lines
drivers/mmc/host/sdhci.c +6 -0
+	if (host->quirks & SDHCI_QUIRK_FORCE_ERASE_SINGLE) {
+		/* Set the timeout to be the maximum value */
+		if (cmd->erase_timeout)
+			sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
+	}
+
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags),

SDHCI_COMMAND);

Hmm, this looks like another argument for Philip's idea to always use the maximum timeout value and skip the quirks related to it?

Quoted 4 lines

Yes, if always using the maximum timeout value is OK for other command, the patch2 can be removed I think.
The new added quirk in the serials patches is used to set the limitation of request queue, not only just to set the timeout control reg. Even the timeout value was set to be 0xE (the maximum value), erasing too many sectors can still be failed since the timeout time was still not longer enough. So the count of erased sectors passed down by request queue should be reduced by using this quirk.

Yes, I think I understand the issue. Have you tried adding a callback, so we don't have the options between MAX_UINT and '1' but rather MAX_UINT and return value of the callback?

Regards,

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#4 lore raw
Quoted 30 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Wolfram Sang
Sent: Sunday, December 05, 2010 5:34 AM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3 2/3]set timeout control reg for such SDHCI host

Quoted 15 lines · Chuanxiao
Quoted 14 lines · Chuanxiao Dong
Quoted 9 lines
drivers/mmc/host/sdhci.c +6 -0
+	if (host->quirks & SDHCI_QUIRK_FORCE_ERASE_SINGLE) {
+		/* Set the timeout to be the maximum value */
+		if (cmd->erase_timeout)
+			sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
+	}
+
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags),

SDHCI_COMMAND);

Hmm, this looks like another argument for Philip's idea to always use the maximum timeout value and skip the quirks related to it?

Quoted 1 line

Yes, if always using the maximum timeout value is OK for other command, the

patch2 can be removed I think.

Quoted 1 line

The new added quirk in the serials patches is used to set the limitation of request

queue, not only just to set the timeout control reg.

Quoted 8 lines
Quoted 1 line

Even the timeout value was set to be 0xE (the maximum value), erasing too many

sectors can still be failed since the timeout time was still not longer enough.

Quoted 1 line

So the count of erased sectors passed down by request queue should be reduced

by using this quirk.

Yes, I think I understand the issue. Have you tried adding a callback, so we don't have the options between MAX_UINT and '1' but rather MAX_UINT and return value of the callback?

I added a function to calculate a more suitable max_discard_sectors value for SDHCI host controller in version 1 patches. This value was calculated by the timeout time host controller can wait and the erase timeout. Is that the callback you mean? In this version, I added a function called mmc_set_discard_limit to detect whether the host has a new cap called MMC_CAP_ERASE_SINGLE. They can be found in patch1. If host has been set MMC_CAP_ERASE_SINGLE cap, just returns '1'. And if not, return MAX_UINT. I used this way in version 3 patches instead of the calculated way since I thought using an easier way to work around a hardware issue was better. Any suggestion about solving this kind of problem?

Thanks
Chuanxiao