[PATCH 00/14] mmc: sh-mmcif: 3.8 fixes and improvements

[PATCH 00/14] mmc: sh-mmcif: 3.8 fixes and improvements

[Guennadi Liakhovetski]

This patch series fixes several bugs in the sh_mmcif.c driver, that showed 
up on SMP with DMA (patches 10, 13), adds new features (3, 4) and adds 
multiple further improvements and minor fixes. Even though these bugs 
currently exist in the mainline, this patch series is not considered to 
be suitable for @stable. Patches are rather experimental and it is better 
to test them over a complete development cycle. They also are easier 
implemented on top of preparatory cosmetic patches.

Guennadi Liakhovetski (6):
  mmc: sh-mmcif: fix missing and consolidate IO completion timeouts
  mmc: sh-mmcif: (cosmetic) simplify boolean return blocks
  mmc: sh-mmcif: fix a race, causing an Oops on SMP
  mmc: sh-mmcif: reset error code for any opcode
  mmc: sh-mmcif: reset DMA completion immediately before starting DMA
  mmc: sh-mmcif: fix I/O errors

Shinya Kuribayashi (1):
  mmc: sh_mmcif: add support for bundled MMCIF IRQs

Teppei Kamijou (7):
  mmc: sh_mmcif: force to fail CMD52 immediately
  mmc: sh_mmcif: ensure run-time suspend call is processed before
    suspend
  mmc: sh_mmcif: Add support for eMMC Dual Data Rate
  mmc: sh_mmcif: Use msecs_to_jiffies() for host->timeout
  mmc: sh_mmcif: Avoid unnecessary mmc_delay() at mmc_card_sleepawake()
  mmc: sh-mmcif: Terminate DMA transactions when detecting timeout or
    error
  mmc: sh-mmcif: report all errors

 drivers/mmc/host/sh_mmcif.c |  215 +++++++++++++++++++++++++++++--------------
 1 files changed, 145 insertions(+), 70 deletions(-)

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 01/14] mmc: sh_mmcif: force to fail CMD52 immediately

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

mmc_rescan() sends CMD52 (SD_IO_RW_DIRECT) to reset SDIO card during
card detection. CMD52 should be ignored by SD/eMMC cards, but we can
also abort it in the driver immediately, since MMCIF doesn't support
SDIO cards anyway.

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 9a4c151..543f6ce 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -911,6 +911,7 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		if ((mrq->cmd->flags & MMC_CMD_MASK) != MMC_CMD_BCR)
 			break;
 	case MMC_APP_CMD:
+	case SD_IO_RW_DIRECT:
 		host->state = STATE_IDLE;
 		mrq->cmd->error = -ETIMEDOUT;
 		mmc_request_done(mmc, mrq);
-- 
1.7.2.5

[PATCH 02/14] mmc: sh_mmcif: ensure run-time suspend call is processed before suspend

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

With this post-v2.6.35 change applied:

commit a0a1a5fd4fb15ec61117c759fe9f5c16c53d9e9c
Author: Tejun Heo <tj@kernel.org>
Date:   Tue Jun 29 10:07:12 2010 +0200
    workqueue: reimplement workqueue freeze using max_active

freeze_workqueues_begin() was introduced and workqueue now gets frozen
before device drivers suspend operations.

We have to ensure that run-time PM suspend operation completes before
system-wide suspend is started.

igned-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
igned-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 543f6ce..8a188fe 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -982,7 +982,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			}
 		}
 		if (host->power) {
-			pm_runtime_put(&host->pd->dev);
+			pm_runtime_put_sync(&host->pd->dev);
 			clk_disable(host->hclk);
 			host->power = false;
 			if (ios->power_mode == MMC_POWER_OFF)
-- 
1.7.2.5

[PATCH 03/14] mmc: sh_mmcif: add support for bundled MMCIF IRQs

[Guennadi Liakhovetski]

From: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>

On newer SoCs like R-Mobile U2, MMCIF interrupts are bundled.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 8a188fe..d80c294 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1307,10 +1307,11 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	struct sh_mmcif_plat_data *pd = pdev->dev.platform_data;
 	struct resource *res;
 	void __iomem *reg;
+	const char *name;
 
 	irq[0] = platform_get_irq(pdev, 0);
 	irq[1] = platform_get_irq(pdev, 1);
-	if (irq[0] < 0 || irq[1] < 0) {
+	if (irq[0] < 0) {
 		dev_err(&pdev->dev, "Get irq error\n");
 		return -ENXIO;
 	}
@@ -1375,15 +1376,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
-	ret = request_threaded_irq(irq[0], sh_mmcif_intr, sh_mmcif_irqt, 0, "sh_mmc:error", host);
+	name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
+	ret = request_threaded_irq(irq[0], sh_mmcif_intr, sh_mmcif_irqt, 0, name, host);
 	if (ret) {
-		dev_err(&pdev->dev, "request_irq error (sh_mmc:error)\n");
+		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
 		goto ereqirq0;
 	}
-	ret = request_threaded_irq(irq[1], sh_mmcif_intr, sh_mmcif_irqt, 0, "sh_mmc:int", host);
-	if (ret) {
-		dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
-		goto ereqirq1;
+	if (irq[1] >= 0) {
+		ret = request_threaded_irq(irq[1], sh_mmcif_intr, sh_mmcif_irqt,
+					   0, "sh_mmc:int", host);
+		if (ret) {
+			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
+			goto ereqirq1;
+		}
 	}
 
 	if (pd && pd->use_cd_gpio) {
@@ -1408,7 +1413,8 @@ emmcaddh:
 	if (pd && pd->use_cd_gpio)
 		mmc_gpio_free_cd(mmc);
 erqcd:
-	free_irq(irq[1], host);
+	if (irq[1] >= 0)
+		free_irq(irq[1], host);
 ereqirq1:
 	free_irq(irq[0], host);
 ereqirq0:
@@ -1457,7 +1463,8 @@ static int sh_mmcif_remove(struct platform_device *pdev)
 	irq[1] = platform_get_irq(pdev, 1);
 
 	free_irq(irq[0], host);
-	free_irq(irq[1], host);
+	if (irq[1] >= 0)
+		free_irq(irq[1], host);
 
 	platform_set_drvdata(pdev, NULL);
 
-- 
1.7.2.5

[PATCH 04/14] mmc: sh_mmcif: Add support for eMMC Dual Data Rate

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

Some MMCIF implementations support the Dual Data Rate. With this patch,
platforms can set the MMC_CAP_UHS_DDR50 capability flag in MMCIF platform
data. This will let the MMC core to actually use the DDR mode.

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index d80c294..1ec3339 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -88,6 +88,7 @@
 #define CMD_SET_TBIT		(1 << 7) /* 1: tran mission bit "Low" */
 #define CMD_SET_OPDM		(1 << 6) /* 1: open/drain */
 #define CMD_SET_CCSH		(1 << 5)
+#define CMD_SET_DARS		(1 << 2) /* Dual Data Rate */
 #define CMD_SET_DATW_1		((0 << 1) | (0 << 0)) /* 1bit */
 #define CMD_SET_DATW_4		((0 << 1) | (1 << 0)) /* 4bit */
 #define CMD_SET_DATW_8		((1 << 1) | (0 << 0)) /* 8bit */
@@ -216,6 +217,7 @@ struct sh_mmcif_host {
 	struct clk *hclk;
 	unsigned int clk;
 	int bus_width;
+	unsigned char timing;
 	bool sd_error;
 	bool dying;
 	long timeout;
@@ -781,6 +783,17 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 			dev_err(&host->pd->dev, "Unsupported bus width.\n");
 			break;
 		}
+		switch (host->timing) {
+		case MMC_TIMING_UHS_DDR50:
+			/*
+			 * MMC core will only set this timing, if the host
+			 * advertises the MMC_CAP_UHS_DDR50 capability. MMCIF
+			 * implementations with this capability, e.g. sh73a0,
+			 * will have to set it in their platform data.
+			 */
+			tmp |= CMD_SET_DARS;
+			break;
+		}
 	}
 	/* DWEN */
 	if (opc == MMC_WRITE_BLOCK || opc == MMC_WRITE_MULTIPLE_BLOCK)
@@ -1002,6 +1015,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		sh_mmcif_clock_control(host, ios->clock);
 	}
 
+	host->timing = ios->timing;
 	host->bus_width = ios->bus_width;
 	host->state = STATE_IDLE;
 }
-- 
1.7.2.5

[PATCH 05/14] mmc: sh_mmcif: Use msecs_to_jiffies() for host->timeout

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

Timeout period should be properly normalized using msecs_to_jiffies().

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 1ec3339..1665ae4 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1348,7 +1348,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
 	host->addr	= reg;
-	host->timeout	= 1000;
+	host->timeout	= msecs_to_jiffies(1000);
 
 	host->pd = pdev;
 
-- 
1.7.2.5

[PATCH 06/14] mmc: sh_mmcif: Avoid unnecessary mmc_delay() at mmc_card_sleepawake()

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

SH/R-Mobile MMCIF host controller can wait while the card signals busy.
Set MMC_CAP_WAIT_WHILE_BUSY to inform an upper layer (core/mmc_ops.c)
not to insert unnecessary mmc_delay().

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 1665ae4..092c415 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -758,6 +758,7 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 	}
 	switch (opc) {
 	/* RBSY */
+	case MMC_SLEEP_AWAKE:
 	case MMC_SWITCH:
 	case MMC_STOP_TRANSMISSION:
 	case MMC_SET_WRITE_PROT:
@@ -851,6 +852,7 @@ static void sh_mmcif_start_cmd(struct sh_mmcif_host *host,
 
 	switch (opc) {
 	/* response busy check */
+	case MMC_SLEEP_AWAKE:
 	case MMC_SWITCH:
 	case MMC_STOP_TRANSMISSION:
 	case MMC_SET_WRITE_PROT:
@@ -1357,7 +1359,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	mmc->ops = &sh_mmcif_ops;
 	sh_mmcif_init_ocr(host);
 
-	mmc->caps = MMC_CAP_MMC_HIGHSPEED;
+	mmc->caps = MMC_CAP_MMC_HIGHSPEED | MMC_CAP_WAIT_WHILE_BUSY;
 	if (pd && pd->caps)
 		mmc->caps |= pd->caps;
 	mmc->max_segs = 32;
-- 
1.7.2.5

[PATCH 07/14] mmc: sh-mmcif: fix missing and consolidate IO completion timeouts

[Guennadi Liakhovetski]

Read block and write block operations are currently missing completion
timeouts. Add missing timeouts and consolidate them at one location.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |   45 +++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 092c415..ac4c3c3 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -564,7 +564,6 @@ static void sh_mmcif_single_read(struct sh_mmcif_host *host,
 			   BLOCK_SIZE_MASK) + 3;
 
 	host->wait_for = MMCIF_WAIT_FOR_READ;
-	schedule_delayed_work(&host->timeout_work, host->timeout);
 
 	/* buf read enable */
 	sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFREN);
@@ -606,7 +605,7 @@ static void sh_mmcif_multi_read(struct sh_mmcif_host *host,
 	host->sg_idx = 0;
 	host->sg_blkidx = 0;
 	host->pio_ptr = sg_virt(data->sg);
-	schedule_delayed_work(&host->timeout_work, host->timeout);
+
 	sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFREN);
 }
 
@@ -629,7 +628,6 @@ static bool sh_mmcif_mread_block(struct sh_mmcif_host *host)
 	if (!sh_mmcif_next_block(host, p))
 		return false;
 
-	schedule_delayed_work(&host->timeout_work, host->timeout);
 	sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFREN);
 
 	return true;
