[PATCH] mmc:core: fix hs400 timing selection

[PATCH] mmc:core: fix hs400 timing selection

[Chanho Min]

According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 mode,
host should perform the following steps.

 1. HS200 mode selection completed
 2. Set HS_TIMING to 0x01(High Speed)
 3. Host changes frequency to =< 52MHz
 4. Set the bus width to DDR 8bit (CMD6)
 5. Host may read Driver Strength (CMD8)
 6. Set HS_TIMING to 0x03 (HS400)

In current implementation, the order of 2 and 3 is reversed.
The HS_TIMING field should be set to 0x1 before the clock frequency
is set to a value not greater than 52 MHz. Otherwise, Initialization of
timing can be failed. Also, the host contoller's UHS timing mode should
be set to DDR50 after the bus width is set to DDR 8bit.

Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
Signed-off-by: Chanho Min <chanho.min@lge.com>
---
 drivers/mmc/core/mmc.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index a301a78..52f78e0 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
 	 * Before switching to dual data rate operation for HS400,
 	 * it is required to convert from HS200 mode to HS mode.
 	 */
-	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
-	mmc_set_bus_speed(card);
-
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
 			   card->ext_csd.generic_cmd6_time,
@@ -1074,6 +1071,14 @@ static int mmc_select_hs400(struct mmc_card *card)
 		return err;
 	}
 
+	/*
+	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
+	 * be set to a value not greater than 52MHz after the HS_TIMING
+	 * field is set to 0x1.
+	 */
+	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
+	mmc_set_bus_speed(card);
+
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			 EXT_CSD_BUS_WIDTH,
 			 EXT_CSD_DDR_BUS_WIDTH_8,
@@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
 		return err;
 	}
 
+	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
+
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
 			   card->ext_csd.generic_cmd6_time,
-- 
1.7.9.5

Re: [PATCH] mmc:core: fix hs400 timing selection

[Jaehoon Chung]

Hi, Chanho.

On 10/22/2014 11:55 AM, Chanho Min wrote:
> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 mode,
> host should perform the following steps.
> 
>  1. HS200 mode selection completed
>  2. Set HS_TIMING to 0x01(High Speed)
>  3. Host changes frequency to =< 52MHz
>  4. Set the bus width to DDR 8bit (CMD6)
>  5. Host may read Driver Strength (CMD8)
>  6. Set HS_TIMING to 0x03 (HS400)
> 
> In current implementation, the order of 2 and 3 is reversed.
> The HS_TIMING field should be set to 0x1 before the clock frequency
> is set to a value not greater than 52 MHz. Otherwise, Initialization of
> timing can be failed. Also, the host contoller's UHS timing mode should
> be set to DDR50 after the bus width is set to DDR 8bit.
> 
> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
> Signed-off-by: Chanho Min <chanho.min@lge.com>
> ---
>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index a301a78..52f78e0 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>  	 * Before switching to dual data rate operation for HS400,
>  	 * it is required to convert from HS200 mode to HS mode.
>  	 */
> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> -	mmc_set_bus_speed(card);
> -
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>  			   card->ext_csd.generic_cmd6_time,
> @@ -1074,6 +1071,14 @@ static int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	/*
> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
> +	 * be set to a value not greater than 52MHz after the HS_TIMING
> +	 * field is set to 0x1.
> +	 */
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> +	mmc_set_bus_speed(card);
> +
>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			 EXT_CSD_BUS_WIDTH,
>  			 EXT_CSD_DDR_BUS_WIDTH_8,
> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
> +

I didn't know why timing is set to ddr50.

Best Regards,
Jaehoon Chung

>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>  			   card->ext_csd.generic_cmd6_time,
> 

RE: [PATCH] mmc:core: fix hs400 timing selection

[유한경]

Hi I'm Hankyung Yu

I will answer instead Chanho Min

After mmc_set_timing(card->host, MMC_TIMING_MMC_HS); 

Host controller set to SDR transfer

so is to change to a DDR transfer mode.


-----Original Message-----
From: Jaehoon Chung [mailto:jh80.chung@samsung.com] 
Sent: Tuesday, October 28, 2014 1:38 PM
To: Chanho Min; Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung
Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; HyoJun Im;
gunho.lee@lge.com; Hankyung Yu; CPGS
Subject: Re: [PATCH] mmc:core: fix hs400 timing selection

Hi, Chanho.

