Re: [PATCH] mmc: core: Restore (almost) the busy polling for MMC_SEND_OP_COND

From: Ulf Hansson
Date: Mon Mar 07 2022 - 07:18:00 EST

On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
> Commit 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1"),
> significantly decreased the polling period from ~10-12ms into just a couple
> of us. The purpose was to decrease the total time spent in the busy polling
> loop, but unfortunate it has lead to problems, that causes eMMC cards to
> never gets out busy and thus fails to be initialized.
>
> To fix the problem, but also to try to keep some of the new improved
> behaviour, let's start by using a polling period of 1-2ms, which then
> increases for each loop, according to common polling loop in
> __mmc_poll_for_busy().
>
> Reported-by: Jean Rene Dawin <jdawin@xxxxxxxxxxxxxxxxxxxxx>
> Reported-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> Cc: Huijin Park <huijin.park@xxxxxxxxxxx>
> Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>

Applied for fixes and by adding two tested-by tags from you, thanks!

Kind regards
Uffe


> ---
>
> Jean Rene and H. Nikolaus, if this doesn't work, please try extending the
> the MMC_OP_COND_PERIOD_US a bit, to so see if we can find a value that always
> works.
>
> Kind regards
> Uffe
>
> ---
> drivers/mmc/core/block.c | 2 +-
> drivers/mmc/core/mmc.c | 2 +-
> drivers/mmc/core/mmc_ops.c | 13 +++++++++----
> drivers/mmc/core/mmc_ops.h | 3 ++-
> drivers/mmc/core/sd.c | 2 +-
> 5 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 8d718aa56d33..4e67c1403cc9 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1908,7 +1908,7 @@ 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->host, MMC_BLK_TIMEOUT_MS,
> + err = __mmc_poll_for_busy(card->host, 0, MMC_BLK_TIMEOUT_MS,
> &mmc_blk_busy_cb, &cb_data);
>
> /*
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 13abfcd130a5..43d1b9b2fa49 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(host, timeout_ms, &mmc_sleep_busy_cb, host);
> + err = __mmc_poll_for_busy(host, 0, 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 d63d1c735335..180d7e9d3400 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -21,6 +21,8 @@
>
> #define MMC_BKOPS_TIMEOUT_MS (120 * 1000) /* 120s */
> #define MMC_SANITIZE_TIMEOUT_MS (240 * 1000) /* 240s */
> +#define MMC_OP_COND_PERIOD_US (1 * 1000) /* 1ms */
> +#define MMC_OP_COND_TIMEOUT_MS 1000 /* 1s */
>
> static const u8 tuning_blk_pattern_4bit[] = {
> 0xff, 0x0f, 0xff, 0x00, 0xff, 0xcc, 0xc3, 0xcc,
> @@ -232,7 +234,9 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
> cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
> cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR;
>
> - err = __mmc_poll_for_busy(host, 1000, &__mmc_send_op_cond_cb, &cb_data);
> + err = __mmc_poll_for_busy(host, MMC_OP_COND_PERIOD_US,
> + MMC_OP_COND_TIMEOUT_MS,
> + &__mmc_send_op_cond_cb, &cb_data);
> if (err)
> return err;
>
> @@ -495,13 +499,14 @@ static int mmc_busy_cb(void *cb_data, bool *busy)
> return 0;
> }
>
> -int __mmc_poll_for_busy(struct mmc_host *host, unsigned int timeout_ms,
> +int __mmc_poll_for_busy(struct mmc_host *host, unsigned int period_us,
> + unsigned int timeout_ms,
> int (*busy_cb)(void *cb_data, bool *busy),
> void *cb_data)
> {
> int err;
> unsigned long timeout;
> - unsigned int udelay = 32, udelay_max = 32768;
> + unsigned int udelay = period_us ? period_us : 32, udelay_max = 32768;
> bool expired = false;
> bool busy = false;
>
> @@ -546,7 +551,7 @@ int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
> cb_data.retry_crc_err = retry_crc_err;
> cb_data.busy_cmd = busy_cmd;
>
> - return __mmc_poll_for_busy(host, timeout_ms, &mmc_busy_cb, &cb_data);
> + return __mmc_poll_for_busy(host, 0, 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 9c813b851d0b..09ffbc00908b 100644
> --- a/drivers/mmc/core/mmc_ops.h
> +++ b/drivers/mmc/core/mmc_ops.h
> @@ -41,7 +41,8 @@ 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_host *host, unsigned int timeout_ms,
> +int __mmc_poll_for_busy(struct mmc_host *host, unsigned int period_us,
> + 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 24b0418a24bb..68df6b2f49cc 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1671,7 +1671,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->host, SD_POWEROFF_NOTIFY_TIMEOUT_MS,
> + err = __mmc_poll_for_busy(card->host, 0, SD_POWEROFF_NOTIFY_TIMEOUT_MS,
> &sd_busy_poweroff_notify_cb, &cb_data);
>
> out:
> --
> 2.25.1
>