@@ -642,7 +640,6 @@ static void sh_mmcif_single_write(struct sh_mmcif_host *host,
 			   BLOCK_SIZE_MASK) + 3;
 
 	host->wait_for = MMCIF_WAIT_FOR_WRITE;
-	schedule_delayed_work(&host->timeout_work, host->timeout);
 
 	/* buf write enable */
 	sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFWEN);
@@ -684,7 +681,7 @@ static void sh_mmcif_multi_write(struct sh_mmcif_host *host,
 	host->sg_idx = 0;
 	host->sg_blkidx = 0;
 	host->pio_ptr = sg_virt(data->sg);
-	schedule_delayed_work(&host->timeout_work, host->timeout);
+
 	sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFWEN);
 }
 
@@ -707,7 +704,6 @@ static bool sh_mmcif_mwrite_block(struct sh_mmcif_host *host)
 	if (!sh_mmcif_next_block(host, p))
 		return false;
 
-	schedule_delayed_work(&host->timeout_work, host->timeout);
 	sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MBUFWEN);
 
 	return true;
@@ -900,7 +896,6 @@ static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 	}
 
 	host->wait_for = MMCIF_WAIT_FOR_STOP;
-	schedule_delayed_work(&host->timeout_work, host->timeout);
 }
 
 static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
