Add device tree support for mxs-mmc

Use global stmp_device functionality to reduce mach-dependency.

Signed-off-by Shawn Guo <[email protected]>

---
 drivers/mmc/host/mxs-mmc.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

drivers/mmc/host/mxs-mmc.c +3 -3

index bb03ddd..2ea5361 100644
@@ -40,9 +40,9 @@
 #include <linux/module.h>
 #include <linux/fsl/mxs-dma.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/stmp_device.h>
 
 #include <mach/mxs.h>
-#include <mach/common.h>
 #include <mach/mmc.h>
 
 #define DRIVER_NAME	"mxs-mmc"

@@ -191,7 +191,7 @@ static void mxs_mmc_reset(struct mxs_mmc_host *host)
 {
 	u32 ctrl0, ctrl1;
 
-	mxs_reset_block(host->base);
+	stmp_reset_block(host->base);
 
 	ctrl0 = BM_SSP_CTRL0_IGNORE_CRC;
 	ctrl1 = BF_SSP(0x3, CTRL1_SSP_MODE) |

@@ -279,7 +279,7 @@ static irqreturn_t mxs_mmc_irq_handler(int irq, void
 *dev_id)
 
 	stat = readl(host->base + HW_SSP_CTRL1);
 	writel(stat & MXS_MMC_IRQ_BITS,
-	       host->base + HW_SSP_CTRL1 + MXS_CLR_ADDR);
+	       host->base + HW_SSP_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))

drivers/mmc/host/mxs-mmc.c +4 -4

 		mmc_signal_sdio_irq(host->mmc);

@@ -637,18 +637,18 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host
 *mmc, int enable)
 
 	if (enable) {
 		writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
-		       host->base + HW_SSP_CTRL0 + MXS_SET_ADDR);
+		       host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
 		writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
-		       host->base + HW_SSP_CTRL1 + MXS_SET_ADDR);
+		       host->base + HW_SSP_CTRL1 + STMP_OFFSET_REG_SET);
 
 		if (readl(host->base + HW_SSP_STATUS) & BM_SSP_STATUS_SDIO_IRQ)
 			mmc_signal_sdio_irq(host->mmc);
 
 	} else {
 		writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
-		       host->base + HW_SSP_CTRL0 + MXS_CLR_ADDR);
+		       host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
 		writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
-		       host->base + HW_SSP_CTRL1 + MXS_CLR_ADDR);
+		       host->base + HW_SSP_CTRL1 + STMP_OFFSET_REG_CLR);
 	}
 

spin_unlock_irqrestore(&host->lock, flags);

Let macro ssp_is_old take host as parameter to make the code easy to read.

Signed-off-by Shawn Guo <[email protected]>

---
 drivers/mmc/host/mxs-mmc.c |   59

++++++++++++++++++++++--------------------- 1 files changed, 30 insertions(+), 29 deletions(-)

drivers/mmc/host/mxs-mmc.c +0 -0

index 2ea5361..13907e4 100644

drivers/mmc/host/mxs-mmc.c +8 -8

@@ -183,7 +183,7 @@ static int mxs_mmc_get_cd(struct mmc_host *mmc)
 {
 	struct mxs_mmc_host *host = mmc_priv(mmc);
 
-	return !(readl(host->base + HW_SSP_STATUS) &
+	return !(readl(host->base + HW_SSP_STATUS(host)) &
 		 BM_SSP_STATUS_CARD_DETECT);
 }
 

@@ -207,7 +207,7 @@ static void mxs_mmc_reset(struct mxs_mmc_host *host)
 	writel(BF_SSP(0xffff, TIMING_TIMEOUT) |
 	       BF_SSP(2, TIMING_CLOCK_DIVIDE) |
 	       BF_SSP(0, TIMING_CLOCK_RATE),
-	       host->base + HW_SSP_TIMING);
+	       host->base + HW_SSP_TIMING(host));
 
 	if (host->sdio_irq_en) {
 		ctrl0 |= BM_SSP_CTRL0_SDIO_IRQ_CHECK;

@@ -215,7 +215,7 @@ static void mxs_mmc_reset(struct mxs_mmc_host *host)
 	}
 
 	writel(ctrl0, host->base + HW_SSP_CTRL0);
