[PATCH RFC 0/10] Freescale "eSDHC" SDHCI support (was [PATCH] mmc: Add driver for Freescale eSDHC controllers)

[PATCH RFC 0/10] Freescale "eSDHC" SDHCI support (was [PATCH] mmc: Add driver for Freescale eSDHC controllers)

[Anton Vorontsov]

On Thu, Jan 15, 2009 at 07:37:00AM +0800, Liu Dave wrote:
> > This patch adds support for the Freescale Enhanced Secure Digital
> > Host Controller Interface as found in some Freescale PowerPC
> > microprocessors (e.g. MPC837x SOCs).
> 
> The Freescale ESDHC controller is basically compatible with the
> standard SDHC controller. but the eshdc expand some bits.
> 
> The esdhc.c is much like the orignal sdhci.c.
> it is possible to merge them.

Seem to work indeed. Quite a lot of quirks needed though.

Patches on the way...

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

[PATCH 01/10] sdhci: Add quirk for controllers with no end-of-busy IRQ

[Anton Vorontsov]

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 4d010a9..72cb7ef 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1286,8 +1286,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 31f4b15..73c03c6 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

[PATCH 02/10] sdhci: Add support for bus-specific IO memory accessors

[Anton Vorontsov]

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

In this patch I swapped write{l,b,w}'s value and address arguments,
I don't like it anymore though. So in the next revision I'll revert
to the traditional arguments order.

Though... with current sdhci_{read,write} accessors it's quite
easy to make a mistake in the arguments, because there is no type
checking.

Pierre, would it be better if I'd use something like this:
host->writel(value, host->ioddr + reg) instead of
sdhci_writel(host, value, reg)? It's longer, but type checked...

 drivers/mmc/host/sdhci.c |  205 ++++++++++++++++++++++++++++------------------
 drivers/mmc/host/sdhci.h |   53 ++++++++++++
 2 files changed, 177 insertions(+), 81 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 72cb7ef..0f5037d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -43,35 +43,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");
 }
@@ -82,17 +82,47 @@ static void sdhci_dumpregs(struct sdhci_host *host)
  *                                                                           *
 \*****************************************************************************/
 
+static u32 sdhci_def_readl(struct sdhci_host *host, int reg)
+{
+	return readl(host->ioaddr + reg);
+}
+
+static u16 sdhci_def_readw(struct sdhci_host *host, int reg)
+{
+	return readw(host->ioaddr + reg);
+}
+
+static u8 sdhci_def_readb(struct sdhci_host *host, int reg)
+{
+	return readb(host->ioaddr + reg);
+}
+
+static void sdhci_def_writel(struct sdhci_host *host, int reg, u32 val)
+{
+	writel(val, host->ioaddr + reg);
+}
+
+static void sdhci_def_writew(struct sdhci_host *host, int reg, u16 val)
+{
+	writew(val, host->ioaddr + reg);
+}
+
+static void sdhci_def_writeb(struct sdhci_host *host, int reg, u8 val)
+{
+	writeb(val, host->ioaddr + reg);
+}
+
 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, SDHCI_SOFTWARE_RESET, mask);
 
 	if (mask & SDHCI_RESET_ALL)
 		host->clock = 0;
@@ -101,7 +131,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);
@@ -127,26 +157,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, SDHCI_INT_ENABLE, intmask);
+	sdhci_writel(host, SDHCI_SIGNAL_ENABLE, intmask);
 }
 
 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, SDHCI_HOST_CONTROL, ctrl);
 }
 
 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, SDHCI_HOST_CONTROL, ctrl);
 }
 
 #ifdef CONFIG_LEDS_CLASS
@@ -200,7 +230,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;
 			}
 
@@ -252,7 +282,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, SDHCI_BUFFER, scratch);
 				chunk = 0;
 				scratch = 0;
 			}
@@ -287,7 +317,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
@@ -576,7 +606,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, SDHCI_TIMEOUT_CONTROL, count);
 
 	if (host->flags & SDHCI_USE_DMA)
 		host->flags |= SDHCI_REQ_USE_DMA;
@@ -656,8 +686,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, SDHCI_ADMA_ADDRESS,
+					host->adma_addr);
 			}
 		} else {
 			int sg_cnt;
@@ -676,8 +706,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, SDHCI_DMA_ADDRESS,
+					sg_dma_address(data->sg));
 			}
 		}
 	}