@@ -1121,6 +1116,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
 	struct mmc_request *mrq = host->mrq;
+	bool wait = false;
 
 	cancel_delayed_work_sync(&host->timeout_work);
 
@@ -1133,29 +1129,24 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 		/* We're too late, the timeout has already kicked in */
 		return IRQ_HANDLED;
 	case MMCIF_WAIT_FOR_CMD:
-		if (sh_mmcif_end_cmd(host))
-			/* Wait for data */
-			return IRQ_HANDLED;
+		/* Wait for data? */
+		wait = sh_mmcif_end_cmd(host);
 		break;
 	case MMCIF_WAIT_FOR_MREAD:
-		if (sh_mmcif_mread_block(host))
-			/* Wait for more data */
-			return IRQ_HANDLED;
+		/* Wait for more data? */
+		wait = sh_mmcif_mread_block(host);
 		break;
 	case MMCIF_WAIT_FOR_READ:
-		if (sh_mmcif_read_block(host))
-			/* Wait for data end */
-			return IRQ_HANDLED;
+		/* Wait for data end? */
+		wait = sh_mmcif_read_block(host);
 		break;
 	case MMCIF_WAIT_FOR_MWRITE:
-		if (sh_mmcif_mwrite_block(host))
-			/* Wait data to write */
-			return IRQ_HANDLED;
+		/* Wait data to write? */
+		wait = sh_mmcif_mwrite_block(host);
 		break;
 	case MMCIF_WAIT_FOR_WRITE:
-		if (sh_mmcif_write_block(host))
-			/* Wait for data end */
-			return IRQ_HANDLED;
+		/* Wait for data end? */
+		wait = sh_mmcif_write_block(host);
 		break;
 	case MMCIF_WAIT_FOR_STOP:
 		if (host->sd_error) {
@@ -1174,6 +1165,12 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 		BUG();
 	}
 
+	if (wait) {
+		schedule_delayed_work(&host->timeout_work, host->timeout);
+		/* Wait for more data */
+		return IRQ_HANDLED;
+	}
+
 	if (host->wait_for != MMCIF_WAIT_FOR_STOP) {
 		struct mmc_data *data = mrq->data;
 		if (!mrq->cmd->error && data && !data->error)
@@ -1182,8 +1179,10 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 
 		if (mrq->stop && !mrq->cmd->error && (!data || !data->error)) {
 			sh_mmcif_stop_cmd(host, mrq);
-			if (!mrq->stop->error)
+			if (!mrq->stop->error) {
+				schedule_delayed_work(&host->timeout_work, host->timeout);
 				return IRQ_HANDLED;
+			}
 		}
 	}
 
-- 
1.7.2.5

[PATCH 08/14] mmc: sh-mmcif: Terminate DMA transactions when detecting timeout or error

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

If a DMA transaction fails, terminate all outstanding DMA transfers and
unmap buffers.

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
[g.liakhovetski@gmx.de: forward-port, add dma_unmap_sg() in error cases]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index ac4c3c3..d8d65c0 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -263,15 +263,6 @@ static void mmcif_dma_complete(void *arg)
 		 dev_name(&host->pd->dev)))
 		return;
 
-	if (data->flags & MMC_DATA_READ)
-		dma_unmap_sg(host->chan_rx->device->dev,
-			     data->sg, data->sg_len,
-			     DMA_FROM_DEVICE);
-	else
-		dma_unmap_sg(host->chan_tx->device->dev,
-			     data->sg, data->sg_len,
-			     DMA_TO_DEVICE);
-
 	complete(&host->dma_complete);
 }
 
@@ -1088,14 +1079,20 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 	/* Running in the IRQ thread, can sleep */
 	time = wait_for_completion_interruptible_timeout(&host->dma_complete,
 							 host->timeout);
