[PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx

[PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx

[Shawn Guo]

The first patch copies platform data into driver private data, and
do not reference platform data after probe.  The second one removes
the uses of cpu_is_mx().  The third one makes a dt related fix on
sdhci-pltfm.c, and the last one adds actual device tree probe for
sdhci-esdhc-imx driver.

Changes since v2:
 * Add patch #1
 * Do not use Linux details of cd-type and wp-type for binding

Changes since v1:
 * Address review comments given by Grant

Shawn Guo (4):
      mmc: sdhci-esdhc-imx: do not reference platform data after probe
      mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
      mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
      mmc: sdhci-esdhc-imx: add device tree probe support

 .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |   34 +++++
 arch/arm/mach-imx/clock-imx25.c                    |    4 +-
 arch/arm/mach-imx/clock-imx35.c                    |    6 +-
 arch/arm/mach-mx5/clock-mx51-mx53.c                |   16 +-
 arch/arm/mach-mx5/mx51_efika.c                     |    4 +-
 .../plat-mxc/devices/platform-sdhci-esdhc-imx.c    |   17 ++-
 arch/arm/plat-mxc/include/mach/devices-common.h    |    1 +
 drivers/mmc/host/sdhci-esdhc-imx.c                 |  151 +++++++++++++++++---
 drivers/mmc/host/sdhci-pltfm.c                     |    3 +-
 9 files changed, 194 insertions(+), 42 deletions(-)

[PATCH v3 1/4] mmc: sdhci-esdhc-imx: do not reference platform data after probe

[Shawn Guo]

The patch copies platform data into pltfm_imx_data and reference
the data there than platform data after probe.

This work is inspired by Grant Likely and Troy Kisky.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Chris Ball <cjb@laptop.org>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 3cc6f61..0b0e62e 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -45,6 +45,7 @@
 struct pltfm_imx_data {
 	int flags;
 	u32 scratchpad;
+	struct esdhc_platform_data boarddata;
 };
 
 static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
@@ -69,8 +70,9 @@ static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, i
 
 static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 {
-	struct esdhc_platform_data *boarddata =
-			host->mmc->parent->platform_data;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
 	/* fake CARD_PRESENT flag */
 	u32 val = readl(host->ioaddr + reg);
@@ -92,8 +94,7 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	struct esdhc_platform_data *boarddata =
-			host->mmc->parent->platform_data;
+	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
 	if (unlikely((reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)
 			&& (boarddata->cd_type == ESDHC_CD_GPIO)))
@@ -210,8 +211,9 @@ static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
 
 static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
 {
-	struct esdhc_platform_data *boarddata =
-			host->mmc->parent->platform_data;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
 	switch (boarddata->wp_type) {
 	case ESDHC_WP_GPIO:
@@ -291,12 +293,14 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51()))
 		imx_data->flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
 
-	boarddata = host->mmc->parent->platform_data;
-	if (!boarddata) {
+	if (!host->mmc->parent->platform_data) {
 		dev_err(mmc_dev(host->mmc), "no board data!\n");
 		err = -EINVAL;
 		goto no_board_data;
 	}
+	imx_data->boarddata = *((struct esdhc_platform_data *)
+				host->mmc->parent->platform_data);
+	boarddata = &imx_data->boarddata;
 
 	/* write_protect */
 	if (boarddata->wp_type == ESDHC_WP_GPIO) {
@@ -373,8 +377,8 @@ static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct esdhc_platform_data *boarddata = host->mmc->parent->platform_data;
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
 
 	sdhci_remove_host(host, dead);
-- 
1.7.4.1

[PATCH v3 2/4] mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()

[Shawn Guo]

The patch removes all the uses of cpu_is_mx().  Instead, it utilizes
platform_device_id to distinguish the esdhc differences among SoCs.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Chris Ball <cjb@laptop.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
 arch/arm/mach-imx/clock-imx25.c                    |    4 +-
 arch/arm/mach-imx/clock-imx35.c                    |    6 +-
 arch/arm/mach-mx5/clock-mx51-mx53.c                |   16 +++---
 arch/arm/mach-mx5/mx51_efika.c                     |    4 +-
 .../plat-mxc/devices/platform-sdhci-esdhc-imx.c    |   17 +++---
 arch/arm/plat-mxc/include/mach/devices-common.h    |    1 +
 drivers/mmc/host/sdhci-esdhc-imx.c                 |   60 ++++++++++++++++++-
 7 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c
index 42ef76a..9bb9062 100644
--- a/arch/arm/mach-imx/clock-imx25.c
+++ b/arch/arm/mach-imx/clock-imx25.c
@@ -301,8 +301,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("imx2-wdt.0", NULL, wdt_clk)
 	_REGISTER_CLOCK("imx-ssi.0", NULL, ssi1_clk)
 	_REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx25.0", NULL, esdhc1_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx25.1", NULL, esdhc2_clk)
 	_REGISTER_CLOCK("mx2-camera.0", NULL, csi_clk)
 	_REGISTER_CLOCK(NULL, "audmux", audmux_clk)
 	_REGISTER_CLOCK("flexcan.0", NULL, can1_clk)
diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
index b44cb06..2d0b4c8 100644
--- a/arch/arm/mach-imx/clock-imx35.c
+++ b/arch/arm/mach-imx/clock-imx35.c
@@ -458,9 +458,9 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("imx-epit.0", NULL, epit1_clk)
 	_REGISTER_CLOCK("imx-epit.1", NULL, epit2_clk)
 	_REGISTER_CLOCK(NULL, "esai", esai_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx35.0", NULL, esdhc1_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx35.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx35.2", NULL, esdhc3_clk)
 	_REGISTER_CLOCK("fec.0", NULL, fec_clk)
 	_REGISTER_CLOCK(NULL, "gpio", gpio1_clk)
 	_REGISTER_CLOCK(NULL, "gpio", gpio2_clk)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 6634f6c..0e1aa03 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1454,10 +1454,10 @@ static struct clk_lookup mx51_lookups[] = {
 	_REGISTER_CLOCK("imx51-ecspi.0", NULL, ecspi1_clk)
 	_REGISTER_CLOCK("imx51-ecspi.1", NULL, ecspi2_clk)
 	_REGISTER_CLOCK("imx51-cspi.0", NULL, cspi_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx51.0", NULL, esdhc1_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx51.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx51.2", NULL, esdhc3_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx51.3", NULL, esdhc4_clk)
 	_REGISTER_CLOCK(NULL, "cpu_clk", cpu_clk)
 	_REGISTER_CLOCK(NULL, "iim_clk", iim_clk)
 	_REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
@@ -1482,10 +1482,10 @@ static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk)
 	_REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk)
 	_REGISTER_CLOCK("imx-i2c.2", NULL, i2c3_mx53_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_mx53_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_mx53_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx53.0", NULL, esdhc1_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx53.1", NULL, esdhc2_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx53.2", NULL, esdhc3_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx53.3", NULL, esdhc4_mx53_clk)
 	_REGISTER_CLOCK("imx53-ecspi.0", NULL, ecspi1_clk)
 	_REGISTER_CLOCK("imx53-ecspi.1", NULL, ecspi2_clk)
 	_REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk)
diff --git a/arch/arm/mach-mx5/mx51_efika.c b/arch/arm/mach-mx5/mx51_efika.c
index 56739c2..4435e03 100644
--- a/arch/arm/mach-mx5/mx51_efika.c
+++ b/arch/arm/mach-mx5/mx51_efika.c
@@ -260,8 +260,8 @@ static struct regulator_consumer_supply vvideo_consumers[] = {
 };
 
 static struct regulator_consumer_supply vsd_consumers[] = {
-	REGULATOR_SUPPLY("vmmc", "sdhci-esdhc-imx.0"),
-	REGULATOR_SUPPLY("vmmc", "sdhci-esdhc-imx.1"),
+	REGULATOR_SUPPLY("vmmc", "sdhci-esdhc-imx51.0"),
+	REGULATOR_SUPPLY("vmmc", "sdhci-esdhc-imx51.1"),
 };
 
 static struct regulator_consumer_supply pwgt1_consumer[] = {
diff --git a/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c b/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c
index 79d6d71..5955f5d 100644
--- a/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c
+++ b/arch/arm/plat-mxc/devices/platform-sdhci-esdhc-imx.c
@@ -10,21 +10,22 @@
 #include <mach/devices-common.h>
 #include <mach/esdhc.h>
 
-#define imx_sdhci_esdhc_imx_data_entry_single(soc, _id, hwid) \
+#define imx_sdhci_esdhc_imx_data_entry_single(soc, _devid, _id, hwid) \
 	{								\
+		.devid = _devid,					\
 		.id = _id,						\
 		.iobase = soc ## _ESDHC ## hwid ## _BASE_ADDR,	\
 		.irq = soc ## _INT_ESDHC ## hwid,			\
 	}
 
-#define imx_sdhci_esdhc_imx_data_entry(soc, id, hwid)	\
-	[id] = imx_sdhci_esdhc_imx_data_entry_single(soc, id, hwid)
+#define imx_sdhci_esdhc_imx_data_entry(soc, devid, id, hwid)	\
+	[id] = imx_sdhci_esdhc_imx_data_entry_single(soc, devid, id, hwid)
 
 #ifdef CONFIG_SOC_IMX25
 const struct imx_sdhci_esdhc_imx_data
 imx25_sdhci_esdhc_imx_data[] __initconst = {
 #define imx25_sdhci_esdhc_imx_data_entry(_id, _hwid)			\
-	imx_sdhci_esdhc_imx_data_entry(MX25, _id, _hwid)
+	imx_sdhci_esdhc_imx_data_entry(MX25, "sdhci-esdhc-imx25", _id, _hwid)
 	imx25_sdhci_esdhc_imx_data_entry(0, 1),
 	imx25_sdhci_esdhc_imx_data_entry(1, 2),
 };
@@ -34,7 +35,7 @@ imx25_sdhci_esdhc_imx_data[] __initconst = {
 const struct imx_sdhci_esdhc_imx_data
 imx35_sdhci_esdhc_imx_data[] __initconst = {
 #define imx35_sdhci_esdhc_imx_data_entry(_id, _hwid)			\
-	imx_sdhci_esdhc_imx_data_entry(MX35, _id, _hwid)
+	imx_sdhci_esdhc_imx_data_entry(MX35, "sdhci-esdhc-imx35", _id, _hwid)
 	imx35_sdhci_esdhc_imx_data_entry(0, 1),
 	imx35_sdhci_esdhc_imx_data_entry(1, 2),
 	imx35_sdhci_esdhc_imx_data_entry(2, 3),
@@ -45,7 +46,7 @@ imx35_sdhci_esdhc_imx_data[] __initconst = {
 const struct imx_sdhci_esdhc_imx_data
 imx51_sdhci_esdhc_imx_data[] __initconst = {
 #define imx51_sdhci_esdhc_imx_data_entry(_id, _hwid)			\
-	imx_sdhci_esdhc_imx_data_entry(MX51, _id, _hwid)
+	imx_sdhci_esdhc_imx_data_entry(MX51, "sdhci-esdhc-imx51", _id, _hwid)
 	imx51_sdhci_esdhc_imx_data_entry(0, 1),
 	imx51_sdhci_esdhc_imx_data_entry(1, 2),
 	imx51_sdhci_esdhc_imx_data_entry(2, 3),
@@ -57,7 +58,7 @@ imx51_sdhci_esdhc_imx_data[] __initconst = {
 const struct imx_sdhci_esdhc_imx_data
 imx53_sdhci_esdhc_imx_data[] __initconst = {
 #define imx53_sdhci_esdhc_imx_data_entry(_id, _hwid)			\
-	imx_sdhci_esdhc_imx_data_entry(MX53, _id, _hwid)
+	imx_sdhci_esdhc_imx_data_entry(MX53, "sdhci-esdhc-imx53", _id, _hwid)
 	imx53_sdhci_esdhc_imx_data_entry(0, 1),
 	imx53_sdhci_esdhc_imx_data_entry(1, 2),
 	imx53_sdhci_esdhc_imx_data_entry(2, 3),
@@ -93,6 +94,6 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
 	if (!pdata)
 		pdata = &default_esdhc_pdata;
 
-	return imx_add_platform_device("sdhci-esdhc-imx", data->id, res,
+	return imx_add_platform_device(data->devid, data->id, res,
 			ARRAY_SIZE(res), pdata, sizeof(*pdata));
 }
diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h
index bf93820..c9e7b66 100644
--- a/arch/arm/plat-mxc/include/mach/devices-common.h
+++ b/arch/arm/plat-mxc/include/mach/devices-common.h
@@ -276,6 +276,7 @@ struct platform_device *__init imx_add_mxc_w1(
 
 #include <mach/esdhc.h>
 struct imx_sdhci_esdhc_imx_data {
+	const char *devid;
 	int id;
 	resource_size_t iobase;
 	resource_size_t irq;
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 0b0e62e..f3efb3b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -20,7 +20,6 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/sdio.h>
-#include <mach/hardware.h>
 #include <mach/esdhc.h>
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
@@ -42,12 +41,58 @@
  */
 #define ESDHC_FLAG_MULTIBLK_NO_INT	(1 << 1)
 
+enum imx_esdhc_type {
+	IMX25_ESDHC,
+	IMX35_ESDHC,
+	IMX51_ESDHC,
+	IMX53_ESDHC,
+};
+
 struct pltfm_imx_data {
 	int flags;
 	u32 scratchpad;
+	enum imx_esdhc_type devtype;
 	struct esdhc_platform_data boarddata;
 };
 
+static struct platform_device_id imx_esdhc_devtype[] = {
+	{
+		.name = "sdhci-esdhc-imx25",
+		.driver_data = IMX25_ESDHC,
+	}, {
+		.name = "sdhci-esdhc-imx35",
+		.driver_data = IMX35_ESDHC,
+	}, {
+		.name = "sdhci-esdhc-imx51",
+		.driver_data = IMX51_ESDHC,
+	}, {
+		.name = "sdhci-esdhc-imx53",
+		.driver_data = IMX53_ESDHC,
+	}, {
+		/* sentinel */
+	}
+};
+
+static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
+{
+	return data->devtype == IMX25_ESDHC;
+}
+
+static inline int is_imx35_esdhc(struct pltfm_imx_data *data)
+{
+	return data->devtype == IMX35_ESDHC;
+}
+
+static inline int is_imx51_esdhc(struct pltfm_imx_data *data)
+{
+	return data->devtype == IMX51_ESDHC;
+}
+
+static inline int is_imx53_esdhc(struct pltfm_imx_data *data)
+{
+	return data->devtype == IMX53_ESDHC;
+}
+
 static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
 {
 	/* Force 2048 bytes, which is the maximum supported size in SDHCI. */
@@ -56,8 +101,12 @@ static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
 
 static unsigned int esdhc_pltfm_get_max_blk_count(struct sdhci_host *host)
 {
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+
 	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
-	return (cpu_is_mx25() || cpu_is_mx35()) ? 1 : 65535;
+	return (is_imx25_esdhc(imx_data) || is_imx35_esdhc(imx_data)) ?
+		1 : 65535;
 }
 
 static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
@@ -276,6 +325,8 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
 	if (!imx_data)
 		return -ENOMEM;
+
+	imx_data->devtype = pdev->id_entry->driver_data;
 	pltfm_host->priv = imx_data;
 
 	clk = clk_get(mmc_dev(host->mmc), NULL);
@@ -287,10 +338,10 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	clk_enable(clk);
 	pltfm_host->clk = clk;
 
-	if (!cpu_is_mx25())
+	if (!is_imx25_esdhc(imx_data))
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
-	if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51()))
+	if (is_imx53_esdhc(imx_data))
 		imx_data->flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
 
 	if (!host->mmc->parent->platform_data) {
@@ -405,6 +456,7 @@ static struct platform_driver sdhci_esdhc_imx_driver = {
 		.name	= "sdhci-esdhc-imx",
 		.owner	= THIS_MODULE,
 	},
+	.id_table	= imx_esdhc_devtype,
 	.probe		= sdhci_esdhc_imx_probe,
 	.remove		= __devexit_p(sdhci_esdhc_imx_remove),
 #ifdef CONFIG_PM
-- 
1.7.4.1

[PATCH v3 3/4] mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host

[Shawn Guo]

Neither platform based nor dt based device needs to pass the parent
to sdhci_alloc_host.  There is no difference between platform and dt
on this point.

The patch makes the change to pass device itself than its parent to
sdhci_alloc_host for dt case too.  Otherwise the probe function of
sdhci based drivers which is shared between platform and dt will
fail on dt case.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Chris Ball <cjb@laptop.org>
---
 drivers/mmc/host/sdhci-pltfm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 71c0ce1..6414efe 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -85,6 +85,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 {
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *iomem;
 	int ret;
 
@@ -98,7 +99,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		dev_err(&pdev->dev, "Invalid iomem size!\n");
 
 	/* Some PCI-based MFD need the parent here */
-	if (pdev->dev.parent != &platform_bus)
+	if (pdev->dev.parent != &platform_bus && !np)
 		host = sdhci_alloc_host(pdev->dev.parent, sizeof(*pltfm_host));
 	else
 		host = sdhci_alloc_host(&pdev->dev, sizeof(*pltfm_host));
-- 
1.7.4.1

[PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Shawn Guo]

The patch adds device tree probe support for sdhci-esdhc-imx driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Chris Ball <cjb@laptop.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |   34 +++++++++
 drivers/mmc/host/sdhci-esdhc-imx.c                 |   77 +++++++++++++++++---
 2 files changed, 102 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
new file mode 100644
index 0000000..ab22fe6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -0,0 +1,34 @@
+* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
+
+The Enhanced Secure Digital Host Controller on Freescale i.MX family
+provides an interface for MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible : Should be "fsl,<chip>-esdhc"
+- reg : Should contain eSDHC registers location and length
+- interrupts : Should contain eSDHC interrupt
+
+Optional properties:
+- fsl,card-wired : Indicate the card is wired to host permanently
+- fsl,cd-internal : Indicate to use controller internal card detection
+- fsl,wp-internal : Indicate to use controller internal write protection
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
+
+Examples:
+
+esdhc@70004000 {
+	compatible = "fsl,imx51-esdhc";
+	reg = <0x70004000 0x4000>;
+	interrupts = <1>;
+	fsl,cd-internal;
+	fsl,wp-internal;
+};
+
+esdhc@70008000 {
+	compatible = "fsl,imx51-esdhc";
+	reg = <0x70008000 0x4000>;
+	interrupts = <2>;
+	cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
+	wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
+};
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f3efb3b..cbdd91f 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -20,6 +20,9 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/sdio.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <mach/esdhc.h>
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
@@ -73,6 +76,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
 	}
 };
 
+static const struct of_device_id imx_esdhc_dt_ids[] = {
+	{ .compatible = "fsl,imx25-esdhc", .data = &imx_esdhc_devtype[IMX25_ESDHC], },
+	{ .compatible = "fsl,imx35-esdhc", .data = &imx_esdhc_devtype[IMX35_ESDHC], },
+	{ .compatible = "fsl,imx51-esdhc", .data = &imx_esdhc_devtype[IMX51_ESDHC], },
+	{ .compatible = "fsl,imx53-esdhc", .data = &imx_esdhc_devtype[IMX53_ESDHC], },
+	{ /* sentinel */ }
+};
+
 static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
 {
 	return data->devtype == IMX25_ESDHC;
@@ -307,8 +318,48 @@ static irqreturn_t cd_irq(int irq, void *data)
 	return IRQ_HANDLED;
 };
 
+#ifdef CONFIG_OF
+static int __devinit
+sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
+			 struct esdhc_platform_data *boarddata)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (!np)
+		return -ENODEV;
+
+	if (of_get_property(np, "fsl,card-wired", NULL))
+		boarddata->cd_type = ESDHC_CD_PERMANENT;
+
+	if (of_get_property(np, "fsl,cd-controller", NULL))
+		boarddata->cd_type = ESDHC_CD_CONTROLLER;
+
+	if (of_get_property(np, "fsl,wp-controller", NULL))
+		boarddata->wp_type = ESDHC_WP_CONTROLLER;
+
+	boarddata->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
+	if (gpio_is_valid(boarddata->cd_gpio))
+		boarddata->cd_type = ESDHC_CD_GPIO;
+
+	boarddata->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
+	if (gpio_is_valid(boarddata->wp_gpio))
+		boarddata->wp_type = ESDHC_WP_GPIO;
+
+	return 0;
+}
+#else
+static inline int
+sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
+			 struct esdhc_platform_data *boarddata)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+			of_match_device(imx_esdhc_dt_ids, &pdev->dev);
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_host *host;
 	struct esdhc_platform_data *boarddata;
@@ -323,9 +374,13 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	pltfm_host = sdhci_priv(host);
 
 	imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
-	if (!imx_data)
-		return -ENOMEM;
+	if (!imx_data) {
+		err = -ENOMEM;
+		goto err_imx_data;
+	}
 
+	if (of_id)
+		pdev->id_entry = of_id->data;
 	imx_data->devtype = pdev->id_entry->driver_data;
 	pltfm_host->priv = imx_data;
 
@@ -344,14 +399,16 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (is_imx53_esdhc(imx_data))
 		imx_data->flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
 
-	if (!host->mmc->parent->platform_data) {
-		dev_err(mmc_dev(host->mmc), "no board data!\n");
-		err = -EINVAL;
-		goto no_board_data;
-	}
-	imx_data->boarddata = *((struct esdhc_platform_data *)
-				host->mmc->parent->platform_data);
 	boarddata = &imx_data->boarddata;
+	if (sdhci_esdhc_imx_probe_dt(pdev, boarddata) < 0) {
+		if (!host->mmc->parent->platform_data) {
+			dev_err(mmc_dev(host->mmc), "no board data!\n");
+			err = -EINVAL;
+			goto no_board_data;
+		}
+		imx_data->boarddata = *((struct esdhc_platform_data *)
+					host->mmc->parent->platform_data);
+	}
 
 	/* write_protect */
 	if (boarddata->wp_type == ESDHC_WP_GPIO) {
@@ -420,6 +477,7 @@ no_board_data:
 	clk_put(pltfm_host->clk);
 err_clk_get:
 	kfree(imx_data);
+err_imx_data:
 	sdhci_pltfm_free(pdev);
 	return err;
 }
@@ -455,6 +513,7 @@ static struct platform_driver sdhci_esdhc_imx_driver = {
 	.driver		= {
 		.name	= "sdhci-esdhc-imx",
 		.owner	= THIS_MODULE,
+		.of_match_table = imx_esdhc_dt_ids,
 	},
 	.id_table	= imx_esdhc_devtype,
 	.probe		= sdhci_esdhc_imx_probe,
-- 
1.7.4.1

Re: [PATCH v3 1/4] mmc: sdhci-esdhc-imx: do not reference platform data after probe

[Grant Likely]

On Thu, Jul 07, 2011 at 12:47:47AM +0800, Shawn Guo wrote:
> The patch copies platform data into pltfm_imx_data and reference
> the data there than platform data after probe.
> 
> This work is inspired by Grant Likely and Troy Kisky.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Chris Ball <cjb@laptop.org>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   22 +++++++++++++---------
>  1 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 3cc6f61..0b0e62e 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -45,6 +45,7 @@
>  struct pltfm_imx_data {
>  	int flags;
>  	u32 scratchpad;
> +	struct esdhc_platform_data boarddata;
>  };
>  
>  static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
> @@ -69,8 +70,9 @@ static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, i
>  
>  static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
>  {
> -	struct esdhc_platform_data *boarddata =
> -			host->mmc->parent->platform_data;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct pltfm_imx_data *imx_data = pltfm_host->priv;
> +	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
>  
>  	/* fake CARD_PRESENT flag */
>  	u32 val = readl(host->ioaddr + reg);
> @@ -92,8 +94,7 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct pltfm_imx_data *imx_data = pltfm_host->priv;
> -	struct esdhc_platform_data *boarddata =
> -			host->mmc->parent->platform_data;
> +	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
>  
>  	if (unlikely((reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)
>  			&& (boarddata->cd_type == ESDHC_CD_GPIO)))
> @@ -210,8 +211,9 @@ static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
>  
>  static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
>  {
> -	struct esdhc_platform_data *boarddata =
> -			host->mmc->parent->platform_data;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct pltfm_imx_data *imx_data = pltfm_host->priv;
> +	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
>  
>  	switch (boarddata->wp_type) {
>  	case ESDHC_WP_GPIO:
> @@ -291,12 +293,14 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51()))
>  		imx_data->flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
>  
> -	boarddata = host->mmc->parent->platform_data;
> -	if (!boarddata) {
> +	if (!host->mmc->parent->platform_data) {
>  		dev_err(mmc_dev(host->mmc), "no board data!\n");
>  		err = -EINVAL;
>  		goto no_board_data;
>  	}
> +	imx_data->boarddata = *((struct esdhc_platform_data *)
> +				host->mmc->parent->platform_data);
> +	boarddata = &imx_data->boarddata;
>  
>  	/* write_protect */
>  	if (boarddata->wp_type == ESDHC_WP_GPIO) {
> @@ -373,8 +377,8 @@ static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
>  {
>  	struct sdhci_host *host = platform_get_drvdata(pdev);
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct esdhc_platform_data *boarddata = host->mmc->parent->platform_data;
>  	struct pltfm_imx_data *imx_data = pltfm_host->priv;
> +	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
>  	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
>  
>  	sdhci_remove_host(host, dead);
> -- 
> 1.7.4.1
> 
> 

Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Grant Likely]

On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
> The patch adds device tree probe support for sdhci-esdhc-imx driver.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |   34 +++++++++
>  drivers/mmc/host/sdhci-esdhc-imx.c                 |   77 +++++++++++++++++---
>  2 files changed, 102 insertions(+), 9 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> new file mode 100644
> index 0000000..ab22fe6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> @@ -0,0 +1,34 @@
> +* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
> +
> +The Enhanced Secure Digital Host Controller on Freescale i.MX family
> +provides an interface for MMC, SD, and SDIO types of memory cards.
> +
> +Required properties:
> +- compatible : Should be "fsl,<chip>-esdhc"
> +- reg : Should contain eSDHC registers location and length
> +- interrupts : Should contain eSDHC interrupt
> +
> +Optional properties:
> +- fsl,card-wired : Indicate the card is wired to host permanently
> +- fsl,cd-internal : Indicate to use controller internal card detection
> +- fsl,wp-internal : Indicate to use controller internal write protection
> +- cd-gpios : Specify GPIOs for card detection
> +- wp-gpios : Specify GPIOs for write protection
> +
> +Examples:
> +
> +esdhc@70004000 {
> +	compatible = "fsl,imx51-esdhc";
> +	reg = <0x70004000 0x4000>;
> +	interrupts = <1>;
> +	fsl,cd-internal;
> +	fsl,wp-internal;
> +};
> +
> +esdhc@70008000 {
> +	compatible = "fsl,imx51-esdhc";
> +	reg = <0x70008000 0x4000>;
> +	interrupts = <2>;
> +	cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
> +	wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
> +};
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index f3efb3b..cbdd91f 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -20,6 +20,9 @@
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/mmc.h>
>  #include <linux/mmc/sdio.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
>  #include <mach/esdhc.h>
>  #include "sdhci-pltfm.h"
>  #include "sdhci-esdhc.h"
> @@ -73,6 +76,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
>  	}
>  };
>  
> +static const struct of_device_id imx_esdhc_dt_ids[] = {
> +	{ .compatible = "fsl,imx25-esdhc", .data = &imx_esdhc_devtype[IMX25_ESDHC], },
> +	{ .compatible = "fsl,imx35-esdhc", .data = &imx_esdhc_devtype[IMX35_ESDHC], },
> +	{ .compatible = "fsl,imx51-esdhc", .data = &imx_esdhc_devtype[IMX51_ESDHC], },
> +	{ .compatible = "fsl,imx53-esdhc", .data = &imx_esdhc_devtype[IMX53_ESDHC], },
> +	{ /* sentinel */ }
> +};
> +
>  static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
>  {
>  	return data->devtype == IMX25_ESDHC;
> @@ -307,8 +318,48 @@ static irqreturn_t cd_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  };
>  
> +#ifdef CONFIG_OF
> +static int __devinit
> +sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
> +			 struct esdhc_platform_data *boarddata)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	if (of_get_property(np, "fsl,card-wired", NULL))
> +		boarddata->cd_type = ESDHC_CD_PERMANENT;
> +
> +	if (of_get_property(np, "fsl,cd-controller", NULL))
> +		boarddata->cd_type = ESDHC_CD_CONTROLLER;
> +
> +	if (of_get_property(np, "fsl,wp-controller", NULL))
> +		boarddata->wp_type = ESDHC_WP_CONTROLLER;
> +
> +	boarddata->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
> +	if (gpio_is_valid(boarddata->cd_gpio))
> +		boarddata->cd_type = ESDHC_CD_GPIO;
> +
> +	boarddata->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
> +	if (gpio_is_valid(boarddata->wp_gpio))
> +		boarddata->wp_type = ESDHC_WP_GPIO;
> +
> +	return 0;
> +}
> +#else
> +static inline int
> +sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
> +			 struct esdhc_platform_data *boarddata)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>  static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *of_id =
> +			of_match_device(imx_esdhc_dt_ids, &pdev->dev);
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_host *host;
>  	struct esdhc_platform_data *boarddata;
> @@ -323,9 +374,13 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	pltfm_host = sdhci_priv(host);
>  
>  	imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
> -	if (!imx_data)
> -		return -ENOMEM;
> +	if (!imx_data) {
> +		err = -ENOMEM;
> +		goto err_imx_data;
> +	}
>  
> +	if (of_id)
> +		pdev->id_entry = of_id->data;
>  	imx_data->devtype = pdev->id_entry->driver_data;
>  	pltfm_host->priv = imx_data;
>  
> @@ -344,14 +399,16 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	if (is_imx53_esdhc(imx_data))
>  		imx_data->flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
>  
> -	if (!host->mmc->parent->platform_data) {
> -		dev_err(mmc_dev(host->mmc), "no board data!\n");
> -		err = -EINVAL;
> -		goto no_board_data;
> -	}
> -	imx_data->boarddata = *((struct esdhc_platform_data *)
> -				host->mmc->parent->platform_data);
>  	boarddata = &imx_data->boarddata;
> +	if (sdhci_esdhc_imx_probe_dt(pdev, boarddata) < 0) {
> +		if (!host->mmc->parent->platform_data) {
> +			dev_err(mmc_dev(host->mmc), "no board data!\n");
> +			err = -EINVAL;
> +			goto no_board_data;
> +		}
> +		imx_data->boarddata = *((struct esdhc_platform_data *)
> +					host->mmc->parent->platform_data);
> +	}
>  
>  	/* write_protect */
>  	if (boarddata->wp_type == ESDHC_WP_GPIO) {
> @@ -420,6 +477,7 @@ no_board_data:
>  	clk_put(pltfm_host->clk);
>  err_clk_get:
>  	kfree(imx_data);
> +err_imx_data:
>  	sdhci_pltfm_free(pdev);
>  	return err;
>  }
> @@ -455,6 +513,7 @@ static struct platform_driver sdhci_esdhc_imx_driver = {
>  	.driver		= {
>  		.name	= "sdhci-esdhc-imx",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = imx_esdhc_dt_ids,
>  	},
>  	.id_table	= imx_esdhc_devtype,
>  	.probe		= sdhci_esdhc_imx_probe,
> -- 
> 1.7.4.1
> 
> 

Re: [PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx

[Chris Ball]

Hi Shawn,

On Wed, Jul 06 2011, Shawn Guo wrote:
> The first patch copies platform data into driver private data, and
> do not reference platform data after probe.  The second one removes
> the uses of cpu_is_mx().  The third one makes a dt related fix on
> sdhci-pltfm.c, and the last one adds actual device tree probe for
> sdhci-esdhc-imx driver.
>
> Changes since v2:
>  * Add patch #1
>  * Do not use Linux details of cd-type and wp-type for binding
>
> Changes since v1:
>  * Address review comments given by Grant
>
> Shawn Guo (4):
>       mmc: sdhci-esdhc-imx: do not reference platform data after probe
>       mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
>       mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
>       mmc: sdhci-esdhc-imx: add device tree probe support

These don't apply against mmc-next -- what is your tree based on?

Thanks,

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

Re: [PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx

[Shawn Guo]

Hi Chris,

On Sat, Jul 09, 2011 at 06:14:43PM -0400, Chris Ball wrote:
> Hi Shawn,
> 
> On Wed, Jul 06 2011, Shawn Guo wrote:
> > The first patch copies platform data into driver private data, and
> > do not reference platform data after probe.  The second one removes
> > the uses of cpu_is_mx().  The third one makes a dt related fix on
> > sdhci-pltfm.c, and the last one adds actual device tree probe for
> > sdhci-esdhc-imx driver.
> >
> > Changes since v2:
> >  * Add patch #1
> >  * Do not use Linux details of cd-type and wp-type for binding
> >
> > Changes since v1:
> >  * Address review comments given by Grant
> >
> > Shawn Guo (4):
> >       mmc: sdhci-esdhc-imx: do not reference platform data after probe
> >       mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
> >       mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
> >       mmc: sdhci-esdhc-imx: add device tree probe support
> 
> These don't apply against mmc-next -- what is your tree based on?
> 
It's based on linux-next with following patch applied.

[PATCH v4 4/4] mmc: sdhci-esdhc-imx: extend card_detect and write_protect support for mx5
http://permalink.gmane.org/gmane.linux.kernel.mmc/8748

You applied the 3 of 4 patches in that series, and expect Sascha to
send this one through his tree after you send those 3.

Now, we got another series pending in the line.

I'm wondering if it works that I create a branch off your mmc-next,
merge Sascha's next, apply my patches on top it, and then send a pull
request for you (and Sascha) to pull the branch.

-- 
Regards,
Shawn

Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Anton Vorontsov]

Hi,

On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
> On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
> > The patch adds device tree probe support for sdhci-esdhc-imx driver.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > Cc: Chris Ball <cjb@laptop.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
[...]
> > +Optional properties:
> > +- fsl,card-wired : Indicate the card is wired to host permanently
> > +- fsl,cd-internal : Indicate to use controller internal card detection
> > +- fsl,wp-internal : Indicate to use controller internal write protection
> > +- cd-gpios : Specify GPIOs for card detection
> > +- wp-gpios : Specify GPIOs for write protection
[...]
> > +	cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
> > +	wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */

Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.

Just a few disadvantages off the top of my head:

- You can't statically validate your device tree for correctness.
  Today dtc checks #{address,size}-cells sanity for 'regs' and
  'ranges';
- You can't automatically convert named resources into 'struct
  resource *', as we do for platform devices now;
- Any pros for using named resources in the device tree? I don't
  see any.

So, I suggest to at least discuss this stuff a little bit more
before polluting device trees with dubious ideas.

p.s.

As for this particular patch, I really don't see why we should
use named GPIOs. For consistency, I'd suggest to reuse bindings
from Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt.

Plus, fsl,cd-internal and fsl,wp-internal is not needed: either
you specify GPIOs or not. That already signals whether driver
should use internal detection (i.e. 'present' register) or
external (i.e. GPIO).

And also, why {cd,wp}-gpioS (plural)?

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Scott Wood]

On Wed, 13 Jul 2011 19:52:12 +0400
Anton Vorontsov <cbouatmailru@gmail.com> wrote:

> On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
> > On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
> > > +- cd-gpios : Specify GPIOs for card detection
> > > +- wp-gpios : Specify GPIOs for write protection
> [...]
> > > +	cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
> > > +	wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
> 
> Is there any particular reason why we started using named GPIOs
> in the device tree? To me, that's quite drastic change... should
> we start using named regs and interrupts as well? Personally, I
> don't think that the idea is great, as now you don't know where
> to expect memory resources, interrupt resources and, eventually
> GPIO resources.

Just check the binding, and you'll know. :-)

It makes it easier to deal with cases where some resources are optional,
especially when dealing with a binding for a family of devices that grows
over time, and helps avoid resources being mismatched since order no longer
matters.

> Just a few disadvantages off the top of my head:
> 
> - You can't statically validate your device tree for correctness.
>   Today dtc checks #{address,size}-cells sanity for 'regs' and
>   'ranges';

The vast majority of stuff in the device tree already cannot be checked in
this manner, without adding binding knowledge to dtc.

Perhaps it could check things that end in "-gpios", "-regs", etc., if we
avoid adding new properties that match those patterns that aren't what they
appear to be, and let dtc know about any existing cases.

> - You can't automatically convert named resources into 'struct
>   resource *', as we do for platform devices now;

So add named resource support to platform devices. :-)

-Scott

Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Anton Vorontsov]

On Wed, Jul 13, 2011 at 11:09:40AM -0500, Scott Wood wrote:
[...]
> > - You can't automatically convert named resources into 'struct
> >   resource *', as we do for platform devices now;
> 
> So add named resource support to platform devices. :-)

There are surely no technical difficulties, it's more about 'type
safety', and as a consequence -- resources being discoverable.

If we want to use named resources, I think we should explicitly reserve
the *-{regs,interrupts,gpios} suffixes for such purposes. And once such
a reservation is done, we're safe again -- resources become discoverable.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Grant Likely]

On Thu, Jul 14, 2011 at 12:52 AM, Anton Vorontsov
<cbouatmailru@gmail.com> wrote:
> Hi,
>
> On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
>> On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
>> > The patch adds device tree probe support for sdhci-esdhc-imx driver.
>> >
>> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>> > Cc: Wolfram Sang <w.sang@pengutronix.de>
>> > Cc: Chris Ball <cjb@laptop.org>
>> > Cc: Grant Likely <grant.likely@secretlab.ca>
>>
>> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> [...]
>> > +Optional properties:
>> > +- fsl,card-wired : Indicate the card is wired to host permanently
>> > +- fsl,cd-internal : Indicate to use controller internal card detection
>> > +- fsl,wp-internal : Indicate to use controller internal write protection
>> > +- cd-gpios : Specify GPIOs for card detection
>> > +- wp-gpios : Specify GPIOs for write protection
> [...]
>> > +   cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
>> > +   wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
>
> Is there any particular reason why we started using named GPIOs
> in the device tree? To me, that's quite drastic change... should
> we start using named regs and interrupts as well? Personally, I
> don't think that the idea is great, as now you don't know where
> to expect memory resources, interrupt resources and, eventually
> GPIO resources.

No, there are no plans to move to named irqs or regs.  irq and reg
resources tend to be a lot more regular than gpios.  It's not a
drastic change though because existing bindings remain unaffected, and
new bindings can still use unnamed gpios if that is more appropriate.

>
> Just a few disadvantages off the top of my head:
>
> - You can't statically validate your device tree for correctness.
>  Today dtc checks #{address,size}-cells sanity for 'regs' and
>  'ranges';

We can. The gpio binding was extended to be in the form
[<name>-]gpios.  Any property with the string "gpios" at the end can
be statically validated.

> - You can't automatically convert named resources into 'struct
>  resource *', as we do for platform devices now;

Only for irqs and regs.  gpios have never been automatically loaded
into resources.

> - Any pros for using named resources in the device tree? I don't
>  see any.

Human readability.  To know exactly what a gpio is intended to be used
for.  Particularly for the case where a device might not use all the
gpios that it could use.  Yes, the gpios property can have 'holes' in
it, but the observation was made by several people that it is easy to
get wrong.  I for one thing the concern was well justified.

> So, I suggest to at least discuss this stuff a little bit more
> before polluting device trees with dubious ideas.

It was discussed on list quite a while ago.

>
> p.s.
>
> As for this particular patch, I really don't see why we should
> use named GPIOs. For consistency, I'd suggest to reuse bindings
> from Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt.
>
> Plus, fsl,cd-internal and fsl,wp-internal is not needed: either
> you specify GPIOs or not. That already signals whether driver
> should use internal detection (i.e. 'present' register) or
> external (i.e. GPIO).

wp-internal and cd-internal differentiates between using the internal
functionality and not having wp/cd at all.

>
> And also, why {cd,wp}-gpioS (plural)?

For consistency.  Doing it that way means that the plural "gpios" is
always the suffix for both the "gpios" and "<name>-gpios" use cases.

>
> --
> Anton Vorontsov
> Email: cbouatmailru@gmail.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support

[Anton Vorontsov]

On Thu, Jul 14, 2011 at 01:36:39AM +0900, Grant Likely wrote:
[...]
> Only for irqs and regs.  gpios have never been automatically loaded
> into resources.

Which doesn't mean we wouldn't want it sooner or later.

> > - Any pros for using named resources in the device tree? I don't
> >  see any.
> 
> Human readability.  To know exactly what a gpio is intended to be used
> for.  Particularly for the case where a device might not use all the
> gpios that it could use.  Yes, the gpios property can have 'holes' in
> it, but the observation was made by several people that it is easy to
> get wrong.  I for one thing the concern was well justified.

The GPIO bindings are no harder to deal with than PCI memory bindings,
not even close to that complexity. So I don't really see why you try
to simplify GPIOs, but disagree on making the same for memory and
interrupt resources.

For example arch/powerpc/boot/dts/ebony.dts, 'mcmal' node has five
interrupts (txeob, rxeob, serr, txde, rxde). Or, gianfar nodes have
either three interrupts (tx, rx, err) or just one.

The average user of 'gpios' has 1-2 entries (the noticeable exception
is USB FHCI, which has 8 GPIOs).

I.e., I don't see how GPIOs are special. I'm all for consistency,
that's it. If that doesn't work for IRQs, then I want to understand
why so. And if you explain why named resources are no good for IRQs,
maybe I could use the same argument against named GPIOs? :-)

Or it could be otherwise: we agree that named resources are good, and
we should explicitly write when to use named and when to use anonymous
resources.

> > So, I suggest to at least discuss this stuff a little bit more
> > before polluting device trees with dubious ideas.
> 
> It was discussed on list quite a while ago.

I probably wasn't Cc'ed, can you point me to that thread?

The last time I was Cc'ed on a such discussion, we (well who cared
enough to 'vote') agreed* that we should wait with deploying named
GPIOs scheme, and discuss it later. And here we are.

The patch that added of_get_named_gpio() triggered no discussion
at all, but I wasn't Cc'ed either.

* http://lists.ozlabs.org/pipermail/linuxppc-dev/2008-October/064701.html

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx

[Chris Ball]

Hi Shawn, Sascha,

On Sat, Jul 09 2011, Shawn Guo wrote:
>> > Shawn Guo (4):
>> >       mmc: sdhci-esdhc-imx: do not reference platform data after probe
>> >       mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
>> >       mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
>> >       mmc: sdhci-esdhc-imx: add device tree probe support
>> 
>> These don't apply against mmc-next -- what is your tree based on?
>> 
> It's based on linux-next with following patch applied.
>
> [PATCH v4 4/4] mmc: sdhci-esdhc-imx: extend card_detect and
> write_protect support for mx5
> http://permalink.gmane.org/gmane.linux.kernel.mmc/8748
>
> You applied the 3 of 4 patches in that series, and expect Sascha to
> send this one through his tree after you send those 3.

I see.  In that case, either Sascha can send this new series with my:

Acked-by: Chris Ball <cjb@laptop.org>

or I'll send it as a second pull request after Sascha sends [4/4].

Thanks,

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

Re: [PATCH v3 0/4] Add device tree probe for sdhci-esdhc-imx

[Sascha Hauer]

On Mon, Jul 18, 2011 at 09:51:28PM -0400, Chris Ball wrote:
> Hi Shawn, Sascha,
> 
> On Sat, Jul 09 2011, Shawn Guo wrote:
> >> > Shawn Guo (4):
> >> >       mmc: sdhci-esdhc-imx: do not reference platform data after probe
> >> >       mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
> >> >       mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
> >> >       mmc: sdhci-esdhc-imx: add device tree probe support
> >> 
> >> These don't apply against mmc-next -- what is your tree based on?
> >> 
> > It's based on linux-next with following patch applied.
> >
> > [PATCH v4 4/4] mmc: sdhci-esdhc-imx: extend card_detect and
> > write_protect support for mx5
> > http://permalink.gmane.org/gmane.linux.kernel.mmc/8748
> >
> > You applied the 3 of 4 patches in that series, and expect Sascha to
> > send this one through his tree after you send those 3.

Shawn, please ping me once all dependencies have been merged. I have no
influence when my patches get merged since my patches go via Arnd.

> 
> I see.  In that case, either Sascha can send this new series with my:
> 
> Acked-by: Chris Ball <cjb@laptop.org>
> 
> or I'll send it as a second pull request after Sascha sends [4/4].
> 
> Thanks,
> 
> - Chris.
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> 

-- 
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 v3 0/4] Add device tree probe for sdhci-esdhc-imx

[Shawn Guo]

On Tue, Jul 19, 2011 at 10:10:27AM +0200, Sascha Hauer wrote:
> On Mon, Jul 18, 2011 at 09:51:28PM -0400, Chris Ball wrote:
> > Hi Shawn, Sascha,
> > 
> > On Sat, Jul 09 2011, Shawn Guo wrote:
> > >> > Shawn Guo (4):
> > >> >       mmc: sdhci-esdhc-imx: do not reference platform data after probe
> > >> >       mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
> > >> >       mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
> > >> >       mmc: sdhci-esdhc-imx: add device tree probe support
> > >> 
> > >> These don't apply against mmc-next -- what is your tree based on?
> > >> 
> > > It's based on linux-next with following patch applied.
> > >
> > > [PATCH v4 4/4] mmc: sdhci-esdhc-imx: extend card_detect and
> > > write_protect support for mx5
> > > http://permalink.gmane.org/gmane.linux.kernel.mmc/8748
> > >
> > > You applied the 3 of 4 patches in that series, and expect Sascha to
> > > send this one through his tree after you send those 3.
> 
> Shawn, please ping me once all dependencies have been merged. I have no
> influence when my patches get merged since my patches go via Arnd.
> 
Ok, will do.

-- 
Regards,
Shawn