[U-Boot] [PATCH 22/22] mmc: Retry some MMC cmds on failure

[Jean-Jacques Hiblot]

On 15/05/2017 05:28, Simon Glass wrote:
> On 12 May 2017 at 12:16, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>> From: Vignesh R <vigneshr 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 at least thrice
>> as done in Linux kernel.
>> Similarly, it is seen that MMC_CMD_SET_BLOCKLEN may fail on first
>> attempt, therefore retry this cmd five times as done in kernel.
>>
>> 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/mmc.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
> So sad to see this sort of thing.
>
> Can we enable this via a quirk and a Kconfig? Could default to on, but
> I'm not sure we want this code in regardless.
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
I admit that it's ugly...  no clean code survives contact with a few 
hundred different hardware parts.
We could add KConfig options to enable/disable those quirks but I don't 
see the point of being able to. It's just a basic retry that has no 
impact on things that work well already and might make other work more 
reliably.

>
>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index c7dda64..49edf52 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -275,6 +275,8 @@ int mmc_send_status(struct mmc *mmc, int timeout)
>>   int mmc_set_blocklen(struct mmc *mmc, int len)
>>   {
>>          struct mmc_cmd cmd;
>> +       int retries = 5;
>> +       int err;
>>
>>          if (mmc->ddr_mode)
>>                  return 0;
>> @@ -282,8 +284,13 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
>>          cmd.cmdidx = MMC_CMD_SET_BLOCKLEN;
>>          cmd.resp_type = MMC_RSP_R1;
>>          cmd.cmdarg = len;
>> +       do {
>> +               err = mmc_send_cmd(mmc, &cmd, NULL);
>> +               if (!err)
>> +                       break;
>> +       } while (retries--);
>>
>> -       return mmc_send_cmd(mmc, &cmd, NULL);
>> +       return err;
>>   }
>>
>>   static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
>> @@ -1867,6 +1874,7 @@ static int mmc_startup(struct mmc *mmc)
>>          u64 cmult, csize;
>>          struct mmc_cmd cmd;
>>          struct blk_desc *bdesc;
>> +       int retries = 3;
>>
>>   #ifdef CONFIG_MMC_SPI_CRC_ON
>>          if (mmc_host_is_spi(mmc)) { /* enable CRC check for spi */
>> @@ -1874,7 +1882,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;
>>          }
>> @@ -1886,7 +1893,9 @@ static int mmc_startup(struct mmc *mmc)
>>          cmd.resp_type = MMC_RSP_R2;
>>          cmd.cmdarg = 0;
>>
>> -       err = mmc_send_cmd(mmc, &cmd, NULL);
>> +       do {
>> +               err = mmc_send_cmd(mmc, &cmd, NULL);
>> +       } while (err && retries-- > 0);
>>
>>          if (err)
>>                  return err;
>> --
>> 1.9.1
>>