[PATCH 0/2] mmc: Add support for JMicron 388 SD/MMC controller

[PATCH 0/2] mmc: Add support for JMicron 388 SD/MMC controller

[Takashi Iwai]

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

[PATCH 1/2] mmc: Add support for JMicron 388 SD/MMC controller

[Takashi Iwai]

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;
 	}
 
-	if (ocr & MMC_VDD_165_195) {
+	if ((ocr & MMC_VDD_165_195) &&
+	    !(host->ocr_avail_sd & MMC_VDD_165_195)) {
 		printk(KERN_WARNING "%s: SD card claims to support the "
 		       "incompletely defined 'low voltage range'. This "
 		       "will be ignored.\n", mmc_hostname(host));
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index f332c52..f7b89e0 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -668,6 +668,8 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	WARN_ON(!host->claimed);
 
 	mmc_attach_bus(host, &mmc_sdio_ops);
+	if (host->ocr_avail_sdio)
+		host->ocr_avail = host->ocr_avail_sdio;
 
 	/*
 	 * Sanity check the voltages that the card claims to
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index e8aa99d..e2af522 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -173,6 +173,7 @@ static int jmicron_pmos(struct sdhci_pci_chip *chip, int on)
 static int jmicron_probe(struct sdhci_pci_chip *chip)
 {
 	int ret;
+	u16 mmcdev = 0;
 
 	if (chip->pdev->revision == 0) {
 		chip->quirks |= SDHCI_QUIRK_32BIT_DMA_ADDR |
@@ -194,12 +195,17 @@ static int jmicron_probe(struct sdhci_pci_chip *chip)
 	 * 2. The MMC interface has a lower subfunction number
 	 *    than the SD interface.
 	 */
-	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_SD) {
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_SD)
+		mmcdev = PCI_DEVICE_ID_JMICRON_JMB38X_MMC;
+	else if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_SD)
+		mmcdev = PCI_DEVICE_ID_JMICRON_JMB388_ESD;
+
+	if (mmcdev) {
 		struct pci_dev *sd_dev;
 
 		sd_dev = NULL;
 		while ((sd_dev = pci_get_device(PCI_VENDOR_ID_JMICRON,
-			PCI_DEVICE_ID_JMICRON_JMB38X_MMC, sd_dev)) != NULL) {
+						mmcdev, sd_dev)) != NULL) {
 			if ((PCI_SLOT(chip->pdev->devfn) ==
 				PCI_SLOT(sd_dev->devfn)) &&
 				(chip->pdev->bus == sd_dev->bus))
@@ -224,6 +230,10 @@ static int jmicron_probe(struct sdhci_pci_chip *chip)
 		return ret;
 	}
 
+	/* JM388 MMC doesn't support 1.8V while SD supports it */
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
+		chip->quirks |= SDHCI_QUIRK_MMC_VDD_180;
+
 	return 0;
 }
 
@@ -263,7 +273,8 @@ static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
 	 * The secondary interface requires a bit set to get the
 	 * interrupts.
 	 */
-	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC)
+	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
 		jmicron_enable_mmc(slot->host, 1);
 
 	return 0;
@@ -274,7 +285,8 @@ static void jmicron_remove_slot(struct sdhci_pci_slot *slot, int dead)
 	if (dead)
 		return;
 
-	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC)
+	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
 		jmicron_enable_mmc(slot->host, 0);
 }
 
@@ -282,7 +294,8 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip, pm_message_t state)
 {
 	int i;
 
-	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC) {
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD) {
 		for (i = 0;i < chip->num_slots;i++)
 			jmicron_enable_mmc(chip->slots[i]->host, 0);
 	}
@@ -294,7 +307,8 @@ static int jmicron_resume(struct sdhci_pci_chip *chip)
 {
 	int ret, i;
 
-	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC) {
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD) {
 		for (i = 0;i < chip->num_slots;i++)
 			jmicron_enable_mmc(chip->slots[i]->host, 1);
 	}
