linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/13] FSL eSDHC support
@ 2009-02-13 14:46 Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ Anton Vorontsov
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:46 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

Hi all,

Thanks for the comments on the previous version, here comes another
RFC...

Changes since the second RFC:
- Addressed all comments that were raised by Pierre Ossman.
  There were too many to mention them all, so here is the link:
  http://lkml.org/lkml/2009/2/6/320

Changes since the first RFC:
- Use of_iomap() in sdhci-of.c (suggested by Arnd Bergmann). Also added
  Arnd's Acked-by: line for the sdhci-of patch.
- Kconfig help text improved (thanks to Matt Sealey and M. Warner Losh).
- In "sdhci: Add quirk to suppress PIO interrupts during DMA transfers"
  patch: sdhci_init() now clears SDHCI_PIO_DISABLED flag, otherwise we
  won't disable PIO interrupts after suspend.
- New patch: "sdhci: Add type checking for IO memory accessors"

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 02/13] sdhci: Add support for bus-specific IO memory accessors Anton Vorontsov
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

From: Ben Dooks <ben-linux@fluff.org>

The Samsung SDHCI (and FSL eSDHC) controller block seems to fail
to generate an INT_DATA_END after the transfer has completed and
the bus busy state finished.

Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
new busy method are the cause of the behaviour change.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |    5 ++++-
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6b2d1f9..0a1f5c5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1291,8 +1291,11 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask)
 		if (host->cmd->data)
 			DBG("Cannot wait for busy signal when also "
 				"doing a data transfer");
-		else
+		else if (!(host->quirks & SDHCI_QUIRK_NO_BUSY_IRQ))
 			return;
+
+		/* The controller does not support the end-of-busy IRQ,
+		 * fall through and take the SDHCI_INT_RESPONSE */
 	}
 
 	if (intmask & SDHCI_INT_RESPONSE)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 3efba23..2d08dd4 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -210,6 +210,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_BROKEN_SMALL_PIO			(1<<13)
 /* Controller supports high speed but doesn't have the caps bit set */
 #define SDHCI_QUIRK_FORCE_HIGHSPEED			(1<<14)
+/* Controller does not provide transfer-complete interrupt when not busy */
+#define SDHCI_QUIRK_NO_BUSY_IRQ				(1<<15)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/13] sdhci: Add support for bus-specific IO memory accessors
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init() Anton Vorontsov
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

Currently the SDHCI driver works with PCI accessors (write{l,b,w} and
read{l,b,w}).

With this patch drivers may change memory accessors, so that we can
support hosts with "weird" IO memory access requirments.

For example, in "FSL eSDHC" SDHCI hardware all registers are 32 bit
width, with big-endian addressing. That is, readb(0x2f) should turn
into readb(0x2c), and readw(0x2c) should be translated to
le16_to_cpu(readw(0x2e)).

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci-pci.c |    4 +-
 drivers/mmc/host/sdhci.c     |  214 ++++++++++++++++++++++++++----------------
 drivers/mmc/host/sdhci.h     |   14 +++
 3 files changed, 149 insertions(+), 83 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index f07255c..a3ced4d 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -212,7 +212,7 @@ static int jmicron_probe_slot(struct sdhci_pci_slot *slot)
 	if (slot->chip->pdev->revision == 0) {
 		u16 version;
 
-		version = readl(slot->host->ioaddr + SDHCI_HOST_VERSION);
+		version = sdhci_readl(slot->host, SDHCI_HOST_VERSION);
 		version = (version & SDHCI_VENDOR_VER_MASK) >>
 			SDHCI_VENDOR_VER_SHIFT;
 
@@ -583,7 +583,7 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot *slot)
 	u32 scratch;
 
 	dead = 0;
-	scratch = readl(slot->host->ioaddr + SDHCI_INT_STATUS);
+	scratch = sdhci_readl(slot->host, SDHCI_INT_STATUS);
 	if (scratch == (u32)-1)
 		dead = 1;
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0a1f5c5..8682f7f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/dma-mapping.h>
 #include <linux/scatterlist.h>
+#include <linux/compiler.h>
 
 #include <linux/leds.h>
 
@@ -48,35 +49,35 @@ static void sdhci_dumpregs(struct sdhci_host *host)
 	printk(KERN_DEBUG DRIVER_NAME ": ============== REGISTER DUMP ==============\n");
 
 	printk(KERN_DEBUG DRIVER_NAME ": Sys addr: 0x%08x | Version:  0x%08x\n",
-		readl(host->ioaddr + SDHCI_DMA_ADDRESS),
-		readw(host->ioaddr + SDHCI_HOST_VERSION));
+		sdhci_readl(host, SDHCI_DMA_ADDRESS),
+		sdhci_readw(host, SDHCI_HOST_VERSION));
 	printk(KERN_DEBUG DRIVER_NAME ": Blk size: 0x%08x | Blk cnt:  0x%08x\n",
-		readw(host->ioaddr + SDHCI_BLOCK_SIZE),
-		readw(host->ioaddr + SDHCI_BLOCK_COUNT));
+		sdhci_readw(host, SDHCI_BLOCK_SIZE),
+		sdhci_readw(host, SDHCI_BLOCK_COUNT));
 	printk(KERN_DEBUG DRIVER_NAME ": Argument: 0x%08x | Trn mode: 0x%08x\n",
-		readl(host->ioaddr + SDHCI_ARGUMENT),
-		readw(host->ioaddr + SDHCI_TRANSFER_MODE));
+		sdhci_readl(host, SDHCI_ARGUMENT),
+		sdhci_readw(host, SDHCI_TRANSFER_MODE));
 	printk(KERN_DEBUG DRIVER_NAME ": Present:  0x%08x | Host ctl: 0x%08x\n",
-		readl(host->ioaddr + SDHCI_PRESENT_STATE),
-		readb(host->ioaddr + SDHCI_HOST_CONTROL));
+		sdhci_readl(host, SDHCI_PRESENT_STATE),
+		sdhci_readb(host, SDHCI_HOST_CONTROL));
 	printk(KERN_DEBUG DRIVER_NAME ": Power:    0x%08x | Blk gap:  0x%08x\n",
-		readb(host->ioaddr + SDHCI_POWER_CONTROL),
-		readb(host->ioaddr + SDHCI_BLOCK_GAP_CONTROL));
+		sdhci_readb(host, SDHCI_POWER_CONTROL),
+		sdhci_readb(host, SDHCI_BLOCK_GAP_CONTROL));
 	printk(KERN_DEBUG DRIVER_NAME ": Wake-up:  0x%08x | Clock:    0x%08x\n",
-		readb(host->ioaddr + SDHCI_WAKE_UP_CONTROL),
-		readw(host->ioaddr + SDHCI_CLOCK_CONTROL));
+		sdhci_readb(host, SDHCI_WAKE_UP_CONTROL),
+		sdhci_readw(host, SDHCI_CLOCK_CONTROL));
 	printk(KERN_DEBUG DRIVER_NAME ": Timeout:  0x%08x | Int stat: 0x%08x\n",
-		readb(host->ioaddr + SDHCI_TIMEOUT_CONTROL),
-		readl(host->ioaddr + SDHCI_INT_STATUS));
+		sdhci_readb(host, SDHCI_TIMEOUT_CONTROL),
+		sdhci_readl(host, SDHCI_INT_STATUS));
 	printk(KERN_DEBUG DRIVER_NAME ": Int enab: 0x%08x | Sig enab: 0x%08x\n",
-		readl(host->ioaddr + SDHCI_INT_ENABLE),
-		readl(host->ioaddr + SDHCI_SIGNAL_ENABLE));
+		sdhci_readl(host, SDHCI_INT_ENABLE),
+		sdhci_readl(host, SDHCI_SIGNAL_ENABLE));
 	printk(KERN_DEBUG DRIVER_NAME ": AC12 err: 0x%08x | Slot int: 0x%08x\n",
-		readw(host->ioaddr + SDHCI_ACMD12_ERR),
-		readw(host->ioaddr + SDHCI_SLOT_INT_STATUS));
+		sdhci_readw(host, SDHCI_ACMD12_ERR),
+		sdhci_readw(host, SDHCI_SLOT_INT_STATUS));
 	printk(KERN_DEBUG DRIVER_NAME ": Caps:     0x%08x | Max curr: 0x%08x\n",
-		readl(host->ioaddr + SDHCI_CAPABILITIES),
-		readl(host->ioaddr + SDHCI_MAX_CURRENT));
+		sdhci_readl(host, SDHCI_CAPABILITIES),
+		sdhci_readl(host, SDHCI_MAX_CURRENT));
 
 	printk(KERN_DEBUG DRIVER_NAME ": ===========================================\n");
 }
@@ -87,17 +88,71 @@ static void sdhci_dumpregs(struct sdhci_host *host)
  *                                                                           *
 \*****************************************************************************/
 
+void sdhci_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	if (unlikely(host->ops->writel))
+		host->ops->writel(host, val, reg);
+	else
+		writel(val, host->ioaddr + reg);
+}
+EXPORT_SYMBOL_GPL(sdhci_writel);
+
+void sdhci_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	if (unlikely(host->ops->writew))
+		host->ops->writew(host, val, reg);
+	else
+		writew(val, host->ioaddr + reg);
+}
+EXPORT_SYMBOL_GPL(sdhci_writew);
+
+void sdhci_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	if (unlikely(host->ops->writeb))
+		host->ops->writeb(host, val, reg);
+	else
+		writeb(val, host->ioaddr + reg);
+}
+EXPORT_SYMBOL_GPL(sdhci_writeb);
+
+u32 sdhci_readl(struct sdhci_host *host, int reg)
+{
+	if (unlikely(host->ops->readl))
+		return host->ops->readl(host, reg);
+	else
+		return readl(host->ioaddr + reg);
+}
+EXPORT_SYMBOL_GPL(sdhci_readl);
+
+u16 sdhci_readw(struct sdhci_host *host, int reg)
+{
+	if (unlikely(host->ops->readw))
+		return host->ops->readw(host, reg);
+	else
+		return readw(host->ioaddr + reg);
+}
+EXPORT_SYMBOL_GPL(sdhci_readw);
+
+u8 sdhci_readb(struct sdhci_host *host, int reg)
+{
+	if (unlikely(host->ops->readb))
+		return host->ops->readb(host, reg);
+	else
+		return readb(host->ioaddr + reg);
+}
+EXPORT_SYMBOL_GPL(sdhci_readb);
+
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	unsigned long timeout;
 
 	if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) {
-		if (!(readl(host->ioaddr + SDHCI_PRESENT_STATE) &
+		if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
 			SDHCI_CARD_PRESENT))
 			return;
 	}
 
-	writeb(mask, host->ioaddr + SDHCI_SOFTWARE_RESET);
+	sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET);
 
 	if (mask & SDHCI_RESET_ALL)
 		host->clock = 0;
@@ -106,7 +161,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 	timeout = 100;
 
 	/* hw clears the bit when it's done */
