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

[Jason Hui]

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.

>
>>       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

>
>>
>>       /* 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