* SD response type 6 vs. 1
@ 2007-06-04 15:41 andrzej zaborowski
2007-06-04 18:15 ` Pierre Ossman
0 siblings, 1 reply; 6+ messages in thread
From: andrzej zaborowski @ 2007-06-04 15:41 UTC (permalink / raw)
To: Linux OMAP, Pierre Ossman
Hi,
Linux has the same values for MMC_RSP_R1 and MMC_RSP_R6 thus making
the two response formats indistinguishable for mmc/sd hosts. Hardware
like my OMAP310 needs to know exactly whether to expect an R1 or R6
format, otherwise CMD3 fails.
Sure it would be nice if it didn't care, but nevertheless, it's not a
hardware bug - the two formats are specified separately in SD specs
because they *are* different. So the hardware has full right to know
the right response type. Rather it's a hack to rely on R1 and R6
having the same size, like Linux does now. It would also be a hack to
allow the CMD3 to fail with CRC error and ignore the error, like
omap.c did before, because it makes an assumption that's not in the
OMAP specs.
The following is an ugly fix. It's ugly because host code should be
agnostic to opcodes, as mentioned by Pierre Ossman.
I believe this was already submitted by Carlos Aguiar and rejected by
Pierre, which resulted in leaving the driver broken on some boards.
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -22,6 +22,7 @@
#include <linux/spinlock.h>
#include <linux/timer.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/host.h>
#include <linux/mmc/card.h>
#include <linux/clk.h>
@@ -211,6 +221,9 @@ mmc_omap_start_command(struct mmc_omap_host *host,
struct mmc_command *cmd)
case MMC_RSP_R1B:
/* resp 1, 1b, 6, 7 */
resptype = 1;
+ if (mmc_card_sd(host->mmc->card) &&
+ cmd->opcode == SD_SEND_RELATIVE_ADDR)
+ resptype = 6;
break;
case MMC_RSP_R2:
resptype = 2;
----8<---------------------------------
A more correct and more intrusive fix would look like this:
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -25,11 +25,12 @@ struct mmc_command {
#define MMC_RSP_CRC (1 << 2) /* expect valid crc */
#define MMC_RSP_BUSY (1 << 3) /* card may send busy */
#define MMC_RSP_OPCODE (1 << 4) /* response contains opcode */
-#define MMC_CMD_MASK (3 << 5) /* command type */
-#define MMC_CMD_AC (0 << 5)
-#define MMC_CMD_ADTC (1 << 5)
-#define MMC_CMD_BC (2 << 5)
-#define MMC_CMD_BCR (3 << 5)
+#define MMC_RSP_RCA (1 << 5) /* response contains an rca */
+#define MMC_CMD_MASK (3 << 6) /* command type */
+#define MMC_CMD_AC (0 << 6)
+#define MMC_CMD_ADTC (1 << 6)
+#define MMC_CMD_BC (2 << 6)
+#define MMC_CMD_BCR (3 << 6)
/*
* These are the response types, and correspond to valid bit
@@ -41,10 +42,10 @@ struct mmc_command {
#define MMC_RSP_R1B
(MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE|MMC_RSP_BUSY)
#define MMC_RSP_R2 (MMC_RSP_PRESENT|MMC_RSP_136|MMC_RSP_CRC)
#define MMC_RSP_R3 (MMC_RSP_PRESENT)
-#define MMC_RSP_R6 (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
+#define MMC_RSP_R6 (MMC_RSP_PRESENT|MMC_RSP_RCA)
#define MMC_RSP_R7 (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
-#define mmc_resp_type(cmd) ((cmd)->flags &
(MMC_RSP_PRESENT|MMC_RSP_136|MMC_RSP_CRC|MMC_RSP_BUSY|MMC_RSP_OPCODE))
+#define mmc_resp_type(cmd) ((cmd)->flags &
(MMC_RSP_PRESENT|MMC_RSP_136|MMC_RSP_CRC|MMC_RSP_BUSY|MMC_RSP_OPCODE|MMC_RSP_RCA))
/*
* These are the command types.
diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c
--- a/drivers/mmc/host/au1xmmc.c
+++ b/drivers/mmc/host/au1xmmc.c
@@ -206,6 +206,9 @@ static int au1xmmc_send_command(struct
au1xmmc_host *host, int wait,
case MMC_RSP_R3:
mmccmd |= SD_CMD_RT_3;
break;
+ case MMC_RSP_R6:
+ mmccmd |= SD_CMD_RT_6;
+ break;
default:
printk(KERN_INFO "au1xmmc: unhandled response type %02x\n",
mmc_resp_type(cmd));
diff --git a/drivers/mmc/host/imxmmc.c b/drivers/mmc/host/imxmmc.c
--- a/drivers/mmc/host/imxmmc.c
+++ b/drivers/mmc/host/imxmmc.c
@@ -350,6 +350,9 @@ static void imxmci_start_cmd(struct imxmci_host
*host, struct mmc_command *cmd,
case MMC_RSP_R3: /* short */
cmdat |= CMD_DAT_CONT_RESPONSE_FORMAT_R3;
break;
+ case MMC_RSP_R6: /* short RCA */
+ cmdat |= CMD_DAT_CONT_RESPONSE_FORMAT_R6;
+ break;
default:
break;
}
diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -209,7 +219,7 @@ mmc_omap_start_command(struct mmc_omap_host *host,
struct mmc_command *cmd)
break;
case MMC_RSP_R1:
case MMC_RSP_R1B:
- /* resp 1, 1b, 6, 7 */
+ /* resp 1, 1b, 7 */
resptype = 1;
break;
case MMC_RSP_R2:
@@ -218,6 +228,9 @@ mmc_omap_start_command(struct mmc_omap_host *host,
struct mmc_command *cmd)
case MMC_RSP_R3:
resptype = 3;
break;
+ case MMC_RSP_R6:
+ resptype = 6;
+ break;
default:
dev_err(mmc_dev(host->mmc), "Invalid response type:
%04x\n", mmc_resp_type(cmd));
break;
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -170,7 +170,8 @@ static void pxamci_start_cmd(struct pxamci_host
*host, struct mmc_command *cmd,
#define RSP_TYPE(x) ((x) & ~(MMC_RSP_BUSY|MMC_RSP_OPCODE))
switch (RSP_TYPE(mmc_resp_type(cmd))) {
- case RSP_TYPE(MMC_RSP_R1): /* r1, r1b, r6, r7 */
+ case RSP_TYPE(MMC_RSP_R1): /* r1, r1b, r7 */
+ case RSP_TYPE(MMC_RSP_R6):
cmdat |= CMDAT_RESP_SHORT;
break;
case RSP_TYPE(MMC_RSP_R3):
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -549,7 +549,7 @@ static void sdhci_send_command(struct sdhci_host
*host, struct mmc_command *cmd)
if (cmd->flags & MMC_RSP_CRC)
flags |= SDHCI_CMD_CRC;
- if (cmd->flags & MMC_RSP_OPCODE)
+ if (cmd->flags & (MMC_RSP_OPCODE | MMC_RSP_RCA))
flags |= SDHCI_CMD_INDEX;
if (cmd->data)
flags |= SDHCI_CMD_DATA;
diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -345,6 +345,9 @@ static unsigned int tifm_sd_op_flags(struct
mmc_command *cmd)
case MMC_RSP_R3:
rc |= TIFM_MMCSD_RSP_R3;
break;
+ case MMC_RSP_R6:
+ rc |= TIFM_MMCSD_RSP_R6;
+ break;
default:
BUG();
}
What's the preferred way to resolve this?
Regards,
Andrzej
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: SD response type 6 vs. 1
2007-06-04 15:41 SD response type 6 vs. 1 andrzej zaborowski
@ 2007-06-04 18:15 ` Pierre Ossman
2007-06-04 18:55 ` andrzej zaborowski
2007-06-04 18:56 ` Carlos Aguiar
0 siblings, 2 replies; 6+ messages in thread
From: Pierre Ossman @ 2007-06-04 18:15 UTC (permalink / raw)
To: andrzej zaborowski; +Cc: Linux OMAP
andrzej zaborowski wrote:
> Hi,
> Linux has the same values for MMC_RSP_R1 and MMC_RSP_R6 thus making
> the two response formats indistinguishable for mmc/sd hosts. Hardware
> like my OMAP310 needs to know exactly whether to expect an R1 or R6
> format, otherwise CMD3 fails.
>
> Sure it would be nice if it didn't care, but nevertheless, it's not a
> hardware bug - the two formats are specified separately in SD specs
> because they *are* different. So the hardware has full right to know
> the right response type. Rather it's a hack to rely on R1 and R6
> having the same size, like Linux does now. It would also be a hack to
> allow the CMD3 to fail with CRC error and ignore the error, like
> omap.c did before, because it makes an assumption that's not in the
> OMAP specs.
If you're referring to CERR, then I suggest you reread the specs. They
even explicitly say its okay to ignore it.
Anyway, I'm not going into this discussion again without a valid
argument as to why the suggest path won't work.
>
> The following is an ugly fix. It's ugly because host code should be
> agnostic to opcodes, as mentioned by Pierre Ossman.
> I believe this was already submitted by Carlos Aguiar and rejected by
> Pierre, which resulted in leaving the driver broken on some boards.
This just hides the fact that the host parses the data more than we want
and expect it to do.
>
> A more correct and more intrusive fix would look like this:
Afraid not. The response type specifies format, not semantical contents.
The correct fix has already been discussed, but I have seen zero
attempts at implementing it:
http://linux.omap.com/pipermail/linux-omap-open-source/2007-April/009500.html
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: SD response type 6 vs. 1
2007-06-04 18:15 ` Pierre Ossman
@ 2007-06-04 18:55 ` andrzej zaborowski
2007-06-04 18:56 ` Carlos Aguiar
1 sibling, 0 replies; 6+ messages in thread
From: andrzej zaborowski @ 2007-06-04 18:55 UTC (permalink / raw)
To: Pierre Ossman; +Cc: Linux OMAP
On 04/06/07, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> andrzej zaborowski wrote:
> > Hi,
> > Linux has the same values for MMC_RSP_R1 and MMC_RSP_R6 thus making
> > the two response formats indistinguishable for mmc/sd hosts. Hardware
> > like my OMAP310 needs to know exactly whether to expect an R1 or R6
> > format, otherwise CMD3 fails.
> >
> > Sure it would be nice if it didn't care, but nevertheless, it's not a
> > hardware bug - the two formats are specified separately in SD specs
> > because they *are* different. So the hardware has full right to know
> > the right response type. Rather it's a hack to rely on R1 and R6
> > having the same size, like Linux does now. It would also be a hack to
> > allow the CMD3 to fail with CRC error and ignore the error, like
> > omap.c did before, because it makes an assumption that's not in the
> > OMAP specs.
>
> If you're referring to CERR, then I suggest you reread the specs. They
> even explicitly say its okay to ignore it.
If CERR is same as Card_Err bit in MMC_STAT, then I can't find
anything about ignoring it in OMAP310 TRM.
But the assumption here is different - that the only effect of
specifying a wrong response type is this bit getting set wrongly. The
assumption is wrong, the hardware can do anything it likes with the
information we gave it, including parsing the opcode - as long as it
conforms with TRM. Nowhere does it say that it's okay to pass R1 in
place of R6, and if it doesn't say this, the effect is defined as
"undefined". We should avoid this.
>
> Anyway, I'm not going into this discussion again without a valid
> argument as to why the suggest path won't work.
>
> >
> > The following is an ugly fix. It's ugly because host code should be
> > agnostic to opcodes, as mentioned by Pierre Ossman.
> > I believe this was already submitted by Carlos Aguiar and rejected by
> > Pierre, which resulted in leaving the driver broken on some boards.
>
> This just hides the fact that the host parses the data more than we want
> and expect it to do.
We expect what the docs say.
Regards,
Andrzej
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SD response type 6 vs. 1
2007-06-04 18:15 ` Pierre Ossman
2007-06-04 18:55 ` andrzej zaborowski
@ 2007-06-04 18:56 ` Carlos Aguiar
2007-06-06 18:09 ` Pierre Ossman
1 sibling, 1 reply; 6+ messages in thread
From: Carlos Aguiar @ 2007-06-04 18:56 UTC (permalink / raw)
To: ext Pierre Ossman; +Cc: Linux OMAP
[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]
ext Pierre Ossman wrote:
> andrzej zaborowski wrote:
>
>> Hi,
>> Linux has the same values for MMC_RSP_R1 and MMC_RSP_R6 thus making
>> the two response formats indistinguishable for mmc/sd hosts. Hardware
>> like my OMAP310 needs to know exactly whether to expect an R1 or R6
>> format, otherwise CMD3 fails.
>>
>> Sure it would be nice if it didn't care, but nevertheless, it's not a
>> hardware bug - the two formats are specified separately in SD specs
>> because they *are* different. So the hardware has full right to know
>> the right response type. Rather it's a hack to rely on R1 and R6
>> having the same size, like Linux does now. It would also be a hack to
>> allow the CMD3 to fail with CRC error and ignore the error, like
>> omap.c did before, because it makes an assumption that's not in the
>> OMAP specs.
>>
>
> If you're referring to CERR, then I suggest you reread the specs. They
> even explicitly say its okay to ignore it.
>
> Anyway, I'm not going into this discussion again without a valid
> argument as to why the suggest path won't work.
>
>
>> The following is an ugly fix. It's ugly because host code should be
>> agnostic to opcodes, as mentioned by Pierre Ossman.
>> I believe this was already submitted by Carlos Aguiar and rejected by
>> Pierre, which resulted in leaving the driver broken on some boards.
>>
>
> This just hides the fact that the host parses the data more than we want
> and expect it to do.
>
>
>> A more correct and more intrusive fix would look like this:
>>
>
> Afraid not. The response type specifies format, not semantical contents.
>
> The correct fix has already been discussed, but I have seen zero
> attempts at implementing it:
>
> http://linux.omap.com/pipermail/linux-omap-open-source/2007-April/009500.html
>
>
Hi Pierre and andrzej,
Regarding the correct fix mentioned (and discussed) by Pierre, I'd
implemented a piece of this fix by removing CERR, but it's not enough to
make SD works. The patch find attached to this mail.
The part of be able to leave it out to do the card error parsing with
software, like Pierre said, must be implemented.
I'm now busy working in a patches series to MMC multislot support for
OMAP (1510, H2, H3 and H4) platforms.
Regards,
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: 0001-MMC-OMAP-Totally-remove-CERR-interrupt.diff --]
[-- Type: text/plain, Size: 1811 bytes --]
Totally disable of CERR interrupt, by not writing OMAP_MMC_STAT_CARD_ERR
bit to IE register and removing OMAP_MMC_STAT_CARD_ERR quiks on omap.c
Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar@indt.org.br>
Index: linux-omap/drivers/mmc/host/omap.c
===================================================================
--- linux-omap.orig/drivers/mmc/host/omap.c 2007-06-04 14:37:56.000000000 -0400
+++ linux-omap/drivers/mmc/host/omap.c 2007-06-04 14:45:49.000000000 -0400
@@ -253,8 +253,7 @@ mmc_omap_start_command(struct mmc_omap_h
OMAP_MMC_STAT_A_EMPTY | OMAP_MMC_STAT_A_FULL |
OMAP_MMC_STAT_CMD_CRC | OMAP_MMC_STAT_CMD_TOUT |
OMAP_MMC_STAT_DATA_CRC | OMAP_MMC_STAT_DATA_TOUT |
- OMAP_MMC_STAT_END_OF_CMD | OMAP_MMC_STAT_CARD_ERR |
- OMAP_MMC_STAT_END_OF_DATA);
+ OMAP_MMC_STAT_END_OF_CMD | OMAP_MMC_STAT_END_OF_DATA);
OMAP_MMC_WRITE(host, CMD, cmdreg);
}
@@ -523,31 +522,6 @@ static irqreturn_t mmc_omap_irq(int irq,
"command CRC error without cmd?\n");
}
- if (status & OMAP_MMC_STAT_CARD_ERR) {
- if (host->cmd && host->cmd->opcode == MMC_STOP_TRANSMISSION) {
- u32 response = OMAP_MMC_READ(host, RSP6)
- | (OMAP_MMC_READ(host, RSP7) << 16);
- /* STOP sometimes sets must-ignore bits */
- if (!(response & (R1_CC_ERROR
- | R1_ILLEGAL_COMMAND
- | R1_COM_CRC_ERROR))) {
- end_command = 1;
- continue;
- }
- }
-
- dev_dbg(mmc_dev(host->mmc), "card status error (CMD%d)\n",
- host->cmd->opcode);
- if (host->cmd) {
- host->cmd->error = MMC_ERR_FAILED;
- end_command = 1;
- }
- if (host->data) {
- host->data->error = MMC_ERR_FAILED;
- transfer_error = 1;
- }
- }
-
/*
* NOTE: On 1610 the END_OF_CMD may come too early when
* starting a write
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-06-08 13:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-04 15:41 SD response type 6 vs. 1 andrzej zaborowski
2007-06-04 18:15 ` Pierre Ossman
2007-06-04 18:55 ` andrzej zaborowski
2007-06-04 18:56 ` Carlos Aguiar
2007-06-06 18:09 ` Pierre Ossman
[not found] ` <25c21ceb0706061415k5e8ce510t3a1c999bcbb5102c@mail.gmail.com>
[not found] ` <4667C0B6.6090408@drzeus.cx>
2007-06-08 13:55 ` Fwd: " Ragner Magalhães
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox