linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] davinci-mmc: Use IRQ to detect "card not busy" status change
@ 2014-08-29  6:41 Mike Looijmans
  2014-08-29  6:43 ` Mike Looijmans
  0 siblings, 1 reply; 2+ messages in thread
From: Mike Looijmans @ 2014-08-29  6:41 UTC (permalink / raw)
  To: linux-mmc; +Cc: cjb, ulf.hansson, linux-kernel, Mike Looijmans

The davinci-mmc driver uses a busy wait loop to wait for the card to
become ready (BUSY signal). The MMC card uses this to signal the
controller that it's busy writing and cannot handle new requests yet.
This loop often takes 100k cycles and 10ms to complete.

The controller can raise an interrupt when the BUSY signal is deasserted,
so the routine has been adapted to trigger an interrupt and wait for it
using a wait_event_timeout instead of a busy wait loop.

This reduces the CPU usage of the mmcqd process drastically, from 35% to
2% when writing to SD. No noticable change in performance or reliability.

Tested on a custom board with an OMAP-L138 CPU. Patch originally applied
to a 2.6.37 kernel and ported to 3.14.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 drivers/mmc/host/davinci_mmc.c |   52 +++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index d615374..50458f4 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -223,6 +223,7 @@ struct mmc_davinci_host {
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 #endif
+	wait_queue_head_t 	wait_while_busy_queue
 };
 
 static irqreturn_t mmc_davinci_irq(int irq, void *dev_id);
@@ -628,23 +629,28 @@ mmc_davinci_prepare_data(struct mmc_davinci_host *host, struct mmc_request *req)
 static void mmc_davinci_request(struct mmc_host *mmc, struct mmc_request *req)
 {
 	struct mmc_davinci_host *host = mmc_priv(mmc);
-	unsigned long timeout = jiffies + msecs_to_jiffies(900);
-	u32 mmcst1 = 0;
+	unsigned int remaining;
 
 	/* Card may still be sending BUSY after a previous operation,
 	 * typically some kind of write.  If so, we can't proceed yet.
 	 */
-	while (time_before(jiffies, timeout)) {
-		mmcst1  = readl(host->base + DAVINCI_MMCST1);
-		if (!(mmcst1 & MMCST1_BUSY))
-			break;
-		cpu_relax();
-	}
-	if (mmcst1 & MMCST1_BUSY) {
-		dev_err(mmc_dev(host->mmc), "still BUSY? bad ... \n");
-		req->cmd->error = -ETIMEDOUT;
-		mmc_request_done(mmc, req);
-		return;
+	if (readl(host->base + DAVINCI_MMCST1) & MMCST1_BUSY) {
+		/* Arm BSYDNE interrupt and wait for it */
+		writel(MMCST0_BSYDNE, host->base + DAVINCI_MMCIM);
+		remaining = wait_event_timeout(
+				host->wait_while_busy_queue,
+				((readl(host->base + DAVINCI_MMCST1) & MMCST1_BUSY) == 0),
+				msecs_to_jiffies(900));
+		writel(0, host->base + DAVINCI_MMCIM);
+		if (unlikely(remaining == 0))
+		{
+			if (readl(host->base + DAVINCI_MMCST1) & MMCST1_BUSY) {
+				dev_err(mmc_dev(host->mmc), "Timeout waiting for BSYDNE interrupt.\n");
+				req->cmd->error = -ETIMEDOUT;
+				mmc_request_done(mmc, req);
+				return;
+			}
+		}
 	}
 
 	host->do_dma = 0;
@@ -914,18 +920,21 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
 	int end_transfer = 0;
 	struct mmc_data *data = host->data;
 
+	status = readl(host->base + DAVINCI_MMCST0);
+	qstatus = status;
+
 	if (host->cmd == NULL && host->data == NULL) {
-		status = readl(host->base + DAVINCI_MMCST0);
-		dev_dbg(mmc_dev(host->mmc),
-			"Spurious interrupt 0x%04x\n", status);
+		/* We may have been waiting for the busy-done signal */
+		if (qstatus & MMCST0_BSYDNE) {
+			wake_up(&host->wait_while_busy_queue);
+			return IRQ_HANDLED;
+		}
+		dev_dbg(mmc_dev(host->mmc), "Spurious interrupt %#x\n", status);
 		/* Disable the interrupt from mmcsd */
 		writel(0, host->base + DAVINCI_MMCIM);
 		return IRQ_NONE;
 	}
 
-	status = readl(host->base + DAVINCI_MMCST0);
-	qstatus = status;
-
 	/* handle FIFO first when using PIO for data.
 	 * bytes_left will decrease to zero as I/O progress and status will
 	 * read zero over iteration because this controller status
@@ -1044,6 +1053,10 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
 		end_command = (int) host->cmd;
 	}
 
+	if (qstatus & MMCST0_BSYDNE) {
+		wake_up(&host->wait_while_busy_queue);
+	}
+
 	if (end_command)
 		mmc_davinci_cmd_done(host, host->cmd);
 	if (end_transfer)
@@ -1261,6 +1274,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
+	init_waitqueue_head(&host->wait_while_busy_queue);
 
 	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (!r)
-- 
1.7.9.5


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

* Re: [PATCH] davinci-mmc: Use IRQ to detect "card not busy" status change
  2014-08-29  6:41 [PATCH] davinci-mmc: Use IRQ to detect "card not busy" status change Mike Looijmans
@ 2014-08-29  6:43 ` Mike Looijmans
  0 siblings, 0 replies; 2+ messages in thread
From: Mike Looijmans @ 2014-08-29  6:43 UTC (permalink / raw)
  To: linux-mmc; +Cc: cjb, ulf.hansson, linux-kernel

