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

[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

[Pierre Ossman]

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

On Thu, 02 Jul 2009 09:49:56 +1000
Ben Nizette <bn@niasdigital.com> wrote:

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

50 kHz seems very low. I'd be more comfortable if we deviate from
specified behaviour as little as possible, say 300 kHz. Has anyone
checked what USB readers use?

And have you confirmed that the card is actually getting 350 kHz when
it fails? Perhaps there is a bug that is causing it to actually run at
a higher frequency.

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

I like getting that warning for too fast controllers. It makes it
easier to spot possible problems.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. 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 --]

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

[Ben Nizette]

On Thu, 2009-07-02 at 09:18 +0200, Pierre Ossman wrote:

> 
> 50 kHz seems very low. I'd be more comfortable if we deviate from
> specified behaviour as little as possible, say 300 kHz. Has anyone
> checked what USB readers use?

Before this patch my controller used ~135kHz, I was just basing the
floor off that.  If the spec says 400 and this is just to work around
broken cards then sure, 300kHz sounds a good plan.

> 
> And have you confirmed that the card is actually getting 350 kHz when
> it fails? Perhaps there is a bug that is causing it to actually run at
> a higher frequency.

'scope confirmed that when 400kHz is requested my controller is giving
the card ~397kHz.

> 
> > 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);
> >  
> 
> I like getting that warning for too fast controllers. It makes it
> easier to spot possible problems.

For sure, it's just that if the floor is 50kHz then pretty much /every/
controller is a 'fast' controller and the warning's redundant.  If you
just drop the 400k to 300k, I agree the warning's a good thing to have.

Cheers,
	--Ben.

> 
> Rgds

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 --]

[PATCH v3] mmc: Make ID freq configurable

[Ben Nizette]

On 02/01/2010, at 11:08 PM, Pierre Ossman wrote:

> 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.

Yea good point, though given there might not even be a One Freq to Rule Them All, how about:

---8<---

From: Ben Nizette <bn@niasdigital.com>
Subject: [PATCH v3] mmc: Make ID freq configurable

While the SD spec specifies a range of frequencies for the ID phase from 100-400kHz, not all cards play by the rules.  This patch adds a Kconfig option to allow the user to tweak this to their card/host config, though the default of 250kHz should be fine for all spec-abiding cards and most others besides.

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

---
diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index ab37a6d..a8e5003 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -14,3 +14,15 @@ config MMC_UNSAFE_RESUME
          This option is usually just for embedded systems which use
          a MMC/SD card for rootfs. Most people should say N here.
 
+config MMC_ID_FREQ
+       int "SD/MMC Card initialisation frequency (Hz)"
+       default 250000
+       help
+         The SD card specification allows for initialisation
+         frequencies from 100-400kHz however some broken cards are
+         known not to work across this whole range.
+
+         If you're having problems with a particular card not being
+         detected, you may try adjusting this frequency up or down
+         but for widest compatibility just leave it default.
+
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7dab2e5..2eab68f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -891,12 +891,17 @@ 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));
+       /*
+        * CONFIG_MMC_ID_FREQ defaults to 250kHz which should be fine
+        * for most card/host combos.
+        */
+       if (host->f_min > CONFIG_MMC_ID_FREQ) {
+               pr_warning("%s: The host cannot reach the requested ID clock "
+                               "rate, identification may fail\n",
+                               mmc_hostname(host));
                host->ios.clock = host->f_min;
        } else
-               host->ios.clock = 400000;
+               host->ios.clock = CONFIG_MMC_ID_FREQ;
 
        host->ios.power_mode = MMC_POWER_ON;
        mmc_set_ios(host);

Re: [PATCH v3] mmc: Make ID freq configurable

[Robert Hancock]

On 01/02/2010 04:23 PM, Ben Nizette wrote:
>
> On 02/01/2010, at 11:08 PM, Pierre Ossman wrote:
>
>> 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.
>
> Yea good point, though given there might not even be a One Freq to Rule Them All, how about:
>
> ---8<---
>
> From: Ben Nizette<bn@niasdigital.com>
> Subject: [PATCH v3] mmc: Make ID freq configurable
>
> While the SD spec specifies a range of frequencies for the ID phase from 100-400kHz, not all cards play by the rules.  This patch adds a Kconfig option to allow the user to tweak this to their card/host config, though the default of 250kHz should be fine for all spec-abiding cards and most others besides.
>
> Signed-off-by: Ben Nizette<bn@niasdigital.com>
>
> ---
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index ab37a6d..a8e5003 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -14,3 +14,15 @@ config MMC_UNSAFE_RESUME
>            This option is usually just for embedded systems which use
>            a MMC/SD card for rootfs. Most people should say N here.
>
> +config MMC_ID_FREQ
> +       int "SD/MMC Card initialisation frequency (Hz)"
> +       default 250000
> +       help
> +         The SD card specification allows for initialisation
> +         frequencies from 100-400kHz however some broken cards are
> +         known not to work across this whole range.
> +
> +         If you're having problems with a particular card not being
> +         detected, you may try adjusting this frequency up or down
> +         but for widest compatibility just leave it default.

Don't know that a kconfig option is best for this, it'd suck to have to 
recompile the kernel to change it. Module option might be better..

Re: [PATCH v3] mmc: Make ID freq configurable

[Pierre Ossman]

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

On Sun, 3 Jan 2010 09:23:30 +1100
Ben Nizette <bn@niasdigital.com> wrote:

> > 
> > 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.
> 
> Yea good point, though given there might not even be a One Freq to Rule Them All, how about:
> 
> ---8<---
> 
> From: Ben Nizette <bn@niasdigital.com>
> Subject: [PATCH v3] mmc: Make ID freq configurable
> 
> While the SD spec specifies a range of frequencies for the ID phase from 100-400kHz, not all cards play by the rules.  This patch adds a Kconfig option to allow the user to tweak this to their card/host config, though the default of 250kHz should be fine for all spec-abiding cards and most others besides.
> 
> Signed-off-by: Ben Nizette <bn@niasdigital.com>
> 

This is not a good solution. We all use the same pool of cards so we
should all be using the same init sequence. If there isn't a single
frequency where all cards will work, then we'll have to make something
more advanced where the kernel will try the init several times with
different clocking.

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 --]

Re: [PATCH v3] mmc: Make ID freq configurable

[Hein_Tibosch]

Pierre Ossman wrote:
> On Sun, 3 Jan 2010 09:23:30 +1100
> Ben Nizette <bn@niasdigital.com> wrote:
>
>   
>>> 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.
>>>       
>> Yea good point, though given there might not even be a One Freq to Rule Them All, how about:
>>
>> ---8<---
>>
>> From: Ben Nizette <bn@niasdigital.com>
>> Subject: [PATCH v3] mmc: Make ID freq configurable
>>
>> While the SD spec specifies a range of frequencies for the ID phase from 100-400kHz, not all cards play by the rules.  This patch adds a Kconfig option to allow the user to tweak this to their card/host config, though the default of 250kHz should be fine for all spec-abiding cards and most others besides.
>>
>> Signed-off-by: Ben Nizette <bn@niasdigital.com>
>>
>>     
>
> This is not a good solution. We all use the same pool of cards so we
> should all be using the same init sequence. If there isn't a single
> frequency where all cards will work, then we'll have to make something
> more advanced where the kernel will try the init several times with
> different clocking.
>   
As I'm lucky to have main-stream cards that break the rule (on
avr32/AP7) I'll dive deeper into it and see if I can make it "automatic"
so that initialization will work for any of cards. Indeed using a kind
of 'bisect approach' approach.
... although I must say that before Sascha Hauer's patch, we never had
problems with any of the cards, using the controller's minimum freq of
137 Khz.

Hein

Re: [PATCH v3] mmc: Make ID freq configurable

[Hein_Tibosch]

On 3-1-2010 07:38, Pierre Ossman wrote:
>
> <cut>
> We all use the same pool of cards so we
> should all be using the same init sequence. If there isn't a single
> frequency where all cards will work, then we'll have to make something
> more advanced where the kernel will try the init several times with
> different clocking.

