Hello,
On 09/18/2015 09:32 AM, 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:
1. Try to revert changes (which remove endless loop)
2. 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.
I recommend the second option, however please note that SD card doesn't
provide any information about the delay required to finish the command,
beside some informations about block erase operation time.
The block erase time info, probably depends on vendor implementation, so
it would be better to use as long time for delay as possible, and just
rely on the controller error status info instead of breaking the
operation after too short timeout.
In such case I think, that it would be nice if user could be notified
with some time interval, that the card is busy.
In other words, when he's waiting the second minute for the transfer
end, then how does he know if the board doesn't hang?
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
