[PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc

[PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc

[Ulf Hansson]

Changes in v2:

	- From discussions around the difficulties for how to support CMD13
	polling for HS200, HS400 HS400ES, I decided to drop those patches for
	now. It's is particularly due to the need for tuning, after a speed mode
	switch, that makes it hard to rely on CMD13 polling for these cases.
	Perhaps we can allow CMD13 polling when switching to the intermediate
	speed modes, but let's deal with that outside of this series.

	- Folded in a new patch, which checks the SWITCH_ERROR bit in each
	  CMD13 response when polling with CMD13. Patch v2 4/7.

	- No other changes made to the rest of the series.



Several changes has been made for how and when to use CMD13 as a
polling-method, to find out when the mmc cards stops signaling busy after
sending a CMD6. This particularly concerns the cases when switching to new bus
speed modes, such as HS (high-speed), HS DDR, HS200, HS400 and HS400ES.

Currently CMD13 polling is *not* allowed for these cases, but this was recently
changed - and which did cause regressions for card detection for some
platforms.

A simple fix was made to solve these regressions, simply by using a default
CMD6 generic timeout of 500ms, as to avoid using the fall-back timeout in
__mmc_switch() of 10min. That greatly improve the situation and one could
consider the regressions as "solved".

However, this simple fix is still causing unnecessary long card initialization
times, especially for those mmc hosts that doesn't support HW busy detection
(using MMC_CAP_WAIT_WHILE_BUSY and/or implements the ->card_busy() host ops).

This because we wait a fixed amount of time (CMD6 generic timeout) to make sure
the card is done signaling busy, while in practice this happens a lot sooner.

A couple of tests for HS and HS DDR eMMC cards, shows the card only need a few
ms to complete the bus speed mode changes, thus it's far less than the CMD6
generic timeout.

To improve this behaviour and shorten the card initialization time, we need to
allow using CMD13 as polling method to find out when the card stops signaling
busy. Although, enabling CMD13 polling may also introduce other issues as
according to the JEDEC spec, it's not the recommended method. Especially it
mentions that CRC errors may be triggered when sending a CMD13 command during a
bus timing change.

To deal with these issues, we need to change from ignoring those CRC errors but
instead continue to treat the card as being busy and continue to poll with
CMD13. Perhaps this behaviour is one of reasons to why the earlier CMD13 polling
method didn't work out well.

This series requires extensive testing, please help with that!

I have currently tested it with HS and HS DDR eMMC cards, and for combinations
of an mmc hosts (mmci) supporting HW busy detection and not (local hacks in
mmci.c).


Ulf Hansson (9):
  mmc: core: Retry instead of ignore at CRC errors when polling for busy
  mmc: core: Remove redundant __mmc_send_status()
  mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
  mmc: core: Enable __mmc_switch() to change bus speed timing for the
    host
  mmc: core: Allow CMD13 polling when switching to HS mode for mmc
  mmc: core: Update CMD13 polling policy when switch to HS DDR mode
  mmc: core: Allow CMD13 polling when switch to HS200 mode
  mmc: core: Allow CMD13 polling when switch to HS400 mode
  mmc: core: Allow CMD13 polling when switch to HS400ES mode

 drivers/mmc/core/core.c    |   2 +-
 drivers/mmc/core/mmc.c     | 156 ++++++++++++++-------------------------------
 drivers/mmc/core/mmc_ops.c |  45 +++++++------
 drivers/mmc/core/mmc_ops.h |   4 +-
 4 files changed, 77 insertions(+), 130 deletions(-)

-- 
1.9.1






*** BLURB HERE ***

Ulf Hansson (7):
  mmc: core: Retry instead of ignore at CRC errors when polling for busy
  mmc: core: Remove redundant __mmc_send_status()
  mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
  mmc: core: Check SWITCH_ERROR bit from each CMD13 response when
    polling
  mmc: core: Enable __mmc_switch() to change bus speed timing for the
    host
  mmc: core: Allow CMD13 polling when switching to HS mode for mmc
  mmc: core: Update CMD13 polling policy when switch to HS DDR mode

 drivers/mmc/core/core.c    |  2 +-
 drivers/mmc/core/mmc.c     | 53 +++++++++++++++++++++++-----------------------
 drivers/mmc/core/mmc_ops.c | 51 ++++++++++++++++++++++++++------------------
 drivers/mmc/core/mmc_ops.h |  4 ++--
 4 files changed, 60 insertions(+), 50 deletions(-)

-- 
1.9.1

[PATCH v2 1/7] mmc: core: Retry instead of ignore at CRC errors when polling for busy

[Ulf Hansson]

After a CMD6 command has been sent, the __mmc_switch() function might be
advised to poll the card for busy by using CMD13 and also by ignoring CRC
errors.

In the case of ignoring CRC errors, the mmc core tells the mmc host to also
ignore these errors via masking the MMC_RSP_CRC response flag. This seems
wrong, as it leads to that the mmc host could propagate an unreliable
response, instead of a proper error code.

What we really want, is not to ignore CRC errors but instead retry the
polling attempt. So, let's change this by treating a CRC error as the card
is still being busy and thus continue to run the polling loop.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc_ops.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 481bbdb..4773c56 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -503,10 +503,13 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
 		if (host->ops->card_busy) {
 			busy = host->ops->card_busy(host);
 		} else {
-			err = __mmc_send_status(card, &status, ignore_crc);
-			if (err)
+			err = mmc_send_status(card, &status);
+			if (ignore_crc && err == -EILSEQ)
+				busy = true;
+			else if (err)
 				return err;
-			busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
+			else
+				busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
 		}
 
 		/* Timeout if the device still remains busy. */
-- 
1.9.1

[PATCH v2 2/7] mmc: core: Remove redundant __mmc_send_status()

[Ulf Hansson]

There are only one users left which calls __mmc_send_status(). Moreover,
the ignore_crc parameter isn't being used, so let's just remove these
redundant parts.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc_ops.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 4773c56..0515748 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -54,8 +54,7 @@
 	0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
 };
 
-static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
-				    bool ignore_crc)
+int mmc_send_status(struct mmc_card *card, u32 *status)
 {
 	int err;
 	struct mmc_command cmd = {0};
@@ -67,8 +66,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 	if (!mmc_host_is_spi(card->host))
 		cmd.arg = card->rca << 16;
 	cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
-	if (ignore_crc)
-		cmd.flags &= ~MMC_RSP_CRC;
 
 	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
 	if (err)
@@ -83,11 +80,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 	return 0;
 }
 
