Re: [RFC][PATCH] MMC: Use write timeout value as read from CSR

["Matt Fleming" <mattjfleming@googlemail.com>]

[-- Attachment #1: Type: text/plain, Size: 6361 bytes --]

2008/9/7 Pierre Ossman <drzeus-mmc@drzeus.cx>:
>
> This actually isn't fully correct for SD either as it only specifies
> that 250 ms is the upper bound on the timeout.
>
> I believe the proper way of solving this is to have mmc_spi respect the
> timeouts set in the request structure. Modify mmc_set_data_timeout() if
> necessary.
>

You're right, I changed the patch accordingly (I also fixed the read
timeout path).

>From 03b2d04b2e597ec073f2ff61224df1d3dee3b9de Mon Sep 17 00:00:00 2001
From: Matthew Fleming <matthew.fleming@imgtec.com>
Date: Mon, 8 Sep 2008 14:10:14 +0100
Subject: [PATCH] MMC: Use timeout values from CSR

Currently, the MMC/SD over SPI code has a hard-coded timeout value of
250ms on writes and 100ms on reads. This is correct for SDHC cards
and is specified in the spec, but it is not correct for MMC/SD cards.

For MMC cards the values that are read from the CSR should be used.
For SD cards the values from the CSR should be used as long as they
do not exceed 250ms for writes and 100ms for reads.

Signed-off-by: Matthew Fleming <matthew.fleming@imgtec.com>
---
 drivers/mmc/host/mmc_spi.c |   56 +++++++++++++++++++++++++++++++++----------
 1 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 7503b81..35f18f5 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -34,6 +34,7 @@

 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>             /* for R1_SPI_* bit values */
+#include <linux/mmc/card.h>

 #include <linux/spi/spi.h>
 #include <linux/spi/mmc_spi.h>
@@ -89,14 +90,12 @@
 #define MMC_SPI_BLOCKSIZE      512


-/* These fixed timeouts come from the latest SD specs, which say to ignore
- * the CSD values.  The R1B value is for card erase (e.g. the "I forgot the
+/*
+ * The R1B value is for card erase (e.g. the "I forgot the
  * card's password" scenario); it's mostly applied to STOP_TRANSMISSION after
  * reads which takes nowhere near that long.  Older cards may be able to use
  * shorter timeouts ... but why bother?
  */
-#define readblock_timeout      ktime_set(0, 100 * 1000 * 1000)
-#define writeblock_timeout     ktime_set(0, 250 * 1000 * 1000)
 #define r1b_timeout            ktime_set(3, 0)


@@ -214,15 +213,44 @@ mmc_spi_skip(struct mmc_spi_host *host, ktime_t
timeout, unsigned n, u8 byte)
        return -ETIMEDOUT;
 }

+/*
+ * Return the timeout value for this card.
+ */
+static ktime_t
+mmc_get_timeout(struct mmc_spi_host *host, struct mmc_data *data)
+{
+       unsigned int timeout_ns;
+
+       /* If card hasn't been initialised, use default timeouts. */
+       if (!host->mmc->card) {
+               if (data->flags & MMC_DATA_WRITE)
+                       timeout_ns = 250000000;
+               else
+                       timeout_ns = 100000000;
+       } else {
+               timeout_ns = data->timeout_ns;
+               timeout_ns += data->timeout_clks * 1000  * 1000 /
+                       (host->mmc->card->host->ios.clock);
+       }
+
+       /*
+        * The reason we're adding 0 and a nanosecond value here is because
+        * the timeout_ns value may be greater than one second, which
+        * ktime_set() does not handle properly. The correct logic to handle
+        * this overflow is in ktime_add_ns() however.
+        */
+       return ktime_add_ns(ktime_set(0, 0), timeout_ns);
+}
+
 static inline int
 mmc_spi_wait_unbusy(struct mmc_spi_host *host, ktime_t timeout)
 {
        return mmc_spi_skip(host, timeout, sizeof(host->data->status), 0);
 }

-static int mmc_spi_readtoken(struct mmc_spi_host *host)
+static int mmc_spi_readtoken(struct mmc_spi_host *host, ktime_t timeout)
 {
-       return mmc_spi_skip(host, readblock_timeout, 1, 0xff);
+       return mmc_spi_skip(host, timeout, 1, 0xff);
 }


