[2/3] mmc: host: omap_hsmmc: use generic_cmd6_time to program timeout value for CMD6

Message ID 1485771118-13748-3-git-send-email-rk@ti.com
State New
Headers show
Series
  • Untitled series #236
Related show

Commit Message

Ravikumar Kattekola Jan. 30, 2017, 10:11 a.m.
From: Kishon Vijay Abraham I <kishon@ti.com>


commit e2bf08d643a244ccb ("omap_hsmmc: set a large data timeout for
commands with busy signal") sets an arbitrary timeout value (100ms) for
commands like CMD6 (MMC SWITCH). However extended CSD register defined
in the eMMC standard has a field for GENERIC_CMD6_TIME which indicates
the default maximum timeout for a SWITCH command.
Use busy_timeout of cmd structure (populated with GENERIC_CMD6_TIME
in the case of SWITCH command) to program the data timeout value in
omap_hsmmc driver.
SWITCH command to turn the cache on took more than 100ms to complete
with MICRON eMMC card present in AM572x IDK REV 1.3A resulting in
timeout and failed enumeration. It is fixed here by programming the
timeout with the value advertised in GENERIC_CMD6_TIME.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

Signed-off-by: Sekhar Nori <nsekhar@ti.com>

Signed-off-by: Ravikumar Kattekola <rk@ti.com>

---
 drivers/mmc/host/omap_hsmmc.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ulf Hansson Jan. 31, 2017, 10:12 a.m. | #1
On 30 January 2017 at 11:11, Ravikumar Kattekola <rk@ti.com> wrote:
> From: Kishon Vijay Abraham I <kishon@ti.com>

>

> commit e2bf08d643a244ccb ("omap_hsmmc: set a large data timeout for

> commands with busy signal") sets an arbitrary timeout value (100ms) for

> commands like CMD6 (MMC SWITCH). However extended CSD register defined

> in the eMMC standard has a field for GENERIC_CMD6_TIME which indicates

> the default maximum timeout for a SWITCH command.

> Use busy_timeout of cmd structure (populated with GENERIC_CMD6_TIME

> in the case of SWITCH command) to program the data timeout value in

> omap_hsmmc driver.

> SWITCH command to turn the cache on took more than 100ms to complete

> with MICRON eMMC card present in AM572x IDK REV 1.3A resulting in

> timeout and failed enumeration. It is fixed here by programming the

> timeout with the value advertised in GENERIC_CMD6_TIME.

>

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> Signed-off-by: Sekhar Nori <nsekhar@ti.com>

> Signed-off-by: Ravikumar Kattekola <rk@ti.com>

> ---

>  drivers/mmc/host/omap_hsmmc.c | 20 ++++++++++++++------

>  1 file changed, 14 insertions(+), 6 deletions(-)

>

> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c

> index 0ee5650..51ca3d7 100644

> --- a/drivers/mmc/host/omap_hsmmc.c

> +++ b/drivers/mmc/host/omap_hsmmc.c

> @@ -1527,16 +1527,24 @@ static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host)

>  omap_hsmmc_prepare_data(struct omap_hsmmc_host *host, struct mmc_request *req)

>  {

>         int ret;

> +       unsigned int timeout;

> +

>         host->data = req->data;

>

>         if (req->data == NULL) {

>                 OMAP_HSMMC_WRITE(host->base, BLK, 0);

> -               /*

> -                * Set an arbitrary 100ms data timeout for commands with

> -                * busy signal.

> -                */

> -               if (req->cmd->flags & MMC_RSP_BUSY)

> -                       set_data_timeout(host, 100000000U, 0);

> +               if (req->cmd->flags & MMC_RSP_BUSY) {

> +                       timeout = req->cmd->busy_timeout * NSEC_PER_MSEC;

> +

> +                       /*

> +                        * Set an arbitrary 100ms data timeout for commands with

> +                        * busy signal and no indication of busy_timeout.

> +                        */

> +                       if (!timeout)


This is a bug in the mmc core if this ever happen.

Therefore I am particularly interested to find out if this is really
needed or it's just playing safe?

> +                               timeout = 100000000U;

> +

> +                       set_data_timeout(host, timeout, 0);

> +               }

>                 return 0;

>         }

>

> --

> 1.9.1

>


Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 0ee5650..51ca3d7 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1527,16 +1527,24 @@  static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host)
 omap_hsmmc_prepare_data(struct omap_hsmmc_host *host, struct mmc_request *req)
 {
 	int ret;
+	unsigned int timeout;
+
 	host->data = req->data;
 
 	if (req->data == NULL) {
 		OMAP_HSMMC_WRITE(host->base, BLK, 0);
-		/*
-		 * Set an arbitrary 100ms data timeout for commands with
-		 * busy signal.
-		 */
-		if (req->cmd->flags & MMC_RSP_BUSY)
-			set_data_timeout(host, 100000000U, 0);
+		if (req->cmd->flags & MMC_RSP_BUSY) {
+			timeout = req->cmd->busy_timeout * NSEC_PER_MSEC;
+
+			/*
+			 * Set an arbitrary 100ms data timeout for commands with
+			 * busy signal and no indication of busy_timeout.
+			 */
+			if (!timeout)
+				timeout = 100000000U;
+
+			set_data_timeout(host, timeout, 0);
+		}
 		return 0;
 	}