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

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

[Ulf Hansson]

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@math.uni-bielefeld.de>
Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
Cc: Huijin Park <huijin.park@samsung.com>
Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

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

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

[H. Nikolaus Schaller]

Hi Ulf,

> Am 04.03.2022 um 11:56 schrieb Ulf Hansson <ulf.hansson@linaro.org>:
> 
> 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@math.uni-bielefeld.de>
> Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Huijin Park <huijin.park@samsung.com>
> Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> 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.

Works for me.

BR and thanks,
Nikolaus

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

[Jean Rene Dawin]

Ulf Hansson wrote on Fri  4/03/22 11:56:
> 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@math.uni-bielefeld.de>
> Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Huijin Park <huijin.park@samsung.com>
> Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> 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(-)

Hi,

thanks, this works fine.

Regads,
Jean Rene Dawin

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

[Ulf Hansson]

On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@linaro.org> 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@math.uni-bielefeld.de>
> Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Huijin Park <huijin.park@samsung.com>
> Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

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
>

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

[Jean Rene Dawin]

Ulf Hansson wrote on Mon  7/03/22 13:17:
> On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@linaro.org> 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@math.uni-bielefeld.de>
> > Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> > Cc: Huijin Park <huijin.park@samsung.com>
> > Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> > 
> > 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

> 
> Applied for fixes and by adding two tested-by tags from you, thanks!
> 
> Kind regards
> Uffe

Hi,

with the current value of MMC_OP_COND_PERIOD_US = 1ms I still see

mmc1: Card stuck being busy! __mmc_poll_for_busy
mmc1: error -110 doing runtime resume

regularly. The same with 2ms. Setting it to 4ms makes the messages go
away. Would it be ok to increase MMC_OP_COND_PERIOD_US to 4ms?


