[PATCH 08/10] mmc: mxs-mmc: add device tree support

On Sat, May 12, 2012 at 10:44:42AM -0400, Chris Ball wrote:

Hi Shawn, just minor language changes,

On Mon, May 07 2012, Shawn Guo wrote:

quoted

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

Signed-off-by: Shawn Guo <redacted>
---
 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

diff --git a/Documentation/devicetree/bindings/mmc/mxs-mmc.txt b/Documentation/devicetree/bindings/mmc/mxs-mmc.txt
new file mode 100644
index 0000000..d7c2a40
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mxs-mmc.txt

@@ -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
+  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.

quoted

+
+Optional properties:
+- wp-gpios: Specify GPIOs for write protection
+
+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";
+};

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?

This is something part of DT core, and does not really belong to mxs-mmc
bindings, so remove it from the example.

quoted

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 66f792a..c6b56b4 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c

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

"helpers"

Fixed.  Thanks for the catch.

quoted

+		 */
+		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,
 #ifdef CONFIG_PM
 		.pm	= &mxs_mmc_pm_ops,
+		.of_match_table = mxs_mmc_dt_ids,
 #endif
 	},
 };

Rest looks fine, thanks.

Thanks, Chris.

-- 
Regards,
Shawn