Note: I have not been able to test or even compile this on recent 
kernels. Can somebody verify that this does not kill the SD driver?

Mike.

On 08/29/2014 08:41 AM, Mike Looijmans wrote:
> The davinci-mmc driver uses a busy wait loop to wait for the card to
> become ready (BUSY signal). The MMC card uses this to signal the
> controller that it's busy writing and cannot handle new requests yet.
> This loop often takes 100k cycles and 10ms to complete.
>
> The controller can raise an interrupt when the BUSY signal is deasserted,
> so the routine has been adapted to trigger an interrupt and wait for it
> using a wait_event_timeout instead of a busy wait loop.
>
> This reduces the CPU usage of the mmcqd process drastically, from 35% to
> 2% when writing to SD. No noticable change in performance or reliability.
>
> Tested on a custom board with an OMAP-L138 CPU. Patch originally applied
> to a 2.6.37 kernel and ported to 3.14.
>
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
>   drivers/mmc/host/davinci_mmc.c |   52 +++++++++++++++++++++++++---------------
>   1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index d615374..50458f4 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -223,6 +223,7 @@ struct mmc_davinci_host {
>   #ifdef CONFIG_CPU_FREQ
>   	struct notifier_block	freq_transition;
>   #endif
> +	wait_queue_head_t 	wait_while_busy_queue
>   };
>
>   static irqreturn_t mmc_davinci_irq(int irq, void *dev_id);
> @@ -628,23 +629,28 @@ mmc_davinci_prepare_data(struct mmc_davinci_host *host, struct mmc_request *req)
>   static void mmc_davinci_request(struct mmc_host *mmc, struct mmc_request *req)
>   {
>   	struct mmc_davinci_host *host = mmc_priv(mmc);
> -	unsigned long timeout = jiffies + msecs_to_jiffies(900);
> -	u32 mmcst1 = 0;
> +	unsigned int remaining;
>
>   	/* Card may still be sending BUSY after a previous operation,
>   	 * typically some kind of write.  If so, we can't proceed yet.
>   	 */
> -	while (time_before(jiffies, timeout)) {
> -		mmcst1  = readl(host->base + DAVINCI_MMCST1);
> -		if (!(mmcst1 & MMCST1_BUSY))
> -			break;
> -		cpu_relax();
> -	}
> -	if (mmcst1 & MMCST1_BUSY) {
> -		dev_err(mmc_dev(host->mmc), "still BUSY? bad ... \n");
> -		req->cmd->error = -ETIMEDOUT;
> -		mmc_request_done(mmc, req);
> -		return;
> +	if (readl(host->base + DAVINCI_MMCST1) & MMCST1_BUSY) {
> +		/* Arm BSYDNE interrupt and wait for it */
> +		writel(MMCST0_BSYDNE, host->base + DAVINCI_MMCIM);
> +		remaining = wait_event_timeout(
> +				host->wait_while_busy_queue,
> +				((readl(host->base + DAVINCI_MMCST1) & MMCST1_BUSY) == 0),
> +				msecs_to_jiffies(900));
> +		writel(0, host->base + DAVINCI_MMCIM);
> +		if (unlikely(remaining == 0))
> +		{
> +			if (readl(host->base + DAVINCI_MMCST1) & MMCST1_BUSY) {
> +				dev_err(mmc_dev(host->mmc), "Timeout waiting for BSYDNE interrupt.\n");
> +				req->cmd->error = -ETIMEDOUT;
> +				mmc_request_done(mmc, req);
> +				return;
> +			}
> +		}
>   	}
>
>   	host->do_dma = 0;
> @@ -914,18 +920,21 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
>   	int end_transfer = 0;
>   	struct mmc_data *data = host->data;
>
> +	status = readl(host->base + DAVINCI_MMCST0);
> +	qstatus = status;
> +
>   	if (host->cmd == NULL && host->data == NULL) {
> -		status = readl(host->base + DAVINCI_MMCST0);
> -		dev_dbg(mmc_dev(host->mmc),
> -			"Spurious interrupt 0x%04x\n", status);
> +		/* We may have been waiting for the busy-done signal */
> +		if (qstatus & MMCST0_BSYDNE) {
> +			wake_up(&host->wait_while_busy_queue);
> +			return IRQ_HANDLED;
> +		}
> +		dev_dbg(mmc_dev(host->mmc), "Spurious interrupt %#x\n", status);
>   		/* Disable the interrupt from mmcsd */
>   		writel(0, host->base + DAVINCI_MMCIM);
>   		return IRQ_NONE;
>   	}
>
> -	status = readl(host->base + DAVINCI_MMCST0);
> -	qstatus = status;
> -
>   	/* handle FIFO first when using PIO for data.
>   	 * bytes_left will decrease to zero as I/O progress and status will
>   	 * read zero over iteration because this controller status
> @@ -1044,6 +1053,10 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
>   		end_command = (int) host->cmd;
>   	}
>
> +	if (qstatus & MMCST0_BSYDNE) {
> +		wake_up(&host->wait_while_busy_queue);
> +	}
> +
>   	if (end_command)
>   		mmc_davinci_cmd_done(host, host->cmd);
>   	if (end_transfer)
> @@ -1261,6 +1274,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
>
>   	host = mmc_priv(mmc);
>   	host->mmc = mmc;	/* Important */
> +	init_waitqueue_head(&host->wait_while_busy_queue);
>
>   	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
>   	if (!r)
>


-- 
Mike Looijmans

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

end of thread, other threads:[~2014-08-29  6:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-29  6:41 [PATCH] davinci-mmc: Use IRQ to detect "card not busy" status change Mike Looijmans
2014-08-29  6:43 ` Mike Looijmans

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