-int mmc_send_status(struct mmc_card *card, u32 *status)
-{
-	return __mmc_send_status(card, status, false);
-}
-
 static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
 {
 	struct mmc_command cmd = {0};
-- 
1.9.1

[PATCH v2 3/7] mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose

[Ulf Hansson]

The ignore_crc parameter/variable name is used at a couple of places in the
mmc core. Let's rename it to retry_crc_err to reflect its new purpose.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc_ops.c | 10 +++++-----
 drivers/mmc/core/mmc_ops.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0515748..214e734 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -461,7 +461,7 @@ int mmc_switch_status(struct mmc_card *card)
 }
 
 static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
-			bool send_status, bool ignore_crc)
+			bool send_status, bool retry_crc_err)
 {
 	struct mmc_host *host = card->host;
 	int err;
@@ -496,7 +496,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
 			busy = host->ops->card_busy(host);
 		} else {
 			err = mmc_send_status(card, &status);
-			if (ignore_crc && err == -EILSEQ)
+			if (retry_crc_err && err == -EILSEQ)
 				busy = true;
 			else if (err)
 				return err;
@@ -528,13 +528,13 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
  *                   timeout of zero implies maximum possible timeout
  *	@use_busy_signal: use the busy signal as response type
  *	@send_status: send status cmd to poll for busy
- *	@ignore_crc: ignore CRC errors when sending status cmd to poll for busy
+ *	@retry_crc_err: retry when CRC errors when polling with CMD13 for busy
  *
  *	Modifies the EXT_CSD register for selected card.
  */
 int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		unsigned int timeout_ms, bool use_busy_signal, bool send_status,
-		bool ignore_crc)
+		bool retry_crc_err)
 {
 	struct mmc_host *host = card->host;
 	int err;
@@ -590,7 +590,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 	}
 
 	/* Let's try to poll to find out when the command is completed. */
-	err = mmc_poll_for_busy(card, timeout_ms, send_status, ignore_crc);
+	err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err);
 out:
 	mmc_retune_release(host);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 7f6c0e9..9fccddb 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -30,7 +30,7 @@
 int mmc_switch_status(struct mmc_card *card);
 int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		unsigned int timeout_ms, bool use_busy_signal, bool send_status,
-		bool ignore_crc);
+		bool retry_crc_err);
 
 #endif
 
-- 
1.9.1

[PATCH v2 4/7] mmc: core: Check SWITCH_ERROR bit from each CMD13 response when polling

[Ulf Hansson]

According to the JEDEC specification, the SWITCH_ERROR bit in the device
status from a R1 response, is an error bit which may be cleared as soon as
the response that reports the error is sent.

When polling with CMD13 to find out when the card stops signaling busy
after a CMD6 has been sent, we currently parse only the last CMD13 response
for the SWITCH_ERROR bit. Consequentially we could loose important
information about the card.

In worst case if the card stops signaling busy within the allowed timeout,
we could end up believing that the CMD6 command completed successfully,
when in fact it didn't.

To improve the behaviour, let's parse each CMD13 response to see if the
SWITCH_ERROR bit is set in the device status. In such case, we abort the
polling loop and report the error.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- New patch.

---
 drivers/mmc/core/mmc_ops.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 214e734..fba5d29 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -496,12 +496,16 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
 			busy = host->ops->card_busy(host);
 		} else {
 			err = mmc_send_status(card, &status);
-			if (retry_crc_err && err == -EILSEQ)
+			if (retry_crc_err && err == -EILSEQ) {
 				busy = true;
-			else if (err)
+			} else if (err) {
 				return err;
-			else
+			} else {
+				err = mmc_switch_status_error(host, status);
+				if (err)
+					return err;
 				busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
+			}
 		}
 
 		/* Timeout if the device still remains busy. */
@@ -515,7 +519,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
 	if (host->ops->card_busy && send_status)
 		return mmc_switch_status(card);
 
-	return mmc_switch_status_error(host, status);
+	return 0;
 }
 
 /**
-- 
1.9.1

[PATCH v2 5/7] mmc: core: Enable __mmc_switch() to change bus speed timing for the host

[Ulf Hansson]

In cases when a speed mode change is requested for mmc cards, a CMD6 is
sent by calling __mmc_switch() during the card initialization. The CMD6
leads to the card entering a busy period. When that is completed, the host
must parse the CMD6 status to find out whether the change of the speed mode
succeeded.

To enable the mmc core to poll the card by using CMD13 to find out when the
busy period is completed, it's reasonable to make sure polling is done by
having the mmc host and the mmc card, being configured to operate at the
same selected bus speed timing.

Therefore, let's extend __mmc_switch() to take yet another parameter, which
allow its callers to update the bus speed timing of the mmc host. In this
way, __mmc_switch() also becomes capable of reading and validating the CMD6
status by sending a CMD13, in cases when that's desired.

If __mmc_switch() encounters a failure, we make sure to restores the old
bus speed timing for the mmc host, before propagating the error code.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/core.c    |  2 +-
 drivers/mmc/core/mmc.c     | 18 +++++++++---------
 drivers/mmc/core/mmc_ops.c | 20 +++++++++++++++-----
 drivers/mmc/core/mmc_ops.h |  4 ++--
 4 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 50bb9a1..060f767 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -380,7 +380,7 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
 	mmc_retune_hold(card->host);
 
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-			EXT_CSD_BKOPS_START, 1, timeout,
+			EXT_CSD_BKOPS_START, 1, timeout, 0,
 			use_busy_signal, true, false);
 	if (err) {
 		pr_warn("%s: Error %d starting bkops\n",
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 9355366..cb1cf4e 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1012,7 +1012,7 @@ static int mmc_select_hs(struct mmc_card *card)
 
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
-			   card->ext_csd.generic_cmd6_time,
+			   card->ext_csd.generic_cmd6_time, 0,
 			   true, false, true);
 	if (!err) {
 		mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
@@ -1115,7 +1115,7 @@ static int mmc_select_hs400(struct mmc_card *card)
 	val = EXT_CSD_TIMING_HS;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, val,
-			   card->ext_csd.generic_cmd6_time,
+			   card->ext_csd.generic_cmd6_time, 0,
 			   true, false, true);
 	if (err) {
 		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
@@ -1150,7 +1150,7 @@ static int mmc_select_hs400(struct mmc_card *card)
 	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, val,
-			   card->ext_csd.generic_cmd6_time,
+			   card->ext_csd.generic_cmd6_time, 0,
 			   true, false, true);
 	if (err) {
 		pr_err("%s: switch to hs400 failed, err:%d\n",
@@ -1193,7 +1193,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
 	/* Switch HS400 to HS DDR */
 	val = EXT_CSD_TIMING_HS;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
