linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] atmel-mci: updates for 2.6.28
@ 2008-10-05 16:21 Haavard Skinnemoen
  2008-10-05 16:21 ` [PATCH v2 1/7] atmel-mci: Implement tasklet as a state machine Haavard Skinnemoen
  2008-10-05 18:46 ` [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
  0 siblings, 2 replies; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

Hi Pierre,

The following patches does a few cleanups of the atmel-mci driver,
adds support for multiple MMC slots (AT32AP7000 support two, but this
should be fairly easy to extend in the future), and adds optional DMA
support. If it looks fine to you (and everyone else who wants to look
over it), please queue it up for 2.6.28.

The DMA support appears to work fine with all the cards I have
available. The CPU usage is a bit high if the card is fast, but that's
something I intend to look into later. It's a lot better and faster
than the current driver.

Changes since the last version of this patchset:
  - The clock is no longer stopped between transfers.
  - The clock speed is limited to the speed of the slowest card when
    using multiple slots, but there may still be issues I don't fully
    understand, so I added a warning for the board code. Boards which
    use only one slot should not be affected, however.
  - A few DMA-related kerneldoc comments were moved into the DMA patch.
  - A data transfer error while not using DMA could hang the state
    machine. This has been fixed.
  - A missing call to flush_dcache_page() in the PIO code was added.
    This is a no-op on avr32, so it probably doesn't deserve
    backporting to 2.6.27 or -stable.

Haavard Skinnemoen (7):
      atmel-mci: Implement tasklet as a state machine
      atmel-mci: Don't stop the clock between transfers
      atmel-mci: Platform code for supporting multiple mmc slots
      atmel-mci: support multiple mmc slots
      atmel-mci: Add experimental DMA support
      atmel-mci: Don't overwrite error bits when NOTBUSY is set
      atmel-mci: Add missing flush_dcache_page() in PIO transfer code

 arch/avr32/boards/atngw100/setup.c      |    7 +-
 arch/avr32/boards/atstk1000/atstk1002.c |   18 +-
 arch/avr32/boards/atstk1000/atstk1003.c |   12 +-
 arch/avr32/boards/atstk1000/atstk1004.c |   12 +-
 arch/avr32/include/asm/atmel-mci.h      |   32 +-
 arch/avr32/mach-at32ap/at32ap700x.c     |   90 ++-
 drivers/mmc/host/Kconfig                |   11 +
 drivers/mmc/host/atmel-mci-regs.h       |    6 +-
 drivers/mmc/host/atmel-mci.c            | 1352 ++++++++++++++++++++++---------
 9 files changed, 1127 insertions(+), 413 deletions(-)

Haavard

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

* [PATCH v2 1/7] atmel-mci: Implement tasklet as a state machine
  2008-10-05 16:21 [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
@ 2008-10-05 16:21 ` Haavard Skinnemoen
  2008-10-05 16:21   ` [PATCH v2 2/7] atmel-mci: Don't stop the clock between transfers Haavard Skinnemoen
  2008-10-05 18:46 ` [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
  1 sibling, 1 reply; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

With the current system of completed/pending events, things may get
handled in different order depending on which event triggers first. For
example, if the data transfer is complete before the command, the stop
command must be sent after the command is complete, not the data. This
creates a bit of complexity around the stop command.

By having the tasklet go through a sequence of clearly defined states,
things always happen in a certain order even if the events come at
different times, so the stop command can simply be sent when we exit the
"sending data" state because we will never enter that state before the
command has been sent successfully.

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/mmc/host/atmel-mci.c |  249 ++++++++++++++++++++++++------------------
 1 files changed, 145 insertions(+), 104 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 0000896..d834e37 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -36,11 +36,17 @@
 
 enum {
 	EVENT_CMD_COMPLETE = 0,
-	EVENT_DATA_ERROR,
-	EVENT_DATA_COMPLETE,
-	EVENT_STOP_SENT,
-	EVENT_STOP_COMPLETE,
 	EVENT_XFER_COMPLETE,
+	EVENT_DATA_COMPLETE,
+	EVENT_DATA_ERROR,
+};
+
+enum atmel_mci_state {
+	STATE_SENDING_CMD = 0,
+	STATE_SENDING_DATA,
+	STATE_DATA_BUSY,
+	STATE_SENDING_STOP,
+	STATE_DATA_ERROR,
 };
 
 struct atmel_mci {
@@ -56,7 +62,6 @@ struct atmel_mci {
 
 	u32			cmd_status;
 	u32			data_status;
-	u32			stop_status;
 	u32			stop_cmdr;
 
 	u32			mode_reg;
@@ -65,6 +70,7 @@ struct atmel_mci {
 	struct tasklet_struct	tasklet;
 	unsigned long		pending_events;
 	unsigned long		completed_events;
+	enum atmel_mci_state	state;
 
 	int			present;
 	int			detect_pin;
@@ -79,18 +85,12 @@ struct atmel_mci {
 	struct platform_device	*pdev;
 };
 
-#define atmci_is_completed(host, event)				\
-	test_bit(event, &host->completed_events)
 #define atmci_test_and_clear_pending(host, event)		\
 	test_and_clear_bit(event, &host->pending_events)
-#define atmci_test_and_set_completed(host, event)		\
-	test_and_set_bit(event, &host->completed_events)
 #define atmci_set_completed(host, event)			\
 	set_bit(event, &host->completed_events)
 #define atmci_set_pending(host, event)				\
 	set_bit(event, &host->pending_events)
-#define atmci_clear_pending(host, event)			\
-	clear_bit(event, &host->pending_events)
 
 /*
  * The debugfs stuff below is mostly optimized away when
@@ -258,6 +258,10 @@ static void atmci_init_debugfs(struct atmel_mci *host)
 	if (!node)
 		goto err;
 
+	node = debugfs_create_u32("state", S_IRUSR, root, (u32 *)&host->state);
+	if (!node)
+		goto err;
+
 	node = debugfs_create_x32("pending_events", S_IRUSR, root,
 				     (u32 *)&host->pending_events);
 	if (!node)
@@ -378,8 +382,6 @@ static void atmci_start_command(struct atmel_mci *host,
 				struct mmc_command *cmd,
 				u32 cmd_flags)
 {
-	/* Must read host->cmd after testing event flags */
-	smp_rmb();
 	WARN_ON(host->cmd);
 	host->cmd = cmd;
 
@@ -472,6 +474,7 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	host->mrq = mrq;
 	host->pending_events = 0;
 	host->completed_events = 0;
+	host->state = STATE_SENDING_CMD;
 
 	atmci_enable(host);
 
@@ -596,8 +599,10 @@ static struct mmc_host_ops atmci_ops = {
 };
 
 static void atmci_command_complete(struct atmel_mci *host,
-			struct mmc_command *cmd, u32 status)
+			struct mmc_command *cmd)
 {
+	u32		status = host->cmd_status;
+
 	/* Read the response from the card (up to 16 bytes) */
 	cmd->resp[0] = mci_readl(host, RSPR);
 	cmd->resp[1] = mci_readl(host, RSPR);
@@ -666,20 +671,32 @@ static void atmci_detect_change(unsigned long data)
 			 * commands or data transfers.
 			 */
 			mci_writel(host, CR, MCI_CR_SWRST);
+			mci_readl(host, SR);
 
-			if (!atmci_is_completed(host, EVENT_CMD_COMPLETE))
-				mrq->cmd->error = -ENOMEDIUM;
+			host->data = NULL;
+			host->cmd = NULL;
 
-			if (mrq->data && !atmci_is_completed(host,
-						EVENT_DATA_COMPLETE)) {
-				host->data = NULL;
+			switch (host->state) {
+			case STATE_SENDING_CMD:
+				mrq->cmd->error = -ENOMEDIUM;
+				if (!mrq->data)
+					break;
+				/* fall through */
+			case STATE_SENDING_DATA:
 				mrq->data->error = -ENOMEDIUM;
-			}
-			if (mrq->stop && !atmci_is_completed(host,
-						EVENT_STOP_COMPLETE))
+				break;
+			case STATE_DATA_BUSY:
+			case STATE_DATA_ERROR:
+				if (mrq->data->error == -EINPROGRESS)
+					mrq->data->error = -ENOMEDIUM;
+				if (!mrq->stop)
+					break;
+				/* fall through */
+			case STATE_SENDING_STOP:
 				mrq->stop->error = -ENOMEDIUM;
+				break;
+			}
 
-			host->cmd = NULL;
 			atmci_request_end(host->mmc, mrq);
 		}
 
@@ -693,81 +710,116 @@ static void atmci_tasklet_func(unsigned long priv)
 	struct atmel_mci	*host = mmc_priv(mmc);
 	struct mmc_request	*mrq = host->mrq;
 	struct mmc_data		*data = host->data;
+	struct mmc_command	*cmd = host->cmd;
+	enum atmel_mci_state	state = host->state;
+	enum atmel_mci_state	prev_state;
+	u32			status;
+
+	state = host->state;
 
 	dev_vdbg(&mmc->class_dev,
-		"tasklet: pending/completed/mask %lx/%lx/%x\n",
-		host->pending_events, host->completed_events,
+		"tasklet: state %u pending/completed/mask %lx/%lx/%x\n",
+		state, host->pending_events, host->completed_events,
 		mci_readl(host, IMR));
 
-	if (atmci_test_and_clear_pending(host, EVENT_CMD_COMPLETE)) {
-		/*
-		 * host->cmd must be set to NULL before the interrupt
-		 * handler sees EVENT_CMD_COMPLETE
-		 */
-		host->cmd = NULL;
-		smp_wmb();
-		atmci_set_completed(host, EVENT_CMD_COMPLETE);
-		atmci_command_complete(host, mrq->cmd, host->cmd_status);
-
-		if (!mrq->cmd->error && mrq->stop
-				&& atmci_is_completed(host, EVENT_XFER_COMPLETE)
-				&& !atmci_test_and_set_completed(host,
-					EVENT_STOP_SENT))
-			send_stop_cmd(host->mmc, mrq->data);
-	}
-	if (atmci_test_and_clear_pending(host, EVENT_STOP_COMPLETE)) {
-		/*
-		 * host->cmd must be set to NULL before the interrupt
-		 * handler sees EVENT_STOP_COMPLETE
-		 */
-		host->cmd = NULL;
-		smp_wmb();
-		atmci_set_completed(host, EVENT_STOP_COMPLETE);
-		atmci_command_complete(host, mrq->stop, host->stop_status);
-	}
-	if (atmci_test_and_clear_pending(host, EVENT_DATA_ERROR)) {
-		u32 status = host->data_status;
+	do {
+		prev_state = state;
 
-		dev_vdbg(&mmc->class_dev, "data error: status=%08x\n", status);
+		switch (state) {
+		case STATE_SENDING_CMD:
+			if (!atmci_test_and_clear_pending(host,
+						EVENT_CMD_COMPLETE))
+				break;
 
-		atmci_set_completed(host, EVENT_DATA_ERROR);
-		atmci_set_completed(host, EVENT_DATA_COMPLETE);
+			host->cmd = NULL;
+			atmci_set_completed(host, EVENT_CMD_COMPLETE);
+			atmci_command_complete(host, mrq->cmd);
+			if (!mrq->data || cmd->error) {
+				atmci_request_end(mmc, host->mrq);
+				break;
+			}
 
-		if (status & MCI_DTOE) {
-			dev_dbg(&mmc->class_dev,
-					"data timeout error\n");
-			data->error = -ETIMEDOUT;
-		} else if (status & MCI_DCRCE) {
-			dev_dbg(&mmc->class_dev, "data CRC error\n");
-			data->error = -EILSEQ;
-		} else {
-			dev_dbg(&mmc->class_dev,
-					"data FIFO error (status=%08x)\n",
-					status);
-			data->error = -EIO;
-		}
+			prev_state = state = STATE_SENDING_DATA;
+			/* fall through */
 
-		if (host->present && data->stop
-				&& atmci_is_completed(host, EVENT_CMD_COMPLETE)
-				&& !atmci_test_and_set_completed(
-					host, EVENT_STOP_SENT))
-			send_stop_cmd(host->mmc, data);
+		case STATE_SENDING_DATA:
+			if (atmci_test_and_clear_pending(host,
+						EVENT_DATA_ERROR)) {
+				if (data->stop)
+					send_stop_cmd(host->mmc, data);
+				state = STATE_DATA_ERROR;
+				break;
+			}
 
-		host->data = NULL;
-	}
-	if (atmci_test_and_clear_pending(host, EVENT_DATA_COMPLETE)) {
-		atmci_set_completed(host, EVENT_DATA_COMPLETE);
+			if (!atmci_test_and_clear_pending(host,
+						EVENT_XFER_COMPLETE))
+				break;
 
-		if (!atmci_is_completed(host, EVENT_DATA_ERROR)) {
-			data->bytes_xfered = data->blocks * data->blksz;
-			data->error = 0;
-		}
+			atmci_set_completed(host, EVENT_XFER_COMPLETE);
+			prev_state = state = STATE_DATA_BUSY;
+			/* fall through */
 
-		host->data = NULL;
-	}
+		case STATE_DATA_BUSY:
+			if (!atmci_test_and_clear_pending(host,
+						EVENT_DATA_COMPLETE))
+				break;
+
+			host->data = NULL;
+			atmci_set_completed(host, EVENT_DATA_COMPLETE);
+			status = host->data_status;
+			if (unlikely(status & ATMCI_DATA_ERROR_FLAGS)) {
+				if (status & MCI_DTOE) {
+					dev_dbg(&mmc->class_dev,
+							"data timeout error\n");
+					data->error = -ETIMEDOUT;
+				} else if (status & MCI_DCRCE) {
+					dev_dbg(&mmc->class_dev,
+							"data CRC error\n");
+					data->error = -EILSEQ;
+				} else {
+					dev_dbg(&mmc->class_dev,
+						"data FIFO error (status=%08x)\n",
+						status);
+					data->error = -EIO;
+				}
+			} else {
+				data->bytes_xfered = data->blocks * data->blksz;
+				data->error = 0;
+			}
+
+			if (!data->stop) {
+				atmci_request_end(mmc, host->mrq);
+				prev_state = state;
+				break;
+			}
 
-	if (host->mrq && !host->cmd && !host->data)
-		atmci_request_end(mmc, host->mrq);
+			prev_state = state = STATE_SENDING_STOP;
+			if (!data->error)
+				send_stop_cmd(host->mmc, data);
+			/* fall through */
+
+		case STATE_SENDING_STOP:
+			if (!atmci_test_and_clear_pending(host,
+						EVENT_CMD_COMPLETE))
+				break;
+
+			host->cmd = NULL;
+			atmci_command_complete(host, mrq->stop);
+			atmci_request_end(mmc, host->mrq);
+			prev_state = state;
+			break;
+
+		case STATE_DATA_ERROR:
+			if (!atmci_test_and_clear_pending(host,
+						EVENT_XFER_COMPLETE))
+				break;
+
+			state = STATE_DATA_BUSY;
+			break;
+		}
+	} while (state != prev_state);
+
+	host->state = state;
 }
 
 static void atmci_read_data_pio(struct atmel_mci *host)
@@ -832,10 +884,7 @@ done:
 	mci_writel(host, IDR, MCI_RXRDY);
 	mci_writel(host, IER, MCI_NOTBUSY);
 	data->bytes_xfered += nbytes;
-	atmci_set_completed(host, EVENT_XFER_COMPLETE);
-	if (data->stop && atmci_is_completed(host, EVENT_CMD_COMPLETE)
-			&& !atmci_test_and_set_completed(host, EVENT_STOP_SENT))
-		send_stop_cmd(host->mmc, data);
+	atmci_set_pending(host, EVENT_XFER_COMPLETE);
 }
 
 static void atmci_write_data_pio(struct atmel_mci *host)
@@ -903,10 +952,7 @@ done:
 	mci_writel(host, IDR, MCI_TXRDY);
 	mci_writel(host, IER, MCI_NOTBUSY);
 	data->bytes_xfered += nbytes;
-	atmci_set_completed(host, EVENT_XFER_COMPLETE);
-	if (data->stop && atmci_is_completed(host, EVENT_CMD_COMPLETE)
-			&& !atmci_test_and_set_completed(host, EVENT_STOP_SENT))
-		send_stop_cmd(host->mmc, data);
+	atmci_set_pending(host, EVENT_XFER_COMPLETE);
 }
 
 static void atmci_cmd_interrupt(struct mmc_host *mmc, u32 status)
@@ -915,14 +961,8 @@ static void atmci_cmd_interrupt(struct mmc_host *mmc, u32 status)
 
 	mci_writel(host, IDR, MCI_CMDRDY);
 
-	if (atmci_is_completed(host, EVENT_STOP_SENT)) {
-		host->stop_status = status;
-		atmci_set_pending(host, EVENT_STOP_COMPLETE);
-	} else {
-		host->cmd_status = status;
-		atmci_set_pending(host, EVENT_CMD_COMPLETE);
-	}
-
+	host->cmd_status = status;
+	atmci_set_pending(host, EVENT_CMD_COMPLETE);
 	tasklet_schedule(&host->tasklet);
 }
 
@@ -951,8 +991,9 @@ static irqreturn_t atmci_interrupt(int irq, void *dev_id)
 			tasklet_schedule(&host->tasklet);
 		}
 		if (pending & MCI_NOTBUSY) {
-			mci_writel(host, IDR, (MCI_NOTBUSY
-					       | ATMCI_DATA_ERROR_FLAGS));
+			mci_writel(host, IDR,
+					ATMCI_DATA_ERROR_FLAGS | MCI_NOTBUSY);
+			host->data_status = status;
 			atmci_set_pending(host, EVENT_DATA_COMPLETE);
 			tasklet_schedule(&host->tasklet);
 		}
-- 
1.5.6.5


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

* [PATCH v2 2/7] atmel-mci: Don't stop the clock between transfers
  2008-10-05 16:21 ` [PATCH v2 1/7] atmel-mci: Implement tasklet as a state machine Haavard Skinnemoen
@ 2008-10-05 16:21   ` Haavard Skinnemoen
  2008-10-05 16:21     ` [PATCH v2 3/7] atmel-mci: Platform code for supporting multiple mmc slots Haavard Skinnemoen
  0 siblings, 1 reply; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

Some cards might get upset if we turn off the clock for extended periods
of time. So keep the clock running until the mmc core tells us to turn
it off.

Also, don't reset the controller between each transfer. That was an
attempt to work around earlier bugs, and it never really worked very
well.

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/mmc/host/atmel-mci.c |   60 +++++++++++++++--------------------------
 1 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index d834e37..14ab28d 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -279,23 +279,6 @@ err:
 		"failed to initialize debugfs for controller\n");
 }
 
