[PATCH 0/2] omap_hsmmc patches

[PATCH 0/2] omap_hsmmc patches

[Adrian Hunter]

Here are two patches for omap_hsmmc.  They are applied
on top of Jean Pihet's "OMAP: MMC: recover from transfer
failures" patch, which is here:

http://marc.info/?l=linux-omap&m=123141577308177&w=2

[PATCH 1/2] OMAP: HSMMC: Do dma cleanup also with data CRC errors

[Adrian Hunter]

>From 60c6ac58ae03cda78117aef7b0ad3dddbe4deb1a Mon Sep 17 00:00:00 2001
From: Jarkko Lavinen <jarkko.lavinen@nokia.com>
Date: Fri, 5 Dec 2008 12:31:46 +0200
Subject: [PATCH] OMAP: HSMMC: Do dma cleanup also with data CRC errors

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 97150c0..115d114 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -378,9 +378,9 @@ mmc_omap_cmd_done(struct mmc_omap_host *host, struct mmc_command *cmd)
 /*
  * DMA clean up for command errors
  */
-static void mmc_dma_cleanup(struct mmc_omap_host *host)
+static void mmc_dma_cleanup(struct mmc_omap_host *host, int errno)
 {
-	host->data->error = -ETIMEDOUT;
+	host->data->error = errno;
 
 	if (host->use_dma && host->dma_ch != -1) {
 		dma_unmap_sg(mmc_dev(host->mmc), host->data->sg, host->dma_len,
@@ -465,7 +465,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 				end_cmd = 1;
 			}
 			if (host->data) {
-				mmc_dma_cleanup(host);
+				mmc_dma_cleanup(host, -ETIMEDOUT);
 				OMAP_HSMMC_WRITE(host->base, SYSCTL,
 					OMAP_HSMMC_READ(host->base,
 							SYSCTL) | SRD);
@@ -474,13 +474,12 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 					;
 			}
 		}
-		if ((status & DATA_TIMEOUT) ||
-			(status & DATA_CRC)) {
+		if ((status & DATA_TIMEOUT) || (status & DATA_CRC)) {
 			if (host->data) {
 				if (status & DATA_TIMEOUT)
-					mmc_dma_cleanup(host);
+					mmc_dma_cleanup(host, -ETIMEDOUT);
 				else
-					host->data->error = -EILSEQ;
+					mmc_dma_cleanup(host, -EILSEQ);
 				OMAP_HSMMC_WRITE(host->base, SYSCTL,
 					OMAP_HSMMC_READ(host->base,
 							SYSCTL) | SRD);
-- 
1.5.4.3

[PATCH 2/2] OMAP: HSMMC: Fix response type for busy after response

[Adrian Hunter]

>From 410bc62034c021f8767c8dae469c3215783992ca Mon Sep 17 00:00:00 2001
From: Adrian Hunter <ext-adrian.hunter@nokia.com>
Date: Mon, 12 Jan 2009 16:13:08 +0200
Subject: [PATCH] OMAP: HSMMC: Fix response type for busy after response

Some MMC commands result in the card becoming busy after
the response is received.  This needs to be specified
for the omap_hsmmc host controller, which is what this
patch does.  However, the effect is that some commands
with no data will cause a Transfer Complete (TC) interrupt
in addition to the Command Complete (CC) interrupt.
In order to deal with that, the irq handler has needed
a few changes also.

The benefit of this change is that the omap_hsmmc host
controller driver now waits for the TC interrupt while
the card is busy, so the mmc_block driver needs to poll
the card status just once instead of repeatedly.
i.e. the net result is more sleep and less cpu.

This also fixes the hidden problem that the controller
was turning off the clock to the card while the card
was busy - the clock was then switched on for the
duration of each Send Status command used for polling,
which is why writes did not lock up altogether.
i.e. really dysfunctional.

The command sequence for open-ended multi-block write
with DMA is now:

	Issue write command CMD25
	Receive CC interrupt
	Data is sent
	Receive TC interrupt (DMA is done)
	Issue stop command CMD12
	Receive CC interrupt
	Card is busy
	Receive TC interrupt
	Card is now ready for next transfer

Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   42 +++++++++++++++++++++++++++++++---------
 1 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 115d114..1a27e5b 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -151,6 +151,7 @@ struct mmc_omap_host {
 	int			initstr;
 	int			slot_id;
 	int			dbclk_enabled;
+	int			response_busy;
 
 	struct timer_list       idle_timer;
 	spinlock_t		clk_lock;     /* for changing enabled state */
@@ -288,10 +289,14 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd,
 	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
 	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
 
+	host->response_busy = 0;
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136)
 			resptype = 1;
-		else
+		else if (cmd->flags & MMC_RSP_BUSY) {
+			resptype = 3;
+			host->response_busy = 1;
+		} else
 			resptype = 2;
 	}
 
@@ -326,6 +331,15 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd,
 static void
 mmc_omap_xfer_done(struct mmc_omap_host *host, struct mmc_data *data)
 {
+	if (!data) {
+		struct mmc_request *mrq = host->mrq;
+
+		host->mrq = NULL;
+		mmc_omap_fclk_lazy_disable(host);
+		mmc_request_done(host->mmc, mrq);
+		return;
+	}
+
 	host->data = NULL;
 
 	if (host->use_dma && host->dma_ch != -1)
@@ -368,7 +382,7 @@ mmc_omap_cmd_done(struct mmc_omap_host *host, struct mmc_command *cmd)
 			cmd->resp[0] = OMAP_HSMMC_READ(host->base, RSP10);
 		}
 	}
-	if (host->data == NULL || cmd->error) {
+	if ((host->data == NULL && !host->response_busy) || cmd->error) {
 		host->mrq = NULL;
 		mmc_omap_fclk_lazy_disable(host);
 		mmc_request_done(host->mmc, cmd->mrq);
@@ -433,7 +447,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 	struct mmc_data *data;
 	int end_cmd = 0, end_trans = 0, status;
 
-	if (host->cmd == NULL && host->data == NULL) {
+	if (host->mrq == NULL) {
 		OMAP_HSMMC_WRITE(host->base, STAT,
 			OMAP_HSMMC_READ(host->base, STAT));
 		return IRQ_HANDLED;
@@ -464,8 +478,11 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 				}
 				end_cmd = 1;
 			}
-			if (host->data) {
-				mmc_dma_cleanup(host, -ETIMEDOUT);
+			if (host->data || host->response_busy) {
+				if (host->data)
+					mmc_dma_cleanup(host, -ETIMEDOUT);
+				host->response_busy = 0;
+
 				OMAP_HSMMC_WRITE(host->base, SYSCTL,
 					OMAP_HSMMC_READ(host->base,
 							SYSCTL) | SRD);
@@ -475,11 +492,16 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 			}
 		}
 		if ((status & DATA_TIMEOUT) || (status & DATA_CRC)) {
-			if (host->data) {
-				if (status & DATA_TIMEOUT)
-					mmc_dma_cleanup(host, -ETIMEDOUT);
-				else
-					mmc_dma_cleanup(host, -EILSEQ);
+			if (host->data || host->response_busy) {
+				if (host->data) {
+					if (status & DATA_TIMEOUT)
+						mmc_dma_cleanup(host,
+								-ETIMEDOUT);
+					else
+						mmc_dma_cleanup(host, -EILSEQ);
+				}
+				host->response_busy = 0;
+
 				OMAP_HSMMC_WRITE(host->base, SYSCTL,
 					OMAP_HSMMC_READ(host->base,
 							SYSCTL) | SRD);
-- 
1.5.4.3

Re: [PATCH 2/2] OMAP: HSMMC: Fix response type for busy after response

[Pierre Ossman]

On Tue, 13 Jan 2009 15:38:41 +0200
Adrian Hunter <ext-adrian.hunter@nokia.com> wrote:

> This also fixes the hidden problem that the controller
> was turning off the clock to the card while the card
> was busy - the clock was then switched on for the
> duration of each Send Status command used for polling,
> which is why writes did not lock up altogether.
> i.e. really dysfunctional.

Does the driver only keep the clock running as long as there is an
ongoing request? That kind of policy should really be handled in the
core where we have more information and can control it better. E.g. some
cards need the clock to finish up internal housekeeping so we need to
delay things for those.

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.

Re: [PATCH 2/2] OMAP: HSMMC: Fix response type for busy after response

[Adrian Hunter]

Pierre Ossman wrote:
> On Tue, 13 Jan 2009 15:38:41 +0200
> Adrian Hunter <ext-adrian.hunter@nokia.com> wrote:
> 
>> This also fixes the hidden problem that the controller
>> was turning off the clock to the card while the card
>> was busy - the clock was then switched on for the
>> duration of each Send Status command used for polling,
>> which is why writes did not lock up altogether.
>> i.e. really dysfunctional.
> 
> Does the driver only keep the clock running as long as there is an
> ongoing request? That kind of policy should really be handled in the
> core where we have more information and can control it better. E.g. some
> cards need the clock to finish up internal housekeeping so we need to
> delay things for those.
> 
> Rgds

Unfortunately it seems to be something the controller does by itself,
beyond the control of the driver.  Added Jarkko Lavinen and 
Madhusudhan Chikkature to CC, maybe they can correct me.

If it obeys the standard, it should give an additional 8 clock cycles.

Re: [PATCH 2/2] OMAP: HSMMC: Fix response type for busy after response

[Adrian Hunter]

Adrian Hunter wrote:
>>From 410bc62034c021f8767c8dae469c3215783992ca Mon Sep 17 00:00:00 2001
> From: Adrian Hunter <ext-adrian.hunter@nokia.com>
> Date: Mon, 12 Jan 2009 16:13:08 +0200
> Subject: [PATCH] OMAP: HSMMC: Fix response type for busy after response
> 
> Some MMC commands result in the card becoming busy after
> the response is received.  This needs to be specified
> for the omap_hsmmc host controller, which is what this
> patch does.  However, the effect is that some commands
> with no data will cause a Transfer Complete (TC) interrupt
> in addition to the Command Complete (CC) interrupt.
> In order to deal with that, the irq handler has needed
> a few changes also.
> 
> The benefit of this change is that the omap_hsmmc host
> controller driver now waits for the TC interrupt while
> the card is busy, so the mmc_block driver needs to poll
> the card status just once instead of repeatedly.
> i.e. the net result is more sleep and less cpu.
> 
> This also fixes the hidden problem that the controller
> was turning off the clock to the card while the card
> was busy - the clock was then switched on for the
> duration of each Send Status command used for polling,
> which is why writes did not lock up altogether.
> i.e. really dysfunctional.

That paragraph is not true.  According to the standard:

"The host is allowed to shut down the clock of a “busy” card. The card will complete the programming
operation regardless of the host clock. However, the host must provide a clock edge for the card to turn
off its busy signal. Without a clock edge the card (unless previously disconnected by a deselect command
-CMD7) will force the DAT0 line down, forever."

i.e. during busy state the clock is only needed to check whether the busy state is over.

I will resend the patch, without that paragraph and with a fix to report the
error code when there is a timeout error in the busy state.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] OMAP: HSMMC: Fix response type for busy after response

[Adrian Hunter]

>From f1038d1f1b3360130b41ffe77c568d76bc4102e9 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <ext-adrian.hunter@nokia.com>
Date: Mon, 12 Jan 2009 16:13:08 +0200
Subject: [PATCH] OMAP: HSMMC: Fix response type for busy after response

Some MMC commands result in the card becoming busy after
the response is received.  This needs to be specified
for the omap_hsmmc host controller, which is what this
patch does.  However, the effect is that some commands
with no data will cause a Transfer Complete (TC) interrupt
in addition to the Command Complete (CC) interrupt.
In order to deal with that, the irq handler has needed
a few changes also.

The benefit of this change is that the omap_hsmmc host
controller driver now waits for the TC interrupt while
the card is busy, so the mmc_block driver needs to poll
the card status just once instead of repeatedly.
i.e. the net result is more sleep and less cpu.

The command sequence for open-ended multi-block write
with DMA is now:

	Issue write command CMD25
	Receive CC interrupt
	Data is sent
	Receive TC interrupt (DMA is done)
	Issue stop command CMD12
	Receive CC interrupt
	Card is busy
	Receive TC interrupt
	Card is now ready for next transfer

Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   40 +++++++++++++++++++++++++++++++---------
 1 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 115d114..3ce2a38 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -151,6 +151,7 @@ struct mmc_omap_host {
 	int			initstr;
 	int			slot_id;
 	int			dbclk_enabled;
+	int			response_busy;
 
 	struct timer_list       idle_timer;
 	spinlock_t		clk_lock;     /* for changing enabled state */
@@ -288,10 +289,14 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd,
 	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
 	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
 
+	host->response_busy = 0;
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136)
 			resptype = 1;
-		else
+		else if (cmd->flags & MMC_RSP_BUSY) {
+			resptype = 3;
+			host->response_busy = 1;
+		} else
 			resptype = 2;
 	}
 
@@ -326,6 +331,15 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd,
 static void
 mmc_omap_xfer_done(struct mmc_omap_host *host, struct mmc_data *data)
 {
+	if (!data) {
+		struct mmc_request *mrq = host->mrq;
+
+		host->mrq = NULL;
+		mmc_omap_fclk_lazy_disable(host);
+		mmc_request_done(host->mmc, mrq);
+		return;
+	}
+
 	host->data = NULL;
 
 	if (host->use_dma && host->dma_ch != -1)
@@ -368,7 +382,7 @@ mmc_omap_cmd_done(struct mmc_omap_host *host, struct mmc_command *cmd)
 			cmd->resp[0] = OMAP_HSMMC_READ(host->base, RSP10);
 		}
 	}
-	if (host->data == NULL || cmd->error) {
+	if ((host->data == NULL && !host->response_busy) || cmd->error) {
 		host->mrq = NULL;
 		mmc_omap_fclk_lazy_disable(host);
 		mmc_request_done(host->mmc, cmd->mrq);
@@ -433,7 +447,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 	struct mmc_data *data;
 	int end_cmd = 0, end_trans = 0, status;
 
-	if (host->cmd == NULL && host->data == NULL) {
+	if (host->mrq == NULL) {
 		OMAP_HSMMC_WRITE(host->base, STAT,
 			OMAP_HSMMC_READ(host->base, STAT));
 		return IRQ_HANDLED;
@@ -464,8 +478,11 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 				}
 				end_cmd = 1;
 			}
-			if (host->data) {
-				mmc_dma_cleanup(host, -ETIMEDOUT);
+			if (host->data || host->response_busy) {
+				if (host->data)
+					mmc_dma_cleanup(host, -ETIMEDOUT);
+				host->response_busy = 0;
+
 				OMAP_HSMMC_WRITE(host->base, SYSCTL,
 					OMAP_HSMMC_READ(host->base,
 							SYSCTL) | SRD);
@@ -475,11 +492,16 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
 			}
 		}
 		if ((status & DATA_TIMEOUT) || (status & DATA_CRC)) {
-			if (host->data) {
-				if (status & DATA_TIMEOUT)
-					mmc_dma_cleanup(host, -ETIMEDOUT);
+			if (host->data || host->response_busy) {
+				int err = (status & DATA_TIMEOUT) ?
+						-ETIMEDOUT : -EILSEQ;
+
+				if (host->data)
+					mmc_dma_cleanup(host, err);
 				else
-					mmc_dma_cleanup(host, -EILSEQ);
+					host->mrq->cmd->error = err;
+				host->response_busy = 0;
+
 				OMAP_HSMMC_WRITE(host->base, SYSCTL,
 					OMAP_HSMMC_READ(host->base,
 							SYSCTL) | SRD);
-- 
1.5.4.3