-	while (readb(host->ioaddr + SDHCI_SOFTWARE_RESET) & mask) {
+	while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
 		if (timeout == 0) {
 			printk(KERN_ERR "%s: Reset 0x%x never completed.\n",
 				mmc_hostname(host->mmc), (int)mask);
@@ -132,26 +187,26 @@ static void sdhci_init(struct sdhci_host *host)
 		SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
 		SDHCI_INT_ADMA_ERROR;
 
-	writel(intmask, host->ioaddr + SDHCI_INT_ENABLE);
-	writel(intmask, host->ioaddr + SDHCI_SIGNAL_ENABLE);
+	sdhci_writel(host, intmask, SDHCI_INT_ENABLE);
+	sdhci_writel(host, intmask, SDHCI_SIGNAL_ENABLE);
 }
 
 static void sdhci_activate_led(struct sdhci_host *host)
 {
 	u8 ctrl;
 
-	ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
 	ctrl |= SDHCI_CTRL_LED;
-	writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 }
 
 static void sdhci_deactivate_led(struct sdhci_host *host)
 {
 	u8 ctrl;
 
-	ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
 	ctrl &= ~SDHCI_CTRL_LED;
-	writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 }
 
 #ifdef SDHCI_USE_LEDS_CLASS
@@ -205,7 +260,7 @@ static void sdhci_read_block_pio(struct sdhci_host *host)
 
 		while (len) {
 			if (chunk == 0) {
-				scratch = readl(host->ioaddr + SDHCI_BUFFER);
+				scratch = sdhci_readl(host, SDHCI_BUFFER);
 				chunk = 4;
 			}
 
@@ -257,7 +312,7 @@ static void sdhci_write_block_pio(struct sdhci_host *host)
 			len--;
 
 			if ((chunk == 4) || ((len == 0) && (blksize == 0))) {
-				writel(scratch, host->ioaddr + SDHCI_BUFFER);
+				sdhci_writel(host, scratch, SDHCI_BUFFER);
 				chunk = 0;
 				scratch = 0;
 			}
@@ -292,7 +347,7 @@ static void sdhci_transfer_pio(struct sdhci_host *host)
 		(host->data->blocks == 1))
 		mask = ~0;
 
-	while (readl(host->ioaddr + SDHCI_PRESENT_STATE) & mask) {
+	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
 		if (host->data->flags & MMC_DATA_READ)
 			sdhci_read_block_pio(host);
 		else
@@ -581,7 +636,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	host->data_early = 0;
 
 	count = sdhci_calc_timeout(host, data);
-	writeb(count, host->ioaddr + SDHCI_TIMEOUT_CONTROL);
+	sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL);
 
 	if (host->flags & SDHCI_USE_DMA)
 		host->flags |= SDHCI_REQ_USE_DMA;
@@ -661,8 +716,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 				WARN_ON(1);
 				host->flags &= ~SDHCI_REQ_USE_DMA;
 			} else {
-				writel(host->adma_addr,
-					host->ioaddr + SDHCI_ADMA_ADDRESS);
+				sdhci_writel(host, host->adma_addr,
+					SDHCI_ADMA_ADDRESS);
 			}
 		} else {
 			int sg_cnt;
@@ -681,8 +736,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 				host->flags &= ~SDHCI_REQ_USE_DMA;
 			} else {
 				WARN_ON(sg_cnt != 1);
-				writel(sg_dma_address(data->sg),
-					host->ioaddr + SDHCI_DMA_ADDRESS);
+				sdhci_writel(host, sg_dma_address(data->sg),
+					SDHCI_DMA_ADDRESS);
 			}
 		}
 	}
@@ -693,14 +748,14 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	 * is ADMA.
 	 */
 	if (host->version >= SDHCI_SPEC_200) {
-		ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
 		ctrl &= ~SDHCI_CTRL_DMA_MASK;
 		if ((host->flags & SDHCI_REQ_USE_DMA) &&
 			(host->flags & SDHCI_USE_ADMA))
 			ctrl |= SDHCI_CTRL_ADMA32;
 		else
 			ctrl |= SDHCI_CTRL_SDMA;
-		writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 	}
 
 	if (!(host->flags & SDHCI_REQ_USE_DMA)) {
@@ -710,9 +765,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	}
 
 	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
-	writew(SDHCI_MAKE_BLKSZ(7, data->blksz),
-		host->ioaddr + SDHCI_BLOCK_SIZE);
-	writew(data->blocks, host->ioaddr + SDHCI_BLOCK_COUNT);
+	sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE);
+	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
@@ -733,7 +787,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	if (host->flags & SDHCI_REQ_USE_DMA)
 		mode |= SDHCI_TRNS_DMA;
 
-	writew(mode, host->ioaddr + SDHCI_TRANSFER_MODE);
+	sdhci_writew(host, mode, SDHCI_TRANSFER_MODE);
 }
 
 static void sdhci_finish_data(struct sdhci_host *host)
@@ -802,7 +856,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	if (host->mrq->data && (cmd == host->mrq->data->stop))
 		mask &= ~SDHCI_DATA_INHIBIT;
 