-static void atmci_enable(struct atmel_mci *host)
-{
-	clk_enable(host->mck);
-	mci_writel(host, CR, MCI_CR_MCIEN);
-	mci_writel(host, MR, host->mode_reg);
-	mci_writel(host, SDCR, host->sdc_reg);
-}
-
-static void atmci_disable(struct atmel_mci *host)
-{
-	mci_writel(host, CR, MCI_CR_SWRST);
-
-	/* Stall until write is complete, then disable the bus clock */
-	mci_readl(host, SR);
-	clk_disable(host->mck);
-}
-
 static inline unsigned int ns_to_clocks(struct atmel_mci *host,
 					unsigned int ns)
 {
@@ -408,8 +391,6 @@ static void atmci_request_end(struct mmc_host *mmc, struct mmc_request *mrq)
 	WARN_ON(host->cmd || host->data);
 	host->mrq = NULL;
 
-	atmci_disable(host);
-
 	mmc_request_done(mmc, mrq);
 }
 
@@ -476,8 +457,6 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	host->completed_events = 0;
 	host->state = STATE_SENDING_CMD;
 
-	atmci_enable(host);
-
 	/* We don't support multiple blocks of weird lengths. */
 	data = mrq->data;
 	if (data) {
@@ -520,7 +499,6 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	return;
 
 fail:
-	atmci_disable(host);
 	host->mrq = NULL;
 	mrq->cmd->error = -EINVAL;
 	mmc_request_done(mmc, mrq);
@@ -530,9 +508,21 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct atmel_mci	*host = mmc_priv(mmc);
 
+	switch (ios->bus_width) {
+	case MMC_BUS_WIDTH_1:
+		host->sdc_reg = 0;
+		break;
+	case MMC_BUS_WIDTH_4:
+		host->sdc_reg = MCI_SDCBUS_4BIT;
+		break;
+	}
+
 	if (ios->clock) {
 		u32 clkdiv;
 
+		if (!host->mode_reg)
+			clk_enable(host->mck);
+
 		/* Set clock rate */
 		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * ios->clock) - 1;
 		if (clkdiv > 255) {
@@ -544,26 +534,20 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 		host->mode_reg = MCI_MR_CLKDIV(clkdiv) | MCI_MR_WRPROOF
 					| MCI_MR_RDPROOF;
-	}
 
-	switch (ios->bus_width) {
-	case MMC_BUS_WIDTH_1:
-		host->sdc_reg = 0;
-		break;
-	case MMC_BUS_WIDTH_4:
-		host->sdc_reg = MCI_SDCBUS_4BIT;
-		break;
+		mci_writel(host, CR, MCI_CR_MCIEN);
+		mci_writel(host, MR, host->mode_reg);
+		mci_writel(host, SDCR, host->sdc_reg);
+	} else {
+		mci_writel(host, CR, MCI_CR_MCIDIS);
+		if (host->mode_reg) {
+			mci_readl(host, MR);
+			clk_disable(host->mck);
+		}
+		host->mode_reg = 0;
 	}
 
 	switch (ios->power_mode) {
-	case MMC_POWER_ON:
-		/* Send init sequence (74 clock cycles) */
-		atmci_enable(host);
-		mci_writel(host, CMDR, MCI_CMDR_SPCMD_INIT);
-		while (!(mci_readl(host, SR) & MCI_CMDRDY))
-			cpu_relax();
-		atmci_disable(host);
-		break;
 	default:
 		/*
 		 * TODO: None of the currently available AVR32-based
-- 
1.5.6.5


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

* [PATCH v2 3/7] atmel-mci: Platform code for supporting multiple mmc slots
  2008-10-05 16:21   ` [PATCH v2 2/7] atmel-mci: Don't stop the clock between transfers Haavard Skinnemoen
@ 2008-10-05 16:21     ` Haavard Skinnemoen
  2008-10-05 16:21       ` [PATCH v2 4/7] atmel-mci: support " Haavard Skinnemoen
  0 siblings, 1 reply; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

Add the necessary platform infrastructure to support multiple mmc/sdcard
slots all at once through a single controller. Currently, the driver
will use the first valid slot it finds and stick with that, but later
patches will add support for switching between several slots on the fly.

Extend the platform data structure with per-slot information: MMC/SDcard
bus width and card detect/write protect pins. This will affect the pin
muxing as well as the capabilities announced to the mmc core.

Note that board code is now required to supply a mci_platform_data
struct to at32_add_device_mci().

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 arch/avr32/boards/atngw100/setup.c      |    7 ++-
 arch/avr32/boards/atstk1000/atstk1002.c |   18 +++++---
 arch/avr32/boards/atstk1000/atstk1003.c |   12 +++++-
 arch/avr32/boards/atstk1000/atstk1004.c |   12 +++++-
 arch/avr32/include/asm/atmel-mci.h      |   22 +++++++++-
 arch/avr32/mach-at32ap/at32ap700x.c     |   74 +++++++++++++++++++++++--------
 drivers/mmc/host/atmel-mci-regs.h       |    6 ++-
 drivers/mmc/host/atmel-mci.c            |   24 ++++++++--
 8 files changed, 138 insertions(+), 37 deletions(-)

diff --git a/arch/avr32/boards/atngw100/setup.c b/arch/avr32/boards/atngw100/setup.c
index b8286f1..f308520 100644
--- a/arch/avr32/boards/atngw100/setup.c
+++ b/arch/avr32/boards/atngw100/setup.c
@@ -53,8 +53,11 @@ static struct spi_board_info spi0_board_info[] __initdata = {
 };
 
 static struct mci_platform_data __initdata mci0_data = {
-	.detect_pin	= GPIO_PIN_PC(25),
-	.wp_pin		= GPIO_PIN_PE(0),
+	.slot[0] = {
+		.bus_width	= 4,
+		.detect_pin	= GPIO_PIN_PC(25),
+		.wp_pin		= GPIO_PIN_PE(0),
+	},
 };
 
 /*
diff --git a/arch/avr32/boards/atstk1000/atstk1002.c b/arch/avr32/boards/atstk1000/atstk1002.c
index dfc3443..4fedbc4 100644
--- a/arch/avr32/boards/atstk1000/atstk1002.c
+++ b/arch/avr32/boards/atstk1000/atstk1002.c
@@ -264,16 +264,20 @@ void __init setup_board(void)
 
 #ifndef CONFIG_BOARD_ATSTK100X_SW2_CUSTOM
 
+static struct mci_platform_data __initdata mci0_data = {
+	.slot[0] = {
+		.bus_width	= 4,
+
 /* MMC card detect requires MACB0 *NOT* be used */
 #ifdef CONFIG_BOARD_ATSTK1002_SW6_CUSTOM
-static struct mci_platform_data __initdata mci0_data = {
-	.detect_pin	= GPIO_PIN_PC(14),	/* gpio30/sdcd */
-	.wp_pin		= GPIO_PIN_PC(15),	/* gpio31/sdwp */
-};
-#define MCI_PDATA	&mci0_data
+		.detect_pin	= GPIO_PIN_PC(14), /* gpio30/sdcd */
+		.wp_pin		= GPIO_PIN_PC(15), /* gpio31/sdwp */
 #else
-#define MCI_PDATA	NULL
+		.detect_pin	= -ENODEV,
+		.wp_pin		= -ENODEV,
 #endif	/* SW6 for sd{cd,wp} routing */
+	},
+};
 
 #endif	/* SW2 for MMC signal routing */
 
@@ -326,7 +330,7 @@ static int __init atstk1002_init(void)
 	at32_add_device_spi(1, spi1_board_info, ARRAY_SIZE(spi1_board_info));
 #endif
 #ifndef CONFIG_BOARD_ATSTK100X_SW2_CUSTOM
-	at32_add_device_mci(0, MCI_PDATA);
+	at32_add_device_mci(0, &mci0_pdata);
 #endif
 #ifdef CONFIG_BOARD_ATSTK1002_SW5_CUSTOM
 	set_hw_addr(at32_add_device_eth(1, &eth_data[1]));
diff --git a/arch/avr32/boards/atstk1000/atstk1003.c b/arch/avr32/boards/atstk1000/atstk1003.c
index 0cf6641..acc6123 100644
--- a/arch/avr32/boards/atstk1000/atstk1003.c
+++ b/arch/avr32/boards/atstk1000/atstk1003.c
@@ -66,6 +66,16 @@ static struct spi_board_info spi1_board_info[] __initdata = { {
 } };
 #endif
 
+#ifndef CONFIG_BOARD_ATSTK100X_SW2_CUSTOM
+static struct mci_platform_data __initdata mci0_data = {
+	.slot[0] = {
+		.bus_width	= 4,
+		.detect_pin	= -ENODEV,
+		.wp_pin		= -ENODEV,
+	},
+};
+#endif
+
 #ifdef CONFIG_BOARD_ATSTK1000_EXTDAC
 static void __init atstk1003_setup_extdac(void)
 {
@@ -154,7 +164,7 @@ static int __init atstk1003_init(void)
 	at32_add_device_spi(1, spi1_board_info, ARRAY_SIZE(spi1_board_info));
 #endif
 #ifndef CONFIG_BOARD_ATSTK100X_SW2_CUSTOM
-	at32_add_device_mci(0, NULL);
+	at32_add_device_mci(0, &mci0_data);
 #endif
 	at32_add_device_usba(0, NULL);
 #ifndef CONFIG_BOARD_ATSTK100X_SW3_CUSTOM
diff --git a/arch/avr32/boards/atstk1000/atstk1004.c b/arch/avr32/boards/atstk1000/atstk1004.c
index 50a5273..d6a2d02 100644
--- a/arch/avr32/boards/atstk1000/atstk1004.c
+++ b/arch/avr32/boards/atstk1000/atstk1004.c
@@ -71,6 +71,16 @@ static struct spi_board_info spi1_board_info[] __initdata = { {
 } };
 #endif
 
+#ifndef CONFIG_BOARD_ATSTK100X_SW2_CUSTOM
+static struct mci_platform_data __initdata mci0_data = {
+	.slot[0] = {
+		.bus_width	= 4,
+		.detect_pin	= -ENODEV,
+		.wp_pin		= -ENODEV,
+	},
+};
+#endif
+
 #ifdef CONFIG_BOARD_ATSTK1000_EXTDAC
 static void __init atstk1004_setup_extdac(void)
 {
@@ -137,7 +147,7 @@ static int __init atstk1004_init(void)
 	at32_add_device_spi(1, spi1_board_info, ARRAY_SIZE(spi1_board_info));
 #endif
 #ifndef CONFIG_BOARD_ATSTK100X_SW2_CUSTOM
-	at32_add_device_mci(0, NULL);
+	at32_add_device_mci(0, &mci0_data);
 #endif
 	at32_add_device_lcdc(0, &atstk1000_lcdc_data,
 			     fbmem_start, fbmem_size, 0);
diff --git a/arch/avr32/include/asm/atmel-mci.h b/arch/avr32/include/asm/atmel-mci.h
index c2ea6e1..d38c64c 100644
--- a/arch/avr32/include/asm/atmel-mci.h
+++ b/arch/avr32/include/asm/atmel-mci.h
@@ -1,9 +1,29 @@
 #ifndef __ASM_AVR32_ATMEL_MCI_H
 #define __ASM_AVR32_ATMEL_MCI_H
 
-struct mci_platform_data {
+/**
+ * struct mci_slot_pdata - board-specific per-slot configuration
+ * @bus_width: Number of data lines wired up the slot
+ * @detect_pin: GPIO pin wired to the card detect switch
+ * @wp_pin: GPIO pin wired to the write protect sensor
+ *
+ * If a given slot is not present on the board, @bus_width should be
+ * set to 0. The other fields are ignored in this case.
+ *
+ * Any pins that aren't available should be set to a negative value.
+ */
+struct mci_slot_pdata {
+	unsigned int		bus_width;
 	int			detect_pin;
 	int			wp_pin;
 };
 
+/**
+ * struct mci_platform_data - board-specific MMC/SDcard configuration
+ * @slot: Per-slot configuration data.
+ */
+struct mci_platform_data {
+	struct mci_slot_pdata	slot[2];
+};
+
 #endif /* __ASM_AVR32_ATMEL_MCI_H */
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index e01dbe4..9967d5a 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1272,10 +1272,13 @@ static struct clk atmel_mci0_pclk = {
 struct platform_device *__init
 at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
 {
-	struct mci_platform_data	_data;
 	struct platform_device		*pdev;
 
-	if (id != 0)
+	if (id != 0 || !data)
+		return NULL;
+
+	/* Must have at least one usable slot */
+	if (!data->slot[0].bus_width && !data->slot[1].bus_width)
 		return NULL;
 
 	pdev = platform_device_alloc("atmel_mci", id);
@@ -1286,28 +1289,61 @@ at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
 				ARRAY_SIZE(atmel_mci0_resource)))
 		goto fail;
 
-	if (!data) {
-		data = &_data;
-		memset(data, -1, sizeof(struct mci_platform_data));
-		data->detect_pin = GPIO_PIN_NONE;
-		data->wp_pin = GPIO_PIN_NONE;
-	}
 
 	if (platform_device_add_data(pdev, data,
 				sizeof(struct mci_platform_data)))
 		goto fail;
 
-	select_peripheral(PA(10), PERIPH_A, 0);	/* CLK	 */
-	select_peripheral(PA(11), PERIPH_A, 0);	/* CMD	 */
-	select_peripheral(PA(12), PERIPH_A, 0);	/* DATA0 */
-	select_peripheral(PA(13), PERIPH_A, 0);	/* DATA1 */
-	select_peripheral(PA(14), PERIPH_A, 0);	/* DATA2 */
-	select_peripheral(PA(15), PERIPH_A, 0);	/* DATA3 */
+	/* CLK line is common to both slots */
+	select_peripheral(PA(10), PERIPH_A, 0);
 
-	if (gpio_is_valid(data->detect_pin))
-		at32_select_gpio(data->detect_pin, 0);
-	if (gpio_is_valid(data->wp_pin))
-		at32_select_gpio(data->wp_pin, 0);
+	switch (data->slot[0].bus_width) {
+	case 4:
+		select_peripheral(PA(13), PERIPH_A, 0);	/* DATA1 */
+		select_peripheral(PA(14), PERIPH_A, 0);	/* DATA2 */
+		select_peripheral(PA(15), PERIPH_A, 0);	/* DATA3 */
+		/* fall through */
+	case 1:
+		select_peripheral(PA(11), PERIPH_A, 0);	/* CMD	 */
+		select_peripheral(PA(12), PERIPH_A, 0);	/* DATA0 */
+
+		if (gpio_is_valid(data->slot[0].detect_pin))
+			at32_select_gpio(data->slot[0].detect_pin, 0);
+		if (gpio_is_valid(data->slot[0].wp_pin))
+			at32_select_gpio(data->slot[0].wp_pin, 0);
+		break;
+	case 0:
+		/* Slot is unused */
+		break;
+	default:
+		goto fail;
+	}
+
+	switch (data->slot[1].bus_width) {
+	case 4:
+		select_peripheral(PB(8),  PERIPH_B, 0);	/* DATA1 */
+		select_peripheral(PB(9),  PERIPH_B, 0);	/* DATA2 */
+		select_peripheral(PB(10), PERIPH_B, 0);	/* DATA3 */
+		/* fall through */
+	case 1:
+		select_peripheral(PB(6),  PERIPH_B, 0); /* CMD	 */
+		select_peripheral(PB(7),  PERIPH_B, 0); /* DATA0 */
+
+		if (gpio_is_valid(data->slot[1].detect_pin))
+			at32_select_gpio(data->slot[1].detect_pin, 0);
+		if (gpio_is_valid(data->slot[1].wp_pin))
+			at32_select_gpio(data->slot[1].wp_pin, 0);
+		break;
+	case 0:
+		/* Slot is unused */
+		break;
+	default:
+		if (!data->slot[0].bus_width)
+			goto fail;
+
+		data->slot[1].bus_width = 0;
+		break;
+	}
 
 	atmel_mci0_pclk.dev = &pdev->dev;
 
diff --git a/drivers/mmc/host/atmel-mci-regs.h b/drivers/mmc/host/atmel-mci-regs.h
index 26bd80e..b58364e 100644
--- a/drivers/mmc/host/atmel-mci-regs.h
+++ b/drivers/mmc/host/atmel-mci-regs.h
@@ -25,8 +25,10 @@
 #define MCI_SDCR		0x000c	/* SD Card / SDIO */
 # define MCI_SDCSEL_SLOT_A	(  0 <<  0)	/* Select SD slot A */
 # define MCI_SDCSEL_SLOT_B	(  1 <<  0)	/* Select SD slot A */
-# define MCI_SDCBUS_1BIT	(  0 <<  7)	/* 1-bit data bus */
-# define MCI_SDCBUS_4BIT	(  1 <<  7)	/* 4-bit data bus */
+# define MCI_SDCSEL_MASK	(  3 <<  0)
+# define MCI_SDCBUS_1BIT	(  0 <<  6)	/* 1-bit data bus */
+# define MCI_SDCBUS_4BIT	(  2 <<  6)	/* 4-bit data bus */
+# define MCI_SDCBUS_MASK	(  3 <<  6)
 #define MCI_ARGR		0x0010	/* Command Argument */
 #define MCI_CMDR		0x0014	/* Command */
 # define MCI_CMDR_CMDNB(x)	((x) <<  0)	/* Command Opcode */
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 14ab28d..8170905 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -508,9 +508,10 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct atmel_mci	*host = mmc_priv(mmc);
 
+	host->sdc_reg &= ~MCI_SDCBUS_MASK;
 	switch (ios->bus_width) {
 	case MMC_BUS_WIDTH_1:
-		host->sdc_reg = 0;
+		host->sdc_reg |= MCI_SDCBUS_1BIT;
 		break;
 	case MMC_BUS_WIDTH_4:
 		host->sdc_reg = MCI_SDCBUS_4BIT;
@@ -1014,9 +1015,11 @@ static irqreturn_t atmci_detect_interrupt(int irq, void *dev_id)
 static int __init atmci_probe(struct platform_device *pdev)
 {
 	struct mci_platform_data	*pdata;
+	struct mci_slot_pdata		*slot;
 	struct atmel_mci *host;
 	struct mmc_host *mmc;
 	struct resource *regs;
+	u32 sdc_reg;
 	int irq;
 	int ret;
 
@@ -1030,6 +1033,17 @@ static int __init atmci_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	/* TODO: Allow using several slots at once */
+	if (pdata->slot[0].bus_width) {
+		sdc_reg = MCI_SDCSEL_SLOT_A;
+		slot = &pdata->slot[0];
+	} else if (pdata->slot[1].bus_width) {
+		sdc_reg = MCI_SDCSEL_SLOT_B;
+		slot = &pdata->slot[1];
+	} else {
+		return -EINVAL;
+	}
+
 	mmc = mmc_alloc_host(sizeof(struct atmel_mci), &pdev->dev);
 	if (!mmc)
 		return -ENOMEM;
@@ -1037,8 +1051,9 @@ static int __init atmci_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->pdev = pdev;
 	host->mmc = mmc;
-	host->detect_pin = pdata->detect_pin;
-	host->wp_pin = pdata->wp_pin;
+	host->detect_pin = slot->detect_pin;
+	host->wp_pin = slot->wp_pin;
+	host->sdc_reg = sdc_reg;
 
 	host->mck = clk_get(&pdev->dev, "mci_clk");
 	if (IS_ERR(host->mck)) {
@@ -1062,7 +1077,8 @@ static int __init atmci_probe(struct platform_device *pdev)
 	mmc->f_min = (host->bus_hz + 511) / 512;
 	mmc->f_max = host->bus_hz / 2;
 	mmc->ocr_avail	= MMC_VDD_32_33 | MMC_VDD_33_34;
-	mmc->caps |= MMC_CAP_4_BIT_DATA;
+	if (slot->bus_width >= 4)
+		mmc->caps |= MMC_CAP_4_BIT_DATA;
 
 	mmc->max_hw_segs = 64;
 	mmc->max_phys_segs = 64;
-- 
1.5.6.5


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

* [PATCH v2 4/7] atmel-mci: support multiple mmc slots
  2008-10-05 16:21     ` [PATCH v2 3/7] atmel-mci: Platform code for supporting multiple mmc slots Haavard Skinnemoen
@ 2008-10-05 16:21       ` Haavard Skinnemoen
  2008-10-05 16:21         ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Haavard Skinnemoen
  2008-10-12  8:39         ` [PATCH v2 4/7] atmel-mci: support multiple mmc slots Pierre Ossman
  0 siblings, 2 replies; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

The Atmel MCI controller can drive multiple cards through separate sets
of pins, but only one at a time. This patch adds support for
multiplexing access to the controller so that multiple card slots can be
used as if they were hooked up to separate mmc controllers.

The atmel-mci driver registers each slot as a separate mmc_host. Both
access the same common controller state, but they also have some state
on their own for card detection/write protect handling, and separate
shadows of the MR and SDCR registers.

When one of the slots receives a request from the mmc core, the common
controller state is checked. If it's idle, the request is submitted
immediately. If not, the request is added to a queue. When a request is
done, the queue is checked and if there is a queued request, it is
submitted before the completion callback is called.

This patch also includes a few cleanups and fixes, including a locking
overhaul. I had to change the locking extensively in any case, so I
might as well try to get it right. The driver no longer takes any
irq-safe locks, which may or may not improve the overall system
performance.

This patch also adds a bit of documentation of the internal data
structures.

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 arch/avr32/include/asm/atmel-mci.h |    8 +-
 drivers/mmc/host/atmel-mci.c       |  922 ++++++++++++++++++++++++------------
 2 files changed, 621 insertions(+), 309 deletions(-)

diff --git a/arch/avr32/include/asm/atmel-mci.h b/arch/avr32/include/asm/atmel-mci.h
index d38c64c..5d5ae12 100644
--- a/arch/avr32/include/asm/atmel-mci.h
+++ b/arch/avr32/include/asm/atmel-mci.h
@@ -1,6 +1,8 @@
 #ifndef __ASM_AVR32_ATMEL_MCI_H
 #define __ASM_AVR32_ATMEL_MCI_H
 
+#define ATMEL_MCI_MAX_NR_SLOTS	2
+
 /**
  * struct mci_slot_pdata - board-specific per-slot configuration
  * @bus_width: Number of data lines wired up the slot
@@ -11,6 +13,10 @@
  * set to 0. The other fields are ignored in this case.
  *
  * Any pins that aren't available should be set to a negative value.
+ *
+ * Note that support for multiple slots is experimental -- some cards
+ * might get upset if we don't get the clock management exactly right.
+ * But in most cases, it should work just fine.
  */
 struct mci_slot_pdata {
 	unsigned int		bus_width;
@@ -23,7 +29,7 @@ struct mci_slot_pdata {
  * @slot: Per-slot configuration data.
  */
 struct mci_platform_data {
-	struct mci_slot_pdata	slot[2];
+	struct mci_slot_pdata	slot[ATMEL_MCI_MAX_NR_SLOTS];
 };
 
 #endif /* __ASM_AVR32_ATMEL_MCI_H */
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 8170905..d8ab351 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -42,20 +42,86 @@ enum {
 };
 
 enum atmel_mci_state {
-	STATE_SENDING_CMD = 0,
+	STATE_IDLE = 0,
+	STATE_SENDING_CMD,
 	STATE_SENDING_DATA,
 	STATE_DATA_BUSY,
 	STATE_SENDING_STOP,
 	STATE_DATA_ERROR,
 };
 
+/**
+ * struct atmel_mci - MMC controller state shared between all slots
+ * @lock: Spinlock protecting the queue and associated data.
+ * @regs: Pointer to MMIO registers.
+ * @sg: Scatterlist entry currently being processed by PIO code, if any.
+ * @pio_offset: Offset into the current scatterlist entry.
+ * @cur_slot: The slot which is currently using the controller.
+ * @mrq: The request currently being processed on @cur_slot,
+ *	or NULL if the controller is idle.
+ * @cmd: The command currently being sent to the card, or NULL.
+ * @data: The data currently being transferred, or NULL if no data
+ *	transfer is in progress.
+ * @cmd_status: Snapshot of SR taken upon completion of the current
+ *	command. Only valid when EVENT_CMD_COMPLETE is pending.
+ * @data_status: Snapshot of SR taken upon completion of the current
+ *	data transfer. Only valid when EVENT_DATA_COMPLETE or
+ *	EVENT_DATA_ERROR is pending.
+ * @stop_cmdr: Value to be loaded into CMDR when the stop command is
+ *	to be sent.
+ * @tasklet: Tasklet running the request state machine.
+ * @pending_events: Bitmask of events flagged by the interrupt handler
+ *	to be processed by the tasklet.
+ * @completed_events: Bitmask of events which the state machine has
+ *	processed.
+ * @state: Tasklet state.
+ * @queue: List of slots waiting for access to the controller.
+ * @need_clock_update: Update the clock rate before the next request.
+ * @need_reset: Reset controller before next request.
+ * @mode_reg: Value of the MR register.
+ * @bus_hz: The rate of @mck in Hz. This forms the basis for MMC bus
+ *	rate and timeout calculations.
+ * @mapbase: Physical address of the MMIO registers.
+ * @mck: The peripheral bus clock hooked up to the MMC controller.
+ * @pdev: Platform device associated with the MMC controller.
+ * @slot: Slots sharing this MMC controller.
+ *
+ * Locking
+ * =======
+ *
+ * @lock is a softirq-safe spinlock protecting @queue as well as
+ * @cur_slot, @mrq and @state. These must always be updated
+ * at the same time while holding @lock.
+ *
+ * @lock also protects mode_reg and need_clock_update since these are
+ * used to synchronize mode register updates with the queue
+ * processing.
+ *
+ * The @mrq field of struct atmel_mci_slot is also protected by @lock,
+ * and must always be written at the same time as the slot is added to
+ * @queue.
+ *
+ * @pending_events and @completed_events are accessed using atomic bit
+ * operations, so they don't need any locking.
+ *
+ * None of the fields touched by the interrupt handler need any
+ * locking. However, ordering is important: Before EVENT_DATA_ERROR or
+ * EVENT_DATA_COMPLETE is set in @pending_events, all data-related
+ * interrupts must be disabled and @data_status updated with a
+ * snapshot of SR. Similarly, before EVENT_CMD_COMPLETE is set, the
+ * CMDRDY interupt must be disabled and @cmd_status updated with a
+ * snapshot of SR, and before EVENT_XFER_COMPLETE can be set, the
+ * bytes_xfered field of @data must be written. This is ensured by
+ * using barriers.
+ */
 struct atmel_mci {
-	struct mmc_host		*mmc;
+	spinlock_t		lock;
 	void __iomem		*regs;
 
 	struct scatterlist	*sg;
 	unsigned int		pio_offset;
 
+	struct atmel_mci_slot	*cur_slot;
 	struct mmc_request	*mrq;
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
@@ -64,25 +130,59 @@ struct atmel_mci {
 	u32			data_status;
 	u32			stop_cmdr;
 
-	u32			mode_reg;
-	u32			sdc_reg;
-
 	struct tasklet_struct	tasklet;
 	unsigned long		pending_events;
 	unsigned long		completed_events;
 	enum atmel_mci_state	state;
+	struct list_head	queue;
 
-	int			present;
-	int			detect_pin;
-	int			wp_pin;
-
-	/* For detect pin debouncing */
-	struct timer_list	detect_timer;
-
+	bool			need_clock_update;
+	bool			need_reset;
+	u32			mode_reg;
 	unsigned long		bus_hz;
 	unsigned long		mapbase;
 	struct clk		*mck;
 	struct platform_device	*pdev;
+
+	struct atmel_mci_slot	*slot[ATMEL_MCI_MAX_NR_SLOTS];
+};
+
+/**
+ * struct atmel_mci_slot - MMC slot state
+ * @mmc: The mmc_host representing this slot.
+ * @host: The MMC controller this slot is using.
+ * @sdc_reg: Value of SDCR to be written before using this slot.
+ * @mrq: mmc_request currently being processed or waiting to be
+ *	processed, or NULL when the slot is idle.
+ * @queue_node: List node for placing this node in the @queue list of
+ *	&struct atmel_mci.
+ * @clock: Clock rate configured by set_ios(). Protected by host->lock.
+ * @flags: Random state bits associated with the slot.
+ * @detect_pin: GPIO pin used for card detection, or negative if not
+ *	available.
+ * @wp_pin: GPIO pin used for card write protect sending, or negative
+ *	if not available.
+ * @detect_timer: Timer used for debouncing @detect_pin interrupts.
+ */
+struct atmel_mci_slot {
+	struct mmc_host		*mmc;
+	struct atmel_mci	*host;
+
+	u32			sdc_reg;
+
+	struct mmc_request	*mrq;
+	struct list_head	queue_node;
+
+	unsigned int		clock;
+	unsigned long		flags;
+#define ATMCI_CARD_PRESENT	0
+#define ATMCI_CARD_NEED_INIT	1
+#define ATMCI_SHUTDOWN		2
+
+	int			detect_pin;
+	int			wp_pin;
+
+	struct timer_list	detect_timer;
 };
 
 #define atmci_test_and_clear_pending(host, event)		\
@@ -98,14 +198,15 @@ struct atmel_mci {
  */
 static int atmci_req_show(struct seq_file *s, void *v)
 {
-	struct atmel_mci	*host = s->private;
-	struct mmc_request	*mrq = host->mrq;
+	struct atmel_mci_slot	*slot = s->private;
+	struct mmc_request	*mrq;
 	struct mmc_command	*cmd;
 	struct mmc_command	*stop;
 	struct mmc_data		*data;
 
 	/* Make sure we get a consistent snapshot */
-	spin_lock_irq(&host->mmc->lock);
+	spin_lock_bh(&slot->host->lock);
+	mrq = slot->mrq;
 
 	if (mrq) {
 		cmd = mrq->cmd;
@@ -130,7 +231,7 @@ static int atmci_req_show(struct seq_file *s, void *v)
 				stop->resp[2], stop->error);
 	}
 
-	spin_unlock_irq(&host->mmc->lock);
+	spin_unlock_bh(&slot->host->lock);
 
 	return 0;
 }
@@ -193,12 +294,16 @@ static int atmci_regs_show(struct seq_file *s, void *v)
 	if (!buf)
 		return -ENOMEM;
 
-	/* Grab a more or less consistent snapshot */
-	spin_lock_irq(&host->mmc->lock);
+	/*
+	 * Grab a more or less consistent snapshot. Note that we're
+	 * not disabling interrupts, so IMR and SR may not be
+	 * consistent.
+	 */
+	spin_lock_bh(&host->lock);
 	clk_enable(host->mck);
 	memcpy_fromio(buf, host->regs, MCI_REGS_SIZE);
 	clk_disable(host->mck);
-	spin_unlock_irq(&host->mmc->lock);
+	spin_unlock_bh(&host->lock);
 
 	seq_printf(s, "MR:\t0x%08x%s%s CLKDIV=%u\n",
 			buf[MCI_MR / 4],
@@ -236,13 +341,13 @@ static const struct file_operations atmci_regs_fops = {
 	.release	= single_release,
 };
 
-static void atmci_init_debugfs(struct atmel_mci *host)
+static void atmci_init_debugfs(struct atmel_mci_slot *slot)
 {
-	struct mmc_host	*mmc;
-	struct dentry	*root;
-	struct dentry	*node;
+	struct mmc_host		*mmc = slot->mmc;
+	struct atmel_mci	*host = slot->host;
+	struct dentry		*root;
+	struct dentry		*node;
 
-	mmc = host->mmc;
 	root = mmc->debugfs_root;
 	if (!root)
 		return;
@@ -254,7 +359,7 @@ static void atmci_init_debugfs(struct atmel_mci *host)
 	if (!node)
 		goto err;
 
-	node = debugfs_create_file("req", S_IRUSR, root, host, &atmci_req_fops);
+	node = debugfs_create_file("req", S_IRUSR, root, slot, &atmci_req_fops);
 	if (!node)
 		goto err;
 
@@ -275,8 +380,7 @@ static void atmci_init_debugfs(struct atmel_mci *host)
 	return;
 
 err:
-	dev_err(&host->pdev->dev,
-		"failed to initialize debugfs for controller\n");
+	dev_err(&mmc->class_dev, "failed to initialize debugfs for slot\n");
 }
 
 static inline unsigned int ns_to_clocks(struct atmel_mci *host,
@@ -286,7 +390,7 @@ static inline unsigned int ns_to_clocks(struct atmel_mci *host,
 }
 
 static void atmci_set_timeout(struct atmel_mci *host,
-			      struct mmc_data *data)
+		struct atmel_mci_slot *slot, struct mmc_data *data)
 {
 	static unsigned	dtomul_to_shift[] = {
 		0, 4, 7, 8, 10, 12, 16, 20
@@ -309,7 +413,7 @@ static void atmci_set_timeout(struct atmel_mci *host,
 		dtocyc = 15;
 	}
 
-	dev_vdbg(&host->mmc->class_dev, "setting timeout to %u cycles\n",
+	dev_vdbg(&slot->mmc->class_dev, "setting timeout to %u cycles\n",
 			dtocyc << dtomul_to_shift[dtomul]);
 	mci_writel(host, DTOR, (MCI_DTOMUL(dtomul) | MCI_DTOCYC(dtocyc)));
 }
@@ -362,13 +466,12 @@ static u32 atmci_prepare_command(struct mmc_host *mmc,
 }
 
 static void atmci_start_command(struct atmel_mci *host,
-				struct mmc_command *cmd,
-				u32 cmd_flags)
+		struct mmc_command *cmd, u32 cmd_flags)
 {
 	WARN_ON(host->cmd);
 	host->cmd = cmd;
 
-	dev_vdbg(&host->mmc->class_dev,
+	dev_vdbg(&host->pdev->dev,
 			"start command: ARGR=0x%08x CMDR=0x%08x\n",
 			cmd->arg, cmd_flags);
 
@@ -376,32 +479,19 @@ static void atmci_start_command(struct atmel_mci *host,
 	mci_writel(host, CMDR, cmd_flags);
 }
 
-static void send_stop_cmd(struct mmc_host *mmc, struct mmc_data *data)
+static void send_stop_cmd(struct atmel_mci *host, struct mmc_data *data)
 {
-	struct atmel_mci *host = mmc_priv(mmc);
-
 	atmci_start_command(host, data->stop, host->stop_cmdr);
 	mci_writel(host, IER, MCI_CMDRDY);
 }
 
-static void atmci_request_end(struct mmc_host *mmc, struct mmc_request *mrq)
-{
-	struct atmel_mci *host = mmc_priv(mmc);
-
-	WARN_ON(host->cmd || host->data);
-	host->mrq = NULL;
-
-	mmc_request_done(mmc, mrq);
-}
-
 /*
  * Returns a mask of interrupt flags to be enabled after the whole
  * request has been prepared.
  */
-static u32 atmci_submit_data(struct mmc_host *mmc, struct mmc_data *data)
+static u32 atmci_submit_data(struct atmel_mci *host, struct mmc_data *data)
 {
-	struct atmel_mci	*host = mmc_priv(mmc);
-	u32			iflags;
+	u32 iflags;
 
 	data->error = -EINPROGRESS;
 
@@ -409,10 +499,19 @@ static u32 atmci_submit_data(struct mmc_host *mmc, struct mmc_data *data)
 	host->sg = NULL;
 	host->data = data;
 
-	dev_vdbg(&mmc->class_dev, "BLKR=0x%08x\n",
-			MCI_BCNT(data->blocks) | MCI_BLKLEN(data->blksz));
-
 	iflags = ATMCI_DATA_ERROR_FLAGS;
+
+	/*
+	 * Errata: MMC data write operation with less than 12
+	 * bytes is impossible.
+	 *
+	 * Errata: MCI Transmit Data Register (TDR) FIFO
+	 * corruption when length is not multiple of 4.
+	 */
+	if (data->blocks * data->blksz < 12
+			|| (data->blocks * data->blksz) & 3)
+		host->need_reset = true;
+
 	host->sg = data->sg;
 	host->pio_offset = 0;
 	if (data->flags & MMC_DATA_READ)
@@ -423,62 +522,62 @@ static u32 atmci_submit_data(struct mmc_host *mmc, struct mmc_data *data)
 	return iflags;
 }
 
-static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
+static void atmci_start_request(struct atmel_mci *host,
+		struct atmel_mci_slot *slot)
 {
-	struct atmel_mci	*host = mmc_priv(mmc);
-	struct mmc_data		*data;
+	struct mmc_request	*mrq;
 	struct mmc_command	*cmd;
+	struct mmc_data		*data;
 	u32			iflags;
-	u32			cmdflags = 0;
-
-	iflags = mci_readl(host, IMR);
-	if (iflags)
-		dev_warn(&mmc->class_dev, "WARNING: IMR=0x%08x\n",
-				mci_readl(host, IMR));
-
-	WARN_ON(host->mrq != NULL);
-
-	/*
-	 * We may "know" the card is gone even though there's still an
-	 * electrical connection. If so, we really need to communicate
-	 * this to the MMC core since there won't be any more
-	 * interrupts as the card is completely removed. Otherwise,
-	 * the MMC core might believe the card is still there even
-	 * though the card was just removed very slowly.
-	 */
-	if (!host->present) {
-		mrq->cmd->error = -ENOMEDIUM;
-		mmc_request_done(mmc, mrq);
-		return;
-	}
+	u32			cmdflags;
 
+	mrq = slot->mrq;
+	host->cur_slot = slot;
 	host->mrq = mrq;
+
 	host->pending_events = 0;
 	host->completed_events = 0;
-	host->state = STATE_SENDING_CMD;
 
-	/* We don't support multiple blocks of weird lengths. */
+	if (host->need_reset) {
+		mci_writel(host, CR, MCI_CR_SWRST);
+		mci_writel(host, CR, MCI_CR_MCIEN);
+		mci_writel(host, MR, host->mode_reg);
+		host->need_reset = false;
+	}
+	mci_writel(host, SDCR, slot->sdc_reg);
+
+	iflags = mci_readl(host, IMR);
+	if (iflags)
+		dev_warn(&slot->mmc->class_dev, "WARNING: IMR=0x%08x\n",
+				iflags);
+
+	if (unlikely(test_and_clear_bit(ATMCI_CARD_NEED_INIT, &slot->flags))) {
+		/* Send init sequence (74 clock cycles) */
+		mci_writel(host, CMDR, MCI_CMDR_SPCMD_INIT);
+		while (!(mci_readl(host, SR) & MCI_CMDRDY))
+			cpu_relax();
+	}
 	data = mrq->data;
 	if (data) {
-		if (data->blocks > 1 && data->blksz & 3)
-			goto fail;
-		atmci_set_timeout(host, data);
+		atmci_set_timeout(host, slot, data);
 
 		/* Must set block count/size before sending command */
 		mci_writel(host, BLKR, MCI_BCNT(data->blocks)
 				| MCI_BLKLEN(data->blksz));
+		dev_vdbg(&slot->mmc->class_dev, "BLKR=0x%08x\n",
+			MCI_BCNT(data->blocks) | MCI_BLKLEN(data->blksz));
 	}
 
 	iflags = MCI_CMDRDY;
 	cmd = mrq->cmd;
-	cmdflags = atmci_prepare_command(mmc, cmd);
+	cmdflags = atmci_prepare_command(slot->mmc, cmd);
 	atmci_start_command(host, cmd, cmdflags);
 
 	if (data)
-		iflags |= atmci_submit_data(mmc, data);
+		iflags |= atmci_submit_data(host, data);
 
 	if (mrq->stop) {
-		host->stop_cmdr = atmci_prepare_command(mmc, mrq->stop);
+		host->stop_cmdr = atmci_prepare_command(slot->mmc, mrq->stop);
 		host->stop_cmdr |= MCI_CMDR_STOP_XFER;
 		if (!(data->flags & MMC_DATA_WRITE))
 			host->stop_cmdr |= MCI_CMDR_TRDIR_READ;
@@ -495,65 +594,156 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	 * prepared yet.)
 	 */
 	mci_writel(host, IER, iflags);
+}
 
-	return;
+static void atmci_queue_request(struct atmel_mci *host,
+		struct atmel_mci_slot *slot, struct mmc_request *mrq)
+{
+	dev_vdbg(&slot->mmc->class_dev, "queue request: state=%d\n",
+			host->state);
+
+	spin_lock_bh(&host->lock);
+	slot->mrq = mrq;
+	if (host->state == STATE_IDLE) {
+		host->state = STATE_SENDING_CMD;
+		atmci_start_request(host, slot);
+	} else {
+		list_add_tail(&slot->queue_node, &host->queue);
+	}
+	spin_unlock_bh(&host->lock);
+}
 
-fail:
-	host->mrq = NULL;
-	mrq->cmd->error = -EINVAL;
-	mmc_request_done(mmc, mrq);
+static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct atmel_mci_slot	*slot = mmc_priv(mmc);
+	struct atmel_mci	*host = slot->host;
+	struct mmc_data		*data;
+
+	WARN_ON(slot->mrq);
+
+	/*
+	 * We may "know" the card is gone even though there's still an
+	 * electrical connection. If so, we really need to communicate
+	 * this to the MMC core since there won't be any more
+	 * interrupts as the card is completely removed. Otherwise,
+	 * the MMC core might believe the card is still there even
+	 * though the card was just removed very slowly.
+	 */
+	if (!test_bit(ATMCI_CARD_PRESENT, &slot->flags)) {
+		mrq->cmd->error = -ENOMEDIUM;
+		mmc_request_done(mmc, mrq);
+		return;
+	}
+
+	/* We don't support multiple blocks of weird lengths. */
+	data = mrq->data;
+	if (data && data->blocks > 1 && data->blksz & 3) {
+		mrq->cmd->error = -EINVAL;
+		mmc_request_done(mmc, mrq);
+	}
+
+	atmci_queue_request(host, slot, mrq);
 }
 
 static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
-	struct atmel_mci	*host = mmc_priv(mmc);
+	struct atmel_mci_slot	*slot = mmc_priv(mmc);
+	struct atmel_mci	*host = slot->host;
+	unsigned int		i;
 
-	host->sdc_reg &= ~MCI_SDCBUS_MASK;
+	slot->sdc_reg &= ~MCI_SDCBUS_MASK;
 	switch (ios->bus_width) {
 	case MMC_BUS_WIDTH_1:
-		host->sdc_reg |= MCI_SDCBUS_1BIT;
+		slot->sdc_reg |= MCI_SDCBUS_1BIT;
 		break;
 	case MMC_BUS_WIDTH_4:
-		host->sdc_reg = MCI_SDCBUS_4BIT;
+		slot->sdc_reg = MCI_SDCBUS_4BIT;
 		break;
 	}
 
 	if (ios->clock) {
+		unsigned int clock_min = ~0U;
 		u32 clkdiv;
 
-		if (!host->mode_reg)
+		spin_lock_bh(&host->lock);
+		if (!host->mode_reg) {
 			clk_enable(host->mck);
+			mci_writel(host, CR, MCI_CR_SWRST);
+			mci_writel(host, CR, MCI_CR_MCIEN);
+		}
 
-		/* Set clock rate */
-		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * ios->clock) - 1;
+		/*
+		 * Use mirror of ios->clock to prevent race with mmc
+		 * core ios update when finding the minimum.
+		 */
+		slot->clock = ios->clock;
+		for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) {
+			if (host->slot[i] && host->slot[i]->clock
+					&& host->slot[i]->clock < clock_min)
+				clock_min = host->slot[i]->clock;
+		}
+
+		/* Calculate clock divider */
+		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * clock_min) - 1;
 		if (clkdiv > 255) {
 			dev_warn(&mmc->class_dev,
 				"clock %u too slow; using %lu\n",
-				ios->clock, host->bus_hz / (2 * 256));
+				clock_min, host->bus_hz / (2 * 256));
 			clkdiv = 255;
 		}
 
+		/*
+		 * WRPROOF and RDPROOF prevent overruns/underruns by
+		 * stopping the clock when the FIFO is full/empty.
+		 * This state is not expected to last for long.
+		 */
 		host->mode_reg = MCI_MR_CLKDIV(clkdiv) | MCI_MR_WRPROOF
 					| MCI_MR_RDPROOF;
 
-		mci_writel(host, CR, MCI_CR_MCIEN);
-		mci_writel(host, MR, host->mode_reg);
-		mci_writel(host, SDCR, host->sdc_reg);
+		if (list_empty(&host->queue))
+			mci_writel(host, MR, host->mode_reg);
+		else
+			host->need_clock_update = true;
+
+		spin_unlock_bh(&host->lock);
 	} else {
-		mci_writel(host, CR, MCI_CR_MCIDIS);
-		if (host->mode_reg) {
-			mci_readl(host, MR);
-			clk_disable(host->mck);
+		bool any_slot_active = false;
+
+		spin_lock_bh(&host->lock);
+		slot->clock = 0;
+		for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) {
+			if (host->slot[i] && host->slot[i]->clock) {
+				any_slot_active = true;
+				break;
+			}
 		}
-		host->mode_reg = 0;
+		if (!any_slot_active) {
+			mci_writel(host, CR, MCI_CR_MCIDIS);
+			if (host->mode_reg) {
+				mci_readl(host, MR);
+				clk_disable(host->mck);
+			}
+			host->mode_reg = 0;
+		}
+		spin_unlock_bh(&host->lock);
 	}
 
 	switch (ios->power_mode) {
+	case MMC_POWER_UP:
+		set_bit(ATMCI_CARD_NEED_INIT, &slot->flags);
+		break;
 	default:
 		/*
 		 * TODO: None of the currently available AVR32-based
 		 * boards allow MMC power to be turned off. Implement
 		 * power control when this can be tested properly.
+		 *
+		 * We also need to hook this into the clock management
+		 * somehow so that newly inserted cards aren't
+		 * subjected to a fast clock before we have a chance
+		 * to figure out what the maximum rate is. Currently,
+		 * there's no way to avoid this, and there never will
+		 * be for boards that don't support power control.
 		 */
 		break;
 	}
@@ -561,28 +751,77 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 static int atmci_get_ro(struct mmc_host *mmc)
 {
-	int			read_only = 0;
-	struct atmel_mci	*host = mmc_priv(mmc);
+	int			read_only = -ENOSYS;
+	struct atmel_mci_slot	*slot = mmc_priv(mmc);
 
-	if (gpio_is_valid(host->wp_pin)) {
-		read_only = gpio_get_value(host->wp_pin);
+	if (gpio_is_valid(slot->wp_pin)) {
+		read_only = gpio_get_value(slot->wp_pin);
 		dev_dbg(&mmc->class_dev, "card is %s\n",
 				read_only ? "read-only" : "read-write");
-	} else {
-		dev_dbg(&mmc->class_dev,
-			"no pin for checking read-only switch."
-			" Assuming write-enable.\n");
 	}
 
 	return read_only;
 }
 
-static struct mmc_host_ops atmci_ops = {
+static int atmci_get_cd(struct mmc_host *mmc)
+{
+	int			present = -ENOSYS;
+	struct atmel_mci_slot	*slot = mmc_priv(mmc);
+
+	if (gpio_is_valid(slot->detect_pin)) {
+		present = !gpio_get_value(slot->detect_pin);
+		dev_dbg(&mmc->class_dev, "card is %spresent\n",
+				present ? "" : "not ");
+	}
+
+	return present;
+}
+
+static const struct mmc_host_ops atmci_ops = {
 	.request	= atmci_request,
 	.set_ios	= atmci_set_ios,
 	.get_ro		= atmci_get_ro,
+	.get_cd		= atmci_get_cd,
 };
 
+/* Called with host->lock held */
+static void atmci_request_end(struct atmel_mci *host, struct mmc_request *mrq)
+	__releases(&host->lock)
+	__acquires(&host->lock)
+{
+	struct atmel_mci_slot	*slot = NULL;
+	struct mmc_host		*prev_mmc = host->cur_slot->mmc;
+
+	WARN_ON(host->cmd || host->data);
+
+	/*
+	 * Update the MMC clock rate if necessary. This may be
+	 * necessary if set_ios() is called when a different slot is
+	 * busy transfering data.
+	 */
+	if (host->need_clock_update)
+		mci_writel(host, MR, host->mode_reg);
+
+	host->cur_slot->mrq = NULL;
+	host->mrq = NULL;
+	if (!list_empty(&host->queue)) {
+		slot = list_entry(host->queue.next,
+				struct atmel_mci_slot, queue_node);
+		list_del(&slot->queue_node);
+		dev_vdbg(&host->pdev->dev, "list not empty: %s is next\n",
+				mmc_hostname(slot->mmc));
+		host->state = STATE_SENDING_CMD;
+		atmci_start_request(host, slot);
+	} else {
+		dev_vdbg(&host->pdev->dev, "list empty\n");
+		host->state = STATE_IDLE;
+	}
+
+	spin_unlock(&host->lock);
+	mmc_request_done(prev_mmc, mrq);
+	spin_lock(&host->lock);
+}
+
 static void atmci_command_complete(struct atmel_mci *host,
 			struct mmc_command *cmd)
 {
@@ -604,7 +843,7 @@ static void atmci_command_complete(struct atmel_mci *host,
 		cmd->error = 0;
 
 	if (cmd->error) {
-		dev_dbg(&host->mmc->class_dev,
+		dev_dbg(&host->pdev->dev,
 			"command error: status=0x%08x\n", status);
 
 		if (cmd->data) {
@@ -618,81 +857,102 @@ static void atmci_command_complete(struct atmel_mci *host,
 
 static void atmci_detect_change(unsigned long data)
 {
-	struct atmel_mci *host = (struct atmel_mci *)data;
-	struct mmc_request *mrq = host->mrq;
-	int present;
+	struct atmel_mci_slot	*slot = (struct atmel_mci_slot *)data;
+	bool			present;
+	bool			present_old;
 
 	/*
-	 * atmci_remove() sets detect_pin to -1 before freeing the
-	 * interrupt. We must not re-enable the interrupt if it has
-	 * been freed.
+	 * atmci_cleanup_slot() sets the ATMCI_SHUTDOWN flag before
+	 * freeing the interrupt. We must not re-enable the interrupt
+	 * if it has been freed, and if we're shutting down, it
+	 * doesn't really matter whether the card is present or not.
 	 */
 	smp_rmb();
-	if (!gpio_is_valid(host->detect_pin))
+	if (test_bit(ATMCI_SHUTDOWN, &slot->flags))
 		return;
 
-	enable_irq(gpio_to_irq(host->detect_pin));
-	present = !gpio_get_value(host->detect_pin);
+	enable_irq(gpio_to_irq(slot->detect_pin));
+	present = !gpio_get_value(slot->detect_pin);
+	present_old = test_bit(ATMCI_CARD_PRESENT, &slot->flags);
 
-	dev_vdbg(&host->pdev->dev, "detect change: %d (was %d)\n",
-			present, host->present);
+	dev_vdbg(&slot->mmc->class_dev, "detect change: %d (was %d)\n",
+			present, present_old);
 
-	if (present != host->present) {
-		dev_dbg(&host->mmc->class_dev, "card %s\n",
+	if (present != present_old) {
+		struct atmel_mci	*host = slot->host;
+		struct mmc_request	*mrq;
+
+		dev_dbg(&slot->mmc->class_dev, "card %s\n",
 			present ? "inserted" : "removed");
-		host->present = present;
 
-		/* Reset controller if card is gone */
-		if (!present) {
-			mci_writel(host, CR, MCI_CR_SWRST);
-			mci_writel(host, IDR, ~0UL);
-			mci_writel(host, CR, MCI_CR_MCIEN);
-		}
+		spin_lock(&host->lock);
+
+		if (!present)
+			clear_bit(ATMCI_CARD_PRESENT, &slot->flags);
+		else
+			set_bit(ATMCI_CARD_PRESENT, &slot->flags);
 
 		/* Clean up queue if present */
+		mrq = slot->mrq;
 		if (mrq) {
-			/*
-			 * Reset controller to terminate any ongoing
-			 * commands or data transfers.
-			 */
-			mci_writel(host, CR, MCI_CR_SWRST);
-			mci_readl(host, SR);
-
-			host->data = NULL;
-			host->cmd = NULL;
-
-			switch (host->state) {
-			case STATE_SENDING_CMD:
-				mrq->cmd->error = -ENOMEDIUM;
-				if (!mrq->data)
+			if (mrq == host->mrq) {
+				/*
+				 * Reset controller to terminate any ongoing
+				 * commands or data transfers.
+				 */
+				mci_writel(host, CR, MCI_CR_SWRST);
+				mci_writel(host, CR, MCI_CR_MCIEN);
+				mci_writel(host, MR, host->mode_reg);
+
+				host->data = NULL;
+				host->cmd = NULL;
+
+				switch (host->state) {
+				case STATE_IDLE:
 					break;
-				/* fall through */
-			case STATE_SENDING_DATA:
-				mrq->data->error = -ENOMEDIUM;
-				break;
-			case STATE_DATA_BUSY:
-			case STATE_DATA_ERROR:
-				if (mrq->data->error == -EINPROGRESS)
+				case STATE_SENDING_CMD:
+					mrq->cmd->error = -ENOMEDIUM;
+					if (!mrq->data)
+						break;
+					/* fall through */
+				case STATE_SENDING_DATA:
 					mrq->data->error = -ENOMEDIUM;
-				if (!mrq->stop)
 					break;
-				/* fall through */
-			case STATE_SENDING_STOP:
-				mrq->stop->error = -ENOMEDIUM;
-				break;
-			}
+				case STATE_DATA_BUSY:
+				case STATE_DATA_ERROR:
+					if (mrq->data->error == -EINPROGRESS)
+						mrq->data->error = -ENOMEDIUM;
+					if (!mrq->stop)
+						break;
+					/* fall through */
+				case STATE_SENDING_STOP:
+					mrq->stop->error = -ENOMEDIUM;
+					break;
+				}
 
-			atmci_request_end(host->mmc, mrq);
+				atmci_request_end(host, mrq);
+			} else {
+				list_del(&slot->queue_node);
+				mrq->cmd->error = -ENOMEDIUM;
+				if (mrq->data)
+					mrq->data->error = -ENOMEDIUM;
+				if (mrq->stop)
+					mrq->stop->error = -ENOMEDIUM;
+
+				spin_unlock(&host->lock);
+				mmc_request_done(slot->mmc, mrq);
+				spin_lock(&host->lock);
+			}
 		}
+		spin_unlock(&host->lock);
 
-		mmc_detect_change(host->mmc, 0);
+		mmc_detect_change(slot->mmc, 0);
 	}
 }
 
 static void atmci_tasklet_func(unsigned long priv)
 {
-	struct mmc_host		*mmc = (struct mmc_host *)priv;
-	struct atmel_mci	*host = mmc_priv(mmc);
+	struct atmel_mci	*host = (struct atmel_mci *)priv;
 	struct mmc_request	*mrq = host->mrq;
 	struct mmc_data		*data = host->data;
 	struct mmc_command	*cmd = host->cmd;
@@ -700,9 +960,11 @@ static void atmci_tasklet_func(unsigned long priv)
 	enum atmel_mci_state	prev_state;
 	u32			status;
 
+	spin_lock(&host->lock);
+
 	state = host->state;
 
-	dev_vdbg(&mmc->class_dev,
+	dev_vdbg(&host->pdev->dev,
 		"tasklet: state %u pending/completed/mask %lx/%lx/%x\n",
 		state, host->pending_events, host->completed_events,
 		mci_readl(host, IMR));
@@ -711,6 +973,9 @@ static void atmci_tasklet_func(unsigned long priv)
 		prev_state = state;
 
 		switch (state) {
+		case STATE_IDLE:
+			break;
+
 		case STATE_SENDING_CMD:
 			if (!atmci_test_and_clear_pending(host,
 						EVENT_CMD_COMPLETE))
@@ -720,8 +985,8 @@ static void atmci_tasklet_func(unsigned long priv)
 			atmci_set_completed(host, EVENT_CMD_COMPLETE);
 			atmci_command_complete(host, mrq->cmd);
 			if (!mrq->data || cmd->error) {
-				atmci_request_end(mmc, host->mrq);
-				break;
+				atmci_request_end(host, host->mrq);
+				goto unlock;
 			}
 
 			prev_state = state = STATE_SENDING_DATA;
@@ -731,7 +996,7 @@ static void atmci_tasklet_func(unsigned long priv)
 			if (atmci_test_and_clear_pending(host,
 						EVENT_DATA_ERROR)) {
 				if (data->stop)
-					send_stop_cmd(host->mmc, data);
+					send_stop_cmd(host, data);
 				state = STATE_DATA_ERROR;
 				break;
 			}
@@ -754,15 +1019,15 @@ static void atmci_tasklet_func(unsigned long priv)
 			status = host->data_status;
 			if (unlikely(status & ATMCI_DATA_ERROR_FLAGS)) {
 				if (status & MCI_DTOE) {
-					dev_dbg(&mmc->class_dev,
+					dev_dbg(&host->pdev->dev,
 							"data timeout error\n");
 					data->error = -ETIMEDOUT;
 				} else if (status & MCI_DCRCE) {
-					dev_dbg(&mmc->class_dev,
+					dev_dbg(&host->pdev->dev,
 							"data CRC error\n");
 					data->error = -EILSEQ;
 				} else {
-					dev_dbg(&mmc->class_dev,
+					dev_dbg(&host->pdev->dev,
 						"data FIFO error (status=%08x)\n",
 						status);
 					data->error = -EIO;
@@ -773,14 +1038,13 @@ static void atmci_tasklet_func(unsigned long priv)
 			}
 
 			if (!data->stop) {
-				atmci_request_end(mmc, host->mrq);
-				prev_state = state;
-				break;
+				atmci_request_end(host, host->mrq);
+				goto unlock;
 			}
 
 			prev_state = state = STATE_SENDING_STOP;
 			if (!data->error)
-				send_stop_cmd(host->mmc, data);
+				send_stop_cmd(host, data);
 			/* fall through */
 
 		case STATE_SENDING_STOP:
@@ -790,9 +1054,8 @@ static void atmci_tasklet_func(unsigned long priv)
 
 			host->cmd = NULL;
 			atmci_command_complete(host, mrq->stop);
-			atmci_request_end(mmc, host->mrq);
-			prev_state = state;
-			break;
+			atmci_request_end(host, host->mrq);
+			goto unlock;
 
 		case STATE_DATA_ERROR:
 			if (!atmci_test_and_clear_pending(host,
@@ -805,6 +1068,9 @@ static void atmci_tasklet_func(unsigned long priv)
 	} while (state != prev_state);
 
 	host->state = state;
+
+unlock:
+	spin_unlock(&host->lock);
 }
 
 static void atmci_read_data_pio(struct atmel_mci *host)
@@ -854,9 +1120,11 @@ static void atmci_read_data_pio(struct atmel_mci *host)
 			mci_writel(host, IDR, (MCI_NOTBUSY | MCI_RXRDY
 						| ATMCI_DATA_ERROR_FLAGS));
 			host->data_status = status;
+			data->bytes_xfered += nbytes;
+			smp_wmb();
 			atmci_set_pending(host, EVENT_DATA_ERROR);
 			tasklet_schedule(&host->tasklet);
-			break;
+			return;
 		}
 	} while (status & MCI_RXRDY);
 
@@ -869,6 +1137,7 @@ done:
 	mci_writel(host, IDR, MCI_RXRDY);
 	mci_writel(host, IER, MCI_NOTBUSY);
 	data->bytes_xfered += nbytes;
+	smp_wmb();
 	atmci_set_pending(host, EVENT_XFER_COMPLETE);
 }
 
@@ -922,9 +1191,11 @@ static void atmci_write_data_pio(struct atmel_mci *host)
 			mci_writel(host, IDR, (MCI_NOTBUSY | MCI_TXRDY
 						| ATMCI_DATA_ERROR_FLAGS));
 			host->data_status = status;
+			data->bytes_xfered += nbytes;
+			smp_wmb();
 			atmci_set_pending(host, EVENT_DATA_ERROR);
 			tasklet_schedule(&host->tasklet);
-			break;
+			return;
 		}
 	} while (status & MCI_TXRDY);
 
@@ -937,29 +1208,26 @@ done:
 	mci_writel(host, IDR, MCI_TXRDY);
 	mci_writel(host, IER, MCI_NOTBUSY);
 	data->bytes_xfered += nbytes;
+	smp_wmb();
 	atmci_set_pending(host, EVENT_XFER_COMPLETE);
 }
 
-static void atmci_cmd_interrupt(struct mmc_host *mmc, u32 status)
+static void atmci_cmd_interrupt(struct atmel_mci *host, u32 status)
 {
-	struct atmel_mci	*host = mmc_priv(mmc);
-
 	mci_writel(host, IDR, MCI_CMDRDY);
 
 	host->cmd_status = status;
+	smp_wmb();
 	atmci_set_pending(host, EVENT_CMD_COMPLETE);
 	tasklet_schedule(&host->tasklet);
 }
 
 static irqreturn_t atmci_interrupt(int irq, void *dev_id)
 {
-	struct mmc_host		*mmc = dev_id;
-	struct atmel_mci	*host = mmc_priv(mmc);
+	struct atmel_mci	*host = dev_id;
 	u32			status, mask, pending;
 	unsigned int		pass_count = 0;
 
-	spin_lock(&mmc->lock);
-
 	do {
 		status = mci_readl(host, SR);
 		mask = mci_readl(host, IMR);
@@ -971,7 +1239,9 @@ static irqreturn_t atmci_interrupt(int irq, void *dev_id)
 			mci_writel(host, IDR, ATMCI_DATA_ERROR_FLAGS
 					| MCI_RXRDY | MCI_TXRDY);
 			pending &= mci_readl(host, IMR);
+
 			host->data_status = status;
+			smp_wmb();
 			atmci_set_pending(host, EVENT_DATA_ERROR);
 			tasklet_schedule(&host->tasklet);
 		}
@@ -979,6 +1249,7 @@ static irqreturn_t atmci_interrupt(int irq, void *dev_id)
 			mci_writel(host, IDR,
 					ATMCI_DATA_ERROR_FLAGS | MCI_NOTBUSY);
 			host->data_status = status;
+			smp_wmb();
 			atmci_set_pending(host, EVENT_DATA_COMPLETE);
 			tasklet_schedule(&host->tasklet);
 		}
@@ -988,18 +1259,15 @@ static irqreturn_t atmci_interrupt(int irq, void *dev_id)
 			atmci_write_data_pio(host);
 
 		if (pending & MCI_CMDRDY)
-			atmci_cmd_interrupt(mmc, status);
+			atmci_cmd_interrupt(host, status);
 	} while (pass_count++ < 5);
 
-	spin_unlock(&mmc->lock);
-
 	return pass_count ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static irqreturn_t atmci_detect_interrupt(int irq, void *dev_id)
 {
-	struct mmc_host		*mmc = dev_id;
-	struct atmel_mci	*host = mmc_priv(mmc);
+	struct atmel_mci_slot	*slot = dev_id;
 
 	/*
 	 * Disable interrupts until the pin has stabilized and check
@@ -1007,21 +1275,122 @@ static irqreturn_t atmci_detect_interrupt(int irq, void *dev_id)
 	 * middle of the timer routine when this interrupt triggers.
 	 */
 	disable_irq_nosync(irq);
-	mod_timer(&host->detect_timer, jiffies + msecs_to_jiffies(20));
+	mod_timer(&slot->detect_timer, jiffies + msecs_to_jiffies(20));
 
 	return IRQ_HANDLED;
 }
 
+static int __init atmci_init_slot(struct atmel_mci *host,
+		struct mci_slot_pdata *slot_data, unsigned int id,
+		u32 sdc_reg)
+{
+	struct mmc_host			*mmc;
+	struct atmel_mci_slot		*slot;
+
+	mmc = mmc_alloc_host(sizeof(struct atmel_mci_slot), &host->pdev->dev);
+	if (!mmc)
+		return -ENOMEM;
+
+	slot = mmc_priv(mmc);
+	slot->mmc = mmc;
+	slot->host = host;
+	slot->detect_pin = slot_data->detect_pin;
+	slot->wp_pin = slot_data->wp_pin;
+	slot->sdc_reg = sdc_reg;
+
+	mmc->ops = &atmci_ops;
+	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 512);
+	mmc->f_max = host->bus_hz / 2;
+	mmc->ocr_avail	= MMC_VDD_32_33 | MMC_VDD_33_34;
+	if (slot_data->bus_width >= 4)
+		mmc->caps |= MMC_CAP_4_BIT_DATA;
+
+	mmc->max_hw_segs = 64;
+	mmc->max_phys_segs = 64;
+	mmc->max_req_size = 32768 * 512;
+	mmc->max_blk_size = 32768;
+	mmc->max_blk_count = 512;
+
+	/* Assume card is present initially */
+	set_bit(ATMCI_CARD_PRESENT, &slot->flags);
+	if (gpio_is_valid(slot->detect_pin)) {
+		if (gpio_request(slot->detect_pin, "mmc_detect")) {
+			dev_dbg(&mmc->class_dev, "no detect pin available\n");
+			slot->detect_pin = -EBUSY;
+		} else if (gpio_get_value(slot->detect_pin)) {
+			clear_bit(ATMCI_CARD_PRESENT, &slot->flags);
+		}
+	}
+
+	if (!gpio_is_valid(slot->detect_pin))
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
+	if (gpio_is_valid(slot->wp_pin)) {
+		if (gpio_request(slot->wp_pin, "mmc_wp")) {
+			dev_dbg(&mmc->class_dev, "no WP pin available\n");
+			slot->wp_pin = -EBUSY;
+		}
+	}
+
+	host->slot[id] = slot;
+	mmc_add_host(mmc);
+
+	if (gpio_is_valid(slot->detect_pin)) {
+		int ret;
+
+		setup_timer(&slot->detect_timer, atmci_detect_change,
+				(unsigned long)slot);
+
+		ret = request_irq(gpio_to_irq(slot->detect_pin),
+				atmci_detect_interrupt,
+				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
+				"mmc-detect", slot);
+		if (ret) {
+			dev_dbg(&mmc->class_dev,
+				"could not request IRQ %d for detect pin\n",
+				gpio_to_irq(slot->detect_pin));
+			gpio_free(slot->detect_pin);
+			slot->detect_pin = -EBUSY;
+		}
+	}
+
+	atmci_init_debugfs(slot);
+
+	return 0;
+}
+
+static void __exit atmci_cleanup_slot(struct atmel_mci_slot *slot,
+		unsigned int id)
+{
+	/* Debugfs stuff is cleaned up by mmc core */
+
+	set_bit(ATMCI_SHUTDOWN, &slot->flags);
+	smp_wmb();
+
+	mmc_remove_host(slot->mmc);
+
+	if (gpio_is_valid(slot->detect_pin)) {
+		int pin = slot->detect_pin;
+
+		free_irq(gpio_to_irq(pin), slot);
+		del_timer_sync(&slot->detect_timer);
+		gpio_free(pin);
+	}
+	if (gpio_is_valid(slot->wp_pin))
+		gpio_free(slot->wp_pin);
+
+	slot->host->slot[id] = NULL;
+	mmc_free_host(slot->mmc);
+}
+
 static int __init atmci_probe(struct platform_device *pdev)
 {
 	struct mci_platform_data	*pdata;
-	struct mci_slot_pdata		*slot;
-	struct atmel_mci *host;
-	struct mmc_host *mmc;
-	struct resource *regs;
-	u32 sdc_reg;
-	int irq;
-	int ret;
+	struct atmel_mci		*host;
+	struct resource			*regs;
+	unsigned int			nr_slots;
+	int				irq;
+	int				ret;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs)
@@ -1033,27 +1402,13 @@ static int __init atmci_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	/* TODO: Allow using several slots at once */
-	if (pdata->slot[0].bus_width) {
-		sdc_reg = MCI_SDCSEL_SLOT_A;
-		slot = &pdata->slot[0];
-	} else if (pdata->slot[1].bus_width) {
-		sdc_reg = MCI_SDCSEL_SLOT_B;
-		slot = &pdata->slot[1];
-	} else {
-		return -EINVAL;
-	}
-
-	mmc = mmc_alloc_host(sizeof(struct atmel_mci), &pdev->dev);
-	if (!mmc)
+	host = kzalloc(sizeof(struct atmel_mci), GFP_KERNEL);
+	if (!host)
 		return -ENOMEM;
 
-	host = mmc_priv(mmc);
 	host->pdev = pdev;
-	host->mmc = mmc;
-	host->detect_pin = slot->detect_pin;
-	host->wp_pin = slot->wp_pin;
-	host->sdc_reg = sdc_reg;
+	spin_lock_init(&host->lock);
+	INIT_LIST_HEAD(&host->queue);
 
 	host->mck = clk_get(&pdev->dev, "mci_clk");
 	if (IS_ERR(host->mck)) {
@@ -1073,123 +1428,74 @@ static int __init atmci_probe(struct platform_device *pdev)
 
 	host->mapbase = regs->start;
 
-	mmc->ops = &atmci_ops;
-	mmc->f_min = (host->bus_hz + 511) / 512;
-	mmc->f_max = host->bus_hz / 2;
-	mmc->ocr_avail	= MMC_VDD_32_33 | MMC_VDD_33_34;
-	if (slot->bus_width >= 4)
-		mmc->caps |= MMC_CAP_4_BIT_DATA;
-
-	mmc->max_hw_segs = 64;
-	mmc->max_phys_segs = 64;
-	mmc->max_req_size = 32768 * 512;
-	mmc->max_blk_size = 32768;
-	mmc->max_blk_count = 512;
-
-	tasklet_init(&host->tasklet, atmci_tasklet_func, (unsigned long)mmc);
+	tasklet_init(&host->tasklet, atmci_tasklet_func, (unsigned long)host);
 
-	ret = request_irq(irq, atmci_interrupt, 0, pdev->dev.bus_id, mmc);
+	ret = request_irq(irq, atmci_interrupt, 0, pdev->dev.bus_id, host);
 	if (ret)
 		goto err_request_irq;
 
-	/* Assume card is present if we don't have a detect pin */
-	host->present = 1;
-	if (gpio_is_valid(host->detect_pin)) {
-		if (gpio_request(host->detect_pin, "mmc_detect")) {
-			dev_dbg(&mmc->class_dev, "no detect pin available\n");
-			host->detect_pin = -1;
-		} else {
-			host->present = !gpio_get_value(host->detect_pin);
-		}
-	}
-
-	if (!gpio_is_valid(host->detect_pin))
-		mmc->caps |= MMC_CAP_NEEDS_POLL;
-
-	if (gpio_is_valid(host->wp_pin)) {
-		if (gpio_request(host->wp_pin, "mmc_wp")) {
-			dev_dbg(&mmc->class_dev, "no WP pin available\n");
-			host->wp_pin = -1;
-		}
-	}
-
 	platform_set_drvdata(pdev, host);
 
-	mmc_add_host(mmc);
-
-	if (gpio_is_valid(host->detect_pin)) {
-		setup_timer(&host->detect_timer, atmci_detect_change,
-				(unsigned long)host);
-
-		ret = request_irq(gpio_to_irq(host->detect_pin),
-				atmci_detect_interrupt,
-				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-				"mmc-detect", mmc);
-		if (ret) {
-			dev_dbg(&mmc->class_dev,
-				"could not request IRQ %d for detect pin\n",
-				gpio_to_irq(host->detect_pin));
-			gpio_free(host->detect_pin);
-			host->detect_pin = -1;
-		}
+	/* We need at least one slot to succeed */
+	nr_slots = 0;
+	ret = -ENODEV;
+	if (pdata->slot[0].bus_width) {
+		ret = atmci_init_slot(host, &pdata->slot[0],
+				MCI_SDCSEL_SLOT_A, 0);
+		if (!ret)
+			nr_slots++;
+	}
+	if (pdata->slot[1].bus_width) {
+		ret = atmci_init_slot(host, &pdata->slot[1],
+				MCI_SDCSEL_SLOT_B, 1);
+		if (!ret)
+			nr_slots++;
 	}
 
-	dev_info(&mmc->class_dev,
-			"Atmel MCI controller at 0x%08lx irq %d\n",
-			host->mapbase, irq);
+	if (!nr_slots)
+		goto err_init_slot;
 
-	atmci_init_debugfs(host);
+	dev_info(&pdev->dev,
+			"Atmel MCI controller at 0x%08lx irq %d, %u slots\n",
+			host->mapbase, irq, nr_slots);
 
 	return 0;
 
+err_init_slot:
+	free_irq(irq, host);
 err_request_irq:
 	iounmap(host->regs);
 err_ioremap:
 	clk_put(host->mck);
 err_clk_get:
-	mmc_free_host(mmc);
+	kfree(host);
 	return ret;
 }
 
 static int __exit atmci_remove(struct platform_device *pdev)
 {
-	struct atmel_mci *host = platform_get_drvdata(pdev);
+	struct atmel_mci	*host = platform_get_drvdata(pdev);
+	unsigned int		i;
 
 	platform_set_drvdata(pdev, NULL);
 
-	if (host) {
-		/* Debugfs stuff is cleaned up by mmc core */
-
-		if (gpio_is_valid(host->detect_pin)) {
-			int pin = host->detect_pin;
-
-			/* Make sure the timer doesn't enable the interrupt */
-			host->detect_pin = -1;
-			smp_wmb();
-
-			free_irq(gpio_to_irq(pin), host->mmc);
-			del_timer_sync(&host->detect_timer);
-			gpio_free(pin);
-		}
-
-		mmc_remove_host(host->mmc);
-
-		clk_enable(host->mck);
-		mci_writel(host, IDR, ~0UL);
-		mci_writel(host, CR, MCI_CR_MCIDIS);
-		mci_readl(host, SR);
-		clk_disable(host->mck);
+	for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) {
+		if (host->slot[i])
+			atmci_cleanup_slot(host->slot[i], i);
+	}
 
-		if (gpio_is_valid(host->wp_pin))
-			gpio_free(host->wp_pin);
+	clk_enable(host->mck);
+	mci_writel(host, IDR, ~0UL);
+	mci_writel(host, CR, MCI_CR_MCIDIS);
+	mci_readl(host, SR);
+	clk_disable(host->mck);
 
-		free_irq(platform_get_irq(pdev, 0), host->mmc);
-		iounmap(host->regs);
+	free_irq(platform_get_irq(pdev, 0), host);
+	iounmap(host->regs);
 
-		clk_put(host->mck);
+	clk_put(host->mck);
+	kfree(host);
 
-		mmc_free_host(host->mmc);
-	}
 	return 0;
 }
 
-- 
1.5.6.5


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

* [PATCH v2 5/7] atmel-mci: Add experimental DMA support
  2008-10-05 16:21       ` [PATCH v2 4/7] atmel-mci: support " Haavard Skinnemoen
@ 2008-10-05 16:21         ` Haavard Skinnemoen
  2008-10-05 16:21           ` [PATCH v2 6/7] atmel-mci: Don't overwrite error bits when NOTBUSY is set Haavard Skinnemoen
  2008-10-12  8:44           ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Pierre Ossman
  2008-10-12  8:39         ` [PATCH v2 4/7] atmel-mci: support multiple mmc slots Pierre Ossman
  1 sibling, 2 replies; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

This adds support for DMA transfers through the generic DMA engine
framework with the DMA slave extensions.

The driver has been tested using mmc-block and ext3fs on several SD,
SDHC and MMC+ cards. Reads and writes work fine, with read transfer
rates up to 7.5 MiB/s on fast cards with debugging disabled.

Unfortunately, the driver has been known to lock up from time to time
with DMA enabled, so DMA support is currently optional and marked
EXPERIMENTAL. However, I didn't see any problems while testing 13
different cards (MMC, SD and SDHC of different brands and sizes), so I
suspect the "Initialize BLKR before sending data transfer command" fix
that was posted earlier fixed this as well.

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 arch/avr32/include/asm/atmel-mci.h  |    4 +
 arch/avr32/mach-at32ap/at32ap700x.c |   16 ++
 drivers/mmc/host/Kconfig            |   11 ++
 drivers/mmc/host/atmel-mci.c        |  274 +++++++++++++++++++++++++++++++++--
 4 files changed, 289 insertions(+), 16 deletions(-)

diff --git a/arch/avr32/include/asm/atmel-mci.h b/arch/avr32/include/asm/atmel-mci.h
index 5d5ae12..59f3fad 100644
--- a/arch/avr32/include/asm/atmel-mci.h
+++ b/arch/avr32/include/asm/atmel-mci.h
@@ -3,6 +3,8 @@
 
 #define ATMEL_MCI_MAX_NR_SLOTS	2
 
+struct dma_slave;
+
 /**
  * struct mci_slot_pdata - board-specific per-slot configuration
  * @bus_width: Number of data lines wired up the slot
@@ -26,9 +28,11 @@ struct mci_slot_pdata {
 
 /**
  * struct mci_platform_data - board-specific MMC/SDcard configuration
+ * @dma_slave: DMA slave interface to use in data transfers, or NULL.
  * @slot: Per-slot configuration data.
  */
 struct mci_platform_data {
+	struct dma_slave	*dma_slave;
 	struct mci_slot_pdata	slot[ATMEL_MCI_MAX_NR_SLOTS];
 };
 
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index 9967d5a..f1b9a3a 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1273,6 +1273,7 @@ struct platform_device *__init
 at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
 {
 	struct platform_device		*pdev;
+	struct dw_dma_slave		*dws;
 
 	if (id != 0 || !data)
 		return NULL;
@@ -1289,6 +1290,21 @@ at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
 				ARRAY_SIZE(atmel_mci0_resource)))
 		goto fail;
 
+	if (data->dma_slave)
+		dws = kmemdup(to_dw_dma_slave(data->dma_slave),
+				sizeof(struct dw_dma_slave), GFP_KERNEL);
+	else
+		dws = kzalloc(sizeof(struct dw_dma_slave), GFP_KERNEL);
+
+	dws->slave.dev = &pdev->dev;
+	dws->slave.dma_dev = &dw_dmac0_device.dev;
+	dws->slave.reg_width = DMA_SLAVE_WIDTH_32BIT;
+	dws->cfg_hi = (DWC_CFGH_SRC_PER(0)
+				| DWC_CFGH_DST_PER(1));
+	dws->cfg_lo &= ~(DWC_CFGL_HS_DST_POL
+				| DWC_CFGL_HS_SRC_POL);
+
+	data->dma_slave = &dws->slave;
 
 	if (platform_device_add_data(pdev, data,
 				sizeof(struct mci_platform_data)))
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index ea8d7a3..1ce21d4 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -114,6 +114,17 @@ config MMC_ATMELMCI
 
 	  If unsure, say N.
 
+config MMC_ATMELMCI_DMA
+	bool "Atmel MCI DMA support (EXPERIMENTAL)"
+	depends on MMC_ATMELMCI && DMA_ENGINE && EXPERIMENTAL
+	help
+	  Say Y here to have the Atmel MCI driver use a DMA engine to
+	  do data transfers and thus increase the throughput and
+	  reduce the CPU utilization. Note that this is highly
+	  experimental and may cause the driver to lock up.
+
+	  If unsure, say N.
+
 config MMC_IMX
 	tristate "Motorola i.MX Multimedia Card Interface support"
 	depends on ARCH_IMX
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index d8ab351..d45dfa2 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -11,6 +11,8 @@
 #include <linux/clk.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/init.h>
@@ -33,6 +35,7 @@
 #include "atmel-mci-regs.h"
 
 #define ATMCI_DATA_ERROR_FLAGS	(MCI_DCRCE | MCI_DTOE | MCI_OVRE | MCI_UNRE)
+#define ATMCI_DMA_THRESHOLD	16
 
 enum {
 	EVENT_CMD_COMPLETE = 0,
@@ -50,6 +53,14 @@ enum atmel_mci_state {
 	STATE_DATA_ERROR,
 };
 
+struct atmel_mci_dma {
+#ifdef CONFIG_MMC_ATMELMCI_DMA
+	struct dma_client		client;
+	struct dma_chan			*chan;
+	struct dma_async_tx_descriptor	*data_desc;
+#endif
+};
+
 /**
  * struct atmel_mci - MMC controller state shared between all slots
  * @lock: Spinlock protecting the queue and associated data.
@@ -62,6 +73,8 @@ enum atmel_mci_state {
  * @cmd: The command currently being sent to the card, or NULL.
  * @data: The data currently being transferred, or NULL if no data
  *	transfer is in progress.
+ * @dma: DMA client state.
+ * @data_chan: DMA channel being used for the current data transfer.
  * @cmd_status: Snapshot of SR taken upon completion of the current
  *	command. Only valid when EVENT_CMD_COMPLETE is pending.
  * @data_status: Snapshot of SR taken upon completion of the current
@@ -126,6 +139,9 @@ struct atmel_mci {
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 
+	struct atmel_mci_dma	dma;
+	struct dma_chan		*data_chan;
+
 	u32			cmd_status;
 	u32			data_status;
 	u32			stop_cmdr;
@@ -485,6 +501,144 @@ static void send_stop_cmd(struct atmel_mci *host, struct mmc_data *data)
 	mci_writel(host, IER, MCI_CMDRDY);
 }
 
+#ifdef CONFIG_MMC_ATMELMCI_DMA
+static void atmci_dma_cleanup(struct atmel_mci *host)
+{
+	struct mmc_data			*data = host->data;
+
+	dma_unmap_sg(&host->pdev->dev, data->sg, data->sg_len,
+		     ((data->flags & MMC_DATA_WRITE)
+		      ? DMA_TO_DEVICE : DMA_FROM_DEVICE));
+}
+
+static void atmci_stop_dma(struct atmel_mci *host)
+{
+	struct dma_chan *chan = host->data_chan;
+
+	if (chan) {
+		chan->device->device_terminate_all(chan);
+		atmci_dma_cleanup(host);
+	} else {
+		/* Data transfer was stopped by the interrupt handler */
+		atmci_set_pending(host, EVENT_XFER_COMPLETE);
+		mci_writel(host, IER, MCI_NOTBUSY);
+	}
+}
+
+/* This function is called by the DMA driver from tasklet context. */
+static void atmci_dma_complete(void *arg)
+{
+	struct atmel_mci	*host = arg;
+	struct mmc_data		*data = host->data;
+
+	dev_vdbg(&host->pdev->dev, "DMA complete\n");
+
+	atmci_dma_cleanup(host);
+
+	/*
+	 * If the card was removed, data will be NULL. No point trying
+	 * to send the stop command or waiting for NBUSY in this case.
+	 */
+	if (data) {
+		atmci_set_pending(host, EVENT_XFER_COMPLETE);
+		tasklet_schedule(&host->tasklet);
+
+		/*
+		 * Regardless of what the documentation says, we have
+		 * to wait for NOTBUSY even after block read
+		 * operations.
+		 *
+		 * When the DMA transfer is complete, the controller
+		 * may still be reading the CRC from the card, i.e.
+		 * the data transfer is still in progress and we
+		 * haven't seen all the potential error bits yet.
+		 *
+		 * The interrupt handler will schedule a different
+		 * tasklet to finish things up when the data transfer
+		 * is completely done.
+		 *
+		 * We may not complete the mmc request here anyway
+		 * because the mmc layer may call back and cause us to
+		 * violate the "don't submit new operations from the
+		 * completion callback" rule of the dma engine
+		 * framework.
+		 */
+		mci_writel(host, IER, MCI_NOTBUSY);
+	}
+}
+
+static int
+atmci_submit_data_dma(struct atmel_mci *host, struct mmc_data *data)
+{
+	struct dma_chan			*chan;
+	struct dma_async_tx_descriptor	*desc;
+	struct scatterlist		*sg;
+	unsigned int			i;
+	enum dma_data_direction		direction;
+
+	/*
+	 * We don't do DMA on "complex" transfers, i.e. with
+	 * non-word-aligned buffers or lengths. Also, we don't bother
+	 * with all the DMA setup overhead for short transfers.
+	 */
+	if (data->blocks * data->blksz < ATMCI_DMA_THRESHOLD)
+		return -EINVAL;
+	if (data->blksz & 3)
+		return -EINVAL;
+
+	for_each_sg(data->sg, sg, data->sg_len, i) {
+		if (sg->offset & 3 || sg->length & 3)
+			return -EINVAL;
+	}
+
+	/* If we don't have a channel, we can't do DMA */
+	chan = host->dma.chan;
+	if (chan) {
+		dma_chan_get(chan);
+		host->data_chan = chan;
+	}
+
+	if (!chan)
+		return -ENODEV;
+
+	if (data->flags & MMC_DATA_READ)
+		direction = DMA_FROM_DEVICE;
+	else
+		direction = DMA_TO_DEVICE;
+
+	desc = chan->device->device_prep_slave_sg(chan,
+			data->sg, data->sg_len, direction,
+			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!desc)
+		return -ENOMEM;
+
+	host->dma.data_desc = desc;
+	desc->callback = atmci_dma_complete;
+	desc->callback_param = host;
+	desc->tx_submit(desc);
+
+	/* Go! */
+	chan->device->device_issue_pending(chan);
+
+	return 0;
+}
+
+#else /* CONFIG_MMC_ATMELMCI_DMA */
+
+static int atmci_submit_data_dma(struct atmel_mci *host, struct mmc_data *data)
+{
+	return -ENOSYS;
+}
+
+static void atmci_stop_dma(struct atmel_mci *host)
+{
+	/* Data transfer was stopped by the interrupt handler */
+	atmci_set_pending(host, EVENT_XFER_COMPLETE);
+	mci_writel(host, IER, MCI_NOTBUSY);
+}
+
+#endif /* CONFIG_MMC_ATMELMCI_DMA */
+
 /*
  * Returns a mask of interrupt flags to be enabled after the whole
  * request has been prepared.
@@ -500,24 +654,27 @@ static u32 atmci_submit_data(struct atmel_mci *host, struct mmc_data *data)
 	host->data = data;
 
 	iflags = ATMCI_DATA_ERROR_FLAGS;
+	if (atmci_submit_data_dma(host, data)) {
+		host->data_chan = NULL;
 
-	/*
-	 * Errata: MMC data write operation with less than 12
-	 * bytes is impossible.
-	 *
-	 * Errata: MCI Transmit Data Register (TDR) FIFO
-	 * corruption when length is not multiple of 4.
-	 */
-	if (data->blocks * data->blksz < 12
-			|| (data->blocks * data->blksz) & 3)
-		host->need_reset = true;
+		/*
+		 * Errata: MMC data write operation with less than 12
+		 * bytes is impossible.
+		 *
+		 * Errata: MCI Transmit Data Register (TDR) FIFO
+		 * corruption when length is not multiple of 4.
+		 */
+		if (data->blocks * data->blksz < 12
+				|| (data->blocks * data->blksz) & 3)
+			host->need_reset = true;
 
-	host->sg = data->sg;
-	host->pio_offset = 0;
-	if (data->flags & MMC_DATA_READ)
-		iflags |= MCI_RXRDY;
-	else
-		iflags |= MCI_TXRDY;
+		host->sg = data->sg;
+		host->pio_offset = 0;
+		if (data->flags & MMC_DATA_READ)
+			iflags |= MCI_RXRDY;
+		else
+			iflags |= MCI_TXRDY;
+	}
 
 	return iflags;
 }
@@ -848,6 +1005,7 @@ static void atmci_command_complete(struct atmel_mci *host,
 
 		if (cmd->data) {
 			host->data = NULL;
+			atmci_stop_dma(host);
 			mci_writel(host, IDR, MCI_NOTBUSY
 					| MCI_TXRDY | MCI_RXRDY
 					| ATMCI_DATA_ERROR_FLAGS);
@@ -917,6 +1075,7 @@ static void atmci_detect_change(unsigned long data)
 					/* fall through */
 				case STATE_SENDING_DATA:
 					mrq->data->error = -ENOMEDIUM;
+					atmci_stop_dma(host);
 					break;
 				case STATE_DATA_BUSY:
 				case STATE_DATA_ERROR:
@@ -995,6 +1154,7 @@ static void atmci_tasklet_func(unsigned long priv)
 		case STATE_SENDING_DATA:
 			if (atmci_test_and_clear_pending(host,
 						EVENT_DATA_ERROR)) {
+				atmci_stop_dma(host);
 				if (data->stop)
 					send_stop_cmd(host, data);
 				state = STATE_DATA_ERROR;
@@ -1280,6 +1440,60 @@ static irqreturn_t atmci_detect_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_MMC_ATMELMCI_DMA
+
+static inline struct atmel_mci *
+dma_client_to_atmel_mci(struct dma_client *client)
+{
+	return container_of(client, struct atmel_mci, dma.client);
+}
+
+static enum dma_state_client atmci_dma_event(struct dma_client *client,
+		struct dma_chan *chan, enum dma_state state)
+{
+	struct atmel_mci	*host;
+	enum dma_state_client	ret = DMA_NAK;
+
+	host = dma_client_to_atmel_mci(client);
+
+	switch (state) {
+	case DMA_RESOURCE_AVAILABLE:
+		spin_lock_bh(&host->lock);
+		if (!host->dma.chan) {
+			host->dma.chan = chan;
+			ret = DMA_ACK;
+		}
+		spin_unlock_bh(&host->lock);
+
+		if (ret == DMA_ACK)
+			dev_info(&host->pdev->dev,
+					"Using %s for DMA transfers\n",
+					chan->dev.bus_id);
+		break;
+
+	case DMA_RESOURCE_REMOVED:
+		spin_lock_bh(&host->lock);
+		if (host->dma.chan == chan) {
+			host->dma.chan = NULL;
+			ret = DMA_ACK;
+		}
+		spin_unlock_bh(&host->lock);
+
+		if (ret == DMA_ACK)
+			dev_info(&host->pdev->dev,
+					"Lost %s, falling back to PIO\n",
+					chan->dev.bus_id);
+		break;
+
+	default:
+		break;
+	}
+
+
+	return ret;
+}
+#endif /* CONFIG_MMC_ATMELMCI_DMA */
+
 static int __init atmci_init_slot(struct atmel_mci *host,
 		struct mci_slot_pdata *slot_data, unsigned int id,
 		u32 sdc_reg)
@@ -1434,6 +1648,25 @@ static int __init atmci_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_request_irq;
 
+#ifdef CONFIG_MMC_ATMELMCI_DMA
+	if (pdata->dma_slave) {
+		struct dma_slave *slave = pdata->dma_slave;
+
+		slave->tx_reg = regs->start + MCI_TDR;
+		slave->rx_reg = regs->start + MCI_RDR;
+
+		/* Try to grab a DMA channel */
+		host->dma.client.event_callback = atmci_dma_event;
+		dma_cap_set(DMA_SLAVE, host->dma.client.cap_mask);
+		host->dma.client.slave = slave;
+
+		dma_async_client_register(&host->dma.client);
+		dma_async_client_chan_request(&host->dma.client);
+	} else {
+		dev_notice(&pdev->dev, "DMA not available, using PIO\n");
+	}
+#endif /* CONFIG_MMC_ATMELMCI_DMA */
+
 	platform_set_drvdata(pdev, host);
 
 	/* We need at least one slot to succeed */
@@ -1462,6 +1695,10 @@ static int __init atmci_probe(struct platform_device *pdev)
 	return 0;
 
 err_init_slot:
+#ifdef CONFIG_MMC_ATMELMCI_DMA
+	if (pdata->dma_slave)
+		dma_async_client_unregister(&host->dma.client);
+#endif
 	free_irq(irq, host);
 err_request_irq:
 	iounmap(host->regs);
@@ -1490,6 +1727,11 @@ static int __exit atmci_remove(struct platform_device *pdev)
 	mci_readl(host, SR);
 	clk_disable(host->mck);
 
+#ifdef CONFIG_MMC_ATMELMCI_DMA
+	if (host->dma.client.slave)
+		dma_async_client_unregister(&host->dma.client);
+#endif
+
 	free_irq(platform_get_irq(pdev, 0), host);
 	iounmap(host->regs);
 
-- 
1.5.6.5


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

* [PATCH v2 6/7] atmel-mci: Don't overwrite error bits when NOTBUSY is set
  2008-10-05 16:21         ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Haavard Skinnemoen
@ 2008-10-05 16:21           ` Haavard Skinnemoen
  2008-10-05 16:21             ` [PATCH v2 7/7] atmel-mci: Add missing flush_dcache_page() in PIO transfer code Haavard Skinnemoen
  2008-10-12  8:44           ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Pierre Ossman
  1 sibling, 1 reply; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

After a data error, we wait for the NOTBUSY bit to be set so that we can
be sure the data transfer is completely finished. However, when NOTBUSY
is set, the interrupt handler copies the contents of SR into
data_status, overwriting any error bits we may have detected earlier.

To avoid this, initialize data_status to 0 before starting a request, and
don't overwrite it unless it still contains 0.

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/mmc/host/atmel-mci.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index d45dfa2..02529af 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -694,6 +694,7 @@ static void atmci_start_request(struct atmel_mci *host,
 
 	host->pending_events = 0;
 	host->completed_events = 0;
+	host->data_status = 0;
 
 	if (host->need_reset) {
 		mci_writel(host, CR, MCI_CR_SWRST);
@@ -1408,7 +1409,8 @@ static irqreturn_t atmci_interrupt(int irq, void *dev_id)
 		if (pending & MCI_NOTBUSY) {
 			mci_writel(host, IDR,
 					ATMCI_DATA_ERROR_FLAGS | MCI_NOTBUSY);
-			host->data_status = status;
+			if (!host->data_status)
+				host->data_status = status;
 			smp_wmb();
 			atmci_set_pending(host, EVENT_DATA_COMPLETE);
 			tasklet_schedule(&host->tasklet);
-- 
1.5.6.5


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

* [PATCH v2 7/7] atmel-mci: Add missing flush_dcache_page() in PIO transfer code
  2008-10-05 16:21           ` [PATCH v2 6/7] atmel-mci: Don't overwrite error bits when NOTBUSY is set Haavard Skinnemoen
@ 2008-10-05 16:21             ` Haavard Skinnemoen
  0 siblings, 0 replies; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 16:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel, Haavard Skinnemoen

Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/mmc/host/atmel-mci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 02529af..7a3f243 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -1253,6 +1253,7 @@ static void atmci_read_data_pio(struct atmel_mci *host)
 			nbytes += 4;
 
 			if (offset == sg->length) {
+				flush_dcache_page(sg_page(sg));
 				host->sg = sg = sg_next(sg);
 				if (!sg)
 					goto done;
-- 
1.5.6.5


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

* Re: [PATCH v2 0/7] atmel-mci: updates for 2.6.28
  2008-10-05 16:21 [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
  2008-10-05 16:21 ` [PATCH v2 1/7] atmel-mci: Implement tasklet as a state machine Haavard Skinnemoen
@ 2008-10-05 18:46 ` Haavard Skinnemoen
  2008-10-12  8:50   ` Pierre Ossman
  1 sibling, 1 reply; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-05 18:46 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel

Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:
>  arch/avr32/boards/atngw100/setup.c      |    7 +-
>  arch/avr32/boards/atstk1000/atstk1002.c |   18 +-
>  arch/avr32/boards/atstk1000/atstk1003.c |   12 +-
>  arch/avr32/boards/atstk1000/atstk1004.c |   12 +-
>  arch/avr32/include/asm/atmel-mci.h      |   32 +-
>  arch/avr32/mach-at32ap/at32ap700x.c     |   90 ++-

Btw, this will conflict with some upcoming pinmux changes in the avr32
code...

If you're happy with this patchset, could you please pull

  git://git.kernel.org/pub/scm/linux/kernel/git/hskinnemoen/atmel-mci-2.6.28.git master

instead of applying the patches? If you do that, I can pull it into my
avr32 tree as well and fix up any conflicts. I won't do that before
you've pulled it, of course, and it's probably best if no one base any
further work on it until it's accepted because I might have to rebase
it if someone finds any issues during review.

Haavard

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

* Re: [PATCH v2 4/7] atmel-mci: support multiple mmc slots
  2008-10-05 16:21       ` [PATCH v2 4/7] atmel-mci: support " Haavard Skinnemoen
  2008-10-05 16:21         ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Haavard Skinnemoen
@ 2008-10-12  8:39         ` Pierre Ossman
  2008-10-12 13:13           ` Haavard Skinnemoen
  1 sibling, 1 reply; 15+ messages in thread
From: Pierre Ossman @ 2008-10-12  8:39 UTC (permalink / raw)
  To: Haavard Skinnemoen; +Cc: kernel, linux-kernel, Haavard Skinnemoen

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

On Sun,  5 Oct 2008 18:21:27 +0200
Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:

>  
>  	if (ios->clock) {
> +		unsigned int clock_min = ~0U;
>  		u32 clkdiv;
>  
> -		if (!host->mode_reg)
> +		spin_lock_bh(&host->lock);
> +		if (!host->mode_reg) {
>  			clk_enable(host->mck);
> +			mci_writel(host, CR, MCI_CR_SWRST);
> +			mci_writel(host, CR, MCI_CR_MCIEN);
> +		}
>  
> -		/* Set clock rate */
> -		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * ios->clock) - 1;
> +		/*
> +		 * Use mirror of ios->clock to prevent race with mmc
> +		 * core ios update when finding the minimum.
> +		 */
> +		slot->clock = ios->clock;
> +		for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) {
> +			if (host->slot[i] && host->slot[i]->clock
> +					&& host->slot[i]->clock < clock_min)
> +				clock_min = host->slot[i]->clock;
> +		}
> +

Hmm... This does not cover the power up case. You can only ignore the
other slot's clock setting if it is unpowered.

> +		/* Calculate clock divider */
> +		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * clock_min) - 1;
>  		if (clkdiv > 255) {
>  			dev_warn(&mmc->class_dev,
>  				"clock %u too slow; using %lu\n",
> -				ios->clock, host->bus_hz / (2 * 256));
> +				clock_min, host->bus_hz / (2 * 256));
>  			clkdiv = 255;
>  		}
>  

Has this ever happened, or is it just a bit defensive? :)

(not that there is something wrong with that)

>  	default:
>  		/*
>  		 * TODO: None of the currently available AVR32-based
>  		 * boards allow MMC power to be turned off. Implement
>  		 * power control when this can be tested properly.
> +		 *
> +		 * We also need to hook this into the clock management
> +		 * somehow so that newly inserted cards aren't
> +		 * subjected to a fast clock before we have a chance
> +		 * to figure out what the maximum rate is. Currently,
> +		 * there's no way to avoid this, and there never will
> +		 * be for boards that don't support power control.
>  		 */
>  		break;
>  	}

Hmm again... As you've pointed out, this could be a problem. The card
should survive getting jittery power during the insertion, but I
wouldn't chance having the clock running at that point (which will
happen if the other slot is populated).

I don't see a decent way of solving this, so I guess we'll have to
stick our heads in the sand for now...

(did I mention that I think this slot multiplexing thing is a bad idea?)

> @@ -922,9 +1191,11 @@ static void atmci_write_data_pio(struct atmel_mci *host)
>  			mci_writel(host, IDR, (MCI_NOTBUSY | MCI_TXRDY
>  						| ATMCI_DATA_ERROR_FLAGS));
>  			host->data_status = status;
> +			data->bytes_xfered += nbytes;
> +			smp_wmb();
>  			atmci_set_pending(host, EVENT_DATA_ERROR);
>  			tasklet_schedule(&host->tasklet);
> -			break;
> +			return;
>  		}
>  	} while (status & MCI_TXRDY);
>  
> @@ -937,29 +1208,26 @@ done:
>  	mci_writel(host, IDR, MCI_TXRDY);
>  	mci_writel(host, IER, MCI_NOTBUSY);
>  	data->bytes_xfered += nbytes;
> +	smp_wmb();
>  	atmci_set_pending(host, EVENT_XFER_COMPLETE);
>  }
>  

I'm not sure I commented on this during the last run, but bytes_xfered
should only be updated upon confirmation from the card, not as it goes
out over the bus (there might be CRC error for instance).

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  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: 197 bytes --]

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

* Re: [PATCH v2 5/7] atmel-mci: Add experimental DMA support
  2008-10-05 16:21         ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Haavard Skinnemoen
  2008-10-05 16:21           ` [PATCH v2 6/7] atmel-mci: Don't overwrite error bits when NOTBUSY is set Haavard Skinnemoen
@ 2008-10-12  8:44           ` Pierre Ossman
  1 sibling, 0 replies; 15+ messages in thread
From: Pierre Ossman @ 2008-10-12  8:44 UTC (permalink / raw)
  To: Haavard Skinnemoen; +Cc: kernel, linux-kernel, Haavard Skinnemoen

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

On Sun,  5 Oct 2008 18:21:28 +0200
Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:

> This adds support for DMA transfers through the generic DMA engine
> framework with the DMA slave extensions.
> 
> The driver has been tested using mmc-block and ext3fs on several SD,
> SDHC and MMC+ cards. Reads and writes work fine, with read transfer
> rates up to 7.5 MiB/s on fast cards with debugging disabled.
> 
> Unfortunately, the driver has been known to lock up from time to time
> with DMA enabled, so DMA support is currently optional and marked
> EXPERIMENTAL. However, I didn't see any problems while testing 13
> different cards (MMC, SD and SDHC of different brands and sizes), so I
> suspect the "Initialize BLKR before sending data transfer command" fix
> that was posted earlier fixed this as well.
> 
> Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> ---

Looks good, but I assume the plan is to remove this Kconfig option once
you're confident it is stable?

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  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: 197 bytes --]

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

* Re: [PATCH v2 0/7] atmel-mci: updates for 2.6.28
  2008-10-05 18:46 ` [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
@ 2008-10-12  8:50   ` Pierre Ossman
  0 siblings, 0 replies; 15+ messages in thread
From: Pierre Ossman @ 2008-10-12  8:50 UTC (permalink / raw)
  To: Haavard Skinnemoen; +Cc: kernel, linux-kernel

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

On Sun, 5 Oct 2008 20:46:17 +0200
Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:

> 
> If you're happy with this patchset, could you please pull
> 

Fairly happy. There is the one thing to fix with the clock, but as no
cards implement power control yet, you can send that patch later.

There's also the open issue of bytes_xfered. Please send a patch for
that ASAP (provided it just isn't a misunderstanding on my part :)).

>   git://git.kernel.org/pub/scm/linux/kernel/git/hskinnemoen/atmel-mci-2.6.28.git master
> 
> instead of applying the patches? If you do that, I can pull it into my
> avr32 tree as well and fix up any conflicts. I won't do that before
> you've pulled it, of course, and it's probably best if no one base any
> further work on it until it's accepted because I might have to rebase
> it if someone finds any issues during review.

Pulled and will be sent to Linus just as soon as I have everything else
queued up.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  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: 197 bytes --]

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

* Re: [PATCH v2 4/7] atmel-mci: support multiple mmc slots
  2008-10-12  8:39         ` [PATCH v2 4/7] atmel-mci: support multiple mmc slots Pierre Ossman
@ 2008-10-12 13:13           ` Haavard Skinnemoen
  2008-10-12 15:58             ` Pierre Ossman
  0 siblings, 1 reply; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-12 13:13 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> On Sun,  5 Oct 2008 18:21:27 +0200
> Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:
> 
> >  
> >  	if (ios->clock) {
> > +		unsigned int clock_min = ~0U;
> >  		u32 clkdiv;
> >  
> > -		if (!host->mode_reg)
> > +		spin_lock_bh(&host->lock);
> > +		if (!host->mode_reg) {
> >  			clk_enable(host->mck);
> > +			mci_writel(host, CR, MCI_CR_SWRST);
> > +			mci_writel(host, CR, MCI_CR_MCIEN);
> > +		}
> >  
> > -		/* Set clock rate */
> > -		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * ios->clock) - 1;
> > +		/*
> > +		 * Use mirror of ios->clock to prevent race with mmc
> > +		 * core ios update when finding the minimum.
> > +		 */
> > +		slot->clock = ios->clock;
> > +		for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) {
> > +			if (host->slot[i] && host->slot[i]->clock
> > +					&& host->slot[i]->clock < clock_min)
> > +				clock_min = host->slot[i]->clock;
> > +		}
> > +
> 
> Hmm... This does not cover the power up case. You can only ignore the
> other slot's clock setting if it is unpowered.

True. I don't have any boards which can control the power to the mmc
slots, so there's currently no way to avoid having a newly inserted
card exposed to a possibly high-speed clock while powered.

Once I get my hands on a board which can do that, I'll try to halt the
queue while powering up a new card and restart it when the clock speed
for the new card is known.

> > +		/* Calculate clock divider */
> > +		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * clock_min) - 1;
> >  		if (clkdiv > 255) {
> >  			dev_warn(&mmc->class_dev,
> >  				"clock %u too slow; using %lu\n",
> > -				ios->clock, host->bus_hz / (2 * 256));
> > +				clock_min, host->bus_hz / (2 * 256));
> >  			clkdiv = 255;
> >  		}
> >  
> 
> Has this ever happened, or is it just a bit defensive? :)
> 
> (not that there is something wrong with that)

It can happen if the controller is hooked up to a fast bus and fOD is
slow for whatever reason. And if it does, clkdiv will wrap around and
end up at a rate much faster than requested, so saturating it at the
maximum divider is the safe thing to do.

If the card ends up behaving strangely as a result of the clock
frequency being a bit off, at least you have a warning which may
indicate what's wrong.

> >  	default:
> >  		/*
> >  		 * TODO: None of the currently available AVR32-based
> >  		 * boards allow MMC power to be turned off. Implement
> >  		 * power control when this can be tested properly.
> > +		 *
> > +		 * We also need to hook this into the clock management
> > +		 * somehow so that newly inserted cards aren't
> > +		 * subjected to a fast clock before we have a chance
> > +		 * to figure out what the maximum rate is. Currently,
> > +		 * there's no way to avoid this, and there never will
> > +		 * be for boards that don't support power control.
> >  		 */
> >  		break;
> >  	}
> 
> Hmm again... As you've pointed out, this could be a problem. The card
> should survive getting jittery power during the insertion, but I
> wouldn't chance having the clock running at that point (which will
> happen if the other slot is populated).
> 
> I don't see a decent way of solving this, so I guess we'll have to
> stick our heads in the sand for now...

I think we'll just have to say that using multiplexing without power
control might cause problems with some cards.

> (did I mention that I think this slot multiplexing thing is a bad idea?)

Yes, I think you did. But note that this feature is optional (most
boards only request one slot), and I think people who actually have
multiple slots would rather use both of them than just one.

> > @@ -922,9 +1191,11 @@ static void atmci_write_data_pio(struct atmel_mci *host)
> >  			mci_writel(host, IDR, (MCI_NOTBUSY | MCI_TXRDY
> >  						| ATMCI_DATA_ERROR_FLAGS));
> >  			host->data_status = status;
> > +			data->bytes_xfered += nbytes;
> > +			smp_wmb();
> >  			atmci_set_pending(host, EVENT_DATA_ERROR);
> >  			tasklet_schedule(&host->tasklet);
> > -			break;
> > +			return;
> >  		}
> >  	} while (status & MCI_TXRDY);
> >  
> > @@ -937,29 +1208,26 @@ done:
> >  	mci_writel(host, IDR, MCI_TXRDY);
> >  	mci_writel(host, IER, MCI_NOTBUSY);
> >  	data->bytes_xfered += nbytes;
> > +	smp_wmb();
> >  	atmci_set_pending(host, EVENT_XFER_COMPLETE);
> >  }
> >  
> 
> I'm not sure I commented on this during the last run, but bytes_xfered
> should only be updated upon confirmation from the card, not as it goes
> out over the bus (there might be CRC error for instance).

