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

[Marek Vasut]

> On Sun, Nov 13, 2011 at 12:35 AM, Marek Vasut <marek.vasut at gmail.com> wrote:
> >> 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.hui at 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 ?
> 
> Yes, I think we can. I will drop it later.

Please drop now so it's not forgotten ;-)

> 
> >>       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?
> 
> This is due to that on i.mx6q usdhc, it adds one extra mixctrl(offset
> 0x48) register to accommodate the
> low 16-bits of tranfertype and the high 16-bits of tranfertype will
> still go via the xfertyp register.
> On the i.mx6q usdhc, the xfertyp(offset 0xc) register, the low 16-bits
> is not used and only
> high 16-bits have been used. I can make the above code clear as the
> followings,
> 
> #if defined(CONFIG_FSL_USDHC)
> esdhc_write32(&regs->mixctrl, xfertyp & 0x0000FFFF);
> esdhc_write32(&regs->xfertyp, xfertyp & 0xFFFF0000);
> #else
> esdhc_write32(&regs->xfertyp, xfertyp);
> #endif

Well this is insane, is the datasheet for mx6q available already so I can check 
for myself?

> 
> >>       /* 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 ?
> 
> OK, I will do it.
> 
> >>       regs = (struct fsl_esdhc *)cfg->esdhc_base;
> >> 
> >>       /* First reset the eSDHC controller */
> 
> Jason Liu