-	while (readl(host->ioaddr + SDHCI_PRESENT_STATE) & mask) {
+	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
 		if (timeout == 0) {
 			printk(KERN_ERR "%s: Controller never released "
 				"inhibit bit(s).\n", mmc_hostname(host->mmc));
@@ -821,7 +875,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_prepare_data(host, cmd->data);
 
-	writel(cmd->arg, host->ioaddr + SDHCI_ARGUMENT);
+	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
 	sdhci_set_transfer_mode(host, cmd->data);
 
@@ -849,8 +903,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	if (cmd->data)
 		flags |= SDHCI_CMD_DATA;
 
-	writew(SDHCI_MAKE_CMD(cmd->opcode, flags),
-		host->ioaddr + SDHCI_COMMAND);
+	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
 }
 
 static void sdhci_finish_command(struct sdhci_host *host)
@@ -863,15 +916,15 @@ static void sdhci_finish_command(struct sdhci_host *host)
 		if (host->cmd->flags & MMC_RSP_136) {
 			/* CRC is stripped so we need to do some shifting. */
 			for (i = 0;i < 4;i++) {
-				host->cmd->resp[i] = readl(host->ioaddr +
+				host->cmd->resp[i] = sdhci_readl(host,
 					SDHCI_RESPONSE + (3-i)*4) << 8;
 				if (i != 3)
 					host->cmd->resp[i] |=
-						readb(host->ioaddr +
+						sdhci_readb(host,
 						SDHCI_RESPONSE + (3-i)*4-1);
 			}
 		} else {
-			host->cmd->resp[0] = readl(host->ioaddr + SDHCI_RESPONSE);
+			host->cmd->resp[0] = sdhci_readl(host, SDHCI_RESPONSE);
 		}
 	}
 
@@ -895,7 +948,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (clock == host->clock)
 		return;
 
-	writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
+	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
 
 	if (clock == 0)
 		goto out;
@@ -908,11 +961,11 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 
 	clk = div << SDHCI_DIVIDER_SHIFT;
 	clk |= SDHCI_CLOCK_INT_EN;
-	writew(clk, host->ioaddr + SDHCI_CLOCK_CONTROL);
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
 	/* Wait max 10 ms */
 	timeout = 10;
-	while (!((clk = readw(host->ioaddr + SDHCI_CLOCK_CONTROL))
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
 		& SDHCI_CLOCK_INT_STABLE)) {
 		if (timeout == 0) {
 			printk(KERN_ERR "%s: Internal clock never "
@@ -925,7 +978,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 
 	clk |= SDHCI_CLOCK_CARD_EN;
-	writew(clk, host->ioaddr + SDHCI_CLOCK_CONTROL);
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
 out:
 	host->clock = clock;
@@ -939,7 +992,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 		return;
 
 	if (power == (unsigned short)-1) {
-		writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
+		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
 		goto out;
 	}
 
@@ -948,7 +1001,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 	 * a new value. Some controllers don't seem to like this though.
 	 */
 	if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE))
-		writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
+		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
 
 	pwr = SDHCI_POWER_ON;
 
@@ -973,10 +1026,9 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 	 * and set turn on power at the same time, so set the voltage first.
 	 */
 	if ((host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER))
-		writeb(pwr & ~SDHCI_POWER_ON,
-				host->ioaddr + SDHCI_POWER_CONTROL);
+		sdhci_writeb(host, pwr & ~SDHCI_POWER_ON, SDHCI_POWER_CONTROL);
 
-	writeb(pwr, host->ioaddr + SDHCI_POWER_CONTROL);
+	sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
 
 out:
 	host->power = power;
@@ -1005,7 +1057,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	host->mrq = mrq;
 
-	if (!(readl(host->ioaddr + SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
+	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
 		|| (host->flags & SDHCI_DEVICE_DEAD)) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
@@ -1034,7 +1086,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * Should clear out any weird states.
 	 */
 	if (ios->power_mode == MMC_POWER_OFF) {
-		writel(0, host->ioaddr + SDHCI_SIGNAL_ENABLE);
+		sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
 		sdhci_init(host);
 	}
 
@@ -1045,7 +1097,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	else
 		sdhci_set_power(host, ios->vdd);
 
-	ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
 
 	if (ios->bus_width == MMC_BUS_WIDTH_4)
 		ctrl |= SDHCI_CTRL_4BITBUS;
@@ -1057,7 +1109,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	else
 		ctrl &= ~SDHCI_CTRL_HISPD;
 
-	writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 
 	/*
 	 * Some (ENE) controllers go apeshit on some ios operation,
@@ -1085,7 +1137,7 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		present = 0;
 	else
-		present = readl(host->ioaddr + SDHCI_PRESENT_STATE);
+		present = sdhci_readl(host, SDHCI_PRESENT_STATE);
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
@@ -1105,14 +1157,14 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		goto out;
 
-	ier = readl(host->ioaddr + SDHCI_INT_ENABLE);
+	ier = sdhci_readl(host, SDHCI_INT_ENABLE);
 
 	ier &= ~SDHCI_INT_CARD_INT;
 	if (enable)
 		ier |= SDHCI_INT_CARD_INT;
 
-	writel(ier, host->ioaddr + SDHCI_INT_ENABLE);
-	writel(ier, host->ioaddr + SDHCI_SIGNAL_ENABLE);
+	sdhci_writel(host, ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
 
 out:
 	mmiowb();
@@ -1142,7 +1194,7 @@ static void sdhci_tasklet_card(unsigned long param)
 
 	spin_lock_irqsave(&host->lock, flags);
 
-	if (!(readl(host->ioaddr + SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) {
+	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) {
 		if (host->mrq) {
 			printk(KERN_ERR "%s: Card removed during transfer!\n",
 				mmc_hostname(host->mmc));
@@ -1346,8 +1398,8 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 		 * we need to at least restart the transfer.
 		 */
 		if (intmask & SDHCI_INT_DMA_END)
-			writel(readl(host->ioaddr + SDHCI_DMA_ADDRESS),
-				host->ioaddr + SDHCI_DMA_ADDRESS);
+			sdhci_writel(host, sdhci_readl(host, SDHCI_DMA_ADDRESS),
+				SDHCI_DMA_ADDRESS);
 
 		if (intmask & SDHCI_INT_DATA_END) {
 			if (host->cmd) {
@@ -1373,7 +1425,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 
 	spin_lock(&host->lock);
 
-	intmask = readl(host->ioaddr + SDHCI_INT_STATUS);
+	intmask = sdhci_readl(host, SDHCI_INT_STATUS);
 
 	if (!intmask || intmask == 0xffffffff) {
 		result = IRQ_NONE;
@@ -1384,22 +1436,22 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 		mmc_hostname(host->mmc), intmask);
 
 	if (intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
-		writel(intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE),
-			host->ioaddr + SDHCI_INT_STATUS);
+		sdhci_writel(host, intmask & (SDHCI_INT_CARD_INSERT |
+			SDHCI_INT_CARD_REMOVE), SDHCI_INT_STATUS);
 		tasklet_schedule(&host->card_tasklet);
 	}
 
 	intmask &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
 
 	if (intmask & SDHCI_INT_CMD_MASK) {
-		writel(intmask & SDHCI_INT_CMD_MASK,
-			host->ioaddr + SDHCI_INT_STATUS);
+		sdhci_writel(host, intmask & SDHCI_INT_CMD_MASK,
+			SDHCI_INT_STATUS);
 		sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
 	}
 
 	if (intmask & SDHCI_INT_DATA_MASK) {
-		writel(intmask & SDHCI_INT_DATA_MASK,
-			host->ioaddr + SDHCI_INT_STATUS);
+		sdhci_writel(host, intmask & SDHCI_INT_DATA_MASK,
+			SDHCI_INT_STATUS);
 		sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
 	}
 
@@ -1410,7 +1462,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 	if (intmask & SDHCI_INT_BUS_POWER) {
 		printk(KERN_ERR "%s: Card is consuming too much power!\n",
 			mmc_hostname(host->mmc));
-		writel(SDHCI_INT_BUS_POWER, host->ioaddr + SDHCI_INT_STATUS);
+		sdhci_writel(host, SDHCI_INT_BUS_POWER, SDHCI_INT_STATUS);
 	}
 
 	intmask &= ~SDHCI_INT_BUS_POWER;
@@ -1425,7 +1477,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 			mmc_hostname(host->mmc), intmask);
 		sdhci_dumpregs(host);
 
-		writel(intmask, host->ioaddr + SDHCI_INT_STATUS);
+		sdhci_writel(host, intmask, SDHCI_INT_STATUS);
 	}
 
 	result = IRQ_HANDLED;
@@ -1537,7 +1589,7 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	sdhci_reset(host, SDHCI_RESET_ALL);
 
-	host->version = readw(host->ioaddr + SDHCI_HOST_VERSION);
+	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
 	if (host->version > SDHCI_SPEC_200) {
@@ -1546,7 +1598,7 @@ int sdhci_add_host(struct sdhci_host *host)
 			host->version);
 	}
 
-	caps = readl(host->ioaddr + SDHCI_CAPABILITIES);
+	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
 
 	if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
 		host->flags |= SDHCI_USE_DMA;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 2d08dd4..e907441 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -9,6 +9,7 @@
  * your option) any later version.
  */
 
+#include <linux/types.h>
 #include <linux/scatterlist.h>
 
 /*
@@ -268,9 +269,22 @@ struct sdhci_host {
 
 
 struct sdhci_ops {
+	u32		(*readl)(struct sdhci_host *host, int reg);
+	u16		(*readw)(struct sdhci_host *host, int reg);
+	u8		(*readb)(struct sdhci_host *host, int reg);
+	void		(*writel)(struct sdhci_host *host, u32 val, int reg);
+	void		(*writew)(struct sdhci_host *host, u16 val, int reg);
+	void		(*writeb)(struct sdhci_host *host, u8 val, int reg);
+
 	int		(*enable_dma)(struct sdhci_host *host);
 };
 
+extern void sdhci_writel(struct sdhci_host *host, u32 val, int reg);
+extern void sdhci_writew(struct sdhci_host *host, u16 val, int reg);
+extern void sdhci_writeb(struct sdhci_host *host, u8 val, int reg);
+extern u32 sdhci_readl(struct sdhci_host *host, int reg);
+extern u16 sdhci_readw(struct sdhci_host *host, int reg);
+extern u8 sdhci_readb(struct sdhci_host *host, int reg);
 
 extern struct sdhci_host *sdhci_alloc_host(struct device *dev,
 	size_t priv_size);
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init()
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 02/13] sdhci: Add support for bus-specific IO memory accessors Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-21 15:58   ` Pierre Ossman
  2009-02-13 14:47 ` [PATCH 04/13] sdhci: Enable only relevant (DMA/PIO) interrupts during transfers Anton Vorontsov
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

Card detection interrupts should be handled separately as they should
not be enabled before mmc_add_host() returns and should be disabled
before calling mmc_remove_host(). The same is for suspend and resume
routines.

sdhci_init() no longer enables card-detection irqs. Instead, two new
functions implemented: sdhci_enable_card_detection() and
sdhci_disable_card_detection().

New sdhci_reinit() call implemented to behave the same way as the old
sdhci_init().

Also, this patch implements and uses few new helpers to manage IRQs in
a more conveinient way, that is:

- sdhci_clear_set_irqs()
- sdhci_unmask_irqs()
- sdhci_mask_irqs()
- SDHCI_INT_ALL_MASK constant

sdhci_enable_sdio_irq() converted to these new helpers, plus the
helpers will be used by the subsequent patches.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   78 ++++++++++++++++++++++++++++++++++++----------
 drivers/mmc/host/sdhci.h |    4 ++
 2 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8682f7f..fadaeb8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -142,6 +142,47 @@ u8 sdhci_readb(struct sdhci_host *host, int reg)
 }
 EXPORT_SYMBOL_GPL(sdhci_readb);
 
+static void sdhci_clear_set_irqs(struct sdhci_host *host, u32 clear, u32 set)
+{
+	u32 ier;
+
+	ier = sdhci_readl(host, SDHCI_INT_ENABLE);
+	ier &= ~clear;
+	ier |= set;
+	sdhci_writel(host, ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+}
+
+static void sdhci_unmask_irqs(struct sdhci_host *host, u32 irqs)
+{
+	sdhci_clear_set_irqs(host, 0, irqs);
+}
+
+static void sdhci_mask_irqs(struct sdhci_host *host, u32 irqs)
+{
+	sdhci_clear_set_irqs(host, irqs, 0);
+}
+
+static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
+{
+	u32 irqs = SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT;
+
+	if (enable)
+		sdhci_unmask_irqs(host, irqs);
+	else
+		sdhci_mask_irqs(host, irqs);
+}
+
+static void sdhci_enable_card_detection(struct sdhci_host *host)
+{
+	sdhci_set_card_detection(host, true);
+}
+
+static void sdhci_disable_card_detection(struct sdhci_host *host)
+{
+	sdhci_set_card_detection(host, false);
+}
+
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	unsigned long timeout;
@@ -175,20 +216,21 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 
 static void sdhci_init(struct sdhci_host *host)
 {
-	u32 intmask;
-
 	sdhci_reset(host, SDHCI_RESET_ALL);
 
-	intmask = SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
+	sdhci_clear_set_irqs(host, SDHCI_INT_ALL_MASK,
+		SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
 		SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_INDEX |
 		SDHCI_INT_END_BIT | SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
-		SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT |
 		SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL |
 		SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
-		SDHCI_INT_ADMA_ERROR;
+		SDHCI_INT_ADMA_ERROR);
+}
 
-	sdhci_writel(host, intmask, SDHCI_INT_ENABLE);
-	sdhci_writel(host, intmask, SDHCI_SIGNAL_ENABLE);
+static void sdhci_reinit(struct sdhci_host *host)
+{
+	sdhci_init(host);
+	sdhci_enable_card_detection(host);
 }
 
 static void sdhci_activate_led(struct sdhci_host *host)
@@ -1087,7 +1129,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 */
 	if (ios->power_mode == MMC_POWER_OFF) {
 		sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
-		sdhci_init(host);
+		sdhci_reinit(host);
 	}
 
 	sdhci_set_clock(host, ios->clock);
@@ -1148,7 +1190,6 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 {
 	struct sdhci_host *host;
 	unsigned long flags;
-	u32 ier;
 
 	host = mmc_priv(mmc);
 
@@ -1157,15 +1198,10 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		goto out;
 
-	ier = sdhci_readl(host, SDHCI_INT_ENABLE);
-
-	ier &= ~SDHCI_INT_CARD_INT;
 	if (enable)
-		ier |= SDHCI_INT_CARD_INT;
-
-	sdhci_writel(host, ier, SDHCI_INT_ENABLE);
-	sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
-
+		sdhci_unmask_irqs(host, SDHCI_INT_CARD_INT);
+	else
+		sdhci_mask_irqs(host, SDHCI_INT_CARD_INT);
 out:
 	mmiowb();
 
@@ -1507,6 +1543,8 @@ int sdhci_suspend_host(struct sdhci_host *host, pm_message_t state)
 {
 	int ret;
 
+	sdhci_disable_card_detection(host);
+
 	ret = mmc_suspend_host(host->mmc, state);
 	if (ret)
 		return ret;
@@ -1539,6 +1577,8 @@ int sdhci_resume_host(struct sdhci_host *host)
 	if (ret)
 		return ret;
 
+	sdhci_enable_card_detection(host);
+
 	return 0;
 }
 
@@ -1792,6 +1832,8 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	mmc_add_host(mmc);
 
+	sdhci_enable_card_detection(host);
+
 	printk(KERN_INFO "%s: SDHCI controller on %s [%s] using %s%s\n",
 		mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
 		(host->flags & SDHCI_USE_ADMA)?"A":"",
@@ -1833,6 +1875,8 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 		spin_unlock_irqrestore(&host->lock, flags);
 	}
 
+	sdhci_disable_card_detection(host);
+
 	mmc_remove_host(host->mmc);
 
 #ifdef SDHCI_USE_LEDS_CLASS
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index e907441..45c8309 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -124,6 +124,10 @@
 		SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL | \
 		SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
 		SDHCI_INT_DATA_END_BIT)
+#define SDHCI_INT_ALL_MASK	(SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK | \
+		SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE | \
+		SDHCI_INT_CARD_INT | SDHCI_INT_ERROR | SDHCI_INT_BUS_POWER | \
+		SDHCI_INT_ACMD12ERR | SDHCI_INT_ADMA_ERROR)
 
 #define SDHCI_ACMD12_ERR	0x3C
 
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/13] sdhci: Enable only relevant (DMA/PIO) interrupts during transfers
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (2 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init() Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 05/13] sdhci: Add support for card-detection polling Anton Vorontsov
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

Some hosts (that is, FSL eSDHC) throw PIO interrupts during DMA
transfers, this causes tons of unneeded interrupts, and thus highly
degraded speed.

This patch modifies the driver so that now we only enable relevant
(DMA or PIO) interrupts during transfers.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fadaeb8..2728e90 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -222,9 +222,7 @@ static void sdhci_init(struct sdhci_host *host)
 		SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
 		SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_INDEX |
 		SDHCI_INT_END_BIT | SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
