Hi Marek, On Mon, 2015-07-27 at 22:39 +0200, Marek Vasut wrote: > Endless timeouts are bad, since if we get stuck in one, we have no > way out. Zap this one by implementing proper timeout. > > Signed-off-by: Marek Vasut <ma...@denx.de> > Cc: Dinh Nguyen <dingu...@opensource.altera.com> > Cc: Pantelis Antoniou <pa...@antoniou-consulting.com> > Cc: Tom Rini <tr...@konsulko.com> > --- > drivers/mmc/dw_mmc.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c > index 3fffa71..0f61f16 100644 > --- a/drivers/mmc/dw_mmc.c > +++ b/drivers/mmc/dw_mmc.c > @@ -211,14 +211,29 @@ static int dwmci_send_cmd(struct mmc *mmc, struct > mmc_cmd *cmd, > } > > if (data) { > - do { > + start = get_timer(0); > + timeout = 1000; > + for (;;) { > mask = dwmci_readl(host, DWMCI_RINTSTS); > + /* Error during data transfer. */ > if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) { > printf("%s: DATA ERROR!\n", __func__); > bounce_buffer_stop(&bbstate); > return -1; > } > - } while (!(mask & DWMCI_INTMSK_DTO)); > + > + /* Data arrived correctly. */ > + if (mask & DWMCI_INTMSK_DTO) > + break; > + > + /* Check for timeout. */ > + if (get_timer(start) > timeout) { > + printf("%s: Timeout waiting for data!\n", > + __func__); > + bounce_buffer_stop(&bbstate); > + return TIMEOUT; > + } > + } > > dwmci_writel(host, DWMCI_RINTSTS, mask); >
It turned out that patch breaks functionality in some cases. For me on every attempt to download something significant (at least I see it on 5/7 Mb files) from SD I'm seeing timeout firing too early. I added a bit of extra instrumentation to see where time is spent and why. So my diff is: ----------------------------------->8-------------------------------- diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 77b87e0..2da77a7 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -213,7 +213,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, if (data) { start = get_timer(0); - timeout = 1000; + timeout = 10000; // That's required to get to the end of the transfer for (;;) { mask = dwmci_readl(host, DWMCI_RINTSTS); /* Error during data transfer. */ @@ -226,6 +226,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, /* Data arrived correctly. */ if (mask & DWMCI_INTMSK_DTO) { ret = 0; + printf(" * time spent: %d, data size: %d, blocks: %d\n", (int)get_timer(start), data ->blocksize * data->blocks, data->blocks); break; } ----------------------------------->8-------------------------------- And that's what I see then: ----------------------------------->8-------------------------------- AXS# fatload mmc 0 * time spent: 0, data size: 8, blocks: 1 * time spent: 0, data size: 512, blocks: 1 * time spent: 0, data size: 512, blocks: 1 * time spent: 0, data size: 512, blocks: 1 reading uImage * time spent: 1, data size: 512, blocks: 1 * time spent: 0, data size: 1024, blocks: 2 * time spent: 1, data size: 3072, blocks: 6 * time spent: 1, data size: 3072, blocks: 6 * time spent: 1, data size: 3072, blocks: 6 * time spent: 0, data size: 3072, blocks: 6 * time spent: 0, data size: 3072, blocks: 6 * time spent: 1599, data size: 13338112, blocks: 26051 * time spent: 0, data size: 512, blocks: 1 13338188 bytes read in 1651 ms (7.7 MiB/s) ----------------------------------->8-------------------------------- So you see real data transfer takes ~1.7 seconds when getting 26k blocks. In other words timeout check has to be a bit smarter, for example taking into account number of blocks to be transferred. Any thoughts? -Alexey _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot