[PATCH] mmc: mmc: Fix HS switch failure in mmc_select_hs400()

[PATCH] mmc: mmc: Fix HS switch failure in mmc_select_hs400()

[Ziyuan Xu]

To slove the issue which was found on gru board for hs400.

[    4.616946] sdhci: Secure Digital Host Controller Interface driver
[    4.623135] sdhci: Copyright(c) Pierre Ossman
[    4.722575] sdhci-pltfm: SDHCI platform and OF driver helper
[    4.730962] sdhci-arasan fe330000.sdhci: No vmmc regulator found
[    4.737444] sdhci-arasan fe330000.sdhci: No vqmmc regulator found
[    4.774930] mmc0: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
[    4.980295] mmc0: switch to high-speed from hs200 failed, err:-84
[    4.986487] mmc0: error -84 whilst initialising MMC card

We should change HS400 mode selection timing to meet JEDEC
specification. The JEDEC 5.1 said that change the frequency to <= 52MHZ
after HS_TIMING switch. Refer to section 6.6.2.3 "HS400" timing mode
selection:
Set the "Timing Interface" parameter in the HS_TIMING[185] field of the
Extended CSD register to 0x1 to switch to High Speed mode and then set
the clock frequency to a value not greater than 52MHZ.

Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
---
 drivers/mmc/core/mmc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index b81b08f..8e4d059 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1078,10 +1078,6 @@ static int mmc_select_hs400(struct mmc_card *card)
 	if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
 		send_status = false;
 
-	/* Reduce frequency to HS frequency */
-	max_dtr = card->ext_csd.hs_max_dtr;
-	mmc_set_clock(host, max_dtr);
-
 	/* Switch card to HS mode */
 	val = EXT_CSD_TIMING_HS;
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
@@ -1097,6 +1093,10 @@ static int mmc_select_hs400(struct mmc_card *card)
 	/* Set host controller to HS timing */
 	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
 