On 10/22/2014 11:55 AM, Chanho Min wrote:
> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
> mode, host should perform the following steps.
> 
>  1. HS200 mode selection completed
>  2. Set HS_TIMING to 0x01(High Speed)
>  3. Host changes frequency to =< 52MHz  4. Set the bus width to DDR 
> 8bit (CMD6)  5. Host may read Driver Strength (CMD8)  6. Set HS_TIMING 
> to 0x03 (HS400)
> 
> In current implementation, the order of 2 and 3 is reversed.
> The HS_TIMING field should be set to 0x1 before the clock frequency is 
> set to a value not greater than 52 MHz. Otherwise, Initialization of 
> timing can be failed. Also, the host contoller's UHS timing mode 
> should be set to DDR50 after the bus width is set to DDR 8bit.
> 
> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
> Signed-off-by: Chanho Min <chanho.min@lge.com>
> ---
>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
> a301a78..52f78e0 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>  	 * Before switching to dual data rate operation for HS400,
>  	 * it is required to convert from HS200 mode to HS mode.
>  	 */
> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> -	mmc_set_bus_speed(card);
> -
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
+1071,14 @@ static 
> int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	/*
> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
> +	 * be set to a value not greater than 52MHz after the HS_TIMING
> +	 * field is set to 0x1.
> +	 */
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> +	mmc_set_bus_speed(card);
> +
>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			 EXT_CSD_BUS_WIDTH,
>  			 EXT_CSD_DDR_BUS_WIDTH_8,
> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
> +

I didn't know why timing is set to ddr50.

Best Regards,
Jaehoon Chung

>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>  			   card->ext_csd.generic_cmd6_time,
> 

Re: [PATCH] mmc:core: fix hs400 timing selection

[Jaehoon Chung]

Hi,

On 10/29/2014 08:52 AM, 유한경 wrote:
> Hi I'm Hankyung Yu
> 
> I will answer instead Chanho Min
> 
> After mmc_set_timing(card->host, MMC_TIMING_MMC_HS); 
> 
> Host controller set to SDR transfer
> 
> so is to change to a DDR transfer mode.

As commit message was mentioned, I have checked the JEDEC v5.01 spec(6.6.5).
There is no mention that mode needs to change to DDR mode.

And i know HS400 mode is only support the 8bit buswidth.
If HS200 mode was set to 4bit buswidth, is HS400 working fine?

Best Regards,
Jaehoon Chung

> 
> 
> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com] 
> Sent: Tuesday, October 28, 2014 1:38 PM
> To: Chanho Min; Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung
> Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; HyoJun Im;
> gunho.lee@lge.com; Hankyung Yu; CPGS
> Subject: Re: [PATCH] mmc:core: fix hs400 timing selection
> 
> Hi, Chanho.
> 
> On 10/22/2014 11:55 AM, Chanho Min wrote:
>> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
>> mode, host should perform the following steps.
>>
>>  1. HS200 mode selection completed
>>  2. Set HS_TIMING to 0x01(High Speed)
>>  3. Host changes frequency to =< 52MHz  4. Set the bus width to DDR 
>> 8bit (CMD6)  5. Host may read Driver Strength (CMD8)  6. Set HS_TIMING 
>> to 0x03 (HS400)
>>
>> In current implementation, the order of 2 and 3 is reversed.
>> The HS_TIMING field should be set to 0x1 before the clock frequency is 
>> set to a value not greater than 52 MHz. Otherwise, Initialization of 
>> timing can be failed. Also, the host contoller's UHS timing mode 
>> should be set to DDR50 after the bus width is set to DDR 8bit.
>>
>> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
>> a301a78..52f78e0 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  	 * Before switching to dual data rate operation for HS400,
>>  	 * it is required to convert from HS200 mode to HS mode.
>>  	 */
>> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -	mmc_set_bus_speed(card);
>> -
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
> +1071,14 @@ static 
>> int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	/*
>> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> +	 * be set to a value not greater than 52MHz after the HS_TIMING
>> +	 * field is set to 0x1.
>> +	 */
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> +	mmc_set_bus_speed(card);
>> +
>>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			 EXT_CSD_BUS_WIDTH,
>>  			 EXT_CSD_DDR_BUS_WIDTH_8,
>> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>> +
> 
> I didn't know why timing is set to ddr50.
> 
> Best Regards,
> Jaehoon Chung
> 
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>>  			   card->ext_csd.generic_cmd6_time,
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] mmc:core: fix hs400 timing selection

[유한경]

Hi

I have read the specifications document to answer your question.
It is better, I think, the host controller is changed into DDR50 mode when
to change the eMMC bus mode into DDR50
There will no problem because the no data line will be used in DDR50 mode
during the transition to HS400.


So we drop this patch

Sorry for confusing. Thanks for the helpful response.

-----Original Message-----
From: Jaehoon Chung [mailto:jh80.chung@samsung.com] 
Sent: Wednesday, October 29, 2014 11:05 AM
To: 유한경; 'Jaehoon Chung'; 'Chanho Min'; 'Chris Ball'; 'Ulf Hansson';
'Seungwon Jeon'
Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; 'HyoJun Im';
gunho.lee@lge.com; 'CPGS'
Subject: Re: [PATCH] mmc:core: fix hs400 timing selection

