[RFC PATCH] mmc: core: HS DDR switch, don't change timing before checking status

[Nicolas Ferre]

Le 15/03/2017 à 12:48, Nicolas Ferre a écrit :
> Le 10/03/2017 à 15:21, Ludovic Desroches a écrit :
>> From: Ludovic Desroches <ludovic.desroches at microchip.com>
>>
>> The commit e173f8911f09 mmc: core: Update CMD13 polling policy when
>> switch to HS DDR mode in addition to fix the management of CRC error,
>> changes the place where the DDR52 timing is set.
>>
>> Before this commit, the sequence was:
>> - set width to 8 with MMC_HS timing
>> - send the switch command
>> - check the status
>> - set width to 8 with MMC_DDR52 timing
>> - send the switch command
>> - check the status
>> Now:
>> - set width to 8 with MMC_HS timing
>> - send the switch command
>> - set width to 8 with MMC_DDR52 timing
>> - check the status
>>
>> It may lead to get an error when checking the status with some devices.
>>
>> Signed-off-by: Ludovic Desroches <ludovic.desroches at microchip.com>
> 
> Tested-by: Nicolas Ferre <nicolas.ferre at microchip.com>
> 
> On sama5d2 Xplained (eMMC on sdhci).
> 
> Note that without this patch the system is unable to boot. Even if it
> was present on 4.10 but we didn't spot it, I see now this as a regression.

Thorsten,

I believe that Ludovic found the root cause of this issue that we were
experiencing on our platform.

He investigated with the help of Ulf and it is now solved by his patch
"[PATCH] mmc: sdhci-of-at91: fix MMC_DDR_52 timing selection" already
integrated in Linus' tree last Friday.

As far as we are concerned, you can remove the mention of this
regression from your report
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1368368.html

Thanks a lot for maintaining this useful tool!

Best regards,

> We would also need to add the tags:
> Cc: stable <stable at vger.kernel.org> #4.10+
> Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch
> to HS DDR mode")
> 
> 
> Best regards,
> 
>> ---
>>  drivers/mmc/core/mmc.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 0fccca0..b837148 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1062,7 +1062,7 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
>>  			   EXT_CSD_BUS_WIDTH,
>>  			   ext_csd_bits,
>>  			   card->ext_csd.generic_cmd6_time,
>> -			   MMC_TIMING_MMC_DDR52,
>> +			   0,
>>  			   true, true, true);
>>  	if (err) {
>>  		pr_err("%s: switch to bus width %d ddr failed\n",
>> @@ -1106,6 +1106,9 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
>>  	if (err)
>>  		err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330);
>>  
>> +	if (!err)
>> +		mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
>> +
>>  	return err;
>>  }
>>  
>>
> 
> 


-- 
Nicolas Ferre