[U-Boot] [PATCH] mmc: fsl_esdhc: Add workaround for auto-clock gate errata ENGcm03648

[Stefano Babic]

On 08/03/2012 13:36, Dirk Behme wrote:
> This patch imports three patches from the Freescale U-Boot with the following
> commit messages:
> 
> ENGR00156405 ESDHC: Add workaround for auto-clock gate errata ENGcm03648
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/drivers/mmc/imx_esdhc.c?h=imx_v2009.08_12.01.01&id=e436525a70fe47623d346bc7d9f08f12ff8ad787
> The errata, not applicable to USDHC, causes ESDHC to shut off clock to the card
> when auto-clock gating is enabled for commands with busy signalling and no data
> phase. The card might require the clock to exit the busy state, so the workaround
> is to disable the auto-clock gate bits in SYSCTL register for such commands. The
> workaround also entails polling on DAT0 bit in the PRSSTAT register to learn when
> busy state is complete. Auto-clock gating is re-enabled at the end of busy state.
> 
> ENGR00156670-1 ESDHC/USDHC: Remove delay before each cmd and some bug fixes
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/drivers/mmc/imx_esdhc.c?h=imx_v2009.08_12.01.01&id=a77c6fec8596891be96b2cdbc742c9824844b92a
> Removed delay of 10 ms before each command. There should not be a need to have this
> delay after the ENGR00156405 patch that polls until card is not busy anymore before
> proceeding to next cmd.
> 
> ENGR00161852: remove u-boot build warnings for mx6q
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/drivers/mmc/imx_esdhc.c?h=imx_v2009.08_12.01.01&id=97efee177f82b082db9d2019ed641c5b99b3f54b
> Remove u-boot build warnings for mx6q.
> 
> SYSCTL_RSTA was defined twice. Remove one definition.
> 
> Signed-off-by: Dirk Behme <dirk.behme at de.bosch.com>
> CC: Andy Fleming <afleming at freescale.com>
> CC: Fabio Estevam <fabio.estevam at freescale.com>
> ---
>  drivers/mmc/fsl_esdhc.c |   61 ++++++++++++++++++++++++++++++++++++++++------

This is not a blocking point for me, but is it maybe better to split
this into three patches as it was originally ? All together makes your
patch not so easy to read.


>  include/fsl_esdhc.h     |    4 ++-
>  2 files changed, 56 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 30db030..694d6fd 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -259,6 +259,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  {
>  	uint	xfertyp;
>  	uint	irqstat;
> +	uint	sysctl_restore = 0;
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
>  	volatile struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
>  
> @@ -279,13 +280,6 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  	while (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA)
>  		;
>  
> -	/* Wait at least 8 SD clock cycles before the next command */
> -	/*
> -	 * Note: This is way more than 8 cycles, but 1ms seems to
> -	 * resolve timing issues with some cards
> -	 */
> -	udelay(1000);
> -

You drop a delay - is it ok for PowerPC, too ? Maybe some PowerPC guys
can answer to this.

>  	/* Set up for a data transfer if we have one */
>  	if (data) {
>  		int err;
> @@ -298,6 +292,14 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  	/* Figure out the transfer arguments */
>  	xfertyp = esdhc_xfertyp(cmd, data);
>  
> +	/* ESDHC errata ENGcm03648: Turn off auto-clock gate for commands
> +	 * with busy signaling and no data
> +	 */

Wrong multiline comment

> +	if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
> +		sysctl_restore = esdhc_read32(&regs->sysctl);
> +		esdhc_write32(&regs->sysctl, sysctl_restore | 0xF);
> +	}
> +

I see two issue. There is not a SDCLKEN bit in the PowerPC ESDHC, as far
as I can see (for example, in MPC8536). Should we not use the HOSTVER
register to handle these cases ?

The comment says you are disabling auto-clock. However, in the register
you are enabling all clocks (PEREN / SDCLKEN /..). Can you explain
better the reason ? Do you want really disabling clocks ?


>  	/* Send the command */
>  	esdhc_write32(&regs->cmdarg, cmd->cmdarg);
>  #if defined(CONFIG_FSL_USDHC)
> @@ -307,19 +309,62 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  #else
>  	esdhc_write32(&regs->xfertyp, xfertyp);
>  #endif
> +
> +	/* Mask all irqs */
> +	esdhc_write32(&regs->irqsigen, 0);
> +
>  	/* Wait for the command to complete */
> -	while (!(esdhc_read32(&regs->irqstat) & IRQSTAT_CC))
> +	while (!(esdhc_read32(&regs->irqstat) & (IRQSTAT_CC | IRQSTAT_CTOE)))
>  		;
>  
>  	irqstat = esdhc_read32(&regs->irqstat);
>  	esdhc_write32(&regs->irqstat, irqstat);
>  
> +	/* Reset CMD and DATA portions on error */
> +	if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {
> +		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))
> +				;
> +		}
> +
> +		/* Restore auto-clock gate if error */
> +		if (!data && (cmd->resp_type & MMC_RSP_BUSY))
> +			esdhc_write32(&regs->sysctl, sysctl_restore);
> +	}
> +

Ok, you reset the DAT lines (according to the manual) before returning
the error.

>  	if (irqstat & CMD_ERR)
>  		return COMM_ERR;
>  
>  	if (irqstat & IRQSTAT_CTOE)
>  		return TIMEOUT;
>  
> +	/* Workaround for ESDHC errata ENGcm03648 */

This ENGcm03648 is not mentioned in the commit message, it seems another
issue...

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================