-			   val, card->ext_csd.generic_cmd6_time,
+			   val, card->ext_csd.generic_cmd6_time, 0,
 			   true, false, true);
 	if (err)
 		goto out_err;
@@ -1207,7 +1207,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
 	/* Switch HS DDR to HS */
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
 			   EXT_CSD_BUS_WIDTH_8, card->ext_csd.generic_cmd6_time,
-			   true, false, true);
+			   0, true, false, true);
 	if (err)
 		goto out_err;
 
@@ -1221,7 +1221,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
 	val = EXT_CSD_TIMING_HS200 |
 	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
-			   val, card->ext_csd.generic_cmd6_time,
+			   val, card->ext_csd.generic_cmd6_time, 0,
 			   true, false, true);
 	if (err)
 		goto out_err;
@@ -1295,7 +1295,7 @@ static int mmc_select_hs400es(struct mmc_card *card)
 	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, val,
-			   card->ext_csd.generic_cmd6_time,
+			   card->ext_csd.generic_cmd6_time, 0,
 			   true, false, true);
 	if (err) {
 		pr_err("%s: switch to hs400es failed, err:%d\n",
@@ -1377,7 +1377,7 @@ static int mmc_select_hs200(struct mmc_card *card)
 		      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
 		err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				   EXT_CSD_HS_TIMING, val,
-				   card->ext_csd.generic_cmd6_time,
+				   card->ext_csd.generic_cmd6_time, 0,
 				   true, false, true);
 		if (err)
 			goto err;
@@ -1841,7 +1841,7 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
 
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			EXT_CSD_POWER_OFF_NOTIFICATION,
-			notify_type, timeout, true, false, false);
+			notify_type, timeout, 0, true, false, false);
 	if (err)
 		pr_err("%s: Power Off Notification timed out, %u\n",
 		       mmc_hostname(card->host), timeout);
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index fba5d29..9b2617c 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -530,6 +530,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
  *	@value: value to program into EXT_CSD register
  *	@timeout_ms: timeout (ms) for operation performed by register write,
  *                   timeout of zero implies maximum possible timeout
+ *	@timing: new timing to change to
  *	@use_busy_signal: use the busy signal as response type
  *	@send_status: send status cmd to poll for busy
  *	@retry_crc_err: retry when CRC errors when polling with CMD13 for busy
@@ -537,13 +538,14 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
  *	Modifies the EXT_CSD register for selected card.
  */
 int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
-		unsigned int timeout_ms, bool use_busy_signal, bool send_status,
-		bool retry_crc_err)
+		unsigned int timeout_ms, unsigned char timing,
+		bool use_busy_signal, bool send_status,	bool retry_crc_err)
 {
 	struct mmc_host *host = card->host;
 	int err;
 	struct mmc_command cmd = {0};
 	bool use_r1b_resp = use_busy_signal;
+	unsigned char old_timing = host->ios.timing;
 
 	mmc_retune_hold(host);
 
@@ -585,16 +587,24 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 	if (!use_busy_signal)
 		goto out;
 
+	/* Switch to new timing before poll and check switch status. */
+	if (timing)
+		mmc_set_timing(host, timing);
+
 	/*If SPI or used HW busy detection above, then we don't need to poll. */
 	if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) ||
 		mmc_host_is_spi(host)) {
 		if (send_status)
 			err = mmc_switch_status(card);
-		goto out;
+		goto out_tim;
 	}
 
 	/* Let's try to poll to find out when the command is completed. */
 	err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err);
+
+out_tim:
+	if (err && timing)
+		mmc_set_timing(host, old_timing);
 out:
 	mmc_retune_release(host);
 
