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

[Stefano Babic <sbabic@denx.de>]

On 17/03/2012 10:33, Dirk Behme wrote:
> Hmm, I'm unsure about this. Looking at the 3 referenced Freescale
> patches and at the way the Freescale commits look, I was under the
> impression that the first patch was improved by Freescale two times to
> the then 'final' state.
> 
> At least the first patch
> 
> 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
> 
> 
> and the third
> 
> 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
> 
> 
> definitely should be one patch. The third one wouldn't be there if there
> would have been a proper review of the first one ;)
> 
> And the commit message of the second patch
> 
> 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
> 
> 
> mentions that the delay can be removed due to the first patch. So I was
> under the impression that this could have been done in the first patch,
> already, too.
> 
> But anyway, as you said, this shouldn't be a blocker.

No, it is not. I am fine with your explanation.

>>> +    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 ?
> 
> I have to admit that I have no idea :(
> 
> I'm no SD/MMC expert. This patch was created doing a functional diff. On
> a custom board we found that the imx_esdhc.c driver in Freescale's old
> U-Boot works fine, while the mainline fsl_esdhc.c we talk about here
> doesn't. Then, we traced it down to the changes we talk about here and
> ported them to the mainline fsl_esdhc.c. This fixes our issue.
> Unfortunately, there is no real understanding what these changes do.

At least the comment seems wrong : code is enabling cloks in the sysctl
register. You should change or drop the comment.

> 
> Could any SD expert help with this?
>> This ENGcm03648 is not mentioned in the commit message, it seems another
>> issue...
> 
> Hmm? The subject is about "errata ENGcm03648" And the commit messages
> refers to
> 
> ...
> 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

Ah, I see, I was confused with the ENG numbers...

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