login
Search
Hi, this is a patch series to add the support for the new JMicron 388 SD/MMC controller. The first patch is to add the support for the new PCI device, including the quirks to handle different voltages per medium. The second patch is for improving the handling of different bus- widths dynamically. thanks, Takashi --
JMicron 388 SD/MMC combo controller supports the 1.8V low-voltage for SD, but MMC doesn't work with the low-voltage, resulting in an error at probing. This patch adds the support for multiple voltage mask per device type, so that SD works with 1.8V while MMC forces 3.3V. Here new ocr_avail_* fields for each device are introduced, so that the actual OCR mask is switched dynamically. Also, the restriction of low-voltage in core/sd.c is removed when the bit is allowed explicitly via ocr_avail_sd mask. This patch was rewritten from scratch based on Aries' original code. Signed-off-by: Aries Lee <arieslee@jmicron.com> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/mmc/core/mmc.c | 2 ++ drivers/mmc/core/sd.c | 5 ++++- drivers/mmc/core/sdio.c | 2 ++ drivers/mmc/host/sdhci-pci.c | 42 ++++++++++++++++++++++++++++++++++++------ drivers/mmc/host/sdhci.c | 11 +++++++++++ drivers/mmc/host/sdhci.h | 2 ++ include/linux/mmc/host.h | 3 +++ include/linux/pci_ids.h | 2 ++ 8 files changed, 62 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 6909a54..e81e6fe 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -703,6 +703,8 @@ int mmc_attach_mmc(struct mmc_host *host, u32 ocr) WARN_ON(!host->claimed); mmc_attach_bus_ops(host); + if (host->ocr_avail_mmc) + host->ocr_avail = host->ocr_avail_mmc; /* * We need to get OCR a different way for SPI. diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0f52410..7f25b9c 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -768,6 +768,8 @@ int mmc_attach_sd(struct mmc_host *host, u32 ocr) WARN_ON(!host->claimed); mmc_sd_attach_bus_ops(host); + if (host->ocr_avail_sd) + host->ocr_avail = host->ocr_avail_sd; /* * We need to get OCR a different way for SPI. @@ -791,7 +793,8 @@ int mmc_attach_sd(struct mmc_host *host, u32 ocr) ocr &= ~0x7F; ...
Can't we handle this more generic? Like setting ocr_avail_* from sdhci-pci.c somehow (just brainstorming, didn't have a deeper look)? I'd like to avoid new quirks as much as possible. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
At Wed, 24 Nov 2010 10:59:35 +0100, OK, what about the one below? thanks, Takashi === From 7521d3ccdd3373598b91125c049e3f0dfdeb2598 Mon Sep 17 00:00:00 2001 From: Takashi Iwai <tiwai@suse.de> Date: Tue, 2 Nov 2010 09:10:20 +0100 Subject: [PATCH 1/2] mmc: Add support for JMicron 388 SD/MMC controller (v2) JMicron 388 SD/MMC combo controller supports the 1.8V low-voltage for SD, but MMC doesn't work with the low-voltage, resulting in an error at probing. This patch adds the support for multiple voltage mask per device type, so that SD works with 1.8V while MMC forces 3.3V. Here new ocr_avail_* fields for each device are introduced, so that the actual OCR mask is switched dynamically. Also, the restriction of low-voltage in core/sd.c is removed when the bit is allowed explicitly via ocr_avail_sd mask. This patch was rewritten from scratch based on Aries' original code. Signed-off-by: Aries Lee <arieslee@jmicron.com> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- v2: Keep ocr_avail_* in sdhci_host to avoid yet another quirk bit drivers/mmc/core/mmc.c | 2 + drivers/mmc/core/sd.c | 5 +++- drivers/mmc/core/sdio.c | 2 + drivers/mmc/host/sdhci-pci.c | 47 ++++++++++++++++++++++++++++++++++++----- drivers/mmc/host/sdhci.c | 23 ++++++++++++++++---- drivers/mmc/host/sdhci.h | 4 +++ include/linux/mmc/host.h | 3 ++ include/linux/pci_ids.h | 2 + 8 files changed, 76 insertions(+), 12 deletions(-) diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 6909a54..e81e6fe 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -703,6 +703,8 @@ int mmc_attach_mmc(struct mmc_host *host, u32 ocr) WARN_ON(!host->claimed); mmc_attach_bus_ops(host); + if (host->ocr_avail_mmc) + host->ocr_avail = host->ocr_avail_mmc; /* * We need to get OCR a different way for SPI. diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0f52410..7f25b9c 100644 --- ...
Looks better to me. (sadly, still from a glimpse, no thorough review) Thanks! -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
At Wed, 24 Nov 2010 23:04:25 +0100, Chris, any chance to take a look at this? thanks, Takashi --
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]
Signed-off-by: Aries Lee <arieslee@jmicron.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/mmc/core/mmc.c | 49 ++++++++++++---------
drivers/mmc/core/mmc_ops.c | 102 ++++++++++++++++++++++++++++++++++++++++++++
drivers/mmc/core/mmc_ops.h | 1 +
3 files changed, 131 insertions(+), 21 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index e81e6fe..5d8b4b2 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -507,29 +507,36 @@ 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) {
- ext_csd_bit = EXT_CSD_BUS_WIDTH_8;
- bus_width = MMC_BUS_WIDTH_8;
- } else {
- ext_csd_bit = EXT_CSD_BUS_WIDTH_4;
- bus_width = MMC_BUS_WIDTH_4;
+ static unsigned ext_csd_bits[] = {
+ EXT_CSD_BUS_WIDTH_8,
+ EXT_CSD_BUS_WIDTH_4,
+ EXT_CSD_BUS_WIDTH_1
+ };
+ static unsigned bus_widths[] = {
+ MMC_BUS_WIDTH_8,
+ MMC_BUS_WIDTH_4,
+ MMC_BUS_WIDTH_1
+ };
+ unsigned idx;
+
+ if (host->caps & MMC_CAP_8_BIT_DATA)
+ idx = 0;
+ else
+ idx = 1;
+ for (; idx < ARRAY_SIZE(bus_widths); idx++) {
+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_BUS_WIDTH, ext_csd_bits[idx]);
+ if (!err) {
+ mmc_set_bus_width(card->host, bus_widths[idx]);
+ err = mmc_bus_test(card, bus_widths[idx]);
+ if (!err)
+ break;
+ }
}
-
- err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BUS_WIDTH, ext_csd_bit);
-
- if (err && err != -EBADMSG)
- goto free_card;
-
if (err) ...Hi Takashi, Philip, We have two independent patches for performing MMC bus-width testing now: https://patchwork.kernel.org/patch/351781/ https://patchwork.kernel.org/patch/361702/ I'm planning on taking Takashi's since it looks a little cleaner; Philip, please could you take a look at Takashi's patch and add anything you think should be present from your own patch as a new incremental patch? Thanks very much, - Chris. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child --
Hi Chris, At Sun, 5 Dec 2010 04:02:24 +0000, One missing thing in my (originally Aries') patch is the quirk bit to enable/disable the bus-width test. In Philip's latest patch, the default is off. I'm also not sure whether this bus-width test should be enabled as default. I guess it's better for performance, so I'd vote for turning on as default. But, having a quirk for turning off would be safer for working around old hardware problems, of course. thanks, Takashi --
Hi Takashi, Philip, Sounds good, although we're trying hard not to add new quirk bits. I don't see a way around doing that here, though. We've got plenty of time until the .38 release -- let's turn it on by default and test during .38-rc, and we can revisit whether to leave it on closer to the release. It's not as simple as "turning it on could cause problems so let's not do that", because we're *already* seeing problems from things like enabling 8-bit width on setups that don't support it. Thanks! -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child --
Chris, One other difference I have been told between the patches but NOT verified is a) Takasi code was not tested with Dual Data Rate b) Performance is not an issue since we are dealing with uSeconds and the code is called once when mmc is detected. c) I will adapt my code to what Takahi did over the next few days and post changes to the CAP d) The work was independent and done at the same time !! (amazing) --
if (data.error && (data.error != -EILSEQ)) Could you add code here to ignore CRC error of CMD14. According to spec, CRC bits from card are optional in CMD14, and it is ignored by host. --
At Tue, 14 Dec 2010 23:03:42 -0500, Philip corrected my patch and added such a check (in sdhci.c side). I'm going to resend the revised patch soon. thanks, Takashi --