This change depends on my SDHC patch and fixes a bug that was revealed during the
development of that patch. The R6 response type should be identical to R1 (and R7)
but was incorrectly defined differently. Fixing the R6 definition breaks assumptions
in these two drivers that response type flags are unique. Pierre and Alex both
believe that treating R6 and R7 as R1 will be sufficient. ie: The controllers do
not care about the differences between them. Due to lack of hardware, I have done
no testing.
Signed-off-by: Philip Langdale <[email protected]>
Cc: Alex Dubov <[email protected]>
Cc: Pavel Pisa <[email protected]>
---
drivers/mmc/imxmmc.c | 3 ---
drivers/mmc/omap.c | 2 +-
drivers/mmc/pxamci.c | 2 +-
drivers/mmc/tifm_sd.c | 3 ---
include/linux/mmc/mmc.h | 2 +-
5 files changed, 3 insertions(+), 9 deletions(-)
diff -ur linux-2.6.19-sdhc/drivers/mmc/imxmmc.c linux-2.6.19-rspfix/drivers/mmc/imxmmc.c
--- linux-2.6.19-sdhc/drivers/mmc/imxmmc.c 2007-01-04 06:52:12.000000000 -0800
+++ linux-2.6.19-rspfix/drivers/mmc/imxmmc.c 2007-01-04 06:53:16.000000000 -0800
@@ -351,9 +351,6 @@
case MMC_RSP_R3: /* short */
cmdat |= CMD_DAT_CONT_RESPONSE_FORMAT_R3;
break;
- case MMC_RSP_R6: /* short CRC */
- cmdat |= CMD_DAT_CONT_RESPONSE_FORMAT_R6;
- break;
default:
break;
}
diff -ur linux-2.6.19-sdhc/drivers/mmc/omap.c linux-2.6.19-rspfix/drivers/mmc/omap.c
--- linux-2.6.19-sdhc/drivers/mmc/omap.c 2007-01-04 06:52:12.000000000 -0800
+++ linux-2.6.19-rspfix/drivers/mmc/omap.c 2007-01-04 05:46:24.000000000 -0800
@@ -206,7 +206,7 @@
/* Our hardware needs to know exact type */
switch (RSP_TYPE(mmc_resp_type(cmd))) {
case RSP_TYPE(MMC_RSP_R1):
- /* resp 1, resp 1b */
+ /* resp 1, 1b, 6, 7 */
resptype = 1;
break;
case RSP_TYPE(MMC_RSP_R2):
diff -ur linux-2.6.19-sdhc/drivers/mmc/pxamci.c linux-2.6.19-rspfix/drivers/mmc/pxamci.c
--- linux-2.6.19-sdhc/drivers/mmc/pxamci.c 2007-01-04 06:52:12.000000000 -0800
+++ linux-2.6.19-rspfix/drivers/mmc/pxamci.c 2007-01-04 05:46:36.000000000 -0800
@@ -171,7 +171,7 @@
#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 */
+ case RSP_TYPE(MMC_RSP_R1): /* r1, r1b, r6, r7 */
cmdat |= CMDAT_RESP_SHORT;
break;
case RSP_TYPE(MMC_RSP_R3):
diff -ur linux-2.6.19-sdhc/drivers/mmc/tifm_sd.c linux-2.6.19-rspfix/drivers/mmc/tifm_sd.c
--- linux-2.6.19-sdhc/drivers/mmc/tifm_sd.c 2007-01-04 06:52:12.000000000 -0800
+++ linux-2.6.19-rspfix/drivers/mmc/tifm_sd.c 2007-01-04 06:53:38.000000000 -0800
@@ -173,9 +173,6 @@
case MMC_RSP_R3:
rc |= TIFM_MMCSD_RSP_R3;
break;
- case MMC_RSP_R6:
- rc |= TIFM_MMCSD_RSP_R6;
- break;
default:
BUG();
}
diff -ur linux-2.6.19-sdhc/include/linux/mmc/mmc.h linux-2.6.19-rspfix/include/linux/mmc/mmc.h
--- linux-2.6.19-sdhc/include/linux/mmc/mmc.h 2007-01-04 06:51:09.000000000 -0800
+++ linux-2.6.19-rspfix/include/linux/mmc/mmc.h 2007-01-04 05:41:49.000000000 -0800
@@ -42,7 +42,7 @@
#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)
+#define MMC_RSP_R6 (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
#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))
Hello Philip,
On Thursday 04 January 2007 16:04, Philip Langdale wrote:
> This change depends on my SDHC patch and fixes a bug that was revealed
> during the development of that patch. The R6 response type should be
> identical to R1 (and R7) but was incorrectly defined differently. Fixing
> the R6 definition breaks assumptions in these two drivers that response
> type flags are unique. Pierre and Alex both believe that treating R6 and R7
> as R1 will be sufficient. ie: The controllers do not care about the
> differences between them. Due to lack of hardware, I have done no testing.
I have tested your patch.
Kernel builds. I have not found much time for testing.
But I would not like to block changes and I am going
for next week to project meeting in Spain, so there is
my reply.
I have 2.6.19 + realtime-patches rt14 on the hand.
I have been able to mount and use some cards, but it
I have observed some problems probably related to timing
when I have tried to change CPU frequency.
I need to find time to do more checking on vanilla and RT kernels
when I return. I have some ideas what could be enhanced to ensure
better MX1 SDHC cards recognition under RT kernels. I am not sure,
what causes other seen problems, but I have observed these things
on RT even without your patch.
Conclusion: I knowledge your patch and admit, that I need to
find time for my homeworks.
Best wishes
Pavel Pisa
Pavel Pisa wrote:
> I have tested your patch.
> Kernel builds. I have not found much time for testing.
> But I would not like to block changes and I am going
> for next week to project meeting in Spain, so there is
> my reply.
>
> I have 2.6.19 + realtime-patches rt14 on the hand.
> I have been able to mount and use some cards, but it
> I have observed some problems probably related to timing
> when I have tried to change CPU frequency.
>
>
>From what I gather, the imx driver/hw is a bit funky in several areas.
My plan with this patch is -mm for this release, and merged during the next.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
Greetings.
It appears to me that under certain circumstances mmc layer will issue requests to the host after
mmc_host_remove returns. This happens, for example, in tifm_sd driver because mmc_host may be
removed mid-transfer, as the socket shall be freed for possible reuse by different media type.
Currently, the only solution is to sleep a little somewhere after mmc_remove_host but before
mmc_free_host. I think the correct way to handle the situation is to ensure that mmc_host is never
accessed by the mmc layer after mmc_remove_host returns.
I think a easy way to handle this is to modify __mmc_claim_host to fail if the mmc_host is marked
for removal (this implies that its return value should be checked on use, which is not currently
the case everywhere). This way, mmc_host_remove can claim host, mark it as "dead" and then return
safely knowing that nobody will send any more requests to the host.
Some questions:
1. Will this suffice for the task?
2. Are there any reasons not to do this?
3. Is it possible to replace the fancy locking loop in the __mmc_claim_host with mutex based
locking (mutex does the same thing, isn't it)?
____________________________________________________________________________________
Get your own web address.
Have a HUGE year through Yahoo! Small Business.
http://smallbusiness.yahoo.com/domains/?p=BESTDEAL
Alex Dubov wrote:
> Greetings.
>
> It appears to me that under certain circumstances mmc layer will issue requests to the host after
> mmc_host_remove returns. This happens, for example, in tifm_sd driver because mmc_host may be
> removed mid-transfer, as the socket shall be freed for possible reuse by different media type.
> Currently, the only solution is to sleep a little somewhere after mmc_remove_host but before
> mmc_free_host. I think the correct way to handle the situation is to ensure that mmc_host is never
> accessed by the mmc layer after mmc_remove_host returns.
>
>
That shouldn't be possible. Are you using the block queue fixes I wrote?
Otherwise you will get problems like this.
Basically, when you call mmc_host_remove(), it will remove all card
devices. That shouldn't complete until all card drivers have released
control of the card. At that point there is no one else accessing the
device. If you see something else, then we have a bug somewhere.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
> That shouldn't be possible. Are you using the block queue fixes I wrote?
> Otherwise you will get problems like this.
>
> Basically, when you call mmc_host_remove(), it will remove all card
> devices. That shouldn't complete until all card drivers have released
> control of the card. At that point there is no one else accessing the
> device. If you see something else, then we have a bug somewhere.
>
Indeed, I may be out of sync on this. Simply, I have this rather ugly hack in the tifm_sd remove
code which I was forced to add because of the issue in question.
I'll do some tests with newer kernels then.
____________________________________________________________________________________
Now that's room service! Choose from over 150,000 hotels
in 45,000 destinations on Yahoo! Travel to find your fit.
http://farechase.yahoo.com/promo-generic-14795097
Alex Dubov wrote:
>
> Indeed, I may be out of sync on this. Simply, I have this rather ugly hack in the tifm_sd remove
> code which I was forced to add because of the issue in question.
> I'll do some tests with newer kernels then.
>
>
Please do. And if you see a problem, please try to figure out who it is
accessing the device.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org