Hi,

On 10/29/2014 08:52 AM, 유한경 wrote:
> Hi I'm Hankyung Yu
> 
> I will answer instead Chanho Min
> 
> After mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> 
> Host controller set to SDR transfer
> 
> so is to change to a DDR transfer mode.

As commit message was mentioned, I have checked the JEDEC v5.01 spec(6.6.5).
There is no mention that mode needs to change to DDR mode.

And i know HS400 mode is only support the 8bit buswidth.
If HS200 mode was set to 4bit buswidth, is HS400 working fine?

Best Regards,
Jaehoon Chung

> 
> 
> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Tuesday, October 28, 2014 1:38 PM
> To: Chanho Min; Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung
> Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; HyoJun 
> Im; gunho.lee@lge.com; Hankyung Yu; CPGS
> Subject: Re: [PATCH] mmc:core: fix hs400 timing selection
> 
> Hi, Chanho.
> 
> On 10/22/2014 11:55 AM, Chanho Min wrote:
>> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
>> mode, host should perform the following steps.
>>
>>  1. HS200 mode selection completed
>>  2. Set HS_TIMING to 0x01(High Speed)  3. Host changes frequency to 
>> =< 52MHz  4. Set the bus width to DDR 8bit (CMD6)  5. Host may read 
>> Driver Strength (CMD8)  6. Set HS_TIMING to 0x03 (HS400)
>>
>> In current implementation, the order of 2 and 3 is reversed.
>> The HS_TIMING field should be set to 0x1 before the clock frequency 
>> is set to a value not greater than 52 MHz. Otherwise, Initialization 
>> of timing can be failed. Also, the host contoller's UHS timing mode 
>> should be set to DDR50 after the bus width is set to DDR 8bit.
>>
>> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
>> a301a78..52f78e0 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  	 * Before switching to dual data rate operation for HS400,
>>  	 * it is required to convert from HS200 mode to HS mode.
>>  	 */
>> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -	mmc_set_bus_speed(card);
>> -
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
> +1071,14 @@ static
>> int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	/*
>> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> +	 * be set to a value not greater than 52MHz after the HS_TIMING
>> +	 * field is set to 0x1.
>> +	 */
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> +	mmc_set_bus_speed(card);
>> +
>>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			 EXT_CSD_BUS_WIDTH,
>>  			 EXT_CSD_DDR_BUS_WIDTH_8,
>> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>> +
> 
> I didn't know why timing is set to ddr50.
> 
> Best Regards,
> Jaehoon Chung
> 
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>>  			   card->ext_csd.generic_cmd6_time,
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" 
> in the body of a message to majordomo@vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] mmc:core: fix hs400 timing selection

[Adrian Hunter]

On 22/10/14 05:55, Chanho Min wrote:
> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 mode,
> host should perform the following steps.
> 
>  1. HS200 mode selection completed
>  2. Set HS_TIMING to 0x01(High Speed)
>  3. Host changes frequency to =< 52MHz
>  4. Set the bus width to DDR 8bit (CMD6)
>  5. Host may read Driver Strength (CMD8)
>  6. Set HS_TIMING to 0x03 (HS400)
> 
> In current implementation, the order of 2 and 3 is reversed.

But HS200 mode supports running at speeds less than 52 MHz whereas
High Speed mode does not support running at speeds greater than
52 MHz.

So the switch command might succeed, but the subsequent send_status
command (see __mmc_switch) could be expected to fail unless the frequency is
changed first.

> The HS_TIMING field should be set to 0x1 before the clock frequency
> is set to a value not greater than 52 MHz. Otherwise, Initialization of
> timing can be failed. Also, the host contoller's UHS timing mode should
> be set to DDR50 after the bus width is set to DDR 8bit.
> 
> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
> Signed-off-by: Chanho Min <chanho.min@lge.com>
> ---
>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index a301a78..52f78e0 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>  	 * Before switching to dual data rate operation for HS400,
>  	 * it is required to convert from HS200 mode to HS mode.
>  	 */
> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> -	mmc_set_bus_speed(card);
> -
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>  			   card->ext_csd.generic_cmd6_time,
> @@ -1074,6 +1071,14 @@ static int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	/*
> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
> +	 * be set to a value not greater than 52MHz after the HS_TIMING
> +	 * field is set to 0x1.
> +	 */
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> +	mmc_set_bus_speed(card);
> +
>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			 EXT_CSD_BUS_WIDTH,
>  			 EXT_CSD_DDR_BUS_WIDTH_8,
> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
> +
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>  			   card->ext_csd.generic_cmd6_time,
> 