-	writel(ctrl1, host->base + HW_SSP_CTRL1);
+	writel(ctrl1, host->base + HW_SSP_CTRL1(host));
 }
 
 static void mxs_mmc_start_cmd(struct mxs_mmc_host *host,

@@ -229,12 +229,12 @@ static void mxs_mmc_request_done(struct mxs_mmc_host
 *host)
 
 	if (mmc_resp_type(cmd) & MMC_RSP_PRESENT) {
 		if (mmc_resp_type(cmd) & MMC_RSP_136) {
-			cmd->resp[3] = readl(host->base + HW_SSP_SDRESP0);
-			cmd->resp[2] = readl(host->base + HW_SSP_SDRESP1);
-			cmd->resp[1] = readl(host->base + HW_SSP_SDRESP2);
-			cmd->resp[0] = readl(host->base + HW_SSP_SDRESP3);
+			cmd->resp[3] = readl(host->base + HW_SSP_SDRESP0(host));
+			cmd->resp[2] = readl(host->base + HW_SSP_SDRESP1(host));
+			cmd->resp[1] = readl(host->base + HW_SSP_SDRESP2(host));
+			cmd->resp[0] = readl(host->base + HW_SSP_SDRESP3(host));
 		} else {
-			cmd->resp[0] = readl(host->base + HW_SSP_SDRESP0);
+			cmd->resp[0] = readl(host->base + HW_SSP_SDRESP0(host));
 		}
 	}

@@ -277,11 +277,11 @@ static irqreturn_t mxs_mmc_irq_handler(int irq, void
 *dev_id)
 

drivers/mmc/host/mxs-mmc.c +0 -0

 		mmc_signal_sdio_irq(host->mmc);
 

drivers/mmc/host/mxs-mmc.c +11 -10

 	spin_unlock(&host->lock);

@@ -485,7 +485,7 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 		blocks = 1;
 
 	/* xfer count, block size and count need to be set differently */
-	if (ssp_is_old()) {
+	if (ssp_is_old(host)) {
 		ctrl0 |= BF_SSP(data_size, CTRL0_XFER_COUNT);
 		cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
 			BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);

@@ -509,10 +509,10 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 
 	/* set the timeout count */
 	timeout = mxs_ns_to_ssp_ticks(host->clk_rate, data->timeout_ns);
-	val = readl(host->base + HW_SSP_TIMING);
+	val = readl(host->base + HW_SSP_TIMING(host));
 	val &= ~(BM_SSP_TIMING_TIMEOUT);
 	val |= BF_SSP(timeout, TIMING_TIMEOUT);
-	writel(val, host->base + HW_SSP_TIMING);
+	writel(val, host->base + HW_SSP_TIMING(host));
 
 	/* pio */
 	host->ssp_pio_words[0] = ctrl0;

@@ -598,11 +598,11 @@ static void mxs_mmc_set_clk_rate(struct mxs_mmc_host
 *host, unsigned int rate)
 
 	ssp_sck = ssp_clk / clock_divide / (1 + clock_rate);
 
-	val = readl(host->base + HW_SSP_TIMING);
+	val = readl(host->base + HW_SSP_TIMING(host));
 	val &= ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
 	val |= BF_SSP(clock_divide, TIMING_CLOCK_DIVIDE);
 	val |= BF_SSP(clock_rate, TIMING_CLOCK_RATE);
-	writel(val, host->base + HW_SSP_TIMING);
+	writel(val, host->base + HW_SSP_TIMING(host));
 
 	host->clk_rate = ssp_sck;

@@ -639,16 +639,17 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host
 *mmc, int enable) writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
 		       host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
 		writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
-		       host->base + HW_SSP_CTRL1 + STMP_OFFSET_REG_SET);
+		       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_SET);
 
-		if (readl(host->base + HW_SSP_STATUS) & BM_SSP_STATUS_SDIO_IRQ)
+		if (readl(host->base + HW_SSP_STATUS(host)) &
+				BM_SSP_STATUS_SDIO_IRQ)
 			mmc_signal_sdio_irq(host->mmc);
 
 	} else {
 		writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
 		       host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
 		writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
-		       host->base + HW_SSP_CTRL1 + STMP_OFFSET_REG_CLR);
+		       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 	}
 
 	spin_unlock_irqrestore(&host->lock, flags);

@@ -765,8 +766,8 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 
 	mmc->max_segs = 52;
 	mmc->max_blk_size = 1 << 0xf;
-	mmc->max_blk_count = (ssp_is_old()) ? 0xff : 0xffffff;
-	mmc->max_req_size = (ssp_is_old()) ? 0xffff : 0xffffffff;
+	mmc->max_blk_count = (ssp_is_old(host)) ? 0xff : 0xffffff;
+	mmc->max_req_size = (ssp_is_old(host)) ? 0xffff : 0xffffffff;
 	mmc->max_seg_size = dma_get_max_seg_size(host->dmach->device->dev);
 
 	platform_set_drvdata(pdev, mmc);

drivers/mmc/host/mxs-mmc.c line 54

Quoted 3 lines

#define SSP_VERSION_LATEST 4

drivers/mmc/host/mxs-mmc.c +1 -1

-#define ssp_is_old()		(host->version < SSP_VERSION_LATEST)
+#define ssp_is_old(host)	(host->version < SSP_VERSION_LATEST)