You're right. I'll remove it -- it's updated when the card finishes its
busy signaling anyway.

Haavard

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

* Re: [PATCH v2 4/7] atmel-mci: support multiple mmc slots
  2008-10-12 13:13           ` Haavard Skinnemoen
@ 2008-10-12 15:58             ` Pierre Ossman
  2008-10-12 16:07               ` Haavard Skinnemoen
  0 siblings, 1 reply; 15+ messages in thread
From: Pierre Ossman @ 2008-10-12 15:58 UTC (permalink / raw)
  To: Haavard Skinnemoen; +Cc: kernel, linux-kernel

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

On Sun, 12 Oct 2008 15:13:44 +0200
Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:

> Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> > On Sun,  5 Oct 2008 18:21:27 +0200
> > Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:
> > 
> > > +		/* Calculate clock divider */
> > > +		clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * clock_min) - 1;
> > >  		if (clkdiv > 255) {
> > >  			dev_warn(&mmc->class_dev,
> > >  				"clock %u too slow; using %lu\n",
> > > -				ios->clock, host->bus_hz / (2 * 256));
> > > +				clock_min, host->bus_hz / (2 * 256));
> > >  			clkdiv = 255;
> > >  		}
> > >  
> > 
> > Has this ever happened, or is it just a bit defensive? :)
> > 
> > (not that there is something wrong with that)
> 
> It can happen if the controller is hooked up to a fast bus and fOD is
> slow for whatever reason. And if it does, clkdiv will wrap around and
> end up at a rate much faster than requested, so saturating it at the
> maximum divider is the safe thing to do.
> 

Isn't this calculated for f_min though so that the MMC core will never
request such a frequency?

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  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: 197 bytes --]

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

* Re: [PATCH v2 4/7] atmel-mci: support multiple mmc slots
  2008-10-12 15:58             ` Pierre Ossman