On 4-1-2010 20:15, Sascha Hauer wrote:
> The problem was that in some setups the mxcmmc driver allows frequencies
> as low as a few Hz and the card initialization took ages. Increasing the
> frequency to 400kHz seemed sensible since it's the value the spec
> describes. 50kHz should be fine as well.
>

I tried out several main-stream sd-cards and with the Atmel (AP7) controller
they all initialize with F between 138 (=f_min) and 280 Khz.

Below a patch which tries mmc-initialization using several frequencies
from an array 400, 300, 200 and 100. When it comes to 200 Khz, it works.
The two failed attempts lasted about 10 ms together.

It looks like a hell-of-a-patch but that's because the indentation changes.

Note that the new mmc_host member f_init will be used later in power_restore
and resume. As power_up always comes first, it should have a descend value
by then.

But I still wonder, why not just keep using Sascha's patch, changing 400000
to 50000? Has anyone ever had problems with mmc-init on 50 Khz?


Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es>

---
diff -Nurp a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -891,12 +891,7 @@ static void mmc_power_up(struct mmc_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;
+	host->ios.clock = host->f_init;

      	host->ios.power_mode = MMC_POWER_ON;
      	mmc_set_ios(host);
@@ -1041,6 +1036,8 @@ void mmc_rescan(struct work_struct *work
      		container_of(work, struct mmc_host, detect.work);
      	u32 ocr;
      	int err;
+	int i;
+	unsigned freqs[] = { 400000, 300000, 200000, 100000 };

      	mmc_bus_get(host);

@@ -1070,46 +1067,55 @@ void mmc_rescan(struct work_struct *work
      	if (host->ops->get_cd && host->ops->get_cd(host) == 0)
      		goto out;

-	mmc_claim_host(host);
+	for (i = 0; i < ARRAY_SIZE(freqs); i++) {
+		mmc_claim_host(host);

-	mmc_power_up(host);
-	mmc_go_idle(host);
+		if (freqs[i] >= host->f_min)
+			host->f_init = freqs[i];
+		else if (i && freqs[i-1] <= host->f_min)
+			goto out;
+		else
+			host->f_init = host->f_min;

-	mmc_send_if_cond(host, host->ocr_avail);
+		printk ("mmc_rescan: trying %u Hz\n", host->f_init);
+		mmc_power_up(host);
+		mmc_go_idle(host);

-	/*
-	 * First we search for SDIO...
-	 */
-	err = mmc_send_io_op_cond(host, 0, &ocr);
-	if (!err) {
-		if (mmc_attach_sdio(host, ocr))
-			mmc_power_off(host);
-		goto out;
-	}
+		mmc_send_if_cond(host, host->ocr_avail);

-	/*
-	 * ...then normal SD...
-	 */
-	err = mmc_send_app_op_cond(host, 0, &ocr);
-	if (!err) {
-		if (mmc_attach_sd(host, ocr))
-			mmc_power_off(host);
-		goto out;
-	}
+		/*
+		 * First we search for SDIO...
+		 */
+		err = mmc_send_io_op_cond(host, 0, &ocr);
+		if (!err) {
+			if (mmc_attach_sdio(host, ocr))
+				mmc_power_off(host);
+			goto out;
+		}

-	/*
-	 * ...and finally MMC.
-	 */
-	err = mmc_send_op_cond(host, 0, &ocr);
-	if (!err) {
-		if (mmc_attach_mmc(host, ocr))
-			mmc_power_off(host);
-		goto out;
-	}
+		/*
+		 * ...then normal SD...
+		 */
+		err = mmc_send_app_op_cond(host, 0, &ocr);
+		if (!err) {
+			if (mmc_attach_sd(host, ocr))
+				mmc_power_off(host);
+			goto out;
+		}

-	mmc_release_host(host);
-	mmc_power_off(host);
+		/*
+		 * ...and finally MMC.
+		 */
+		err = mmc_send_op_cond(host, 0, &ocr);
+		if (!err) {
+			if (mmc_attach_mmc(host, ocr))
+				mmc_power_off(host);
+			goto out;
+		}

+		mmc_release_host(host);
+		mmc_power_off(host);
+	}
      out:
      	if (host->caps & MMC_CAP_NEEDS_POLL)
      		mmc_schedule_delayed_work(&host->detect, HZ);
diff -Nurp a/include/linux/mmc/host.h b/include/linux/mmc/host.h
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -119,6 +119,7 @@ struct mmc_host {
      	const struct mmc_host_ops *ops;
      	unsigned int		f_min;
      	unsigned int		f_max;
+	unsigned int		f_init;
      	u32			ocr_avail;

      #define MMC_VDD_165_195		0x00000080	/* VDD voltage 1.65 - 1.95 */

Re: [PATCH v3] mmc: Make ID freq configurable

[Sascha Hauer]

On Tue, Jan 05, 2010 at 05:07:21AM +0800, Hein_Tibosch wrote:
> On 3-1-2010 07:38, Pierre Ossman wrote:
>>
>> <cut>
>> We all use the same pool of cards so we
>> should all be using the same init sequence. If there isn't a single
>> frequency where all cards will work, then we'll have to make something
>> more advanced where the kernel will try the init several times with
>> different clocking.
>
> On 4-1-2010 20:15, Sascha Hauer wrote:
>> The problem was that in some setups the mxcmmc driver allows frequencies
>> as low as a few Hz and the card initialization took ages. Increasing the
>> frequency to 400kHz seemed sensible since it's the value the spec
>> describes. 50kHz should be fine as well.
>>
>
> I tried out several main-stream sd-cards and with the Atmel (AP7) controller
> they all initialize with F between 138 (=f_min) and 280 Khz.
>
> Below a patch which tries mmc-initialization using several frequencies
> from an array 400, 300, 200 and 100. When it comes to 200 Khz, it works.
> The two failed attempts lasted about 10 ms together.
>
> It looks like a hell-of-a-patch but that's because the indentation changes.
>
> Note that the new mmc_host member f_init will be used later in power_restore
> and resume. As power_up always comes first, it should have a descend value
> by then.
>
> But I still wonder, why not just keep using Sascha's patch, changing 400000
> to 50000? Has anyone ever had problems with mmc-init on 50 Khz?

I already wrote it to Hein in private, once again for the rest:

The original reason to increase the minimum frequency was that the
Freescale i.MX MMC core allows minimum frequencies of just a few Hz.
With this the card initialisation took ages, so we increased the minimum
frequency. 400kHz seemed like a good value since all cards should
support this, but I'm also fine with any other value between 50kHz and
400kHz.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3] mmc: Make ID freq configurable

[Hein_Tibosch]

In my last mail the spaces in the patch were mistreated, sorry for that.

Below a patch which tries mmc-initialization using several frequencies
from an array 400, 300, 200 and 100.


Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es>

---
diff -Nurp a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -891,12 +891,7 @@ static void mmc_power_up(struct mmc_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;
+	host->ios.clock = host->f_init;

 	host->ios.power_mode = MMC_POWER_ON;
 	mmc_set_ios(host);
@@ -1041,6 +1036,8 @@ void mmc_rescan(struct work_struct *work
 		container_of(work, struct mmc_host, detect.work);
 	u32 ocr;
 	int err;
+	int i;
+	unsigned freqs[] = { 400000, 300000, 200000, 100000 };

 	mmc_bus_get(host);

@@ -1070,46 +1067,55 @@ void mmc_rescan(struct work_struct *work
 	if (host->ops->get_cd && host->ops->get_cd(host) == 0)
 		goto out;

-	mmc_claim_host(host);
+	for (i = 0; i < ARRAY_SIZE(freqs); i++) {
+		mmc_claim_host(host);

-	mmc_power_up(host);
-	mmc_go_idle(host);
+		if (freqs[i] >= host->f_min)
+			host->f_init = freqs[i];
+		else if (i && freqs[i-1] <= host->f_min)
+			goto out;
+		else
+			host->f_init = host->f_min;

-	mmc_send_if_cond(host, host->ocr_avail);
+		printk ("mmc_rescan: trying %u Hz\n", host->f_init);
+		mmc_power_up(host);
+		mmc_go_idle(host);

-	/*
-	 * First we search for SDIO...
-	 */
-	err = mmc_send_io_op_cond(host, 0, &ocr);
-	if (!err) {
-		if (mmc_attach_sdio(host, ocr))
-			mmc_power_off(host);
-		goto out;
-	}
+		mmc_send_if_cond(host, host->ocr_avail);

-	/*
-	 * ...then normal SD...
-	 */
-	err = mmc_send_app_op_cond(host, 0, &ocr);
-	if (!err) {
-		if (mmc_attach_sd(host, ocr))
-			mmc_power_off(host);
-		goto out;
-	}
+		/*
+		 * First we search for SDIO...
+		 */
+		err = mmc_send_io_op_cond(host, 0, &ocr);
+		if (!err) {
+			if (mmc_attach_sdio(host, ocr))
+				mmc_power_off(host);
+			goto out;
+		}

-	/*
-	 * ...and finally MMC.
-	 */
-	err = mmc_send_op_cond(host, 0, &ocr);
-	if (!err) {
-		if (mmc_attach_mmc(host, ocr))
-			mmc_power_off(host);
-		goto out;
-	}
+		/*
+		 * ...then normal SD...
+		 */
+		err = mmc_send_app_op_cond(host, 0, &ocr);
+		if (!err) {
+			if (mmc_attach_sd(host, ocr))
+				mmc_power_off(host);
+			goto out;
+		}

-	mmc_release_host(host);
-	mmc_power_off(host);
+		/*
+		 * ...and finally MMC.
+		 */
+		err = mmc_send_op_cond(host, 0, &ocr);
+		if (!err) {
+			if (mmc_attach_mmc(host, ocr))
+				mmc_power_off(host);
+			goto out;
+		}

+		mmc_release_host(host);
+		mmc_power_off(host);
+	}
 out:
 	if (host->caps & MMC_CAP_NEEDS_POLL)
 		mmc_schedule_delayed_work(&host->detect, HZ);
diff -Nurp a/include/linux/mmc/host.h b/include/linux/mmc/host.h
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -119,6 +119,7 @@ struct mmc_host {
 	const struct mmc_host_ops *ops;
 	unsigned int		f_min;
 	unsigned int		f_max;
+	unsigned int		f_init;
 	u32			ocr_avail;

 #define MMC_VDD_165_195		0x00000080	/* VDD voltage 1.65 - 1.95 */

Re: [PATCH v3] mmc: Make ID freq configurable

[Chris Ball]

Hi,

This looks like the best of the min_freq patches I've seen, and is
implementing a suggestion of Pierre's.  Should we take it?

- Chris.

On Tue, Jan 05, 2010 at 05:58:34AM +0800, Hein_Tibosch wrote:
> 
> In my last mail the spaces in the patch were mistreated, sorry for that.
> 
> Below a patch which tries mmc-initialization using several frequencies
> from an array 400, 300, 200 and 100.
> 
> 
> Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es>
> 
> ---
> diff -Nurp a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -891,12 +891,7 @@ static void mmc_power_up(struct mmc_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;
> +	host->ios.clock = host->f_init;
> 
>  	host->ios.power_mode = MMC_POWER_ON;
>  	mmc_set_ios(host);
> @@ -1041,6 +1036,8 @@ void mmc_rescan(struct work_struct *work
>  		container_of(work, struct mmc_host, detect.work);
>  	u32 ocr;
>  	int err;
> +	int i;
> +	unsigned freqs[] = { 400000, 300000, 200000, 100000 };
> 
>  	mmc_bus_get(host);
> 
> @@ -1070,46 +1067,55 @@ void mmc_rescan(struct work_struct *work
>  	if (host->ops->get_cd && host->ops->get_cd(host) == 0)
>  		goto out;
> 
> -	mmc_claim_host(host);
> +	for (i = 0; i < ARRAY_SIZE(freqs); i++) {
> +		mmc_claim_host(host);
> 
> -	mmc_power_up(host);
> -	mmc_go_idle(host);
> +		if (freqs[i] >= host->f_min)
> +			host->f_init = freqs[i];
> +		else if (i && freqs[i-1] <= host->f_min)
> +			goto out;
> +		else
> +			host->f_init = host->f_min;
> 
> -	mmc_send_if_cond(host, host->ocr_avail);
> +		printk ("mmc_rescan: trying %u Hz\n", host->f_init);
> +		mmc_power_up(host);
> +		mmc_go_idle(host);
> 
> -	/*
> -	 * First we search for SDIO...
> -	 */
> -	err = mmc_send_io_op_cond(host, 0, &ocr);
> -	if (!err) {
> -		if (mmc_attach_sdio(host, ocr))
> -			mmc_power_off(host);
> -		goto out;
> -	}
> +		mmc_send_if_cond(host, host->ocr_avail);
> 
> -	/*
> -	 * ...then normal SD...
> -	 */
> -	err = mmc_send_app_op_cond(host, 0, &ocr);
> -	if (!err) {
> -		if (mmc_attach_sd(host, ocr))
> -			mmc_power_off(host);
> -		goto out;
> -	}
> +		/*
> +		 * First we search for SDIO...
> +		 */
> +		err = mmc_send_io_op_cond(host, 0, &ocr);
> +		if (!err) {
> +			if (mmc_attach_sdio(host, ocr))
> +				mmc_power_off(host);
> +			goto out;
> +		}
> 
> -	/*
> -	 * ...and finally MMC.
> -	 */
> -	err = mmc_send_op_cond(host, 0, &ocr);
> -	if (!err) {
> -		if (mmc_attach_mmc(host, ocr))
> -			mmc_power_off(host);
> -		goto out;
> -	}
> +		/*
> +		 * ...then normal SD...
> +		 */
> +		err = mmc_send_app_op_cond(host, 0, &ocr);
> +		if (!err) {
> +			if (mmc_attach_sd(host, ocr))
> +				mmc_power_off(host);
> +			goto out;
> +		}
> 
> -	mmc_release_host(host);
> -	mmc_power_off(host);
> +		/*
> +		 * ...and finally MMC.
> +		 */
> +		err = mmc_send_op_cond(host, 0, &ocr);
> +		if (!err) {
> +			if (mmc_attach_mmc(host, ocr))
> +				mmc_power_off(host);
> +			goto out;
> +		}
> 
> +		mmc_release_host(host);
> +		mmc_power_off(host);
> +	}
>  out:
>  	if (host->caps & MMC_CAP_NEEDS_POLL)
>  		mmc_schedule_delayed_work(&host->detect, HZ);
> diff -Nurp a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -119,6 +119,7 @@ struct mmc_host {
>  	const struct mmc_host_ops *ops;
>  	unsigned int		f_min;
>  	unsigned int		f_max;
> +	unsigned int		f_init;
>  	u32			ocr_avail;
> 
>  #define MMC_VDD_165_195		0x00000080	/* VDD voltage 1.65 - 1.95 */
> 
> 
> --

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v3] mmc: Make ID freq configurable

[Hein_Tibosch]

 On 28-8-2010 04:44, Chris Ball wrote:
> Hi,
>
> This looks like the best of the min_freq patches I've seen, and is
> implementing a suggestion of Pierre's.  Should we take it?
>
> - Chris.
>
> On Tue, Jan 05, 2010 at 05:58:34AM +0800, Hein_Tibosch wrote:
>> Below a patch which tries mmc-initialization using several frequencies
>> from an array 400, 300, 200 and 100.
>>
>>
>> Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es>
>> <cut>
Hi Chris,

Thanks. I can confirm that the above patch has been working properly for
7 months now, using various SD-cards.

Here it tries 3 frequencies, which takes 290 ms:

   000 mmc_rescan: trying 400000 Hz
       Waiting 1sec before mounting root device...
   061 mci: F now 400000 (actual 397727)
       usb 1-2: configuration #1 chosen from 1 choice
   126 mmc_rescan: trying 300000 Hz
       input: eGalax Inc. USB TouchController as /class/input/input1
   193 mci: F now 300000 (actual 299145)
   227 mmc_rescan: trying 200000 Hz
   255 mci: F now 200000 (actual 200000)
   290 mmc0: new SD card at address e624

In a meanwhile it is doing other things, so not much time is wasted.
But most importantly: it never failed to initialize a card.

Hein Tibosch