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

[Marek Vasut]

On Friday, September 11, 2015 at 11:45:28 PM, Lukasz Majewski wrote:
> Hi Alexey,

Hi,

> > Hi Marek, Lukasz,
> > 
> > > On Wednesday, September 09, 2015 at 09:01:30 AM, Lukasz Majewski
> > > 
> > > wrote:
> > > > Hi,
> > > > 
> > > > > 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.
> > > > > 
> > > > > Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> > > > > Cc: Marek Vasut <marex at denx.de>
> > > > > Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
> > > > > Cc: Tom Rini <trini at konsulko.com>
> > > > 
> > > > Are there any more questions regarding this patch or is it ready
> > > > for submission as fix for v2015.10?
> > > 
> > > No comments, just apply this.
> > > 
> > > But this should really be fixed properly in the next MW.
> > > 
> > > Best regards,
> > > Marek Vasut
> > 
> > FWIW I faced similar problem even reading data.
> > At least on one of my boards reading of ~8Mb file
> > took ~1.7 seconds and so 1 second timeout was interrupting data
> > exchange.
> 
> Was it SD card or eMMC device?
> 
> > So indeed we need to have some dirty hack for upcoming release
> > like bumping timeout to something really huge but later we
> > need to fix that problem properly.
> > 
> > I though proper solution would be to set timeout depending of amount
> > of data to be exchanged... something like take slowest speed of SD/MMC
> > that is allowed by spec and calculate delay based on how much time it
> > might take for that slow device and for safety multiply it by say 2.
> 
> As fair as I remember, card provide this kind of information. We can
> try to investigate this possibility.

Please do.

> > Now from this thread I see that there're other reasons that might
> > affect length of at least write operation. In other words it could be
> > complicated unfortunately.
> 
> My gut feeling is that proper handling of eMMC would require quite a
> fair mmc subsystem rework.

Can you please elaborate ?

> > Still we need to fix regression first with virtually infinite
> > timeout :) I would even thing that simple revert of Marek's patch may
> > make sense for now.
> 
> +1 - unfortunately there were some other patches applied to this
> particular patch. Simple revert might be a bit tricky here.

-1 - In case the card gets removed during the DMA transfer and the
board doesn't have a watchdog, it will get stuck indefinitelly. We
absolutelly don't want this sort of behavior in U-Boot. I understand
that this is the easiest way for everyone to achieve some sort of
"working" solution, but it is definitelly not the correct one. While
I do agree to increasing the timeout, I do not agree to unbounded
loops, sorry.

> > From both points of view for keeping history
> > clean (compared to stacked fixes/workarounds) and from removal of
> > regression root cause.
> 
> As I said before - +1 from me.

As I said before, -1 from me. Btw. did anything regress in here? To me,
this seems like a newly discovered bug ...

> > It's not that I like to have infinite loops but given previous
> > implementation worked fine for people in the previous U-Boot release.
> 
> Good justification

It is never a justified to return to a potentially problematic version
for the sake of getting some sort of crappy hardware operational.

Best regards,
Marek Vasut