[U-Boot] [PATCH v1] mmc: dwmmc: Enable small delay before returning error

[Ang, Chee Hong]

On Mon, 2019-02-18 at 12:57 +0100, Marek Vasut wrote:
> On 2/18/19 5:16 AM, chee.hong.ang at intel.com wrote:
> > 
> > From: "Ang, Chee Hong" <chee.hong.ang at intel.com>
> > 
> > 'SET_BLOCKLEN' may occasionally fail on first attempt.
> Why ?
This is part of the workaround of mmc driver which is enabled with
'CONFIG_MMC_QUIRKS' config.
https://github.com/u-boot/u-boot/blob/dbe70c7d4e3d5c705a98d82952e05a591
efd0683/drivers/mmc/mmc.c#L272

> 
> > 
> > This patch enable a small delay in dwmci_send_cmd() on
> > busy, I/O or CRC error to allow the MMC controller recovers
> > from the failure/error on subsequent retries.
> Why 100 uS ?
This is a good question. Perhaps the driver's authors can explain this.
The driver delay 100us after dwmci_send_cmd() success with the command
sent. But it never delay 100us whenever mmc controller response to the
command sent with I/O or CRC errors. MMC drivers expect the first
'SET_BLOCKEN' command issued by dwmci_send_cmd() may fail
intermittently so it will retry up to 4 times before it gave up and
return error. Without this 100us delay after error response,
'SET_BLOCKEN' may sometime fail in 4 attempts in a row. Therefore, we
encountered intermittent failure in loading u-boot (SSBL) from MMC.

> 
> Can we rather detect whether the controller recovered by polling some
> bit?
Hmmm...I can take a look at the databook of this controller and dig
further. Probably this is the limitation of the controller itself. The
mmc driver code which introduce 100us delay after every command sent in
dwmci_send_cmd() looks iffy.
> 
> > 
> > Signed-off-by: Ang, Chee Hong <chee.hong.ang at intel.com>
> > ---
> >  drivers/mmc/dw_mmc.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> > index 7544b84..8dcc518 100644
> > --- a/drivers/mmc/dw_mmc.c
> > +++ b/drivers/mmc/dw_mmc.c
> > @@ -266,8 +266,11 @@ static int dwmci_send_cmd(struct mmc *mmc,
> > struct mmc_cmd *cmd,
> >  	if (data)
> >  		flags = dwmci_set_transfer_mode(host, data);
> >  
> > -	if ((cmd->resp_type & MMC_RSP_136) && (cmd->resp_type &
> > MMC_RSP_BUSY))
> > -		return -1;
> > +	if ((cmd->resp_type & MMC_RSP_136) &&
> > +	    (cmd->resp_type & MMC_RSP_BUSY)) {
> > +		ret = -1;
> > +		goto delay_ret;
> > +	}
> >  
> >  	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> >  		flags |= DWMCI_CMD_ABORT_STOP;
> > @@ -316,11 +319,13 @@ static int dwmci_send_cmd(struct mmc *mmc,
> > struct mmc_cmd *cmd,
> >  		return -ETIMEDOUT;
> >  	} else if (mask & DWMCI_INTMSK_RE) {
> >  		debug("%s: Response Error.\n", __func__);
> > -		return -EIO;
> > +		ret = -EIO;
> > +		goto delay_ret;
> >  	} else if ((cmd->resp_type & MMC_RSP_CRC) &&
> >  		   (mask & DWMCI_INTMSK_RCRC)) {
> >  		debug("%s: Response CRC Error.\n", __func__);
> > -		return -EIO;
> > +		ret = -EIO;
> > +		goto delay_ret;
> >  	}
> >  
> >  
> > @@ -347,6 +352,7 @@ static int dwmci_send_cmd(struct mmc *mmc,
> > struct mmc_cmd *cmd,
> >  		}
> >  	}
> >  
> > +delay_ret:
> >  	udelay(100);
> >  
> >  	return ret;
> > 
>