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/i...
and the third
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/drivers/mmc/i...
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/i...
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(®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 ?
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/i...
Ah, I see, I was confused with the ENG numbers...
Best regards, Stefano Babic