[U-Boot] [PATCH v1 00/11] Improvements on the MMC switch

[U-Boot] [PATCH v1 00/11] Improvements on the MMC switch

[Jean-Jacques Hiblot]

This series brings some improvements related to the switch command:
- use timeouts specified in the EXT_CSD instead of fixed values
- retry switching partitions a few time before calling it a failure
- make switching mode more reliable by checking the status of operation
  afterward.
- make switching faster by using the DAT0 line as a ready/busy signal
- Allow HS200/H400 modes with boot partitions. That didn't work well before
  because the switch was not reliable enough.

This has been tested on DRA76-evm and beaglebone black.
Travis build: https://travis-ci.org/jjhiblot/u-boot/builds/550855605


Jean-Jacques Hiblot (11):
  mmc: omap_hsmmc: reset FSM for DAT and CMD lines if needed before a
    new command
  mmc: omap_hsmmc: don't fill the send_init_stream callback
  Revert "mmc: Add a new callback function to perform the 74 clocks
    cycle sequence"
  mmc: omap_hsmmc: provide wait_dat0 even if UHS modes are not supported
  mmc: add mmc_poll_for_busy() and change the purpose of
    mmc_send_status()
  mmc: if possible, poll the busy state using DAT0
  mmc: use the generic timeout for cmd6 (SWITCH) provided in the ext_csd
  mmc: When switching partition, use the timeout specified in the
    ext_csd
  mmc: During a switch, poll on dat0 if available and check the final
    status
  mmc: do not change mode when accessing a boot partition
  mmc: retry a few times if a partition switch failed

 drivers/mmc/mmc-uclass.c  |  15 ----
 drivers/mmc/mmc.c         | 169 +++++++++++++++++++++-----------------
 drivers/mmc/mmc_private.h |   9 +-
 drivers/mmc/mmc_write.c   |   4 +-
 drivers/mmc/omap_hsmmc.c  |  26 ++----
 include/mmc.h             |  19 ++---
 6 files changed, 115 insertions(+), 127 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH v1 01/11] mmc: omap_hsmmc: reset FSM for DAT and CMD lines if needed before a new command

[Jean-Jacques Hiblot]

It sometimes happen that the PSTATE register does not indicate that the
bus is ready when it really is. This usually happens after a mode switch.
In that case it makes sense to reset the FSM handling the CMD and DATA

Also reset the FSMs if the STATE register cannot be cleared. This also
sometimes happens after a mode switch.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/omap_hsmmc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 133cdc1352..5446ca8b8d 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -1065,18 +1065,17 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 		if (get_timer(0) - start > MAX_RETRY_MS) {
 			printf("%s: timedout waiting on cmd inhibit to clear\n",
 					__func__);
+			mmc_reset_controller_fsm(mmc_base, SYSCTL_SRD);
+			mmc_reset_controller_fsm(mmc_base, SYSCTL_SRC);
 			return -ETIMEDOUT;
 		}
 	}
 	writel(0xFFFFFFFF, &mmc_base->stat);
