[PATCH v2 0/2] mmc: core: adjust polling interval for CMD1

[PATCH v2 0/2] mmc: core: adjust polling interval for CMD1

[Huijin Park]

This series is for adjusting polling interval for CMD1 using
__mmc_poll_for_busy() common function which provides a stricter
timeout checking and a throttling mechanism.

v1...v2:
 - Change __mmc_poll_for_busy() first parameter type to 'mmc_host*'.
 - Use __mmc_poll_for_busy() common function.

Huijin Park (2):
  mmc: core: change __mmc_poll_for_busy() parameter type
  mmc: core: adjust polling interval for CMD1

 drivers/mmc/core/block.c   |  4 +-
 drivers/mmc/core/mmc.c     |  2 +-
 drivers/mmc/core/mmc_ops.c | 89 ++++++++++++++++++++++++--------------
 drivers/mmc/core/mmc_ops.h |  2 +-
 drivers/mmc/core/sd.c      |  2 +-
 5 files changed, 62 insertions(+), 37 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] mmc: core: change __mmc_poll_for_busy() parameter type

[Huijin Park]

This patch changes the __mmc_poll_for_busy() first parameter type
from 'struct mmc_card*' to 'struct mmc_host*'.
Because the function refers only 'struct mmc_host' to get hostname.

Signed-off-by: Huijin Park <huijin.park@samsung.com>

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 61590cf7a7b1..5a39026e5b0f 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1908,8 +1908,8 @@ static int mmc_blk_card_busy(struct mmc_card *card, struct request *req)
 
 	cb_data.card = card;
 	cb_data.status = 0;
