2021-10-22 06:40:40

by Huijin Park

[permalink] [raw]
Subject: [PATCH 1/2] mmc: core: adjust polling interval for CMD1

In mmc_send_op_cond(), loops are continuously performed at the same
interval of 10 ms. However the behaviour is not good for some eMMC
which can be out from a busy state earlier than 10 ms if normal.

Therefore, this patch adjusts the waiting interval time. The interval
time starts at 1 ms, but doubles until the range reaches 10 ms for
each loop.

The reason for adjusting the interval time is that it is important
to reduce the eMMC initialization time, especially in devices that
use eMMC as rootfs.

Test log(eMMC:KLM8G1GETF-B041):

before: 12 ms (0.439407 - 0.427186)
[0.419407] mmc0: starting CMD0 arg 00000000 flags 000000c0
[0.422652] mmc0: starting CMD1 arg 00000000 flags 000000e1
[0.424270] mmc0: starting CMD0 arg 00000000 flags 000000c0
[0.427186] mmc0: starting CMD1 arg 40000080 flags 000000e1<-start
[0.439407] mmc0: starting CMD1 arg 40000080 flags 000000e1<-finish
[0.439721] mmc0: starting CMD2 arg 00000000 flags 00000007

after: 4 ms (0.431725 - 0.427352)
[0.419575] mmc0: starting CMD0 arg 00000000 flags 000000c0
[0.422819] mmc0: starting CMD1 arg 00000000 flags 000000e1
[0.424435] mmc0: starting CMD0 arg 00000000 flags 000000c0
[0.427352] mmc0: starting CMD1 arg 40000080 flags 000000e1<-start
[0.428913] mmc0: starting CMD1 arg 40000080 flags 000000e1
[0.431725] mmc0: starting CMD1 arg 40000080 flags 000000e1<-finish
[0.432038] mmc0: starting CMD2 arg 00000000 flags 00000007

Signed-off-by: Huijin Park <[email protected]>

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0c54858e89c0..61b4ffdc89ce 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -177,6 +177,7 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
{
struct mmc_command cmd = {};
int i, err = 0;
+ int interval = 1, interval_max = 10;

cmd.opcode = MMC_SEND_OP_COND;
cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
@@ -198,7 +199,9 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)

err = -ETIMEDOUT;

- mmc_delay(10);
+ mmc_delay(interval);
+ if (interval < interval_max)
+ interval = min(interval * 2, interval_max);

/*
* According to eMMC specification v5.1 section 6.4.3, we
--
2.17.1


2021-10-22 06:41:25

by Huijin Park

[permalink] [raw]
Subject: [PATCH 2/2] mmc: core: use jiffies to checking timeout for CMD1

This patch uses jiffies for checking timeout instead of loop count.
Because the previous patch which is adjusting interval time changes
the timeout value. Besides using jiffies is more clearly for checking
timeout instead of counting loop.

Signed-off-by: Huijin Park <[email protected]>

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 61b4ffdc89ce..f48216d65d2c 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -176,14 +176,16 @@ int mmc_go_idle(struct mmc_host *host)
int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
{
struct mmc_command cmd = {};
- int i, err = 0;
+ int err = 0;
int interval = 1, interval_max = 10;
+ unsigned long timeout;

cmd.opcode = MMC_SEND_OP_COND;
cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR;

- for (i = 100; i; i--) {
+ timeout = jiffies + msecs_to_jiffies(1000);
+ while (true) {
err = mmc_wait_for_cmd(host, &cmd, 0);
if (err)
break;
@@ -197,7 +199,12 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
break;
}

- err = -ETIMEDOUT;
+ if (time_after(jiffies, timeout)) {
+ pr_err("%s: Card stuck being busy! %s\n",
+ mmc_hostname(host), __func__);
+ err = -ETIMEDOUT;
+ break;
+ }

mmc_delay(interval);
if (interval < interval_max)
--
2.17.1

2021-10-27 01:33:51

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 1/2] mmc: core: adjust polling interval for CMD1

On Fri, 22 Oct 2021 at 08:39, Huijin Park <[email protected]> wrote:
>
> In mmc_send_op_cond(), loops are continuously performed at the same
> interval of 10 ms. However the behaviour is not good for some eMMC
> which can be out from a busy state earlier than 10 ms if normal.
>
> Therefore, this patch adjusts the waiting interval time. The interval
> time starts at 1 ms, but doubles until the range reaches 10 ms for
> each loop.
>
> The reason for adjusting the interval time is that it is important
> to reduce the eMMC initialization time, especially in devices that
> use eMMC as rootfs.
>
> Test log(eMMC:KLM8G1GETF-B041):
>
> before: 12 ms (0.439407 - 0.427186)
> [0.419407] mmc0: starting CMD0 arg 00000000 flags 000000c0
> [0.422652] mmc0: starting CMD1 arg 00000000 flags 000000e1
> [0.424270] mmc0: starting CMD0 arg 00000000 flags 000000c0
> [0.427186] mmc0: starting CMD1 arg 40000080 flags 000000e1<-start
> [0.439407] mmc0: starting CMD1 arg 40000080 flags 000000e1<-finish
> [0.439721] mmc0: starting CMD2 arg 00000000 flags 00000007
>
> after: 4 ms (0.431725 - 0.427352)
> [0.419575] mmc0: starting CMD0 arg 00000000 flags 000000c0
> [0.422819] mmc0: starting CMD1 arg 00000000 flags 000000e1
> [0.424435] mmc0: starting CMD0 arg 00000000 flags 000000c0
> [0.427352] mmc0: starting CMD1 arg 40000080 flags 000000e1<-start
> [0.428913] mmc0: starting CMD1 arg 40000080 flags 000000e1
> [0.431725] mmc0: starting CMD1 arg 40000080 flags 000000e1<-finish
> [0.432038] mmc0: starting CMD2 arg 00000000 flags 00000007
>
> Signed-off-by: Huijin Park <[email protected]>
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 0c54858e89c0..61b4ffdc89ce 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -177,6 +177,7 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
> {
> struct mmc_command cmd = {};
> int i, err = 0;
> + int interval = 1, interval_max = 10;
>
> cmd.opcode = MMC_SEND_OP_COND;
> cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
> @@ -198,7 +199,9 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
>
> err = -ETIMEDOUT;
>
> - mmc_delay(10);
> + mmc_delay(interval);
> + if (interval < interval_max)
> + interval = min(interval * 2, interval_max);

It looks like we should be able to replace the above polling loop with
__mmc_poll_for_busy(). We would need a callback function and a
callback data, specific for CMD1, but that looks far better to me, if
we can get that to work.

Would you mind having a look?

>
> /*
> * According to eMMC specification v5.1 section 6.4.3, we

Kind regards
Uffe