-		SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL |
-		SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
-		SDHCI_INT_ADMA_ERROR);
+		SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE);
 }
 
 static void sdhci_reinit(struct sdhci_host *host)
@@ -658,6 +656,17 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_data *data)
 	return count;
 }
 
+static void sdhci_set_transfer_irqs(struct sdhci_host *host)
+{
+	u32 pio_irqs = SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL;
+	u32 dma_irqs = SDHCI_INT_DMA_END | SDHCI_INT_ADMA_ERROR;
+
+	if (host->flags & SDHCI_REQ_USE_DMA)
+		sdhci_clear_set_irqs(host, pio_irqs, dma_irqs);
+	else
+		sdhci_clear_set_irqs(host, dma_irqs, pio_irqs);
+}
+
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 {
 	u8 count;
@@ -806,6 +815,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 		host->blocks = data->blocks;
 	}
 
+	sdhci_set_transfer_irqs(host);
+
 	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
 	sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE);
 	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/13] sdhci: Add support for card-detection polling
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (3 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 04/13] sdhci: Enable only relevant (DMA/PIO) interrupts during transfers Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-21 15:58   ` Pierre Ossman
  2009-02-13 14:47 ` [PATCH 06/13] sdhci: Add support for hosts reporting inverted write-protect state Anton Vorontsov
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

This patch adds SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk. When specified,
sdhci driver will set MMC_CAP_NEEDS_POLL MMC host capability, and won't
enable card insert/remove interrupts.

This is needed for hosts with unreliable card detection, such as FSL
eSDHC. The original eSDHC driver was tring to "debounce" card-detection
IRQs by reading present state and disabling particular interrupts. But
with this debouncing scheme I noticed that sometimes we miss card
insertion/removal events.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   17 ++++++++++++++---
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2728e90..782fb58 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -167,6 +167,9 @@ static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
 {
 	u32 irqs = SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		return;
+
 	if (enable)
 		sdhci_unmask_irqs(host, irqs);
 	else
@@ -1110,13 +1113,18 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	host->mrq = mrq;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		goto send;
+
 	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
 		|| (host->flags & SDHCI_DEVICE_DEAD)) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
-	} else
-		sdhci_send_command(host, mrq->cmd);
-
+		goto out;
+	}
+send:
+	sdhci_send_command(host, mrq->cmd);
+out:
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
 }
@@ -1742,6 +1750,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	mmc->f_max = host->max_clk;
 	mmc->caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	if ((caps & SDHCI_CAN_DO_HISPD) ||
 		(host->quirks & SDHCI_QUIRK_FORCE_HIGHSPEED))
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 45c8309..b10920d 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -217,6 +217,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_FORCE_HIGHSPEED			(1<<14)
 /* Controller does not provide transfer-complete interrupt when not busy */
 #define SDHCI_QUIRK_NO_BUSY_IRQ				(1<<15)
+/* Controller has unreliable card detection */
+#define SDHCI_QUIRK_BROKEN_CARD_DETECTION		(1<<16)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/13] sdhci: Add support for hosts reporting inverted write-protect state
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (4 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 05/13] sdhci: Add support for card-detection polling Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing Anton Vorontsov
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

This patch adds SDHCI_QUIRK_INVERTED_WRITE_PROTECT quirk. When
specified, the sdhci driver will invert WP state.

p.s. Actually, the quirk is more board-specific than
     controller-specific.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |    2 ++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 782fb58..d668625 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1202,6 +1202,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
+	if (host->quirks & SDHCI_QUIRK_INVERTED_WRITE_PROTECT)
+		return !!(present & SDHCI_WRITE_PROTECT);
 	return !(present & SDHCI_WRITE_PROTECT);
 }
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index b10920d..051fef5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -219,6 +219,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_NO_BUSY_IRQ				(1<<15)
 /* Controller has unreliable card detection */
 #define SDHCI_QUIRK_BROKEN_CARD_DETECTION		(1<<16)
+/* Controller reports inverted write-protect state */
+#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT		(1<<17)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (5 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 06/13] sdhci: Add support for hosts reporting inverted write-protect state Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-21 15:58   ` Pierre Ossman
  2009-02-13 14:47 ` [PATCH 08/13] sdhci: Add get_{max,timeout}_clock callbacks Anton Vorontsov
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

SDHCI driver must take special care when working with "triggering"
registers on hosts with strict 32 bit addressing.

In FSL eSDHC hosts all registers are 32 bit width, writing to the
first half of any register will cause [undefined?] write the second
half of the register. That is, 16 bit write to the TRANSFER_MODE
register, makes hardware see a bogus write to the COMMAND register
(these two registers are adjacent).

This patch adds SDHCI_QUIRK_32BIT_REGISTERS quirk. When specified,
the sdhci driver will try to "pack" all dangerous writes into single
32 bit write transaction.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   19 ++++++++++++++-----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d668625..1c36a25 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -825,13 +825,13 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
 }
 
-static void sdhci_set_transfer_mode(struct sdhci_host *host,
+static u16 sdhci_get_transfer_mode(struct sdhci_host *host,
 	struct mmc_data *data)
 {
 	u16 mode;
 
 	if (data == NULL)
-		return;
+		return 0;
 
 	WARN_ON(!host->data);
 
@@ -843,7 +843,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	if (host->flags & SDHCI_REQ_USE_DMA)
 		mode |= SDHCI_TRNS_DMA;
 
-	sdhci_writew(host, mode, SDHCI_TRANSFER_MODE);
+	return mode;
 }
 
 static void sdhci_finish_data(struct sdhci_host *host)
@@ -895,6 +895,7 @@ static void sdhci_finish_data(struct sdhci_host *host)
 static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	int flags;
+	u16 xfer_mode;
 	u32 mask;
 	unsigned long timeout;
 
@@ -933,7 +934,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	xfer_mode = sdhci_get_transfer_mode(host, cmd->data);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -959,7 +960,15 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	if (cmd->data)
 		flags |= SDHCI_CMD_DATA;
 
-	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
+	if (host->quirks & SDHCI_QUIRK_32BIT_REGISTERS) {
+		sdhci_writel(host,
+			SDHCI_MAKE_CMD_32BIT(cmd->opcode, flags, xfer_mode),
+			SDHCI_TRANSFER_MODE);
+	} else {
+		sdhci_writew(host, xfer_mode, SDHCI_TRANSFER_MODE);
+		sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags),
+			SDHCI_COMMAND);
+	}
 }
 
 static void sdhci_finish_command(struct sdhci_host *host)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 051fef5..7b115d8 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -44,6 +44,8 @@
 #define  SDHCI_CMD_RESP_SHORT_BUSY 0x03
 
 #define SDHCI_MAKE_CMD(c, f) (((c & 0xff) << 8) | (f & 0xff))
+#define SDHCI_MAKE_CMD_32BIT(c, f, m) \
+	((((c) & 0xff) << 24) | (((f) & 0xff) << 16) | ((m) & 0x37))
 
 #define SDHCI_RESPONSE		0x10
 
@@ -221,6 +223,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_BROKEN_CARD_DETECTION		(1<<16)
 /* Controller reports inverted write-protect state */
 #define SDHCI_QUIRK_INVERTED_WRITE_PROTECT		(1<<17)
+/* Controller has all registers of 32 bit width */
+#define SDHCI_QUIRK_32BIT_REGISTERS			(1<<18)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/13] sdhci: Add get_{max,timeout}_clock callbacks
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (6 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 09/13] sdhci: Add set_clock callback and a quirk for nonstandard clocks Anton Vorontsov
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

From: Ben Dooks <ben-linux@fluff.org>

Some controllers do not provide clock information in their capabilities
(in the Samsung case, it is because there are multiple clock sources
available to the controller). Add hooks to allow the system to supply
clock information.

p.s.
In the original Ben's patch there was a bug that makes sdhci_add_host()
return -ENODEV even if callbacks were specified. This is fixed now.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   22 +++++++++++++++-------
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1c36a25..96a0482 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1736,19 +1736,27 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	host->max_clk =
 		(caps & SDHCI_CLOCK_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
+	host->max_clk *= 1000000;
 	if (host->max_clk == 0) {
-		printk(KERN_ERR "%s: Hardware doesn't specify base clock "
-			"frequency.\n", mmc_hostname(mmc));
-		return -ENODEV;
+		if (!host->ops->get_max_clock) {
+			printk(KERN_ERR
+			       "%s: Hardware doesn't specify base clock "
+			       "frequency.\n", mmc_hostname(mmc));
+			return -ENODEV;
+		}
+		host->max_clk = host->ops->get_max_clock(host);
 	}
-	host->max_clk *= 1000000;
 
 	host->timeout_clk =
 		(caps & SDHCI_TIMEOUT_CLK_MASK) >> SDHCI_TIMEOUT_CLK_SHIFT;
 	if (host->timeout_clk == 0) {
-		printk(KERN_ERR "%s: Hardware doesn't specify timeout clock "
-			"frequency.\n", mmc_hostname(mmc));
-		return -ENODEV;
+		if (!host->ops->get_timeout_clock) {
+			printk(KERN_ERR
+			       "%s: Hardware doesn't specify timeout clock "
+			       "frequency.\n", mmc_hostname(mmc));
+			return -ENODEV;
+		}
+		host->timeout_clk = host->ops->get_timeout_clock(host);
 	}
 	if (caps & SDHCI_TIMEOUT_CLK_UNIT)
 		host->timeout_clk *= 1000;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7b115d8..ef70900 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -289,6 +289,8 @@ struct sdhci_ops {
 	void		(*writeb)(struct sdhci_host *host, u8 val, int reg);
 
 	int		(*enable_dma)(struct sdhci_host *host);
+	unsigned int	(*get_max_clock)(struct sdhci_host *host);
+	unsigned int	(*get_timeout_clock)(struct sdhci_host *host);
 };
 
 extern void sdhci_writel(struct sdhci_host *host, u32 val, int reg);
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/13] sdhci: Add set_clock callback and a quirk for nonstandard clocks
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (7 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 08/13] sdhci: Add get_{max,timeout}_clock callbacks Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 10/13] sdhci: Add quirk for controllers that need small delays for PIO Anton Vorontsov
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

FSL eSDHC hosts have incompatible register map to manage the SDCLK.
This patch adds set_clock callback so that drivers could overwrite
set_clock behaviour.

Similar patch[1] was posted by Ben Dooks, though in Ben's version the
callback is named change_clock, plus the patch has some unrelated bits
that makes the patch difficult to reuse.

[1] http://lkml.org/lkml/2008/12/2/160

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |    6 ++++++
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 96a0482..f63db25 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1013,6 +1013,12 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (clock == host->clock)
 		return;
 
+	if (host->ops->set_clock) {
+		host->ops->set_clock(host, clock);
+		if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
+			return;
+	}
+
 	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
 
 	if (clock == 0)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index ef70900..63b436a 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -225,6 +225,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_INVERTED_WRITE_PROTECT		(1<<17)
 /* Controller has all registers of 32 bit width */
 #define SDHCI_QUIRK_32BIT_REGISTERS			(1<<18)
+/* Controller has nonstandard clock management */
+#define SDHCI_QUIRK_NONSTANDARD_CLOCK			(1<<19)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
@@ -288,6 +290,8 @@ struct sdhci_ops {
 	void		(*writew)(struct sdhci_host *host, u16 val, int reg);
 	void		(*writeb)(struct sdhci_host *host, u8 val, int reg);
 
+	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);
+
 	int		(*enable_dma)(struct sdhci_host *host);
 	unsigned int	(*get_max_clock)(struct sdhci_host *host);
 	unsigned int	(*get_timeout_clock)(struct sdhci_host *host);
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/13] sdhci: Add quirk for controllers that need small delays for PIO
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (8 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 09/13] sdhci: Add set_clock callback and a quirk for nonstandard clocks Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 14:47 ` [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset Anton Vorontsov
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

Small udelay is needed to make eSDHC work in PIO mode. Without
the delay reading causes endless interrupt storm, and writing
corrupts data. The first guess would be that we must wait for
some bit in some register, but I didn't find any reliable bits
that change before and after the delay.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |    3 +++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f63db25..eff615d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -391,6 +391,9 @@ static void sdhci_transfer_pio(struct sdhci_host *host)
 		mask = ~0;
 
 	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
+		if (host->quirks & SDHCI_QUIRK_PIO_NEEDS_DELAY)
+			udelay(100);
+
 		if (host->data->flags & MMC_DATA_READ)
 			sdhci_read_block_pio(host);
 		else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 63b436a..44c820a 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -227,6 +227,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_32BIT_REGISTERS			(1<<18)
 /* Controller has nonstandard clock management */
 #define SDHCI_QUIRK_NONSTANDARD_CLOCK			(1<<19)
+/* Controller does not like fast PIO transfers */
+#define SDHCI_QUIRK_PIO_NEEDS_DELAY			(1<<20)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (9 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 10/13] sdhci: Add quirk for controllers that need small delays for PIO Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-13 15:47   ` Laurent Pinchart
  2009-02-13 14:47 ` [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes Anton Vorontsov
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

FSL eSDHC controllers losing signal/interrupt enable states after
reset, so we should re-enable them.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |    7 +++++++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index eff615d..b308dbf 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -189,6 +189,7 @@ static void sdhci_disable_card_detection(struct sdhci_host *host)
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	unsigned long timeout;
+	u32 ier = 0; /* shut up gcc */
 
 	if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) {
 		if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
@@ -196,6 +197,9 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 			return;
 	}
 
+	if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
+		ier = sdhci_readl(host, SDHCI_INT_ENABLE);
+
 	sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET);
 
 	if (mask & SDHCI_RESET_ALL)
@@ -215,6 +219,9 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 		timeout--;
 		mdelay(1);
 	}
+
+	if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
+		sdhci_clear_set_irqs(host, SDHCI_INT_ALL_MASK, ier);
 }
 
 static void sdhci_init(struct sdhci_host *host)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 44c820a..5c5a950 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -229,6 +229,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_NONSTANDARD_CLOCK			(1<<19)
 /* Controller does not like fast PIO transfers */
 #define SDHCI_QUIRK_PIO_NEEDS_DELAY			(1<<20)
+/* Controller losing signal/interrupt enable states after reset */
+#define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET		(1<<21)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (10 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-21 15:58   ` Pierre Ossman
  2009-02-13 14:47 ` [PATCH 13/13] mmc: Add OpenFirmware bindings for SDHCI driver Anton Vorontsov
  2009-02-17 16:31 ` [PATCH RFC 0/13] FSL eSDHC support Ben Dooks
  13 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

FSL eSDHC controllers can support maximum block size up to 4096
bytes. The MBL (Maximum Block Length) field in the capabilities
register extended by one bit, and bits 13:15 in the block size
register reserved.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   28 ++++++++++++++++++++--------
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b308dbf..b341a9a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -682,6 +682,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host)
 
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 {
+	u16 blksz;
 	u8 count;
 	u8 ctrl;
 	int ret;
@@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	sdhci_set_transfer_irqs(host);
 
 	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
-	sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE);
+	if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096)
+		blksz = data->blksz;
+	else
+		blksz = SDHCI_MAKE_BLKSZ(7, data->blksz);
+
+	sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE);
 	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
 }
 