@ 2008-10-12 16:07               ` Haavard Skinnemoen
  0 siblings, 0 replies; 15+ messages in thread
From: Haavard Skinnemoen @ 2008-10-12 16:07 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: kernel, linux-kernel

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> > It can happen if the controller is hooked up to a fast bus and fOD is
> > slow for whatever reason. And if it does, clkdiv will wrap around and
> > end up at a rate much faster than requested, so saturating it at the
> > maximum divider is the safe thing to do.
> >   
> 
> Isn't this calculated for f_min though so that the MMC core will never
> request such a frequency?

Hmm...yes, you're probably right. Still, I think it's a nice safety net
if someone manages to mess up f_min or starts doing more aggressive
frequency scaling.

Haavard

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

end of thread, other threads:[~2008-10-12 16:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-05 16:21 [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
2008-10-05 16:21 ` [PATCH v2 1/7] atmel-mci: Implement tasklet as a state machine Haavard Skinnemoen
2008-10-05 16:21   ` [PATCH v2 2/7] atmel-mci: Don't stop the clock between transfers Haavard Skinnemoen
2008-10-05 16:21     ` [PATCH v2 3/7] atmel-mci: Platform code for supporting multiple mmc slots Haavard Skinnemoen
2008-10-05 16:21       ` [PATCH v2 4/7] atmel-mci: support " Haavard Skinnemoen
2008-10-05 16:21         ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Haavard Skinnemoen
2008-10-05 16:21           ` [PATCH v2 6/7] atmel-mci: Don't overwrite error bits when NOTBUSY is set Haavard Skinnemoen
2008-10-05 16:21             ` [PATCH v2 7/7] atmel-mci: Add missing flush_dcache_page() in PIO transfer code Haavard Skinnemoen
2008-10-12  8:44           ` [PATCH v2 5/7] atmel-mci: Add experimental DMA support Pierre Ossman
2008-10-12  8:39         ` [PATCH v2 4/7] atmel-mci: support multiple mmc slots Pierre Ossman
2008-10-12 13:13           ` Haavard Skinnemoen
2008-10-12 15:58             ` Pierre Ossman
2008-10-12 16:07               ` Haavard Skinnemoen
2008-10-05 18:46 ` [PATCH v2 0/7] atmel-mci: updates for 2.6.28 Haavard Skinnemoen
2008-10-12  8:50   ` Pierre Ossman

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