@@ -605,7 +633,8 @@ mmc_spi_setup_data_message(
  * Return negative errno, else success.
  */
 static int
-mmc_spi_writeblock(struct mmc_spi_host *host, struct spi_transfer *t)
+mmc_spi_writeblock(struct mmc_spi_host *host, struct spi_transfer *t,
+       struct mmc_data *data)
 {
        struct spi_device       *spi = host->spi;
        int                     status, i;
@@ -673,7 +702,7 @@ mmc_spi_writeblock(struct mmc_spi_host *host,
struct spi_transfer *t)
                if (scratch->status[i] != 0)
                        return 0;
        }
-       return mmc_spi_wait_unbusy(host, writeblock_timeout);
+       return mmc_spi_wait_unbusy(host, mmc_get_timeout(host, data));
 }

 /*
@@ -693,7 +722,8 @@ mmc_spi_writeblock(struct mmc_spi_host *host,
struct spi_transfer *t)
  * STOP_TRANSMISSION command.
  */
 static int
-mmc_spi_readblock(struct mmc_spi_host *host, struct spi_transfer *t)
+mmc_spi_readblock(struct mmc_spi_host *host, struct spi_transfer *t,
+       struct mmc_data *data)
 {
        struct spi_device       *spi = host->spi;
        int                     status;
@@ -707,7 +737,7 @@ mmc_spi_readblock(struct mmc_spi_host *host,
struct spi_transfer *t)
                return status;
        status = scratch->status[0];
        if (status == 0xff || status == 0)
-               status = mmc_spi_readtoken(host);
+               status = mmc_spi_readtoken(host, mmc_get_timeout(host, data));

        if (status == SPI_TOKEN_SINGLE) {
                if (host->dma_dev) {
@@ -832,9 +862,9 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct
mmc_command *cmd,
                                t->len);

                        if (direction == DMA_TO_DEVICE)
-                               status = mmc_spi_writeblock(host, t);
+                               status = mmc_spi_writeblock(host, t, data);
                        else
-                               status = mmc_spi_readblock(host, t);
+                               status = mmc_spi_readblock(host, t, data);
                        if (status < 0)
                                break;

@@ -917,7 +947,7 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct
mmc_command *cmd,
                        if (scratch->status[tmp] != 0)
                                return;
                }
-               tmp = mmc_spi_wait_unbusy(host, writeblock_timeout);
+               tmp = mmc_spi_wait_unbusy(host, mmc_get_timeout(host, data));
                if (tmp < 0 && !data->error)
                        data->error = tmp;
        }
--
1.5.5.2

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-MMC-Use-timeout-values-from-CSR.patch --]
[-- Type: text/x-diff; name=0001-MMC-Use-timeout-values-from-CSR.patch, Size: 5163 bytes --]

From 03b2d04b2e597ec073f2ff61224df1d3dee3b9de Mon Sep 17 00:00:00 2001
From: Matthew Fleming <matthew.fleming@imgtec.com>
Date: Mon, 8 Sep 2008 14:10:14 +0100
Subject: [PATCH] MMC: Use timeout values from CSR

Currently, the MMC/SD over SPI code has a hard-coded timeout value of
250ms on writes and 100ms on reads. This is correct for SDHC cards
and is specified in the spec, but it is not correct for MMC/SD cards.

For MMC cards the values that are read from the CSR should be used.
For SD cards the values from the CSR should be used as long as they
do not exceed 250ms for writes and 100ms for reads.

Signed-off-by: Matthew Fleming <matthew.fleming@imgtec.com>
---
 drivers/mmc/host/mmc_spi.c |   56 +++++++++++++++++++++++++++++++++----------
 1 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 7503b81..35f18f5 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -34,6 +34,7 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>		/* for R1_SPI_* bit values */
+#include <linux/mmc/card.h>
 
 #include <linux/spi/spi.h>
 #include <linux/spi/mmc_spi.h>
@@ -89,14 +90,12 @@
 #define MMC_SPI_BLOCKSIZE	512
 
 
-/* These fixed timeouts come from the latest SD specs, which say to ignore
- * the CSD values.  The R1B value is for card erase (e.g. the "I forgot the
+/*
+ * The R1B value is for card erase (e.g. the "I forgot the
  * card's password" scenario); it's mostly applied to STOP_TRANSMISSION after
  * reads which takes nowhere near that long.  Older cards may be able to use
  * shorter timeouts ... but why bother?
  */
-#define readblock_timeout	ktime_set(0, 100 * 1000 * 1000)
-#define writeblock_timeout	ktime_set(0, 250 * 1000 * 1000)
 #define r1b_timeout		ktime_set(3, 0)
 
 
@@ -214,15 +213,44 @@ mmc_spi_skip(struct mmc_spi_host *host, ktime_t timeout, unsigned n, u8 byte)
 	return -ETIMEDOUT;
 }
 
+/*
+ * Return the timeout value for this card.
+ */
+static ktime_t
+mmc_get_timeout(struct mmc_spi_host *host, struct mmc_data *data)
+{
+	unsigned int timeout_ns;
+
+	/* If card hasn't been initialised, use default timeouts. */
+	if (!host->mmc->card) {
+		if (data->flags & MMC_DATA_WRITE)
+			timeout_ns = 250000000;
+		else
+			timeout_ns = 100000000;
+	} else {
+		timeout_ns = data->timeout_ns;
+		timeout_ns += data->timeout_clks * 1000  * 1000 /
+			(host->mmc->card->host->ios.clock);
+	}
+
+	/* 
+	 * The reason we're adding 0 and a nanosecond value here is because
+	 * the timeout_ns value may be greater than one second, which 
+	 * ktime_set() does not handle properly. The correct logic to handle
+	 * this overflow is in ktime_add_ns() however.
+	 */
+	return ktime_add_ns(ktime_set(0, 0), timeout_ns);
+}
+
 static inline int
 mmc_spi_wait_unbusy(struct mmc_spi_host *host, ktime_t timeout)
 {
 	return mmc_spi_skip(host, timeout, sizeof(host->data->status), 0);
 }
 
-static int mmc_spi_readtoken(struct mmc_spi_host *host)
+static int mmc_spi_readtoken(struct mmc_spi_host *host, ktime_t timeout)
 {
-	return mmc_spi_skip(host, readblock_timeout, 1, 0xff);
+	return mmc_spi_skip(host, timeout, 1, 0xff);
 }
 
 
@@ -605,7 +633,8 @@ mmc_spi_setup_data_message(
  * Return negative errno, else success.
  */
 static int
-mmc_spi_writeblock(struct mmc_spi_host *host, struct spi_transfer *t)
+mmc_spi_writeblock(struct mmc_spi_host *host, struct spi_transfer *t,
+	struct mmc_data *data)
 {
 	struct spi_device	*spi = host->spi;
 	int			status, i;
@@ -673,7 +702,7 @@ mmc_spi_writeblock(struct mmc_spi_host *host, struct spi_transfer *t)
 		if (scratch->status[i] != 0)
 			return 0;
 	}
-	return mmc_spi_wait_unbusy(host, writeblock_timeout);
+	return mmc_spi_wait_unbusy(host, mmc_get_timeout(host, data));
 }
 
 /*
@@ -693,7 +722,8 @@ mmc_spi_writeblock(struct mmc_spi_host *host, struct spi_transfer *t)
  * STOP_TRANSMISSION command.
  */
 static int
-mmc_spi_readblock(struct mmc_spi_host *host, struct spi_transfer *t)
+mmc_spi_readblock(struct mmc_spi_host *host, struct spi_transfer *t,
+	struct mmc_data *data)
 {
 	struct spi_device	*spi = host->spi;
 	int			status;
@@ -707,7 +737,7 @@ mmc_spi_readblock(struct mmc_spi_host *host, struct spi_transfer *t)
 		return status;
 	status = scratch->status[0];
 	if (status == 0xff || status == 0)
-		status = mmc_spi_readtoken(host);
+		status = mmc_spi_readtoken(host, mmc_get_timeout(host, data));
 
 	if (status == SPI_TOKEN_SINGLE) {
 		if (host->dma_dev) {
@@ -832,9 +862,9 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct mmc_command *cmd,
 				t->len);
 
 			if (direction == DMA_TO_DEVICE)
-				status = mmc_spi_writeblock(host, t);
+				status = mmc_spi_writeblock(host, t, data);
 			else
-				status = mmc_spi_readblock(host, t);
+				status = mmc_spi_readblock(host, t, data);
 			if (status < 0)
 				break;
 
@@ -917,7 +947,7 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct mmc_command *cmd,
 			if (scratch->status[tmp] != 0)
 				return;
 		}
-		tmp = mmc_spi_wait_unbusy(host, writeblock_timeout);
+		tmp = mmc_spi_wait_unbusy(host, mmc_get_timeout(host, data));
 		if (tmp < 0 && !data->error)
 			data->error = tmp;
 	}
-- 
1.5.5.2