Re: [RFC PATCH] mmc: dw_mmc: rework the accurate timeout calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29 March 2018 at 10:18, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
> The intention of this patch is to rework the timeout calculation
> for dw_mmc, which isn't ideal enough. It's harmless to add longest
> timeout as normally the transfer is fine and the occasional failure
> could break out by no matter the Hardware interrupt or software timer.
>
> However, it doesn't make sense in the tuning process, as it could
> trigger more timeout than usual, which is very painful for booting
> time. Currently we set TMOUT to 0xffffffff anyway. Assume the max
> clock is 200MHz when doing tuning for HS200, the we have:
>
> MSEC_PER_SEC * 0xffffff / 200MHz = 83ms for hardware timemout interrupt
> and 93ms for software timer.
>
> However, we should note that the cards should guarantee to complete a
> sequence of 40 times CMD21 execulations within 150ms per the spec. But
> we now have 83ms timeout for each CMD21, which only contains 128Bytes
> data playload.
>
> Maybe the simplest way is to reduce the HW/SW timeout value for tuning
> process but it's still worth trying to rework a more accurate timeout.
>
> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>

Jaehoon, any comments on this one?

Kind regards
Uffe

>
> ---
>
>  drivers/mmc/host/dw_mmc.c | 107 +++++++++++++++++++++++++++++++++-------------
>  drivers/mmc/host/dw_mmc.h |   4 ++
>  2 files changed, 82 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 623f4d2..c85d703 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -378,23 +378,84 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci *host, struct mmc_command *cmd)
>         return cmdr;
>  }
>
> -static inline void dw_mci_set_cto(struct dw_mci *host)
> +static void dw_mci_calc_timeout(struct dw_mci *host)
>  {
> -       unsigned int cto_clks;
> -       unsigned int cto_div;
> -       unsigned int cto_ms;
> -       unsigned long irqflags;
> +       struct mmc_data *data = host->data;
> +       struct mmc_host *mmc = host->slot->mmc;
> +       u64 trsf_time_per_blksz;
> +       u64 idle_time_per_trsf;
> +       u64 val;
> +       struct mmc_ios *ios = &mmc->ios;
> +       unsigned char bus_width = ios->bus_width;
> +       unsigned int freq, clk_div;
> +       unsigned int cto_clks, drto_clks;
> +
> +       clk_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> +       if (clk_div == 0)
> +               clk_div = 1;
> +
> +       freq = host->bus_hz / clk_div;
> +
> +       /*
> +        * The largest 136 bit response in 100KHz is nearly 1ms,
> +        * and add other transfer parameter per spec, we should
> +        * allow 2ms maybe. And should also add some spare time
> +        * to tolerate anything unknown leading to unexpect long
> +        * latency.
> +        */
> +       host->cmd_timeout_ns = 10 * NSEC_PER_MSEC;
> +       host->data_timeout_ns = 10 * NSEC_PER_MSEC;
> +
> +       if (data) {
> +               trsf_time_per_blksz = DIV_ROUND_UP_ULL((u64)data->blksz *
> +                                       NSEC_PER_SEC * (8 / bus_width), freq);
> +
> +               /*
> +                * Especially multiply by 2 to account for write for anything
> +                * unknown in the cards, for each block.
> +                */
> +               if (data->flags & MMC_DATA_WRITE)
> +                       trsf_time_per_blksz *= 2;
> +
> +               /* Calculate idle time from core basd on spec */
> +               idle_time_per_trsf = DIV_ROUND_UP_ULL(data->timeout_ns,
> +                                                     NSEC_PER_USEC);
> +               if (data->timeout_clks) {
> +                       /* Align up from cycle of MHz to Hz */
> +                       val = 1000000ULL * data->timeout_clks;
> +                       if (DIV_ROUND_UP_ULL(val, freq))
> +                               idle_time_per_trsf++;
> +                       idle_time_per_trsf += val;
> +               }
> +
> +               /* Calculate timeout for the entire data */
> +               host->data_timeout_ns +=
> +                       data->blocks * (DIV_ROUND_UP_ULL(idle_time_per_trsf,
> +                                       NSEC_PER_USEC) + trsf_time_per_blksz);
>
> -       cto_clks = mci_readl(host, TMOUT) & 0xff;
> -       cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> -       if (cto_div == 0)
> -               cto_div = 1;
> +               drto_clks = DIV_ROUND_UP_ULL(host->data_timeout_ns * freq,
> +                                            NSEC_PER_SEC);
> +               if (drto_clks > 0xffffff)
> +                       drto_clks = 0xffffff;
>
> -       cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
> -                                 host->bus_hz);
> +               /* Set accurate hw data timeout */
> +               mci_writel(host, TMOUT, drto_clks << 8);
> +       }
>
> -       /* add a bit spare time */
> -       cto_ms += 10;
> +       host->cmd_timeout_ns += host->cmd->busy_timeout * NSEC_PER_MSEC;
> +       cto_clks = DIV_ROUND_UP_ULL(host->cmd_timeout_ns * freq,
> +                                   NSEC_PER_SEC);
> +       if (cto_clks > 0xff)
> +               cto_clks = 0xff;
> +
> +       /* Set accurate cmd timeout */
> +       mci_writel(host, TMOUT, cto_clks |
> +                               (mci_readl(host, TMOUT) & 0xffffff00));
> +}
> +
> +static inline void dw_mci_set_cto(struct dw_mci *host)
> +{
> +       unsigned long irqflags;
>
>         /*
>          * The durations we're working with are fairly short so we have to be
> @@ -412,7 +473,7 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
>         spin_lock_irqsave(&host->irq_lock, irqflags);
>         if (!test_bit(EVENT_CMD_COMPLETE, &host->pending_events))
>                 mod_timer(&host->cto_timer,
> -                       jiffies + msecs_to_jiffies(cto_ms) + 1);
> +                       jiffies + nsecs_to_jiffies(host->cmd_timeout_ns) + 1);
>         spin_unlock_irqrestore(&host->irq_lock, irqflags);
>  }
>
> @@ -424,6 +485,8 @@ static void dw_mci_start_command(struct dw_mci *host,
>                  "start command: ARGR=0x%08x CMDR=0x%08x\n",
>                  cmd->arg, cmd_flags);
>
> +       dw_mci_calc_timeout(host);
> +
>         mci_writel(host, CMDARG, cmd->arg);
>         wmb(); /* drain writebuffer */
>         dw_mci_wait_while_busy(host, cmd_flags);
> @@ -1923,26 +1986,12 @@ static int dw_mci_data_complete(struct dw_mci *host, struct mmc_data *data)
>
>  static void dw_mci_set_drto(struct dw_mci *host)
>  {
> -       unsigned int drto_clks;
> -       unsigned int drto_div;
> -       unsigned int drto_ms;
>         unsigned long irqflags;
>
> -       drto_clks = mci_readl(host, TMOUT) >> 8;
> -       drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> -       if (drto_div == 0)
> -               drto_div = 1;
> -
> -       drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
> -                                  host->bus_hz);
> -
> -       /* add a bit spare time */
> -       drto_ms += 10;
> -
>         spin_lock_irqsave(&host->irq_lock, irqflags);
>         if (!test_bit(EVENT_DATA_COMPLETE, &host->pending_events))
>                 mod_timer(&host->dto_timer,
> -                         jiffies + msecs_to_jiffies(drto_ms));
> +                         jiffies + nsecs_to_jiffies(host->data_timeout_ns));
>         spin_unlock_irqrestore(&host->irq_lock, irqflags);
>  }
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 46e9f8e..4330128 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -126,7 +126,9 @@ struct dw_mci_dma_slave {
>   * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>   * @cmd11_timer: Timer for SD3.0 voltage switch over scheme.
>   * @cto_timer: Timer for broken command transfer over scheme.
> + * @cmd_timeout_ns: Timeout value accociated with cto_timer
>   * @dto_timer: Timer for broken data transfer over scheme.
> + * @data_timeout_ns: Timeout value accociated with dto_timer
>   *
>   * Locking
>   * =======
> @@ -233,7 +235,9 @@ struct dw_mci {
>
>         struct timer_list       cmd11_timer;
>         struct timer_list       cto_timer;
> +       u64                     cmd_timeout_ns;
>         struct timer_list       dto_timer;
> +       u64                     data_timeout_ns;
>  };
>
>  /* DMA ops for Internal/External DMAC interface */
> --
> 1.9.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux