[U-Boot] [PATCH v2 23/26] mmc: Retry some MMC cmds on failure

[Jean-Jacques Hiblot]

On 22/09/2017 15:54, Jaehoon Chung wrote:
> On 09/21/2017 11:30 PM, Jean-Jacques Hiblot wrote:
>> From: Kishon Vijay Abraham I <kishon at ti.com>
>>
>> With certain SD cards like Kingston 8GB/16GB UHS card, it is seen that
>> MMC_CMD_ALL_SEND_CID cmd fails on first attempt, but succeeds
>> subsequently. Therefore, retry MMC_CMD_ALL_SEND_CID cmd a few time
>> as done in Linux kernel.
>> Similarly, it is seen that MMC_CMD_SET_BLOCKLEN may fail on first
>> attempt, therefore retry this cmd a few times as done in kernel.
>>
>> To make it clear that those are optionnal workarounds, a new Kconfig
>> option 'MMC_QUIRKS' is added (enabled by default).
>>
>> Signed-off-by: Vignesh R <vigneshr at ti.com>
>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>> ---
>>   drivers/mmc/Kconfig |  9 +++++++++
>>   drivers/mmc/mmc.c   | 41 +++++++++++++++++++++++++++++++++++++++--
>>   include/mmc.h       |  4 ++++
>>   3 files changed, 52 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 3d577e0..78e58d4 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -33,6 +33,15 @@ config SPL_DM_MMC
>>   
>>   if MMC
>>   
>> +config MMC_QUIRKS
>> +	bool "Enable quirks"
>> +	default y
>> +	help
>> +	  Some cards and hosts may sometimes behave unexpectedly (quirks).
>> +	  This option enable workarounds to handle those quirks. Some of them
>> +	  are enabled by default, other may require additionnal flags or are
>> +	  enabled by the host driver.
>> +
>>   config MMC_VERBOSE
>>   	bool "Output more information about the MMC"
>>   	default y
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index c5eaeaf..6d1bf94 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -279,6 +279,7 @@ int mmc_send_status(struct mmc *mmc, int timeout)
>>   int mmc_set_blocklen(struct mmc *mmc, int len)
>>   {
>>   	struct mmc_cmd cmd;
>> +	int err;
>>   
>>   	if (mmc->ddr_mode)
>>   		return 0;
>> @@ -287,7 +288,24 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
>>   	cmd.resp_type = MMC_RSP_R1;
>>   	cmd.cmdarg = len;
>>   
>> -	return mmc_send_cmd(mmc, &cmd, NULL);
>> +	err = mmc_send_cmd(mmc, &cmd, NULL);
>> +
>> +#ifdef CONFIG_MMC_QUIRKS
>> +	if (err && (mmc->quirks & MMC_QUIRK_RETRY_SET_BLOCKLEN)) {
>> +		int retries = 4;
>> +		/*
>> +		 * It has been seen that SET_BLOCKLEN may fail on the first
>> +		 * attempt, let's try a few more time
>> +		 */
>> +		do {
>> +			err = mmc_send_cmd(mmc, &cmd, NULL);
>> +			if (!err)
>> +				break;
>> +		} while (retries--);
>> +	}
>> +#endif
>> +
>> +	return err;
>>   }
>>   
>>   static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
>> @@ -1881,7 +1899,6 @@ static int mmc_startup(struct mmc *mmc)
>>   		cmd.resp_type = MMC_RSP_R1;
>>   		cmd.cmdarg = 1;
>>   		err = mmc_send_cmd(mmc, &cmd, NULL);
>> -
>>   		if (err)
>>   			return err;
>>   	}
>> @@ -1895,6 +1912,21 @@ static int mmc_startup(struct mmc *mmc)
>>   
>>   	err = mmc_send_cmd(mmc, &cmd, NULL);
>>   
>> +#ifdef CONFIG_MMC_QUIRKS
>> +	if (err && (mmc->quirks & MMC_QUIRK_RETRY_SEND_CID)) {
>> +		int retries = 4;
>> +		/*
>> +		 * It has been seen that SEND_CID may fail on the first
>> +		 * attempt, let's try a few more time
>> +		 */
>> +		do {
>> +			err = mmc_send_cmd(mmc, &cmd, NULL);
>> +			if (!err)
>> +				break;
>> +		} while (retries--);
>> +	}
>> +#endif
>> +
>>   	if (err)
>>   		return err;
>>   
>> @@ -2239,6 +2271,11 @@ int mmc_start_init(struct mmc *mmc)
>>   	if (err)
>>   		return err;
>>   
>> +#ifdef CONFIG_MMC_QUIRKS
>> +	mmc->quirks = MMC_QUIRK_RETRY_SET_BLOCKLEN |
>> +		      MMC_QUIRK_RETRY_SEND_CID;
>> +#endif
>> +
>>   	err = mmc_power_cycle(mmc);
>>   	if (err) {
>>   		/*
>> diff --git a/include/mmc.h b/include/mmc.h
>> index a8901bf..a9ebc88 100644
>> --- a/include/mmc.h
>> +++ b/include/mmc.h
>> @@ -306,6 +306,9 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>>   #define ENHNCD_SUPPORT		(0x2)
>>   #define PART_ENH_ATTRIB		(0x1f)
>>   
>> +#define MMC_QUIRK_RETRY_SEND_CID	BIT(0)
>> +#define MMC_QUIRK_RETRY_SET_BLOCKLEN	BIT(1)
>> +
>>   enum mmc_voltage {
>>   	MMC_SIGNAL_VOLTAGE_000 = 0,
>>   	MMC_SIGNAL_VOLTAGE_120,
>> @@ -591,6 +594,7 @@ struct mmc {
>>   				  * operating mode due to limitations when
>>   				  * accessing the boot partitions
>>   				  */
>> +	u32 quirks;
> Use the #ifdef MMC_QUIRK for quirks?
OK. I'll fix it in the next round
Thanks,
Jean-Jacques
>
>>   };
>>   
>>   struct mmc_hwpart_conf {
>>
>