@@ -1840,13 +1846,19 @@ int sdhci_add_host(struct sdhci_host *host)
 	 * Maximum block size. This varies from controller to controller and
 	 * is specified in the capabilities register.
 	 */
-	mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >> SDHCI_MAX_BLOCK_SHIFT;
-	if (mmc->max_blk_size >= 3) {
-		printk(KERN_WARNING "%s: Invalid maximum block size, "
-			"assuming 512 bytes\n", mmc_hostname(mmc));
-		mmc->max_blk_size = 512;
-	} else
-		mmc->max_blk_size = 512 << mmc->max_blk_size;
+	if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096) {
+		mmc->max_blk_size = 3;
+	} else {
+		mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >>
+				SDHCI_MAX_BLOCK_SHIFT;
+		if (mmc->max_blk_size >= 3) {
+			printk(KERN_WARNING "%s: Invalid maximum block size, "
+				"assuming 512 bytes\n", mmc_hostname(mmc));
+			mmc->max_blk_size = 0;
+		}
+	}
+
+	mmc->max_blk_size = 512 << mmc->max_blk_size;
 
 	/*
 	 * Maximum block count.
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 5c5a950..c8628b4 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -231,6 +231,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_PIO_NEEDS_DELAY			(1<<20)
 /* Controller losing signal/interrupt enable states after reset */
 #define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET		(1<<21)
+/* Controller supports nonstandard maximum block length of 4096 bytes */
+#define SDHCI_QUIRK_MAX_BLK_SZ_4096			(1<<22)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 13/13] mmc: Add OpenFirmware bindings for SDHCI driver
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (11 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes Anton Vorontsov
@ 2009-02-13 14:47 ` Anton Vorontsov
  2009-02-17 16:31 ` [PATCH RFC 0/13] FSL eSDHC support Ben Dooks
  13 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 14:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

This patch adds a new driver: sdhci-of. The driver is similar to
the sdhci-pci, it contains common probe code, and controller-specific
ops and quirks.

So far there are only Freescale eSDHC ops and quirks.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mmc/host/Kconfig    |   10 ++
 drivers/mmc/host/Makefile   |    1 +
 drivers/mmc/host/sdhci-of.c |  286 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 297 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-of.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 99d4b28..73b79e1 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -65,6 +65,16 @@ config MMC_RICOH_MMC
 
 	  If unsure, say Y.
 
+config MMC_SDHCI_OF
+	tristate "SDHCI support on OpenFirmware platforms"
+	depends on MMC_SDHCI && PPC_OF
+	help
+	  This selects the OF support for Secure Digital Host Controller
+	  Interfaces. So far, only the Freescale eSDHC controller is known
+	  to exist on OF platforms.
+
+	  If unsure, say N.
+
 config MMC_OMAP
 	tristate "TI OMAP Multimedia Card Interface support"
 	depends on ARCH_OMAP
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index dedec55..dd512d9 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MMC_MXC)		+= mxcmmc.o
 obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
 obj-$(CONFIG_MMC_RICOH_MMC)	+= ricoh_mmc.o
+obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
 obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
 obj-$(CONFIG_MMC_OMAP)		+= omap.o