(host)->version ... to make the macro safer.

drivers/mmc/host/mxs-mmc.c static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id) · line 284

Quoted 2 lines

drivers/mmc/host/mxs-mmc.c +1 -1

-	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
+	if ((sta & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))

^ s/sta/stat/ ... did you try compiling this at all ? :-)

The register HW_SSP_VERSION is broken for ssp version detection, as the address of the register is different between imx23 and imx28. Let's use platform_device_id to detect the device, so that the use of cpu_is_xxx can be removed.

Signed-off-by Shawn Guo <[email protected]>

drivers/mmc/host/mxs-mmc.c +0 -1

index 13907e4..54bbb8b 100644
@@ -42,7 +42,6 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/stmp_device.h>
 
-#include <mach/mxs.h>
 #include <mach/mmc.h>
 
 #define DRIVER_NAME	"mxs-mmc"

drivers/mmc/host/mxs-mmc.c +21 -6

-#define  BP_SSP_VERSION_MAJOR			(24)
 
 #define BF_SSP(value, field)	(((value) << BP_SSP_##field) &
 BM_SSP_##field)

@@ -139,6 +135,11 @@
 
 #define SSP_PIO_NUM	3
 
+enum mxs_mmc_id {
+	IMX23_MMC,
+	IMX28_MMC,
+};
+
 struct mxs_mmc_host {
 	struct mmc_host			*mmc;
 	struct mmc_request		*mrq;

@@ -158,7 +159,7 @@ struct mxs_mmc_host {
 	enum dma_transfer_direction	slave_dirn;
 	u32				ssp_pio_words[SSP_PIO_NUM];
 
-	unsigned int			version;
+	enum mxs_mmc_id			devid;
 	unsigned char			bus_width;
 	spinlock_t			lock;
 	int				sdio_irq_en;

@@ -678,6 +679,19 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan,
 void *param) return true;
 }
 
+static struct platform_device_id mxs_mmc_ids[] = {
+	{
+		.name = "imx23-mmc",
+		.driver_data = IMX23_MMC,
+	}, {
+		.name = "imx28-mmc",
+		.driver_data = IMX28_MMC,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, mxs_mmc_ids);
+
 static int mxs_mmc_probe(struct platform_device *pdev)
 {
 	struct mxs_mmc_host *host;

@@ -712,10 +726,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		goto out_mmc_free;
 	}
 
-	/* only major verion does matter */
-	host->version = readl(host->base + HW_SSP_VERSION) >>
-			BP_SSP_VERSION_MAJOR;
-
+	host->devid = pdev->id_entry->driver_data;
 	host->mmc = mmc;
 	host->res = r;
 	host->dma_res = dmares;

@@ -866,6 +877,7 @@ static const struct dev_pm_ops mxs_mmc_pm_ops = {
 static struct platform_driver mxs_mmc_driver = {
 	.probe		= mxs_mmc_probe,
 	.remove		= mxs_mmc_remove,
+	.id_table	= mxs_mmc_ids,
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,

drivers/mmc/host/mxs-mmc.c line 52

Quoted 3 lines

drivers/mmc/host/mxs-mmc.c +1 -2

-#define SSP_VERSION_LATEST	4
-#define ssp_is_old(host)	(host->version < SSP_VERSION_LATEST)
+#define ssp_is_old(host)	(host->devid == IMX23_MMC)

(host)->devid ...

Rename arch/arm/mach-mxs/include/mach/mmc.h to include/linux/mmc/mxs-mmc.h, so that mxs-mmc driver becomes <mach/*> inclusion free.

Signed-off-by Shawn Guo <[email protected]>

---
 arch/arm/mach-mxs/include/mach/devices-common.h    |    2 +-
 drivers/mmc/host/mxs-mmc.c                         |    3 +--
 .../mach/mmc.h => include/linux/mmc/mxs-mmc.h      |    7 ++++---
 3 files changed, 6 insertions(+), 6 deletions(-)
 rename arch/arm/mach-mxs/include/mach/mmc.h => include/linux/mmc/mxs-mmc.h

(81%)

(quoted diff) +0 -0

b/arch/arm/mach-mxs/include/mach/devices-common.h index 2b37689..6fc0601 100644

arch/arm/mach-mxs/include/mach/devices-common.h +1 -1

@@ -87,7 +87,7 @@ struct platform_device * __init mxs_add_mxs_i2c(
 		const struct mxs_mxs_i2c_data *data);
 
 /* mmc */
-#include <mach/mmc.h>
+#include <linux/mmc/mxs-mmc.h>
 struct mxs_mxs_mmc_data {
 	const char *devid;
 	int id;

drivers/mmc/host/mxs-mmc.c +1 -2

index 54bbb8b..c70c7c7 100644
@@ -41,8 +41,7 @@
 #include <linux/fsl/mxs-dma.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/stmp_device.h>
-
-#include <mach/mmc.h>
+#include <linux/mmc/mxs-mmc.h>
 
 #define DRIVER_NAME	"mxs-mmc"
 

(quoted diff) +0 -0

b/include/linux/mmc/mxs-mmc.h similarity index 81% rename from arch/arm/mach-mxs/include/mach/mmc.h rename to include/linux/mmc/mxs-mmc.h index 211547a..7c2ad3a 100644

include/linux/mmc/mxs-mmc.h renamed from arch/arm/mach-mxs/include/mach/mmc.h +2 -2

@@ -6,8 +6,8 @@
  * published by the Free Software Foundation.
  */
 
-#ifndef __MACH_MXS_MMC_H__
-#define __MACH_MXS_MMC_H__
+#ifndef __LINUX_MMC_MXS_MMC_H__
+#define __LINUX_MMC_MXS_MMC_H__
 
 struct mxs_mmc_platform_data {
 	int wp_gpio;	/* write protect pin */

Use devm_request_and_ioremap and devm_request_irq helpers to clean up the code a little bit.

Signed-off-by Shawn Guo <[email protected]>

---
 drivers/mmc/host/mxs-mmc.c |   40 +++++++++-------------------------------
 1 files changed, 9 insertions(+), 31 deletions(-)

drivers/mmc/host/mxs-mmc.c +9 -27

index c70c7c7..1a8ba314 100644
@@ -146,8 +146,6 @@ struct mxs_mmc_host {
 	struct mmc_data			*data;
 
 	void __iomem			*base;
-	int				irq;
-	struct resource			*res;
 	struct resource			*dma_res;
 	struct clk			*clk;
 	unsigned int			clk_rate;

@@ -695,7 +693,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 {
 	struct mxs_mmc_host *host;
 	struct mmc_host *mmc;
-	struct resource *iores, *dmares, *r;
+	struct resource *iores, *dmares;
 	struct mxs_mmc_platform_data *pdata;
 	struct pinctrl *pinctrl;
 	int ret = 0, irq_err, irq_dma;

@@ -708,28 +706,20 @@ static int mxs_mmc_probe(struct platform_device
 *pdev) if (!iores || !dmares || irq_err < 0 || irq_dma < 0)
 		return -EINVAL;
 
-	r = request_mem_region(iores->start, resource_size(iores), pdev->name);
-	if (!r)
-		return -EBUSY;
-
 	mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
-		goto out_release_mem;
-	}
+	if (!mmc)
+		return -ENOMEM;
 
 	host = mmc_priv(mmc);
-	host->base = ioremap(r->start, resource_size(r));
+	host->base = devm_request_and_ioremap(&pdev->dev, iores);
 	if (!host->base) {
-		ret = -ENOMEM;
+		ret = -EADDRNOTAVAIL;
 		goto out_mmc_free;
 	}
 
 	host->devid = pdev->id_entry->driver_data;
 	host->mmc = mmc;
-	host->res = r;
 	host->dma_res = dmares;
-	host->irq = irq_err;
 	host->sdio_irq_en = 0;
 
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);

@@ -741,7 +731,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	host->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
-		goto out_iounmap;
+		goto out_mmc_free;
 	}
 	clk_prepare_enable(host->clk);
 

@@ -782,7 +772,8 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mmc);
 
-	ret = request_irq(host->irq, mxs_mmc_irq_handler, 0, DRIVER_NAME, host);
+	ret = devm_request_irq(&pdev->dev, irq_err, mxs_mmc_irq_handler, 0,
+			       DRIVER_NAME, host);
 	if (ret)
 		goto out_free_dma;
 

@@ -790,26 +781,20 @@ static int mxs_mmc_probe(struct platform_device
 *pdev)
 
 	ret = mmc_add_host(mmc);
 	if (ret)
-		goto out_free_irq;
+		goto out_free_dma;
 
 	dev_info(mmc_dev(host->mmc), "initialized\n");
 
 	return 0;
 
-out_free_irq:
-	free_irq(host->irq, host);
 out_free_dma:
 	if (host->dmach)
 		dma_release_channel(host->dmach);
 out_clk_put:
 	clk_disable_unprepare(host->clk);
 	clk_put(host->clk);
-out_iounmap:
-	iounmap(host->base);
 out_mmc_free:
 	mmc_free_host(mmc);
-out_release_mem:
-	release_mem_region(iores->start, resource_size(iores));
 	return ret;
 }

@@ -817,12 +802,9 @@ static int mxs_mmc_remove(struct platform_device
 *pdev) {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
 	struct mxs_mmc_host *host = mmc_priv(mmc);
-	struct resource *res = host->res;
 
 	mmc_remove_host(mmc);
 
-	free_irq(host->irq, host);
-
 	platform_set_drvdata(pdev, NULL);
 
 	if (host->dmach)

@@ -831,12 +813,8 @@ static int mxs_mmc_remove(struct platform_device
 *pdev) clk_disable_unprepare(host->clk);

btw do we have i2c dt bindings ?

Acked-by Marek Vasut <[email protected]>

It replaces dma_res with dma_channel in struct mxs_mmc_host, so that the device tree support will be a little easier, since dma channel can not be retrieved from "struct resource *dma_res".

Signed-off-by Shawn Guo <[email protected]>

---
 drivers/mmc/host/mxs-mmc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

drivers/mmc/host/mxs-mmc.c +2 -2

index 1a8ba314..e905721 100644
@@ -146,7 +146,7 @@ struct mxs_mmc_host {
 	struct mmc_data			*data;
 
 	void __iomem			*base;
-	struct resource			*dma_res;
+	int				dma_channel;
 	struct clk			*clk;
 	unsigned int			clk_rate;
 

@@ -668,7 +668,7 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan,
 void *param) if (!mxs_dma_is_apbh(chan))
 		return false;
 
-	if (chan->chan_id != host->dma_res->start)
+	if (chan->chan_id != host->dma_channel)
 		return false;
 
 	chan->private = &host->dma_data;

It has wp_gpio copied in struct mxs_mmc_host from platform_data,

so that the use of platform_data can be limited in probe function, which will ease the device tree probe.

Signed-off-by Shawn Guo <[email protected]>

---
 drivers/mmc/host/mxs-mmc.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

drivers/mmc/host/mxs-mmc.c +3 -7

index e905721..66f792a 100644
@@ -160,21 +160,17 @@ struct mxs_mmc_host {
 	unsigned char			bus_width;
 	spinlock_t			lock;
 	int				sdio_irq_en;
+	int				wp_gpio;
 };
 
 static int mxs_mmc_get_ro(struct mmc_host *mmc)
 {
 	struct mxs_mmc_host *host = mmc_priv(mmc);
-	struct mxs_mmc_platform_data *pdata =
-		mmc_dev(host->mmc)->platform_data;
 
-	if (!pdata)
-		return -EFAULT;
-
-	if (!gpio_is_valid(pdata->wp_gpio))
+	if (!gpio_is_valid(host->wp_gpio))
 		return -EINVAL;
 
-	return gpio_get_value(pdata->wp_gpio);
+	return gpio_get_value(host->wp_gpio);
 }
 
 static int mxs_mmc_get_cd(struct mmc_host *mmc)

Dear Shawn Guo,

Quoted 1 line

It has wp_gpio copied in struct mxs_mmc_host from platform_data,

Copy wp_gpio into struct ...

Quoted 4 lines

so that the use of platform_data can be limited in probe function, which will ease the device tree probe.

Signed-off-by Shawn Guo <[email protected]>

Otherwise

Acked-by Marek Vasut <[email protected]>

It adds device tree probe support for mxs-mmc driver.

Signed-off-by Shawn Guo <[email protected]>

---
 Documentation/devicetree/bindings/mmc/mxs-mmc.txt |   26 +++++++++++
 drivers/mmc/host/mxs-mmc.c                        |   48
++++++++++++++++++-- 2 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mxs-mmc.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file mode 100644 index 0000000..d7c2a40

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +3 -0

@@ -0,0 +1,26 @@
+* Freescale MXS MMC controller
+
+The Freescale MXS Synchronous Serial Ports (SSP) can act as a MMC

controller +to support MMC, SD, and SDIO types of memory cards.

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +1 -0

+

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +11 -0

+
+Examples:
+
+ssp0: ssp at 80010000 {
+	compatible = "fsl,imx28-mmc";
+	reg = <0x80010000 2000>;
+	interrupts = <96 82>;
+	fsl,ssp-dma-channel = <0>;
+	fsl,bus-width = <8>;
+	status = "okay";
+};

drivers/mmc/host/mxs-mmc.c +42 -5

index 66f792a..c6b56b4 100644
@@ -23,6 +23,9 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>

@@ -685,8 +688,18 @@ static struct platform_device_id mxs_mmc_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, mxs_mmc_ids);
 
+static const struct of_device_id mxs_mmc_dt_ids[] = {
+	{ .compatible = "fsl,imx23-mmc", .data = (void *) IMX23_MMC, },
+	{ .compatible = "fsl,imx28-mmc", .data = (void *) IMX28_MMC, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
+
 static int mxs_mmc_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+			of_match_device(mxs_mmc_dt_ids, &pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct mxs_mmc_host *host;
 	struct mmc_host *mmc;
 	struct resource *iores, *dmares;

@@ -699,7 +712,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	irq_err = platform_get_irq(pdev, 0);
 	irq_dma = platform_get_irq(pdev, 1);
-	if (!iores || !dmares || irq_err < 0 || irq_dma < 0)
+	if (!iores || irq_err < 0 || irq_dma < 0)
 		return -EINVAL;
 
 	mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);

@@ -713,15 +726,31 @@ static int mxs_mmc_probe(struct platform_device
 *pdev) goto out_mmc_free;
 	}
 
-	host->devid = pdev->id_entry->driver_data;
+	if (np) {
+		host->devid = (enum mxs_mmc_id) of_id->data;
+		/*
+		 * TODO: This is a temporary solution and should be changed
+		 * to use generic DMA binding later when the helplers get in.
+		 */
+		ret = of_property_read_u32(np, "fsl,ssp-dma-channel",
+					   &host->dma_channel);
+		if (ret) {
+			dev_err(mmc_dev(host->mmc),
+				"failed to get dma channel\n");
+			goto out_mmc_free;
+		}
+	} else {
+		host->devid = pdev->id_entry->driver_data;
+		host->dma_channel = dmares->start;
+	}
+
 	host->mmc = mmc;
-	host->dma_channel = dmares->start;
 	host->sdio_irq_en = 0;
 
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
 	if (IS_ERR(pinctrl)) {
 		ret = PTR_ERR(pinctrl);
-		goto out_iounmap;
+		goto out_mmc_free;
 	}
 
 	host->clk = clk_get(&pdev->dev, NULL);

@@ -749,7 +778,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		    MMC_CAP_SDIO_IRQ | MMC_CAP_NEEDS_POLL;
 
 	pdata =	mmc_dev(host->mmc)->platform_data;
-	if (pdata) {
+	if (!pdata) {
+		u32 bus_width = 0;
+		of_property_read_u32(np, "fsl,bus-width", &bus_width);
+		if (bus_width == 4)
+			mmc->caps |= MMC_CAP_4_BIT_DATA;
+		else if (bus_width == 8)
+			mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
+		host->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
+	} else {
 		if (pdata->flags & SLOTF_8_BIT_CAPABLE)
 			mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
 		if (pdata->flags & SLOTF_4_BIT_CAPABLE)

Quoted 2 lines

+Optional properties:
+- wp-gpios: Specify GPIOs for write protection

You can have multiple wp-gpio per port? :-)

It adds device tree probe support for mxs-mmc driver.

Signed-off-by Shawn Guo <[email protected]>

---
 Documentation/devicetree/bindings/mmc/mxs-mmc.txt |   26 +++++++++++
 drivers/mmc/host/mxs-mmc.c                        |   48 ++++++++++++++++++--
 2 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mxs-mmc.txt

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +2 -0

new file mode 100644
index 0000000..d7c2a40
@@ -0,0 +1,26 @@
+* Freescale MXS MMC controller
+

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +4 -0

+
+Optional properties:
+- wp-gpios: Specify GPIOs for write protection
+

drivers/mmc/host/mxs-mmc.c +14 -1

index 66f792a..c6b56b4 100644
@@ -23,6 +23,9 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>

@@ -685,8 +688,18 @@ static struct platform_device_id mxs_mmc_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, mxs_mmc_ids);
 
+static const struct of_device_id mxs_mmc_dt_ids[] = {
+	{ .compatible = "fsl,imx23-mmc", .data = (void *) IMX23_MMC, },
+	{ .compatible = "fsl,imx28-mmc", .data = (void *) IMX28_MMC, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
+
 static int mxs_mmc_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+			of_match_device(mxs_mmc_dt_ids, &pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct mxs_mmc_host *host;
 	struct mmc_host *mmc;
 	struct resource *iores, *dmares;

@@ -699,7 +712,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	irq_err = platform_get_irq(pdev, 0);
 	irq_dma = platform_get_irq(pdev, 1);
-	if (!iores || !dmares || irq_err < 0 || irq_dma < 0)
+	if (!iores || irq_err < 0 || irq_dma < 0)
 		return -EINVAL;
 
 	mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);

drivers/mmc/host/mxs-mmc.c +23 -3

+		 */
+		ret = of_property_read_u32(np, "fsl,ssp-dma-channel",
+					   &host->dma_channel);
+		if (ret) {
+			dev_err(mmc_dev(host->mmc),
+				"failed to get dma channel\n");
+			goto out_mmc_free;
+		}
+	} else {
+		host->devid = pdev->id_entry->driver_data;
+		host->dma_channel = dmares->start;
+	}
+
 	host->mmc = mmc;
-	host->dma_channel = dmares->start;
 	host->sdio_irq_en = 0;
 
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
 	if (IS_ERR(pinctrl)) {
 		ret = PTR_ERR(pinctrl);
-		goto out_iounmap;
+		goto out_mmc_free;
 	}
 
 	host->clk = clk_get(&pdev->dev, NULL);

@@ -749,7 +778,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		    MMC_CAP_SDIO_IRQ | MMC_CAP_NEEDS_POLL;
 
 	pdata =	mmc_dev(host->mmc)->platform_data;
-	if (pdata) {
+	if (!pdata) {
+		u32 bus_width = 0;
+		of_property_read_u32(np, "fsl,bus-width", &bus_width);
+		if (bus_width == 4)
+			mmc->caps |= MMC_CAP_4_BIT_DATA;
+		else if (bus_width == 8)
+			mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
+		host->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
+	} else {
 		if (pdata->flags & SLOTF_8_BIT_CAPABLE)
 			mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
 		if (pdata->flags & SLOTF_4_BIT_CAPABLE)

Hi Shawn, just minor language changes,

Quoted 22 lines · Shawn Guo

It adds device tree probe support for mxs-mmc driver.

Signed-off-by Shawn Guo <[email protected]>

---
 Documentation/devicetree/bindings/mmc/mxs-mmc.txt |   26 +++++++++++
 drivers/mmc/host/mxs-mmc.c                        |   48 ++++++++++++++++++--
 2 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mxs-mmc.txt

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +7 -0

new file mode 100644
index 0000000..d7c2a40
@@ -0,0 +1,26 @@
+* Freescale MXS MMC controller
+
+The Freescale MXS Synchronous Serial Ports (SSP) can act as a MMC controller
+to support MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible: Should be "fsl,<chip>-mmc".  The supported chips include

Documentation/devicetree/bindings/mmc/mxs-mmc.txt line 12

Quoted 5 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +5 -0

+  imx23 and imx28.
+- reg: Should contain registers location and length
+- interrupts: Should contain ERROR and DMA interrupts
+- fsl,ssp-dma-channel: APBH DMA channel for the SSP
+- fsl,bus-width: Number of data lines, can be <1>, <4>, or <8>

Please don't use a prefix on "bus-width" -- see Arnd's proposed bindings:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/091993.html

Quoted 9 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +9 -0

+
+Optional properties:
+- wp-gpios: Specify GPIOs for write protection
+
+Examples:
+
+ssp0: ssp@80010000 {
+	compatible = "fsl,imx28-mmc";
+	reg = <0x80010000 2000>;

Documentation/devicetree/bindings/mmc/mxs-mmc.txt line 26

Quoted 5 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +5 -0

+	interrupts = <96 82>;
+	fsl,ssp-dma-channel = <0>;
+	fsl,bus-width = <8>;
+	status = "okay";
+};

You list a "status" property but it isn't mentioned in the required or optional properties section, or used in the code. What's it for?

Quoted 46 lines

drivers/mmc/host/mxs-mmc.c +14 -1

index 66f792a..c6b56b4 100644
@@ -23,6 +23,9 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>

@@ -685,8 +688,18 @@ static struct platform_device_id mxs_mmc_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, mxs_mmc_ids);
 
+static const struct of_device_id mxs_mmc_dt_ids[] = {
+	{ .compatible = "fsl,imx23-mmc", .data = (void *) IMX23_MMC, },
+	{ .compatible = "fsl,imx28-mmc", .data = (void *) IMX28_MMC, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
+
 static int mxs_mmc_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+			of_match_device(mxs_mmc_dt_ids, &pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct mxs_mmc_host *host;
 	struct mmc_host *mmc;
 	struct resource *iores, *dmares;

@@ -699,7 +712,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	irq_err = platform_get_irq(pdev, 0);
 	irq_dma = platform_get_irq(pdev, 1);
-	if (!iores || !dmares || irq_err < 0 || irq_dma < 0)
+	if (!iores || irq_err < 0 || irq_dma < 0)
 		return -EINVAL;
 
 	mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);

@@ -713,15 +726,31 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		goto out_mmc_free;
 	}
 

drivers/mmc/host/mxs-mmc.c static int mxs_mmc_probe(struct platform_device *pdev) · line 733

Quoted 6 lines

drivers/mmc/host/mxs-mmc.c +5 -1

-	host->devid = pdev->id_entry->driver_data;
+	if (np) {
+		host->devid = (enum mxs_mmc_id) of_id->data;
+		/*
+		 * TODO: This is a temporary solution and should be changed
+		 * to use generic DMA binding later when the helplers get in.

"helpers"

Quoted 44 lines

drivers/mmc/host/mxs-mmc.c +23 -3

+		 */
+		ret = of_property_read_u32(np, "fsl,ssp-dma-channel",
+					   &host->dma_channel);
+		if (ret) {
+			dev_err(mmc_dev(host->mmc),
+				"failed to get dma channel\n");
+			goto out_mmc_free;
+		}
+	} else {
+		host->devid = pdev->id_entry->driver_data;
+		host->dma_channel = dmares->start;
+	}
+
 	host->mmc = mmc;
-	host->dma_channel = dmares->start;
 	host->sdio_irq_en = 0;
 
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
 	if (IS_ERR(pinctrl)) {
 		ret = PTR_ERR(pinctrl);
-		goto out_iounmap;
+		goto out_mmc_free;
 	}
 
 	host->clk = clk_get(&pdev->dev, NULL);

@@ -749,7 +778,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		    MMC_CAP_SDIO_IRQ | MMC_CAP_NEEDS_POLL;
 
 	pdata =	mmc_dev(host->mmc)->platform_data;
-	if (pdata) {
+	if (!pdata) {
+		u32 bus_width = 0;
+		of_property_read_u32(np, "fsl,bus-width", &bus_width);
+		if (bus_width == 4)
+			mmc->caps |= MMC_CAP_4_BIT_DATA;
+		else if (bus_width == 8)
+			mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
+		host->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
+	} else {
 		if (pdata->flags & SLOTF_8_BIT_CAPABLE)
 			mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
 		if (pdata->flags & SLOTF_4_BIT_CAPABLE)

@@ -857,6 +894,7 @@ static struct platform_driver mxs_mmc_driver = {
 		.owner	= THIS_MODULE,

drivers/mmc/host/mxs-mmc.c static struct platform_driver mxs_mmc_driver = { · line 900

Quoted 6 lines

drivers/mmc/host/mxs-mmc.c +1 -0

 #ifdef CONFIG_PM
 		.pm	= &mxs_mmc_pm_ops,
+		.of_match_table = mxs_mmc_dt_ids,
 #endif
 	},
 };

Rest looks fine, thanks.

Quoted 13 lines

Quoted 13 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +7 -0

new file mode 100644
index 0000000..d7c2a40
@@ -0,0 +1,26 @@
+* Freescale MXS MMC controller
+
+The Freescale MXS Synchronous Serial Ports (SSP) can act as a MMC controller
+to support MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible: Should be "fsl,<chip>-mmc".  The supported chips include

Quoted 9 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt line 12

Quoted 5 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +5 -0

+  imx23 and imx28.
+- reg: Should contain registers location and length
+- interrupts: Should contain ERROR and DMA interrupts
+- fsl,ssp-dma-channel: APBH DMA channel for the SSP
+- fsl,bus-width: Number of data lines, can be <1>, <4>, or <8>

Please don't use a prefix on "bus-width" -- see Arnd's proposed bindings:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/091993.html

Yes, I've been keeping my eyes on the discussion. Before that generic bindings get landed, the vendor prefix can be treated as a marker that this is something should use generic binding.

Hi, adding Arnd,

Quoted 22 lines · Shawn Guo

Quoted 13 lines

Quoted 13 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +7 -0

new file mode 100644
index 0000000..d7c2a40
@@ -0,0 +1,26 @@
+* Freescale MXS MMC controller
+
+The Freescale MXS Synchronous Serial Ports (SSP) can act as a MMC controller
+to support MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible: Should be "fsl,<chip>-mmc". ?The supported chips include

Quoted 9 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt line 12

Quoted 5 lines

Documentation/devicetree/bindings/mmc/mxs-mmc.txt new file +5 -0

+ ?imx23 and imx28.
+- reg: Should contain registers location and length
+- interrupts: Should contain ERROR and DMA interrupts
+- fsl,ssp-dma-channel: APBH DMA channel for the SSP
+- fsl,bus-width: Number of data lines, can be <1>, <4>, or <8>

Please don't use a prefix on "bus-width" -- see Arnd's proposed bindings:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/091993.html

Quoted 3 lines

Yes, I've been keeping my eyes on the discussion. ?Before that generic bindings get landed, the vendor prefix can be treated as a marker that this is something should use generic binding.

Sorry, I don't understand this explanation. ?As soon as Arnd's patch lands (possibly even for 3.5), we will be making sure that all of the bindings are consistently using "bus-width" with no prefix -- that's what Arnd's patch does to the existing .dts files. ?Why introduce an inconsistent binding now that we have to change later, instead of getting it right straight away?

I'd like bindings going into mainline to be as correct as possible from the moment that they're merged into mainline, because they describe an API with the kernel. ?The fact that we have some bindings currently in the tree that we'll have to change to use the consistent naming scheme Arnd proposes is regrettable, not intentional, so we shouldn't be planning on doing more of it. ?Does that make sense?

I'll take a look at merging Arnd's bindings patch and fixing up the review comments on it now; it sounds like we really need to get it merged and adopted very soon.

In that case, ok, I will remove the prefix right away.