[PATCH] [MMC] Fix SD timeout calculation

[PATCH] [MMC] Fix SD timeout calculation

[Pierre Ossman]

Secure Digital cards use a different algorithm to calculate the timeout
for data transfers. Using the MMC one works often, but not always.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
---

 drivers/mmc/mmc.c       |   15 +++++++++++++--
 drivers/mmc/mmc_block.c |   44 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 6201f30..bd15f99 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -913,6 +913,7 @@ static void mmc_read_scrs(struct mmc_hos
 	struct mmc_request mrq;
 	struct mmc_command cmd;
 	struct mmc_data data;
+	unsigned int timeout_us;
 
 	struct scatterlist sg;
 
@@ -948,8 +949,18 @@ static void mmc_read_scrs(struct mmc_hos
 
 		memset(&data, 0, sizeof(struct mmc_data));
 
-		data.timeout_ns = card->csd.tacc_ns * 10;
-		data.timeout_clks = card->csd.tacc_clks * 10;
+		data.timeout_ns = card->csd.tacc_ns * 100;
+		data.timeout_clks = card->csd.tacc_clks * 100;
+
+		timeout_us = data.timeout_ns / 1000;
+		timeout_us += data.timeout_clks * 1000 /
+			(host->ios.clock / 1000);
+
+		if (timeout_us > 100000) {
+			data.timeout_ns = 100000000;
+			data.timeout_clks = 0;
+		}
+
 		data.blksz_bits = 3;
 		data.blksz = 1 << 3;
 		data.blocks = 1;
diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
index 96049e2..69fa6bc 100644
--- a/drivers/mmc/mmc_block.c
+++ b/drivers/mmc/mmc_block.c
@@ -31,6 +31,7 @@ #include <linux/devfs_fs_kernel.h>
 #include <linux/mutex.h>
 
 #include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
 #include <linux/mmc/protocol.h>
 
 #include <asm/system.h>
@@ -172,8 +173,6 @@ static int mmc_blk_issue_rq(struct mmc_q
 
 		brq.cmd.arg = req->sector << 9;
 		brq.cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
-		brq.data.timeout_ns = card->csd.tacc_ns * 10;
-		brq.data.timeout_clks = card->csd.tacc_clks * 10;
 		brq.data.blksz_bits = md->block_bits;
 		brq.data.blksz = 1 << md->block_bits;
 		brq.data.blocks = req->nr_sectors >> (md->block_bits - 9);
@@ -181,6 +180,41 @@ static int mmc_blk_issue_rq(struct mmc_q
 		brq.stop.arg = 0;
 		brq.stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
 
+		brq.data.timeout_ns = card->csd.tacc_ns * 10;
+		brq.data.timeout_clks = card->csd.tacc_clks * 10;
+
+		/*
+		 * Scale up the timeout by the r2w factor
+		 */
+		if (rq_data_dir(req) == WRITE) {
+			brq.data.timeout_ns <<= card->csd.r2w_factor;
+			brq.data.timeout_clks <<= card->csd.r2w_factor;
+		}
+
+		/*
+		 * SD cards use a 100 multiplier and has a upper limit
+		 */
+		if (mmc_card_sd(card)) {
+			unsigned int limit_us, timeout_us;
+
+			brq.data.timeout_ns *= 10;
+			brq.data.timeout_clks *= 10;
+
+			if (rq_data_dir(req) == READ)
+				limit_us = 100000;
+			else
+				limit_us = 250000;
+
+			timeout_us = brq.data.timeout_ns / 1000;
+			timeout_us += brq.data.timeout_clks * 1000 /
+				(card->host->ios.clock / 1000);
+
+			if (timeout_us > limit_us) {
+				brq.data.timeout_ns = limit_us * 1000;
+				brq.data.timeout_clks = 0;
+			}
+		}
+
 		if (rq_data_dir(req) == READ) {
 			brq.cmd.opcode = brq.data.blocks > 1 ? MMC_READ_MULTIPLE_BLOCK : MMC_READ_SINGLE_BLOCK;
 			brq.data.flags |= MMC_DATA_READ;
@@ -188,12 +222,6 @@ static int mmc_blk_issue_rq(struct mmc_q
 			brq.cmd.opcode = MMC_WRITE_BLOCK;
 			brq.data.flags |= MMC_DATA_WRITE;
 			brq.data.blocks = 1;
-
-			/*
-			 * Scale up the timeout by the r2w factor
-			 */
-			brq.data.timeout_ns <<= card->csd.r2w_factor;
-			brq.data.timeout_clks <<= card->csd.r2w_factor;
 		}
 
 		if (brq.data.blocks > 1) {

Re: [PATCH] [MMC] Fix SD timeout calculation

[Pierre Ossman]

I have a bunch more in the pipeline, just as soon as your tree gets
merged with Linus' again.

Rgds
Pierre

Re: [MMC] Fix SD timeout calculation

[Russell King]

On Sun, Jun 18, 2006 at 02:34:37PM +0200, Pierre Ossman wrote:
> Secure Digital cards use a different algorithm to calculate the timeout
> for data transfers. Using the MMC one works often, but not always.

Applied, thanks.

I'm wondering about this cleanup though - both the timeout calculations
appear to be identical, so making this a library function seems to be
the right way to go... unless you know different?

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index c0c7ef2..74eaaee 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -247,6 +247,55 @@ int mmc_wait_for_app_cmd(struct mmc_host
 
 EXPORT_SYMBOL(mmc_wait_for_app_cmd);
 
+/**
+ *	mmc_set_data_timeout - set the timeout for a data command
+ *	@data: data phase for command
+ *	@card: the MMC card associated with the data transfer
+ *	@write: flag to differentiate reads from writes
+ */
+void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card,
+			  int write)
+{
+	unsigned int mult;
+
+	/*
+	 * SD cards use a 100 multiplier rather than 10
+	 */
+	mult = mmc_card_sd(card) ? 100 : 10;
+
+	/*
+	 * Scale up the multiplier (and therefore the timeout) by
+	 * the r2w factor for writes.
+	 */
+	if (write)
+		mult <<= card->csd.r2w_factor;
+
+	data->timeout_ns = card->csd.tacc_ns * mult;
+	data->timeout_clks = card->csd.tacc_clks * mult;
+
+	/*
+	 * SD cards also have an upper limit on the timeout.
+	 */
+	if (mmc_card_sd(card)) {
+		unsigned int timeout_us, limit_us;
+
+		timeout_us = data->timeout_ns / 1000;
+		timeout_us += data->timeout_clks * 1000 /
+			(card->host->ios.clock / 1000);
+
+		if (write)
+			limit_us = 250000;
+		else
+			limit_us = 100000;
+
+		if (timeout_us > limit_us) {
+			data->timeout_ns = limit_us * 1000;
+			data->timeout_clks = 0;
+		}
+	}
+}
+EXPORT_SYMBOL(mmc_set_data_timeout);
+
 static int mmc_select_card(struct mmc_host *host, struct mmc_card *card);
 
 /**
@@ -908,12 +957,9 @@ static void mmc_read_scrs(struct mmc_hos
 {
 	int err;
 	struct mmc_card *card;
-
 	struct mmc_request mrq;
 	struct mmc_command cmd;
 	struct mmc_data data;
-	unsigned int timeout_us;
-
 	struct scatterlist sg;
 
 	list_for_each_entry(card, &host->cards, node) {
@@ -948,17 +994,7 @@ static void mmc_read_scrs(struct mmc_hos
 
 		memset(&data, 0, sizeof(struct mmc_data));
 
-		data.timeout_ns = card->csd.tacc_ns * 100;
-		data.timeout_clks = card->csd.tacc_clks * 100;
-
-		timeout_us = data.timeout_ns / 1000;
-		timeout_us += data.timeout_clks * 1000 /
-			(host->ios.clock / 1000);
-
-		if (timeout_us > 100000) {
-			data.timeout_ns = 100000000;
-			data.timeout_clks = 0;
-		}
+		mmc_set_data_timeout(&data, card, 0);
 
 		data.blksz_bits = 3;
 		data.blksz = 1 << 3;
diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
index 515fb22..d6fcc46 100644
--- a/drivers/mmc/mmc_block.c
+++ b/drivers/mmc/mmc_block.c
@@ -179,40 +179,7 @@ static int mmc_blk_issue_rq(struct mmc_q
 		brq.stop.arg = 0;
 		brq.stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
 
-		brq.data.timeout_ns = card->csd.tacc_ns * 10;
-		brq.data.timeout_clks = card->csd.tacc_clks * 10;
-
-		/*
-		 * Scale up the timeout by the r2w factor
-		 */
-		if (rq_data_dir(req) == WRITE) {
-			brq.data.timeout_ns <<= card->csd.r2w_factor;
-			brq.data.timeout_clks <<= card->csd.r2w_factor;
-		}
-
-		/*
-		 * SD cards use a 100 multiplier and has a upper limit
-		 */
-		if (mmc_card_sd(card)) {
-			unsigned int limit_us, timeout_us;
-
-			brq.data.timeout_ns *= 10;
-			brq.data.timeout_clks *= 10;
-
-			if (rq_data_dir(req) == READ)
-				limit_us = 100000;
-			else
-				limit_us = 250000;
-
-			timeout_us = brq.data.timeout_ns / 1000;
-			timeout_us += brq.data.timeout_clks * 1000 /
-				(card->host->ios.clock / 1000);
-
-			if (timeout_us > limit_us) {
-				brq.data.timeout_ns = limit_us * 1000;
-				brq.data.timeout_clks = 0;
-			}
-		}
+		mmc_set_data_timeout(&brq.data, card, rq_data_dir(req) != READ);
 
 		if (rq_data_dir(req) == READ) {
 			brq.cmd.opcode = brq.data.blocks > 1 ? MMC_READ_MULTIPLE_BLOCK : MMC_READ_SINGLE_BLOCK;
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 03a14a3..627e2c0 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -105,6 +105,8 @@ extern int mmc_wait_for_cmd(struct mmc_h
 extern int mmc_wait_for_app_cmd(struct mmc_host *, unsigned int,
 	struct mmc_command *, int);
 
+extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *, int);
+
 extern int __mmc_claim_host(struct mmc_host *host, struct mmc_card *card);
 
 static inline void mmc_claim_host(struct mmc_host *host)

Re: [MMC] Fix SD timeout calculation

[Pierre Ossman]

Russell King wrote:
> On Sun, Jun 18, 2006 at 02:34:37PM +0200, Pierre Ossman wrote:
>> Secure Digital cards use a different algorithm to calculate the timeout
>> for data transfers. Using the MMC one works often, but not always.
> 
> Applied, thanks.
> 
> I'm wondering about this cleanup though - both the timeout calculations
> appear to be identical, so making this a library function seems to be
> the right way to go... unless you know different?
> 

Shouldn't be a problem.

Acked-by: Pierre Ossman <drzeus@drzeus.cx>