[PATCH v2 0/5] mmc: mxcmmc: add mpc512x support

[PATCH v2 0/5] mmc: mxcmmc: add mpc512x support

[Anatolij Gustschin]

The SDHC controller on mpc512x is compatible with i.MX31 SDHC
controller, the existing MMC host driver can be used on
mpc512x with some modifications, the patch series extends
the existing driver. It is based on the following v3 mxcmmc DT
support patch:

   https://patchwork.kernel.org/patch/2368631

Changes in v2 series are mentioned in each patch

Anatolij Gustschin (5):
  mmc: mxcmmc: add mpc512x SDHC support
  mmc: mxcmmc: use slot-gpio API for write-protect detection
  mmc: mxcmmc: constify mxcmci_devtype
  mmc: mxcmmc: enable DMA support on mpc512x
  mmc: mxcmmc: fix race conditions for host->req and host->data access

 arch/powerpc/boot/dts/mpc5121.dtsi |    2 +
 drivers/mmc/host/Kconfig           |   10 +-
 drivers/mmc/host/mxcmmc.c          |  209 ++++++++++++++++++++++++++----------
 3 files changed, 159 insertions(+), 62 deletions(-)

-- 
1.7.5.4

[PATCH v2 1/5] mmc: mxcmmc: add mpc512x SDHC support

[Anatolij Gustschin]

The SDHC controller on mpc512x is compatible with i.MX31 SDHC,
so the mxcmmc driver can be used on mpc512x, too. Extend the
driver to support mpc512x as well.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - change to select register accessors at build time
   and use ioread32be() and iowrite32be() accessors
 - drop 512x specific clk_get() changes for now
 - rebase on top of v3 mxcmmc DT support patch from
   Markus https://patchwork.kernel.org/patch/2368631

 drivers/mmc/host/Kconfig  |   10 ++--
 drivers/mmc/host/mxcmmc.c |  133 ++++++++++++++++++++++++++++++---------------
 2 files changed, 94 insertions(+), 49 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index d88219e..eda4376 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -319,12 +319,12 @@ config MMC_MSM
 	  support for SDIO devices.
 
 config MMC_MXC
-	tristate "Freescale i.MX21/27/31 Multimedia Card Interface support"
-	depends on ARCH_MXC
+	tristate "Freescale i.MX21/27/31 or MPC512x Multimedia Card support"
+	depends on ARCH_MXC || PPC_MPC512x
 	help
-	  This selects the Freescale i.MX21, i.MX27 and i.MX31 Multimedia card
-	  Interface. If you have a i.MX platform with a Multimedia Card slot,
-	  say Y or M here.
+	  This selects the Freescale i.MX21, i.MX27, i.MX31 or MPC512x
+	  Multimedia Card Interface. If you have an i.MX or MPC512x platform
+	  with a Multimedia Card slot, say Y or M here.
 
 	  If unsure, say N.
 
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index ff9e496..07a37d8 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -41,7 +41,6 @@
 
 #include <asm/dma.h>
 #include <asm/irq.h>
-#include <asm/sizes.h>
 #include <linux/platform_data/mmc-mxcmmc.h>
 
 #include <linux/platform_data/dma-imx.h>
@@ -119,6 +118,7 @@
 enum mxcmci_type {
 	IMX21_MMC,
 	IMX31_MMC,
+	MPC512X_MMC,
 };
 
 struct mxcmci_host {
@@ -172,6 +172,9 @@ static struct platform_device_id mxcmci_devtype[] = {
 		.name = "imx31-mmc",
 		.driver_data = IMX31_MMC,
 	}, {
+		.name = "mpc512x-sdhc",
+		.driver_data = MPC512X_MMC,
+	}, {
 		/* sentinel */
 	}
 };
@@ -185,6 +188,9 @@ static const struct of_device_id mxcmci_of_match[] = {
 		.compatible = "fsl,imx31-mmc",
 		.data = &mxcmci_devtype[IMX31_MMC],
 	}, {
+		.compatible = "fsl,mpc5121-sdhc",
+		.data = &mxcmci_devtype[MPC512X_MMC],
+	}, {
 		/* sentinel */
 	}
 };
@@ -195,6 +201,43 @@ static inline int is_imx31_mmc(struct mxcmci_host *host)
 	return host->devtype == IMX31_MMC;
 }
 
