On Fri, Sep 18, 2015 at 09:32:47AM +0200, Lukasz Majewski wrote:
Hi Tom,
On Thu, Sep 17, 2015 at 04:43:33PM +0200, Lukasz Majewski wrote:
Hi Tom,
On Monday, September 14, 2015 at 01:22:20 PM, Lukasz Majewski wrote:
Hi Alexey,
Hi Marek, Lukasz,
On Sun, 2015-09-13 at 16:00 +0200, Marek Vasut wrote: > On Sunday, September 13, 2015 at 12:03:18 PM, Lukasz > Majewski wrote: > > Hi Marek, > > Hi, > > [...] > > > > > > 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. > > > > I'm just wondering here - why the indefinite loop was > > working previously? Was anybody complaining (on the ML) > > about the problem of removing the SD card when some > > operation is ongoing? > > It worked for me for all the workloads I used. Noone was > complaining.
The same story here - previous code with infinite loop was working for my boards. And now I do see a problem with pretty simple scenario that we do use in our products.
> > The problem with potential removal of SD card (after > > booting the board) is with us for quite long time. Even > > with indefinite loop (without your patch) we also could > > "hang" the board if the SD card was removed during a > > transfer. > > Which is why we should weed out the unbounded loops. > > > > 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. > > > > We have agreed to not agree :-) > > Yes :-)
The first thing I care is working U-Boot v2015.10 out of the box on my boards. And so I may agree on any temporary solution. I see it as timeout value either being infinite or obviously very high like 60 seconds.
60 seconds might sound stupid but my thought behind this is to make sure even long transfers succeed. Imagine 100 Mb rootfs or update file downloaded from slow SD-card.
Transfer of rootfs to SD-card (downloaded to memory via tftp) is definitely valid scenario.
> > > > > 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 ... > > > > Yes, this is a bug. We had similar problem with Samsung's > > SDHCI, before we switched to dw_mmc. This issue is new at > > dw_mmc. > > > > > > > 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 > > > > IMHO revering the change (before the release) is from the > > software development point of view better solution than > > adding some heuristic delta to timeout. > > > > > for the sake of getting some sort of crappy hardware > > > operational. > > > > Unfortunately this "crappy hardware" is pervasive and we > > cannot do anything about it. > > > > To sum up (my point of view): > > 1. The best would be to revert the patch - but if simple > > "git revert" is not working then,
Well even if clean revert won't work we may do manual tweaks so that functionally it is "revert". If of any interest I may come up with that sort of patch.
> > 2. We should increase the timeout (with my patch) for > > v2015.10 release
If everybody is OK with that let's go do it. Because release is around the corner and I don't want to explain each and every user how to fix their new problem.
v2015.10 correctness is crucial here.
Yes.
> Let's do this for the sake of crappy cards. > > > 3. After release we can devise some kind of solution > > 4. It is a good topic for U-boot's minisummit discussion > > at Dublin > > > > Marek, Alexey, Tom, Pantelis what do you think? > > I think yes.
What's important we need to make sure Tom is aware of this situation and he won't cut a release until our fix is in place and all involved parties reported their happiness.
I also think that Tom should speak up on this issue.
Tom, could you give your opinion on this issue?
Well, is there a concensus patch now?
There isn't a consensus patch.
There are two options:
Try to revert changes (which remove endless loop)
Increase the delay to e.g. 4 minutes (as it is done in Linux) before
v2015.10 and provide correct solution (based on internal SD card information) after the release.
OK, lets go with #2.