From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Sat, 12 Sep 2015 18:13:25 +0200 Subject: [U-Boot] [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds In-Reply-To: <20150911234528.6ba30dbe@jawa> References: <1440769821-24005-1-git-send-email-l.majewski@samsung.com> <1441991999.10291.15.camel@synopsys.com> <20150911234528.6ba30dbe@jawa> Message-ID: <201509121813.25397.marex@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > > Cc: Marek Vasut > > > > > Cc: Pantelis Antoniou > > > > > Cc: Tom Rini > > > > > > > > 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