@@ -479,6 +493,22 @@ static const struct pci_device_id pci_ids[] __devinitdata = {
 	},
 
 	{
+		.vendor		= PCI_VENDOR_ID_JMICRON,
+		.device		= PCI_DEVICE_ID_JMICRON_JMB388_SD,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.driver_data	= (kernel_ulong_t)&sdhci_jmicron,
+	},
+
+	{
+		.vendor		= PCI_VENDOR_ID_JMICRON,
+		.device		= PCI_DEVICE_ID_JMICRON_JMB388_ESD,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.driver_data	= (kernel_ulong_t)&sdhci_jmicron,
+	},
+
+	{
 		.vendor		= PCI_VENDOR_ID_SYSKONNECT,
 		.device		= 0x8000,
 		.subvendor	= PCI_ANY_ID,
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 401527d..96e32b1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1829,6 +1829,10 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
+	/* normal SD controllers don't support 1.8V; exlcude here */
+	if (!(host->quirks & SDHCI_QUIRK_MMC_VDD_180))
+		caps &= ~SDHCI_CAN_VDD_180;
+
 	mmc->ocr_avail = 0;
 	if (caps & SDHCI_CAN_VDD_330)
 		mmc->ocr_avail |= MMC_VDD_32_33|MMC_VDD_33_34;
@@ -1837,6 +1841,13 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps & SDHCI_CAN_VDD_180)
 		mmc->ocr_avail |= MMC_VDD_165_195;
 
+	mmc->ocr_avail_sdio = mmc->ocr_avail;
+	mmc->ocr_avail_sd = mmc->ocr_avail;
+	mmc->ocr_avail_mmc = mmc->ocr_avail;
+	/* JMicron 388 combo supports 1.8V for SD but not for MMC */
+	if (host->quirks & SDHCI_QUIRK_MMC_VDD_180)
+		mmc->ocr_avail_mmc &= ~MMC_VDD_165_195;
+
 	if (mmc->ocr_avail == 0) {
 		printk(KERN_ERR "%s: Hardware doesn't report any "
 			"support voltages.\n", mmc_hostname(mmc));
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index d316bc7..cb73651 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -247,6 +247,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<28)
 /* Controller doesn't have HISPD bit field in HI-SPEED SD card */
 #define SDHCI_QUIRK_NO_HISPD_BIT			(1<<29)
+/* Controller doesn't support VDD 180 for MMC (for SD/MMC combo chips) */
+#define SDHCI_QUIRK_MMC_VDD_180				(1<<30)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 1575b52..5bb3337 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -124,6 +124,9 @@ struct mmc_host {
 	unsigned int		f_min;
 	unsigned int		f_max;
 	u32			ocr_avail;
+	u32			ocr_avail_sdio;	/* SDIO-specific OCR */
+	u32			ocr_avail_sd;	/* SD-specific OCR */
+	u32			ocr_avail_mmc;	/* MMC-specific OCR */
 	struct notifier_block	pm_notify;
 
 #define MMC_VDD_165_195		0x00000080	/* VDD voltage 1.65 - 1.95 */
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index b4c3d1b..40c5bc7 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2358,6 +2358,8 @@
 #define PCI_DEVICE_ID_JMICRON_JMB38X_SD	0x2381
 #define PCI_DEVICE_ID_JMICRON_JMB38X_MMC 0x2382
 #define PCI_DEVICE_ID_JMICRON_JMB38X_MS	0x2383
+#define PCI_DEVICE_ID_JMICRON_JMB388_SD	0x2391
+#define PCI_DEVICE_ID_JMICRON_JMB388_ESD 0x2392
 
 #define PCI_VENDOR_ID_KORENIX		0x1982
 #define PCI_DEVICE_ID_KORENIX_JETCARDF0	0x1600
-- 
1.7.3.1

Re: [PATCH 1/2] mmc: Add support for JMicron 388 SD/MMC controller

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Wed, Nov 24, 2010 at 07:21:13AM +0100, Takashi Iwai wrote:

> +/* Controller doesn't support VDD 180 for MMC (for SD/MMC combo chips) */
> +#define SDHCI_QUIRK_MMC_VDD_180				(1<<30)

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/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/2] mmc: Add support for JMicron 388 SD/MMC controller

[Takashi Iwai]

At Wed, 24 Nov 2010 10:59:35 +0100,
Wolfram Sang wrote:
> 
> On Wed, Nov 24, 2010 at 07:21:13AM +0100, Takashi Iwai wrote:
> 
> > +/* Controller doesn't support VDD 180 for MMC (for SD/MMC combo chips) */
> > +#define SDHCI_QUIRK_MMC_VDD_180				(1<<30)
> 
> 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.

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
--- 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;
 	}
 
-	if (ocr & MMC_VDD_165_195) {
+	if ((ocr & MMC_VDD_165_195) &&
+	    !(host->ocr_avail_sd & MMC_VDD_165_195)) {
 		printk(KERN_WARNING "%s: SD card claims to support the "
 		       "incompletely defined 'low voltage range'. This "
 		       "will be ignored.\n", mmc_hostname(host));
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index f332c52..f7b89e0 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -668,6 +668,8 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	WARN_ON(!host->claimed);
 
 	mmc_attach_bus(host, &mmc_sdio_ops);
+	if (host->ocr_avail_sdio)
+		host->ocr_avail = host->ocr_avail_sdio;
 
 	/*
 	 * Sanity check the voltages that the card claims to
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index e8aa99d..4f9f4b7 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -173,6 +173,7 @@ static int jmicron_pmos(struct sdhci_pci_chip *chip, int on)
 static int jmicron_probe(struct sdhci_pci_chip *chip)
 {
 	int ret;
+	u16 mmcdev = 0;
 
 	if (chip->pdev->revision == 0) {
 		chip->quirks |= SDHCI_QUIRK_32BIT_DMA_ADDR |
@@ -194,12 +195,17 @@ static int jmicron_probe(struct sdhci_pci_chip *chip)
 	 * 2. The MMC interface has a lower subfunction number
 	 *    than the SD interface.
 	 */
-	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_SD) {
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_SD)
+		mmcdev = PCI_DEVICE_ID_JMICRON_JMB38X_MMC;
+	else if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_SD)
+		mmcdev = PCI_DEVICE_ID_JMICRON_JMB388_ESD;
+
+	if (mmcdev) {
 		struct pci_dev *sd_dev;
 
 		sd_dev = NULL;
 		while ((sd_dev = pci_get_device(PCI_VENDOR_ID_JMICRON,
-			PCI_DEVICE_ID_JMICRON_JMB38X_MMC, sd_dev)) != NULL) {
+						mmcdev, sd_dev)) != NULL) {
 			if ((PCI_SLOT(chip->pdev->devfn) ==
 				PCI_SLOT(sd_dev->devfn)) &&
 				(chip->pdev->bus == sd_dev->bus))
@@ -259,11 +265,21 @@ static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
 			slot->host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
 	}
 
+	/* JM388 MMC doesn't support 1.8V while SD supports it */
+	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD) {
+		slot->host->ocr_avail_sd = MMC_VDD_32_33 | MMC_VDD_33_34 |
+			MMC_VDD_29_30 | MMC_VDD_30_31 |
+			MMC_VDD_165_195; /* allow 1.8V */
+		slot->host->ocr_avail_mmc = MMC_VDD_32_33 | MMC_VDD_33_34 |
+			MMC_VDD_29_30 | MMC_VDD_30_31; /* no 1.8V for MMC */
+	}
+
 	/*
 	 * The secondary interface requires a bit set to get the
 	 * interrupts.
 	 */
-	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC)
+	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
 		jmicron_enable_mmc(slot->host, 1);
 
 	return 0;
@@ -274,7 +290,8 @@ static void jmicron_remove_slot(struct sdhci_pci_slot *slot, int dead)
 	if (dead)
 		return;
 
-	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC)
+	if (slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    slot->chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD)
 		jmicron_enable_mmc(slot->host, 0);
 }
 
@@ -282,7 +299,8 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip, pm_message_t state)
 {
 	int i;
 
-	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC) {
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD) {
 		for (i = 0;i < chip->num_slots;i++)
 			jmicron_enable_mmc(chip->slots[i]->host, 0);
 	}
