On Wed, 4 Jul 2018, Alberto Panizzo wrote:

Errors are reported to happen running U-Boot after SPL kernel load failure.
In this case mmc host is not clean, and card enumeration timeouts
do happen frequently.

Signed-off-by: Alberto Panizzo <albe...@amarulasolutions.com>

See below for requested changes.

---
drivers/mmc/dw_mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0126563..b6890d3 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -199,7 +199,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd 
*cmd,
                        (1 + (data ? DIV_ROUND_UP(data->blocks, 8) : 0)));
        int ret = 0, flags = 0, i;
        unsigned int timeout = 500;
-       u32 retry = 100000;
+       u32 retry = 1000000;

I feel rather uneasy about this, especially as it affects all devices (not just limited to Rockchip) that include a Designware MMC controller.

This affects code that effectivly 'retries' reading the status register to wait for a DONE bit:
        for (i = 0; i < retry; i++) {
                status = readl(...);
                if (status & BITMASK) {
                        writel(...);
                        break;  // done
                }
        }

        if (i == retry)
                // a timeout happened

There's two main issues I have:
(a) the number of retries is an implicit timeout (i.e. depending on the
    CPU frequency, instruction-set, whether the i-cache is enabled, ...) a
    different number of microseconds will expire per pass through the loop
(b) you commit message states (I am paraphrasing here) 'because the
    BootROM has left the state dirty', we need to increase the timeout

So there's two things (addressing these two issues) I'd like to see:
(a) the use of a polling loop that uses a timer where possible (I don't
    know whether there's an execution context in someone's SPL or TPL that
    doesn't have a timebase running, though... so: caveat emptor) --- this
    might even lend itself to the use of include/linux/iopoll.h macros.
(b) a clean reset of the MMC controller for your/our platform in case we
    expect the BootROM having left things in disarray.

        u32 mask, ctrl;
        ulong start = get_timer(0);
        struct bounce_buffer bbstate;

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to