Linux-mmc Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch()
@ 2020-01-22 14:27 Ulf Hansson
  2020-01-22 14:27 ` [PATCH 1/3] mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC Ulf Hansson
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ulf Hansson @ 2020-01-22 14:27 UTC (permalink / raw)
  To: linux-mmc, Ulf Hansson
  Cc: Adrian Hunter, Chaotian Jing, Shawn Lin, mirq-linux

This small series updates timeouts used when sending CMD6 for eMMC, through the
__mmc_switch() function.

Ulf Hansson (3):
  mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC
  mmc: block: Use generic_cmd6_time when modifying
    INAND_CMD38_ARG_EXT_CSD
  mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch()

 drivers/mmc/core/block.c   |  6 +++---
 drivers/mmc/core/mmc_ops.c | 34 +++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 20 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC
  2020-01-22 14:27 [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
@ 2020-01-22 14:27 ` Ulf Hansson
  2020-01-22 14:27 ` [PATCH 2/3] mmc: block: Use generic_cmd6_time when modifying INAND_CMD38_ARG_EXT_CSD Ulf Hansson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2020-01-22 14:27 UTC (permalink / raw)
  To: linux-mmc, Ulf Hansson
  Cc: Adrian Hunter, Chaotian Jing, Shawn Lin, mirq-linux

The timeout values used while waiting for a CMD6 for BKOPS or a CACHE_FLUSH
to complete, are not defined by the eMMC spec. However, a timeout of 10
minutes as is currently being used, is just silly for both of these cases.
Instead, let's specify more reasonable timeouts, 120s for BKOPS and 30s for
CACHE_FLUSH.

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 09113b9ad679..1966abcbc7c0 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -19,7 +19,9 @@
 #include "host.h"
 #include "mmc_ops.h"
 
-#define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */
+#define MMC_OPS_TIMEOUT_MS		(10 * 60 * 1000) /* 10min*/
+#define MMC_BKOPS_TIMEOUT_MS		(120 * 1000) /* 120s */
+#define MMC_CACHE_FLUSH_TIMEOUT_MS	(30 * 1000) /* 30s */
 
 static const u8 tuning_blk_pattern_4bit[] = {
 	0xff, 0x0f, 0xff, 0x00, 0xff, 0xcc, 0xc3, 0xcc,
@@ -941,7 +943,7 @@ void mmc_run_bkops(struct mmc_card *card)
 	 * urgent levels by using an asynchronous background task, when idle.
 	 */
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-			EXT_CSD_BKOPS_START, 1, MMC_OPS_TIMEOUT_MS);
+			 EXT_CSD_BKOPS_START, 1, MMC_BKOPS_TIMEOUT_MS);
 	if (err)
 		pr_warn("%s: Error %d starting bkops\n",
 			mmc_hostname(card->host), err);
@@ -961,7 +963,8 @@ int mmc_flush_cache(struct mmc_card *card)
 			(card->ext_csd.cache_size > 0) &&
 			(card->ext_csd.cache_ctrl & 1)) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-				EXT_CSD_FLUSH_CACHE, 1, 0);
+				 EXT_CSD_FLUSH_CACHE, 1,
+				 MMC_CACHE_FLUSH_TIMEOUT_MS);
 		if (err)
 			pr_err("%s: cache flush error %d\n",
 					mmc_hostname(card->host), err);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] mmc: block: Use generic_cmd6_time when modifying INAND_CMD38_ARG_EXT_CSD
  2020-01-22 14:27 [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
  2020-01-22 14:27 ` [PATCH 1/3] mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC Ulf Hansson