---
 drivers/mmc/core/mmc_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 180d7e9d3400..1fd57f342842 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -21,7 +21,7 @@

 #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_PERIOD_US          (4 * 1000) /* 1ms */
 #define MMC_OP_COND_TIMEOUT_MS         1000 /* 1s */

 static const u8 tuning_blk_pattern_4bit[] = {
--
2.35.1


Regards,
Jean Rene

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

[Ulf Hansson]

On Wed, 4 May 2022 at 07:46, Jean Rene Dawin
<jdawin@math.uni-bielefeld.de> wrote:
>
> Ulf Hansson wrote on Mon  7/03/22 13:17:
> > On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@linaro.org> 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@math.uni-bielefeld.de>
> > > Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> > > Cc: Huijin Park <huijin.park@samsung.com>
> > > Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >
> > > 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
>
> >
> > Applied for fixes and by adding two tested-by tags from you, thanks!
> >
> > Kind regards
> > Uffe
>
> Hi,
>
> with the current value of MMC_OP_COND_PERIOD_US = 1ms I still see
>
> mmc1: Card stuck being busy! __mmc_poll_for_busy
> mmc1: error -110 doing runtime resume
>
> regularly. The same with 2ms. Setting it to 4ms makes the messages go
> away. Would it be ok to increase MMC_OP_COND_PERIOD_US to 4ms?

It doesn't look like we have a very good alternative - unless the
problem is tied to a particular type of eMMC card, is it? (If so, we
can add a card-quirk).

The only other option I see, would then be to add a generic DT
property for eMMCs, that allows us to specify the OP_COND polling
period for it. See
Documentation/devicetree/bindings/mmc/mmc-card.yaml.

Kind regards
Uffe


>
>
> ---
>  drivers/mmc/core/mmc_ops.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 180d7e9d3400..1fd57f342842 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -21,7 +21,7 @@
>
>  #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_PERIOD_US          (4 * 1000) /* 1ms */
>  #define MMC_OP_COND_TIMEOUT_MS         1000 /* 1s */
>
>  static const u8 tuning_blk_pattern_4bit[] = {
> --
> 2.35.1
>
>
> Regards,
> Jean Rene

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

[Jean Rene Dawin]

Ulf Hansson wrote on Wed  4/05/22 11:08:
> On Wed, 4 May 2022 at 07:46, Jean Rene Dawin
> <jdawin@math.uni-bielefeld.de> wrote:
> >
> > Ulf Hansson wrote on Mon  7/03/22 13:17:
> > > On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@linaro.org> 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@math.uni-bielefeld.de>
> > > > Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> > > > Cc: Huijin Park <huijin.park@samsung.com>
> > > > Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > ---
> > > >
> > > > 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
> >
> > >
> > > Applied for fixes and by adding two tested-by tags from you, thanks!
> > >
> > > Kind regards
> > > Uffe
> >
> > Hi,
> >
> > with the current value of MMC_OP_COND_PERIOD_US = 1ms I still see
> >
> > mmc1: Card stuck being busy! __mmc_poll_for_busy
> > mmc1: error -110 doing runtime resume
> >
> > regularly. The same with 2ms. Setting it to 4ms makes the messages go
> > away. Would it be ok to increase MMC_OP_COND_PERIOD_US to 4ms?
> 
> It doesn't look like we have a very good alternative - unless the
> problem is tied to a particular type of eMMC card, is it? (If so, we
> can add a card-quirk).
> 
> The only other option I see, would then be to add a generic DT
> property for eMMCs, that allows us to specify the OP_COND polling
> period for it. See
> Documentation/devicetree/bindings/mmc/mmc-card.yaml.

Hi,

ok, I will test with another beaglebone which has a different emmc chip.

Regards,
Jean Rene

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

[Jean Rene Dawin]

Ulf Hansson wrote on Wed  4/05/22 11:08:
> On Wed, 4 May 2022 at 07:46, Jean Rene Dawin
> <jdawin@math.uni-bielefeld.de> wrote:
> >
> > Ulf Hansson wrote on Mon  7/03/22 13:17:
> > > On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@linaro.org> 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@math.uni-bielefeld.de>
> > > > Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> > > > Cc: Huijin Park <huijin.park@samsung.com>
> > > > Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > ---
> > > >
> > > > 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
> >
> > >
> > > Applied for fixes and by adding two tested-by tags from you, thanks!
> > >
> > > Kind regards
> > > Uffe
> >
> > Hi,
> >
> > with the current value of MMC_OP_COND_PERIOD_US = 1ms I still see
> >
> > mmc1: Card stuck being busy! __mmc_poll_for_busy
> > mmc1: error -110 doing runtime resume
> >
> > regularly. The same with 2ms. Setting it to 4ms makes the messages go
> > away. Would it be ok to increase MMC_OP_COND_PERIOD_US to 4ms?
> 
> It doesn't look like we have a very good alternative - unless the
> problem is tied to a particular type of eMMC card, is it? (If so, we
> can add a card-quirk).
> 
> The only other option I see, would then be to add a generic DT
> property for eMMCs, that allows us to specify the OP_COND polling
> period for it. See
> Documentation/devicetree/bindings/mmc/mmc-card.yaml.
> 
> Kind regards
> Uffe

Hi,

I tested 2 beaglebones now - one with Micron eMMC and the other with Kingston.
With the Kingston chip I don't get the errors. So it seems to be card specific.

Grepping for mmc in dmesg gives the following.

Beaglebone with Micron eMMC:

  sdhci-omap 481d8000.mmc: supply pbias not found, using dummy regulator
  sdhci-omap 481d8000.mmc: supply vqmmc not found, using dummy regulator
  mmc1: SDHCI controller on 481d8000.mmc [481d8000.mmc] using External DMA
  mmc1: new high speed MMC card at address 0001
  mmcblk1: mmc1:0001 MMC04G 3.66 GiB
   mmcblk1: p1
  mmcblk1boot0: mmc1:0001 MMC04G 1.00 MiB
  mmcblk1boot1: mmc1:0001 MMC04G 1.00 MiB
  mmcblk1rpmb: mmc1:0001 MMC04G 128 KiB, chardev (247:0)
  sdhci-omap 48060000.mmc: Got CD GPIO
  sdhci-omap 48060000.mmc: supply pbias not found, using dummy regulator
  sdhci-omap 48060000.mmc: supply vqmmc not found, using dummy regulator
  mmc0: SDHCI controller on 48060000.mmc [48060000.mmc] using External DMA


Beaglebone with Kingston eMMC:

  sdhci-omap 481d8000.mmc: supply pbias not found, using dummy regulator
  sdhci-omap 481d8000.mmc: supply vqmmc not found, using dummy regulator
  mmc1: SDHCI controller on 481d8000.mmc [481d8000.mmc] using External DMA
  mmc1: new high speed MMC card at address 0001
  mmcblk1: mmc1:0001 M62704 3.56 GiB
   mmcblk1: p1
  mmcblk1boot0: mmc1:0001 M62704 2.00 MiB
  mmcblk1boot1: mmc1:0001 M62704 2.00 MiB
  mmcblk1rpmb: mmc1:0001 M62704 512 KiB, chardev (247:0)
  sdhci-omap 48060000.mmc: Got CD GPIO
  sdhci-omap 48060000.mmc: supply pbias not found, using dummy regulator
  sdhci-omap 48060000.mmc: supply vqmmc not found, using dummy regulator
  mmc0: SDHCI controller on 48060000.mmc [48060000.mmc] using External DMA

Is this enough information to identify the mmc card?

Regards,
Jean Rene

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

[Ulf Hansson]

On Wed, 11 May 2022 at 08:46, Jean Rene Dawin
<jdawin@math.uni-bielefeld.de> wrote:
>
> Ulf Hansson wrote on Wed  4/05/22 11:08:
> > On Wed, 4 May 2022 at 07:46, Jean Rene Dawin
> > <jdawin@math.uni-bielefeld.de> wrote:
> > >
> > > Ulf Hansson wrote on Mon  7/03/22 13:17:
> > > > On Fri, 4 Mar 2022 at 11:57, Ulf Hansson <ulf.hansson@linaro.org> 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@math.uni-bielefeld.de>
> > > > > Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> > > > > Cc: Huijin Park <huijin.park@samsung.com>
> > > > > Fixes: 76bfc7ccc2fa ("mmc: core: adjust polling interval for CMD1")
> > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > > ---
> > > > >
> > > > > 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
> > >
> > > >
> > > > Applied for fixes and by adding two tested-by tags from you, thanks!
> > > >
> > > > Kind regards
> > > > Uffe
> > >
> > > Hi,
> > >
> > > with the current value of MMC_OP_COND_PERIOD_US = 1ms I still see
> > >
> > > mmc1: Card stuck being busy! __mmc_poll_for_busy
> > > mmc1: error -110 doing runtime resume
> > >
> > > regularly. The same with 2ms. Setting it to 4ms makes the messages go
> > > away. Would it be ok to increase MMC_OP_COND_PERIOD_US to 4ms?
> >
> > It doesn't look like we have a very good alternative - unless the
> > problem is tied to a particular type of eMMC card, is it? (If so, we
> > can add a card-quirk).
> >
> > The only other option I see, would then be to add a generic DT
> > property for eMMCs, that allows us to specify the OP_COND polling
> > period for it. See
> > Documentation/devicetree/bindings/mmc/mmc-card.yaml.
> >
> > Kind regards
> > Uffe
>
> Hi,
>
> I tested 2 beaglebones now - one with Micron eMMC and the other with Kingston.
> With the Kingston chip I don't get the errors. So it seems to be card specific.
>
> Grepping for mmc in dmesg gives the following.
>
> Beaglebone with Micron eMMC:
>
>   sdhci-omap 481d8000.mmc: supply pbias not found, using dummy regulator
>   sdhci-omap 481d8000.mmc: supply vqmmc not found, using dummy regulator
>   mmc1: SDHCI controller on 481d8000.mmc [481d8000.mmc] using External DMA
>   mmc1: new high speed MMC card at address 0001
>   mmcblk1: mmc1:0001 MMC04G 3.66 GiB
>    mmcblk1: p1
>   mmcblk1boot0: mmc1:0001 MMC04G 1.00 MiB
>   mmcblk1boot1: mmc1:0001 MMC04G 1.00 MiB
>   mmcblk1rpmb: mmc1:0001 MMC04G 128 KiB, chardev (247:0)
>   sdhci-omap 48060000.mmc: Got CD GPIO
>   sdhci-omap 48060000.mmc: supply pbias not found, using dummy regulator
>   sdhci-omap 48060000.mmc: supply vqmmc not found, using dummy regulator
>   mmc0: SDHCI controller on 48060000.mmc [48060000.mmc] using External DMA
>
>
> Beaglebone with Kingston eMMC:
>
>   sdhci-omap 481d8000.mmc: supply pbias not found, using dummy regulator
>   sdhci-omap 481d8000.mmc: supply vqmmc not found, using dummy regulator
>   mmc1: SDHCI controller on 481d8000.mmc [481d8000.mmc] using External DMA
>   mmc1: new high speed MMC card at address 0001
>   mmcblk1: mmc1:0001 M62704 3.56 GiB
>    mmcblk1: p1
>   mmcblk1boot0: mmc1:0001 M62704 2.00 MiB
>   mmcblk1boot1: mmc1:0001 M62704 2.00 MiB
>   mmcblk1rpmb: mmc1:0001 M62704 512 KiB, chardev (247:0)
>   sdhci-omap 48060000.mmc: Got CD GPIO
>   sdhci-omap 48060000.mmc: supply pbias not found, using dummy regulator
>   sdhci-omap 48060000.mmc: supply vqmmc not found, using dummy regulator
>   mmc0: SDHCI controller on 48060000.mmc [48060000.mmc] using External DMA
>
> Is this enough information to identify the mmc card?

Thanks for running the tests and sharing this information.

One thing that I realized, again, is that you seem to be encountering
the problem only during re-initialization (at runtime resume of the
eMMC card), but I guess, at least theoretically, you could meet the
same problem during the first initialization (boot). At this point we
can't use a card quirk, simply because we don't know anything about
the card yet.

That said, I think we should change the MMC_OP_COND_PERIOD_US to 4ms,
to make sure it works for all cases. Do you want to send a patch for
this - or just tell me if you prefer me to do it!?

Additionally, for those eMMC cards that can cope with a smaller
polling period, we could add a new generic DT property for the eMMC
card (Documentation/devicetree/bindings/mmc/mmc-card.yaml). In this
way we can avoid regressing eMMC initializations in general, but at
the same time allow those cards/platforms that can cope with a smaller
timeout, to specify and use this to decrease the initialization time.

Kind regards
Uffe