[PATCH v2] mmc: core: retry CMD1 in mmc_send_op_cond() even if the ocr = 0

[PATCH v2] mmc: core: retry CMD1 in mmc_send_op_cond() even if the ocr = 0

[Yoshihiro Shimoda]

According to eMMC specification v5.1 section 6.4.3, we should issue
CMD1 repeatedly in the idle state until the eMMC is ready even if
the mmc_attach_mmc() calls this function with ocr = 0. Otherwise
some eMMC devices seems to enter the inactive mode after
mmc_init_card() issued CMD0 when the eMMC device is busy.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 Changes from v1 (https://patchwork.kernel.org/patch/10874619/):
 - Revise the comment on the source code.

 We can reproduce this issue if:
  - we add no-sd and no-sdio property into sdhi2 node of salvator-common.dtsi,
  - the renesas_sdhi driver is kernel module,
  - Using a Salvator-XS board that has Samsung eMMC device,
  - enter suspend and exit it,
  - and then insmod renesas_sdhi_{core,internal_dmac}.ko.

 After that, the following error happened and any partitions are not detected.
  mmc1: error -110 whilst initialising MMC card

 I tested this patch on:
  - M3-N Salvator-XS with a Samsung eMMC device,
  - M3-W Salvator-X with SiliconMotion eMMC device,
  - and M3-W Starter Kit with Micron eMMC device.

 drivers/mmc/core/mmc_ops.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index c5208fb..a533cab 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -184,11 +184,7 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 		if (err)
 			break;
 
-		/* if we're just probing, do a single pass */
-		if (ocr == 0)
-			break;
-
-		/* otherwise wait until reset completes */
+		/* wait until reset completes */
 		if (mmc_host_is_spi(host)) {
 			if (!(cmd.resp[0] & R1_SPI_IDLE))
 				break;
@@ -200,6 +196,16 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 		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);
 	}
 
 	if (rocr && !mmc_host_is_spi(host))
-- 
2.7.4

Re: [PATCH v2] mmc: core: retry CMD1 in mmc_send_op_cond() even if the ocr = 0

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

On Tue, Apr 09, 2019 at 08:05:06PM +0900, Yoshihiro Shimoda wrote:
> According to eMMC specification v5.1 section 6.4.3, we should issue
> CMD1 repeatedly in the idle state until the eMMC is ready even if
> the mmc_attach_mmc() calls this function with ocr = 0. Otherwise
> some eMMC devices seems to enter the inactive mode after
> mmc_init_card() issued CMD0 when the eMMC device is busy.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  Changes from v1 (https://patchwork.kernel.org/patch/10874619/):
>  - Revise the comment on the source code.

I like the new comment, very clear, thank you!

I didn't test this change but I double checked the specs and the code
matches them to the best of my understanding:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] mmc: core: retry CMD1 in mmc_send_op_cond() even if the ocr = 0

[Ulf Hansson]

On Tue, 9 Apr 2019 at 13:09, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>
> According to eMMC specification v5.1 section 6.4.3, we should issue
> CMD1 repeatedly in the idle state until the eMMC is ready even if
> the mmc_attach_mmc() calls this function with ocr = 0. Otherwise
> some eMMC devices seems to enter the inactive mode after
> mmc_init_card() issued CMD0 when the eMMC device is busy.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Applied for next, thanks!

Let's get some test coverage of this before we decide if this a
material for stable.

Kind regards
Uffe


> ---
>  Changes from v1 (https://patchwork.kernel.org/patch/10874619/):
>  - Revise the comment on the source code.
>
>  We can reproduce this issue if:
>   - we add no-sd and no-sdio property into sdhi2 node of salvator-common.dtsi,
>   - the renesas_sdhi driver is kernel module,
>   - Using a Salvator-XS board that has Samsung eMMC device,
>   - enter suspend and exit it,
>   - and then insmod renesas_sdhi_{core,internal_dmac}.ko.
>
>  After that, the following error happened and any partitions are not detected.
>   mmc1: error -110 whilst initialising MMC card
>
>  I tested this patch on:
>   - M3-N Salvator-XS with a Samsung eMMC device,
>   - M3-W Salvator-X with SiliconMotion eMMC device,
>   - and M3-W Starter Kit with Micron eMMC device.
>
>  drivers/mmc/core/mmc_ops.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index c5208fb..a533cab 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -184,11 +184,7 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
>                 if (err)
>                         break;
>
> -               /* if we're just probing, do a single pass */
> -               if (ocr == 0)
> -                       break;
> -
> -               /* otherwise wait until reset completes */
> +               /* wait until reset completes */
>                 if (mmc_host_is_spi(host)) {
>                         if (!(cmd.resp[0] & R1_SPI_IDLE))
>                                 break;
> @@ -200,6 +196,16 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
>                 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);
>         }
>
>         if (rocr && !mmc_host_is_spi(host))
> --
> 2.7.4
>