diff mbox series

[v2,23/26] mmc: Retry some MMC cmds on failure

Message ID 1506004213-22620-24-git-send-email-jjhiblot@ti.com
State New
Headers show
Series None | expand

Commit Message

Jean-Jacques Hiblot Sept. 21, 2017, 2:30 p.m. UTC 
From: Kishon Vijay Abraham I <kishon@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@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 drivers/mmc/Kconfig |  9 +++++++++
 drivers/mmc/mmc.c   | 41 +++++++++++++++++++++++++++++++++++++++--
 include/mmc.h       |  4 ++++
 3 files changed, 52 insertions(+), 2 deletions(-)

Comments

Jaehoon Chung Sept. 22, 2017, 1:54 p.m. UTC | #1 
On 09/21/2017 11:30 PM, Jean-Jacques Hiblot wrote:
> From: Kishon Vijay Abraham I <kishon@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@ti.com>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@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?

>  };
>  
>  struct mmc_hwpart_conf {
>
Jean-Jacques Hiblot Oct. 2, 2017, 9:05 a.m. UTC | #2 
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@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@ti.com>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@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 {
>>
>
diff mbox series

Patch

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;
 };
 
 struct mmc_hwpart_conf {