@@ -294,7 +312,8 @@ static int jmicron_resume(struct sdhci_pci_chip *chip)
 {
 	int ret, i;
 
-	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC) {
+	if (chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB38X_MMC ||
+	    chip->pdev->device == PCI_DEVICE_ID_JMICRON_JMB388_ESD) {
 		for (i = 0;i < chip->num_slots;i++)
 			jmicron_enable_mmc(chip->slots[i]->host, 1);
 	}
@@ -479,6 +498,22 @@ static const struct pci_device_id pci_ids[] __devinitdata = {
 	},
 
 	{
+		.vendor		= PCI_VENDOR_ID_JMICRON,
+		.device		= PCI_DEVICE_ID_JMICRON_JMB388_SD,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.driver_data	= (kernel_ulong_t)&sdhci_jmicron,
+	},
+
+	{
+		.vendor		= PCI_VENDOR_ID_JMICRON,
+		.device		= PCI_DEVICE_ID_JMICRON_JMB388_ESD,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.driver_data	= (kernel_ulong_t)&sdhci_jmicron,
+	},
+
+	{
 		.vendor		= PCI_VENDOR_ID_SYSKONNECT,
 		.device		= 0x8000,
 		.subvendor	= PCI_ANY_ID,
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 401527d..9b93ef2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1691,7 +1691,7 @@ EXPORT_SYMBOL_GPL(sdhci_alloc_host);
 int sdhci_add_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
-	unsigned int caps;
+	unsigned int caps, ocr_avail;
 	int ret;
 
 	WARN_ON(host == NULL);
@@ -1829,13 +1829,26 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
-	mmc->ocr_avail = 0;
+	ocr_avail = 0;
 	if (caps & SDHCI_CAN_VDD_330)
-		mmc->ocr_avail |= MMC_VDD_32_33|MMC_VDD_33_34;
+		ocr_avail |= MMC_VDD_32_33 | MMC_VDD_33_34;
 	if (caps & SDHCI_CAN_VDD_300)
-		mmc->ocr_avail |= MMC_VDD_29_30|MMC_VDD_30_31;
+		ocr_avail |= MMC_VDD_29_30 | MMC_VDD_30_31;
 	if (caps & SDHCI_CAN_VDD_180)
-		mmc->ocr_avail |= MMC_VDD_165_195;
+		ocr_avail |= MMC_VDD_165_195;
+
+	mmc->ocr_avail = ocr_avail;
+	mmc->ocr_avail_sdio = ocr_avail;
+	if (host->ocr_avail_sdio)
+		mmc->ocr_avail_sdio &= host->ocr_avail_sdio;
+	mmc->ocr_avail_sd = ocr_avail;
+	if (host->ocr_avail_sd)
+		mmc->ocr_avail_sd &= host->ocr_avail_sd;
+	else /* normal SD controllers don't support 1.8V */
+		mmc->ocr_avail_sd &= ~MMC_VDD_165_195;
+	mmc->ocr_avail_mmc = ocr_avail;
+	if (host->ocr_avail_mmc)
+		mmc->ocr_avail_sd &= host->ocr_avail_mmc;
 
 	if (mmc->ocr_avail == 0) {
 		printk(KERN_ERR "%s: Hardware doesn't report any "
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index d316bc7..d5f1405 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -303,6 +303,10 @@ struct sdhci_host {
 
 	unsigned int		caps;		/* Alternative capabilities */
 
+	unsigned int            ocr_avail_sdio;	/* OCR bit masks */
+	unsigned int            ocr_avail_sd;
+	unsigned int            ocr_avail_mmc;
+
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 1575b52..5bb3337 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -124,6 +124,9 @@ struct mmc_host {
 	unsigned int		f_min;
 	unsigned int		f_max;
 	u32			ocr_avail;
+	u32			ocr_avail_sdio;	/* SDIO-specific OCR */
+	u32			ocr_avail_sd;	/* SD-specific OCR */
+	u32			ocr_avail_mmc;	/* MMC-specific OCR */
 	struct notifier_block	pm_notify;
 
 #define MMC_VDD_165_195		0x00000080	/* VDD voltage 1.65 - 1.95 */
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index b4c3d1b..40c5bc7 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2358,6 +2358,8 @@
 #define PCI_DEVICE_ID_JMICRON_JMB38X_SD	0x2381
 #define PCI_DEVICE_ID_JMICRON_JMB38X_MMC 0x2382
 #define PCI_DEVICE_ID_JMICRON_JMB38X_MS	0x2383
+#define PCI_DEVICE_ID_JMICRON_JMB388_SD	0x2391
+#define PCI_DEVICE_ID_JMICRON_JMB388_ESD 0x2392
 
 #define PCI_VENDOR_ID_KORENIX		0x1982
 #define PCI_DEVICE_ID_KORENIX_JETCARDF0	0x1600
-- 
1.7.3.1

Re: [PATCH 1/2] mmc: Add support for JMicron 388 SD/MMC controller

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

On Wed, Nov 24, 2010 at 12:29:57PM +0100, Takashi Iwai wrote:
> At Wed, 24 Nov 2010 10:59:35 +0100,
> Wolfram Sang wrote:
> > 
> > On Wed, Nov 24, 2010 at 07:21:13AM +0100, Takashi Iwai wrote:
> > 
> > > +/* Controller doesn't support VDD 180 for MMC (for SD/MMC combo chips) */
> > > +#define SDHCI_QUIRK_MMC_VDD_180				(1<<30)
> > 
> > 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.
> 
> OK, what about the one below?

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/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/2] mmc: Add support for JMicron 388 SD/MMC controller

[Takashi Iwai]

At Wed, 24 Nov 2010 23:04:25 +0100,
Wolfram Sang wrote:
> 
> On Wed, Nov 24, 2010 at 12:29:57PM +0100, Takashi Iwai wrote:
> > At Wed, 24 Nov 2010 10:59:35 +0100,
> > Wolfram Sang wrote:
> > > 
> > > On Wed, Nov 24, 2010 at 07:21:13AM +0100, Takashi Iwai wrote:
> > > 
> > > > +/* Controller doesn't support VDD 180 for MMC (for SD/MMC combo chips) */
> > > > +#define SDHCI_QUIRK_MMC_VDD_180				(1<<30)
> > > 
> > > 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.
> > 
> > OK, what about the one below?
> 
> Looks better to me. (sadly, still from a glimpse, no thorough review)

Chris, any chance to take a look at this?


thanks,

Takashi

[PATCH 2/2] mmc: Test bus-width for old MMC devices

[Takashi Iwai]

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) {
-			printk(KERN_WARNING "%s: switch to bus width %d "
-			       "failed\n", mmc_hostname(card->host),
-			       1 << bus_width);
-			err = 0;
-		} else {
-			mmc_set_bus_width(card->host, bus_width);
+			printk(KERN_WARNING "%s: switch to bus width failed\n",
+			       mmc_hostname(card->host));
+			goto free_card;
 		}
 	}
 
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 326447c..f8f47f9 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -462,3 +462,105 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
 	return 0;
 }
 
+#define MMC_CMD_BUS_TEST_W		19
+#define MMC_CMD_BUS_TEST_R		14
+
+static int
+mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
+		  u8 len)
+{
+	struct mmc_request mrq;
+	struct mmc_command cmd;
+	struct mmc_data data;
+	struct scatterlist sg;
+	u8 *data_buf;
+	u8 *test_buf;
+	int i, err;
+	static u8 testdata_8bit[8] = { 0x55, 0xaa, 0, 0, 0, 0, 0, 0 };
+	static u8 testdata_4bit[4] = { 0x5a, 0, 0, 0 };
+
+	/* dma onto stack is unsafe/nonportable, but callers to this
+	 * routine normally provide temporary on-stack buffers ...
+	 */
+	data_buf = kmalloc(len, GFP_KERNEL);
+	if (!data_buf)
+		return -ENOMEM;
+
+	if (len == 8)
+		test_buf = testdata_8bit;
+	else if (len == 4)
+		test_buf = testdata_4bit;
+	else {
+		printk(KERN_ERR "%s: Invaild bus_width %d\n",
+		       mmc_hostname(host), len);
+		return -EINVAL;
+	}
+
+	if (opcode == MMC_CMD_BUS_TEST_W)
+		memcpy(data_buf, test_buf, len);
+
+	memset(&mrq, 0, sizeof(struct mmc_request));
+	memset(&cmd, 0, sizeof(struct mmc_command));
+	memset(&data, 0, sizeof(struct mmc_data));
+
+	mrq.cmd = &cmd;
+	mrq.data = &data;
+	cmd.opcode = opcode;
+	cmd.arg = 0;
+
+	/* NOTE HACK:  the MMC_RSP_SPI_R1 is always correct here, but we
+	 * rely on callers to never use this with "native" calls for reading
+	 * CSD or CID.  Native versions of those commands use the R2 type,
+	 * not R1 plus a data block.
+	 */
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+
+	data.blksz = len;
+	data.blocks = 1;
+	if (opcode == MMC_CMD_BUS_TEST_R)
+		data.flags = MMC_DATA_READ;
+	else
+		data.flags = MMC_DATA_WRITE;
+
+	data.sg = &sg;
+	data.sg_len = 1;
+	sg_init_one(&sg, data_buf, len);
+	mmc_wait_for_req(host, &mrq);
+	err = 0;
+	if (opcode == MMC_CMD_BUS_TEST_R) {
+		for (i = 0; i < len / 4; i++)
+			if ((test_buf[i] ^ data_buf[i]) != 0xff) {
+				err = -EIO;
+				break;
+			}
+	}
+	kfree(data_buf);
+
+	if (cmd.error)
+		return cmd.error;
+	if (data.error)
+		return data.error;
+
+	return err;
+}
+
+int mmc_bus_test(struct mmc_card *card, u8 bus_width)
+{
+	int err, width;
+
+	if (bus_width == MMC_BUS_WIDTH_8)
+		width = 8;
+	else if (bus_width == MMC_BUS_WIDTH_4)
+		width = 4;
+	else if (bus_width == MMC_BUS_WIDTH_1)
+		return 0; /* no need for test */
+	else
+		return -EINVAL;
+	err = mmc_send_bus_test(card, card->host, MMC_CMD_BUS_TEST_W, width);
+	if (err < 0)
+		return err;
+	err = mmc_send_bus_test(card, card->host, MMC_CMD_BUS_TEST_R, width);
+	return err;
+
+
+}
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 653eb8e..e6d44b8 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -26,6 +26,7 @@ int mmc_send_cid(struct mmc_host *host, u32 *cid);
 int mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp);
 int mmc_spi_set_crc(struct mmc_host *host, int use_crc);
 int mmc_card_sleepawake(struct mmc_host *host, int sleep);