-	err = __mmc_poll_for_busy(card, MMC_BLK_TIMEOUT_MS, &mmc_blk_busy_cb,
-				  &cb_data);
+	err = __mmc_poll_for_busy(card->host, MMC_BLK_TIMEOUT_MS,
+				  &mmc_blk_busy_cb, &cb_data);
 
 	/*
 	 * Do not assume data transferred correctly if there are any error bits
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index b1c1716dacf0..bbbbcaf70a59 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1962,7 +1962,7 @@ static int mmc_sleep(struct mmc_host *host)
 		goto out_release;
 	}
 
-	err = __mmc_poll_for_busy(card, timeout_ms, &mmc_sleep_busy_cb, host);
+	err = __mmc_poll_for_busy(host, timeout_ms, &mmc_sleep_busy_cb, host);
 
 out_release:
 	mmc_retune_release(host);
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0c54858e89c0..9946733a34c6 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -470,11 +470,10 @@ static int mmc_busy_cb(void *cb_data, bool *busy)
 	return 0;
 }
 
-int __mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
+int __mmc_poll_for_busy(struct mmc_host *host, unsigned int timeout_ms,
 			int (*busy_cb)(void *cb_data, bool *busy),
 			void *cb_data)
 {
-	struct mmc_host *host = card->host;
 	int err;
 	unsigned long timeout;
 	unsigned int udelay = 32, udelay_max = 32768;
@@ -515,13 +514,14 @@ EXPORT_SYMBOL_GPL(__mmc_poll_for_busy);
 int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
 		      bool retry_crc_err, enum mmc_busy_cmd busy_cmd)
 {
+	struct mmc_host *host = card->host;
 	struct mmc_busy_data cb_data;
 
 	cb_data.card = card;
 	cb_data.retry_crc_err = retry_crc_err;
 	cb_data.busy_cmd = busy_cmd;
 
-	return __mmc_poll_for_busy(card, timeout_ms, &mmc_busy_cb, &cb_data);
+	return __mmc_poll_for_busy(host, timeout_ms, &mmc_busy_cb, &cb_data);
 }
 EXPORT_SYMBOL_GPL(mmc_poll_for_busy);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index e5e94567a9a9..9c813b851d0b 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -41,7 +41,7 @@ int mmc_can_ext_csd(struct mmc_card *card);
 int mmc_switch_status(struct mmc_card *card, bool crc_err_fatal);
 bool mmc_prepare_busy_cmd(struct mmc_host *host, struct mmc_command *cmd,
 			  unsigned int timeout_ms);
-int __mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
+int __mmc_poll_for_busy(struct mmc_host *host, unsigned int timeout_ms,
 			int (*busy_cb)(void *cb_data, bool *busy),
 			void *cb_data);
 int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 4646b7a03db6..e223275bbad1 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1665,7 +1665,7 @@ static int sd_poweroff_notify(struct mmc_card *card)
 
 	cb_data.card = card;
 	cb_data.reg_buf = reg_buf;
-	err = __mmc_poll_for_busy(card, SD_POWEROFF_NOTIFY_TIMEOUT_MS,
+	err = __mmc_poll_for_busy(card->host, SD_POWEROFF_NOTIFY_TIMEOUT_MS,
 				  &sd_busy_poweroff_notify_cb, &cb_data);
 
 out:
-- 
2.17.1

[PATCH v2 2/2] mmc: core: adjust polling interval for CMD1

[Huijin Park]

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.

Rather than fixing about the interval time in mmc_send_op_cond(),
let's instead convert into using the common __mmc_poll_for_busy().

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.311016 - 0.298729)
[    0.295823] mmc0: starting CMD0 arg 00000000 flags 000000c0
[    0.298729] mmc0: starting CMD1 arg 40000080 flags 000000e1<-start
[    0.311016] mmc0: starting CMD1 arg 40000080 flags 000000e1<-finish
[    0.311336] mmc0: starting CMD2 arg 00000000 flags 00000007

after: 2 ms (0.301270 - 0.298762)
[    0.295862] mmc0: starting CMD0 arg 00000000 flags 000000c0
[    0.298762] mmc0: starting CMD1 arg 40000080 flags 000000e1<-start
[    0.299067] mmc0: starting CMD1 arg 40000080 flags 000000e1
[    0.299441] mmc0: starting CMD1 arg 40000080 flags 000000e1
[    0.299879] mmc0: starting CMD1 arg 40000080 flags 000000e1
[    0.300446] mmc0: starting CMD1 arg 40000080 flags 000000e1
[    0.301270] mmc0: starting CMD1 arg 40000080 flags 000000e1<-finish
[    0.301572] mmc0: starting CMD2 arg 00000000 flags 00000007

Signed-off-by: Huijin Park <huijin.park@samsung.com>

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 9946733a34c6..d63d1c735335 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -58,6 +58,12 @@ struct mmc_busy_data {
 	enum mmc_busy_cmd busy_cmd;
 };
 
+struct mmc_op_cond_busy_data {
+	struct mmc_host *host;
+	u32 ocr;
+	struct mmc_command *cmd;
+};
+
 int __mmc_send_status(struct mmc_card *card, u32 *status, unsigned int retries)
 {
 	int err;
@@ -173,43 +179,62 @@ int mmc_go_idle(struct mmc_host *host)
 	return err;
 }
 
+static int __mmc_send_op_cond_cb(void *cb_data, bool *busy)
+{
+	struct mmc_op_cond_busy_data *data = cb_data;
+	struct mmc_host *host = data->host;
+	struct mmc_command *cmd = data->cmd;
+	u32 ocr = data->ocr;
+	int err = 0;
+
+	err = mmc_wait_for_cmd(host, cmd, 0);
+	if (err)
+		return err;
+
+	if (mmc_host_is_spi(host)) {
+		if (!(cmd->resp[0] & R1_SPI_IDLE)) {
+			*busy = false;
+			return 0;
+		}
+	} else {
+		if (cmd->resp[0] & MMC_CARD_BUSY) {
+			*busy = false;
+			return 0;
+		}
+	}
+
+	*busy = true;
+
+	/*
+	 * According to eMMC specification v5.1 section 6.4.3, we
+	 * should issue CMD1 repeatedly in the idle state until
+	 * the eMMC is ready. Otherwise some eMMC devices seem to enter
+	 * the inactive mode after mmc_init_card() issued CMD0 when
+	 * the eMMC device is busy.
+	 */
+	if (!ocr && !mmc_host_is_spi(host))
+		cmd->arg = cmd->resp[0] | BIT(30);
+
+	return 0;
+}
+
 int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 {
 	struct mmc_command cmd = {};
-	int i, err = 0;
+	int err = 0;
+	struct mmc_op_cond_busy_data cb_data = {
+		.host = host,
+		.ocr = ocr,
+		.cmd = &cmd
+	};
 
 	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--) {
-		err = mmc_wait_for_cmd(host, &cmd, 0);
-		if (err)
-			break;
-
-		/* wait until reset completes */
-		if (mmc_host_is_spi(host)) {
-			if (!(cmd.resp[0] & R1_SPI_IDLE))
-				break;
-		} else {
-			if (cmd.resp[0] & MMC_CARD_BUSY)
-				break;
-		}
-
-		err = -ETIMEDOUT;
-
-		mmc_delay(10);
-
-		/*
-		 * According to eMMC specification v5.1 section 6.4.3, we
-		 * should issue CMD1 repeatedly in the idle state until
-		 * the eMMC is ready. Otherwise some eMMC devices seem to enter
-		 * the inactive mode after mmc_init_card() issued CMD0 when
-		 * the eMMC device is busy.
-		 */
-		if (!ocr && !mmc_host_is_spi(host))
-			cmd.arg = cmd.resp[0] | BIT(30);
-	}
+	err = __mmc_poll_for_busy(host, 1000, &__mmc_send_op_cond_cb, &cb_data);
+	if (err)
+		return err;
 
 	if (rocr && !mmc_host_is_spi(host))
 		*rocr = cmd.resp[0];
-- 
2.17.1

RE: [PATCH v2 2/2] mmc: core: adjust polling interval for CMD1

[Avri Altman]

 
> 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.
> 
> Rather than fixing about the interval time in mmc_send_op_cond(), let's
> instead convert into using the common __mmc_poll_for_busy().
> 
> 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.
That's an impressive improvement.
Can you share some of the use-cases in which 10ms reduction in boot time is required?

Thanks,
Avri

Re: [PATCH v2 2/2] mmc: core: adjust polling interval for CMD1

[Huijin Park]

2021년 11월 4일 (목) 오후 4:27, Avri Altman <Avri.Altman@wdc.com>님이 작성:
>
>
> > 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.
> >
> > Rather than fixing about the interval time in mmc_send_op_cond(), let's
> > instead convert into using the common __mmc_poll_for_busy().
> >
> > 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.
> That's an impressive improvement.
> Can you share some of the use-cases in which 10ms reduction in boot time is required?

It can be used as one of the improvements and tuning items that
can make rootfs preparation faster for cold booting.
(e.g. if it is delayed, it outputs "Waiting for root device.." log.)
Above all, I think it is not desirable to delay even though mmc
initialization is done.

Thanks,

>
> Thanks,
> Avri

Re: [PATCH v2 0/2] mmc: core: adjust polling interval for CMD1

[Ulf Hansson]

On Thu, 4 Nov 2021 at 07:32, Huijin Park <huijin.park@samsung.com> wrote:
>
> This series is for adjusting polling interval for CMD1 using
> __mmc_poll_for_busy() common function which provides a stricter
> timeout checking and a throttling mechanism.
>
> v1...v2:
>  - Change __mmc_poll_for_busy() first parameter type to 'mmc_host*'.
>  - Use __mmc_poll_for_busy() common function.
>
> Huijin Park (2):
>   mmc: core: change __mmc_poll_for_busy() parameter type
>   mmc: core: adjust polling interval for CMD1
>
>  drivers/mmc/core/block.c   |  4 +-
>  drivers/mmc/core/mmc.c     |  2 +-
>  drivers/mmc/core/mmc_ops.c | 89 ++++++++++++++++++++++++--------------
>  drivers/mmc/core/mmc_ops.h |  2 +-
>  drivers/mmc/core/sd.c      |  2 +-
>  5 files changed, 62 insertions(+), 37 deletions(-)
>
> --
> 2.17.1
>

Applied for next, thanks!

Kind regards
Uffe