+
+	if (data->flags & MMC_DATA_READ)
+		dma_unmap_sg(host->chan_rx->device->dev,
+			     data->sg, data->sg_len,
+			     DMA_FROM_DEVICE);
+	else
+		dma_unmap_sg(host->chan_tx->device->dev,
+			     data->sg, data->sg_len,
+			     DMA_TO_DEVICE);
+
 	if (host->sd_error) {
 		dev_err(host->mmc->parent,
 			"Error IRQ while waiting for DMA completion!\n");
 		/* Woken up by an error IRQ: abort DMA */
-		if (data->flags & MMC_DATA_READ)
-			dmaengine_terminate_all(host->chan_rx);
-		else
-			dmaengine_terminate_all(host->chan_tx);
 		data->error = sh_mmcif_error_manage(host);
 	} else if (!time) {
 		data->error = -ETIMEDOUT;
@@ -1106,8 +1103,14 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 			BUF_ACC_DMAREN | BUF_ACC_DMAWEN);
 	host->dma_active = false;
 
-	if (data->error)
+	if (data->error) {
 		data->bytes_xfered = 0;
+		/* Abort DMA */
+		if (data->flags & MMC_DATA_READ)
+			dmaengine_terminate_all(host->chan_rx);
+		else
+			dmaengine_terminate_all(host->chan_tx);
+	}
 
 	return false;
 }
-- 
1.7.2.5

[PATCH 09/14] mmc: sh-mmcif: (cosmetic) simplify boolean return blocks

[Guennadi Liakhovetski]

Use "return condition" instead of "if (condition) return true; return false"
in functions, returning bool.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index d8d65c0..1a7c602 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -542,10 +542,7 @@ static bool sh_mmcif_next_block(struct sh_mmcif_host *host, u32 *p)
 		host->pio_ptr = p;
 	}
 
