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
