diff mbox

[U-Boot] mmc: dw_mmc: Increase timeout to 4 minutes (as in Linux kernel)

Message ID 1443198325-22460-1-git-send-email-l.majewski@samsung.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Łukasz Majewski Sept. 25, 2015, 4:25 p.m. UTC
The commit: d9dbb97be0e4a550457aec5f11afefb446169c90
"mmc: dw_mmc: Zap endless timeout" removed endless loop waiting for end
of dw mmc transfer.

For some workloads - dfu test @ Odroid XU3 (sending 8MiB file) -
and SD cards (e.g. MicroSD Kingston 4GiB, Adata 4GiB)
the default timeout is to short.

The new value - 4 minutes (240 seconds) - is the same as the one used in
Linux kernel driver. Such fix should be good enough until we come up
with better fix for this issue.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@konsulko.com>
---
 drivers/mmc/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Przemyslaw Marczak Sept. 28, 2015, 1:43 p.m. UTC | #1
Hi Lukasz,

On 09/25/2015 06:25 PM, Lukasz Majewski wrote:
> The commit: d9dbb97be0e4a550457aec5f11afefb446169c90
> "mmc: dw_mmc: Zap endless timeout" removed endless loop waiting for end
> of dw mmc transfer.
>
> For some workloads - dfu test @ Odroid XU3 (sending 8MiB file) -
> and SD cards (e.g. MicroSD Kingston 4GiB, Adata 4GiB)
> the default timeout is to short.
>
> The new value - 4 minutes (240 seconds) - is the same as the one used in
> Linux kernel driver. Such fix should be good enough until we come up
> with better fix for this issue.
>

I think, that you should adjust the commit message, because it
describes the result of the real problem which is in the background.

It doesn't fix the DFU, because it wasn't broken.
What is the real problem and how does this commit fix it?

> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
>   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 a84c1e1..26d34ae 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -214,7 +214,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>
>   	if (data) {
>   		start = get_timer(0);
> -		timeout = 1000;
> +		timeout = 240000;
>   		for (;;) {
>   			mask = dwmci_readl(host, DWMCI_RINTSTS);
>   			/* Error during data transfer. */
>

I tested this patch on Odroid XU3 and Odroid U3, with SD and eMMC cards.

For read/write data of size 1800MB, it doesn't break the operation.
Read takes about one minute for some eMMC card, and the write takes 
about 2 and a half minute.

The normal read/write operations are done partially for big size data, 
so it should be good, which shows test with write 1800MB to SD card.

Writing 1800MB of data takes about 8 minutes on some SD card and then I 
can see only the info that something was started - but I don't know if 
it's going on, or the board is dead... So maybe we should get some info 
from this part of code, that the transfer is going on, e.g. after each 
next 10% of data portion?

Tested-by: Przemyslaw Marczak <p.marczak@samsung.com>

Best regards,
Tom Rini Sept. 28, 2015, 9:08 p.m. UTC | #2
On Fri, Sep 25, 2015 at 06:25:25PM +0200, Łukasz Majewski wrote:

> The commit: d9dbb97be0e4a550457aec5f11afefb446169c90
> "mmc: dw_mmc: Zap endless timeout" removed endless loop waiting for end
> of dw mmc transfer.
> 
> For some workloads - dfu test @ Odroid XU3 (sending 8MiB file) -
> and SD cards (e.g. MicroSD Kingston 4GiB, Adata 4GiB)
> the default timeout is to short.
> 
> The new value - 4 minutes (240 seconds) - is the same as the one used in
> Linux kernel driver. Such fix should be good enough until we come up
> with better fix for this issue.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> Tested-by: Przemyslaw Marczak <p.marczak@samsung.com>

Applied to u-boot/master, thanks!
Tom Rini Sept. 28, 2015, 9:08 p.m. UTC | #3
On Mon, Sep 28, 2015 at 03:43:50PM +0200, Przemyslaw Marczak wrote:
> Hi Lukasz,
> 
> On 09/25/2015 06:25 PM, Lukasz Majewski wrote:
> >The commit: d9dbb97be0e4a550457aec5f11afefb446169c90
> >"mmc: dw_mmc: Zap endless timeout" removed endless loop waiting for end
> >of dw mmc transfer.
> >
> >For some workloads - dfu test @ Odroid XU3 (sending 8MiB file) -
> >and SD cards (e.g. MicroSD Kingston 4GiB, Adata 4GiB)
> >the default timeout is to short.
> >
> >The new value - 4 minutes (240 seconds) - is the same as the one used in
> >Linux kernel driver. Such fix should be good enough until we come up
> >with better fix for this issue.
> >
> 
> I think, that you should adjust the commit message, because it
> describes the result of the real problem which is in the background.
> 
> It doesn't fix the DFU, because it wasn't broken.
> What is the real problem and how does this commit fix it?

I think this message is good enough (esp since I want to apply it now)
and will link back to all of these other messages about it for details.
diff mbox

Patch

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index a84c1e1..26d34ae 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -214,7 +214,7 @@  static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	if (data) {
 		start = get_timer(0);
-		timeout = 1000;
+		timeout = 240000;
 		for (;;) {
 			mask = dwmci_readl(host, DWMCI_RINTSTS);
 			/* Error during data transfer. */