[PATCH v2] mmc: lower init clock frequency to 300kHz

[PATCH v2] mmc: lower init clock frequency to 300kHz

[Ben Nizette]

A good few months ago, commit

commit 8dfd0374be84793360db7fff2e635d2cd3bbcb21
Author: Sascha Hauer <s.hauer@pengutronix.de>
Date:   Thu Apr 9 08:32:02 2009 +0200

    MMC core: limit minimum initialization frequency to 400kHz

broke a few setups with cards which don't quite adhere to the MMC spec - 400kHz is just too fast for them.  In my testing, all of the cards which fail at 400kHz are OK by about 350Khz but this patch drops the floor to 300 to be on the safe side.

Dropping the floor also means that the warning will trigger on some valid setups, albeit ones which won't run particularly crappy cards.  This patch then slightly softens the language of said warning to make it clear it isn't always a problem.

V1 of this patch dropped the floor all the way to 50kHz on the basis that it was only for a few 100 bytes so that very low speed shouldn't matter.  After some discussion [1] the consensus was that 50 was too slow after all so this patch is a bit more sensible.

Signed-off-by: Ben Nizette <bn@niasdigital.com>

[1] http://lkml.org/lkml/2009/7/1/529

---
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7dab2e5..49f0eae 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -891,12 +891,12 @@ static void mmc_power_up(struct mmc_host *host)
 	 */
 	mmc_delay(10);
 
-	if (host->f_min > 400000) {
-		pr_warning("%s: Minimum clock frequency too high for "
+	if (host->f_min > 300000) {
+		pr_warning("%s: Minimum clock frequency may be too high for "
 				"identification mode\n", mmc_hostname(host));
 		host->ios.clock = host->f_min;
 	} else
-		host->ios.clock = 400000;
+		host->ios.clock = 300000;
 
 	host->ios.power_mode = MMC_POWER_ON;
 	mmc_set_ios(host);

Re: [PATCH v2] mmc: lower init clock frequency to 300kHz

[Pavel Machek]

On Sat 2010-01-02 10:11:10, Ben Nizette wrote:
> 
> A good few months ago, commit
> 
> commit 8dfd0374be84793360db7fff2e635d2cd3bbcb21
> Author: Sascha Hauer <s.hauer@pengutronix.de>
> Date:   Thu Apr 9 08:32:02 2009 +0200
> 
>     MMC core: limit minimum initialization frequency to 400kHz
> 
> broke a few setups with cards which don't quite adhere to the MMC spec - 400kHz is just too fast for them.  In my testing, all of the cards which fail at 400kHz are OK by about 350Khz but this patch drops the floor to 300 to be on the safe side.
> 
> Dropping the floor also means that the warning will trigger on some valid setups, albeit ones which won't run particularly crappy cards.  This patch then slightly softens the language of said warning to make it clear it isn't always a problem.
> 
> V1 of this patch dropped the floor all the way to 50kHz on the basis that it was only for a few 100 bytes so that very low speed shouldn't matter.  After some discussion [1] the consensus was that 50 was too slow after all so this patch is a bit more sensible.
> 

(please wrap).

> @@ -891,12 +891,12 @@ static void mmc_power_up(struct mmc_host *host)
>  	 */
>  	mmc_delay(10);
>  
> -	if (host->f_min > 400000) {
> -		pr_warning("%s: Minimum clock frequency too high for "
> +	if (host->f_min > 300000) {
> +		pr_warning("%s: Minimum clock frequency may be too high for "
>  				"identification mode\n", mmc_hostname(host));
>  		host->ios.clock = host->f_min;
>  	} else
> -		host->ios.clock = 400000;
> +		host->ios.clock = 300000;
>  
>  	host->ios.power_mode = MMC_POWER_ON;
>  	mmc_set_ios(host);

Machine with minimum clock of 1MHz is clearly broken, yet you issue
"soft" warning.

What about:

     if (f_min > 400k)
     	print existing warning
     else if (f_min > 300k)
        print warning 'if your card does not work, its broken,  but
	      	           	your host is unhelpful'
     clock = f_min
     if (clock < 3o0)
     	 clock = 300

?
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v2] mmc: lower init clock frequency to 300kHz

[Hein_Tibosch]

On 6-1-2010 15:01, Pavel Machek wrote:
> Machine with minimum clock of 1MHz is clearly broken, yet you issue
> "soft" warning.
>
> What about:
>
>      if (f_min > 400k)
>      	print existing warning
>      else if (f_min > 300k)
>         print warning 'if your card does not work, its broken,  but
> 	      	           	your host is unhelpful'
>      clock = f_min
>      if (clock < 3o0)
>      	 clock = 300
>
> ?
>   
Later in this thread I mentioned that initialization at 300 Khz is
too fast for some platforms/cards (avr32 AP7000/main-stream cards).
They will report CRC-errors so the above won't work.

I have looked at the time it will take to exchange the initial data (about
85 bytes), using different busses and frequencies:
	
        Intface:  8-bits  4-bits  SPI
Bus-freq:
400000 (Hz)       0.213   0.425    1.700 (ms)
300000            0.283   0.567    2.267
200000            0.425   0.850    3.400
100000            0.850   1.700    6.800
50000             1.700   3.400   13.600
100               850     1700    6800

These are netto times, without the msleep's for power-up and without
overhead.

Looking at this table, I would use a fixed bottom of 100 Khz:

	if (f_min > 400k)
		print existing warning
	clock = max (100k, host->f_min);

Regards, Hein

[PATCH] MMC Core: Drop initialization frequency floor to 50kHz

[Ben Nizette]

Patch

commit 8dfd0374be84793360db7fff2e635d2cd3bbcb21
Author: Sascha Hauer <s.hauer@pengutronix.de>
Date:   Thu Apr 9 08:32:02 2009 +0200

    MMC core: limit minimum initialization frequency to 400kHz

Was recently merged.  This is too fast for at least one setup
permutation - the one on my desk which through trial and error won't
initialise at anything above ~350kHz (older Sandisk 256MB SD on
atmel-mci).

To avoid a string of "just found card X which requires clock
(current_clock - epsilon)" this patch drops the floor right down to
50kHz.  This is about the slowest rate before which the discovery
process takes a noticeable slowdown.

Signed-off-by: Ben Nizette <bn@niasdigital.com>
---
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d84c880..6ee1931 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -708,12 +708,13 @@ static void mmc_power_up(struct mmc_host *host)
 	 */
 	mmc_delay(10);
 
-	if (host->f_min > 400000) {
-		pr_warning("%s: Minimum clock frequency too high for "
-				"identification mode\n", mmc_hostname(host));
-		host->ios.clock = host->f_min;
-	} else
-		host->ios.clock = 400000;
+	/*
+	 * Card discovery is typically done at the controller's minimum
+	 * allowable frequency but for some controllers this is minimum
+	 * is unreasonably slow.  In that case we limit slow clock rate
+	 * to 50KHz.
+	 */
+	host->ios.clock = max(host->f_min, 50000);
 
 	host->ios.power_mode = MMC_POWER_ON;
 	mmc_set_ios(host);

Re: [PATCH] MMC Core: Drop initialization frequency floor to 50kHz

[Haavard Skinnemoen]

Ben Nizette wrote:
> Was recently merged.  This is too fast for at least one setup
> permutation - the one on my desk which through trial and error won't
> initialise at anything above ~350kHz (older Sandisk 256MB SD on
> atmel-mci).

What kind of board is this? It sounds a bit like the CMD or DATA0
pull-up is too weak...

Haavard

Re: [PATCH] MMC Core: Drop initialization frequency floor to 50kHz

[Ben Nizette]

On Thu, 2009-07-02 at 13:58 +0200, Haavard Skinnemoen wrote:
> Ben Nizette wrote:
> > Was recently merged.  This is too fast for at least one setup
> > permutation - the one on my desk which through trial and error won't
> > initialise at anything above ~350kHz (older Sandisk 256MB SD on
> > atmel-mci).
> 
> What kind of board is this? It sounds a bit like the CMD or DATA0
> pull-up is too weak...

It is a custom board (you'll see the board support soon) but those
pullups are 47k same as NGW100 and STK1000.

'scope confirms all the waveforms are crisp and correct.

> 
> Haavard

Re: [PATCH v2] mmc: lower init clock frequency to 300kHz

[Ben Nizette]

(Adding linux CCs)

On 02/01/2010, at 7:19 PM, Hein_Tibosch wrote:

> Almost good: on my Kingston Elite Pro (50x) SD-card, initialization will
> only succeed if F <= 282258 Hz.

...

> 
> I checked some datasheets of several makes:
> 
> Delkin microSD min 0 max 400
> Swissbit min 0 max 400
> Sandisk 100 - 400
> Toshiba 100 - 400
> ST electronics 0 400
> SD-specification: Issue continues clock in frequency range of 100KHz-400KHz
>  If the Average frequency of the SDCLK is less than 100KHz then the card
>  may not be able to respond within the 1-second limit
> 
> So what about 100 or 150 Khz?

Ah geez, yeah this something else I was hoping to avoid with the V1 patch - an endless stream of "oh shit, this card's even /moore/ broken!  Patch it, patch it good"

Pierre, thoughts?  All my cards were borderline spec but Hein's got a mainstream, common card which seems to completely ignore the spec.  Can we just take the few-hundred-uS hit and allow initialization down at the bottom end, say 100kHz?

<overkill> Or a KConfig option? </overkill>

	--Ben.

Re: [PATCH v2] mmc: lower init clock frequency to 300kHz

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 980 bytes --]

On Sat, 2 Jan 2010 20:07:01 +1100
Ben Nizette <bn@niasdigital.com> wrote:

> 
> Pierre, thoughts?  All my cards were borderline spec but Hein's got a mainstream, common card which seems to completely ignore the spec.  Can we just take the few-hundred-uS hit and allow initialization down at the bottom end, say 100kHz?
> 

Broken cards seem to be all over the spectrum, so I wouldn't be
suprised if you find ones that break if you go too low as well.

Basically you'll just have to try something and watch the bug reports.
Perhaps doing the bisect approach and choosing the intermediate value
each time? I.e. halvway between 400 and 100 for now (250). If that is
too fast, then halway between that and 100 (175), and so on.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by FRA, a
  Swedish intelligence agency. Make sure your server uses
  encryption for SMTP traffic and consider using PGP for
  end-to-end encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]