@@ -688,14 +718,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, SDHCI_HOST_CONTROL, ctrl);
 	}
 
 	if (!(host->flags & SDHCI_REQ_USE_DMA)) {
@@ -705,9 +735,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_BLOCK_SIZE, SDHCI_MAKE_BLKSZ(7, data->blksz));
+	sdhci_writew(host, SDHCI_BLOCK_COUNT, data->blocks);
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
@@ -728,7 +757,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, SDHCI_TRANSFER_MODE, mode);
 }
 
 static void sdhci_finish_data(struct sdhci_host *host)
@@ -797,7 +826,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));
@@ -816,7 +845,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, SDHCI_ARGUMENT, cmd->arg);
 
 	sdhci_set_transfer_mode(host, cmd->data);
 
@@ -844,8 +873,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_COMMAND, SDHCI_MAKE_CMD(cmd->opcode, flags));
 }
 
 static void sdhci_finish_command(struct sdhci_host *host)
@@ -858,15 +886,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);
 		}
 	}
 
@@ -890,7 +918,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, SDHCI_CLOCK_CONTROL, 0);
 
 	if (clock == 0)
 		goto out;
@@ -903,11 +931,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, SDHCI_CLOCK_CONTROL, clk);
 
 	/* 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 "
@@ -920,7 +948,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, SDHCI_CLOCK_CONTROL, clk);
 
 out:
 	host->clock = clock;
@@ -934,7 +962,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, SDHCI_POWER_CONTROL, 0);
 		goto out;
 	}
 
@@ -943,7 +971,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, SDHCI_POWER_CONTROL, 0);
 
 	pwr = SDHCI_POWER_ON;
 
@@ -968,10 +996,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, SDHCI_POWER_CONTROL, pwr & ~SDHCI_POWER_ON);
 
-	writeb(pwr, host->ioaddr + SDHCI_POWER_CONTROL);
+	sdhci_writeb(host, SDHCI_POWER_CONTROL, pwr);
 
 out:
 	host->power = power;
@@ -1000,7 +1027,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);
@@ -1029,7 +1056,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, SDHCI_SIGNAL_ENABLE, 0);
 		sdhci_init(host);
 	}
 
@@ -1040,7 +1067,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;
@@ -1052,7 +1079,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, SDHCI_HOST_CONTROL, ctrl);
 
 	/*
 	 * Some (ENE) controllers go apeshit on some ios operation,
@@ -1080,7 +1107,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);
 
@@ -1100,14 +1127,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, SDHCI_INT_ENABLE, ier);
+	sdhci_writel(host, SDHCI_SIGNAL_ENABLE, ier);
 
 out:
 	mmiowb();
@@ -1137,7 +1164,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));
@@ -1341,8 +1368,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_DMA_ADDRESS,
+				sdhci_readl(host, SDHCI_DMA_ADDRESS));
 
 		if (intmask & SDHCI_INT_DATA_END) {
 			if (host->cmd) {
@@ -1368,7 +1395,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;
@@ -1379,22 +1406,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, SDHCI_INT_STATUS, intmask &
+			(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE));
 		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, SDHCI_INT_STATUS,
+			intmask & SDHCI_INT_CMD_MASK);
 		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, SDHCI_INT_STATUS,
+			intmask & SDHCI_INT_DATA_MASK);
 		sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
 	}
 
@@ -1405,7 +1432,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_STATUS, SDHCI_INT_BUS_POWER);
 	}
 
 	intmask &= ~SDHCI_INT_BUS_POWER;
@@ -1420,7 +1447,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, SDHCI_INT_STATUS, intmask);
 	}
 
 	result = IRQ_HANDLED;
@@ -1530,9 +1557,25 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (debug_quirks)
 		host->quirks = debug_quirks;
 
+	if (host->ops->readl) {
+		host->readl = host->ops->readl;
+		host->readw = host->ops->readw;
+		host->readb = host->ops->readb;
+		host->writel = host->ops->writel;
+		host->writew = host->ops->writew;
+		host->writeb = host->ops->writeb;
+	} else {
+		host->readl = sdhci_def_readl;
+		host->readw = sdhci_def_readw;
+		host->readb = sdhci_def_readb;
+		host->writel = sdhci_def_writel;
+		host->writew = sdhci_def_writew;
+		host->writeb = sdhci_def_writeb;
+	}
+
 	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) {
@@ -1541,7 +1584,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 73c03c6..fed0022 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>
 
 /*
@@ -263,14 +264,66 @@ struct sdhci_host {
 
 	struct timer_list	timer;		/* Timer for timeouts */
 