-	if (host->sg_idx == data->sg_len)
-		return false;
-
-	return true;
+	return host->sg_idx != data->sg_len;
 }
 
 static void sh_mmcif_single_read(struct sh_mmcif_host *host,
@@ -1071,9 +1068,7 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 
 	if (!host->dma_active) {
 		data->error = sh_mmcif_data_trans(host, host->mrq, cmd->opcode);
-		if (!data->error)
-			return true;
-		return false;
+		return !data->error;
 	}
 
 	/* Running in the IRQ thread, can sleep */
-- 
1.7.2.5

[PATCH 10/14] mmc: sh-mmcif: fix a race, causing an Oops on SMP

[Guennadi Liakhovetski]

Oopses have been observed on SMP in the sh-mmcif IRQ thread, when the two
IRQ threads run simultaneously on two CPUs. Also take care to guard the
timeout work and the DMA completion callback from possible NULL-pointer
dereferences and races.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |   39 ++++++++++++++++++++++++++++++++++++---
 1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 1a7c602..96ab2e1 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -56,6 +56,7 @@
 #include <linux/mmc/sh_mmcif.h>
 #include <linux/mmc/slot-gpio.h>
 #include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
 #include <linux/pagemap.h>
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
@@ -196,6 +197,7 @@ enum mmcif_state {
 	STATE_IDLE,
 	STATE_REQUEST,
 	STATE_IOS,
+	STATE_TIMEOUT,
 };
 
 enum mmcif_wait_for {
@@ -232,6 +234,7 @@ struct sh_mmcif_host {
 	int sg_blkidx;
 	bool power;
 	bool card_present;
+	struct mutex thread_lock;
 
 	/* DMA support */
 	struct dma_chan		*chan_rx;
@@ -255,11 +258,11 @@ static inline void sh_mmcif_bitclr(struct sh_mmcif_host *host,
 static void mmcif_dma_complete(void *arg)
 {
 	struct sh_mmcif_host *host = arg;
-	struct mmc_data *data = host->mrq->data;
+	struct mmc_request *mrq = host->mrq;
 
 	dev_dbg(&host->pd->dev, "Command completed\n");
 
-	if (WARN(!data, "%s: NULL data in DMA completion!\n",
+	if (WARN(!mrq || !mrq->data, "%s: NULL data in DMA completion!\n",
 		 dev_name(&host->pd->dev)))
 		return;
 
@@ -1113,11 +1116,21 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
-	struct mmc_request *mrq = host->mrq;
+	struct mmc_request *mrq;
 	bool wait = false;
 
 	cancel_delayed_work_sync(&host->timeout_work);
 
+	mutex_lock(&host->thread_lock);
+
+	mrq = host->mrq;
+	if (!mrq) {
+		dev_dbg(&host->pd->dev, "IRQ thread state %u, wait %u: NULL mrq!\n",
+			host->state, host->wait_for);
+		mutex_unlock(&host->thread_lock);
+		return IRQ_HANDLED;
+	}
+
 	/*
 	 * All handlers return true, if processing continues, and false, if the
 	 * request has to be completed - successfully or not
@@ -1125,6 +1138,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	switch (host->wait_for) {
 	case MMCIF_WAIT_FOR_REQUEST:
 		/* We're too late, the timeout has already kicked in */
+		mutex_unlock(&host->thread_lock);
 		return IRQ_HANDLED;
 	case MMCIF_WAIT_FOR_CMD:
 		/* Wait for data? */
@@ -1166,6 +1180,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	if (wait) {
 		schedule_delayed_work(&host->timeout_work, host->timeout);
 		/* Wait for more data */
+		mutex_unlock(&host->thread_lock);
 		return IRQ_HANDLED;
 	}
 
@@ -1179,6 +1194,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 			sh_mmcif_stop_cmd(host, mrq);
 			if (!mrq->stop->error) {
 				schedule_delayed_work(&host->timeout_work, host->timeout);
+				mutex_unlock(&host->thread_lock);
 				return IRQ_HANDLED;
 			}
 		}
@@ -1189,6 +1205,8 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	host->mrq = NULL;
 	mmc_request_done(host->mmc, mrq);
 
+	mutex_unlock(&host->thread_lock);
+
 	return IRQ_HANDLED;
 }
 
@@ -1262,11 +1280,24 @@ static void mmcif_timeout_work(struct work_struct *work)
 	struct delayed_work *d = container_of(work, struct delayed_work, work);
 	struct sh_mmcif_host *host = container_of(d, struct sh_mmcif_host, timeout_work);
 	struct mmc_request *mrq = host->mrq;
+	unsigned long flags;
 
 	if (host->dying)
 		/* Don't run after mmc_remove_host() */
 		return;
 
+	dev_dbg(&host->pd->dev, "Timeout waiting for %u, opcode %u\n",
+		host->wait_for, mrq->cmd->opcode);
+
+	spin_lock_irqsave(&host->lock, flags);
+	if (host->state == STATE_IDLE) {
+		spin_unlock_irqrestore(&host->lock, flags);
+		return;
+	}
+
+	host->state = STATE_TIMEOUT;
+	spin_unlock_irqrestore(&host->lock, flags);
+
 	/*
 	 * Handle races with cancel_delayed_work(), unless
 	 * cancel_delayed_work_sync() is used
@@ -1410,6 +1441,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 			goto erqcd;
 	}
 
+	mutex_init(&host->thread_lock);
+
 	clk_disable(host->hclk);
 	ret = mmc_add_host(mmc);
 	if (ret < 0)
-- 
1.7.2.5

[PATCH 11/14] mmc: sh-mmcif: reset error code for any opcode

[Guennadi Liakhovetski]

If a command execution has produced an error, it has to be reset as a part
of the error handling.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 96ab2e1..03493e4 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1041,7 +1041,6 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 		case MMC_SELECT_CARD:
 		case MMC_APP_CMD:
 			cmd->error = -ETIMEDOUT;
-			host->sd_error = false;
 			break;
 		default:
 			cmd->error = sh_mmcif_error_manage(host);
@@ -1049,6 +1048,7 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 				cmd->opcode, cmd->error);
 			break;
 		}
+		host->sd_error = false;
 		return false;
 	}
 	if (!(cmd->flags & MMC_RSP_PRESENT)) {
-- 
1.7.2.5

[PATCH 12/14] mmc: sh-mmcif: reset DMA completion immediately before starting DMA

[Guennadi Liakhovetski]

DMA completion can be signalled from the DMA callback and from the error
handler. If both are called, the completion struct can enter an
inconsistent state. To prevent this move completion initialisation
immediately before activating DMA.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 03493e4..1dd7fc0 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -419,8 +419,6 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 	if (ret < 0)
 		goto ecfgrx;
 
-	init_completion(&host->dma_complete);
-
 	return;
 
 ecfgrx:
@@ -1061,6 +1059,12 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 	if (!data)
 		return false;
 
+	/*
+	 * Completion can be signalled from DMA callback and error, so, have to
+	 * reset here, before setting .dma_active
+	 */
+	init_completion(&host->dma_complete);
+
 	if (data->flags & MMC_DATA_READ) {
 		if (host->chan_rx)
 			sh_mmcif_start_dma_rx(host);
-- 
1.7.2.5

[PATCH 13/14] mmc: sh-mmcif: fix I/O errors

[Guennadi Liakhovetski]

The INT_BUFWEN IRQ often arrives with other bits set too. If they are not
cleared, an additional IRQ can be triggered, sometimes also after the MMC
request has already been completed. This leads to block I/O errors. Earlier
Teppei Kamijou also observed these additional interrupts and proposed to
explicitly wait for them. This patch chooses an alternative approach of
clearing all active bits immediately, when processing the main interrupt.

Reported-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 1dd7fc0..36eee8d 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1238,7 +1238,9 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 		sh_mmcif_writel(host->addr, MMCIF_CE_INT, ~INT_BUFREN);
 		sh_mmcif_bitclr(host, MMCIF_CE_INT_MASK, MASK_MBUFREN);
 	} else if (state & INT_BUFWEN) {
-		sh_mmcif_writel(host->addr, MMCIF_CE_INT, ~INT_BUFWEN);
+		sh_mmcif_writel(host->addr, MMCIF_CE_INT,
+				~(INT_BUFWEN | INT_DTRANE | INT_CMD12DRE |
+				  INT_CMD12RBE | INT_CMD12CRE));
 		sh_mmcif_bitclr(host, MMCIF_CE_INT_MASK, MASK_MBUFWEN);
 	} else if (state & INT_CMD12DRE) {
 		sh_mmcif_writel(host->addr, MMCIF_CE_INT,
-- 
1.7.2.5

[PATCH 14/14] mmc: sh-mmcif: report all errors

[Guennadi Liakhovetski]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

Make error reporting in the driver more verbose. This patch is based on
an earlier work by Teppei Kamijou, but we try to not add any new error
messages to the log in the normal case to avoid confusing the user, and
also add a few more dev_dbg() calls.

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
[g.liakhovetski@gmx.de: avoid producing new errors in normal case]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 36eee8d..7abf009 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -514,13 +514,16 @@ static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 	}
 
 	if (state2 & STS2_CRC_ERR) {
-		dev_dbg(&host->pd->dev, ": CRC error\n");
+		dev_err(&host->pd->dev, " CRC error: state %u, wait %u\n",
+			host->state, host->wait_for);
 		ret = -EIO;
 	} else if (state2 & STS2_TIMEOUT_ERR) {
-		dev_dbg(&host->pd->dev, ": Timeout\n");
+		dev_err(&host->pd->dev, " Timeout: state %u, wait %u\n",
+			host->state, host->wait_for);
 		ret = -ETIMEDOUT;
 	} else {
-		dev_dbg(&host->pd->dev, ": End/Index error\n");
+		dev_dbg(&host->pd->dev, " End/Index error: state %u, wait %u\n",
+			host->state, host->wait_for);
 		ret = -EIO;
 	}
 	return ret;
@@ -566,6 +569,7 @@ static bool sh_mmcif_read_block(struct sh_mmcif_host *host)
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
+		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -606,6 +610,7 @@ static bool sh_mmcif_mread_block(struct sh_mmcif_host *host)
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
+		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -642,6 +647,7 @@ static bool sh_mmcif_write_block(struct sh_mmcif_host *host)
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
+		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -682,6 +688,7 @@ static bool sh_mmcif_mwrite_block(struct sh_mmcif_host *host)
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
+		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -823,7 +830,7 @@ static int sh_mmcif_data_trans(struct sh_mmcif_host *host,
 		sh_mmcif_single_read(host, mrq);
 		return 0;
 	default:
-		dev_err(&host->pd->dev, "UNSUPPORTED CMD = d'%08d\n", opc);
+		dev_err(&host->pd->dev, "Unsupported CMD%d\n", opc);
 		return -EINVAL;
 	}
 }
@@ -894,6 +901,7 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->state != STATE_IDLE) {
+		dev_dbg(&host->pd->dev, "%s() rejected, state %u\n", __func__, host->state);
 		spin_unlock_irqrestore(&host->lock, flags);
 		mrq->cmd->error = -EAGAIN;
 		mmc_request_done(mmc, mrq);
@@ -957,6 +965,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->state != STATE_IDLE) {
+		dev_dbg(&host->pd->dev, "%s() rejected, state %u\n", __func__, host->state);
 		spin_unlock_irqrestore(&host->lock, flags);
 		return;
 	}
@@ -1042,10 +1051,10 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 			break;
 		default:
 			cmd->error = sh_mmcif_error_manage(host);
-			dev_dbg(&host->pd->dev, "Cmd(d'%d) error %d\n",
-				cmd->opcode, cmd->error);
 			break;
 		}
+		dev_dbg(&host->pd->dev, "CMD%d error %d\n",
+			cmd->opcode, cmd->error);
 		host->sd_error = false;
 		return false;
 	}
@@ -1097,8 +1106,11 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 		/* Woken up by an error IRQ: abort DMA */
 		data->error = sh_mmcif_error_manage(host);
 	} else if (!time) {
+		dev_err(host->mmc->parent, "DMA timeout!\n");
 		data->error = -ETIMEDOUT;
 	} else if (time < 0) {
+		dev_err(host->mmc->parent,
+			"wait_for_completion_...() error %ld!\n", time);
 		data->error = time;
 	}
 	sh_mmcif_bitclr(host, MMCIF_CE_BUF_ACC,
@@ -1167,6 +1179,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	case MMCIF_WAIT_FOR_STOP:
 		if (host->sd_error) {
 			mrq->stop->error = sh_mmcif_error_manage(host);
+			dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, mrq->stop->error);
 			break;
 		}
 		sh_mmcif_get_cmd12response(host, mrq->stop);
@@ -1174,8 +1187,10 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 		break;
 	case MMCIF_WAIT_FOR_READ_END:
 	case MMCIF_WAIT_FOR_WRITE_END:
-		if (host->sd_error)
+		if (host->sd_error) {
 			mrq->data->error = sh_mmcif_error_manage(host);
+			dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, mrq->data->error);
+		}
 		break;
 	default:
 		BUG();