@@ -604,8 +614,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		unsigned int timeout_ms)
 {
-	return __mmc_switch(card, set, index, value, timeout_ms, true, true,
-				false);
+	return __mmc_switch(card, set, index, value, timeout_ms, 0,
+			true, true, false);
 }
 EXPORT_SYMBOL_GPL(mmc_switch);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 9fccddb..761cb69 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -29,8 +29,8 @@
 int mmc_can_ext_csd(struct mmc_card *card);
 int mmc_switch_status(struct mmc_card *card);
 int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
-		unsigned int timeout_ms, bool use_busy_signal, bool send_status,
-		bool retry_crc_err);
+		unsigned int timeout_ms, unsigned char timing,
+		bool use_busy_signal, bool send_status,	bool retry_crc_err);
 
 #endif
 
-- 
1.9.1

[PATCH v2 6/7] mmc: core: Allow CMD13 polling when switching to HS mode for mmc

[Ulf Hansson]

In cases when the mmc host doesn't support HW busy detection, polling for a
card being busy by using CMD13 is beneficial. That is because, instead of
waiting a fixed amount of time, 500ms or the generic CMD6 time from
EXT_CSD, we find out a lot sooner when the card stops signaling busy. This
leads to a significant decreased total initialization time for the mmc
card.

However, to allow polling with CMD13 during a bus timing change operation,
such as switching to HS mode, we first need to update the mmc host's bus
timing before starting to poll. Deal with that, simply by providing
MMC_TIMING_MMC_HS as the timing parameter to __mmc_switch() from
mmc_select_hs().

By telling __mmc_switch() to allow polling with CMD13, also makes it
validate the CMD6 status, thus we can remove the corresponding checks.

When switching to HS400ES, the mmc_select_hs() function is called in one of
the intermediate steps. To still prevent CMD13 polling for HS400ES, let's
call the __mmc_switch() function in this path as it enables us to keep
using the existing method.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index cb1cf4e..15dd51c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1012,13 +1012,8 @@ static int mmc_select_hs(struct mmc_card *card)
 
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
-			   card->ext_csd.generic_cmd6_time, 0,
-			   true, false, true);
-	if (!err) {
-		mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
-		err = mmc_switch_status(card);
-	}
-
+			   card->ext_csd.generic_cmd6_time, MMC_TIMING_MMC_HS,
+			   true, true, true);
 	if (err)
 		pr_warn("%s: switch to high-speed failed, err:%d\n",
 			mmc_hostname(card->host), err);
@@ -1268,16 +1263,23 @@ static int mmc_select_hs400es(struct mmc_card *card)
 		goto out_err;
 
 	/* Switch card to HS mode */
-	err = mmc_select_hs(card);
-	if (err)
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
+			   card->ext_csd.generic_cmd6_time, 0,
+			   true, false, true);
+	if (err) {
+		pr_err("%s: switch to hs for hs400es failed, err:%d\n",
+			mmc_hostname(host), err);
 		goto out_err;
+	}
 
-	mmc_set_clock(host, card->ext_csd.hs_max_dtr);
-
+	mmc_set_timing(host, MMC_TIMING_MMC_HS);
 	err = mmc_switch_status(card);
 	if (err)
 		goto out_err;
 
+	mmc_set_clock(host, card->ext_csd.hs_max_dtr);
+
 	/* Switch card to DDR with strobe bit */
 	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-- 
1.9.1

[PATCH v2 7/7] mmc: core: Update CMD13 polling policy when switch to HS DDR mode

[Ulf Hansson]

According to the JEDEC specification, during bus timing change operations
for mmc, sending a CMD13 could trigger CRC errors.

As switching to HS DDR mode indeed causes a bus timing change, polling with
CMD13 to detect card busy, may thus potentially trigger CRC errors.
Currently these errors are treated as the switch to HS DDR mode failed.

To improve this behaviour, let's instead tell __mmc_switch() to retry when
it encounters CRC errors during polling.

Moreover, when switching to HS DDR mode, let's make sure the CMD13 polling
is done by having the mmc host and the mmc card, being configured to
operate at the same selected bus speed timing. Fix this by providing
MMC_TIMING_MMC_DDR52 as the timing parameter to __mmc_switch().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 15dd51c..3268fcd 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1040,10 +1040,12 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
 	ext_csd_bits = (bus_width == MMC_BUS_WIDTH_8) ?
 		EXT_CSD_DDR_BUS_WIDTH_8 : EXT_CSD_DDR_BUS_WIDTH_4;
 