RE: [PATCH] mmc:core: fix hs400 timing selection

[유한경]

Hi I'm Hankyung Yu

I will answer instead Chanho Min

HS200 mode right thing to support less than 52Mhz

However CLK <-> DATA delay timing is dependent on the clock.

So only lower clock without adjusting the timing and mode of a control h/w
ever the problem may occur.

So change the operating mode and to lower the clock.


-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
Sent: Tuesday, October 28, 2014 10:24 PM
To: Chanho Min
Cc: Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung; linux-
mmc@vger.kernel.org; linux-kernel@vger.kernel.org; HyoJun Im; gunho.lee@lge.
com; Hankyung Yu
Subject: Re: [PATCH] mmc:core: fix hs400 timing selection

On 22/10/14 05:55, Chanho Min wrote:
> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
> mode, host should perform the following steps.
> 
>  1. HS200 mode selection completed
>  2. Set HS_TIMING to 0x01(High Speed)
>  3. Host changes frequency to =< 52MHz  4. Set the bus width to DDR 
> 8bit (CMD6)  5. Host may read Driver Strength (CMD8)  6. Set HS_TIMING 
> to 0x03 (HS400)
> 
> In current implementation, the order of 2 and 3 is reversed.

But HS200 mode supports running at speeds less than 52 MHz whereas High
Speed mode does not support running at speeds greater than
52 MHz.

So the switch command might succeed, but the subsequent send_status command
(see __mmc_switch) could be expected to fail unless the frequency is
changed first.

> The HS_TIMING field should be set to 0x1 before the clock frequency is 
> set to a value not greater than 52 MHz. Otherwise, Initialization of 
> timing can be failed. Also, the host contoller's UHS timing mode 
> should be set to DDR50 after the bus width is set to DDR 8bit.
> 
> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
> Signed-off-by: Chanho Min <chanho.min@lge.com>
> ---
>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
> a301a78..52f78e0 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>  	 * Before switching to dual data rate operation for HS400,
>  	 * it is required to convert from HS200 mode to HS mode.
>  	 */
> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> -	mmc_set_bus_speed(card);
> -
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
+1071,14 @@ static 
> int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	/*
> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
> +	 * be set to a value not greater than 52MHz after the HS_TIMING
> +	 * field is set to 0x1.
> +	 */
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
> +	mmc_set_bus_speed(card);
> +
>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			 EXT_CSD_BUS_WIDTH,
>  			 EXT_CSD_DDR_BUS_WIDTH_8,
> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>  		return err;
>  	}
>  
> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
> +
>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>  			   card->ext_csd.generic_cmd6_time,
> 

Re: [PATCH] mmc:core: fix hs400 timing selection

[Adrian Hunter]

On 29/10/14 01:30, 유한경 wrote:
> Hi I'm Hankyung Yu
> 
> I will answer instead Chanho Min
> 
> HS200 mode right thing to support less than 52Mhz
> 
> However CLK <-> DATA delay timing is dependent on the clock.
> 
> So only lower clock without adjusting the timing and mode of a control h/w
> ever the problem may occur.
> 
> So change the operating mode and to lower the clock.

I meant something different.

With your patch I find that __mmc_switch() -> send_status() fails.

So I suggest something like this:

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 92247c2..195d48a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1045,21 +1045,28 @@ static int mmc_select_hs400(struct mmc_card *card)
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
 			   card->ext_csd.generic_cmd6_time,
-			   true, true, true);
+			   true, false, false);
+	if (!err) {
+		u32 status;
+
+		/*
+		 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
+		 * be set to a value not greater than 52MHz after the HS_TIMING
+		 * field is set to 0x1.
+		*/
+		mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
+		mmc_set_bus_speed(card);
+
+		err = __mmc_send_status(card, &status, false, 1);
+		if (!err)
+			err = mmc_check_switch_status(card->host, status);
+	}
 	if (err) {
 		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
 			mmc_hostname(host), err);
 		return err;
 	}
 
-	/*
-	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
-	 * be set to a value not greater than 52MHz after the HS_TIMING
-	 * field is set to 0x1.
-	 */
-	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
-	mmc_set_bus_speed(card);
-
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			 EXT_CSD_BUS_WIDTH,
 			 EXT_CSD_DDR_BUS_WIDTH_8,
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 23aa3a3..f917527 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,15 +23,12 @@
 
 #define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */
 
