All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds
Date: Sun, 13 Sep 2015 16:00:21 +0200	[thread overview]
Message-ID: <201509131600.21256.marex@denx.de> (raw)
In-Reply-To: <20150913120318.2d5e2e5d@jawa>

On Sunday, September 13, 2015 at 12:03:18 PM, Lukasz Majewski wrote:
> Hi Marek,

Hi,

[...]

> > > > 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 ?
> 
> This is only my personal guess (when taking account my previous work
> done with dw_mmc on Exynos HW)
> - I think Pantelis would have more knowledge to elaborate here.

OK

> > > > 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 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 :-)

> > > > 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,
> 2. We should increase the timeout (with my patch) for v2015.10 release

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.

  reply	other threads:[~2015-09-13 14:00 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-28 13:50 [U-Boot] [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds Lukasz Majewski
2015-08-28 13:50 ` [U-Boot] [PATCH 2/2] mmc: dw_mmc: Make timeout error visible to u-boot console Lukasz Majewski
2015-08-28 23:21   ` Simon Glass
2015-08-29 12:09     ` Lukasz Majewski
2015-08-29 15:07       ` Simon Glass
2015-09-03 12:33         ` Lukasz Majewski
2015-09-03 12:21   ` [U-Boot] [PATCH] FIX: fat: Provide correct return code from disk_{read|write} to upper layers Lukasz Majewski
2015-09-03 12:44     ` Tom Rini
2015-09-03 13:40       ` Lukasz Majewski
2015-09-03 14:18         ` Lukasz Majewski
2015-09-23  3:17           ` Stephen Warren
2015-09-23  8:40             ` Lukasz Majewski
2015-09-25  5:47               ` Stephen Warren
2015-09-09  7:02     ` Lukasz Majewski
2015-09-17 14:44       ` Lukasz Majewski
2015-09-12 12:51     ` [U-Boot] " Tom Rini
2015-08-28 21:55 ` [U-Boot] [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds Marek Vasut
2015-08-29 11:55   ` Lukasz Majewski
2015-08-29 13:52     ` Marek Vasut
2015-08-29 16:38       ` Lukasz Majewski
2015-08-29 19:19         ` Marek Vasut
2015-09-01 11:19           ` Lukasz Majewski
2015-09-01 11:33             ` Marek Vasut
2015-09-01 15:25               ` Lukasz Majewski
2015-09-01 15:35                 ` Marek Vasut
2015-09-01 16:22           ` Pantelis Antoniou
2015-09-02  8:06             ` Marek Vasut
2015-09-09  7:01 ` Lukasz Majewski
2015-09-09 11:34   ` Marek Vasut
2015-09-11 17:20     ` Alexey Brodkin
2015-09-11 21:45       ` Lukasz Majewski
2015-09-12 16:13         ` Marek Vasut
2015-09-13 10:03           ` Lukasz Majewski
2015-09-13 14:00             ` Marek Vasut [this message]
2015-09-14 10:15               ` Alexey Brodkin
2015-09-14 11:22                 ` Lukasz Majewski
2015-09-14 13:36                   ` Marek Vasut
2015-09-17 14:43                     ` Lukasz Majewski
2015-09-18  0:31                       ` Tom Rini
2015-09-18  7:32                         ` Lukasz Majewski
2015-09-18  8:07                           ` Przemyslaw Marczak
2015-09-18 19:27                           ` Tom Rini
2015-09-21 15:32                             ` Pantelis Antoniou
2015-09-14 10:30         ` Alexey Brodkin
2015-09-14 11:15           ` Przemyslaw Marczak
2015-09-14 10:33   ` Przemyslaw Marczak
2015-09-25 16:25 ` [U-Boot] [PATCH] mmc: dw_mmc: Increase timeout to 4 minutes (as in Linux kernel) Lukasz Majewski
2015-09-28 13:43   ` Przemyslaw Marczak
2015-09-28 21:08     ` Tom Rini
2015-09-28 21:08   ` [U-Boot] " Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201509131600.21256.marex@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.