+	/*
+	 * These accessors duplicate sdhci_ops, but there are two reasons for
+	 * this:
+	 * 1. sdhci_ops are const, so the sdhci driver won't able to assign
+	 *    default ops;
+	 * 2. Using host->X instead of host->ops->X saves us one dereference.
+	 *    This can be useful in PIO mode. (Though the benefit of this
+	 *    is negligibly small).
+	 */
+	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, int reg, u32 val);
+	void		(*writew)(struct sdhci_host *host, int reg, u16 val);
+	void		(*writeb)(struct sdhci_host *host, int reg, u8 val);
+
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
 
 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, int reg, u32 val);
+	void		(*writew)(struct sdhci_host *host, int reg, u16 val);
+	void		(*writeb)(struct sdhci_host *host, int reg, u8 val);
+
 	int		(*enable_dma)(struct sdhci_host *host);
 };
 
+static inline void sdhci_writel(struct sdhci_host *host, int reg, u32 val)
+{
+	host->writel(host, reg, val);
+}
+
+static inline void sdhci_writew(struct sdhci_host *host, int reg, u16 val)
+{
+	host->writew(host, reg, val);
+}
+
+static inline void sdhci_writeb(struct sdhci_host *host, int reg, u8 val)
+{
+	host->writeb(host, reg, val);
+}
+
+static inline u32 sdhci_readl(struct sdhci_host *host, int reg)
+{
+	return host->readl(host, reg);
+}
+
+static inline u16 sdhci_readw(struct sdhci_host *host, int reg)
+{
+	return host->readw(host, reg);
+}
+
+static inline u8 sdhci_readb(struct sdhci_host *host, int reg)
+{
+	return host->readb(host, reg);
+}
 
 extern struct sdhci_host *sdhci_alloc_host(struct device *dev,
 	size_t priv_size);
-- 
1.5.6.5

[PATCH 03/10] sdhci: Add support for card-detection polling

[Anton Vorontsov]

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 |    6 ++++++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0f5037d..5a7a584 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -157,6 +157,9 @@ static void sdhci_init(struct sdhci_host *host)
 		SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
 		SDHCI_INT_ADMA_ERROR;
 
+	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+		intmask &= ~(SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT);
+
 	sdhci_writel(host, SDHCI_INT_ENABLE, intmask);
 	sdhci_writel(host, SDHCI_SIGNAL_ENABLE, intmask);
 }
@@ -1677,6 +1680,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 fed0022..f357722 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -213,6 +213,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

[PATCH 04/10] sdhci: Add support for hosts reporting inverted write-protect state

[Anton Vorontsov]

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 5a7a584..63809f5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1114,6 +1114,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 f357722..9cd2b78 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -215,6 +215,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

[PATCH 05/10] sdhci: Add support for hosts with strict 32 bit addressing

[Anton Vorontsov]

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 |   18 +++++++++++++-----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 63809f5..c3737fe 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -742,13 +742,13 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	sdhci_writew(host, SDHCI_BLOCK_COUNT, data->blocks);
 }
 
-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);
 
@@ -760,7 +760,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, SDHCI_TRANSFER_MODE, mode);
+	return mode;
 }
 
 static void sdhci_finish_data(struct sdhci_host *host)
@@ -812,6 +812,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;
 
@@ -850,7 +851,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, SDHCI_ARGUMENT, cmd->arg);
 
-	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",
@@ -876,7 +877,14 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	if (cmd->data)
 		flags |= SDHCI_CMD_DATA;
 