-	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-			EXT_CSD_BUS_WIDTH,
-			ext_csd_bits,
-			card->ext_csd.generic_cmd6_time);
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			   EXT_CSD_BUS_WIDTH,
+			   ext_csd_bits,
+			   card->ext_csd.generic_cmd6_time,
+			   MMC_TIMING_MMC_DDR52,
+			   true, true, true);
 	if (err) {
 		pr_err("%s: switch to bus width %d ddr failed\n",
 			mmc_hostname(host), 1 << bus_width);
@@ -1086,9 +1088,6 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
 	if (err)
 		err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330);
 
-	if (!err)
-		mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
-
 	return err;
 }
 
-- 
1.9.1

Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc

[Ulf Hansson]

On 22 November 2016 at 21:56, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v2:
>
>         - From discussions around the difficulties for how to support CMD13
>         polling for HS200, HS400 HS400ES, I decided to drop those patches for
>         now. It's is particularly due to the need for tuning, after a speed mode
>         switch, that makes it hard to rely on CMD13 polling for these cases.
>         Perhaps we can allow CMD13 polling when switching to the intermediate
>         speed modes, but let's deal with that outside of this series.
>
>         - Folded in a new patch, which checks the SWITCH_ERROR bit in each
>           CMD13 response when polling with CMD13. Patch v2 4/7.
>
>         - No other changes made to the rest of the series.
>
>
>
> Several changes has been made for how and when to use CMD13 as a
> polling-method, to find out when the mmc cards stops signaling busy after
> sending a CMD6. This particularly concerns the cases when switching to new bus
> speed modes, such as HS (high-speed), HS DDR, HS200, HS400 and HS400ES.
>
> Currently CMD13 polling is *not* allowed for these cases, but this was recently
> changed - and which did cause regressions for card detection for some
> platforms.
>
> A simple fix was made to solve these regressions, simply by using a default
> CMD6 generic timeout of 500ms, as to avoid using the fall-back timeout in
> __mmc_switch() of 10min. That greatly improve the situation and one could
> consider the regressions as "solved".
>
> However, this simple fix is still causing unnecessary long card initialization
> times, especially for those mmc hosts that doesn't support HW busy detection
> (using MMC_CAP_WAIT_WHILE_BUSY and/or implements the ->card_busy() host ops).
>
> This because we wait a fixed amount of time (CMD6 generic timeout) to make sure
> the card is done signaling busy, while in practice this happens a lot sooner.
>
> A couple of tests for HS and HS DDR eMMC cards, shows the card only need a few
> ms to complete the bus speed mode changes, thus it's far less than the CMD6
> generic timeout.
>
> To improve this behaviour and shorten the card initialization time, we need to
> allow using CMD13 as polling method to find out when the card stops signaling
> busy. Although, enabling CMD13 polling may also introduce other issues as
> according to the JEDEC spec, it's not the recommended method. Especially it
> mentions that CRC errors may be triggered when sending a CMD13 command during a
> bus timing change.
>
> To deal with these issues, we need to change from ignoring those CRC errors but
> instead continue to treat the card as being busy and continue to poll with
> CMD13. Perhaps this behaviour is one of reasons to why the earlier CMD13 polling
> method didn't work out well.
>
> This series requires extensive testing, please help with that!
>
> I have currently tested it with HS and HS DDR eMMC cards, and for combinations
> of an mmc hosts (mmci) supporting HW busy detection and not (local hacks in
> mmci.c).
>
>
> Ulf Hansson (9):
>   mmc: core: Retry instead of ignore at CRC errors when polling for busy
>   mmc: core: Remove redundant __mmc_send_status()
>   mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
>   mmc: core: Enable __mmc_switch() to change bus speed timing for the
>     host
>   mmc: core: Allow CMD13 polling when switching to HS mode for mmc
>   mmc: core: Update CMD13 polling policy when switch to HS DDR mode
>   mmc: core: Allow CMD13 polling when switch to HS200 mode
>   mmc: core: Allow CMD13 polling when switch to HS400 mode
>   mmc: core: Allow CMD13 polling when switch to HS400ES mode
>
>  drivers/mmc/core/core.c    |   2 +-
>  drivers/mmc/core/mmc.c     | 156 ++++++++++++++-------------------------------
>  drivers/mmc/core/mmc_ops.c |  45 +++++++------
>  drivers/mmc/core/mmc_ops.h |   4 +-
>  4 files changed, 77 insertions(+), 130 deletions(-)