diff --git a/drivers/mmc/host/sdhci-of.c b/drivers/mmc/host/sdhci-of.c
new file mode 100644
index 0000000..ee7a847
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of.c
@@ -0,0 +1,286 @@
+/*
+ * OpenFirmware bindings for Secure Digital Host Controller Interface.
+ *
+ * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Copyright (c) 2009 MontaVista Software, Inc.
+ *
+ * Authors: Xiaobo Xie <X.Xie@freescale.com>
+ *	    Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/mmc/host.h>
+#include "sdhci.h"
+
+struct sdhci_of_data {
+	unsigned int quirks;
+	struct sdhci_ops ops;
+};
+
+struct sdhci_of_host {
+	unsigned int clock;
+};
+
+/*
+ * Ops and quirks for the Freescale eSDHC controller.
+ */
+
+#define ESDHC_DMA_SYSCTL	0x40c
+#define ESDHC_DMA_SNOOP		0x00000040
+
+#define ESDHC_SYSTEM_CONTROL	0x2c
+#define ESDHC_CLOCK_MASK	0x0000fff0
+#define ESDHC_PREDIV_SHIFT	8
+#define ESDHC_DIVIDER_SHIFT	4
+#define ESDHC_CLOCK_PEREN	0x00000004
+#define ESDHC_CLOCK_HCKEN	0x00000002
+#define ESDHC_CLOCK_IPGEN	0x00000001
+
+static u32 esdhc_readl(struct sdhci_host *host, int reg)
+{
+	return in_be32(host->ioaddr + reg);
+}
+
+static u16 esdhc_readw(struct sdhci_host *host, int reg)
+{
+	return in_be16(host->ioaddr + (reg ^ 0x2));
+}
+
+static u8 esdhc_readb(struct sdhci_host *host, int reg)
+{
+	return in_8(host->ioaddr + (reg ^ 0x3));
+}
+
+static void esdhc_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	out_be32(host->ioaddr + reg, val);
+}
+
+static void esdhc_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	int base = reg & ~0x3;
+	int shift = (reg & 0x2) * 8;
+
+	clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
+}
+
+static void esdhc_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	int base = reg & ~0x3;
+	int shift = (reg & 0x3) * 8;
+
+	clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
+}
+
+static void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	int div;
+	int pre_div = 1;
+
+	setbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL, ESDHC_CLOCK_IPGEN |
+		  ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN);
+
+	clrbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL, ESDHC_CLOCK_MASK);
+
+	if (clock == 0)
+		goto out;
+
+	if (host->max_clk / 16 > clock) {
+		for (; pre_div < 256; pre_div *= 2) {
+			if (host->max_clk / pre_div < clock * 16)
+				break;
+		}
+	}
+
+	for (div = 1; div <= 16; div++) {
+		if (host->max_clk / (div * pre_div) <= clock)
+			break;
+	}
+
+	pre_div >>= 1;
+	div -= 1;
+
+	setbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL,
+		div << ESDHC_DIVIDER_SHIFT | pre_div << ESDHC_PREDIV_SHIFT);
+	mdelay(10);
+out:
+	host->clock = clock;
+}
+
+static int esdhc_enable_dma(struct sdhci_host *host)
+{
+	setbits32(host->ioaddr + ESDHC_DMA_SYSCTL, ESDHC_DMA_SNOOP);
+	return 0;
+}
+
+static unsigned int esdhc_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_of_host *of_host = sdhci_priv(host);
+
+	return of_host->clock;
+}
+
+static unsigned int esdhc_get_timeout_clock(struct sdhci_host *host)
+{
+	struct sdhci_of_host *of_host = sdhci_priv(host);
+
+	return of_host->clock / 1000;
+}
+
+static struct sdhci_of_data sdhci_esdhc = {
+	.quirks = SDHCI_QUIRK_32BIT_REGISTERS |
+		  SDHCI_QUIRK_BROKEN_CARD_DETECTION |
+		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
+		  SDHCI_QUIRK_NO_BUSY_IRQ |
+		  SDHCI_QUIRK_NONSTANDARD_CLOCK |
+		  SDHCI_QUIRK_PIO_NEEDS_DELAY |
+		  SDHCI_QUIRK_MAX_BLK_SZ_4096 |
+		  SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |
+		  SDHCI_QUIRK_NO_CARD_NO_RESET,
+	.ops = {
+		.readl = esdhc_readl,
+		.readw = esdhc_readw,
+		.readb = esdhc_readb,
+		.writel = esdhc_writel,
+		.writew = esdhc_writew,
+		.writeb = esdhc_writeb,
+		.set_clock = esdhc_set_clock,
+		.enable_dma = esdhc_enable_dma,
+		.get_max_clock = esdhc_get_max_clock,
+		.get_timeout_clock = esdhc_get_timeout_clock,
+	},
+};
+
+#ifdef CONFIG_PM
+
+static int sdhci_of_suspend(struct of_device *ofdev, pm_message_t state)
+{
+	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
+
+	return mmc_suspend_host(host->mmc, state);
+}
+
+static int sdhci_of_resume(struct of_device *ofdev)
+{
+	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
+
+	return mmc_resume_host(host->mmc);
+}
+
+#else
+
+#define sdhci_of_suspend NULL
+#define sdhci_of_resume NULL
+
+#endif
+
+static int __devinit sdhci_of_probe(struct of_device *ofdev,
+				 const struct of_device_id *match)
+{
+	struct device_node *np = ofdev->node;
+	struct sdhci_of_data *sdhci_of_data = match->data;
+	struct sdhci_host *host;
+	struct sdhci_of_host *of_host;
+	const u32 *clk;
+	int size;
+	int ret;
+
+	host = sdhci_alloc_host(&ofdev->dev, sizeof(*of_host));
+	if (!host)
+		return -ENOMEM;
+
+	of_host = sdhci_priv(host);
+	dev_set_drvdata(&ofdev->dev, host);
+
+	host->ioaddr = of_iomap(np, 0);
+	if (!host->ioaddr) {
+		ret = -ENOMEM;
+		goto err_addr_map;
+	}
+
+	host->irq = irq_of_parse_and_map(np, 0);
+	if (!host->irq) {
+		ret = -EINVAL;
+		goto err_no_irq;
+	}
+
+	host->hw_name = dev_name(&ofdev->dev);
+	if (sdhci_of_data) {
+		host->quirks = sdhci_of_data->quirks;
+		host->ops = &sdhci_of_data->ops;
+	}
+
+	clk = of_get_property(np, "clock-frequency", &size);
+	if (clk && size == sizeof(*clk) && *clk)
+		of_host->clock = *clk;
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
+	return 0;
+
+err_add_host:
+	irq_dispose_mapping(host->irq);
+err_no_irq:
+	iounmap(host->ioaddr);
+err_addr_map:
+	sdhci_free_host(host);
+	return ret;
+}
+
+static int __devexit sdhci_of_remove(struct of_device *ofdev)
+{
+	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
+
+	sdhci_remove_host(host, 0);
+	sdhci_free_host(host);
+	irq_dispose_mapping(host->irq);
+	iounmap(host->ioaddr);
+	return 0;
+}
+
+static const struct of_device_id sdhci_of_match[] = {
+	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc, },
+	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc, },
+	{ .compatible = "generic-sdhci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sdhci_of_match);
+
+static struct of_platform_driver sdhci_of_driver = {
+	.driver.name = "sdhci-of",
+	.match_table = sdhci_of_match,
+	.probe = sdhci_of_probe,
+	.remove = __devexit_p(sdhci_of_remove),
+	.suspend = sdhci_of_suspend,
+	.resume	= sdhci_of_resume,
+};
+
+static int __init sdhci_of_init(void)
+{
+	return of_register_platform_driver(&sdhci_of_driver);
+}
+module_init(sdhci_of_init);
+
+static void __exit sdhci_of_exit(void)
+{
+	of_unregister_platform_driver(&sdhci_of_driver);
+}
+module_exit(sdhci_of_exit);
+
+MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
+MODULE_AUTHOR("Xiaobo Xie <X.Xie@freescale.com>, "
+	      "Anton Vorontsov <avorontsov@ru.mvista.com>");
+MODULE_LICENSE("GPL");
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset
  2009-02-13 14:47 ` [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset Anton Vorontsov
@ 2009-02-13 15:47   ` Laurent Pinchart
  2009-02-13 17:30     ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2009-02-13 15:47 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: sdhci-devel, Arnd Bergmann, Liu Dave, linux-kernel, Ben Dooks,
	Pierre Ossman

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

Hi Anton,

On Friday 13 February 2009 15:47:38 Anton Vorontsov wrote:
> FSL eSDHC controllers losing signal/interrupt enable states after
> reset, so we should re-enable them.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  drivers/mmc/host/sdhci.c |    7 +++++++
>  drivers/mmc/host/sdhci.h |    2 ++
>  2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index eff615d..b308dbf 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -189,6 +189,7 @@ static void sdhci_disable_card_detection(struct
> sdhci_host *host) static void sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
>  	unsigned long timeout;
> +	u32 ier = 0; /* shut up gcc */

You should use

	u32 uninitialized_var(ier);

instead to avoid generating extra code.

>
>  	if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) {
>  		if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
> @@ -196,6 +197,9 @@ static void sdhci_reset(struct sdhci_host *host, u8
> mask) return;
>  	}
>
> +	if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
> +		ier = sdhci_readl(host, SDHCI_INT_ENABLE);
> +
>  	sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET);
>
>  	if (mask & SDHCI_RESET_ALL)
> @@ -215,6 +219,9 @@ static void sdhci_reset(struct sdhci_host *host, u8
> mask) timeout--;
>  		mdelay(1);
>  	}
> +
> +	if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
> +		sdhci_clear_set_irqs(host, SDHCI_INT_ALL_MASK, ier);
>  }
>
>  static void sdhci_init(struct sdhci_host *host)
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 44c820a..5c5a950 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -229,6 +229,8 @@ struct sdhci_host {
>  #define SDHCI_QUIRK_NONSTANDARD_CLOCK			(1<<19)
>  /* Controller does not like fast PIO transfers */
>  #define SDHCI_QUIRK_PIO_NEEDS_DELAY			(1<<20)
> +/* Controller losing signal/interrupt enable states after reset */
> +#define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET		(1<<21)
>
>  	int			irq;		/* Device IRQ */
>  	void __iomem *		ioaddr;		/* Mapped address */

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset
  2009-02-13 15:47   ` Laurent Pinchart
@ 2009-02-13 17:30     ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-13 17:30 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: sdhci-devel, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	Ben Dooks, Pierre Ossman

On Fri, Feb 13, 2009 at 04:47:13PM +0100, Laurent Pinchart wrote:
> Hi Anton,
> 
> On Friday 13 February 2009 15:47:38 Anton Vorontsov wrote:
[...]
> > sdhci_host *host) static void sdhci_reset(struct sdhci_host *host, u8 mask)
> >  {
> >  	unsigned long timeout;
> > +	u32 ier = 0; /* shut up gcc */
> 
> You should use
> 
> 	u32 uninitialized_var(ier);
> 
> instead to avoid generating extra code.

Thanks Laurent, will fix.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH RFC 0/13] FSL eSDHC support
  2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
                   ` (12 preceding siblings ...)
  2009-02-13 14:47 ` [PATCH 13/13] mmc: Add OpenFirmware bindings for SDHCI driver Anton Vorontsov
@ 2009-02-17 16:31 ` Ben Dooks
  13 siblings, 0 replies; 29+ messages in thread
From: Ben Dooks @ 2009-02-17 16:31 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

On Fri, Feb 13, 2009 at 05:46:30PM +0300, Anton Vorontsov wrote:
> Hi all,
> 
> Thanks for the comments on the previous version, here comes another
> RFC...
> 
> Changes since the second RFC:
> - Addressed all comments that were raised by Pierre Ossman.
>   There were too many to mention them all, so here is the link:
>   http://lkml.org/lkml/2009/2/6/320
> 
> Changes since the first RFC:
> - Use of_iomap() in sdhci-of.c (suggested by Arnd Bergmann). Also added
>   Arnd's Acked-by: line for the sdhci-of patch.
> - Kconfig help text improved (thanks to Matt Sealey and M. Warner Losh).
> - In "sdhci: Add quirk to suppress PIO interrupts during DMA transfers"
>   patch: sdhci_init() now clears SDHCI_PIO_DISABLED flag, otherwise we
>   won't disable PIO interrupts after suspend.
> - New patch: "sdhci: Add type checking for IO memory accessors"

It would be useful to know what Pierre thinks of these, I would like
to update the sdhci-s3c binding before the next merge window.

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init()
  2009-02-13 14:47 ` [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init() Anton Vorontsov
@ 2009-02-21 15:58   ` Pierre Ossman
  0 siblings, 0 replies; 29+ messages in thread
From: Pierre Ossman @ 2009-02-21 15:58 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

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

On Fri, 13 Feb 2009 17:47:15 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> Card detection interrupts should be handled separately as they should
> not be enabled before mmc_add_host() returns and should be disabled
> before calling mmc_remove_host(). The same is for suspend and resume
> routines.
> 
> sdhci_init() no longer enables card-detection irqs. Instead, two new
> functions implemented: sdhci_enable_card_detection() and
> sdhci_disable_card_detection().
> 
> New sdhci_reinit() call implemented to behave the same way as the old
> sdhci_init().
> 
> Also, this patch implements and uses few new helpers to manage IRQs in
> a more conveinient way, that is:
> 
> - sdhci_clear_set_irqs()
> - sdhci_unmask_irqs()
> - sdhci_mask_irqs()
> - SDHCI_INT_ALL_MASK constant
> 
> sdhci_enable_sdio_irq() converted to these new helpers, plus the
> helpers will be used by the subsequent patches.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---

That's a lot of indirection, but fair enough. :)

> @@ -1792,6 +1832,8 @@ int sdhci_add_host(struct sdhci_host *host)
>  
>  	mmc_add_host(mmc);
>  
> +	sdhci_enable_card_detection(host);
> +
>  	printk(KERN_INFO "%s: SDHCI controller on %s [%s] using %s%s\n",
>  		mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>  		(host->flags & SDHCI_USE_ADMA)?"A":"",

There is a small race here, but I'm not sure it's worth dealing with.

> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index e907441..45c8309 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -124,6 +124,10 @@
>  		SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL | \
>  		SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
>  		SDHCI_INT_DATA_END_BIT)
> +#define SDHCI_INT_ALL_MASK	(SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK | \
> +		SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE | \
> +		SDHCI_INT_CARD_INT | SDHCI_INT_ERROR | SDHCI_INT_BUS_POWER | \
> +		SDHCI_INT_ACMD12ERR | SDHCI_INT_ADMA_ERROR)
>  

In the context this is used, why not just use (unsigned)-1?

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/13] sdhci: Add support for card-detection polling
  2009-02-13 14:47 ` [PATCH 05/13] sdhci: Add support for card-detection polling Anton Vorontsov
@ 2009-02-21 15:58   ` Pierre Ossman
  2009-03-04 17:49     ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Pierre Ossman @ 2009-02-21 15:58 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

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

On Fri, 13 Feb 2009 17:47:18 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> @@ -1110,13 +1113,18 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  
>  	host->mrq = mrq;
>  
> +	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> +		goto send;
> +
>  	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
>  		|| (host->flags & SDHCI_DEVICE_DEAD)) {
>  		host->mrq->cmd->error = -ENOMEDIUM;
>  		tasklet_schedule(&host->finish_tasklet);
> -	} else
> -		sdhci_send_command(host, mrq->cmd);
> -
> +		goto out;
> +	}
> +send:
> +	sdhci_send_command(host, mrq->cmd);
> +out:
>  	mmiowb();
>  	spin_unlock_irqrestore(&host->lock, flags);
>  }

goto:s seem unnecessary here, and your patch is even incorrect as it
ignores the SDHCI_DEVICE_DEAD flag. Just modify the if-clause and
things will work.

Might want to add a comment also to make it more obvious what the
if-clause does.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing
  2009-02-13 14:47 ` [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing Anton Vorontsov
@ 2009-02-21 15:58   ` Pierre Ossman
  2009-03-04 17:48     ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Pierre Ossman @ 2009-02-21 15:58 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

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

On Fri, 13 Feb 2009 17:47:22 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> SDHCI driver must take special care when working with "triggering"
> registers on hosts with strict 32 bit addressing.
> 
> In FSL eSDHC hosts all registers are 32 bit width, writing to the
> first half of any register will cause [undefined?] write the second
> half of the register. That is, 16 bit write to the TRANSFER_MODE
> register, makes hardware see a bogus write to the COMMAND register
> (these two registers are adjacent).
> 
> This patch adds SDHCI_QUIRK_32BIT_REGISTERS quirk. When specified,
> the sdhci driver will try to "pack" all dangerous writes into single
> 32 bit write transaction.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---

What about the other places where we have 16 and 8 bit registers?

-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes
  2009-02-13 14:47 ` [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes Anton Vorontsov
@ 2009-02-21 15:58   ` Pierre Ossman
  2009-03-04 17:47     ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Pierre Ossman @ 2009-02-21 15:58 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

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

On Fri, 13 Feb 2009 17:47:39 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> @@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
>  	sdhci_set_transfer_irqs(host);
>  
>  	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
> -	sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE);
> +	if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096)
> +		blksz = data->blksz;
> +	else
> +		blksz = SDHCI_MAKE_BLKSZ(7, data->blksz);
> +
> +	sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE);
>  	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
>  }
>  

Hmm.. I seem to have overlooked this part previously. I guess they've
basically stripped out the DMA boundary stuff and used the bits for
other things?

At this point I'm leaning more towards simply not supporting their
extended block size. After all, is it ever used?

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes
  2009-02-21 15:58   ` Pierre Ossman
@ 2009-03-04 17:47     ` Anton Vorontsov
  2009-03-08 14:21       ` Pierre Ossman
  0 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-03-04 17:47 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

On Sat, Feb 21, 2009 at 04:58:44PM +0100, Pierre Ossman wrote:
> On Fri, 13 Feb 2009 17:47:39 +0300
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > @@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
> >  	sdhci_set_transfer_irqs(host);
> >  
> >  	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
> > -	sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE);
> > +	if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096)
> > +		blksz = data->blksz;
> > +	else
> > +		blksz = SDHCI_MAKE_BLKSZ(7, data->blksz);
> > +
> > +	sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE);
> >  	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
> >  }
> >  
> 
> Hmm.. I seem to have overlooked this part previously. I guess they've
> basically stripped out the DMA boundary stuff and used the bits for
> other things?

Yes, the last two "DMA boundary" bits are reserved, and the first
one is re-used for blksz of 4096 bytes.

> At this point I'm leaning more towards simply not supporting their
> extended block size.

Eh. But well, OK. We can always persuade you later. :-)

I'll get rid of this particular patch, and put some BLOCK_SIZE
magic into the writew accessor (to clean the DMA bits) instead.

Though, I'll prepare another patch to force blksz to 2048, since
eSDHC specifies "3" in the blksz capability bitfield, and that
causes SDHCI core to fall back to the 512 byte blocks.

> After all, is it ever used?

Not sure, maybe `dd bs=' can use it? A bit lazy to check this
right now, but from the quick tests, enabling/disabling "blksz
of 4096 bytes" doesn't cause any performance change. At least
with the ordinary SD cards.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing
  2009-02-21 15:58   ` Pierre Ossman
@ 2009-03-04 17:48     ` Anton Vorontsov
  2009-03-08 14:17       ` Pierre Ossman
  0 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-03-04 17:48 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

On Sat, Feb 21, 2009 at 04:58:33PM +0100, Pierre Ossman wrote:
> On Fri, 13 Feb 2009 17:47:22 +0300
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > SDHCI driver must take special care when working with "triggering"
> > registers on hosts with strict 32 bit addressing.
> > 
> > In FSL eSDHC hosts all registers are 32 bit width, writing to the
> > first half of any register will cause [undefined?] write the second
> > half of the register. That is, 16 bit write to the TRANSFER_MODE
> > register, makes hardware see a bogus write to the COMMAND register
> > (these two registers are adjacent).
> > 
> > This patch adds SDHCI_QUIRK_32BIT_REGISTERS quirk. When specified,
> > the sdhci driver will try to "pack" all dangerous writes into single
> > 32 bit write transaction.
> > 
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > ---
> 
> What about the other places where we have 16 and 8 bit registers?

These are handled by the accessors:

+static void esdhc_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+       int base = reg & ~0x3;
+       int shift = (reg & 0x3) * 8;
+
+       clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);

^^ See, we're always issuing 32bit ops.

The point of this patch is to take care of "triggering" registers,
i.e. those registers that are used trigger "send some data" command.

There is just one (from the eSDHC point of view) such register:
TRANSFER_MODE+COMMAND (which is a single 32 bit register in eSDHC,
but two independant word-sized registers in standard SDHCI).

That register must be accessed with 32bit ops just _once_ per
data transfer, not two 32bit writes (half of a register one time,
and half of a register next time -- this won't work).

But I see the point of confusion... Instead of teaching
"SDHCI core" to work with 32 bits hosts, we'd better handle this
in the eSDHC part, in the accessors.

This is relatively trivial and should not cause much overhead
(at least when using DMA), just a small state machine with
the xfer mode register shadowed in software (plus, notice that
this also handles BLOCK_SIZE, as I promised in another email):

diff --git a/drivers/mmc/host/sdhci-of.c b/drivers/mmc/host/sdhci-of.c
index 22bf006..5100d1d 100644
--- a/drivers/mmc/host/sdhci-of.c
+++ b/drivers/mmc/host/sdhci-of.c
@@ -30,6 +30,7 @@ struct sdhci_of_data {
 
 struct sdhci_of_host {
 	unsigned int clock;
+	u16 xfer_mode_shadow;
 };
 
 /*
@@ -69,9 +70,31 @@ static void esdhc_writel(struct sdhci_host *host, u32 val, int reg)
 
 static void esdhc_writew(struct sdhci_host *host, u16 val, int reg)
 {
+	struct sdhci_of_host *of_host = sdhci_priv(host);
 	int base = reg & ~0x3;
 	int shift = (reg & 0x2) * 8;
 
+	switch (reg) {
+	case SDHCI_TRANSFER_MODE:
+		/*
+		 * Postpone this write, we must do it together with a
+		 * command write that is down below.
+		 */
+		of_host->xfer_mode_shadow = val;
+		return;
+	case SDHCI_COMMAND:
+		esdhc_writel(host, val << 16 | of_host->xfer_mode_shadow,
+			     SDHCI_TRANSFER_MODE);
+		return;
+	case SDHCI_BLOCK_SIZE:
+		/*
+		 * Two last DMA bits are reserved, and first one is used for
+		 * non-standard blksz of 4096 bytes that we don't support
+		 * yet. So clear the DMA boundary bits.
+		 */
+		val &= ~SDHCI_MAKE_BLKSZ(0x7, 0);
+		/* fall through */
+	}
 	clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
 }
 
@@ -137,13 +160,11 @@ static unsigned int esdhc_get_timeout_clock(struct sdhci_host *host)
 }
 
 static struct sdhci_of_data sdhci_esdhc = {
-	.quirks = SDHCI_QUIRK_32BIT_REGISTERS |
-		  SDHCI_QUIRK_BROKEN_CARD_DETECTION |
+	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
 		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
 		  SDHCI_QUIRK_NO_BUSY_IRQ |
 		  SDHCI_QUIRK_NONSTANDARD_CLOCK |
 		  SDHCI_QUIRK_PIO_NEEDS_DELAY |
-		  SDHCI_QUIRK_MAX_BLK_SZ_4096 |
 		  SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |
 		  SDHCI_QUIRK_NO_CARD_NO_RESET,
 	.ops = {

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/13] sdhci: Add support for card-detection polling
  2009-02-21 15:58   ` Pierre Ossman
@ 2009-03-04 17:49     ` Anton Vorontsov
  2009-03-08 14:11       ` Pierre Ossman
  0 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2009-03-04 17:49 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel, Pierre Ossman

On Sat, Feb 21, 2009 at 04:58:21PM +0100, Pierre Ossman wrote:
> On Fri, 13 Feb 2009 17:47:18 +0300
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > @@ -1110,13 +1113,18 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> >  
> >  	host->mrq = mrq;
> >  
> > +	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> > +		goto send;
> > +
> >  	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
> >  		|| (host->flags & SDHCI_DEVICE_DEAD)) {
> >  		host->mrq->cmd->error = -ENOMEDIUM;
> >  		tasklet_schedule(&host->finish_tasklet);
> > -	} else
> > -		sdhci_send_command(host, mrq->cmd);
> > -
> > +		goto out;
> > +	}
> > +send:
> > +	sdhci_send_command(host, mrq->cmd);
> > +out:
> >  	mmiowb();
> >  	spin_unlock_irqrestore(&host->lock, flags);
> >  }
> 
> goto:s seem unnecessary here, and your patch is even incorrect as it
> ignores the SDHCI_DEVICE_DEAD flag.

Oops.

> Just modify the if-clause and
> things will work.