+static inline int is_mpc512x_mmc(struct mxcmci_host *host)
+{
+	return host->devtype == MPC512X_MMC;
+}
+
+static inline u32 mxcmci_readl(struct mxcmci_host *host, int reg)
+{
+	if (IS_ENABLED(CONFIG_PPC_MPC512x))
+		return ioread32be(host->base + reg);
+	else
+		return readl(host->base + reg);
+}
+
+static inline void mxcmci_writel(struct mxcmci_host *host, u32 val, int reg)
+{
+	if (IS_ENABLED(CONFIG_PPC_MPC512x))
+		iowrite32be(val, host->base + reg);
+	else
+		writel(val, host->base + reg);
+}
+
+static inline u16 mxcmci_readw(struct mxcmci_host *host, int reg)
+{
+	if (IS_ENABLED(CONFIG_PPC_MPC512x))
+		return ioread32be(host->base + reg);
+	else
+		return readw(host->base + reg);
+}
+
+static inline void mxcmci_writew(struct mxcmci_host *host, u16 val, int reg)
+{
+	if (IS_ENABLED(CONFIG_PPC_MPC512x))
+		iowrite32be(val, host->base + reg);
+	else
+		writew(val, host->base + reg);
+}
+
 static void mxcmci_set_clk_rate(struct mxcmci_host *host, unsigned int clk_ios);
 
 static inline void mxcmci_init_ocr(struct mxcmci_host *host)
@@ -246,14 +289,14 @@ static void mxcmci_softreset(struct mxcmci_host *host)
 	dev_dbg(mmc_dev(host->mmc), "mxcmci_softreset\n");
 
 	/* reset sequence */
-	writew(STR_STP_CLK_RESET, host->base + MMC_REG_STR_STP_CLK);
-	writew(STR_STP_CLK_RESET | STR_STP_CLK_START_CLK,
-			host->base + MMC_REG_STR_STP_CLK);
+	mxcmci_writew(host, STR_STP_CLK_RESET, MMC_REG_STR_STP_CLK);
+	mxcmci_writew(host, STR_STP_CLK_RESET | STR_STP_CLK_START_CLK,
+			MMC_REG_STR_STP_CLK);
 
 	for (i = 0; i < 8; i++)
-		writew(STR_STP_CLK_START_CLK, host->base + MMC_REG_STR_STP_CLK);
+		mxcmci_writew(host, STR_STP_CLK_START_CLK, MMC_REG_STR_STP_CLK);
 
-	writew(0xff, host->base + MMC_REG_RES_TO);
+	mxcmci_writew(host, 0xff, MMC_REG_RES_TO);
 }
 static int mxcmci_setup_dma(struct mmc_host *mmc);
 
@@ -272,8 +315,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
 	host->data = data;
 	data->bytes_xfered = 0;
 
-	writew(nob, host->base + MMC_REG_NOB);
-	writew(blksz, host->base + MMC_REG_BLK_LEN);
+	mxcmci_writew(host, nob, MMC_REG_NOB);
+	mxcmci_writew(host, blksz, MMC_REG_BLK_LEN);
 	host->datasize = datasize;
 
 	if (!mxcmci_use_dma(host))
@@ -329,13 +372,13 @@ static void mxcmci_dma_callback(void *data)
 
 	del_timer(&host->watchdog);
 
-	stat = readl(host->base + MMC_REG_STATUS);
-	writel(stat & ~STATUS_DATA_TRANS_DONE, host->base + MMC_REG_STATUS);
+	stat = mxcmci_readl(host, MMC_REG_STATUS);
+	mxcmci_writel(host, stat & ~STATUS_DATA_TRANS_DONE, MMC_REG_STATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "%s: 0x%08x\n", __func__, stat);
 
 	if (stat & STATUS_READ_OP_DONE)
-		writel(STATUS_READ_OP_DONE, host->base + MMC_REG_STATUS);
+		mxcmci_writel(host, STATUS_READ_OP_DONE, MMC_REG_STATUS);
 
 	mxcmci_data_done(host, stat);
 }
@@ -383,12 +426,12 @@ static int mxcmci_start_cmd(struct mxcmci_host *host, struct mmc_command *cmd,
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->use_sdio)
 		int_cntr |= INT_SDIO_IRQ_EN;
-	writel(int_cntr, host->base + MMC_REG_INT_CNTR);
+	mxcmci_writel(host, int_cntr, MMC_REG_INT_CNTR);
 	spin_unlock_irqrestore(&host->lock, flags);
 
