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 <[email protected]>
> CC: Andy Fleming <[email protected]>
> CC: Fabio Estevam <[email protected]>
> ---
> 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(®s->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(®s->sysctl);
> + esdhc_write32(®s->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(®s->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(®s->xfertyp, xfertyp);
> #endif
> +
> + /* Mask all irqs */
> + esdhc_write32(®s->irqsigen, 0);
> +
> /* Wait for the command to complete */
> - while (!(esdhc_read32(®s->irqstat) & IRQSTAT_CC))
> + while (!(esdhc_read32(®s->irqstat) & (IRQSTAT_CC | IRQSTAT_CTOE)))
> ;
>
> irqstat = esdhc_read32(®s->irqstat);
> esdhc_write32(®s->irqstat, irqstat);
>
> + /* Reset CMD and DATA portions on error */
> + if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {
> + esdhc_write32(®s->sysctl, esdhc_read32(®s->sysctl) |
> + SYSCTL_RSTC);
> + while (esdhc_read32(®s->sysctl) & SYSCTL_RSTC)
> + ;
> +
> + if (data) {
> + esdhc_write32(®s->sysctl,
> + esdhc_read32(®s->sysctl) |
> + SYSCTL_RSTD);
> + while ((esdhc_read32(®s->sysctl) & SYSCTL_RSTD))
> + ;
> + }
> +
> + /* Restore auto-clock gate if error */
> + if (!data && (cmd->resp_type & MMC_RSP_BUSY))
> + esdhc_write32(®s->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: [email protected]
=====================================================================
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
