[U-Boot] [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds

[Marek Vasut]

On Saturday, August 29, 2015 at 01:55:36 PM, Lukasz Majewski wrote:
> On Fri, 28 Aug 2015 23:55:17 +0200

Hi!

> Marek Vasut <marex at denx.de> wrote:
> > On Friday, August 28, 2015 at 03:50:20 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 - 20 seconds - takes into account the situation when
> > > SD card triggers internal clean up. Such process may take more than
> > > 10 seconds on some cards.
> > 
> > What happens if you pull the SD card out of the slot during such a
> > process?
> 
> Then we would wait 20 seconds :-) to proceed.

Oops, I think I was not clear here. I was wondering what happens to the
card if you yank it out of the slot whole it's performing it's internal
cleanup or whatever. Is it possible that the card suffers data corruption,
effectively trashing user data ? Is this behavior specific to Samsung SD
cards ?

> To be clear - the mentioned patch introduced regression.

That's a bug, not a regression, but anyway, that's not the point. I do
agree with you that we do have a problem and I'm inclined to Ack this
patch, I'd like to understand what the real implications of such a behavior
of these cards are.

> It works for
> small files on a commonly available SD cards (like 4 GiB
> Kingston/Adata).
> 
> When I ran DFU tests I've discovered that there is a problem with
> storing 8MiB file (dat_8M.img).
> 
> Even worse - when one wants to store Image.itb file (which might be 4-6
> MiB) it sometimes works and sometimes not. Nightmare for debugging.

Ew, that's one crappy card you have there. I'm reading the SD card
"Physical Layer Simplified Specification Version 4.10" (part1_410.pdf)
section 4.6.2.2 and it states that for SDHC cards, the write operation
should take at most 250mS, for SDXC it's 500mS. Could it be that your
card is violating the spec ?

> Please correct me if I'm wrong - but is seems like we are now using 1
> second timeout for detection if SD card has been removed?
> 
> Shouldn't we use polling on the card detect IO pin instead? We could add
> such polling in several places in the MMC subsystem (like we do it
> with watchdog).
> 
> Marek, Pantelis, what do you think about this?

If you implement board_mmc_getcd(), you can check if the card is present
this way instead of waiting for command to time out. The infrastructure
for that is already in place. Right ?

It'd be cool if the MMC subsystem could pull the wp-gpios and cd-gpios
from DT though :)

> > Also, where did you find out there is such "cleanup" mechanism
> > please ?
> 
> Internally we did some tests with several SD cards. We were stunned when
> it turned out that for some workloads it took up to 15 seconds to
> end write operation for small data.
> 
> The culprit is the SD Card embedded controller responsible for FTL -
> flash translation layer.
> It allows NAND memory on the card to be visible as the block device.
> More importantly it also takes care of wear leveling and bad block
> management.
> 
> Hence, we don't know when it would start housekeeping operations.
> We can only poll/wait until this controller finishes it work.
> The code as it was (with the indefinite loop) was taking this situation
> into account.
> 
> The 1 second timeout is apparently too short and makes using SD card
> non-deterministic and error prone in u-boot.
> 
> Even worse, many devices use SD card as the only storage device.

Yes, horrible.

Best regards,
Marek Vasut