Re: [PATCH V14 07/24] mmc: block: Use data timeout in card_busy_detect()

[Ulf Hansson]

On 21 November 2017 at 14:42, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> card_busy_detect() has a 10 minute timeout. However the correct timeout is
> the data timeout. Change card_busy_detect() to use the data timeout.

Unfortunate I don't think there is "correct" timeout for this case.

The data->timeout_ns is to indicate for the host to how long the
maximum time it's allowed to take between blocks that are written to
the data lines.

I haven't found a definition of the busy timeout, after the data write
has completed. The spec only mentions that the device moves to
programming state and pulls DAT0 to indicate busy.

Sure, 10 min seems crazy, perhaps something along the lines of 10-20 s
is more reasonable. What do you think?

Br
Uffe

>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  drivers/mmc/core/block.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index e44f6d90aeb4..0874ab3e5c92 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -63,7 +63,6 @@
>  #endif
>  #define MODULE_PARAM_PREFIX "mmcblk."
>
> -#define MMC_BLK_TIMEOUT_MS  (10 * 60 * 1000)        /* 10 minute timeout */
>  #define MMC_SANITIZE_REQ_TIMEOUT 240000
>  #define MMC_EXTRACT_INDEX_FROM_ARG(x) ((x & 0x00FF0000) >> 16)
>
> @@ -921,14 +920,48 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>         return 0;
>  }
>
> -static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
> -               bool hw_busy_detect, struct request *req, bool *gen_err)
> +static unsigned int mmc_blk_clock_khz(struct mmc_host *host)
>  {
> -       unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> +       if (host->actual_clock)
> +               return host->actual_clock / 1000;
> +
> +       /* Clock may be subject to a divisor, fudge it by a factor of 2. */
> +       if (host->ios.clock)
> +               return host->ios.clock / 2000;
> +
> +       /* How can there be no clock */
> +       WARN_ON_ONCE(1);
> +       return 100; /* 100 kHz is minimum possible value */
> +}
> +
> +static unsigned long mmc_blk_data_timeout_jiffies(struct mmc_host *host,
> +                                                 struct mmc_data *data)
> +{
> +       unsigned int ms = DIV_ROUND_UP(data->timeout_ns, 1000000);
> +       unsigned int khz;
> +
> +       if (data->timeout_clks) {
> +               khz = mmc_blk_clock_khz(host);
> +               ms += DIV_ROUND_UP(data->timeout_clks, khz);
> +       }
> +
> +       return msecs_to_jiffies(ms);
> +}
> +
> +static int card_busy_detect(struct mmc_card *card, bool hw_busy_detect,
> +                           struct request *req, bool *gen_err)
> +{
> +       struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> +       struct mmc_data *data = &mqrq->brq.data;
> +       unsigned long timeout;
>         int err = 0;
>         u32 status;
>
> +       timeout = jiffies + mmc_blk_data_timeout_jiffies(card->host, data);
> +
>         do {
> +               bool done = time_after(jiffies, timeout);
> +
>                 err = __mmc_send_status(card, &status, 5);
>                 if (err) {
>                         pr_err("%s: error %d requesting status\n",
> @@ -951,7 +984,7 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
>                  * Timeout if the device never becomes ready for data and never
>                  * leaves the program state.
>                  */
> -               if (time_after(jiffies, timeout)) {
> +               if (done) {
>                         pr_err("%s: Card stuck in programming state! %s %s\n",
>                                 mmc_hostname(card->host),
>                                 req->rq_disk->disk_name, __func__);
> @@ -1011,7 +1044,7 @@ static int send_stop(struct mmc_card *card, unsigned int timeout_ms,
>                 *gen_err = true;
>         }
>
> -       return card_busy_detect(card, timeout_ms, use_r1b_resp, req, gen_err);
> +       return card_busy_detect(card, use_r1b_resp, req, gen_err);
>  }
>
>  #define ERR_NOMEDIUM   3
> @@ -1546,8 +1579,7 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
>                         gen_err = true;
>                 }
>
> -               err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, false, req,
> -                                       &gen_err);
> +               err = card_busy_detect(card, false, req, &gen_err);
>                 if (err)
>                         return MMC_BLK_CMD_ERR;
>         }
> --
> 1.9.1
>