@ 2020-01-22 14:27 ` Ulf Hansson
  2020-01-22 14:27 ` [PATCH 3/3] mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch() Ulf Hansson
  2020-01-24 11:25 ` [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
  3 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2020-01-22 14:27 UTC (permalink / raw)
  To: linux-mmc, Ulf Hansson
  Cc: Adrian Hunter, Chaotian Jing, Shawn Lin, mirq-linux

The INAND_CMD38_ARG_EXT_CSD is a vendor specific EXT_CSD register, which is
used to prepare an erase/trim operation. However, it doesn't make sense to
use a timeout of 10 minutes while updating the register, which becomes the
case when the timeout_ms argument for mmc_switch() is set to zero.

Instead, let's use the generic_cmd6_time, as that seems like a reasonable
timeout to use for these cases.

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

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 95b41c0891d0..663d87924e5e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1107,7 +1107,7 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
 					 card->erase_arg == MMC_TRIM_ARG ?
 					 INAND_CMD38_ARG_TRIM :
 					 INAND_CMD38_ARG_ERASE,
-					 0);
+					 card->ext_csd.generic_cmd6_time);
 		}
 		if (!err)
 			err = mmc_erase(card, from, nr, card->erase_arg);
@@ -1149,7 +1149,7 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
 				 arg == MMC_SECURE_TRIM1_ARG ?
 				 INAND_CMD38_ARG_SECTRIM1 :
 				 INAND_CMD38_ARG_SECERASE,
-				 0);
+				 card->ext_csd.generic_cmd6_time);
 		if (err)
 			goto out_retry;
 	}
@@ -1167,7 +1167,7 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
 			err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 					 INAND_CMD38_ARG_EXT_CSD,
 					 INAND_CMD38_ARG_SECTRIM2,
-					 0);
+					 card->ext_csd.generic_cmd6_time);
 			if (err)
 				goto out_retry;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch()
  2020-01-22 14:27 [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
  2020-01-22 14:27 ` [PATCH 1/3] mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC Ulf Hansson
  2020-01-22 14:27 ` [PATCH 2/3] mmc: block: Use generic_cmd6_time when modifying INAND_CMD38_ARG_EXT_CSD Ulf Hansson
@ 2020-01-22 14:27 ` Ulf Hansson
  2020-01-24 11:25 ` [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
  3 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2020-01-22 14:27 UTC (permalink / raw)
  To: linux-mmc, Ulf Hansson
  Cc: Adrian Hunter, Chaotian Jing, Shawn Lin, mirq-linux

All callers of __mmc_switch() should now be specifying a valid timeout for
the CMD6 command. However, to sure let's print a warning and default to use
the generic_cmd6_time in case the provided timeout_ms argument is zero.

In this context, let's also simplify some of corresponding code and clarify
some related comments.

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

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 1966abcbc7c0..da425ee2d9bf 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -460,10 +460,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
 	bool expired = false;
 	bool busy = false;
 
-	/* We have an unspecified cmd timeout, use the fallback value. */
-	if (!timeout_ms)
-		timeout_ms = MMC_OPS_TIMEOUT_MS;
-
 	/*
 	 * In cases when not allowed to poll by using CMD13 or because we aren't
 	 * capable of polling by using ->card_busy(), then rely on waiting the
@@ -536,14 +532,19 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 
 	mmc_retune_hold(host);
 
+	if (!timeout_ms) {
+		pr_warn("%s: unspecified timeout for CMD6 - use generic\n",
+			mmc_hostname(host));
+		timeout_ms = card->ext_csd.generic_cmd6_time;
+	}
+
 	/*
-	 * If the cmd timeout and the max_busy_timeout of the host are both
-	 * specified, let's validate them. A failure means we need to prevent
-	 * the host from doing hw busy detection, which is done by converting
-	 * to a R1 response instead of a R1B.
+	 * If the max_busy_timeout of the host is specified, make sure it's
+	 * enough to fit the used timeout_ms. In case it's not, let's instruct
+	 * the host to avoid HW busy detection, by converting to a R1 response
+	 * instead of a R1B.
 	 */
-	if (timeout_ms && host->max_busy_timeout &&
-		(timeout_ms > host->max_busy_timeout))
+	if (host->max_busy_timeout && (timeout_ms > host->max_busy_timeout))
 		use_r1b_resp = false;
 
 	cmd.opcode = MMC_SWITCH;
@@ -554,10 +555,6 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 	cmd.flags = MMC_CMD_AC;
 	if (use_r1b_resp) {
 		cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
-		/*
-		 * A busy_timeout of zero means the host can decide to use
-		 * whatever value it finds suitable.
-		 */
 		cmd.busy_timeout = timeout_ms;
 	} else {
 		cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch()
  2020-01-22 14:27 [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
                   ` (2 preceding siblings ...)
  2020-01-22 14:27 ` [PATCH 3/3] mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch() Ulf Hansson
@ 2020-01-24 11:25 ` Ulf Hansson
  3 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2020-01-24 11:25 UTC (permalink / raw)
  To: linux-mmc
  Cc: Adrian Hunter, Chaotian Jing, Shawn Lin,
	Michał Mirosław, Ulf Hansson

On Wed, 22 Jan 2020 at 15:27, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> This small series updates timeouts used when sending CMD6 for eMMC, through the
> __mmc_switch() function.
>
> Ulf Hansson (3):
>   mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC
>   mmc: block: Use generic_cmd6_time when modifying
>     INAND_CMD38_ARG_EXT_CSD
>   mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch()
>
>  drivers/mmc/core/block.c   |  6 +++---
>  drivers/mmc/core/mmc_ops.c | 34 +++++++++++++++++-----------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
>
> --
> 2.17.1
>

To widen the test cases, I have queued up this for next, also by
amending patch3 for fixing some spelling mistakes in the changelog.

Please tell if you see any problems with this and feel free to provide
tested by tags.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-22 14:27 [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson
2020-01-22 14:27 ` [PATCH 1/3] mmc: core: Specify timeouts for BKOPS and CACHE_FLUSH for eMMC Ulf Hansson
2020-01-22 14:27 ` [PATCH 2/3] mmc: block: Use generic_cmd6_time when modifying INAND_CMD38_ARG_EXT_CSD Ulf Hansson
2020-01-22 14:27 ` [PATCH 3/3] mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch() Ulf Hansson
2020-01-24 11:25 ` [PATCH 0/3] mmc: core: Update timeouts for __mmc_switch() Ulf Hansson

Linux-mmc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mmc/0 linux-mmc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mmc linux-mmc/ https://lore.kernel.org/linux-mmc \
		linux-mmc@vger.kernel.org
	public-inbox-index linux-mmc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mmc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git