+int mmc_bus_test(struct mmc_card *card, u8 bus_width);
 
 #endif
 
-- 
1.7.3.1

Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[Chris Ball]

Hi Takashi, Philip,

On Wed, Nov 24, 2010 at 07:21:14AM +0100, Takashi Iwai wrote:
> 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>

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

Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[Takashi Iwai]

Hi Chris,

At Sun, 5 Dec 2010 04:02:24 +0000,
Chris Ball wrote:
> 
> Hi Takashi, Philip,
> 
> On Wed, Nov 24, 2010 at 07:21:14AM +0100, Takashi Iwai wrote:
> > 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>
> 
> We have two independent patches for performing MMC bus-width testing now:
> 
>   https://patchwork.kernel.org/patch/351781/
>   https://patchwork.kernel.org/patch/361702/

Yeah, Philip and I discussed shortly in a mail thread after that.

> 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?

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

Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[Chris Ball]

Hi Takashi, Philip,

On Sun, Dec 05, 2010 at 11:50:40AM +0100, Takashi Iwai wrote:
> > 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?
> 
> 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.

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

Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[Philip Rakity]

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)


On Dec 5, 2010, at 10:40 AM, Chris Ball wrote:

> Hi Takashi, Philip,
> 
> On Sun, Dec 05, 2010 at 11:50:40AM +0100, Takashi Iwai wrote:
>>> 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?
>> 
>> 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.
> 
> 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

Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[zhangfei gao]