+	/* Reduce frequency to HS frequency */
+	max_dtr = card->ext_csd.hs_max_dtr;
+	mmc_set_clock(host, max_dtr);
+
 	if (!send_status) {
 		err = mmc_switch_status(card);
 		if (err)
-- 
1.9.1

Re: [PATCH] mmc: mmc: Fix HS switch failure in mmc_select_hs400()

[Adrian Hunter]

On 26/05/16 08:50, Ziyuan Xu wrote:
> To slove the issue which was found on gru board for hs400.
> 
> [    4.616946] sdhci: Secure Digital Host Controller Interface driver
> [    4.623135] sdhci: Copyright(c) Pierre Ossman
> [    4.722575] sdhci-pltfm: SDHCI platform and OF driver helper
> [    4.730962] sdhci-arasan fe330000.sdhci: No vmmc regulator found
> [    4.737444] sdhci-arasan fe330000.sdhci: No vqmmc regulator found
> [    4.774930] mmc0: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
> [    4.980295] mmc0: switch to high-speed from hs200 failed, err:-84
> [    4.986487] mmc0: error -84 whilst initialising MMC card
> 
> We should change HS400 mode selection timing to meet JEDEC
> specification. The JEDEC 5.1 said that change the frequency to <= 52MHZ
> after HS_TIMING switch. Refer to section 6.6.2.3 "HS400" timing mode
> selection:
> Set the "Timing Interface" parameter in the HS_TIMING[185] field of the
> Extended CSD register to 0x1 to switch to High Speed mode and then set
> the clock frequency to a value not greater than 52MHZ.

Do you need a fix also for re-tuning? i.e. does re-tuning work with this
patch? Or do you not need re-tuning?

> 
> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> ---
>  drivers/mmc/core/mmc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index b81b08f..8e4d059 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1078,10 +1078,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>  	if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
>  		send_status = false;
>  
> -	/* Reduce frequency to HS frequency */
> -	max_dtr = card->ext_csd.hs_max_dtr;
> -	mmc_set_clock(host, max_dtr);
> -
>  	/* Switch card to HS mode */
>  	val = EXT_CSD_TIMING_HS;
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> @@ -1097,6 +1093,10 @@ static int mmc_select_hs400(struct mmc_card *card)
>  	/* Set host controller to HS timing */
>  	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>  
> +	/* Reduce frequency to HS frequency */
> +	max_dtr = card->ext_csd.hs_max_dtr;
> +	mmc_set_clock(host, max_dtr);
> +
>  	if (!send_status) {
>  		err = mmc_switch_status(card);
>  		if (err)
> 

Re: [PATCH] mmc: mmc: Fix HS switch failure in mmc_select_hs400()

[Ziyuan Xu]

On 2016年05月26日 17:06, Adrian Hunter wrote:
> On 26/05/16 08:50, Ziyuan Xu wrote:
>> To slove the issue which was found on gru board for hs400.
>>
>> [    4.616946] sdhci: Secure Digital Host Controller Interface driver
>> [    4.623135] sdhci: Copyright(c) Pierre Ossman
>> [    4.722575] sdhci-pltfm: SDHCI platform and OF driver helper
>> [    4.730962] sdhci-arasan fe330000.sdhci: No vmmc regulator found
>> [    4.737444] sdhci-arasan fe330000.sdhci: No vqmmc regulator found
>> [    4.774930] mmc0: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
>> [    4.980295] mmc0: switch to high-speed from hs200 failed, err:-84
>> [    4.986487] mmc0: error -84 whilst initialising MMC card
>>
>> We should change HS400 mode selection timing to meet JEDEC
>> specification. The JEDEC 5.1 said that change the frequency to <= 52MHZ
>> after HS_TIMING switch. Refer to section 6.6.2.3 "HS400" timing mode
>> selection:
>> Set the "Timing Interface" parameter in the HS_TIMING[185] field of the
>> Extended CSD register to 0x1 to switch to High Speed mode and then set
>> the clock frequency to a value not greater than 52MHZ.
> Do you need a fix also for re-tuning? i.e. does re-tuning work with this
> patch? Or do you not need re-tuning?

Do you mean that revise the similar timing in mmc_hs400_to_hs200()as 
shown below?

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 8e4d059..4232a42 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1161,10 +1161,6 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
         if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
                 send_status = false;

-       /* Reduce frequency to HS */
-       max_dtr = card->ext_csd.hs_max_dtr;
-       mmc_set_clock(host, max_dtr);
-
         /* Switch HS400 to HS DDR */
         val = EXT_CSD_TIMING_HS;
         err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
@@ -1175,6 +1171,10 @@ int mmc_hs400_to_hs200(struct mmc_card *card)

         mmc_set_timing(host, MMC_TIMING_MMC_DDR52);

+       /* Reduce frequency to HS */
+       max_dtr = card->ext_csd.hs_max_dtr;
+       mmc_set_clock(host, max_dtr);
+
         if (!send_status) {
                 err = mmc_switch_status(card);
                 if (err)

The spec doesn't require mode selection from hs400 to hs200, moreover I 
have not yet hit any failure in mmc_hs400_to_hs200.
Thus I don't know whether a fix is needed for re-tuning.
By the way, what's your opinion for this patch?   I know, you did it on 
purpose that I found some message from git log.

>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> ---
>>   drivers/mmc/core/mmc.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index b81b08f..8e4d059 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1078,10 +1078,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>>   	if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
>>   		send_status = false;
>>   
>> -	/* Reduce frequency to HS frequency */
>> -	max_dtr = card->ext_csd.hs_max_dtr;
>> -	mmc_set_clock(host, max_dtr);
>> -
>>   	/* Switch card to HS mode */
>>   	val = EXT_CSD_TIMING_HS;
>>   	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> @@ -1097,6 +1093,10 @@ static int mmc_select_hs400(struct mmc_card *card)
>>   	/* Set host controller to HS timing */
>>   	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>>   
>> +	/* Reduce frequency to HS frequency */
>> +	max_dtr = card->ext_csd.hs_max_dtr;
>> +	mmc_set_clock(host, max_dtr);
>> +
>>   	if (!send_status) {
>>   		err = mmc_switch_status(card);
>>   		if (err)
>>
>
>
>

Re: [PATCH] mmc: mmc: Fix HS switch failure in mmc_select_hs400()

[Ulf Hansson]

On 26 May 2016 at 07:50, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> To slove the issue which was found on gru board for hs400.
>
> [    4.616946] sdhci: Secure Digital Host Controller Interface driver
> [    4.623135] sdhci: Copyright(c) Pierre Ossman
> [    4.722575] sdhci-pltfm: SDHCI platform and OF driver helper
> [    4.730962] sdhci-arasan fe330000.sdhci: No vmmc regulator found
> [    4.737444] sdhci-arasan fe330000.sdhci: No vqmmc regulator found
> [    4.774930] mmc0: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
> [    4.980295] mmc0: switch to high-speed from hs200 failed, err:-84
> [    4.986487] mmc0: error -84 whilst initialising MMC card
>
> We should change HS400 mode selection timing to meet JEDEC
> specification. The JEDEC 5.1 said that change the frequency to <= 52MHZ
> after HS_TIMING switch. Refer to section 6.6.2.3 "HS400" timing mode
> selection:
> Set the "Timing Interface" parameter in the HS_TIMING[185] field of the
> Extended CSD register to 0x1 to switch to High Speed mode and then set
> the clock frequency to a value not greater than 52MHZ.
>
> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>

Thanks, applied for next!

Future wise, please make sure to send your responses to peoples
comments in plain text!

Kind regards
Uffe

> ---
>  drivers/mmc/core/mmc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index b81b08f..8e4d059 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1078,10 +1078,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>         if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
>                 send_status = false;
>
> -       /* Reduce frequency to HS frequency */
> -       max_dtr = card->ext_csd.hs_max_dtr;
> -       mmc_set_clock(host, max_dtr);
> -
>         /* Switch card to HS mode */
>         val = EXT_CSD_TIMING_HS;
>         err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> @@ -1097,6 +1093,10 @@ static int mmc_select_hs400(struct mmc_card *card)
>         /* Set host controller to HS timing */
>         mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>
> +       /* Reduce frequency to HS frequency */
> +       max_dtr = card->ext_csd.hs_max_dtr;
> +       mmc_set_clock(host, max_dtr);
> +
>         if (!send_status) {
>                 err = mmc_switch_status(card);
>                 if (err)
> --
> 1.9.1
>
>