[PATCH] mmc: dw_mmc: introduce timer for broken command transfer over scheme

[Jaehoon Chung]

Hi Shawn,

On 07/11/2017 06:38 PM, Shawn Lin wrote:
> From: Addy Ke <addy.ke at rock-chips.com>
> 
> Per the databook of designware mmc controller 2.70a, table 3-2, cmd
> done interrupt should be fired as soon as the the cmd is sent via
> cmd line. And the response timeout interrupt should be generated
> unconditioinally as well if the controller doesn't receive the resp.
> However that doesn't seem to meet the fact of rockchip specified Soc
> platforms using dwmmc. We have continuously found the the cmd done or
> response timeout interrupt missed somehow which took us a long time to
> understand what was happening. Finally we narrow down the root to
> the reconstruction of sample circuit for dwmmc IP introduced by
> rockchip and the buggy design sweeps over all the existing rockchip
> Socs using dwmmc disastrously.
> 
> It seems no way to work around this bug without the proper break-out
> mechanism so that we seek for a parallel pair the same as the handling
> for missing data response timeout, namely dto timer. Adding this cto
> timer seems easily to handle this bug but it's hard to restrict the code
> under the rockchip specified context. So after merging this patch, it
> sets up the cto timer for all the platforms using dwmmc IP which isn't
> ideal but at least we don't advertise new quirk here. Fortunately, no
> obvious performance regression was found by test and the pre-existing
> similar catch-all timer for sdhci has proved it's an acceptant way to
> make the code as robust as possible.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196321
> Signed-off-by: Addy Ke <addy.ke at rock-chips.com>
> Signed-off-by: Ziyuan Xu <xzy.xu at rock-chips.com>
> [shawn.lin: rewrite the code and the commit msg throughout]
> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>

Will pick this after testing with my targets.

Best Regards,
Jaehoon Chung

> 
> ---
> 
>  drivers/mmc/host/dw_mmc.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/dw_mmc.h |  2 ++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a9dfb26..2f6121e 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -398,6 +398,21 @@ 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)
> +{
> +	unsigned int cto_clks;
> +	unsigned int cto_ms;
> +
> +	cto_clks = mci_readl(host, TMOUT) & 0xff;
> +	cto_ms = DIV_ROUND_UP(cto_clks, host->bus_hz / 1000);
> +
> +	/* add a bit spare time */
> +	cto_ms += 10;
> +
> +	mod_timer(&host->cto_timer,
> +		  jiffies + msecs_to_jiffies(cto_ms) + 1);
> +}
> +
>  static void dw_mci_start_command(struct dw_mci *host,
>  				 struct mmc_command *cmd, u32 cmd_flags)
>  {
> @@ -410,6 +425,10 @@ static void dw_mci_start_command(struct dw_mci *host,
>  	wmb(); /* drain writebuffer */
>  	dw_mci_wait_while_busy(host, cmd_flags);
>  
> +	/* response expected command only */
> +	if (cmd_flags & SDMMC_CMD_RESP_EXP)
> +		dw_mci_set_cto(host);
> +
>  	mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START);
>  }
>  
> @@ -2599,6 +2618,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>  		}
>  
>  		if (pending & DW_MCI_CMD_ERROR_FLAGS) {
> +			del_timer(&host->cto_timer);
>  			mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS);
>  			host->cmd_status = pending;
>  			smp_wmb(); /* drain writebuffer */
> @@ -2642,6 +2662,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>  		}
>  
>  		if (pending & SDMMC_INT_CMD_DONE) {
> +			del_timer(&host->cto_timer);
>  			mci_writel(host, RINTSTS, SDMMC_INT_CMD_DONE);
>  			dw_mci_cmd_interrupt(host, pending);
>  		}
> @@ -2914,6 +2935,30 @@ static void dw_mci_cmd11_timer(unsigned long arg)
>  	tasklet_schedule(&host->tasklet);
>  }
>  
> +static void dw_mci_cto_timer(unsigned long arg)
> +{
> +	struct dw_mci *host = (struct dw_mci *)arg;
> +
> +	switch (host->state) {
> +	case STATE_SENDING_CMD11:
> +	case STATE_SENDING_CMD:
> +	case STATE_SENDING_STOP:
> +		/*
> +		 * If CMD_DONE interrupt does NOT come in sending command
> +		 * state, we should notify the driver to terminate current
> +		 * transfer and report a command timeout to the core.
> +		 */
> +		host->cmd_status = SDMMC_INT_RTO;
> +		set_bit(EVENT_CMD_COMPLETE, &host->pending_events);
> +		tasklet_schedule(&host->tasklet);
> +		break;
> +	default:
> +		dev_warn(host->dev, "Unexpected command timeout, state %d\n",
> +			 host->state);
> +		break;
> +	}
> +}
> +
>  static void dw_mci_dto_timer(unsigned long arg)
>  {
>  	struct dw_mci *host = (struct dw_mci *)arg;
> @@ -3085,6 +3130,9 @@ int dw_mci_probe(struct dw_mci *host)
>  	setup_timer(&host->cmd11_timer,
>  		    dw_mci_cmd11_timer, (unsigned long)host);
>  
> +	setup_timer(&host->cto_timer,
> +		    dw_mci_cto_timer, (unsigned long)host);
> +
>  	setup_timer(&host->dto_timer,
>  		    dw_mci_dto_timer, (unsigned long)host);
>  
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 75da375..4210927 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -126,6 +126,7 @@ struct dw_mci_dma_slave {
>   * @irq: The irq value to be passed to request_irq.
>   * @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.
>   * @dto_timer: Timer for broken data transfer over scheme.
>   *
>   * Locking
> @@ -232,6 +233,7 @@ struct dw_mci {
>  	int			sdio_id0;
>  
>  	struct timer_list       cmd11_timer;
> +	struct timer_list       cto_timer;
>  	struct timer_list       dto_timer;
>  };
>  
>