-static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
-				    bool ignore_crc)
+int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
+		      int retries)
 {
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	cmd.opcode = MMC_SEND_STATUS;
 	if (!mmc_host_is_spi(card->host))
 		cmd.arg = card->rca << 16;
@@ -39,7 +36,7 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 	if (ignore_crc)
 		cmd.flags &= ~MMC_RSP_CRC;
 
-	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
+	err = mmc_wait_for_cmd(card->host, &cmd, retries);
 	if (err)
 		return err;
 
@@ -54,7 +51,7 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 
 int mmc_send_status(struct mmc_card *card, u32 *status)
 {
-	return __mmc_send_status(card, status, false);
+	return __mmc_send_status(card, status, false, MMC_CMD_RETRIES);
 }
 
 static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
@@ -419,6 +416,21 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
 	return err;
 }
 
+int mmc_check_switch_status(struct mmc_host *host, u32 status)
+{
+	if (mmc_host_is_spi(host)) {
+		if (status & R1_SPI_ILLEGAL_COMMAND)
+			return -EBADMSG;
+	} else {
+		if (status & 0xFDFFA000)
+			pr_warn("%s: unexpected status %#x after switch\n",
+				mmc_hostname(host), status);
+		if (status & R1_SWITCH_ERROR)
+			return -EBADMSG;
+	}
+	return 0;
+}
+
 /**
  *	__mmc_switch - modify EXT_CSD register
  *	@card: the MMC card associated with the data transfer
@@ -497,7 +509,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 	timeout = jiffies + msecs_to_jiffies(timeout_ms);
 	do {
 		if (send_status) {
-			err = __mmc_send_status(card, &status, ignore_crc);
+			err = __mmc_send_status(card, &status, ignore_crc, 1);
 			if (err)
 				return err;
 		}
@@ -524,18 +536,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		}
 	} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
-	if (mmc_host_is_spi(host)) {
-		if (status & R1_SPI_ILLEGAL_COMMAND)
-			return -EBADMSG;
-	} else {
-		if (status & 0xFDFFA000)
-			pr_warn("%s: unexpected status %#x after switch\n",
-				mmc_hostname(host), status);
-		if (status & R1_SWITCH_ERROR)
-			return -EBADMSG;
-	}
-
-	return 0;
+	return mmc_check_switch_status(host, status);
 }
 EXPORT_SYMBOL_GPL(__mmc_switch);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 6f4b00e..a59319c 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -20,7 +20,10 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
 int mmc_all_send_cid(struct mmc_host *host, u32 *cid);
 int mmc_set_relative_addr(struct mmc_card *card);
 int mmc_send_csd(struct mmc_card *card, u32 *csd);
+int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
+		      int retries);
 int mmc_send_status(struct mmc_card *card, u32 *status);
+int mmc_check_switch_status(struct mmc_host *host, u32 status);
 int mmc_send_cid(struct mmc_host *host, u32 *cid);
 int mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp);
 int mmc_spi_set_crc(struct mmc_host *host, int use_crc);



> 
> 
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
> Sent: Tuesday, October 28, 2014 10:24 PM
> To: Chanho Min
> Cc: Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; HyoJun Im; gunho.lee@lge.
> com; Hankyung Yu
> Subject: Re: [PATCH] mmc:core: fix hs400 timing selection
> 
> On 22/10/14 05:55, Chanho Min wrote:
>> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
>> mode, host should perform the following steps.
>>
>>  1. HS200 mode selection completed
>>  2. Set HS_TIMING to 0x01(High Speed)
>>  3. Host changes frequency to =< 52MHz  4. Set the bus width to DDR 
>> 8bit (CMD6)  5. Host may read Driver Strength (CMD8)  6. Set HS_TIMING 
>> to 0x03 (HS400)
>>
>> In current implementation, the order of 2 and 3 is reversed.
> 
> But HS200 mode supports running at speeds less than 52 MHz whereas High
> Speed mode does not support running at speeds greater than
> 52 MHz.
> 
> So the switch command might succeed, but the subsequent send_status command
> (see __mmc_switch) could be expected to fail unless the frequency is
> changed first.
> 
>> The HS_TIMING field should be set to 0x1 before the clock frequency is 
>> set to a value not greater than 52 MHz. Otherwise, Initialization of 
>> timing can be failed. Also, the host contoller's UHS timing mode 
>> should be set to DDR50 after the bus width is set to DDR 8bit.
>>
>> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
>> a301a78..52f78e0 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  	 * Before switching to dual data rate operation for HS400,
>>  	 * it is required to convert from HS200 mode to HS mode.
>>  	 */
>> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -	mmc_set_bus_speed(card);
>> -
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
> +1071,14 @@ static 
>> int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	/*
>> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> +	 * be set to a value not greater than 52MHz after the HS_TIMING
>> +	 * field is set to 0x1.
>> +	 */
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> +	mmc_set_bus_speed(card);
>> +
>>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			 EXT_CSD_BUS_WIDTH,
>>  			 EXT_CSD_DDR_BUS_WIDTH_8,
>> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>> +
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>>  			   card->ext_csd.generic_cmd6_time,
>>
> 
> 
> 

RE: [PATCH] mmc:core: fix hs400 timing selection

[유한경]

Hi 

I think your suggestion is looks good

I reconsideration my patch & original src

Although Original src is not recommendations of JDEC

Actually change setting order of host controller & emmc device
There would be no problem.

So we drop this patch

Sorry for confusing. Thanks for the helpful response.

-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
Sent: Wednesday, October 29, 2014 6:34 PM
To: 유한경; 'Chanho Min'
Cc: 'Chris Ball'; 'Ulf Hansson'; 'Seungwon Jeon'; 'Jaehoon Chung'; linux-
mmc@vger.kernel.org; linux-kernel@vger.kernel.org; 'HyoJun Im';
gunho.lee@lge.com
Subject: Re: [PATCH] mmc:core: fix hs400 timing selection

On 29/10/14 01:30, 유한경 wrote:
> Hi I'm Hankyung Yu
> 
> I will answer instead Chanho Min
> 
> HS200 mode right thing to support less than 52Mhz
> 
> However CLK <-> DATA delay timing is dependent on the clock.
> 
> So only lower clock without adjusting the timing and mode of a control 
> h/w ever the problem may occur.
> 
> So change the operating mode and to lower the clock.

I meant something different.

With your patch I find that __mmc_switch() -> send_status() fails.

So I suggest something like this:

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 92247c2..
195d48a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1045,21 +1045,28 @@ static int mmc_select_hs400(struct mmc_card *card)
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
 			   card->ext_csd.generic_cmd6_time,
-			   true, true, true);
+			   true, false, false);
+	if (!err) {
+		u32 status;
+
+		/*
+		 * According to JEDEC v5.01 spec (6.6.5), Clock frequency
should
+		 * be set to a value not greater than 52MHz after the
HS_TIMING
+		 * field is set to 0x1.
+		*/
+		mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
+		mmc_set_bus_speed(card);
+
+		err = __mmc_send_status(card, &status, false, 1);
+		if (!err)
+			err = mmc_check_switch_status(card->host, status);
+	}
 	if (err) {
 		pr_err("%s: switch to high-speed from hs200 failed,
err:%d\n",
 			mmc_hostname(host), err);
 		return err;
 	}
 
-	/*
-	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
-	 * be set to a value not greater than 52MHz after the HS_TIMING
-	 * field is set to 0x1.
-	 */
-	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
-	mmc_set_bus_speed(card);
-
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			 EXT_CSD_BUS_WIDTH,
 			 EXT_CSD_DDR_BUS_WIDTH_8,
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index
23aa3a3..f917527 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,15 +23,12 @@
 
 #define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */
 
-static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
-				    bool ignore_crc)
+int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
+		      int retries)
 {
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	cmd.opcode = MMC_SEND_STATUS;
 	if (!mmc_host_is_spi(card->host))
 		cmd.arg = card->rca << 16;
@@ -39,7 +36,7 @@ static inline int __mmc_send_status(struct mmc_card
*card, u32 *status,
 	if (ignore_crc)
 		cmd.flags &= ~MMC_RSP_CRC;
 
-	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
+	err = mmc_wait_for_cmd(card->host, &cmd, retries);
 	if (err)
 		return err;
 
@@ -54,7 +51,7 @@ static inline int __mmc_send_status(struct mmc_card
*card, u32 *status,
 
 int mmc_send_status(struct mmc_card *card, u32 *status)  {
-	return __mmc_send_status(card, status, false);
+	return __mmc_send_status(card, status, false, MMC_CMD_RETRIES);
 }
 
 static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
@@ -419,6 +416,21 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
 	return err;
 }
 
+int mmc_check_switch_status(struct mmc_host *host, u32 status) {
+	if (mmc_host_is_spi(host)) {
+		if (status & R1_SPI_ILLEGAL_COMMAND)
+			return -EBADMSG;
+	} else {
+		if (status & 0xFDFFA000)
+			pr_warn("%s: unexpected status %#x after switch\n",
+				mmc_hostname(host), status);
+		if (status & R1_SWITCH_ERROR)
+			return -EBADMSG;
+	}
+	return 0;
+}
+
 /**
  *	__mmc_switch - modify EXT_CSD register
  *	@card: the MMC card associated with the data transfer
@@ -497,7 +509,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8
index, u8 value,
 	timeout = jiffies + msecs_to_jiffies(timeout_ms);
 	do {
 		if (send_status) {
-			err = __mmc_send_status(card, &status, ignore_crc);
+			err = __mmc_send_status(card, &status, ignore_crc,
1);
 			if (err)
 				return err;
 		}
@@ -524,18 +536,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8
index, u8 value,
 		}
 	} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
-	if (mmc_host_is_spi(host)) {
-		if (status & R1_SPI_ILLEGAL_COMMAND)
-			return -EBADMSG;
-	} else {
-		if (status & 0xFDFFA000)
-			pr_warn("%s: unexpected status %#x after switch\n",
-				mmc_hostname(host), status);
-		if (status & R1_SWITCH_ERROR)
-			return -EBADMSG;
-	}
-
-	return 0;
+	return mmc_check_switch_status(host, status);
 }
 EXPORT_SYMBOL_GPL(__mmc_switch);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h index
6f4b00e..a59319c 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -20,7 +20,10 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32
*rocr);  int mmc_all_send_cid(struct mmc_host *host, u32 *cid);  int
mmc_set_relative_addr(struct mmc_card *card);  int mmc_send_csd(struct
mmc_card *card, u32 *csd);
+int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
+		      int retries);
 int mmc_send_status(struct mmc_card *card, u32 *status);
+int mmc_check_switch_status(struct mmc_host *host, u32 status);
 int mmc_send_cid(struct mmc_host *host, u32 *cid);  int
mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp);  int
mmc_spi_set_crc(struct mmc_host *host, int use_crc);



> 
> 
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> Sent: Tuesday, October 28, 2014 10:24 PM
> To: Chanho Min
> Cc: Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung; linux- 
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; HyoJun Im;
gunho.lee@lge.
> com; Hankyung Yu
> Subject: Re: [PATCH] mmc:core: fix hs400 timing selection
> 
> On 22/10/14 05:55, Chanho Min wrote:
>> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
>> mode, host should perform the following steps.
>>
>>  1. HS200 mode selection completed
>>  2. Set HS_TIMING to 0x01(High Speed)  3. Host changes frequency to 
>> =< 52MHz  4. Set the bus width to DDR 8bit (CMD6)  5. Host may read 
>> Driver Strength (CMD8)  6. Set HS_TIMING to 0x03 (HS400)
>>
>> In current implementation, the order of 2 and 3 is reversed.
> 
> But HS200 mode supports running at speeds less than 52 MHz whereas 
> High Speed mode does not support running at speeds greater than
> 52 MHz.
> 
> So the switch command might succeed, but the subsequent send_status 
> command (see __mmc_switch) could be expected to fail unless the 
> frequency is changed first.
> 
>> The HS_TIMING field should be set to 0x1 before the clock frequency 
>> is set to a value not greater than 52 MHz. Otherwise, Initialization 
>> of timing can be failed. Also, the host contoller's UHS timing mode 
>> should be set to DDR50 after the bus width is set to DDR 8bit.
>>
>> Signed-off-by: Hankyung Yu <hankyung.yu@lge.com>
>> Signed-off-by: Chanho Min <chanho.min@lge.com>
>> ---
>>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
>> a301a78..52f78e0 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  	 * Before switching to dual data rate operation for HS400,
>>  	 * it is required to convert from HS200 mode to HS mode.
>>  	 */
>> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -	mmc_set_bus_speed(card);
>> -
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
> +1071,14 @@ static
>> int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	/*
>> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> +	 * be set to a value not greater than 52MHz after the HS_TIMING
>> +	 * field is set to 0x1.
>> +	 */
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> +	mmc_set_bus_speed(card);
>> +
>>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			 EXT_CSD_BUS_WIDTH,
>>  			 EXT_CSD_DDR_BUS_WIDTH_8,
>> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>> +
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>>  			   card->ext_csd.generic_cmd6_time,
>>
> 
> 
> 

Re: [PATCH] mmc:core: fix hs400 timing selection

[Adrian Hunter]

On 01/11/14 09:23, Chanho Min wrote:
>>> So change the operating mode and to lower the clock.
>>
>> I meant something different.
>>
>> With your patch I find that __mmc_switch() -> send_status() fails.
>>
>> So I suggest something like this:
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 92247c2..195d48a 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1045,21 +1045,28 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>>  			   card->ext_csd.generic_cmd6_time,
>> -			   true, true, true);
>> +			   true, false, false);
>> +	if (!err) {
>> +		u32 status;
>> +
>> +		/*
>> +		 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> +		 * be set to a value not greater than 52MHz after the HS_TIMING
>> +		 * field is set to 0x1.
>> +		*/
>> +		mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> +		mmc_set_bus_speed(card);
>> +
>> +		err = __mmc_send_status(card, &status, false, 1);
> 
> Why was retry-count changed to 1?
> Is there any reason not to use this?
> +               err = mmc_send_status(card, &status);

Error bits are cleared when read, so if the status command is not
successful then error information may have been lost.  So it does
not make sense to retry.

> 
> 
>> +		if (!err)
>> +			err = mmc_check_switch_status(card->host, status);
>> +	}
>>  	if (err) {
>>  		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
>>  			mmc_hostname(host), err);
>>  		return err;
>>  	}
>>
>> -	/*
>> -	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> -	 * be set to a value not greater than 52MHz after the HS_TIMING
>> -	 * field is set to 0x1.
>> -	 */
>> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -	mmc_set_bus_speed(card);
>> -
>>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			 EXT_CSD_BUS_WIDTH,
>>  			 EXT_CSD_DDR_BUS_WIDTH_8,
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 23aa3a3..f917527 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -23,15 +23,12 @@
>>
>>  #define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */
>>
>> -static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
>> -				    bool ignore_crc)
>> +int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
>> +		      int retries)
>>  {
>>  	int err;
>>  	struct mmc_command cmd = {0};
>>
>> -	BUG_ON(!card);
>> -	BUG_ON(!card->host);
>> -
>>  	cmd.opcode = MMC_SEND_STATUS;
>>  	if (!mmc_host_is_spi(card->host))
>>  		cmd.arg = card->rca << 16;
>> @@ -39,7 +36,7 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
>>  	if (ignore_crc)
>>  		cmd.flags &= ~MMC_RSP_CRC;
>>
>> -	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
>> +	err = mmc_wait_for_cmd(card->host, &cmd, retries);
>>  	if (err)
>>  		return err;
>>
>> @@ -54,7 +51,7 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
>>
>>  int mmc_send_status(struct mmc_card *card, u32 *status)
>>  {
>> -	return __mmc_send_status(card, status, false);
>> +	return __mmc_send_status(card, status, false, MMC_CMD_RETRIES);
>>  }
>>
>>  static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
>> @@ -419,6 +416,21 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
>>  	return err;
>>  }
>>
>> +int mmc_check_switch_status(struct mmc_host *host, u32 status)
>> +{
>> +	if (mmc_host_is_spi(host)) {
>> +		if (status & R1_SPI_ILLEGAL_COMMAND)
>> +			return -EBADMSG;
>> +	} else {
>> +		if (status & 0xFDFFA000)
>> +			pr_warn("%s: unexpected status %#x after switch\n",
>> +				mmc_hostname(host), status);
>> +		if (status & R1_SWITCH_ERROR)
>> +			return -EBADMSG;
>> +	}
>> +	return 0;
>> +}
>> +
>>  /**
>>   *	__mmc_switch - modify EXT_CSD register
>>   *	@card: the MMC card associated with the data transfer
>> @@ -497,7 +509,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>>  	timeout = jiffies + msecs_to_jiffies(timeout_ms);
>>  	do {
>>  		if (send_status) {
>> -			err = __mmc_send_status(card, &status, ignore_crc);
>> +			err = __mmc_send_status(card, &status, ignore_crc, 1);
>>  			if (err)
>>  				return err;
>>  		}
>> @@ -524,18 +536,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>>  		}
>>  	} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
>>
>> -	if (mmc_host_is_spi(host)) {
>> -		if (status & R1_SPI_ILLEGAL_COMMAND)
>> -			return -EBADMSG;
>> -	} else {
>> -		if (status & 0xFDFFA000)
>> -			pr_warn("%s: unexpected status %#x after switch\n",
>> -				mmc_hostname(host), status);
>> -		if (status & R1_SWITCH_ERROR)
>> -			return -EBADMSG;
>> -	}
>> -
>> -	return 0;
>> +	return mmc_check_switch_status(host, status);
>>  }
>>  EXPORT_SYMBOL_GPL(__mmc_switch);
>>
>> diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
>> index 6f4b00e..a59319c 100644
>> --- a/drivers/mmc/core/mmc_ops.h
>> +++ b/drivers/mmc/core/mmc_ops.h
>> @@ -20,7 +20,10 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
>>  int mmc_all_send_cid(struct mmc_host *host, u32 *cid);
>>  int mmc_set_relative_addr(struct mmc_card *card);
>>  int mmc_send_csd(struct mmc_card *card, u32 *csd);
>> +int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
>> +		      int retries);
>>  int mmc_send_status(struct mmc_card *card, u32 *status);
>> +int mmc_check_switch_status(struct mmc_host *host, u32 status);
>>  int mmc_send_cid(struct mmc_host *host, u32 *cid);
>>  int mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp);
>>  int mmc_spi_set_crc(struct mmc_host *host, int use_crc);
> 
> Thanks
> Chanho
> 
> 
>