mmc: Fix handling of response types in imxmmc and tifm drivers
Subsystems (2)
PATCH 1/1 mmc: Fix handling of response types in imxmmc and tifm drivers
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.
--- 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(-)
drivers/mmc/imxmmc.c +0 -3
@@ -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 ; }
drivers/mmc/omap.c +1 -1
@@ -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 ):
drivers/mmc/pxamci.c +1 -1
@@ -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 ):
drivers/mmc/tifm_sd.c +0 -3
@@ -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 (); }
include/linux/mmc/mmc.h +1 -1
@@ -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,
Quoted 7 lines · Philip Langdale
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
Quoted 6 lines · Pavel Pisa
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.
Quoted 8 lines
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
Signature · 5 lines
-- Pierre Ossman Linux kernel, MMC maintainer http://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org
mmc: correct semantics of the mmc_host_remove
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
Re: mmc: correct semantics of the mmc_host_remove
Quoted 10 lines · Alex Dubov
Greetings.
It appears to me that under certain circumstances mmc layer will issue requests to the host after
mmc_host_removereturns. This happens, for example, intifm_sddriver becausemmc_hostmay 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 aftermmc_remove_hostbut beforemmc_free_host. I think the correct way to handle the situation is to ensure thatmmc_hostis never accessed by the mmc layer aftermmc_remove_hostreturns.
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
Signature · 5 lines
-- 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: correct semantics of the mmc_host_remove
Quoted 8 lines
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
Re: mmc: correct semantics of the mmc_host_remove
Quoted 6 lines · Alex Dubov
Indeed, I may be out of sync on this. Simply, I have this rather ugly hack in the
tifm_sdremove 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
Signature · 5 lines
-- Pierre Ossman Linux kernel, MMC maintainer http://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org