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 marex@denx.de Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Pantelis Antoniou panto@antoniou-consulting.com Cc: Tom Rini trini@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