@@ -1292,7 +1307,7 @@ static void mmcif_timeout_work(struct work_struct *work)
 		/* Don't run after mmc_remove_host() */
 		return;
 
-	dev_dbg(&host->pd->dev, "Timeout waiting for %u, opcode %u\n",
+	dev_err(&host->pd->dev, "Timeout waiting for %u on CMD%u\n",
 		host->wait_for, mrq->cmd->opcode);
 
 	spin_lock_irqsave(&host->lock, flags);
-- 
1.7.2.5

Re: [PATCH 00/14] mmc: sh-mmcif: 3.8 fixes and improvements

[Chris Ball]

Hi Guennadi,

On Wed, Dec 12 2012, Guennadi Liakhovetski wrote:
> This patch series fixes several bugs in the sh_mmcif.c driver, that showed 
> up on SMP with DMA (patches 10, 13), adds new features (3, 4) and adds 
> multiple further improvements and minor fixes. Even though these bugs 
> currently exist in the mainline, this patch series is not considered to 
> be suitable for @stable. Patches are rather experimental and it is better 
> to test them over a complete development cycle. They also are easier 
> implemented on top of preparatory cosmetic patches.
>
> Guennadi Liakhovetski (6):
>   mmc: sh-mmcif: fix missing and consolidate IO completion timeouts
>   mmc: sh-mmcif: (cosmetic) simplify boolean return blocks
>   mmc: sh-mmcif: fix a race, causing an Oops on SMP
>   mmc: sh-mmcif: reset error code for any opcode
>   mmc: sh-mmcif: reset DMA completion immediately before starting DMA
>   mmc: sh-mmcif: fix I/O errors
>
> Shinya Kuribayashi (1):
>   mmc: sh_mmcif: add support for bundled MMCIF IRQs
>
> Teppei Kamijou (7):
>   mmc: sh_mmcif: force to fail CMD52 immediately
>   mmc: sh_mmcif: ensure run-time suspend call is processed before
>     suspend
>   mmc: sh_mmcif: Add support for eMMC Dual Data Rate
>   mmc: sh_mmcif: Use msecs_to_jiffies() for host->timeout
>   mmc: sh_mmcif: Avoid unnecessary mmc_delay() at mmc_card_sleepawake()
>   mmc: sh-mmcif: Terminate DMA transactions when detecting timeout or
>     error
>   mmc: sh-mmcif: report all errors
>
>  drivers/mmc/host/sh_mmcif.c |  215 +++++++++++++++++++++++++++++--------------
>  1 files changed, 145 insertions(+), 70 deletions(-)

All 14 merged to mmc-next for 3.9, thanks.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child