On Fri, 13 Mar 2020 at 10:53, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> On 12/03/20 5:45 pm, Ulf Hansson wrote:
> > On Thu, 12 Mar 2020 at 15:25, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> >>
> >> Add extra busy wait and retries if transfer mode switch fails.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> >> ---
> >> drivers/mmc/core/mmc_ops.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> >> index aa0cab190cd8..619088a94688 100644
> >> --- a/drivers/mmc/core/mmc_ops.c
> >> +++ b/drivers/mmc/core/mmc_ops.c
> >> @@ -599,6 +599,12 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> >> cmd.sanitize_busy = true;
> >>
> >> err = mmc_wait_for_cmd(host, &cmd, MMC_CMD_RETRIES);
> >> + if (err && index == EXT_CSD_HS_TIMING) {
> >> + /* Try harder for timing changes */
> >> + __mmc_poll_for_busy(card, timeout_ms, send_status,
> >> + retry_crc_err, MMC_BUSY_CMD6);
> >> + err = mmc_wait_for_cmd(host, &cmd, MMC_CMD_RETRIES);
> >> + }
> >
> > Hmm, what do you think of moving this to the caller(s) of
> > __mmc_switch() and in particular only at those places were we find it
> > useful. Me personally, would prefer that option.
> >
> > To do that, we may need to have the possibility of specifying the
> > number of retries that should be used in the mmc_wait_for_cmd() call
> > to the caller can check the error code better.
> >
> > Moreover, it looks a bit risky to do the polling for all kinds of
> > errors - shouldn't we do for CRC errors?
> >
>
> What about this then?
Looks good to me!
Is the retry in __mmc_switch() with MMC_CMD_RETRIES okay for now you think?
Kind regards
Uffe
>
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index c2abd417a84a..faa5d30ed891 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1235,20 +1235,35 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
> int err;
> u8 val;
>
> - /* Reduce frequency to HS */
> - max_dtr = card->ext_csd.hs_max_dtr;
> - mmc_set_clock(host, max_dtr);
> -
> /* Switch HS400 to HS DDR */
> val = EXT_CSD_TIMING_HS;
> err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
> val, card->ext_csd.generic_cmd6_time, 0,
> false, true);
> - if (err)
> - goto out_err;
> + if (err == -EILSEQ) {
> + __mmc_poll_for_busy(card, card->ext_csd.generic_cmd6_time,
> + false, true, MMC_BUSY_CMD6);
> + err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_HS_TIMING, val,
> + card->ext_csd.generic_cmd6_time, 0, false,
> + true);
> + }
>
> mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
>
> + /* Reduce frequency to HS */
> + max_dtr = card->ext_csd.hs_max_dtr;
> + mmc_set_clock(host, max_dtr);
> +
> + if (err) {
> + err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_HS_TIMING, val,
> + card->ext_csd.generic_cmd6_time, 0, false,
> + true);
> + }
> + if (err)
> + goto out_err;
> +
> err = mmc_switch_status(card, true);
> if (err)
> goto out_err;
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 6eb87833d478..54afee7c34ae 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -484,9 +484,9 @@ static int mmc_busy_status(struct mmc_card *card, bool retry_crc_err,
> return 0;
> }
>
> -static int __mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
> - bool send_status, bool retry_crc_err,
> - enum mmc_busy_cmd busy_cmd)
> +int __mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
> + bool send_status, bool retry_crc_err,
> + enum mmc_busy_cmd busy_cmd)
> {
> struct mmc_host *host = card->host;
> int err;
> diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
> index 38dcfeeaf6d5..649985507f87 100644
> --- a/drivers/mmc/core/mmc_ops.h
> +++ b/drivers/mmc/core/mmc_ops.h
> @@ -36,6 +36,9 @@ int mmc_interrupt_hpi(struct mmc_card *card);
> int mmc_can_ext_csd(struct mmc_card *card);
> int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
> int mmc_switch_status(struct mmc_card *card, bool crc_err_fatal);
> +int __mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
> + bool send_status, bool retry_crc_err,
> + enum mmc_busy_cmd busy_cmd);
> int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
> enum mmc_busy_cmd busy_cmd);
> int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>