-	writew(cmd->opcode, host->base + MMC_REG_CMD);
-	writel(cmd->arg, host->base + MMC_REG_ARG);
-	writew(cmdat, host->base + MMC_REG_CMD_DAT_CONT);
+	mxcmci_writew(host, cmd->opcode, MMC_REG_CMD);
+	mxcmci_writel(host, cmd->arg, MMC_REG_ARG);
+	mxcmci_writew(host, cmdat, MMC_REG_CMD_DAT_CONT);
 
 	return 0;
 }
@@ -402,7 +445,7 @@ static void mxcmci_finish_request(struct mxcmci_host *host,
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->use_sdio)
 		int_cntr |= INT_SDIO_IRQ_EN;
-	writel(int_cntr, host->base + MMC_REG_INT_CNTR);
+	mxcmci_writel(host, int_cntr, MMC_REG_INT_CNTR);
 	spin_unlock_irqrestore(&host->lock, flags);
 
 	host->req = NULL;
@@ -477,14 +520,14 @@ static void mxcmci_read_response(struct mxcmci_host *host, unsigned int stat)
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136) {
 			for (i = 0; i < 4; i++) {
-				a = readw(host->base + MMC_REG_RES_FIFO);
-				b = readw(host->base + MMC_REG_RES_FIFO);
+				a = mxcmci_readw(host, MMC_REG_RES_FIFO);
+				b = mxcmci_readw(host, MMC_REG_RES_FIFO);
 				cmd->resp[i] = a << 16 | b;
 			}
 		} else {
-			a = readw(host->base + MMC_REG_RES_FIFO);
-			b = readw(host->base + MMC_REG_RES_FIFO);
-			c = readw(host->base + MMC_REG_RES_FIFO);
+			a = mxcmci_readw(host, MMC_REG_RES_FIFO);
+			b = mxcmci_readw(host, MMC_REG_RES_FIFO);
+			c = mxcmci_readw(host, MMC_REG_RES_FIFO);
 			cmd->resp[0] = a << 24 | b << 8 | c >> 8;
 		}
 	}
@@ -496,7 +539,7 @@ static int mxcmci_poll_status(struct mxcmci_host *host, u32 mask)
 	unsigned long timeout = jiffies + HZ;
 
 	do {
-		stat = readl(host->base + MMC_REG_STATUS);
+		stat = mxcmci_readl(host, MMC_REG_STATUS);
 		if (stat & STATUS_ERR_MASK)
 			return stat;
 		if (time_after(jiffies, timeout)) {
@@ -520,7 +563,7 @@ static int mxcmci_pull(struct mxcmci_host *host, void *_buf, int bytes)
 				STATUS_BUF_READ_RDY | STATUS_READ_OP_DONE);
 		if (stat)
 			return stat;
-		*buf++ = readl(host->base + MMC_REG_BUFFER_ACCESS);
+		*buf++ = cpu_to_le32(mxcmci_readl(host, MMC_REG_BUFFER_ACCESS));
 		bytes -= 4;
 	}
 
@@ -532,7 +575,7 @@ static int mxcmci_pull(struct mxcmci_host *host, void *_buf, int bytes)
 				STATUS_BUF_READ_RDY | STATUS_READ_OP_DONE);
 		if (stat)
 			return stat;
-		tmp = readl(host->base + MMC_REG_BUFFER_ACCESS);
+		tmp = cpu_to_le32(mxcmci_readl(host, MMC_REG_BUFFER_ACCESS));
 		memcpy(b, &tmp, bytes);
 	}
 
@@ -548,7 +591,7 @@ static int mxcmci_push(struct mxcmci_host *host, void *_buf, int bytes)
 		stat = mxcmci_poll_status(host, STATUS_BUF_WRITE_RDY);
 		if (stat)
 			return stat;
-		writel(*buf++, host->base + MMC_REG_BUFFER_ACCESS);
+		mxcmci_writel(host, cpu_to_le32(*buf++), MMC_REG_BUFFER_ACCESS);
 		bytes -= 4;
 	}
 
@@ -561,7 +604,7 @@ static int mxcmci_push(struct mxcmci_host *host, void *_buf, int bytes)
 			return stat;
 
 		memcpy(&tmp, b, bytes);
-		writel(tmp, host->base + MMC_REG_BUFFER_ACCESS);
+		mxcmci_writel(host, cpu_to_le32(tmp), MMC_REG_BUFFER_ACCESS);
 	}
 
 	stat = mxcmci_poll_status(host, STATUS_BUF_WRITE_RDY);
@@ -607,8 +650,8 @@ static void mxcmci_datawork(struct work_struct *work)
 						  datawork);
 	int datastat = mxcmci_transfer_data(host);
 
-	writel(STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE,
-		host->base + MMC_REG_STATUS);
+	mxcmci_writel(host, STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE,
+		MMC_REG_STATUS);
 	mxcmci_finish_data(host, datastat);
 
 	if (host->req->stop) {
@@ -670,9 +713,11 @@ static irqreturn_t mxcmci_irq(int irq, void *devid)
 	bool sdio_irq;
 	u32 stat;
 
-	stat = readl(host->base + MMC_REG_STATUS);
-	writel(stat & ~(STATUS_SDIO_INT_ACTIVE | STATUS_DATA_TRANS_DONE |
-			STATUS_WRITE_OP_DONE), host->base + MMC_REG_STATUS);
+	stat = mxcmci_readl(host, MMC_REG_STATUS);
+	mxcmci_writel(host,
+		stat & ~(STATUS_SDIO_INT_ACTIVE | STATUS_DATA_TRANS_DONE |
+			 STATUS_WRITE_OP_DONE),
+		MMC_REG_STATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "%s: 0x%08x\n", __func__, stat);
 
@@ -682,11 +727,11 @@ static irqreturn_t mxcmci_irq(int irq, void *devid)
 
 	if (mxcmci_use_dma(host) &&
 	    (stat & (STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE)))
-		writel(STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE,
-			host->base + MMC_REG_STATUS);
+		mxcmci_writel(host, STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE,
+			MMC_REG_STATUS);
 
 	if (sdio_irq) {
-		writel(STATUS_SDIO_INT_ACTIVE, host->base + MMC_REG_STATUS);
+		mxcmci_writel(host, STATUS_SDIO_INT_ACTIVE, MMC_REG_STATUS);
 		mmc_signal_sdio_irq(host->mmc);
 	}
 
@@ -768,7 +813,7 @@ static void mxcmci_set_clk_rate(struct mxcmci_host *host, unsigned int clk_ios)
 			prescaler <<= 1;
 	}
 
-	writew((prescaler << 4) | divider, host->base + MMC_REG_CLK_RATE);
+	mxcmci_writew(host, (prescaler << 4) | divider, MMC_REG_CLK_RATE);
 
 	dev_dbg(mmc_dev(host->mmc), "scaler: %d divider: %d in: %d out: %d\n",
 			prescaler, divider, clk_in, clk_ios);
@@ -831,9 +876,9 @@ static void mxcmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	if (ios->clock) {
 		mxcmci_set_clk_rate(host, ios->clock);
-		writew(STR_STP_CLK_START_CLK, host->base + MMC_REG_STR_STP_CLK);
+		mxcmci_writew(host, STR_STP_CLK_START_CLK, MMC_REG_STR_STP_CLK);
 	} else {
-		writew(STR_STP_CLK_STOP_CLK, host->base + MMC_REG_STR_STP_CLK);
+		mxcmci_writew(host, STR_STP_CLK_STOP_CLK, MMC_REG_STR_STP_CLK);
 	}
 
 	host->clock = ios->clock;
@@ -870,14 +915,14 @@ static void mxcmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 
 	spin_lock_irqsave(&host->lock, flags);
 	host->use_sdio = enable;
-	int_cntr = readl(host->base + MMC_REG_INT_CNTR);
+	int_cntr = mxcmci_readl(host, MMC_REG_INT_CNTR);
 
 	if (enable)
 		int_cntr |= INT_SDIO_IRQ_EN;
 	else
 		int_cntr &= ~INT_SDIO_IRQ_EN;
 
-	writel(int_cntr, host->base + MMC_REG_INT_CNTR);
+	mxcmci_writel(host, int_cntr, MMC_REG_INT_CNTR);
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
@@ -915,7 +960,7 @@ static void mxcmci_watchdog(unsigned long data)
 	struct mmc_host *mmc = (struct mmc_host *)data;
 	struct mxcmci_host *host = mmc_priv(mmc);
 	struct mmc_request *req = host->req;
-	unsigned int stat = readl(host->base + MMC_REG_STATUS);
+	unsigned int stat = mxcmci_readl(host, MMC_REG_STATUS);
 
 	if (host->dma_dir == DMA_FROM_DEVICE) {
 		dmaengine_terminate_all(host->dma);
@@ -957,7 +1002,7 @@ static int mxcmci_probe(struct platform_device *pdev)
 	const struct of_device_id *of_id;
 	struct imxmmc_platform_data *pdata = pdev->dev.platform_data;
 
-	pr_info("i.MX SDHC driver\n");
+	pr_info("i.MX/MPC512x SDHC driver\n");
 
 	of_id = of_match_device(mxcmci_of_match, &pdev->dev);
 
@@ -1043,7 +1088,7 @@ static int mxcmci_probe(struct platform_device *pdev)
 
 	mxcmci_softreset(host);
 
-	host->rev_no = readw(host->base + MMC_REG_REV_NO);
+	host->rev_no = mxcmci_readw(host, MMC_REG_REV_NO);
 	if (host->rev_no != 0x400) {
 		ret = -ENODEV;
 		dev_err(mmc_dev(host->mmc), "wrong rev.no. 0x%08x. aborting.\n",
@@ -1055,9 +1100,9 @@ static int mxcmci_probe(struct platform_device *pdev)
 	mmc->f_max = clk_get_rate(host->clk_per) >> 1;
 
 	/* recommended in data sheet */
-	writew(0x2db4, host->base + MMC_REG_READ_TO);
+	mxcmci_writew(host, 0x2db4, MMC_REG_READ_TO);
 
-	writel(host->default_irq_mask, host->base + MMC_REG_INT_CNTR);
+	mxcmci_writel(host, host->default_irq_mask, MMC_REG_INT_CNTR);
 
 	if (!host->pdata) {
 		host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
-- 
1.7.5.4

[PATCH v2 2/5] mmc: mxcmmc: use slot-gpio API for write-protect detection

[Anatolij Gustschin]

slot-gpio API suppors read-only detection when "wp-gpios"
property is present in the device tree mmc node. Use this
API for write-protect detection.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - no changes

 drivers/mmc/host/mxcmmc.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index 07a37d8..7fe93dd 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -38,6 +38,7 @@
 #include <linux/of_device.h>
 #include <linux/of_dma.h>
 #include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include <asm/dma.h>
 #include <asm/irq.h>
@@ -901,10 +902,11 @@ static int mxcmci_get_ro(struct mmc_host *mmc)
 	if (host->pdata && host->pdata->get_ro)
 		return !!host->pdata->get_ro(mmc_dev(mmc));
 	/*
-	 * Board doesn't support read only detection; let the mmc core
-	 * decide what to do.
+	 * If board doesn't support read only detection (no mmc_gpio
+	 * context or gpio is invalid), then let the mmc core decide
+	 * what to do.
 	 */
-	return -ENOSYS;
+	return mmc_gpio_get_ro(mmc);
 }
 
 static void mxcmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
-- 
1.7.5.4

[PATCH v2 3/5] mmc: mxcmmc: constify mxcmci_devtype

[Anatolij Gustschin]

mxcmci_devtype struct contains constant data, so
constify this struct.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - remove build warning fix (already fixed elsewhere) and
   change commit log

 drivers/mmc/host/mxcmmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index 7fe93dd..dbacbde 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -165,7 +165,7 @@ struct mxcmci_host {
 	enum mxcmci_type	devtype;
 };
 
-static struct platform_device_id mxcmci_devtype[] = {
+static const struct platform_device_id mxcmci_devtype[] = {
 	{
 		.name = "imx21-mmc",
 		.driver_data = IMX21_MMC,
-- 
1.7.5.4

[PATCH v2 4/5] mmc: mxcmmc: enable DMA support on mpc512x

[Anatolij Gustschin]

Add SDHC DMA channel description to the mpc512x device
tree to enable slave channel requesting in the mxcmmc
driver.

mpc512x DMA engine doesn't support endianness conversion
when reading/writing data from peripheral's FIFO, so we
have to swap data buffers before each DMA write and after
each DMA read transfer manually.

Since chained SDHC DMA transfers are not supported on
mpc512x, limit 'max_segs' tunable parameter to one and
initialise it to 64 only when running on i.MX platforms.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - only rebased

 arch/powerpc/boot/dts/mpc5121.dtsi |    2 ++
 drivers/mmc/host/mxcmmc.c          |   35 +++++++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
index d1fe070..712b15a 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -152,6 +152,8 @@
 			compatible = "fsl,mpc5121-sdhc";
 			reg = <0x1500 0x100>;
 			interrupts = <8 0x8>;
+			dmas = <&dma0 30>;
+			dma-names = "rx-tx";
 		};
 
 		i2c@1700 {
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index dbacbde..2861e0d 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -301,6 +301,29 @@ static void mxcmci_softreset(struct mxcmci_host *host)
 }
 static int mxcmci_setup_dma(struct mmc_host *mmc);
 
+#if IS_ENABLED(CONFIG_PPC_MPC512x)
+static inline void buffer_swap32(u32 *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < ((len + 3) / 4); i++) {
+		st_le32(buf, *buf);
+		buf++;
+	}
+}
+
+static void mxcmci_fixup_buffers(struct mmc_data *data)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(data->sg, sg, data->sg_len, i)
+		buffer_swap32(sg_virt(sg), sg->length);
+}
+#else
+static inline void mxcmci_fixup_buffers(struct mmc_data *data) {}
+#endif
+
 static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
 {
 	unsigned int nob = data->blocks;
@@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
 	} else {
 		host->dma_dir = DMA_TO_DEVICE;
 		slave_dirn = DMA_MEM_TO_DEV;
+
+		mxcmci_fixup_buffers(data);
 	}
 
 	nents = dma_map_sg(host->dma->device->dev, data->sg,
@@ -461,9 +486,11 @@ static int mxcmci_finish_data(struct mxcmci_host *host, unsigned int stat)
 	struct mmc_data *data = host->data;
 	int data_error;
 
-	if (mxcmci_use_dma(host))
+	if (mxcmci_use_dma(host)) {
 		dma_unmap_sg(host->dma->device->dev, data->sg, data->sg_len,
 				host->dma_dir);
+		mxcmci_fixup_buffers(data);
+	}
 
 	if (stat & STATUS_ERR_MASK) {
 		dev_dbg(mmc_dev(host->mmc), "request failed. status: 0x%08x\n",
@@ -1033,7 +1060,6 @@ static int mxcmci_probe(struct platform_device *pdev)
 		mmc->caps |= MMC_CAP_SDIO_IRQ;
 
 	/* MMC core transfer sizes tunable parameters */
-	mmc->max_segs = 64;
 	mmc->max_blk_size = 2048;
 	mmc->max_blk_count = 65535;
 	mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
@@ -1052,6 +1078,11 @@ static int mxcmci_probe(struct platform_device *pdev)
 	} else {
 		host->devtype = pdev->id_entry->driver_data;
 	}
+
+	/* adjust max_segs after devtype detection */
+	if (!is_mpc512x_mmc(host))
+		mmc->max_segs = 64;
+
 	host->mmc = mmc;
 	host->pdata = pdata;
 	spin_lock_init(&host->lock);
-- 
1.7.5.4

Re: [PATCH v2 4/5] mmc: mxcmmc: enable DMA support on mpc512x

[Sascha Hauer]

On Mon, Apr 01, 2013 at 12:23:04AM +0200, Anatolij Gustschin wrote:
> Add SDHC DMA channel description to the mpc512x device
> tree to enable slave channel requesting in the mxcmmc
> driver.
> 
> mpc512x DMA engine doesn't support endianness conversion
> when reading/writing data from peripheral's FIFO, so we
> have to swap data buffers before each DMA write and after
> each DMA read transfer manually.
> 
> Since chained SDHC DMA transfers are not supported on
> mpc512x, limit 'max_segs' tunable parameter to one and
> initialise it to 64 only when running on i.MX platforms.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> v2:
>  - only rebased
> 
>  arch/powerpc/boot/dts/mpc5121.dtsi |    2 ++
>  drivers/mmc/host/mxcmmc.c          |   35 +++++++++++++++++++++++++++++++++--
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
> index d1fe070..712b15a 100644
> --- a/arch/powerpc/boot/dts/mpc5121.dtsi
> +++ b/arch/powerpc/boot/dts/mpc5121.dtsi
> @@ -152,6 +152,8 @@
>  			compatible = "fsl,mpc5121-sdhc";
>  			reg = <0x1500 0x100>;
>  			interrupts = <8 0x8>;
> +			dmas = <&dma0 30>;
> +			dma-names = "rx-tx";
>  		};
>  
>  		i2c@1700 {
> diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
> index dbacbde..2861e0d 100644
> --- a/drivers/mmc/host/mxcmmc.c
> +++ b/drivers/mmc/host/mxcmmc.c
> @@ -301,6 +301,29 @@ static void mxcmci_softreset(struct mxcmci_host *host)
>  }
>  static int mxcmci_setup_dma(struct mmc_host *mmc);
>  
> +#if IS_ENABLED(CONFIG_PPC_MPC512x)
> +static inline void buffer_swap32(u32 *buf, int len)
> +{
> +	int i;
> +
> +	for (i = 0; i < ((len + 3) / 4); i++) {
> +		st_le32(buf, *buf);
> +		buf++;
> +	}
> +}
> +
> +static void mxcmci_fixup_buffers(struct mmc_data *data)
> +{
> +	struct scatterlist *sg;
> +	int i;
> +
> +	for_each_sg(data->sg, sg, data->sg_len, i)
> +		buffer_swap32(sg_virt(sg), sg->length);
> +}
> +#else
> +static inline void mxcmci_fixup_buffers(struct mmc_data *data) {}
> +#endif
> +
>  static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
>  {
>  	unsigned int nob = data->blocks;
> @@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
>  	} else {
>  		host->dma_dir = DMA_TO_DEVICE;
>  		slave_dirn = DMA_MEM_TO_DEV;
> +
> +		mxcmci_fixup_buffers(data);

For the register accesses it's fine the way you did in this version, but
here I think a

	if (is_mpc512x_mmc())
		swap_buffer()

would look better.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 4/5] mmc: mxcmmc: enable DMA support on mpc512x

[Anatolij Gustschin]

On Tue, 2 Apr 2013 08:56:54 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:
...
> > @@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
> >  	} else {
> >  		host->dma_dir = DMA_TO_DEVICE;
> >  		slave_dirn = DMA_MEM_TO_DEV;
> > +
> > +		mxcmci_fixup_buffers(data);
> 
> For the register accesses it's fine the way you did in this version, but
> here I think a
> 
> 	if (is_mpc512x_mmc())
> 		swap_buffer()
> 
> would look better.

the swapping function uses optimised st_le32() which is not available
on ARM, that's the reason why this is build time selected. Maybe the
"fixup" in the function name is not pretty, I can change it to "swap"
then.

Thanks,

Anatolij

Re: [PATCH v2 4/5] mmc: mxcmmc: enable DMA support on mpc512x

[Sascha Hauer]

On Tue, Apr 02, 2013 at 09:29:44AM +0200, Anatolij Gustschin wrote:
> On Tue, 2 Apr 2013 08:56:54 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> ...
> > > @@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
> > >  	} else {
> > >  		host->dma_dir = DMA_TO_DEVICE;
> > >  		slave_dirn = DMA_MEM_TO_DEV;
> > > +
> > > +		mxcmci_fixup_buffers(data);
> > 
> > For the register accesses it's fine the way you did in this version, but
> > here I think a
> > 
> > 	if (is_mpc512x_mmc())
> > 		swap_buffer()
> > 
> > would look better.
> 
> the swapping function uses optimised st_le32() which is not available
> on ARM, that's the reason why this is build time selected.

Ah, ok. I think it's fine then.

> Maybe the
> "fixup" in the function name is not pretty, I can change it to "swap"
> then.

Yes, that would be better.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2 5/5] mmc: mxcmmc: fix race conditions for host->req and host->data access

[Anatolij Gustschin]

mxcmci_dma_callback() is invoked by DMA drivers in soft-irq
context and can be interrupted by the mxcmci_irq() interrupt
which can finish the mmc request or data transfer and set
host->req or host->data pointers to NULL. Then mxcmci_data_done()
crashes with a null pointer dereferences. Protect all accesses
to host->req and host->data by spin locks.

Also check host->data pointer in mxcmci_watchdog() before
dereferencing it.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - only rebased

 drivers/mmc/host/mxcmmc.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index 2861e0d..23deb2a 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -694,24 +694,40 @@ static void mxcmci_datawork(struct work_struct *work)
 
 static void mxcmci_data_done(struct mxcmci_host *host, unsigned int stat)
 {
-	struct mmc_data *data = host->data;
+	struct mmc_request *req;
 	int data_error;
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->lock, flags);
 
-	if (!data)
+	if (!host->data) {
+		spin_unlock_irqrestore(&host->lock, flags);
 		return;
+	}
+
+	if (!host->req) {
+		spin_unlock_irqrestore(&host->lock, flags);
+		return;
+	}
+
+	req = host->req;
+	if (!req->stop)
+		host->req = NULL; /* we will handle finish req below */
 
 	data_error = mxcmci_finish_data(host, stat);
 
+	spin_unlock_irqrestore(&host->lock, flags);
+
 	mxcmci_read_response(host, stat);
 	host->cmd = NULL;
 
-	if (host->req->stop) {
-		if (mxcmci_start_cmd(host, host->req->stop, 0)) {
-			mxcmci_finish_request(host, host->req);
+	if (req->stop) {
+		if (mxcmci_start_cmd(host, req->stop, 0)) {
+			mxcmci_finish_request(host, req);
 			return;
 		}
 	} else {
-		mxcmci_finish_request(host, host->req);
+		mxcmci_finish_request(host, req);
 	}
 }
 
@@ -1005,7 +1021,8 @@ static void mxcmci_watchdog(unsigned long data)
 
 	/* Mark transfer as erroneus and inform the upper layers */
 
-	host->data->error = -ETIMEDOUT;
+	if (host->data)
+		host->data->error = -ETIMEDOUT;
 	host->req = NULL;
 	host->cmd = NULL;
 	host->data = NULL;
-- 
1.7.5.4

Re: [PATCH v2 5/5] mmc: mxcmmc: fix race conditions for host->req and host->data access

[Sascha Hauer]

On Mon, Apr 01, 2013 at 12:23:05AM +0200, Anatolij Gustschin wrote:
> mxcmci_dma_callback() is invoked by DMA drivers in soft-irq
> context and can be interrupted by the mxcmci_irq() interrupt
> which can finish the mmc request or data transfer and set
> host->req or host->data pointers to NULL. Then mxcmci_data_done()
> crashes with a null pointer dereferences. Protect all accesses
> to host->req and host->data by spin locks.
> 
> Also check host->data pointer in mxcmci_watchdog() before
> dereferencing it.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

This looks like a bugfix which should be first in this series.

Sascha

> ---
> v2:
>  - only rebased
> 
>  drivers/mmc/host/mxcmmc.c |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
> index 2861e0d..23deb2a 100644
> --- a/drivers/mmc/host/mxcmmc.c
> +++ b/drivers/mmc/host/mxcmmc.c
> @@ -694,24 +694,40 @@ static void mxcmci_datawork(struct work_struct *work)
>  
>  static void mxcmci_data_done(struct mxcmci_host *host, unsigned int stat)
>  {
> -	struct mmc_data *data = host->data;
> +	struct mmc_request *req;
>  	int data_error;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&host->lock, flags);
>  
> -	if (!data)
> +	if (!host->data) {
> +		spin_unlock_irqrestore(&host->lock, flags);
>  		return;
> +	}
> +
> +	if (!host->req) {
> +		spin_unlock_irqrestore(&host->lock, flags);
> +		return;
> +	}
> +
> +	req = host->req;
> +	if (!req->stop)
> +		host->req = NULL; /* we will handle finish req below */
>  
>  	data_error = mxcmci_finish_data(host, stat);
>  
> +	spin_unlock_irqrestore(&host->lock, flags);
> +
>  	mxcmci_read_response(host, stat);
>  	host->cmd = NULL;
>  
> -	if (host->req->stop) {
> -		if (mxcmci_start_cmd(host, host->req->stop, 0)) {
> -			mxcmci_finish_request(host, host->req);
> +	if (req->stop) {
> +		if (mxcmci_start_cmd(host, req->stop, 0)) {
> +			mxcmci_finish_request(host, req);
>  			return;
>  		}
>  	} else {
> -		mxcmci_finish_request(host, host->req);
> +		mxcmci_finish_request(host, req);
>  	}
>  }
>  
> @@ -1005,7 +1021,8 @@ static void mxcmci_watchdog(unsigned long data)
>  
>  	/* Mark transfer as erroneus and inform the upper layers */
>  
> -	host->data->error = -ETIMEDOUT;
> +	if (host->data)
> +		host->data->error = -ETIMEDOUT;
>  	host->req = NULL;
>  	host->cmd = NULL;
>  	host->data = NULL;
> -- 
> 1.7.5.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 0/5] mmc: mxcmmc: add mpc512x support

[Sascha Hauer]

On Mon, Apr 01, 2013 at 12:23:00AM +0200, Anatolij Gustschin wrote:
> The SDHC controller on mpc512x is compatible with i.MX31 SDHC
> controller, the existing MMC host driver can be used on
> mpc512x with some modifications, the patch series extends
> the existing driver. It is based on the following v3 mxcmmc DT
> support patch:
> 
>    https://patchwork.kernel.org/patch/2368631
> 
> Changes in v2 series are mentioned in each patch

With the two comments I made fixed you can add my:

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |