MMC: When rescanning cards check existing cards after mmc_setup()

MMC: When rescanning cards check existing cards after mmc_setup()

[Timo Teras]

Some broken cards seem to process CMD1 even in stand-by state. The result is
that the card replies with ILLEGAL_COMMAND error for the next command sent
after rescanning. Currently the next command is select card, which would
return the error. But the CMD7 does actually succeed and retries of the
command will timeout. The solution is to poll card status after the CMD1
which clears the cached error.

Signed-off-by: Timo Teras <timo.teras@solidboot.com>

---

 drivers/mmc/mmc.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

97675ada7049d12523aa9e9908d4613dfd333641
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index ee8863c..3324b6e 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1178,14 +1178,17 @@ static void mmc_rescan(void *data)
 {
 	struct mmc_host *host = data;
 	struct list_head *l, *n;
+	unsigned char power_mode;
 
 	mmc_claim_host(host);
 
-	if (host->ios.power_mode == MMC_POWER_ON)
-		mmc_check_cards(host);
+	power_mode = host->ios.power_mode;
 
 	mmc_setup(host);
 
+	if (power_mode == MMC_POWER_ON)
+		mmc_check_cards(host);
+
 	if (!list_empty(&host->cards)) {
 		/*
 		 * (Re-)calculate the fastest clock rate which the
-- 
1.2.3.g90cc1

Re: MMC: When rescanning cards check existing cards after mmc_setup()

[Pierre Ossman]

Sorry about not responding sooner. I've been a bit swamped.

Timo Teras wrote:
> Some broken cards seem to process CMD1 even in stand-by state. The result is
> that the card replies with ILLEGAL_COMMAND error for the next command sent
> after rescanning. Currently the next command is select card, which would
> return the error. But the CMD7 does actually succeed and retries of the
> command will timeout. The solution is to poll card status after the CMD1
> which clears the cached error.
>
> Signed-off-by: Timo Teras <timo.teras@solidboot.com>
>
>   

I take it these cards do not reply to CMD2?

> ---
>
>  drivers/mmc/mmc.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> 97675ada7049d12523aa9e9908d4613dfd333641
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index ee8863c..3324b6e 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -1178,14 +1178,17 @@ static void mmc_rescan(void *data)
>  {
>  	struct mmc_host *host = data;
>  	struct list_head *l, *n;
> +	unsigned char power_mode;
>  
>  	mmc_claim_host(host);
>  
> -	if (host->ios.power_mode == MMC_POWER_ON)
> -		mmc_check_cards(host);
> +	power_mode = host->ios.power_mode;
>  
>  	mmc_setup(host);
>  
> +	if (power_mode == MMC_POWER_ON)
> +		mmc_check_cards(host);
> +
>  	if (!list_empty(&host->cards)) {
>  		/*
>  		 * (Re-)calculate the fastest clock rate which the
>   

This change is ok right now, but might come back to bite us in the
future if we implement more intelligent voltage selection (right now new
cards will have to make due with what's already selected).

If we check cards on both sides of mmc_setup(), then we should be covered.

Also, please add some comments about why we do this. Otherwise it will
run the risk of getting removed in the future.

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: MMC: When rescanning cards check existing cards after mmc_setup()

[Timo Teras]

On Sun, Oct 22, 2006 at 11:55:17AM +0200, Pierre Ossman wrote:
> Timo Teras wrote:
> > Some broken cards seem to process CMD1 even in stand-by state. The result is
> > that the card replies with ILLEGAL_COMMAND error for the next command sent
> > after rescanning. Currently the next command is select card, which would
> > return the error. But the CMD7 does actually succeed and retries of the
> > command will timeout. The solution is to poll card status after the CMD1
> > which clears the cached error.
>
> I take it these cards do not reply to CMD2?

No. It just caches the error and fails the next command sent.

> This change is ok right now, but might come back to bite us in the
> future if we implement more intelligent voltage selection (right now new
> cards will have to make due with what's already selected).

I see. The voltage selection is done in mmc_setup() based on what cards are
present.

> If we check cards on both sides of mmc_setup(), then we should be covered.

Should I update my patch to do this already? Or is the code fine as is?

> Also, please add some comments about why we do this. Otherwise it will
> run the risk of getting removed in the future.

Will do.

I'll send updated patch later on.

-- Timo

Re: MMC: When rescanning cards check existing cards after mmc_setup()

[Pierre Ossman]

Timo Teras wrote:
> On Sun, Oct 22, 2006 at 11:55:17AM +0200, Pierre Ossman wrote:
>   
>> If we check cards on both sides of mmc_setup(), then we should be covered.
>>     
>
> Should I update my patch to do this already? Or is the code fine as is?
>
>   

Please include it in your revised patch.

>> Also, please add some comments about why we do this. Otherwise it will
>> run the risk of getting removed in the future.
>>     
>
> Will do.
>
> I'll send updated patch later on.
>
>   

Looking forward to it. :)

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

[PATCH] MMC: Poll card status after rescanning cards

[Timo Teras]

Some broken cards seem to process CMD1 even in stand-by state. The result is
that the card replies with ILLEGAL_COMMAND error for the next command sent
after rescanning. Currently the next command is select card, which would
return the error. But CMD7 does actually succeed and retries of the command
will timeout. The workaround is to poll card status after CMD1 to clear the
pending error.

Signed-off-by: Timo Teras <timo.teras@solidboot.com>
---
 drivers/mmc/mmc.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index ee8863c..c236646 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1178,14 +1178,29 @@ static void mmc_rescan(void *data)
 {
 	struct mmc_host *host = data;
 	struct list_head *l, *n;
+	unsigned char power_mode;
 
 	mmc_claim_host(host);
 
-	if (host->ios.power_mode == MMC_POWER_ON)
+	/*
+	 * Check for removed cards and newly inserted ones. We check for
+	 * removed cards first so we can intelligently re-select the VDD.
+	 */
+	power_mode = host->ios.power_mode;
+	if (power_mode == MMC_POWER_ON)
 		mmc_check_cards(host);
 
 	mmc_setup(host);
 
+	/*
+	 * Some broken cards process CMD1 even in stand-by state. There is
+	 * no reply, but an ILLEGAL_COMMAND error is cached and returned
+	 * after next command. We poll for card status here to clear any
+	 * possibly pending error.
+	 */
+	if (power_mode == MMC_POWER_ON)
+		mmc_check_cards(host);
+
 	if (!list_empty(&host->cards)) {
 		/*
 		 * (Re-)calculate the fastest clock rate which the
-- 
1.4.1.1

Re: [PATCH] MMC: Poll card status after rescanning cards

[Pierre Ossman]

Timo Teras wrote:
> Some broken cards seem to process CMD1 even in stand-by state. The result is
> that the card replies with ILLEGAL_COMMAND error for the next command sent
> after rescanning. Currently the next command is select card, which would
> return the error. But CMD7 does actually succeed and retries of the command
> will timeout. The workaround is to poll card status after CMD1 to clear the
> pending error.
>
> Signed-off-by: Timo Teras <timo.teras@solidboot.com>
>   

Thanks, queued up for -mm.

Andrew, this replaces a patch in your set, so you probably need to
remove that before your next pull from me.

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org