[U-Boot] [PATCH] mmc: retry the cmd8 to meet 74 clocks requirement in the spec

[U-Boot] [PATCH] mmc: retry the cmd8 to meet 74 clocks requirement in the spec

[Lei Wen]

For some controller it has dynamic clock gating, and only toggle out clk
when the first cmd0 send out, while some card strictly obey the 74
clocks rule, the interval may not be sufficient between the cmd0 and
this cmd8, retry to fulfil the clock requirement.

Signed-off-by: Lei Wen <leiwen@marvell.com>
---
 drivers/mmc/mmc.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 7e703c0..f9b2223 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1178,7 +1178,7 @@ block_dev_desc_t *mmc_get_dev(int dev)
 
 int mmc_init(struct mmc *mmc)
 {
-	int err;
+	int err, retry = 3;
 
 	if (mmc->has_init)
 		return 0;
@@ -1201,7 +1201,19 @@ int mmc_init(struct mmc *mmc)
 	mmc->part_num = 0;
 
 	/* Test for SD version 2 */
-	err = mmc_send_if_cond(mmc);
+	/*
+	 * retry here for 3 times, as for some controller it has dynamic
+	 * clock gating, and only toggle out clk when the first cmd0 send
+	 * out, while some card strictly obey the 74 clocks rule, the interval
+	 * may not be sufficient between the cmd0 and this cmd8, retry to
+	 * fulfil the clock requirement
+	 */
+	do {
+		err = mmc_send_if_cond(mmc);
+	} while (--retry > 0 && err);
+
+	if (err)
+		return err;
 
 	/* Now try to get the SD card's operating condition */
 	err = sd_send_op_cond(mmc);
-- 
1.7.0.4

[U-Boot] [PATCH] mmc: retry the cmd8 to meet 74 clocks requirement in the spec

[Marek Vasut]

On Saturday, September 03, 2011 05:34:17 AM Lei Wen wrote:
> For some controller it has dynamic clock gating, and only toggle out clk
> when the first cmd0 send out, while some card strictly obey the 74
> clocks rule, the interval may not be sufficient between the cmd0 and
> this cmd8, retry to fulfil the clock requirement.
> 
> Signed-off-by: Lei Wen <leiwen@marvell.com>
> ---
>  drivers/mmc/mmc.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 7e703c0..f9b2223 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -1178,7 +1178,7 @@ block_dev_desc_t *mmc_get_dev(int dev)
> 
>  int mmc_init(struct mmc *mmc)
>  {
> -	int err;
> +	int err, retry = 3;
> 
>  	if (mmc->has_init)
>  		return 0;
> @@ -1201,7 +1201,19 @@ int mmc_init(struct mmc *mmc)
>  	mmc->part_num = 0;
> 
>  	/* Test for SD version 2 */
> -	err = mmc_send_if_cond(mmc);
> +	/*
> +	 * retry here for 3 times, as for some controller it has dynamic
> +	 * clock gating, and only toggle out clk when the first cmd0 send
> +	 * out, while some card strictly obey the 74 clocks rule, the interval
> +	 * may not be sufficient between the cmd0 and this cmd8, retry to
> +	 * fulfil the clock requirement
> +	 */
> +	do {
> +		err = mmc_send_if_cond(mmc);
> +	} while (--retry > 0 && err);

Hi, maybe this condition might be a bit more readable ...

while (--retry) {
	err = mmc_send_if_conf(mmc);
	if (!err)
		break;
}

> +
> +	if (err)
> +		return err;
> 
>  	/* Now try to get the SD card's operating condition */
>  	err = sd_send_op_cond(mmc);

Otherwise, works on my hardware.

Tested-by: Marek Vasut <marek.vasut@gmail.com>

Cheers!

[U-Boot] [PATCH] mmc: retry the cmd8 to meet 74 clocks requirement in the spec

[Andy Fleming]

On Sun, Sep 4, 2011 at 5:18 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On Saturday, September 03, 2011 05:34:17 AM Lei Wen wrote:
>> For some controller it has dynamic clock gating, and only toggle out clk
>> when the first cmd0 send out, while some card strictly obey the 74
>> clocks rule, the interval may not be sufficient between the cmd0 and
>> this cmd8, retry to fulfil the clock requirement.


This seems like the wrong way to handle this. What if another
controller shows up, but it takes *4* retries. Or *5*?

Or *100*? This sort of knowledge should somehow be embedded in either
the driver for that controller, or the board
code. Not the generic MMC code.

Maybe put it in the init code for the controller?

Andy

[U-Boot] [PATCH] mmc: retry the cmd8 to meet 74 clocks requirement in the spec

[Lei Wen]

Hi Andy,

On Mon, Sep 5, 2011 at 6:45 AM, Andy Fleming <afleming@gmail.com> wrote:
> On Sun, Sep 4, 2011 at 5:18 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On Saturday, September 03, 2011 05:34:17 AM Lei Wen wrote:
>>> For some controller it has dynamic clock gating, and only toggle out clk
>>> when the first cmd0 send out, while some card strictly obey the 74
>>> clocks rule, the interval may not be sufficient between the cmd0 and
>>> this cmd8, retry to fulfil the clock requirement.
>
>
> This seems like the wrong way to handle this. What if another
> controller shows up, but it takes *4* retries. Or *5*?
>
> Or *100*? This sort of knowledge should somehow be embedded in either
> the driver for that controller, or the board
> code. Not the generic MMC code.
>
> Maybe put it in the init code for the controller?
>

The card after the cmd0 need sometimes to be stabilize. Linux also add
some kind of
mmc_delay function inside the mmc_go_idle() fucntion in the generic framework.

The retry time cannot be 100 that much. :) Since the 74 clocks is such
short time, that
even the second try would serve good. Make it a part of generic could
remove the ugly
handling in the controller driver itself, and others also could
benefit from it...

Best regards,
Lei