All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Walmsley <paul@pwsan.com>
To: Chris Ball <cjb@laptop.org>
Cc: Chase Maupin <Chase.Maupin@ti.com>,
	linux-omap@vger.kernel.org, linux-mmc@vger.kernel.org,
	sakoman@gmail.com
Subject: Re: [PATCH] mmc: omap_hsmmc: set dto to 14 for all devices
Date: Mon, 12 Mar 2012 04:58:00 -0600 (MDT)	[thread overview]
Message-ID: <alpine.DEB.2.00.1203120435200.11332@utopia.booyaka.com> (raw)
In-Reply-To: <m2399iqtgf.fsf@bob.laptop.org>

+ Steve Sakoman

Hi Chris, Chase,

Sorry for the late comment on this; I just noticed this thread.

On Thu, 8 Mar 2012, Chris Ball wrote:

> On Thu, Mar 01 2012, Chase Maupin wrote:
> > * With certain SD cards timeouts like the following have been seen
> >   due to an improper calculation of the dto value:
> >     mmcblk0: error -110 transferring data, sector 4126233, nr 8,
> >     card status 0xc00
> > * By removing the dto calculation and setting the timeout value
> >   to the maximum specified by the SD card specification part A2
> >   section 2.2.15 these timeouts can be avoided.
> > * This change has been used by beagleboard users as well as the
> >   Texas Instruments SDK without a negative impact.
> > * There are multiple discussion threads about this but the most
> >   relevant ones are:
> >     * http://talk.maemo.org/showthread.php?p=1000707#post1000707
> >     * http://www.mail-archive.com/linux-omap@vger.kernel.org/msg42213.html
> > * Original proposal for this fix was done by Sukumar Ghoral of
> >   Texas Instruments
> >
> > * Tested using a Texas Instruments AM335x EVM
> >
> > Signed-off-by: Chase Maupin <Chase.Maupin@ti.com>
> 
> Thanks, I've pushed this to mmc-next for 3.4.  (With a trivial
> indentation fix, below.)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 82b400793..9d4ce1c 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1360,7 +1360,7 @@ static void set_data_timeout(struct omap_hsmmc_host *host)
>  	if (clkd == 0)
>  		clkd = 1;
>  
> -    /* Use the maximum timeout value allowed in the standard of 14 or 0xE */
> +	/* Use the maximum timeout value allowed in the standard of 14 or 0xE */
>  	dto = 14;
>  
>  	reg &= ~DTO_MASK;

I don't think this is the right fix.  Steve Sakoman and I discussed this a 
few months ago -- we did some debugging and Steve observed the timeouts on 
multiple block writes (0x19):

hsmmc: command 00000019
hsmmc: orig_rate = 96000000, div = 2, dto = 11
hsmmc: using 300000000 ns DTO
hsmmc: command 0000000d
mmcblk0: error -110 transferring data, sector 15257840, nr 96, card status 0xc00

So dto = 11 @ 48MHz is a 350ms timeout ((2^(11+13)) / 48000000).

I don't have an SD bus analyzer, but it looks to me think there are some 
SD cards that are not completing the write within the allotted time 
window.  It should be 250ms according to this SD spec:

https://www.sdcard.org/developers/tech/pls/simplified_specs/Part_1_Physical_Layer_Simplified_Specification_Ver_3.01_Final_100518.pdf

The spec also says "It is strongly recommended for hosts to implement more 
than 500ms timeout value even if the card indicates the 250ms maximum busy 
length."

So to me, this should probably be solved in the generic Linux MMC layer.  
mmc_set_data_timeout() should presumably be using a huge timeout on 
writes.  Don't know what it should be but the current 300ms goal is 
arbitrary anyway - see commit 493890e75d98810a3470b4aae23be628ee5e9667 
("mmc: increase SD write timeout for crappy cards").

Otherwise, if we just change the OMAP host controller driver, we could run 
into the same problem on other controllers.  While this issue could be an 
OMAP specific problem, at the moment I don't see any obvious evidence of 
that.

So perhaps something like the following patch instead?  Unfortunately, I 
don't have one of these crappy SD cards as far as I know, so I can't 
really test it.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 12 Mar 2012 04:46:29 -0600
Subject: [PATCH] mmc: use really long write timeout to deal with crappy cards

Several people have noticed that crappy SD cards take much longer to
complete multiple block writes than the 300ms that Linux specifies.
Try to work around this by using a three second write timeout instead.

<insert Chase's patch description here>

Not-yet-signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 drivers/mmc/core/core.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 690255c..0f2caca 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -526,10 +526,14 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 
 		if (data->flags & MMC_DATA_WRITE)
 			/*
-			 * The limit is really 250 ms, but that is
-			 * insufficient for some crappy cards.
+			 * The MMC spec "It is strongly recommended
+			 * for hosts to implement more than 500ms
+			 * timeout value even if the card indicates
+			 * the 250ms maximum busy length."  Even the
+			 * previous value of 300ms is known to be
+			 * insufficient for some cards.
 			 */
-			limit_us = 300000;
+			limit_us = 3000000;
 		else
 			limit_us = 100000;
 
-- 
1.7.9.1


  parent reply	other threads:[~2012-03-12 10:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-01 17:26 [PATCH] mmc: omap_hsmmc: set dto to 14 for all devices Chase Maupin
2012-03-06 12:54 ` Maupin, Chase
2012-03-09  4:38 ` Chris Ball
2012-03-09 13:06   ` Maupin, Chase
2012-03-12 10:58   ` Paul Walmsley [this message]
2012-03-12 11:10     ` Paul Walmsley
2012-03-12 14:37     ` Maupin, Chase
2012-03-16  3:22     ` Chris Ball
2012-04-05 22:08       ` Tony Lindgren
2012-04-05 22:15         ` Chris Ball
2012-04-05 23:44           ` Paul Walmsley
2012-04-05 23:54             ` Chris Ball
2012-04-12  9:46               ` Tushar Behera
2012-04-12 17:56                 ` Paul Walmsley
2012-04-12 18:19                   ` Chris Ball
2012-04-14  9:38                     ` Paul Walmsley

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=alpine.DEB.2.00.1203120435200.11332@utopia.booyaka.com \
    --to=paul@pwsan.com \
    --cc=Chase.Maupin@ti.com \
    --cc=cjb@laptop.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=sakoman@gmail.com \
    /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.