On Wed, Nov 24, 2010 at 1:21 AM, Takashi Iwai <tiwai@suse.de> wrote:
> 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) {
> -                       printk(KERN_WARNING "%s: switch to bus width %d "
> -                              "failed\n", mmc_hostname(card->host),
> -                              1 << bus_width);
> -                       err = 0;
> -               } else {
> -                       mmc_set_bus_width(card->host, bus_width);
> +                       printk(KERN_WARNING "%s: switch to bus width failed\n",
> +                              mmc_hostname(card->host));
> +                       goto free_card;
>                }
>        }
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 326447c..f8f47f9 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -462,3 +462,105 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
>        return 0;
>  }
>
> +#define MMC_CMD_BUS_TEST_W             19
> +#define MMC_CMD_BUS_TEST_R             14
> +
> +static int
> +mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> +                 u8 len)
> +{
> +       struct mmc_request mrq;
> +       struct mmc_command cmd;
> +       struct mmc_data data;
> +       struct scatterlist sg;
> +       u8 *data_buf;
> +       u8 *test_buf;
> +       int i, err;
> +       static u8 testdata_8bit[8] = { 0x55, 0xaa, 0, 0, 0, 0, 0, 0 };
> +       static u8 testdata_4bit[4] = { 0x5a, 0, 0, 0 };
> +
> +       /* dma onto stack is unsafe/nonportable, but callers to this
> +        * routine normally provide temporary on-stack buffers ...
> +        */
> +       data_buf = kmalloc(len, GFP_KERNEL);
> +       if (!data_buf)
> +               return -ENOMEM;
> +
> +       if (len == 8)
> +               test_buf = testdata_8bit;
> +       else if (len == 4)
> +               test_buf = testdata_4bit;
> +       else {
> +               printk(KERN_ERR "%s: Invaild bus_width %d\n",
> +                      mmc_hostname(host), len);
> +               return -EINVAL;
> +       }
> +
> +       if (opcode == MMC_CMD_BUS_TEST_W)
> +               memcpy(data_buf, test_buf, len);
> +
> +       memset(&mrq, 0, sizeof(struct mmc_request));
> +       memset(&cmd, 0, sizeof(struct mmc_command));
> +       memset(&data, 0, sizeof(struct mmc_data));
> +
> +       mrq.cmd = &cmd;
> +       mrq.data = &data;
> +       cmd.opcode = opcode;
> +       cmd.arg = 0;
> +
> +       /* NOTE HACK:  the MMC_RSP_SPI_R1 is always correct here, but we
> +        * rely on callers to never use this with "native" calls for reading
> +        * CSD or CID.  Native versions of those commands use the R2 type,
> +        * not R1 plus a data block.
> +        */
> +       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> +
> +       data.blksz = len;
> +       data.blocks = 1;
> +       if (opcode == MMC_CMD_BUS_TEST_R)
> +               data.flags = MMC_DATA_READ;
> +       else
> +               data.flags = MMC_DATA_WRITE;
> +
> +       data.sg = &sg;
> +       data.sg_len = 1;
> +       sg_init_one(&sg, data_buf, len);
> +       mmc_wait_for_req(host, &mrq);
> +       err = 0;
> +       if (opcode == MMC_CMD_BUS_TEST_R) {
> +               for (i = 0; i < len / 4; i++)
> +                       if ((test_buf[i] ^ data_buf[i]) != 0xff) {
> +                               err = -EIO;
> +                               break;
> +                       }
> +       }
> +       kfree(data_buf);
> +
> +       if (cmd.error)
> +               return cmd.error;
> +       if (data.error)

	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.
However some host still check and may get crc error here if card does not send.

> +               return data.error;
> +
> +       return err;
> +}
> +
> +int mmc_bus_test(struct mmc_card *card, u8 bus_width)
> +{
> +       int err, width;
> +
> +       if (bus_width == MMC_BUS_WIDTH_8)
> +               width = 8;
> +       else if (bus_width == MMC_BUS_WIDTH_4)
> +               width = 4;
> +       else if (bus_width == MMC_BUS_WIDTH_1)
> +               return 0; /* no need for test */
> +       else
> +               return -EINVAL;
> +       err = mmc_send_bus_test(card, card->host, MMC_CMD_BUS_TEST_W, width);
> +       if (err < 0)
> +               return err;
> +       err = mmc_send_bus_test(card, card->host, MMC_CMD_BUS_TEST_R, width);
> +       return err;
> +
> +
> +}
> diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
> index 653eb8e..e6d44b8 100644
> --- a/drivers/mmc/core/mmc_ops.h
> +++ b/drivers/mmc/core/mmc_ops.h
> @@ -26,6 +26,7 @@ int mmc_send_cid(struct mmc_host *host, u32 *cid);
>  int mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp);
>  int mmc_spi_set_crc(struct mmc_host *host, int use_crc);
>  int mmc_card_sleepawake(struct mmc_host *host, int sleep);
> +int mmc_bus_test(struct mmc_card *card, u8 bus_width);
>
>  #endif
>
> --
> 1.7.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[Takashi Iwai]

