Re: [U-Boot] [PATCH 5/6] i.mx: fsl_esdhc: add the i.mx6q support

[Marek Vasut]

> The mmc host controller on the i.mx6q is called usdhc which
> is redesigned based on the freescale esdhc controller.
> 
> The usdhc controller is almost compatible with esdhc except
> it adds one misc control register from user using experience.
> 
> Signed-off-by: Jason Liu <jason....@linaro.org>
> ---
>  drivers/mmc/fsl_esdhc.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index ec953f0..cd17ef2 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -58,7 +58,12 @@ struct fsl_esdhc {
>       uint    autoc12err;
>       uint    hostcapblt;
>       uint    wml;
> -     char    reserved1[8];
> +#if defined(CONFIG_FSL_USDHC)
> +     uint    mixctrl;
> +     char    reserved1[4];
> +#else
> +     char    reserved1[8];
> +#endif

Hi Jason,

can't we just drop this ifdef ?

>       uint    fevt;
>       char    reserved2[168];
>       uint    hostver;
> @@ -298,6 +303,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> struct mmc_data *data)
> 
>       /* Send the command */
>       esdhc_write32(&regs->cmdarg, cmd->cmdarg);
> +#if defined(CONFIG_FSL_USDHC)
> +     esdhc_write32(&regs->mixctrl, xfertyp & 0xFFFF);
> +#endif
>       esdhc_write32(&regs->xfertyp, xfertyp);

Why is this duplicated? This seems like a huge user-experience nonsense to me :)

If you write to xfertyp register, you still have to write the same thing to 
mixctrl? Or if you do it vice versa, won't it work ? Why did you add the 
register?

> 
>       /* Wait for the command to complete */
> @@ -482,7 +490,11 @@ int fsl_esdhc_initialize(bd_t *bis, struct
> fsl_esdhc_cfg *cfg)
> 
>       mmc = malloc(sizeof(struct mmc));
> 
> +#if defined(CONFIG_FSL_USDHC)
> +     sprintf(mmc->name, "FSL_USDHC");
> +#else
>       sprintf(mmc->name, "FSL_ESDHC");
> +#endif

Why not just rename it to FSL_SDHC and be done with it ?

>       regs = (struct fsl_esdhc *)cfg->esdhc_base;
> 
>       /* First reset the eSDHC controller */
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot