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@samsung.com Cc: Marek Vasut marex@denx.de Cc: Pantelis Antoniou panto@antoniou-consulting.com Cc: Tom Rini trini@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