-	start = get_timer(0);
-	while (readl(&mmc_base->stat)) {
-		if (get_timer(0) - start > MAX_RETRY_MS) {
-			printf("%s: timedout waiting for STAT (%x) to clear\n",
-				__func__, readl(&mmc_base->stat));
-			return -ETIMEDOUT;
-		}
+	if (readl(&mmc_base->stat)) {
+		mmc_reset_controller_fsm(mmc_base, SYSCTL_SRD);
+		mmc_reset_controller_fsm(mmc_base, SYSCTL_SRC);
 	}
+
 	/*
 	 * CMDREG
 	 * CMDIDX[13:8]	: Command index
-- 
2.17.1

[U-Boot] [PATCH v1 02/11] mmc: omap_hsmmc: don't fill the send_init_stream callback

[Jean-Jacques Hiblot]

This is not required. The MMC core sends CMD0 right after the
initialization and it serves the same purpose.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/omap_hsmmc.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 5446ca8b8d..d8f36cd1db 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -775,14 +775,6 @@ tuning_error:
 	return ret;
 }
 #endif
-
-static void omap_hsmmc_send_init_stream(struct udevice *dev)
-{
-	struct omap_hsmmc_data *priv = dev_get_priv(dev);
-	struct hsmmc *mmc_base = priv->base_addr;
-
-	mmc_init_stream(mmc_base);
-}
 #endif
 
 static void mmc_enable_irq(struct mmc *mmc, struct mmc_cmd *cmd)
@@ -1521,7 +1513,6 @@ static const struct dm_mmc_ops omap_hsmmc_ops = {
 #ifdef MMC_SUPPORTS_TUNING
 	.execute_tuning = omap_hsmmc_execute_tuning,
 #endif
-	.send_init_stream	= omap_hsmmc_send_init_stream,
 #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 	.wait_dat0	= omap_hsmmc_wait_dat0,
 #endif
-- 
2.17.1

[U-Boot] [PATCH v1 03/11] Revert "mmc: Add a new callback function to perform the 74 clocks cycle sequence"

[Jean-Jacques Hiblot]

This reverts commit 318a7a576bc49aa8b4207e694d3fbd48c663d6ac.

The last and only user of this callback had been the omap_hsmmc driver.
It is not used anymore. Removing the callback.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc-uclass.c | 13 -------------
 drivers/mmc/mmc.c        |  5 -----
 include/mmc.h            | 10 ----------
 3 files changed, 28 deletions(-)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index a9c8f335c1..6742d99e3d 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -47,19 +47,6 @@ int mmc_set_ios(struct mmc *mmc)
 	return dm_mmc_set_ios(mmc->dev);
 }
 
-void dm_mmc_send_init_stream(struct udevice *dev)
-{
-	struct dm_mmc_ops *ops = mmc_get_ops(dev);
-
-	if (ops->send_init_stream)
-		ops->send_init_stream(dev);
-}
-
-void mmc_send_init_stream(struct mmc *mmc)
-{
-	dm_mmc_send_init_stream(mmc->dev);
-}
-
 #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout)
 {
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 456c1b4cc9..0ccbe10c5e 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1504,10 +1504,6 @@ static int mmc_execute_tuning(struct mmc *mmc, uint opcode)
 }
 #endif
 
-static void mmc_send_init_stream(struct mmc *mmc)
-{
-}
-
 static int mmc_set_ios(struct mmc *mmc)
 {
 	int ret = 0;
@@ -2664,7 +2660,6 @@ int mmc_get_op_cond(struct mmc *mmc)
 
 retry:
 	mmc_set_initial_state(mmc);
-	mmc_send_init_stream(mmc);
 
 	/* Reset the Card */
 	err = mmc_go_idle(mmc);
diff --git a/include/mmc.h b/include/mmc.h
index 1f30f71d25..920f4cc53c 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -414,14 +414,6 @@ struct dm_mmc_ops {
 	 */
 	int (*set_ios)(struct udevice *dev);
 
-	/**
-	 * send_init_stream() - send the initialization stream: 74 clock cycles
-	 * This is used after power up before sending the first command
-	 *
-	 * @dev:	Device to update
-	 */
-	void (*send_init_stream)(struct udevice *dev);
-
 	/**
 	 * get_cd() - See whether a card is present
 	 *
@@ -468,7 +460,6 @@ struct dm_mmc_ops {
 int dm_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 		    struct mmc_data *data);
 int dm_mmc_set_ios(struct udevice *dev);
-void dm_mmc_send_init_stream(struct udevice *dev);
 int dm_mmc_get_cd(struct udevice *dev);
 int dm_mmc_get_wp(struct udevice *dev);
 int dm_mmc_execute_tuning(struct udevice *dev, uint opcode);
@@ -476,7 +467,6 @@ int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout);
 
 /* Transition functions for compatibility */
 int mmc_set_ios(struct mmc *mmc);
-void mmc_send_init_stream(struct mmc *mmc);
 int mmc_getcd(struct mmc *mmc);
 int mmc_getwp(struct mmc *mmc);
 int mmc_execute_tuning(struct mmc *mmc, uint opcode);
-- 
2.17.1

[U-Boot] [PATCH v1 04/11] mmc: omap_hsmmc: provide wait_dat0 even if UHS modes are not supported

[Jean-Jacques Hiblot]

This function can also be used for eMMC devices.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/omap_hsmmc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index d8f36cd1db..3ea7f4e173 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -430,7 +430,6 @@ static void omap_hsmmc_conf_bus_power(struct mmc *mmc, uint signal_voltage)
 	writel(ac12, &mmc_base->ac12);
 }
 
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
 {
 	int ret = -ETIMEDOUT;
@@ -456,7 +455,6 @@ static int omap_hsmmc_wait_dat0(struct udevice *dev, int state, int timeout)
 
 	return ret;
 }
-#endif
 
 #if CONFIG_IS_ENABLED(MMC_IO_VOLTAGE)
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
@@ -1513,9 +1511,7 @@ static const struct dm_mmc_ops omap_hsmmc_ops = {
 #ifdef MMC_SUPPORTS_TUNING
 	.execute_tuning = omap_hsmmc_execute_tuning,
 #endif
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 	.wait_dat0	= omap_hsmmc_wait_dat0,
-#endif
 };
 #else
 static const struct mmc_ops omap_hsmmc_ops = {
-- 
2.17.1

[U-Boot] [PATCH v1 05/11] mmc: add mmc_poll_for_busy() and change the purpose of mmc_send_status()

[Jean-Jacques Hiblot]

mmc_send_status() is currently used to poll the card until it is ready, not
actually returning the status of the card.
Make it return the status and add another function to poll the card.

Also remove the 'extern' declaration in the mmc-private.h header to comply
with the coding standard.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc.c         | 45 ++++++++++++++++++++++++++-------------
 drivers/mmc/mmc_private.h |  9 ++++----
 drivers/mmc/mmc_write.c   |  4 ++--
 3 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 0ccbe10c5e..d4eed62926 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -206,7 +206,7 @@ int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 }
 #endif
 
-int mmc_send_status(struct mmc *mmc, int timeout)
+int mmc_send_status(struct mmc *mmc, unsigned int *status)
 {
 	struct mmc_cmd cmd;
 	int err, retries = 5;
@@ -216,23 +216,39 @@ int mmc_send_status(struct mmc *mmc, int timeout)
 	if (!mmc_host_is_spi(mmc))
 		cmd.cmdarg = mmc->rca << 16;
 
-	while (1) {
+	while (retries--) {
 		err = mmc_send_cmd(mmc, &cmd, NULL);
 		if (!err) {
-			if ((cmd.response[0] & MMC_STATUS_RDY_FOR_DATA) &&
-			    (cmd.response[0] & MMC_STATUS_CURR_STATE) !=
-			     MMC_STATE_PRG)
-				break;
+			mmc_trace_state(mmc, &cmd);
+			*status = cmd.response[0];
+			return 0;
+		}
+	}
+	mmc_trace_state(mmc, &cmd);
+	return -ECOMM;
+}
+
+int mmc_poll_for_busy(struct mmc *mmc, int timeout)
+{
+	unsigned int status;
+	int err;
 
-			if (cmd.response[0] & MMC_STATUS_MASK) {
+	while (1) {
+		err = mmc_send_status(mmc, &status);
+		if (err)
+			return err;
+
+		if ((status & MMC_STATUS_RDY_FOR_DATA) &&
+		    (status & MMC_STATUS_CURR_STATE) !=
+		     MMC_STATE_PRG)
+			break;
+
+		if (status & MMC_STATUS_MASK) {
 #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-				pr_err("Status Error: 0x%08x\n",
-				       cmd.response[0]);
+			pr_err("Status Error: 0x%08x\n", status);
 #endif
-				return -ECOMM;
-			}
-		} else if (--retries < 0)
-			return err;
+			return -ECOMM;
+		}
 
 		if (timeout-- <= 0)
 			break;
@@ -240,7 +256,6 @@ int mmc_send_status(struct mmc *mmc, int timeout)
 		udelay(1000);
 	}
 
-	mmc_trace_state(mmc, &cmd);
 	if (timeout <= 0) {
 #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		pr_err("Timeout waiting card ready\n");
@@ -752,7 +767,7 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 		}
 
 		/* Waiting for the ready status */
-		return mmc_send_status(mmc, timeout);
+		return mmc_poll_for_busy(mmc, timeout);
 	}
 
 	return ret;
diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
index f49b6eb573..35170d03ab 100644
--- a/drivers/mmc/mmc_private.h
+++ b/drivers/mmc/mmc_private.h
@@ -11,10 +11,11 @@
 
 #include <mmc.h>
 
-extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
-			struct mmc_data *data);
-extern int mmc_send_status(struct mmc *mmc, int timeout);
-extern int mmc_set_blocklen(struct mmc *mmc, int len);
+int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data);
+int mmc_send_status(struct mmc *mmc, unsigned int *status);
+int mmc_poll_for_busy(struct mmc *mmc, int timeout);
+
+int mmc_set_blocklen(struct mmc *mmc, int len);
 #ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
 void mmc_adapter_card_type_ident(void);
 #endif
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index c8c83c9188..02648b0f50 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -119,7 +119,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
 		blk += blk_r;
 
 		/* Waiting for the ready status */
-		if (mmc_send_status(mmc, timeout))
+		if (mmc_poll_for_busy(mmc, timeout))
 			return 0;
 	}
 
@@ -177,7 +177,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
 	}
 
 	/* Waiting for the ready status */
-	if (mmc_send_status(mmc, timeout))
+	if (mmc_poll_for_busy(mmc, timeout))
 		return 0;
 
 	return blkcnt;
-- 
2.17.1

[U-Boot] [PATCH v1 06/11] mmc: if possible, poll the busy state using DAT0

[Jean-Jacques Hiblot]

Using the DAT0 line as a rdy/busy line is an alternative to reading the
status register of the card. It especially useful in situation where the
bus is not in a good shape, like when modes are switched.
This is also how the linux driver behaves.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc-uclass.c | 2 --
 drivers/mmc/mmc.c        | 6 ++++--
 include/mmc.h            | 2 --
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 6742d99e3d..65d9da48f6 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -47,7 +47,6 @@ int mmc_set_ios(struct mmc *mmc)
 	return dm_mmc_set_ios(mmc->dev);
 }
 
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout)
 {
 	struct dm_mmc_ops *ops = mmc_get_ops(dev);
@@ -61,7 +60,6 @@ int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
 {
 	return dm_mmc_wait_dat0(mmc->dev, state, timeout);
 }
-#endif
 
 int dm_mmc_get_wp(struct udevice *dev)
 {
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index d4eed62926..89085868c6 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -29,12 +29,10 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps);
 
 #if !CONFIG_IS_ENABLED(DM_MMC)
 
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 static int mmc_wait_dat0(struct mmc *mmc, int state, int timeout)
 {
 	return -ENOSYS;
 }
-#endif
 
 __weak int board_mmc_getwp(struct mmc *mmc)
 {
@@ -233,6 +231,10 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout)
 	unsigned int status;
 	int err;
 
+	err = mmc_wait_dat0(mmc, 1, timeout);
+	if (err != -ENOSYS)
+		return err;
+
 	while (1) {
 		err = mmc_send_status(mmc, &status);
 		if (err)
diff --git a/include/mmc.h b/include/mmc.h
index 920f4cc53c..854778deed 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -441,7 +441,6 @@ struct dm_mmc_ops {
 	int (*execute_tuning)(struct udevice *dev, uint opcode);
 #endif
 
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
 	/**
 	 * wait_dat0() - wait until dat0 is in the target state
 	 *		(CLK must be running during the wait)
@@ -452,7 +451,6 @@ struct dm_mmc_ops {
 	 * @return 0 if dat0 is in the target state, -ve on error
 	 */
 	int (*wait_dat0)(struct udevice *dev, int state, int timeout);
-#endif
 };
 
 #define mmc_get_ops(dev)        ((struct dm_mmc_ops *)(dev)->driver->ops)
-- 
2.17.1

[U-Boot] [PATCH v1 07/11] mmc: use the generic timeout for cmd6 (SWITCH) provided in the ext_csd

[Jean-Jacques Hiblot]

Starting with rev 4.5, the eMMC can define a generic timeout for the
SWITCH command.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc.c | 10 +++++++++-
 include/mmc.h     |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 89085868c6..74590a5657 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -21,6 +21,8 @@
 #include <div64.h>
 #include "mmc_private.h"
 
+#define DEFAULT_CMD6_TIMEOUT_MS  500
+
 static int mmc_set_signal_voltage(struct mmc *mmc, uint signal_voltage);
 static int mmc_power_cycle(struct mmc *mmc);
 #if !CONFIG_IS_ENABLED(MMC_TINY)
@@ -745,10 +747,13 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 			bool send_status)
 {
 	struct mmc_cmd cmd;
-	int timeout = 1000;
+	int timeout = DEFAULT_CMD6_TIMEOUT_MS;
 	int retries = 3;
 	int ret;
 
+	if (mmc->gen_cmd6_time)
+		timeout = mmc->gen_cmd6_time * 10;
+
 	cmd.cmdidx = MMC_CMD_SWITCH;
 	cmd.resp_type = MMC_RSP_R1b;
 	cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
@@ -2135,6 +2140,9 @@ static int mmc_startup_v4(struct mmc *mmc)
 			mmc->capacity_user = capacity;
 	}
 
+	if (mmc->version >= MMC_VERSION_4_5)
+		mmc->gen_cmd6_time = ext_csd[EXT_CSD_GENERIC_CMD6_TIME];
+
 	/* The partition data may be non-zero but it is only
 	 * effective if PARTITION_SETTING_COMPLETED is set in
 	 * EXT_CSD, so ignore any data if this bit is not set,
diff --git a/include/mmc.h b/include/mmc.h
index 854778deed..8159ef1448 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -226,6 +226,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
 #define EXT_CSD_BOOT_MULT		226	/* RO */
+#define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
 #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
 
 /*
@@ -581,6 +582,7 @@ struct mmc {
 	u8 part_attr;
 	u8 wr_rel_set;
 	u8 part_config;
+	u8 gen_cmd6_time;
 	uint tran_speed;
 	uint legacy_speed; /* speed for the legacy mode provided by the card */
 	uint read_bl_len;
-- 
2.17.1

[U-Boot] [PATCH v1 08/11] mmc: When switching partition, use the timeout specified in the ext_csd

[Jean-Jacques Hiblot]

The e-MMC spec allows the e-MMC to specify a timeout for the partition
switch command. It can take up to 2550 ms. There is no lower limit to this
value in the spec, but do as the the linux driver does and force it to be
at least 300ms.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc.c | 10 ++++++++++
 include/mmc.h     |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 74590a5657..0f77348681 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -748,12 +748,17 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 {
 	struct mmc_cmd cmd;
 	int timeout = DEFAULT_CMD6_TIMEOUT_MS;
+	bool is_part_switch = (set == EXT_CSD_CMD_SET_NORMAL) &&
+			      (index == EXT_CSD_PART_CONF);
 	int retries = 3;
 	int ret;
 
 	if (mmc->gen_cmd6_time)
 		timeout = mmc->gen_cmd6_time * 10;
 
+	if (is_part_switch  && mmc->part_switch_time)
+		timeout = mmc->part_switch_time * 10;
+
 	cmd.cmdidx = MMC_CMD_SWITCH;
 	cmd.resp_type = MMC_RSP_R1b;
 	cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
@@ -2152,6 +2157,11 @@ static int mmc_startup_v4(struct mmc *mmc)
 	part_completed = !!(ext_csd[EXT_CSD_PARTITION_SETTING] &
 			    EXT_CSD_PARTITION_SETTING_COMPLETED);
 
+	mmc->part_switch_time = ext_csd[EXT_CSD_PART_SWITCH_TIME];
+	/* Some eMMC set the value too low so set a minimum */
+	if (mmc->part_switch_time < MMC_MIN_PART_SWITCH_TIME && mmc->part_switch_time)
+		mmc->part_switch_time = MMC_MIN_PART_SWITCH_TIME;
+
 	/* store the partition info of emmc */
 	mmc->part_support = ext_csd[EXT_CSD_PARTITIONING_SUPPORT];
 	if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) ||
diff --git a/include/mmc.h b/include/mmc.h
index 8159ef1448..c199d1a0e8 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -222,6 +222,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_HS_TIMING		185	/* R/W */
 #define EXT_CSD_REV			192	/* RO */
 #define EXT_CSD_CARD_TYPE		196	/* RO */
+#define EXT_CSD_PART_SWITCH_TIME	199	/* RO */
 #define EXT_CSD_SEC_CNT			212	/* RO, 4 bytes */
 #define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
@@ -583,6 +584,7 @@ struct mmc {
 	u8 wr_rel_set;
 	u8 part_config;
 	u8 gen_cmd6_time;
+	u8 part_switch_time;
 	uint tran_speed;
 	uint legacy_speed; /* speed for the legacy mode provided by the card */
 	uint read_bl_len;
@@ -829,6 +831,9 @@ extern uint mmc_get_env_part(struct mmc *mmc);
 # endif
 int mmc_get_env_dev(void);
 
+/* Minimum partition switch timeout in units of 10-milliseconds */
+#define MMC_MIN_PART_SWITCH_TIME	30 /* 300 ms */
+
 /* Set block count limit because of 16 bit register limit on some hardware*/
 #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
 #define CONFIG_SYS_MMC_MAX_BLK_COUNT 65535
-- 
2.17.1

[U-Boot] [PATCH v1 09/11] mmc: During a switch, poll on dat0 if available and check the final status

[Jean-Jacques Hiblot]

The switch operation can sometimes make the bus unreliable, in that case
the send_status parameter should be false to indicate not to poll using
CMD13. If polling on dat0 is possible, we should use it to detect the end
of the operation.
At the end of the operation it is safe to use CMD13 to get the status of
the card. It is important to do so because the operation may have failed.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc.c | 49 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 0f77348681..b0684596f9 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -746,6 +746,7 @@ static int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd)
 static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 			bool send_status)
 {
+	unsigned int status, start;
 	struct mmc_cmd cmd;
 	int timeout = DEFAULT_CMD6_TIMEOUT_MS;
 	bool is_part_switch = (set == EXT_CSD_CMD_SET_NORMAL) &&
@@ -765,25 +766,47 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value,
 				 (index << 16) |
 				 (value << 8);
 
-	while (retries > 0) {
+	do {
 		ret = mmc_send_cmd(mmc, &cmd, NULL);
+	} while (ret && retries-- > 0);
 
-		if (ret) {
-			retries--;
-			continue;
-		}
+	if (ret)
+		return ret;
 
-		if (!send_status) {
-			mdelay(50);
-			return 0;
-		}
+	start = get_timer(0);
 
-		/* Waiting for the ready status */
-		return mmc_poll_for_busy(mmc, timeout);
-	}
+	/* poll dat0 for rdy/buys status */
+	ret = mmc_wait_dat0(mmc, 1, timeout);
+	if (ret && ret != -ENOSYS)
+		return ret;
 
-	return ret;
+	/*
+	 * In cases when not allowed to poll by using CMD13 or because we aren't
+	 * capable of polling by using mmc_wait_dat0, then rely on waiting the
+	 * stated timeout to be sufficient.
+	 */
+	if (ret == -ENOSYS && !send_status)
+		mdelay(timeout);
+
+	/* Finally wait until the card is ready or indicates a failure
+	 * to switch. It doesn't hurt to use CMD13 here even if send_status
+	 * is false, because by now (after 'timeout' ms) the bus should be
+	 * reliable.
+	 */
+	do {
+		ret = mmc_send_status(mmc, &status);
+
+		if (!ret && (status & MMC_STATUS_SWITCH_ERROR)) {
+			pr_debug("switch failed %d/%d/0x%x !\n", set, index,
+				 value);
+			return -EIO;
+		}
+		if (!ret && (status & MMC_STATUS_RDY_FOR_DATA))
+			return 0;
+		udelay(100);
+	} while (get_timer(start) < timeout);
 
+	return -ETIMEDOUT;
 }
 
 int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
-- 
2.17.1

[U-Boot] [PATCH v1 10/11] mmc: do not change mode when accessing a boot partition

[Jean-Jacques Hiblot]

Accessing the boot partition had been error prone with HS200 and HS400 and
was disabled. The driver first switched to a lesser mode and then switched
the partition access. It was mostly due to a bad handling of the switch and
has been fixed, so let's remove this 'feature'

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

 drivers/mmc/mmc.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index b0684596f9..709733747a 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -955,46 +955,10 @@ static int mmc_set_capacity(struct mmc *mmc, int part_num)
 	return 0;
 }
 
-#if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
-static int mmc_boot_part_access_chk(struct mmc *mmc, unsigned int part_num)
-{
-	int forbidden = 0;
-	bool change = false;
-
-	if (part_num & PART_ACCESS_MASK)
-		forbidden = MMC_CAP(MMC_HS_200);
-
-	if (MMC_CAP(mmc->selected_mode) & forbidden) {
-		pr_debug("selected mode (%s) is forbidden for part %d\n",
-			 mmc_mode_name(mmc->selected_mode), part_num);
-		change = true;
-	} else if (mmc->selected_mode != mmc->best_mode) {
-		pr_debug("selected mode is not optimal\n");
-		change = true;
-	}
-
-	if (change)
-		return mmc_select_mode_and_width(mmc,
-						 mmc->card_caps & ~forbidden);
-
-	return 0;
-}
-#else
-static inline int mmc_boot_part_access_chk(struct mmc *mmc,
-					   unsigned int part_num)
-{
-	return 0;
-}
-#endif
-
 int mmc_switch_part(struct mmc *mmc, unsigned int part_num)
 {
 	int ret;
 
-	ret = mmc_boot_part_access_chk(mmc, part_num);
-	if (ret)
-		return ret;
-
 	ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF,
 			 (mmc->part_config & ~PART_ACCESS_MASK)
 			 | (part_num & PART_ACCESS_MASK));
-- 
2.17.1

[U-Boot] [PATCH v1 11/11] mmc: retry a few times if a partition switch failed

[Jean-Jacques Hiblot]

This operation may fail. Retry it a few times before giving up and report
a failure.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

---

 drivers/mmc/mmc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 709733747a..cec39a9acf 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -958,10 +958,14 @@ static int mmc_set_capacity(struct mmc *mmc, int part_num)
 int mmc_switch_part(struct mmc *mmc, unsigned int part_num)
 {
 	int ret;
+	int retry = 3;
 
-	ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF,
-			 (mmc->part_config & ~PART_ACCESS_MASK)
-			 | (part_num & PART_ACCESS_MASK));
+	do {
+		ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_PART_CONF,
+				 (mmc->part_config & ~PART_ACCESS_MASK)
+				 | (part_num & PART_ACCESS_MASK));
+	} while (ret && retry--);
 
 	/*
 	 * Set the capacity if the switch succeeded or was intended
-- 
2.17.1

[U-Boot] [PATCH v1 06/11] mmc: if possible, poll the busy state using DAT0

[Marek Vasut]

On 6/27/19 12:31 PM, Jean-Jacques Hiblot wrote:
> Using the DAT0 line as a rdy/busy line is an alternative to reading the
> status register of the card. It especially useful in situation where the
> bus is not in a good shape, like when modes are switched.
> This is also how the linux driver behaves.
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

Is this documented somewhere that this is an OK thing to do ?
If the bus is not in a good shape, what guarantee do we have that DAT0
(part of the bus ;-) ) would be in good shape ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 06/11] mmc: if possible, poll the busy state using DAT0

[Jean-Jacques Hiblot]

On 28/06/2019 02:07, Marek Vasut wrote:
> On 6/27/19 12:31 PM, Jean-Jacques Hiblot wrote:
>> Using the DAT0 line as a rdy/busy line is an alternative to reading the
>> status register of the card. It especially useful in situation where the
>> bus is not in a good shape, like when modes are switched.
>> This is also how the linux driver behaves.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Is this documented somewhere that this is an OK thing to do ?
> If the bus is not in a good shape, what guarantee do we have that DAT0
> (part of the bus ;-) ) would be in good shape ?

The spec states in "6.6.1 Command sets and extended settings": [...] The 
SWITCH command response is of type R1b, therefore, the host should read 
the Device status, using SEND_STATUS command, after the busy signal is 
de-asserted, to check the result of the SWITCH operation.[...]

Also this is how it is done in linux, except that instead of 
wait_dat0(), the host driver provides a card_busy() function.

I think that "not in good shape" probably means transients on the bus, 
maybe due to changes in the voltage levels. Looking at DAT0 should be 
fine because it is grounded while the card is busy.

The only trouble with using DAT0, is that the clock must run while 
waiting otherwise DAT0 may not be de-asserted, hence the encapsulation 
in the wait_dat0() so that the HW can disable automatic clock-gating 
before monitoring DAT0. Here is the relevant text from the spec:  6.7 
"clock control"  [...] The host is allowed to shut down the clock of a 
“busy” Device. The Device will complete the programming operation 
regardless of the host clock. However, the host must provide a clock 
edge for the Device to turn off its busy signal. Without a clock edge 
the Device (unless previously disconnected by a deselect command (CMD7)) 
will force the DAT0 line down, forever. [...]


>

[U-Boot] [PATCH v1 06/11] mmc: if possible, poll the busy state using DAT0

[Marek Vasut]

On 6/28/19 10:28 AM, Jean-Jacques Hiblot wrote:
> 
> On 28/06/2019 02:07, Marek Vasut wrote:
>> On 6/27/19 12:31 PM, Jean-Jacques Hiblot wrote:
>>> Using the DAT0 line as a rdy/busy line is an alternative to reading the
>>> status register of the card. It especially useful in situation where the
>>> bus is not in a good shape, like when modes are switched.
>>> This is also how the linux driver behaves.
>>>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> Is this documented somewhere that this is an OK thing to do ?
>> If the bus is not in a good shape, what guarantee do we have that DAT0
>> (part of the bus ;-) ) would be in good shape ?
> 
> The spec states in "6.6.1 Command sets and extended settings": [...] The
> SWITCH command response is of type R1b, therefore, the host should read
> the Device status, using SEND_STATUS command, after the busy signal is
> de-asserted, to check the result of the SWITCH operation.[...]
> 
> Also this is how it is done in linux, except that instead of
> wait_dat0(), the host driver provides a card_busy() function.
> 
> I think that "not in good shape" probably means transients on the bus,
> maybe due to changes in the voltage levels. Looking at DAT0 should be
> fine because it is grounded while the card is busy.
> 
> The only trouble with using DAT0, is that the clock must run while
> waiting otherwise DAT0 may not be de-asserted, hence the encapsulation
> in the wait_dat0() so that the HW can disable automatic clock-gating
> before monitoring DAT0. Here is the relevant text from the spec:  6.7
> "clock control"  [...] The host is allowed to shut down the clock of a
> “busy” Device. The Device will complete the programming operation
> regardless of the host clock. However, the host must provide a clock
> edge for the Device to turn off its busy signal. Without a clock edge
> the Device (unless previously disconnected by a deselect command (CMD7))
> will force the DAT0 line down, forever. [...]

Ah, good, maybe this should be in the commit message?

Also, which version of the spec is that ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 06/11] mmc: if possible, poll the busy state using DAT0

[Jean-Jacques Hiblot]

On 28/06/2019 15:38, Marek Vasut wrote:
> On 6/28/19 10:28 AM, Jean-Jacques Hiblot wrote:
>> On 28/06/2019 02:07, Marek Vasut wrote:
>>> On 6/27/19 12:31 PM, Jean-Jacques Hiblot wrote:
>>>> Using the DAT0 line as a rdy/busy line is an alternative to reading the
>>>> status register of the card. It especially useful in situation where the
>>>> bus is not in a good shape, like when modes are switched.
>>>> This is also how the linux driver behaves.
>>>>
>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>> Is this documented somewhere that this is an OK thing to do ?
>>> If the bus is not in a good shape, what guarantee do we have that DAT0
>>> (part of the bus ;-) ) would be in good shape ?
>> The spec states in "6.6.1 Command sets and extended settings": [...] The
>> SWITCH command response is of type R1b, therefore, the host should read
>> the Device status, using SEND_STATUS command, after the busy signal is
>> de-asserted, to check the result of the SWITCH operation.[...]
>>
>> Also this is how it is done in linux, except that instead of
>> wait_dat0(), the host driver provides a card_busy() function.
>>
>> I think that "not in good shape" probably means transients on the bus,
>> maybe due to changes in the voltage levels. Looking at DAT0 should be
>> fine because it is grounded while the card is busy.
>>
>> The only trouble with using DAT0, is that the clock must run while
>> waiting otherwise DAT0 may not be de-asserted, hence the encapsulation
>> in the wait_dat0() so that the HW can disable automatic clock-gating
>> before monitoring DAT0. Here is the relevant text from the spec:  6.7
>> "clock control"  [...] The host is allowed to shut down the clock of a
>> “busy” Device. The Device will complete the programming operation
>> regardless of the host clock. However, the host must provide a clock
>> edge for the Device to turn off its busy signal. Without a clock edge
>> the Device (unless previously disconnected by a deselect command (CMD7))
>> will force the DAT0 line down, forever. [...]
> Ah, good, maybe this should be in the commit message?
which part ? the clock stuff ?
>
> Also, which version of the spec is that ?
I'm quoting from JESD84-B51A (rev 5.1A) but IFAIK rdy/busy on DAT0 has 
been there from the start.
>

[U-Boot] [PATCH v1 06/11] mmc: if possible, poll the busy state using DAT0

[Marek Vasut]

On 6/28/19 4:26 PM, Jean-Jacques Hiblot wrote:
> 
> On 28/06/2019 15:38, Marek Vasut wrote:
>> On 6/28/19 10:28 AM, Jean-Jacques Hiblot wrote:
>>> On 28/06/2019 02:07, Marek Vasut wrote:
>>>> On 6/27/19 12:31 PM, Jean-Jacques Hiblot wrote:
>>>>> Using the DAT0 line as a rdy/busy line is an alternative to reading
>>>>> the
>>>>> status register of the card. It especially useful in situation
>>>>> where the
>>>>> bus is not in a good shape, like when modes are switched.
>>>>> This is also how the linux driver behaves.
>>>>>
>>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>>> Is this documented somewhere that this is an OK thing to do ?
>>>> If the bus is not in a good shape, what guarantee do we have that DAT0
>>>> (part of the bus ;-) ) would be in good shape ?
>>> The spec states in "6.6.1 Command sets and extended settings": [...] The
>>> SWITCH command response is of type R1b, therefore, the host should read
>>> the Device status, using SEND_STATUS command, after the busy signal is
>>> de-asserted, to check the result of the SWITCH operation.[...]
>>>
>>> Also this is how it is done in linux, except that instead of
>>> wait_dat0(), the host driver provides a card_busy() function.
>>>
>>> I think that "not in good shape" probably means transients on the bus,
>>> maybe due to changes in the voltage levels. Looking at DAT0 should be
>>> fine because it is grounded while the card is busy.
>>>
>>> The only trouble with using DAT0, is that the clock must run while
>>> waiting otherwise DAT0 may not be de-asserted, hence the encapsulation
>>> in the wait_dat0() so that the HW can disable automatic clock-gating
>>> before monitoring DAT0. Here is the relevant text from the spec:  6.7
>>> "clock control"  [...] The host is allowed to shut down the clock of a
>>> “busy” Device. The Device will complete the programming operation
>>> regardless of the host clock. However, the host must provide a clock
>>> edge for the Device to turn off its busy signal. Without a clock edge
>>> the Device (unless previously disconnected by a deselect command (CMD7))
>>> will force the DAT0 line down, forever. [...]
>> Ah, good, maybe this should be in the commit message?
> which part ? the clock stuff ?
>>
>> Also, which version of the spec is that ?
> I'm quoting from JESD84-B51A (rev 5.1A) but IFAIK rdy/busy on DAT0 has
> been there from the start.

^ yes, that part should likely be in the commit message, this is really
useful info which should be retained in the history IMO.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 11/11] mmc: retry a few times if a partition switch failed

[Peng Fan]

> Subject: [PATCH v1 11/11] mmc: retry a few times if a partition switch failed
> 
> This operation may fail. Retry it a few times before giving up and report a
> failure.
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> 
> ---
> 
>  drivers/mmc/mmc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> 709733747a..cec39a9acf 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -958,10 +958,14 @@ static int mmc_set_capacity(struct mmc *mmc, int
> part_num)  int mmc_switch_part(struct mmc *mmc, unsigned int part_num)
> {
>  	int ret;
> +	int retry = 3;
> 
> -	ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_PART_CONF,
> -			 (mmc->part_config & ~PART_ACCESS_MASK)
> -			 | (part_num & PART_ACCESS_MASK));
> +	do {
> +		ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> +				 EXT_CSD_PART_CONF,
> +				 (mmc->part_config & ~PART_ACCESS_MASK)
> +				 | (part_num & PART_ACCESS_MASK));
> +	} while (ret && retry--);
> 
>  	/*
>  	 * Set the capacity if the switch succeeded or was intended
> --


Patchset applied to mmc/master. Please raise, if any objections.

Thanks,
Peng.

> 2.17.1