These 3 commits from upstream allow us to have more fine grained control over the MMC command timeouts and this solves the following timeouts that we have seen on our systems across suspend/resume cycles:
[ 14.907496] usb usb2: root hub lost power or was reset [ 15.216232] usb 1-1: reset high-speed USB device number 2 using xhci-hcd [ 15.485812] bcmgenet 8f00000.ethernet eth0: Link is Down [ 15.525328] mmc1: error -110 doing runtime resume [ 15.531864] OOM killer enabled.
Thanks!
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 | 25 +++++++++++++------------ 2 files changed, 16 insertions(+), 15 deletions(-)
From: Ulf Hansson ulf.hansson@linaro.org
commit 24ed3bd01d6a844fd5e8a75f48d0a3d10ed71bf9 upstream
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 Link: https://lore.kernel.org/r/20200122142747.5690-2-ulf.hansson@linaro.org Signed-off-by: Florian Fainelli f.fainelli@gmail.com --- 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 334678707deb..3e10023cf485 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -23,7 +23,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, @@ -986,7 +988,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, 0, + EXT_CSD_BKOPS_START, 1, MMC_BKOPS_TIMEOUT_MS, 0, use_busy_signal, true, false); if (err) { pr_warn("%s: Error %d starting bkops\n", @@ -1016,7 +1018,8 @@ int mmc_flush_cache(struct mmc_card *card)
if (mmc_cache_enabled(card->host)) { 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);
From: Ulf Hansson ulf.hansson@linaro.org
commit ad91619aa9d78ab1c6d4a969c3db68bc331ae76c upstream
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 Link: https://lore.kernel.org/r/20200122142747.5690-3-ulf.hansson@linaro.org Signed-off-by: Florian Fainelli f.fainelli@gmail.com --- 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 630f3bcba56d..f7379c3473e1 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1133,7 +1133,7 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req) 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, arg); @@ -1175,7 +1175,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; } @@ -1193,7 +1193,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; }
From: Ulf Hansson ulf.hansson@linaro.org
commit 533a6cfe08f96a7b5c65e06d20916d552c11b256 upstream
All callers of __mmc_switch() should now be specifying a valid timeout for the CMD6 command. However, just to be 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 the corresponding code and clarify some related comments.
Signed-off-by: Ulf Hansson ulf.hansson@linaro.org Link: https://lore.kernel.org/r/20200122142747.5690-4-ulf.hansson@linaro.org Signed-off-by: Florian Fainelli f.fainelli@gmail.com --- drivers/mmc/core/mmc_ops.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index 3e10023cf485..26783a39c547 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -458,10 +458,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 @@ -534,6 +530,12 @@ 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 @@ -542,7 +544,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, * which also means they are on their own when it comes to deal with the * busy timeout. */ - if (!(host->caps & MMC_CAP_NEED_RSP_BUSY) && timeout_ms && + if (!(host->caps & MMC_CAP_NEED_RSP_BUSY) && host->max_busy_timeout && (timeout_ms > host->max_busy_timeout)) use_r1b_resp = false;
@@ -554,10 +556,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;
On 5/17/2022 11:22 AM, Florian Fainelli wrote:
These 3 commits from upstream allow us to have more fine grained control over the MMC command timeouts and this solves the following timeouts that we have seen on our systems across suspend/resume cycles:
[ 14.907496] usb usb2: root hub lost power or was reset [ 15.216232] usb 1-1: reset high-speed USB device number 2 using xhci-hcd [ 15.485812] bcmgenet 8f00000.ethernet eth0: Link is Down [ 15.525328] mmc1: error -110 doing runtime resume [ 15.531864] OOM killer enabled.
Thanks!
Looks like I managed to introduce a build warning due to the unused timeout variable, let me submit a fresher version for 4.19, 4.14 and 4.9.
On 5/19/2022 10:38 AM, Florian Fainelli wrote:
On 5/17/2022 11:22 AM, Florian Fainelli wrote:
These 3 commits from upstream allow us to have more fine grained control over the MMC command timeouts and this solves the following timeouts that we have seen on our systems across suspend/resume cycles:
[ 14.907496] usb usb2: root hub lost power or was reset [ 15.216232] usb 1-1: reset high-speed USB device number 2 using xhci-hcd [ 15.485812] bcmgenet 8f00000.ethernet eth0: Link is Down [ 15.525328] mmc1: error -110 doing runtime resume [ 15.531864] OOM killer enabled.
Thanks!
Looks like I managed to introduce a build warning due to the unused timeout variable, let me submit a fresher version for 4.19, 4.14 and 4.9.
Only v4.19 and v4.14 required a v2, you can find both here:
https://lore.kernel.org/lkml/20220519184536.370540-1-f.fainelli@gmail.com/T/...
https://lore.kernel.org/lkml/20220519190030.377695-1-f.fainelli@gmail.com/T/...
Sorry about that, I will build with W=1 in the future to notice those set but unused variable warnings.
Thanks!
On Thu, May 19, 2022 at 12:04:59PM -0700, Florian Fainelli wrote:
On 5/19/2022 10:38 AM, Florian Fainelli wrote:
On 5/17/2022 11:22 AM, Florian Fainelli wrote:
These 3 commits from upstream allow us to have more fine grained control over the MMC command timeouts and this solves the following timeouts that we have seen on our systems across suspend/resume cycles:
[ 14.907496] usb usb2: root hub lost power or was reset [ 15.216232] usb 1-1: reset high-speed USB device number 2 using xhci-hcd [ 15.485812] bcmgenet 8f00000.ethernet eth0: Link is Down [ 15.525328] mmc1: error -110 doing runtime resume [ 15.531864] OOM killer enabled.
Thanks!
Looks like I managed to introduce a build warning due to the unused timeout variable, let me submit a fresher version for 4.19, 4.14 and 4.9.
Only v4.19 and v4.14 required a v2, you can find both here:
https://lore.kernel.org/lkml/20220519184536.370540-1-f.fainelli@gmail.com/T/...
https://lore.kernel.org/lkml/20220519190030.377695-1-f.fainelli@gmail.com/T/...
Sorry about that, I will build with W=1 in the future to notice those set but unused variable warnings.
I've queued up the updates now, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Florian Fainelli -
Greg Kroah-Hartman