Re: [PATCH 9/9] mmc: core: Allow CMD13 polling when switch to HS400ES mode

[Ulf Hansson <ulf.hansson@linaro.org>]

On 18 November 2016 at 14:35, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 16/11/16 12:51, Ulf Hansson wrote:
>> In cases when the mmc host doesn't support HW busy detection, polling for
>> busy by using CMD13 is beneficial. The reasons have already been explained
>> in earlier change logs.
>>
>> Moreover, when polling with CMD13 during bus timing changes, we should
>> retry instead of fail when we get CRC errors.
>>
>> Switching to HS400ES includes several steps, where each step changes the
>> bus speed timing. Let's improve the behaviour during these sequences, by
>> allowing CMD13 polling for each of the step. Let's also make sure the CMD13
>> polling becomes retried, while receiving a CRC error.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/mmc/core/mmc.c | 35 +++++++++++------------------------
>>  1 file changed, 11 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 24b9e72..b6f0035 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1227,31 +1227,24 @@ static int mmc_select_hs400es(struct mmc_card *card)
>>               goto out_err;
>>
>>       /* Switch card to HS mode */
>> -     err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> -                        EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>> -                        card->ext_csd.generic_cmd6_time, 0,
>> -                        true, false, true);
>> +     err = mmc_select_hs(card);
>>       if (err) {
>>               pr_err("%s: switch to hs for hs400es failed, err:%d\n",
>>                       mmc_hostname(host), err);
>>               goto out_err;
>>       }
>>
>> -     mmc_set_timing(host, MMC_TIMING_MMC_HS);
>> -     err = mmc_switch_status(card);
>> -     if (err)
>> -             goto out_err;
>> -
>>       mmc_set_clock(host, card->ext_csd.hs_max_dtr);
>>
>> -     /* Switch card to DDR with strobe bit */
>> +     /* Switch card to HS DDR with strobe bit */
>>       val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>> -     err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> -                      EXT_CSD_BUS_WIDTH,
>> -                      val,
>> -                      card->ext_csd.generic_cmd6_time);
>> +     err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +                        EXT_CSD_BUS_WIDTH, val,
>> +                        card->ext_csd.generic_cmd6_time,
>> +                        MMC_TIMING_MMC_DDR52,
>> +                        true, true, true);
>>       if (err) {
>> -             pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
>> +             pr_err("%s: switch to hs ddr for hs400es failed, err:%d\n",
>>                       mmc_hostname(host), err);
>>               goto out_err;
>>       }
>> @@ -1261,26 +1254,20 @@ static int mmc_select_hs400es(struct mmc_card *card)
>>             card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
>>       err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>                          EXT_CSD_HS_TIMING, val,
>> -                        card->ext_csd.generic_cmd6_time, 0,
>> -                        true, false, true);
>> +                        card->ext_csd.generic_cmd6_time,
>> +                        MMC_TIMING_MMC_HS400,
>> +                        true, true, true);
>
> This could be a problem because the CMD13 is being sent to a card in HS400
> enhanced strobe mode but we haven't enabled enhanced strobe on the host
> controller yet.  Previously mmc_switch_status(card) was below
> host->ops->hs400_enhanced_strobe().
>

Yes, more or less the same reasons as before. We need the "tuning" (in
this case the "strobing") to be done before validating the switch
status.

Although, the change a little further above, when switching to HS DDR
in the intermediate step - that should be fine, don't you think?

Kind regards
Uffe