Re: [Sdhci-devel] [PATCH 2.6.20-rc2] Add a quirk to allow ENE PCI SD

[Darren Salt <linux@youmustbejoking.demon.co.uk>]

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

I demand that Pierre Ossman may or may not have written...

> Hi Darren,

> It has come to my attention that the current routine for setting power
> is not compliant with the specification. As such, I'd like you to try
> the following and see if removes the need for your patch:

Your patch was mangled by Thunderbird. IME, it always does this; you should
attach patches, not include them inline.

Still, it was easy enough to apply the patch manually.

> I'd appreciate if you could test this sooner rather than later as the merge
> window is just around the corner.

It doesn't work.

After applying my patch and fixing up the rejects, it still doesn't work. I
need to disable the first of the writeb() calls mentioned in the last hunk of
your patch for it to work again. I have the impression that the hardware
doesn't like the power-on bit not being set :-|

... hmm, it looks like there's a small bug in my patch: the label "out" needs
to be before the last writeb() otherwise, if power is -1, no write will
happen regardless. I'm attaching a fixed version along with an adapted
version of your patch.

Pierre, if you're happy to sign off the modified version of your patch, feel
free to convert my not-yet-signed-off-by into a normal signed-off-by.

-- 
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
|   <URL:http://www.youmustbejoking.demon.co.uk/> (PGP 2.6, GPG keys)

Break up a relationship. Buy a computer.

[-- Attachment #2: add-a-quirk-to-allow-at-least-some-ENE-PCI-SD-card-readers-to-work-again.patch --]
[-- Type: text/plain, Size: 1957 bytes --]

Add a quirk to allow at least some ENE PCI SD card readers to work again

Support for these devices was broken for 2.6.18-rc1 and later by commit
146ad66eac836c0b976c98f428d73e1f6a75270d, which added voltage level support.

This restores the previous behaviour for these devices (PCI ID 1524:0550).

Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>

diff -ur linux-2.6.20-rc6.orig/drivers/mmc/sdhci.c linux-2.6.20-rc6/drivers/mmc/sdhci.c
--- linux-2.6.20-rc6.orig/drivers/mmc/sdhci.c	2006-12-30 15:34:11.000000000 +0000
+++ linux-2.6.20-rc6/drivers/mmc/sdhci.c	2007-02-02 20:04:54.000000000 +0000
@@ -37,6 +37,7 @@
 #define SDHCI_QUIRK_FORCE_DMA				(1<<1)
 /* Controller doesn't like some resets when there is no card inserted. */
 #define SDHCI_QUIRK_NO_CARD_NO_RESET			(1<<2)
+#define SDHCI_QUIRK_SINGLE_POWER_WRITE			(1<<3)
 
 static const struct pci_device_id pci_ids[] __devinitdata = {
 	{
@@ -65,6 +66,16 @@
 		.driver_data	= SDHCI_QUIRK_FORCE_DMA,
 	},
 
+	{
+		.class		= PCI_CLASS_SYSTEM_SDHCI << 8,
+		.class_mask	= 0xFFFF00,
+		.vendor		= PCI_VENDOR_ID_ENE,
+		.device		= PCI_ANY_ID,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.driver_data	= SDHCI_QUIRK_SINGLE_POWER_WRITE,
+	},
+
 	{	/* Generic SD host controller */
 		PCI_DEVICE_CLASS((PCI_CLASS_SYSTEM_SDHCI << 8), 0xFFFF00)
 	},
@@ -669,16 +680,17 @@
 
 static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 {
-	u8 pwr;
+	u8 pwr = 0;
 
 	if (host->power == power)
 		return;
 
-	writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
-
 	if (power == (unsigned short)-1)
 		goto out;
 
+	if ((host->chip->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE) == 0)
+		writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
+
 	pwr = SDHCI_POWER_ON;
 
 	switch (power) {
@@ -701,9 +713,9 @@
 		BUG();
 	}
 
+out:
 	writeb(pwr, host->ioaddr + SDHCI_POWER_CONTROL);
 
-out:
 	host->power = power;
 }
 

[-- Attachment #3: make-the-routine-for-setting-power-compliant-with-the-SDHCI-specification.patch --]
[-- Type: text/plain, Size: 1436 bytes --]

Make the routine for setting power compliant with the specification.

Certain ENE chips seem to be sufficiently non-compliant that they don't like
this change; however, the single-power-write quirk should be enough to handle
them (and is, in the case of at least one such device).

Not-yet-signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>

diff -ur linux-2.6.20-rc6.orig/drivers/mmc/sdhci.c linux-2.6.20-rc6/drivers/mmc/sdhci.c
--- linux-2.6.20-rc6.orig/drivers/mmc/sdhci.c	2007-02-02 20:04:54.000000000 +0000
+++ linux-2.6.20-rc6/drivers/mmc/sdhci.c	2007-02-02 20:14:28.000000000 +0000
@@ -691,28 +691,31 @@
 	if ((host->chip->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE) == 0)
 		writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
 
-	pwr = SDHCI_POWER_ON;
-
 	switch (power) {
 	case MMC_VDD_170:
 	case MMC_VDD_180:
 	case MMC_VDD_190:
-		pwr |= SDHCI_POWER_180;
+		pwr = SDHCI_POWER_180;
 		break;
 	case MMC_VDD_290:
 	case MMC_VDD_300:
 	case MMC_VDD_310:
-		pwr |= SDHCI_POWER_300;
+		pwr = SDHCI_POWER_300;
 		break;
 	case MMC_VDD_320:
 	case MMC_VDD_330:
 	case MMC_VDD_340:
-		pwr |= SDHCI_POWER_330;
+		pwr = SDHCI_POWER_330;
 		break;
 	default:
 		BUG();
 	}
 
+	if ((host->chip->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE) == 0)
+		writeb(pwr, host->ioaddr + SDHCI_POWER_CONTROL);
+
+	pwr |= SDHCI_POWER_ON;
+
 out:
 	writeb(pwr, host->ioaddr + SDHCI_POWER_CONTROL);