[PATCH] SD working on OMAP platforms

[PATCH] SD working on OMAP platforms

[Carlos Aguiar]

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

Hi folks,

This patch is a fix in order to make SD cards works again on OMAP platforms.

As we know, on CMD3, response type is R6 to SD cards and R1 to MMC cards.

Following Tony's suggestion, this fix ignores the buggy SD handling on CMD3.

I've tested this patch on H2 and H3 platforms using MMC, SD and SDHC
cards and
looks fine.

BR,

Carlos.

-- 
Carlos Eduardo
Nokia Institute of Technology - INdT
Open Source Mobile Research Center - OSMRC
Phone: +55 92 2126-1079
Mobile: +55 92 8127-1797
E-mail: carlos.aguiar@indt.org.br


[-- Attachment #2: sd_buggy_handling.diff --]
[-- Type: text/plain, Size: 891 bytes --]

On CMD3, response type is R6 to SD cards and R1 to MMC cards.
This fix ignores the buggy SD handling on CMD3.

Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar@indt.org.br>

Index: linux-omap/drivers/mmc/omap.c
===================================================================
--- linux-omap.orig/drivers/mmc/omap.c	2007-03-30 09:39:08.000000000 -0400
+++ linux-omap/drivers/mmc/omap.c	2007-03-30 16:14:08.000000000 -0400
@@ -462,6 +462,14 @@ static irqreturn_t mmc_omap_irq(int irq,
 		mmc_omap_report_irq(status);
 		printk("\n");
 #endif
+		if (host->mmc->mode == MMC_MODE_SD &&
+		    host->cmd->opcode == SD_SEND_RELATIVE_ADDR &&
+		    status == 0x4000) {
+			dev_dbg(mmc_dev(host->mmc),
+				"buggy SD handling, ignoring it\n");
+			status = 0x0001;
+		}
+
 		if (host->total_bytes_left) {
 			if ((status & OMAP_MMC_STAT_A_FULL) ||
 			    (status & OMAP_MMC_STAT_END_OF_DATA))

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] SD working on OMAP platforms

[Pierre Ossman]

Carlos Aguiar wrote:
> On CMD3, response type is R6 to SD cards and R1 to MMC cards.
> This fix ignores the buggy SD handling on CMD3.
>
> Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar@indt.org.br>
>
>   

NAK. Includes opcodes.

I assume 0x4000 is that darn CERR, so have the if-statement test just
(status == 0x4000).

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: [PATCH] SD working on OMAP platforms

[Carlos Aguiar]

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

ext Pierre Ossman wrote:
> Carlos Aguiar wrote:
>   
>> On CMD3, response type is R6 to SD cards and R1 to MMC cards.
>> This fix ignores the buggy SD handling on CMD3.
>>
>> Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar@indt.org.br>
>>
>>   
>>     
>
> NAK. Includes opcodes.
>
> I assume 0x4000 is that darn CERR, so have the if-statement test just
> (status == 0x4000).
>
> Rgds
>
>   
Ok,

Now, without opcodes and if-statement test just
(status == 0x4000).

BR,

Carlos.

-- 
Carlos Eduardo
Nokia Institute of Technology - INdT
Open Source Mobile Research Center - OSMRC
Phone: +55 92 2126-1079
Mobile: +55 92 8127-1797
E-mail: carlos.aguiar@indt.org.br


[-- Attachment #2: sd_buggy_handling.diff --]
[-- Type: text/plain, Size: 797 bytes --]

On CMD3, response type is R6 to SD cards and R1 to MMC cards.
This fix ignores the buggy SD handling on CMD3.

Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar@indt.org.br>

Index: linux-omap/drivers/mmc/omap.c
===================================================================
--- linux-omap.orig/drivers/mmc/omap.c	2007-03-30 09:39:08.000000000 -0400
+++ linux-omap/drivers/mmc/omap.c	2007-03-31 18:03:49.000000000 -0400
@@ -462,6 +462,12 @@ static irqreturn_t mmc_omap_irq(int irq,
 		mmc_omap_report_irq(status);
 		printk("\n");
 #endif
+		if (status == 0x4000) {
+			dev_dbg(mmc_dev(host->mmc),
+				"buggy SD handling, ignoring it\n");
+			status = 0x0001;
+		}
+
 		if (host->total_bytes_left) {
 			if ((status & OMAP_MMC_STAT_A_FULL) ||
 			    (status & OMAP_MMC_STAT_END_OF_DATA))

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] SD working on OMAP platforms

[Pierre Ossman]

Carlos Aguiar wrote:
> On CMD3, response type is R6 to SD cards and R1 to MMC cards.
> This fix ignores the buggy SD handling on CMD3.
>
>   

This is misleading. The problem is the OMAP controller trying to be
smart with the end result of it doing something completely wrong. The
controller parses data when it has no idea what we want parsed.

> +		if (status == 0x4000) {
> +			dev_dbg(mmc_dev(host->mmc),
> +				"buggy SD handling, ignoring it\n");
> +			status = 0x0001;
> +		}
> +
>   

Some defines for those constants would be nice. And the debug message
should be more general

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: [PATCH] SD working on OMAP platforms

[Pierre Ossman]

tony@atomide.com wrote:
> Always ignoring CERR seems totally unsafe to me. Also, it would be
> best to use the existing #define OMAP_MMC_STAT_CARD_ERR.
>
> To recap, CERR needs to be ignored only with SD cards, and only
> with CMD3.
>   

No, CERR needs to be ignored whenever we do not want the controller to
parse response bits for us (i.e. always). It parses them incorrectly
when the reponse type isn't a standard/classic R1 and we might even
_want_ to see some bits indicating error there. As such, respecting CERR
is broken and the should be ignored at every turn.

> Pierre, what do you have as an alternative for working around hardware
> bugs without using opcodes? AFAIK opcodes are needed to work around
> some hardware issues, such as this.
>
>   

Looking at protocol stuff is always the last resort. In this case it
isn't necessary, but it might be in some cases. Really, the only time we
need to look at the opcode is when the controller does too and we know
it does something stupid when it sees some values. The wbsd driver has a
fix based on this principle. It isn't pretty and it means that
controller cannot use some cards, but it was the only way to solve the
issue (and believe me, I tried long and hard to come up with another way).

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: [PATCH] SD working on OMAP platforms

[tony]

* Pierre Ossman <drzeus-list@drzeus.cx> [070403 00:35]:
> Carlos Aguiar wrote:
> > On CMD3, response type is R6 to SD cards and R1 to MMC cards.
> > This fix ignores the buggy SD handling on CMD3.
> >
> >   
> 
> This is misleading. The problem is the OMAP controller trying to be
> smart with the end result of it doing something completely wrong. The
> controller parses data when it has no idea what we want parsed.
> 
> > +		if (status == 0x4000) {
> > +			dev_dbg(mmc_dev(host->mmc),
> > +				"buggy SD handling, ignoring it\n");
> > +			status = 0x0001;
> > +		}
> > +
> >   
> 
> Some defines for those constants would be nice. And the debug message
> should be more general

Always ignoring CERR seems totally unsafe to me. Also, it would be
best to use the existing #define OMAP_MMC_STAT_CARD_ERR.

To recap, CERR needs to be ignored only with SD cards, and only
with CMD3.

Pierre, what do you have as an alternative for working around hardware
bugs without using opcodes? AFAIK opcodes are needed to work around
some hardware issues, such as this.

Regards,

Tony

Re: [PATCH] SD working on OMAP platforms

[Tony Lindgren]

* Pierre Ossman <drzeus-list@drzeus.cx> [070403 09:10]:
> tony@atomide.com wrote:
> > Always ignoring CERR seems totally unsafe to me. Also, it would be
> > best to use the existing #define OMAP_MMC_STAT_CARD_ERR.
> >
> > To recap, CERR needs to be ignored only with SD cards, and only
> > with CMD3.
> >   
> 
> No, CERR needs to be ignored whenever we do not want the controller to
> parse response bits for us (i.e. always). It parses them incorrectly
> when the reponse type isn't a standard/classic R1 and we might even
> _want_ to see some bits indicating error there. As such, respecting CERR
> is broken and the should be ignored at every turn.

OK, thanks for clarifying. In that case it sounds like the right
solution would be to totally disable CERR interrupt. This should be
doable by not writing OMAP_MMC_STAT_CARD_ERR bit to IE register.

Hmm, that might allow us to get rid of the other OMAP_MMC_STAT_CARD_ERR
quirk there too :) The one with MMC_STOP_TRANSMISSION one.

And then there would be only one opcode quirk in mmc_omap_irq() for
OMAP_MMC_STAT_CMD_TOUT.

> > Pierre, what do you have as an alternative for working around hardware
> > bugs without using opcodes? AFAIK opcodes are needed to work around
> > some hardware issues, such as this.
> >
> >   
> 
> Looking at protocol stuff is always the last resort. In this case it
> isn't necessary, but it might be in some cases. Really, the only time we
> need to look at the opcode is when the controller does too and we know
> it does something stupid when it sees some values. The wbsd driver has a
> fix based on this principle. It isn't pretty and it means that
> controller cannot use some cards, but it was the only way to solve the
> issue (and believe me, I tried long and hard to come up with another way).

Yeah, OK. The omap controller has problems with some cards, so
hopefully this will help with that.

Regards,

Tony