At Tue, 14 Dec 2010 23:03:42 -0500,
zhangfei gao wrote:
> 
> On Wed, Nov 24, 2010 at 1:21 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > 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) {
> > -                       printk(KERN_WARNING "%s: switch to bus width %d "
> > -                              "failed\n", mmc_hostname(card->host),
> > -                              1 << bus_width);
> > -                       err = 0;
> > -               } else {
> > -                       mmc_set_bus_width(card->host, bus_width);
> > +                       printk(KERN_WARNING "%s: switch to bus width failed\n",
> > +                              mmc_hostname(card->host));
> > +                       goto free_card;
> >                }
> >        }
> >
> > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> > index 326447c..f8f47f9 100644
> > --- a/drivers/mmc/core/mmc_ops.c
> > +++ b/drivers/mmc/core/mmc_ops.c
> > @@ -462,3 +462,105 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
> >        return 0;
> >  }
> >
> > +#define MMC_CMD_BUS_TEST_W             19
> > +#define MMC_CMD_BUS_TEST_R             14
> > +
> > +static int
> > +mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> > +                 u8 len)
> > +{
> > +       struct mmc_request mrq;
> > +       struct mmc_command cmd;
> > +       struct mmc_data data;
> > +       struct scatterlist sg;
> > +       u8 *data_buf;
> > +       u8 *test_buf;
> > +       int i, err;
> > +       static u8 testdata_8bit[8] = { 0x55, 0xaa, 0, 0, 0, 0, 0, 0 };
> > +       static u8 testdata_4bit[4] = { 0x5a, 0, 0, 0 };
> > +
> > +       /* dma onto stack is unsafe/nonportable, but callers to this
> > +        * routine normally provide temporary on-stack buffers ...
> > +        */
> > +       data_buf = kmalloc(len, GFP_KERNEL);
> > +       if (!data_buf)
> > +               return -ENOMEM;
> > +
> > +       if (len == 8)
> > +               test_buf = testdata_8bit;
> > +       else if (len == 4)
> > +               test_buf = testdata_4bit;
> > +       else {
> > +               printk(KERN_ERR "%s: Invaild bus_width %d\n",
> > +                      mmc_hostname(host), len);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (opcode == MMC_CMD_BUS_TEST_W)
> > +               memcpy(data_buf, test_buf, len);
> > +
> > +       memset(&mrq, 0, sizeof(struct mmc_request));
> > +       memset(&cmd, 0, sizeof(struct mmc_command));
> > +       memset(&data, 0, sizeof(struct mmc_data));
> > +
> > +       mrq.cmd = &cmd;
> > +       mrq.data = &data;
> > +       cmd.opcode = opcode;
> > +       cmd.arg = 0;
> > +
> > +       /* NOTE HACK:  the MMC_RSP_SPI_R1 is always correct here, but we
> > +        * rely on callers to never use this with "native" calls for reading
> > +        * CSD or CID.  Native versions of those commands use the R2 type,
> > +        * not R1 plus a data block.
> > +        */
> > +       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> > +
> > +       data.blksz = len;
> > +       data.blocks = 1;
> > +       if (opcode == MMC_CMD_BUS_TEST_R)
> > +               data.flags = MMC_DATA_READ;
> > +       else
> > +               data.flags = MMC_DATA_WRITE;
> > +
> > +       data.sg = &sg;
> > +       data.sg_len = 1;
> > +       sg_init_one(&sg, data_buf, len);
> > +       mmc_wait_for_req(host, &mrq);
> > +       err = 0;
> > +       if (opcode == MMC_CMD_BUS_TEST_R) {
> > +               for (i = 0; i < len / 4; i++)
> > +                       if ((test_buf[i] ^ data_buf[i]) != 0xff) {
> > +                               err = -EIO;
> > +                               break;
> > +                       }
> > +       }
> > +       kfree(data_buf);
> > +
> > +       if (cmd.error)
> > +               return cmd.error;
> > +       if (data.error)
> 
> 	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.
> However some host still check and may get crc error here if card does not send.

Philip corrected my patch and added such a check (in sdhci.c side).
I'm going to resend the revised patch soon.


thanks,

Takashi