login
Search
From: Aries Lee <arieslee@jmicron.com>
Some old MMC devices fail with the 4/8 bits the driver tries to use
exclusively. This patch adds a test for the given bus setup and falls
back to the lower bit mode (until 1-bit mode) when the test fails.
[Major rework and refactoring by tiwai]
[Quirk addition and many fixes by prakity]
v1->v2:
- Rebased to the code with DDR support, set DDR bit properly
- Return always error when bus-switching fallback failed
- Define MMC_BUS_TEST_{R|W} in linux/mmc/mmc.h
- Add quirk MMC_CAP_BUS_WIDTH_TEST -- default not used for compatibility
- Ignore errors on BUS_TEST_W -- improves chances test will work
Signed-off-by: Aries Lee <arieslee@jmicron.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Philip Rakity <prakity@marvell.com>
Tested-by: Philip Rakity <prakity@marvell.com>
---
Chris, this is a revised version of the patch. Philip merged his patch
into this version and tested. Please apply.
drivers/mmc/core/mmc.c | 76 ++++++++++++++++++++------------
drivers/mmc/core/mmc_ops.c | 102 ++++++++++++++++++++++++++++++++++++++++++++
drivers/mmc/core/mmc_ops.h | 1 +
drivers/mmc/host/sdhci.c | 7 +++-
drivers/mmc/host/sdhci.h | 1 +
include/linux/mmc/host.h | 1 +
include/linux/mmc/mmc.h | 2 +
7 files changed, 160 insertions(+), 30 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 77f93c3..3d51949 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -534,39 +534,57 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
*/
if ((card->csd.mmca_vsn >= CSD_SPEC_VER_4) &&
(host->caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA))) {
- unsigned ext_csd_bit, bus_width;
-
- if (host->caps & MMC_CAP_8_BIT_DATA) {
- if (ddr)
- ext_csd_bit = EXT_CSD_DDR_BUS_WIDTH_8;
- else
- ext_csd_bit = EXT_CSD_BUS_WIDTH_8;
- bus_width = MMC_BUS_WIDTH_8;
- } else {
- if (ddr)
- ext_csd_bit = ...Let's enable the new bus-width tests on SDHCI hosts unless 1-bit data
is set forcibly.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
This is a patch to be applied after
"mmc: Test bus-width for old MMC devices (v2)"
drivers/mmc/host/sdhci.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0502738..51f5209 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1888,8 +1888,10 @@ int sdhci_add_host(struct sdhci_host *host)
* their platform code before calling sdhci_add_host(), and we
* won't assume 8-bit width for hosts without that CAP.
*/
- if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
+ if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA)) {
mmc->caps |= MMC_CAP_4_BIT_DATA;
+ mmc->caps |= MMC_CAP_BUS_WIDTH_TEST;
+ }
if (caps & SDHCI_CAN_DO_HISPD)
mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
--
1.7.3.1
--
At Wed, 15 Dec 2010 14:17:27 +0100, Please drop this patch. After a short discussion with Philip, we concluded that it's safer to enable in individual platform side. I'll send a patch for enabling the feature for JMicron chips later again. thanks, --
Signed-off-by: Takashi Iwai <tiwai@suse.de> --- This is a patch to be applied after "mmc: Test bus-width for old MMC devices (v2)" and a replacement for the previous "mmc: Enable bus-width tests on SDHCI host" patch. Instead of enabling all for sdhci, this one turns on the feature only for JMicron, just to be safer. drivers/mmc/host/sdhci-pci.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c index d2638ff..0dc905b 100644 --- a/drivers/mmc/host/sdhci-pci.c +++ b/drivers/mmc/host/sdhci-pci.c @@ -381,6 +381,8 @@ static int jmicron_probe_slot(struct sdhci_pci_slot *slot) slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD) jmicron_enable_mmc(slot->host, 1); + slot->host->mmc->caps |= MMC_CAP_BUS_WIDTH_TEST; + return 0; } -- 1.7.3.1 --
Hi Takashi, Thanks, pushed to mmc-next. (Do others on the list agree that bus-width testing should be whitelisted per-host like this?) -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child --
Hi Takashi, This looks good, but adds a warning: drivers/mmc/core/mmc.c: In function ‘mmc_init_card’: drivers/mmc/core/mmc.c:547: warning: ‘bus_width’ may be used uninitialized in this function Thanks, -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child --
Chris,
It is not possible for bus_width to be not initialized. This would imply ARRAY_SIZE(bus_widths) is 1. Certainly not true.
We could just initialize by changing
+ unsigned idx, bus_width;
to
+ unsigned idx, bus_width = 0;
I wonder what compiler are you using so we can avoid this issue in future.
Philip
+ static unsigned ext_csd_bits[][2] = {
+ { EXT_CSD_BUS_WIDTH_8, EXT_CSD_DDR_BUS_WIDTH_8 },
+ { EXT_CSD_BUS_WIDTH_4, EXT_CSD_DDR_BUS_WIDTH_4 },
+ { EXT_CSD_BUS_WIDTH_1, EXT_CSD_BUS_WIDTH_1 },
+ };
+ static unsigned bus_widths[] = {
+ MMC_BUS_WIDTH_8,
+ MMC_BUS_WIDTH_4,
+ MMC_BUS_WIDTH_1
+ };
+ unsigned idx, bus_width;
+
+ if (host->caps & MMC_CAP_8_BIT_DATA)
+ idx = 0;
+ else
+ idx = 1;
+ for (; idx < ARRAY_SIZE(bus_widths); idx++) {
+ bus_width = bus_widths[idx];
+ if (bus_width == MMC_BUS_WIDTH_1)
+ ddr = 0; /* no DDR for 1-bit width */
--
Hi Philip, Ah, good point -- I was building with a gcc 4.1.2 ARM cross-compiler, and using a gcc 4.5.1 cross-build instead avoids the warning. Thanks, -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child --
At Fri, 17 Dec 2010 03:43:42 +0000, Well, this is always a gray-zone question. One prefers fixing it either via uninitialized_var() or by setting a bogus value. But, this would cover any possible real bug in future. Thus another prefers ignoring it, because it's just a compiler bug (mostly of old gcc). After all, it's up to maintainer, so take as you like :) thanks, --
Could you help adding this modification?
We found error happen since bus_width is not set at these condition:
1. ddr=0
2. not set MMC_CAP_BUS_WIDTH_TEST
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1d8409f..86cac0d 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -586,7 +586,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
} else if (ddr) {
mmc_card_set_ddr_mode(card);
mmc_set_bus_width_ddr(card->host, bus_width, ddr);
- }
+
+ } else
+ mmc_set_bus_width(card->host,
+ bus_widths[idx]);
+
}
if (!oldcard)
--
Can you please try this diff and see if it works for you.
Will do formal patch after your testing. What card is failing ?
Please let me know the manufacturing information so can add card to my test suite.
Philip
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1d8409f..77072c8 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -558,6 +558,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
EXT_CSD_BUS_WIDTH,
ext_csd_bits[idx][0]);
if (!err) {
+ mmc_set_bus_width_ddr(card->host,
+ bus_width, MMC_SDR_MODE);
/*
* If controller can't handle bus width test,
* use the highest bus width to maintain
@@ -565,8 +567,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
*/
if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
break;
- mmc_set_bus_width_ddr(card->host,
- bus_width, MMC_SDR_MODE);
err = mmc_bus_test(card, bus_width);
if (!err)
break;
@@ -586,7 +586,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
} else if (ddr) {
mmc_card_set_ddr_mode(card);
mmc_set_bus_width_ddr(card->host, bus_width, ddr);
- }
+ } else
+ mmc_set_bus_width (card->host, bus_width);
}
if (!oldcard)
Philip
--
Zhangfei,
Reproduced the problem on mmp2. Fix enclosed. Slight change from Zhangfei
original suggestion.
Please confirm by adding tested-by.
Philip
From 8b35e007d67dd593ff5e8a07b564c438e8303aae Mon Sep 17 00:00:00 2001
From: Philip Rakity <prakity@marvell.com>
Date: Tue, 21 Dec 2010 11:29:33 -0800
Subject: [PATCH] mmc: fix regression testing bus width when CAP not defined
First reported by Zhangfei Gao.
set mmc bus width before checking for CAP for mmc bus width testing.
set bus width when card is not ddr
Signed-off-by: Philip Rakity <prakity@marvell.com>
Tested-by: Philip Rakity
---
drivers/mmc/core/mmc.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1d8409f..3a2905f 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -558,6 +558,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
EXT_CSD_BUS_WIDTH,
ext_csd_bits[idx][0]);
if (!err) {
+ mmc_set_bus_width_ddr(card->host,
+ bus_width, MMC_SDR_MODE);
/*
* If controller can't handle bus width test,
* use the highest bus width to maintain
@@ -565,8 +567,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
*/
if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
break;
- mmc_set_bus_width_ddr(card->host,
- bus_width, MMC_SDR_MODE);
err = mmc_bus_test(card, bus_width);
if (!err)
break;
@@ -586,7 +586,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
} else if (ddr) {
mmc_card_set_ddr_mode(card);
mmc_set_bus_width_ddr(card->host, bus_width, ddr);
- }
+ } else
+ mmc_set_bus_width (card->host, bus_width);
}
if (!oldcard)
--
1.6.0.4
--
Test OK, Curious why move here, then mmc_set_bus_width_ddr is called twice in fact when ddr=0 && (!(host->caps & MMC_CAP_BUS_WIDTH_TEST)), though not impact function. --
At Tue, 21 Dec 2010 22:56:32 -0500,
Right. How about the patch below? This one just moves the call
before caps test, so it's simpler (and avoiding double calls).
thanks,
Takashi
===
From b66b9704f8d2fefa402741fb17949224b2766b4f Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Wed, 22 Dec 2010 09:54:33 +0100
Subject: [PATCH] mmc: fix mmc_set_bus_width_ddr() call without bus-width-test cap
With the bus-width test patch, mmc_set_bus_width*() isn't called properly
when the driver doesn't set MMC_CAP_BUS_WIDTH and no DDR mode.
This patch fixes the regression by moving the call up before the cap test.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/mmc/core/mmc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1d8409f..c86dd73 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -558,6 +558,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
EXT_CSD_BUS_WIDTH,
ext_csd_bits[idx][0]);
if (!err) {
+ mmc_set_bus_width_ddr(card->host,
+ bus_width, MMC_SDR_MODE);
/*
* If controller can't handle bus width test,
* use the highest bus width to maintain
@@ -565,8 +567,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
*/
if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
break;
- mmc_set_bus_width_ddr(card->host,
- bus_width, MMC_SDR_MODE);
err = mmc_bus_test(card, bus_width);
if (!err)
break;
--
1.7.3.4
--
It's also works, solve (ddr=0 & (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))) issue. Is it possible to merge to original patch. --
Hi Takashi, Thanks, I've pushed this fix to mmc-next now. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child --
| Jesse Barnes | Re: [stable] [BUG][PATCH] cpqphp: fix kernel NULL pointer dereference |
| Greg KH | [003/136] p54usb: add Zcomax XG-705A usbid |
| Magnus Damm | [PATCH 03/07] ARM: Use shared GIC entry macros on Realview |
| Oliver Neukum | Re: [Bug #13682] The webcam stopped working when upgrading from 2.6.29 to 2.6.30 |
| Martin Schwidefsky | Re: [PATCH] optimized ktime_get[_ts] for GENERIC_TIME=y |
git: | |
| Junio C Hamano | Re: Some advanced index playing |
| Jeff King | Re: confusion over the new branch and merge config |
| Robin Rosenberg | Re: cvs2svn conversion directly to git ready for experimentation |
| Linus Torvalds | git binary size... |
| Ævar Arnfjörð Bjarmason | Re: Challenge with Git-Bash |