Oops, forgot to clean up the cover letter. Please ignore the summary
of changes above. It's the below changes that are the v2 series.

Apologize for the inconvenience!

Kind regards
Uffe

>
> --
> 1.9.1
>
>
>
>
>
>
> *** BLURB HERE ***
>
> Ulf Hansson (7):
>   mmc: core: Retry instead of ignore at CRC errors when polling for busy
>   mmc: core: Remove redundant __mmc_send_status()
>   mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
>   mmc: core: Check SWITCH_ERROR bit from each CMD13 response when
>     polling
>   mmc: core: Enable __mmc_switch() to change bus speed timing for the
>     host
>   mmc: core: Allow CMD13 polling when switching to HS mode for mmc
>   mmc: core: Update CMD13 polling policy when switch to HS DDR mode
>
>  drivers/mmc/core/core.c    |  2 +-
>  drivers/mmc/core/mmc.c     | 53 +++++++++++++++++++++++-----------------------
>  drivers/mmc/core/mmc_ops.c | 51 ++++++++++++++++++++++++++------------------
>  drivers/mmc/core/mmc_ops.h |  4 ++--
>  4 files changed, 60 insertions(+), 50 deletions(-)
>
> --
> 1.9.1
>

Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc

[Linus Walleij]

On Tue, Nov 22, 2016 at 9:56 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:

> Changes in v2:
>
>         - From discussions around the difficulties for how to support CMD13
>         polling for HS200, HS400 HS400ES, I decided to drop those patches for
>         now. It's is particularly due to the need for tuning, after a speed mode
>         switch, that makes it hard to rely on CMD13 polling for these cases.
>         Perhaps we can allow CMD13 polling when switching to the intermediate
>         speed modes, but let's deal with that outside of this series.

ALso v2 works like a charm on APQ8060 Dragonboard:
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc

[Adrian Hunter]

On 22/11/16 22:56, Ulf Hansson wrote:
> Changes in v2:
> 
> 	- From discussions around the difficulties for how to support CMD13
> 	polling for HS200, HS400 HS400ES, I decided to drop those patches for
> 	now. It's is particularly due to the need for tuning, after a speed mode
> 	switch, that makes it hard to rely on CMD13 polling for these cases.
> 	Perhaps we can allow CMD13 polling when switching to the intermediate
> 	speed modes, but let's deal with that outside of this series.
> 
> 	- Folded in a new patch, which checks the SWITCH_ERROR bit in each
> 	  CMD13 response when polling with CMD13. Patch v2 4/7.
> 
> 	- No other changes made to the rest of the series.
> 

For the whole series:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc

[Ulf Hansson]