-	sdhci_writew(host, SDHCI_COMMAND, SDHCI_MAKE_CMD(cmd->opcode, flags));
+	if (host->quirks & SDHCI_QUIRK_32BIT_REGISTERS) {
+		sdhci_writel(host, SDHCI_TRANSFER_MODE,
+			SDHCI_MAKE_CMD_32BIT(cmd->opcode, flags, xfer_mode));
+	} else {
+		sdhci_writew(host, SDHCI_TRANSFER_MODE, xfer_mode);
+		sdhci_writew(host, SDHCI_COMMAND,
+			SDHCI_MAKE_CMD(cmd->opcode, flags));
+	}
 }
 
 static void sdhci_finish_command(struct sdhci_host *host)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 9cd2b78..2747340 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
 
@@ -217,6 +219,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

[PATCH 06/10] sdhci: Add quirk to suppress PIO interrupts during DMA transfers

[Anton Vorontsov]

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 adds SDHCI_QUIRK_PIO_IRQS_DURING_DMA quirk. When specified,
the sdhci driver will disable PIO interrupts during DMA transfers.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c3737fe..57b8ffe 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -589,6 +589,33 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_data *data)
 	return count;
 }
 
+static void sdhci_set_pio_irqs(struct sdhci_host *host, bool state)
+{
+	bool current_state = !(host->flags & SDHCI_PIO_DISABLED);
+	u32 ier;
+
+	/*
+	 * We only care about PIO IRQs if the host issues PIO IRQs during
+	 * DMA transfers. Otherwise we can keep the irqs always enabled.
+	 */
+	if (!(host->quirks & SDHCI_QUIRK_PIO_IRQS_DURING_DMA))
+		return;
+
+	if (current_state == state)
+		return;
+
+	ier = sdhci_readl(host, SDHCI_INT_ENABLE);
+	if (state) {
+		ier |= SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL;
+		host->flags &= ~SDHCI_PIO_DISABLED;
+	} else {
+		ier &= ~(SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL);
+		host->flags |= SDHCI_PIO_DISABLED;
+	}
+	sdhci_writel(host, SDHCI_INT_ENABLE, ier);
+	sdhci_writel(host, SDHCI_SIGNAL_ENABLE, ier);
+}
+
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 {
 	u8 count;
@@ -735,6 +762,9 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 		sg_miter_start(&host->sg_miter,
 			data->sg, data->sg_len, SG_MITER_ATOMIC);
 		host->blocks = data->blocks;
+		sdhci_set_pio_irqs(host, true);
+	} else {
+		sdhci_set_pio_irqs(host, false);
 	}
 
 	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 2747340..7e3b01f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -221,6 +221,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 issues PIO interrupts during DMA transfers */
+#define SDHCI_QUIRK_PIO_IRQS_DURING_DMA			(1<<19)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
@@ -242,6 +244,7 @@ struct sdhci_host {
 #define SDHCI_USE_ADMA		(1<<1)		/* Host is ADMA capable */
 #define SDHCI_REQ_USE_DMA	(1<<2)		/* Use DMA for this req. */
 #define SDHCI_DEVICE_DEAD	(1<<3)		/* Device unresponsive */
+#define SDHCI_PIO_DISABLED	(1<<4)		/* PIO IRQs disabled */
 
 	unsigned int		version;	/* SDHCI spec. version */
 
-- 
1.5.6.5

[PATCH 07/10] sdhci: Add support for hosts that don't specify clocks in the cap. register

[Anton Vorontsov]

FSL eSDHC hosts don't provide clocks bits in the capabilities register,
instead we're getting clocks values from the device tree.

There is somewhat similar change[1] from Ben Dooks, the change adds
callbacks for getting the clocks. But for eSDHC the callbacks are
superfluous, since the clocks are static.

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

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 57b8ffe..9ac088a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1693,24 +1693,23 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
 	}
 
-	host->max_clk =
-		(caps & SDHCI_CLOCK_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
-	if (host->max_clk == 0) {
-		printk(KERN_ERR "%s: Hardware doesn't specify base clock "
-			"frequency.\n", mmc_hostname(mmc));
-		return -ENODEV;
+	if (!host->max_clk) {
+		host->max_clk = (caps & SDHCI_CLOCK_BASE_MASK) >>
+						SDHCI_CLOCK_BASE_SHIFT;
+		if (host->max_clk == 0) {
+			printk(KERN_ERR "%s: Hardware doesn't specify base "
+				"clock frequency.\n", mmc_hostname(mmc));
+			return -ENODEV;
+		}
+		host->max_clk *= 1000000;
 	}
-	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->timeout_clk) {
+		host->timeout_clk = (caps & SDHCI_TIMEOUT_CLK_MASK) >>
+						SDHCI_TIMEOUT_CLK_SHIFT;
+		if (caps & SDHCI_TIMEOUT_CLK_UNIT)
+			host->timeout_clk *= 1000;
 	}
-	if (caps & SDHCI_TIMEOUT_CLK_UNIT)
-		host->timeout_clk *= 1000;
 
 	/*
 	 * Set host parameters.
-- 
1.5.6.5

[PATCH 08/10] sdhci: Add set_clock callback

[Anton Vorontsov]

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 |    5 +++++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9ac088a..e456ae0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -959,6 +959,11 @@ 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);
+		return;
+	}
+
 	sdhci_writew(host, SDHCI_CLOCK_CONTROL, 0);
 
 	if (clock == 0)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7e3b01f..39cfd5a 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -303,6 +303,8 @@ struct sdhci_ops {
 	void		(*writew)(struct sdhci_host *host, int reg, u16 val);
 	void		(*writeb)(struct sdhci_host *host, int reg, u8 val);
 
+	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);
+
 	int		(*enable_dma)(struct sdhci_host *host);
 };
 
-- 
1.5.6.5

[PATCH 09/10] sdhci: Add quirk for Freescale eSDHC controllers

[Anton Vorontsov]

This patch adds SDHCI_QUIRK_FSL quirk. The quirk is used to instruct
the sdhci driver about various FSL eSDHC host incompatibilities:

1) 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.

   (Should we implement a dedicated quirk for this? I.e.
    SDHCI_QUIRK_MAX_BLK_SZ_4096?)

2) sdhci_init() is needed after error conditions.

   (Can we safely do this for all controllers?)

3) 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 changes before and after the delay. Though, more investigation
   on this is in my todo list.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e456ae0..82085e4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -321,6 +321,10 @@ static void sdhci_transfer_pio(struct sdhci_host *host)
 		mask = ~0;
 
 	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
+		/* FIXME */
+		if (host->quirks & SDHCI_QUIRK_FSL)
+			udelay(100);
+
 		if (host->data->flags & MMC_DATA_READ)
 			sdhci_read_block_pio(host);
 		else
@@ -618,6 +622,7 @@ static void sdhci_set_pio_irqs(struct sdhci_host *host, bool state)
 
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 {
+	u16 blksz;
 	u8 count;
 	u8 ctrl;
 	int ret;
@@ -768,7 +773,12 @@ 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) */
-	sdhci_writew(host, SDHCI_BLOCK_SIZE, SDHCI_MAKE_BLKSZ(7, data->blksz));
+	if (host->quirks & SDHCI_QUIRK_FSL)
+		blksz = data->blksz;
+	else
+		blksz = SDHCI_MAKE_BLKSZ(7, data->blksz);
+
+	sdhci_writew(host, SDHCI_BLOCK_SIZE, blksz);
 	sdhci_writew(host, SDHCI_BLOCK_COUNT, data->blocks);
 }
 
@@ -1270,6 +1280,9 @@ static void sdhci_tasklet_finish(unsigned long param)
 		   controllers do not like that. */
 		sdhci_reset(host, SDHCI_RESET_CMD);
 		sdhci_reset(host, SDHCI_RESET_DATA);
+		/* FSL controllers need this. */
+		if (host->quirks & SDHCI_QUIRK_FSL)
+			sdhci_init(host);
 	}
 
 	host->mrq = NULL;
@@ -1779,13 +1792,24 @@ 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) {
+	if (host->quirks & SDHCI_QUIRK_FSL) {
+		mmc->max_blk_size = (caps & SDHCI_FSL_MAX_BLOCK_MASK) >>
+				SDHCI_MAX_BLOCK_SHIFT;
+		if (mmc->max_blk_size >= 4)
+			mmc->max_blk_size = ~0;
+	} else {
+		mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >>
+				SDHCI_MAX_BLOCK_SHIFT;
+		if (mmc->max_blk_size >= 3)
+			mmc->max_blk_size = ~0;
+	}
+
+	if (mmc->max_blk_size == ~0) {
 		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;
+		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 39cfd5a..7b98ec6 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -138,6 +138,7 @@
 #define  SDHCI_CLOCK_BASE_MASK	0x00003F00
 #define  SDHCI_CLOCK_BASE_SHIFT	8
 #define  SDHCI_MAX_BLOCK_MASK	0x00030000
+#define  SDHCI_FSL_MAX_BLOCK_MASK 0x00070000
 #define  SDHCI_MAX_BLOCK_SHIFT  16
 #define  SDHCI_CAN_DO_ADMA2	0x00080000
 #define  SDHCI_CAN_DO_ADMA1	0x00100000
@@ -223,6 +224,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_32BIT_REGISTERS			(1<<18)
 /* Controller issues PIO interrupts during DMA transfers */
 #define SDHCI_QUIRK_PIO_IRQS_DURING_DMA			(1<<19)
+/* Controller is Freescale eSDHC */
+#define SDHCI_QUIRK_FSL					(1<<20)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

[PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

[Anton Vorontsov]

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>
---
 drivers/mmc/host/Kconfig    |   10 ++
 drivers/mmc/host/Makefile   |    1 +
 drivers/mmc/host/sdhci-of.c |  274 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 285 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 dfa585f..8b5cdef 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 Secure Digital Host Controller Interface.
+	  So far there is only Freescale eSDHC controller known to exists
+	  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 f485328..5296793 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_MMC_IMX)		+= imxmmc.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..8ee89a1
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of.c
@@ -0,0 +1,274 @@
+/*
+ * 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;
+};
+
+/*
+ * 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, int reg, u32 val)
+{
+	out_be32(host->ioaddr + reg, val);
+}
+
+static void esdhc_writew(struct sdhci_host *host, int reg, u16 val)
+{
+	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, int reg, u8 val)
+{
+	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 struct sdhci_of_data sdhci_esdhc = {
+	.quirks = SDHCI_QUIRK_FSL |
+		  SDHCI_QUIRK_32BIT_REGISTERS |
+		  SDHCI_QUIRK_PIO_IRQS_DURING_DMA |
+		  SDHCI_QUIRK_BROKEN_CARD_DETECTION |
+		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
+		  SDHCI_QUIRK_NO_BUSY_IRQ |
+		  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,
+	},
+};
+
+#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 resource mem;
+	const u32 *clk;
+	int size;
+	int ret;
+
+	host = sdhci_alloc_host(&ofdev->dev, sizeof(*host));
+	if (!host)
+		return -ENOMEM;
+
+	dev_set_drvdata(&ofdev->dev, host);
+
+	ret = of_address_to_resource(np, 0, &mem);
+	if (ret)
+		goto err_no_addr;
+
+	host->ioaddr = ioremap(mem.start, resource_size(&mem));
+	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) {
+		host->max_clk = *clk;
+		host->timeout_clk = *clk / 1000;
+	}
+
+	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:
+err_no_addr:
+	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,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

Re: [PATCH 02/10] sdhci: Add support for bus-specific IO memory accessors

[Arnd Bergmann]

On Thursday 22 January 2009, Anton Vorontsov wrote:
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/*
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * These accessors duplicate s=
dhci_ops, but there are two reasons for
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * this:
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * 1. sdhci_ops are const, so =
the sdhci driver won't able to assign
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * =C2=A0 =C2=A0default ops;

You could assign the pointer to a const default_sdhci_ops structure,
which IMHO would be cleaner than copying the function pointers
separately.

> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * 2. Using host->X instead of=
 host->ops->X saves us one dereference.
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * =C2=A0 =C2=A0This can be us=
eful in PIO mode. (Though the benefit of this
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * =C2=A0 =C2=A0is negligibly =
small).
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 */

I doubt that this is even measurable. If it was, you could still use a copy
of that structure, like

struct sdhci_host {
	...
	struct sdhci_ops ops; /* not struct sdhci_ops *ops */
	...
};

and do an assignment of the structure, like

static void assign_ops(struct sdhci_host *host, struct sdhci_ops *ops)
{
	host->ops =3D *ops;
}

	Arnd <><

Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

[Arnd Bergmann]

On Thursday 22 January 2009, Anton Vorontsov wrote:
> 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.

Looks very good overall.

Acked-by: Arnd Bergmann <arnd@arndb.de>

> +	ret = of_address_to_resource(np, 0, &mem);
> +	if (ret)
> +		goto err_no_addr;
> +
> +	host->ioaddr = ioremap(mem.start, resource_size(&mem));
> +	if (!host->ioaddr) {
> +		ret = -ENOMEM;
> +		goto err_addr_map;
> +	}

Minor improvement: you could use of_iomap to do this in one step.

	Arnd <><

Re: [PATCH 02/10] sdhci: Add support for bus-specific IO memory accessors

[Anton Vorontsov]

On Thu, Jan 22, 2009 at 12:55:48PM +0100, Arnd Bergmann wrote:
> On Thursday 22 January 2009, Anton Vorontsov wrote:
> > +       /*
> > +        * These accessors duplicate sdhci_ops, but there are two reasons for
> > +        * this:
> > +        * 1. sdhci_ops are const, so the sdhci driver won't able to assign
> > +        *    default ops;
> 
> You could assign the pointer to a const default_sdhci_ops structure,
> which IMHO would be cleaner than copying the function pointers
> separately.

This won't work because ops also specify non-memory accessors ops
(i.e. enable_dma, set_clock, ...), so we can't just assign
default_ops ptr.

> > +        * 2. Using host->X instead of host->ops->X saves us one dereference.
> > +        *    This can be useful in PIO mode. (Though the benefit of this
> > +        *    is negligibly small).
> > +        */
> 
> I doubt that this is even measurable.

Yeah, I didn't even bother w/ measuring. ;-)

> If it was, you could still use a copy of that structure, like
> 
> struct sdhci_host {
> 	...
> 	struct sdhci_ops ops; /* not struct sdhci_ops *ops */
> 	...
> };
> 
> and do an assignment of the structure, like
> 
> static void assign_ops(struct sdhci_host *host, struct sdhci_ops *ops)
> {
> 	host->ops = *ops;
> }

This will work. Pierre, what do you think? That way drivers
will call this routine instead of "host->ops = driver_ops_ptr".

The other option would be to remove const from the ops. Or implement
another non-const ops struct (struct sdhci_io_ops?).

In any way, the type-checked variant will be even longer to type:

host->mem_ops->writel(val, host->ioaddr + reg);
host->ops.writel(val, host->ioaddr + reg);
host->writel(val, host->ioaddr + reg);


Thanks,

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

Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

[Kumar Gala]

On Jan 21, 2009, at 8:00 PM, Anton Vorontsov wrote:

> 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>
> ---
> drivers/mmc/host/Kconfig    |   10 ++
> drivers/mmc/host/Makefile   |    1 +
> drivers/mmc/host/sdhci-of.c |  274 ++++++++++++++++++++++++++++++++++ 
> +++++++++
> 3 files changed, 285 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-of.c

Still griping about lack of OF binding docs for this.

:)

- k

Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

[Matt Sealey]

Anton Vorontsov wrote:
>  
>  	  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 Secure Digital Host Controller Interface.
> +	  So far there is only Freescale eSDHC controller known to exists
> +	  on OF platforms.

Nit; would probably be better English as

	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.

-- Matt Sealey <matt@genesi-usa.com>

Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

[M. Warner Losh]

In message: <497C09D9.50004@genesi-usa.com>
            Matt Sealey <matt@genesi-usa.com> writes:
: Anton Vorontsov wrote:
: >  
: >  	  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 Secure Digital Host Controller Interface.
: > +	  So far there is only Freescale eSDHC controller known to exists
: > +	  on OF platforms.
: 
: Nit; would probably be better English as
: 
: 	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.

Is this the SD Association standard host controller interface, or a
custom one by Freescale?  If the former, then I'd suggest:

	This selects OF support for the SD Association standard Secure
	Digital Host Controller Interface (version x.xx).  So far,
	only the Freescale eSDHC controller is the only example on OF
	platforms.

if the latter:

	This selects OF support for Freescale's custom Secure Digital
	Host Controller Interface.  So far, only the Freescale eSDHC
	controller is the only example on OF platforms.

Warner