That would look horrid...

        if ((!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
                        !(sdhci_readl(host, SDHCI_PRESENT_STATE) &
                                SDHCI_CARD_PRESENT)) ||
                        (host->flags & SDHCI_DEVICE_DEAD)) {

> Might want to add a comment also to make it more obvious what the
> if-clause does.

Let's try to avoid the if-clause above? How about this:

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0cbde8e..d71c877 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -167,6 +167,9 @@ static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
 {
 	u32 irqs = SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		return;
+
 	if (enable)
 		sdhci_unmask_irqs(host, irqs);
 	else
@@ -1096,6 +1099,7 @@ out:
 static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct sdhci_host *host;
+	bool present;
 	unsigned long flags;
 
 	host = mmc_priv(mmc);
@@ -1110,8 +1114,14 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	host->mrq = mrq;
 
-	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
-		|| (host->flags & SDHCI_DEVICE_DEAD)) {
+	/* If polling, assume that the card is always present. */
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		present = true;
+	else
+		present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
+				SDHCI_CARD_PRESENT;
+
+	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
 	} else
@@ -1745,6 +1755,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	mmc->ocr_avail = 0;
 	if (caps & SDHCI_CAN_VDD_330)
 		mmc->ocr_avail |= MMC_VDD_32_33|MMC_VDD_33_34;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 1c29895..09a4363 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -212,6 +212,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_BROKEN_SMALL_PIO			(1<<13)
 /* Controller does not provide transfer-complete interrupt when not busy */
 #define SDHCI_QUIRK_NO_BUSY_IRQ				(1<<14)
+/* Controller has unreliable card detection */
+#define SDHCI_QUIRK_BROKEN_CARD_DETECTION		(1<<15)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/13] sdhci: Add support for card-detection polling
  2009-03-04 17:49     ` Anton Vorontsov
@ 2009-03-08 14:11       ` Pierre Ossman
  2009-03-16 21:05         ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Pierre Ossman @ 2009-03-08 14:11 UTC (permalink / raw)
  To: avorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

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

On Wed, 4 Mar 2009 20:49:17 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> On Sat, Feb 21, 2009 at 04:58:21PM +0100, Pierre Ossman wrote:
> > Just modify the if-clause and
> > things will work.
> 
> That would look horrid...
> 
>         if ((!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>                         !(sdhci_readl(host, SDHCI_PRESENT_STATE) &
>                                 SDHCI_CARD_PRESENT)) ||
>                         (host->flags & SDHCI_DEVICE_DEAD)) {
> 

There are worse ones in that code, but I see your point. :)

> > Might want to add a comment also to make it more obvious what the
> > if-clause does.
> 
> Let's try to avoid the if-clause above? How about this:
> 

Looks ok.

> @@ -1096,6 +1099,7 @@ out:
>  static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  {
>  	struct sdhci_host *host;
> +	bool present;
>  	unsigned long flags;
>  
>  	host = mmc_priv(mmc);

Can we use bool in the kernel?

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing
  2009-03-04 17:48     ` Anton Vorontsov
@ 2009-03-08 14:17       ` Pierre Ossman
  0 siblings, 0 replies; 29+ messages in thread
From: Pierre Ossman @ 2009-03-08 14:17 UTC (permalink / raw)
  To: avorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

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

On Wed, 4 Mar 2009 20:48:45 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> But I see the point of confusion... Instead of teaching
> "SDHCI core" to work with 32 bits hosts, we'd better handle this
> in the eSDHC part, in the accessors.
> 
> This is relatively trivial and should not cause much overhead
> (at least when using DMA), just a small state machine with
> the xfer mode register shadowed in software (plus, notice that
> this also handles BLOCK_SIZE, as I promised in another email):
> 

Me like. Keeps my life a lot saner. :)

Just be aware that there is a remote risk of breakage as people hacking
on sdhci-core won't be aware of esdhc's, let's call it unique,
behaviour. Some testing now and then on your part would be prudent. :)

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes
  2009-03-04 17:47     ` Anton Vorontsov
@ 2009-03-08 14:21       ` Pierre Ossman
  0 siblings, 0 replies; 29+ messages in thread
From: Pierre Ossman @ 2009-03-08 14:21 UTC (permalink / raw)
  To: avorontsov
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

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

On Wed, 4 Mar 2009 20:47:44 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> 
> I'll get rid of this particular patch, and put some BLOCK_SIZE
> magic into the writew accessor (to clean the DMA bits) instead.
> 
> Though, I'll prepare another patch to force blksz to 2048, since
> eSDHC specifies "3" in the blksz capability bitfield, and that
> causes SDHCI core to fall back to the 512 byte blocks.
> 

Ok.

> > After all, is it ever used?
> 
> Not sure, maybe `dd bs=' can use it? A bit lazy to check this
> right now, but from the quick tests, enabling/disabling "blksz
> of 4096 bytes" doesn't cause any performance change. At least
> with the ordinary SD cards.
> 

Memory cards will not use this (at least not with the current
standards), as the block layer thinks in 512 byte blocks. Also, the
sector size propagates to user space in a way that causes filesystems
to behave differently, making cards incompatible with all other
operating systems (i.e. if we don't use 512 byte blocks).

So the only scenario where this might be used is SDIO, and I'm not sure
such big blocks are a win there either because of the overhead of
changing block size.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/13] sdhci: Add support for card-detection polling
  2009-03-08 14:11       ` Pierre Ossman
@ 2009-03-16 21:05         ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-03-16 21:05 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

On Sun, Mar 08, 2009 at 03:11:21PM +0100, Pierre Ossman wrote:
> On Wed, 4 Mar 2009 20:49:17 +0300
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > On Sat, Feb 21, 2009 at 04:58:21PM +0100, Pierre Ossman wrote:
> > > Just modify the if-clause and
> > > things will work.
> > 
> > That would look horrid...
> > 
> >         if ((!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> >                         !(sdhci_readl(host, SDHCI_PRESENT_STATE) &
> >                                 SDHCI_CARD_PRESENT)) ||
> >                         (host->flags & SDHCI_DEVICE_DEAD)) {
> > 
> 
> There are worse ones in that code, but I see your point. :)
> 
> > > Might want to add a comment also to make it more obvious what the
> > > if-clause does.
> > 
> > Let's try to avoid the if-clause above? How about this:
> > 
> 
> Looks ok.
> 
> > @@ -1096,6 +1099,7 @@ out:
> >  static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> >  {
> >  	struct sdhci_host *host;
> > +	bool present;
> >  	unsigned long flags;
> >  
> >  	host = mmc_priv(mmc);
> 
> Can we use bool in the kernel?

Sure, it's widely used in the kernel, even in such places as
mm/ or kernel/.


Thanks for the review!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 05/13] sdhci: Add support for card-detection polling
  2009-02-20 17:32 [PATCH " Anton Vorontsov
@ 2009-02-20 17:33 ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2009-02-20 17:33 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Ben Dooks, Arnd Bergmann, Liu Dave, linux-kernel, linuxppc-dev,
	sdhci-devel

This patch adds SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk. When specified,
sdhci driver will set MMC_CAP_NEEDS_POLL MMC host capability, and won't
enable card insert/remove interrupts.

This is needed for hosts with unreliable card detection, such as FSL
eSDHC. The original eSDHC driver was tring to "debounce" card-detection
IRQs by reading present state and disabling particular interrupts. But
with this debouncing scheme I noticed that sometimes we miss card
insertion/removal events.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   17 ++++++++++++++---
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2728e90..782fb58 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -167,6 +167,9 @@ static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
 {
 	u32 irqs = SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		return;
+
 	if (enable)
 		sdhci_unmask_irqs(host, irqs);
 	else
@@ -1110,13 +1113,18 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	host->mrq = mrq;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		goto send;
+
 	if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
 		|| (host->flags & SDHCI_DEVICE_DEAD)) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
-	} else
-		sdhci_send_command(host, mrq->cmd);
-
+		goto out;
+	}
+send:
+	sdhci_send_command(host, mrq->cmd);
+out:
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
 }
@@ -1742,6 +1750,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	mmc->f_max = host->max_clk;
 	mmc->caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	if ((caps & SDHCI_CAN_DO_HISPD) ||
 		(host->quirks & SDHCI_QUIRK_FORCE_HIGHSPEED))
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 45c8309..b10920d 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -217,6 +217,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_FORCE_HIGHSPEED			(1<<14)
 /* Controller does not provide transfer-complete interrupt when not busy */
 #define SDHCI_QUIRK_NO_BUSY_IRQ				(1<<15)
+/* Controller has unreliable card detection */
+#define SDHCI_QUIRK_BROKEN_CARD_DETECTION		(1<<16)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2009-03-16 21:05 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-13 14:46 [PATCH RFC 0/13] FSL eSDHC support Anton Vorontsov
2009-02-13 14:47 ` [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ Anton Vorontsov
2009-02-13 14:47 ` [PATCH 02/13] sdhci: Add support for bus-specific IO memory accessors Anton Vorontsov
2009-02-13 14:47 ` [PATCH 03/13] sdhci: Split card-detection IRQs management from sdhci_init() Anton Vorontsov
2009-02-21 15:58   ` Pierre Ossman
2009-02-13 14:47 ` [PATCH 04/13] sdhci: Enable only relevant (DMA/PIO) interrupts during transfers Anton Vorontsov
2009-02-13 14:47 ` [PATCH 05/13] sdhci: Add support for card-detection polling Anton Vorontsov
2009-02-21 15:58   ` Pierre Ossman
2009-03-04 17:49     ` Anton Vorontsov
2009-03-08 14:11       ` Pierre Ossman
2009-03-16 21:05         ` Anton Vorontsov
2009-02-13 14:47 ` [PATCH 06/13] sdhci: Add support for hosts reporting inverted write-protect state Anton Vorontsov
2009-02-13 14:47 ` [PATCH 07/13] sdhci: Add support for hosts with strict 32 bit addressing Anton Vorontsov
2009-02-21 15:58   ` Pierre Ossman
2009-03-04 17:48     ` Anton Vorontsov
2009-03-08 14:17       ` Pierre Ossman
2009-02-13 14:47 ` [PATCH 08/13] sdhci: Add get_{max,timeout}_clock callbacks Anton Vorontsov
2009-02-13 14:47 ` [PATCH 09/13] sdhci: Add set_clock callback and a quirk for nonstandard clocks Anton Vorontsov
2009-02-13 14:47 ` [PATCH 10/13] sdhci: Add quirk for controllers that need small delays for PIO Anton Vorontsov
2009-02-13 14:47 ` [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset Anton Vorontsov
2009-02-13 15:47   ` Laurent Pinchart
2009-02-13 17:30     ` Anton Vorontsov
2009-02-13 14:47 ` [PATCH 12/13] sdhci: Add quirk for controllers with max. block size up to 4096 bytes Anton Vorontsov
2009-02-21 15:58   ` Pierre Ossman
2009-03-04 17:47     ` Anton Vorontsov
2009-03-08 14:21       ` Pierre Ossman
2009-02-13 14:47 ` [PATCH 13/13] mmc: Add OpenFirmware bindings for SDHCI driver Anton Vorontsov
2009-02-17 16:31 ` [PATCH RFC 0/13] FSL eSDHC support Ben Dooks
2009-02-20 17:32 [PATCH " Anton Vorontsov
2009-02-20 17:33 ` [PATCH 05/13] sdhci: Add support for card-detection polling Anton Vorontsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).