[U-Boot] [PATCH 7/7] powerpc/esdhc: Update esdhc command execution process

Pantelis Antoniou panto at antoniou-consulting.com
Sun Dec 8 12:28:49 CET 2013


Hi Haijun,

On Nov 5, 2013, at 8:23 AM, Haijun Zhang wrote:

> The max timeout value esdhc host can accept was about 2.69 sec
> At 50 Mhz SD_CLK period, the max busy timeout
> value = 2^27 * SD_CLK period ~= 2.69 sec.
> 
> In case erase command CMD38 timeout is caculate  by
^ calculated?

> mult * 300ms * num(unit by erase group), so the time one erase
> group need should be more than 300ms, 500ms should be enough.
> 
> 1. Add data reset for data error and command with busy error.
> 2. Add timeout value detecting during waiting transfer complete.
> 3. Ignore Command inhibit (DAT) state when excuting CMD12.
> 4. Add command CRC error detecting.
> 5. Enlarged the timeout value used for busy state release.
> 6. In case eSDHC host version 2.3, host will signal transfer complete
>   interrupt once busy state was release.
> 
> Signed-off-by: Haijun Zhang <Haijun.Zhang at freescale.com>
> ---
> drivers/mmc/fsl_esdhc.c | 165 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 108 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 9f4d3a2..a8503fb 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -266,26 +266,36 @@ static int
> esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> {
> 	uint	xfertyp;
> -	uint	irqstat;
> +	uint	irqstat, mask;
> 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> 	volatile struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
> +	int ret = 0, timeout;
> +
> +	esdhc_write32(&regs->irqstat, -1);

^ Please don't -1 for unsigned quantities. Use 0xffffffffU if need be.

I know it used to be -1, but since we're making the changes here, lets fix
it in every case.

> +
> +	sync();
> +
> +	mask = PRSSTAT_CICHB | PRSSTAT_CIDHB;
> 
> #ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
> 	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> 		return 0;
> +#else
> +	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> +		mask &= ~PRSSTAT_CIDHB;
> #endif
> 
> -	esdhc_write32(&regs->irqstat, -1);
> -
> -	sync();
> -
> 	/* Wait for the bus to be idle */
> -	while ((esdhc_read32(&regs->prsstat) & PRSSTAT_CICHB) ||
> -			(esdhc_read32(&regs->prsstat) & PRSSTAT_CIDHB))
> -		;
> -
> -	while (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA)
> -		;
> +	timeout = 1000;
> +	while (esdhc_read32(&regs->prsstat) & mask) {
> +		if (timeout == 0) {
> +			printf("\nController never released inhibit bit(s).\n");
> +			ret = COMM_ERR;
> +			goto reset;
> +		}
> +		timeout--;
> +		mdelay(1);
> +	}
> 
> 	/* Wait at least 8 SD clock cycles before the next command */
> 	/*
> @@ -296,11 +306,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> 
> 	/* Set up for a data transfer if we have one */
> 	if (data) {
> -		int err;
> -
> -		err = esdhc_setup_data(mmc, data);
> -		if(err)
> -			return err;
> +		ret = esdhc_setup_data(mmc, data);
> +		if (ret)
> +			goto reset;
> 	}
> 
> 	/* Figure out the transfer arguments */
> @@ -325,43 +333,14 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> 
> 	irqstat = esdhc_read32(&regs->irqstat);
> 
> -	/* Reset CMD and DATA portions on error */
> -	if (irqstat & CMD_ERR) {
> -		esdhc_write32(&regs->sysctl, esdhc_read32(&regs->sysctl) |
> -			      SYSCTL_RSTC);
> -		while (esdhc_read32(&regs->sysctl) & SYSCTL_RSTC)
> -			;
> -
> -		if (data) {
> -			esdhc_write32(&regs->sysctl,
> -				      esdhc_read32(&regs->sysctl) |
> -				      SYSCTL_RSTD);
> -			while ((esdhc_read32(&regs->sysctl) & SYSCTL_RSTD))
> -				;
> -		}
> +	if (irqstat & IRQSTAT_CTOE) {
> +		ret = TIMEOUT;
> +		goto reset;
> 	}
> 
> -	if (irqstat & IRQSTAT_CTOE)
> -		return TIMEOUT;
> -
> -	if (irqstat & CMD_ERR)
> -		return COMM_ERR;
> -
> -	/* Workaround for ESDHC errata ENGcm03648 */
> -	if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
> -		int timeout = 2500;
> -
> -		/* Poll on DATA0 line for cmd with busy signal for 250 ms */
> -		while (timeout > 0 && !(esdhc_read32(&regs->prsstat) &
> -					PRSSTAT_DAT0)) {
> -			udelay(100);
> -			timeout--;
> -		}
> -
> -		if (timeout <= 0) {
> -			printf("Timeout waiting for DAT0 to go high!\n");
> -			return TIMEOUT;
> -		}
> +	if (irqstat & CMD_ERR) {
> +		ret = COMM_ERR;
> +		goto reset;
> 	}
> 
> 	/* Copy the response to the response buffer */
> @@ -379,28 +358,100 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> 	} else
> 		cmd->response[0] = esdhc_read32(&regs->cmdrsp0);
> 
> -	/* Wait until all of the blocks are transferred */
> +	/*
> +	 * At 50 Mhz SD_CLK period, the max busy timeout
> +	 * value or data transfer time need was about
> +	 * = 2^27 * SD_CLK period ~= 2.69 sec.
> +	 * So wait max 10 sec for data transfer complete or busy
> +	 * state release.
> +	 */
> +	timeout = 10000;
> +
> +	/*
> +	 * eSDHC host V2.3 has response busy interrupt, so
> +	 * we should wait for busy state to be released and data
> +	 * was out of programing state before next command send.
> +	 */
> +#ifdef CONFIG_FSL_SDHC_V2_3
> +	if (data || (cmd->resp_type & MMC_RSP_BUSY)) {
> +#else
> 	if (data) {
> +#endif
> #ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> 		esdhc_pio_read_write(mmc, data);
> #else
> 		do {
> 			irqstat = esdhc_read32(&regs->irqstat);
> 
> -			if (irqstat & IRQSTAT_DTOE)
> -				return TIMEOUT;
> +			if (irqstat & IRQSTAT_DTOE) {
> +				ret = TIMEOUT;
> +				break;
> +			}
> 
> -			if (irqstat & DATA_ERR)
> -				return COMM_ERR;
> -		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
> +			if (irqstat & DATA_ERR) {
> +				ret = COMM_ERR;
> +				break;
> +			}
> +
> +			if (timeout <= 0) {
> +				ret = TIMEOUT;
> +				break;
> +			}
> +			mdelay(1);
> +			timeout--;
> +		} while (((irqstat & DATA_COMPLETE) != DATA_COMPLETE) &&
> +				(esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
> #endif
> 		if (data->flags & MMC_DATA_READ)
> 			check_and_invalidate_dcache_range(cmd, data);
> 	}
> 
> +	 /* Workaround for ESDHC errata ENGcm03648 */
> +	if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
> +		int timeout = 5000;
> +
> +		/* Poll on DATA0 line for cmd with busy signal for 500 ms */
> +		while (timeout > 0 && !(esdhc_read32(&regs->prsstat) &
> +					PRSSTAT_DAT0)) {
> +			udelay(100);
> +			timeout--;
> +		}
> +
> +		if (timeout <= 0) {
> +			printf("\nTimeout waiting for DAT0 to go high!\n");
> +			ret = TIMEOUT;
> +			goto reset;
> +		}
> +	}
> +
> +	if (ret)
> +		goto reset;
> +
> 	esdhc_write32(&regs->irqstat, -1);
> 
> 	return 0;
> +
> +reset:
> +
> +	/* Reset CMD and DATA portions on error */
> +	if (irqstat & (CMD_ERR | DATA_ERR)) {
> +		esdhc_write32(&regs->sysctl, esdhc_read32(&regs->sysctl) |
> +				SYSCTL_RSTC);
> +		while (esdhc_read32(&regs->sysctl) & SYSCTL_RSTC)
> +			;
> +
> +		if (data || (cmd->resp_type & MMC_RSP_BUSY)) {
> +			esdhc_write32(&regs->sysctl,
> +					esdhc_read32(&regs->sysctl) |
> +					SYSCTL_RSTA);
> +			while ((esdhc_read32(&regs->sysctl) & SYSCTL_RSTA))
> +				;
> +		}

In general I do not like loops without a timeout terminating condition.
I'll let it pass for now, but please follow up with a patch for the whole driver
turning those into timeout condition terminated loops. 

> +	}
> +
> +	esdhc_write32(&regs->irqstat, -1);
> +
> +	return ret;
> }
> 
> static void set_sysctl(struct mmc *mmc, uint clock)
> -- 
> 1.8.4
> 
> 

Regards 

-- Pantelis



More information about the U-Boot mailing list