On 22 November 2016 at 22:01, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 22 November 2016 at 21:56, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Changes in v2:
>>
>>         - From discussions around the difficulties for how to support CMD13
>>         polling for HS200, HS400 HS400ES, I decided to drop those patches for
>>         now. It's is particularly due to the need for tuning, after a speed mode
>>         switch, that makes it hard to rely on CMD13 polling for these cases.
>>         Perhaps we can allow CMD13 polling when switching to the intermediate
>>         speed modes, but let's deal with that outside of this series.
>>
>>         - Folded in a new patch, which checks the SWITCH_ERROR bit in each
>>           CMD13 response when polling with CMD13. Patch v2 4/7.
>>
>>         - No other changes made to the rest of the series.
>>
>>
>>
>> Several changes has been made for how and when to use CMD13 as a
>> polling-method, to find out when the mmc cards stops signaling busy after
>> sending a CMD6. This particularly concerns the cases when switching to new bus
>> speed modes, such as HS (high-speed), HS DDR, HS200, HS400 and HS400ES.
>>
>> Currently CMD13 polling is *not* allowed for these cases, but this was recently
>> changed - and which did cause regressions for card detection for some
>> platforms.
>>
>> A simple fix was made to solve these regressions, simply by using a default
>> CMD6 generic timeout of 500ms, as to avoid using the fall-back timeout in
>> __mmc_switch() of 10min. That greatly improve the situation and one could
>> consider the regressions as "solved".
>>
>> However, this simple fix is still causing unnecessary long card initialization
>> times, especially for those mmc hosts that doesn't support HW busy detection
>> (using MMC_CAP_WAIT_WHILE_BUSY and/or implements the ->card_busy() host ops).
>>
>> This because we wait a fixed amount of time (CMD6 generic timeout) to make sure
>> the card is done signaling busy, while in practice this happens a lot sooner.
>>
>> A couple of tests for HS and HS DDR eMMC cards, shows the card only need a few
>> ms to complete the bus speed mode changes, thus it's far less than the CMD6
>> generic timeout.
>>
>> To improve this behaviour and shorten the card initialization time, we need to
>> allow using CMD13 as polling method to find out when the card stops signaling
>> busy. Although, enabling CMD13 polling may also introduce other issues as
>> according to the JEDEC spec, it's not the recommended method. Especially it
>> mentions that CRC errors may be triggered when sending a CMD13 command during a
>> bus timing change.
>>
>> To deal with these issues, we need to change from ignoring those CRC errors but
>> instead continue to treat the card as being busy and continue to poll with
>> CMD13. Perhaps this behaviour is one of reasons to why the earlier CMD13 polling
>> method didn't work out well.
>>
>> This series requires extensive testing, please help with that!
>>
>> I have currently tested it with HS and HS DDR eMMC cards, and for combinations
>> of an mmc hosts (mmci) supporting HW busy detection and not (local hacks in
>> mmci.c).
>>
>>
>> Ulf Hansson (9):
>>   mmc: core: Retry instead of ignore at CRC errors when polling for busy
>>   mmc: core: Remove redundant __mmc_send_status()
>>   mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
>>   mmc: core: Enable __mmc_switch() to change bus speed timing for the
>>     host
>>   mmc: core: Allow CMD13 polling when switching to HS mode for mmc
>>   mmc: core: Update CMD13 polling policy when switch to HS DDR mode
>>   mmc: core: Allow CMD13 polling when switch to HS200 mode
>>   mmc: core: Allow CMD13 polling when switch to HS400 mode
>>   mmc: core: Allow CMD13 polling when switch to HS400ES mode
>>
>>  drivers/mmc/core/core.c    |   2 +-
>>  drivers/mmc/core/mmc.c     | 156 ++++++++++++++-------------------------------
>>  drivers/mmc/core/mmc_ops.c |  45 +++++++------
>>  drivers/mmc/core/mmc_ops.h |   4 +-
>>  4 files changed, 77 insertions(+), 130 deletions(-)
>
> Oops, forgot to clean up the cover letter. Please ignore the summary
> of changes above. It's the below changes that are the v2 series.
>
> Apologize for the inconvenience!
>
> Kind regards
> Uffe
>
>>
>> --
>> 1.9.1
>>
>>
>>
>>
>>
>>
>> *** BLURB HERE ***
>>
>> Ulf Hansson (7):
>>   mmc: core: Retry instead of ignore at CRC errors when polling for busy
>>   mmc: core: Remove redundant __mmc_send_status()
>>   mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
>>   mmc: core: Check SWITCH_ERROR bit from each CMD13 response when
>>     polling
>>   mmc: core: Enable __mmc_switch() to change bus speed timing for the
>>     host
>>   mmc: core: Allow CMD13 polling when switching to HS mode for mmc
>>   mmc: core: Update CMD13 polling policy when switch to HS DDR mode
>>
>>  drivers/mmc/core/core.c    |  2 +-
>>  drivers/mmc/core/mmc.c     | 53 +++++++++++++++++++++++-----------------------
>>  drivers/mmc/core/mmc_ops.c | 51 ++++++++++++++++++++++++++------------------
>>  drivers/mmc/core/mmc_ops.h |  4 ++--
>>  4 files changed, 60 insertions(+), 50 deletions(-)
>>
>> --
>> 1.9.1
>>

I have applied this for next, let's see what the kernelci reports.

More tests is still warmly appreciated, and I can still add
tested/reviewed-by tags.

Kind regards
Uffe