[PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Thomas Abraham]

This patchset removes all uses of 'clk_type' member from the platform data
of sdhci-s3c driver and adds device tree support for sdhci-s3c driver.

In this patchset, all uses of 'clk_type' member from the platform data are
removed from the sdhci-s3c driver and platform code. The clk_type is a SoC
specific information and not a board/machine specific information. Hence, this
information can be more aptly represented using SoC specific driver data in the
sdhci-s3c driver.

Hence all uses of 'clk_type' member in sdhci-s3c driver's platform data is
removed. In place of that, the sdhci host qurik 'SDHCI_QUIRK_NONSTANDARD_CLOCK'
is used to handle controllers that do not have a standard sdclk division
(like those in the exynos4 SoC's).

This is a pre-requisite for adding device tree support for sdhci-s3c driver.
While migrating towards device tree support, retreving 'clk_type' information
from device tree information does not seem correct and hence it has been added
as SoC specific driver data. All this is handled in patches 1 to 3.

Patch 4 to 6 add device tree support for sdhci-s3c driver. The fourth patch
derives the maximum transfer width host capability from the max_width member
in platform data. The fifth patch modifies the sdhci-s3c driver to mainatain
a local copy of the platform data, which makes it easier to add device tree
support for the driver. The last patch adds device tree based discovery for
the sdhci-s3c driver.

This patchset is based on the following tree.
  http://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git [for-next]

Thomas Abraham (6):
  mmc: sdhci-s3c: Remove usage of clk_type member in platform data
  arm: exynos4: use 'exynos4-sdhci' as device name for sdhci controllers
  arm: samsung: remove all uses of clk_type member in sdhci platform data
  mmc: sdhci-s3c: derive transfer width host capability from max_width in platform data
  mmc: sdhci-s3c: Keep a copy of platform data and use it
  mmc: sdhci-s3c: Add device tree support

 .../devicetree/bindings/mmc/samsung-sdhci.txt      |   70 ++++++
 arch/arm/mach-exynos/clock.c                       |   24 +-
 arch/arm/mach-exynos/common.c                      |    5 +
 arch/arm/mach-exynos/mach-armlex4210.c             |    3 -
 arch/arm/mach-exynos/mach-nuri.c                   |    3 -
 arch/arm/mach-exynos/mach-origen.c                 |    2 -
 arch/arm/mach-exynos/mach-smdk4x12.c               |    2 -
 arch/arm/mach-exynos/mach-smdkv310.c               |    4 -
 arch/arm/mach-exynos/mach-universal_c210.c         |    2 -
 arch/arm/plat-samsung/devs.c                       |    4 -
 arch/arm/plat-samsung/include/plat/sdhci.h         |   35 +++-
 arch/arm/plat-samsung/platformdata.c               |    2 -
 drivers/mmc/host/sdhci-s3c.c                       |  235 +++++++++++++++++++-
 13 files changed, 341 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung-sdhci.txt

[PATCH v3 1/6] mmc: sdhci-s3c: Remove usage of clk_type member in platform data

[Thomas Abraham]

SDHCI controllers on Exynos4 do not include the sdclk divider as per the
sdhci controller specification. This case can be represented using the
sdhci quirk SDHCI_QUIRK_NONSTANDARD_CLOCK instead of using an additional
enum type definition 'clk_types'.

Hence, usage of clk_type member in platform data is removed and the sdhci
quirk is used. In addition to that, since this qurik is SoC specific,
driver data is introduced to represent controllers on SoC's that require
this quirk.

Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Jeongbae Seo <jeongbae.seo@samsung.com>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 drivers/mmc/host/sdhci-s3c.c |   74 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 1af756e..e1b685d 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -53,6 +53,18 @@ struct sdhci_s3c {
 	struct clk		*clk_bus[MAX_BUS_CLK];
 };
 
+/**
+ * struct sdhci_s3c_driver_data - S3C SDHCI platform specific driver data
+ * @sdhci_quirks: sdhci host specific quirks.
+ *
+ * Specifies platform specific configuration of sdhci controller.
+ * Note: A structure for driver specific platform data is used for future
+ * expansion of its usage.
+ */
+struct sdhci_s3c_drv_data {
+	unsigned int	sdhci_quirks;
+};
+
 static inline struct sdhci_s3c *to_s3c(struct sdhci_host *host)
 {
 	return sdhci_priv(host);
@@ -132,10 +144,10 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 		return UINT_MAX;
 
 	/*
-	 * Clock divider's step is different as 1 from that of host controller
-	 * when 'clk_type' is S3C_SDHCI_CLK_DIV_EXTERNAL.
+	 * If controller uses a non-standard clock division, find the best clock
+	 * speed possible with selected clock source and skip the division.
 	 */
-	if (ourhost->pdata->clk_type) {
+	if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
 		rate = clk_round_rate(clksrc, wanted);
 		return wanted - rate;
 	}
@@ -272,6 +284,8 @@ static unsigned int sdhci_cmu_get_min_clock(struct sdhci_host *host)
 static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_s3c *ourhost = to_s3c(host);
+	unsigned long timeout;
+	u16 clk = 0;
 
 	/* don't bother if the clock is going off */
 	if (clock == 0)
@@ -282,6 +296,25 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
 	clk_set_rate(ourhost->clk_bus[ourhost->cur_clk], clock);
 
 	host->clock = clock;
+
+	clk = SDHCI_CLOCK_INT_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+	/* Wait max 20 ms */
+	timeout = 20;
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+		& SDHCI_CLOCK_INT_STABLE)) {
+		if (timeout == 0) {
+			printk(KERN_ERR "%s: Internal clock never "
+				"stabilised.\n", mmc_hostname(host->mmc));
+			return;
+		}
+		timeout--;
+		mdelay(1);
+	}
+
+	clk |= SDHCI_CLOCK_CARD_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 }
 
 /**
@@ -382,9 +415,17 @@ static void sdhci_s3c_setup_card_detect_gpio(struct sdhci_s3c *sc)
 	}
 }
 
+static inline struct sdhci_s3c_drv_data *sdhci_s3c_get_driver_data(
+			struct platform_device *pdev)
+{
+	return (struct sdhci_s3c_drv_data *)
+			platform_get_device_id(pdev)->driver_data;
+}
+
 static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 {
 	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
+	struct sdhci_s3c_drv_data *drv_data;
 	struct device *dev = &pdev->dev;
 	struct sdhci_host *host;
 	struct sdhci_s3c *sc;
@@ -414,6 +455,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 		return PTR_ERR(host);
 	}
 
+	drv_data = sdhci_s3c_get_driver_data(pdev);
 	sc = sdhci_priv(host);
 
 	sc->host = host;
@@ -491,6 +533,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	/* Setup quirks for the controller */
 	host->quirks |= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
 	host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT;
+	if (drv_data)
+		host->quirks |= drv_data->sdhci_quirks;
 
 #ifndef CONFIG_MMC_SDHCI_S3C_DMA
 
@@ -534,7 +578,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	 * If controller does not have internal clock divider,
 	 * we can use overriding functions instead of default.
 	 */
-	if (pdata->clk_type) {
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
 		sdhci_s3c_ops.set_clock = sdhci_cmu_set_clock;
 		sdhci_s3c_ops.get_min_clock = sdhci_cmu_get_min_clock;
 		sdhci_s3c_ops.get_max_clock = sdhci_cmu_get_max_clock;
@@ -647,9 +691,31 @@ static const struct dev_pm_ops sdhci_s3c_pmops = {
 #define SDHCI_S3C_PMOPS NULL
 #endif
 
+#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS4212)
+static struct sdhci_s3c_drv_data exynos4_sdhci_drv_data = {
+	.sdhci_quirks = SDHCI_QUIRK_NONSTANDARD_CLOCK,
+};
+#define EXYNOS4_SDHCI_DRV_DATA ((kernel_ulong_t)&exynos4_sdhci_drv_data)
+#else
+#define EXYNOS4_SDHCI_DRV_DATA ((kernel_ulong_t)NULL)
+#endif
+
+static struct platform_device_id sdhci_s3c_driver_ids[] = {
+	{
+		.name		= "s3c-sdhci",
+		.driver_data	= (kernel_ulong_t)NULL,
+	}, {
+		.name		= "exynos4-sdhci",
+		.driver_data	= EXYNOS4_SDHCI_DRV_DATA,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, sdhci_s3c_driver_ids);
+
 static struct platform_driver sdhci_s3c_driver = {
 	.probe		= sdhci_s3c_probe,
 	.remove		= __devexit_p(sdhci_s3c_remove),
+	.id_table	= sdhci_s3c_driver_ids,
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "s3c-sdhci",
-- 
1.6.6.rc2

[PATCH v3 2/6] arm: exynos4: use 'exynos4-sdhci' as device name for sdhci controllers

[Thomas Abraham]

With the addition of platform specific driver data in the sdhci driver
for exynos4, the device name of sdhci controllers on exynos4 is changed
accordingly.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 arch/arm/mach-exynos/clock.c               |   24 ++++++++++++------------
 arch/arm/mach-exynos/common.c              |    5 +++++
 arch/arm/plat-samsung/include/plat/sdhci.h |   28 ++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-exynos/clock.c b/arch/arm/mach-exynos/clock.c
index 5a8c42e..f94c99d 100644
--- a/arch/arm/mach-exynos/clock.c
+++ b/arch/arm/mach-exynos/clock.c
@@ -496,25 +496,25 @@ static struct clk init_clocks_off[] = {
 		.ctrlbit	= (1 << 0),
 	}, {
 		.name		= "hsmmc",
-		.devname	= "s3c-sdhci.0",
+		.devname	= "exynos4-sdhci.0",
 		.parent		= &clk_aclk_133.clk,
 		.enable		= exynos4_clk_ip_fsys_ctrl,
 		.ctrlbit	= (1 << 5),
 	}, {
 		.name		= "hsmmc",
-		.devname	= "s3c-sdhci.1",
+		.devname	= "exynos4-sdhci.1",
 		.parent		= &clk_aclk_133.clk,
 		.enable		= exynos4_clk_ip_fsys_ctrl,
 		.ctrlbit	= (1 << 6),
 	}, {
 		.name		= "hsmmc",
-		.devname	= "s3c-sdhci.2",
+		.devname	= "exynos4-sdhci.2",
 		.parent		= &clk_aclk_133.clk,
 		.enable		= exynos4_clk_ip_fsys_ctrl,
 		.ctrlbit	= (1 << 7),
 	}, {
 		.name		= "hsmmc",
-		.devname	= "s3c-sdhci.3",
+		.devname	= "exynos4-sdhci.3",
 		.parent		= &clk_aclk_133.clk,
 		.enable		= exynos4_clk_ip_fsys_ctrl,
 		.ctrlbit	= (1 << 8),
@@ -1187,7 +1187,7 @@ static struct clksrc_clk clk_sclk_uart3 = {
 static struct clksrc_clk clk_sclk_mmc0 = {
 	.clk		= {
 		.name		= "sclk_mmc",
-		.devname	= "s3c-sdhci.0",
+		.devname	= "exynos4-sdhci.0",
 		.parent		= &clk_dout_mmc0.clk,
 		.enable		= exynos4_clksrc_mask_fsys_ctrl,
 		.ctrlbit	= (1 << 0),
@@ -1198,7 +1198,7 @@ static struct clksrc_clk clk_sclk_mmc0 = {
 static struct clksrc_clk clk_sclk_mmc1 = {
 	.clk		= {
 		.name		= "sclk_mmc",
-		.devname	= "s3c-sdhci.1",
+		.devname	= "exynos4-sdhci.1",
 		.parent         = &clk_dout_mmc1.clk,
 		.enable		= exynos4_clksrc_mask_fsys_ctrl,
 		.ctrlbit	= (1 << 4),
@@ -1209,7 +1209,7 @@ static struct clksrc_clk clk_sclk_mmc1 = {
 static struct clksrc_clk clk_sclk_mmc2 = {
 	.clk		= {
 		.name		= "sclk_mmc",
-		.devname	= "s3c-sdhci.2",
+		.devname	= "exynos4-sdhci.2",
 		.parent         = &clk_dout_mmc2.clk,
 		.enable		= exynos4_clksrc_mask_fsys_ctrl,
 		.ctrlbit	= (1 << 8),
@@ -1220,7 +1220,7 @@ static struct clksrc_clk clk_sclk_mmc2 = {
 static struct clksrc_clk clk_sclk_mmc3 = {
 	.clk		= {
 		.name		= "sclk_mmc",
-		.devname	= "s3c-sdhci.3",
+		.devname	= "exynos4-sdhci.3",
 		.parent         = &clk_dout_mmc3.clk,
 		.enable		= exynos4_clksrc_mask_fsys_ctrl,
 		.ctrlbit	= (1 << 12),
@@ -1323,10 +1323,10 @@ static struct clk_lookup exynos4_clk_lookup[] = {
 	CLKDEV_INIT("exynos4210-uart.1", "clk_uart_baud0", &clk_sclk_uart1.clk),
 	CLKDEV_INIT("exynos4210-uart.2", "clk_uart_baud0", &clk_sclk_uart2.clk),
 	CLKDEV_INIT("exynos4210-uart.3", "clk_uart_baud0", &clk_sclk_uart3.clk),
-	CLKDEV_INIT("s3c-sdhci.0", "mmc_busclk.2", &clk_sclk_mmc0.clk),
-	CLKDEV_INIT("s3c-sdhci.1", "mmc_busclk.2", &clk_sclk_mmc1.clk),
-	CLKDEV_INIT("s3c-sdhci.2", "mmc_busclk.2", &clk_sclk_mmc2.clk),
-	CLKDEV_INIT("s3c-sdhci.3", "mmc_busclk.2", &clk_sclk_mmc3.clk),
+	CLKDEV_INIT("exynos4-sdhci.0", "mmc_busclk.2", &clk_sclk_mmc0.clk),
+	CLKDEV_INIT("exynos4-sdhci.1", "mmc_busclk.2", &clk_sclk_mmc1.clk),
+	CLKDEV_INIT("exynos4-sdhci.2", "mmc_busclk.2", &clk_sclk_mmc2.clk),
+	CLKDEV_INIT("exynos4-sdhci.3", "mmc_busclk.2", &clk_sclk_mmc3.clk),
 	CLKDEV_INIT("dma-pl330.0", "apb_pclk", &clk_pdma0),
 	CLKDEV_INIT("dma-pl330.1", "apb_pclk", &clk_pdma1),
 	CLKDEV_INIT("s3c64xx-spi.0", "spi_busclk0", &clk_sclk_spi0.clk),
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index c59e188..ca2f78a 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -255,6 +255,11 @@ void __init exynos4_map_io(void)
 	s3c_fimc_setname(2, "exynos4-fimc");
 	s3c_fimc_setname(3, "exynos4-fimc");
 
+	s3c_sdhci_setname(0, "exynos4-sdhci");
+	s3c_sdhci_setname(1, "exynos4-sdhci");
+	s3c_sdhci_setname(2, "exynos4-sdhci");
+	s3c_sdhci_setname(3, "exynos4-sdhci");
+
 	/* The I2C bus controllers are directly compatible with s3c2440 */
 	s3c_i2c0_setname("s3c2440-i2c");
 	s3c_i2c1_setname("s3c2440-i2c");
diff --git a/arch/arm/plat-samsung/include/plat/sdhci.h b/arch/arm/plat-samsung/include/plat/sdhci.h
index f82f888..e00d35a 100644
--- a/arch/arm/plat-samsung/include/plat/sdhci.h
+++ b/arch/arm/plat-samsung/include/plat/sdhci.h
@@ -18,6 +18,8 @@
 #ifndef __PLAT_S3C_SDHCI_H
 #define __PLAT_S3C_SDHCI_H __FILE__
 
+#include <plat/devs.h>
+
 struct platform_device;
 struct mmc_host;
 struct mmc_card;
@@ -354,4 +356,30 @@ static inline void exynos4_default_sdhci3(void) { }
 
 #endif /* CONFIG_EXYNOS4_SETUP_SDHCI */
 
+static inline void s3c_sdhci_setname(int id, char *name)
+{
+	switch (id) {
+#ifdef CONFIG_S3C_DEV_HSMMC
+	case 0:
+		s3c_device_hsmmc0.name = name;
+		break;
+#endif
+#ifdef CONFIG_S3C_DEV_HSMMC1
+	case 1:
+		s3c_device_hsmmc1.name = name;
+		break;
+#endif
+#ifdef CONFIG_S3C_DEV_HSMMC2
+	case 2:
+		s3c_device_hsmmc2.name = name;
+		break;
+#endif
+#ifdef CONFIG_S3C_DEV_HSMMC3
+	case 3:
+		s3c_device_hsmmc3.name = name;
+		break;
+#endif
+	}
+}
+
 #endif /* __PLAT_S3C_SDHCI_H */
-- 
1.6.6.rc2

[PATCH v3 3/6] arm: samsung: remove all uses of clk_type member in sdhci platform data

[Thomas Abraham]

The sdhci driver is modified to be independent of clk_type member in the sdhci
platform data. Hence, all usage of clk_type in platform code is removed.

Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: JeongHyeon Kim <jhkim@insignal.co.kr>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Changhwan Youn <chaos.youn@samsung.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 arch/arm/mach-exynos/mach-armlex4210.c     |    3 ---
 arch/arm/mach-exynos/mach-nuri.c           |    3 ---
 arch/arm/mach-exynos/mach-origen.c         |    2 --
 arch/arm/mach-exynos/mach-smdk4x12.c       |    2 --
 arch/arm/mach-exynos/mach-smdkv310.c       |    4 ----
 arch/arm/mach-exynos/mach-universal_c210.c |    2 --
 arch/arm/plat-samsung/devs.c               |    4 ----
 arch/arm/plat-samsung/include/plat/sdhci.h |    7 -------
 arch/arm/plat-samsung/platformdata.c       |    2 --
 9 files changed, 0 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-armlex4210.c b/arch/arm/mach-exynos/mach-armlex4210.c
index d726fcd..a1e7007 100644
--- a/arch/arm/mach-exynos/mach-armlex4210.c
+++ b/arch/arm/mach-exynos/mach-armlex4210.c
@@ -77,7 +77,6 @@ static struct s3c2410_uartcfg armlex4210_uartcfgs[] __initdata = {
 
 static struct s3c_sdhci_platdata armlex4210_hsmmc0_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_PERMANENT,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 #ifdef CONFIG_EXYNOS4_SDHCI_CH0_8BIT
 	.max_width		= 8,
 	.host_caps		= MMC_CAP_8_BIT_DATA,
@@ -88,13 +87,11 @@ static struct s3c_sdhci_platdata armlex4210_hsmmc2_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_GPIO,
 	.ext_cd_gpio		= EXYNOS4_GPX2(5),
 	.ext_cd_gpio_invert	= 1,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 	.max_width		= 4,
 };
 
 static struct s3c_sdhci_platdata armlex4210_hsmmc3_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_PERMANENT,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 	.max_width		= 4,
 };
 
diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
index 165c876..a45b38a 100644
--- a/arch/arm/mach-exynos/mach-nuri.c
+++ b/arch/arm/mach-exynos/mach-nuri.c
@@ -111,7 +111,6 @@ static struct s3c_sdhci_platdata nuri_hsmmc0_data __initdata = {
 				MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
 				MMC_CAP_DISABLE | MMC_CAP_ERASE),
 	.cd_type		= S3C_SDHCI_CD_PERMANENT,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static struct regulator_consumer_supply emmc_supplies[] = {
@@ -153,7 +152,6 @@ static struct s3c_sdhci_platdata nuri_hsmmc2_data __initdata = {
 	.ext_cd_gpio		= EXYNOS4_GPX3(3),	/* XEINT_27 */
 	.ext_cd_gpio_invert	= 1,
 	.cd_type		= S3C_SDHCI_CD_GPIO,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 /* WLAN */
@@ -162,7 +160,6 @@ static struct s3c_sdhci_platdata nuri_hsmmc3_data __initdata = {
 	.host_caps		= MMC_CAP_4_BIT_DATA |
 				MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED,
 	.cd_type		= S3C_SDHCI_CD_EXTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static void __init nuri_sdhci_init(void)
diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
index e1d87b9..1159da7 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -472,12 +472,10 @@ static struct i2c_board_info i2c0_devs[] __initdata = {
 
 static struct s3c_sdhci_platdata origen_hsmmc0_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_INTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static struct s3c_sdhci_platdata origen_hsmmc2_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_INTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 /* USB EHCI */
diff --git a/arch/arm/mach-exynos/mach-smdk4x12.c b/arch/arm/mach-exynos/mach-smdk4x12.c
index d00e4f0..4be083a 100644
--- a/arch/arm/mach-exynos/mach-smdk4x12.c
+++ b/arch/arm/mach-exynos/mach-smdk4x12.c
@@ -85,7 +85,6 @@ static struct s3c2410_uartcfg smdk4x12_uartcfgs[] __initdata = {
 
 static struct s3c_sdhci_platdata smdk4x12_hsmmc2_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_INTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 #ifdef CONFIG_EXYNOS4_SDHCI_CH2_8BIT
 	.max_width		= 8,
 	.host_caps		= MMC_CAP_8_BIT_DATA,
@@ -94,7 +93,6 @@ static struct s3c_sdhci_platdata smdk4x12_hsmmc2_pdata __initdata = {
 
 static struct s3c_sdhci_platdata smdk4x12_hsmmc3_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_INTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static struct regulator_consumer_supply max8997_buck1 =
diff --git a/arch/arm/mach-exynos/mach-smdkv310.c b/arch/arm/mach-exynos/mach-smdkv310.c
index b2c5557..de9dd6f 100644
--- a/arch/arm/mach-exynos/mach-smdkv310.c
+++ b/arch/arm/mach-exynos/mach-smdkv310.c
@@ -93,7 +93,6 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
 
 static struct s3c_sdhci_platdata smdkv310_hsmmc0_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_INTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 #ifdef CONFIG_EXYNOS4_SDHCI_CH0_8BIT
 	.max_width		= 8,
 	.host_caps		= MMC_CAP_8_BIT_DATA,
@@ -104,12 +103,10 @@ static struct s3c_sdhci_platdata smdkv310_hsmmc1_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_GPIO,
 	.ext_cd_gpio		= EXYNOS4_GPK0(2),
 	.ext_cd_gpio_invert	= 1,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static struct s3c_sdhci_platdata smdkv310_hsmmc2_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_INTERNAL,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 #ifdef CONFIG_EXYNOS4_SDHCI_CH2_8BIT
 	.max_width		= 8,
 	.host_caps		= MMC_CAP_8_BIT_DATA,
@@ -120,7 +117,6 @@ static struct s3c_sdhci_platdata smdkv310_hsmmc3_pdata __initdata = {
 	.cd_type		= S3C_SDHCI_CD_GPIO,
 	.ext_cd_gpio		= EXYNOS4_GPK2(2),
 	.ext_cd_gpio_invert	= 1,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static void lcd_lte480wv_set_power(struct plat_lcd_data *pd,
diff --git a/arch/arm/mach-exynos/mach-universal_c210.c b/arch/arm/mach-exynos/mach-universal_c210.c
index dd75101..f1c83b6 100644
--- a/arch/arm/mach-exynos/mach-universal_c210.c
+++ b/arch/arm/mach-exynos/mach-universal_c210.c
@@ -737,7 +737,6 @@ static struct s3c_sdhci_platdata universal_hsmmc0_data __initdata = {
 				MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
 				MMC_CAP_DISABLE),
 	.cd_type		= S3C_SDHCI_CD_PERMANENT,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 static struct regulator_consumer_supply mmc0_supplies[] = {
@@ -778,7 +777,6 @@ static struct s3c_sdhci_platdata universal_hsmmc2_data __initdata = {
 	.ext_cd_gpio		= EXYNOS4_GPX3(4),      /* XEINT_28 */
 	.ext_cd_gpio_invert	= 1,
 	.cd_type		= S3C_SDHCI_CD_GPIO,
-	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
 };
 
 /* WiFi */
diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
index 145580a..71bc66e 100644
--- a/arch/arm/plat-samsung/devs.c
+++ b/arch/arm/plat-samsung/devs.c
@@ -351,7 +351,6 @@ struct s3c_sdhci_platdata s3c_hsmmc0_def_platdata = {
 	.max_width	= 4,
 	.host_caps	= (MMC_CAP_4_BIT_DATA |
 			   MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED),
-	.clk_type	= S3C_SDHCI_CLK_DIV_INTERNAL,
 };
 
 struct platform_device s3c_device_hsmmc0 = {
@@ -382,7 +381,6 @@ struct s3c_sdhci_platdata s3c_hsmmc1_def_platdata = {
 	.max_width	= 4,
 	.host_caps	= (MMC_CAP_4_BIT_DATA |
 			   MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED),
-	.clk_type	= S3C_SDHCI_CLK_DIV_INTERNAL,
 };
 
 struct platform_device s3c_device_hsmmc1 = {
@@ -415,7 +413,6 @@ struct s3c_sdhci_platdata s3c_hsmmc2_def_platdata = {
 	.max_width	= 4,
 	.host_caps	= (MMC_CAP_4_BIT_DATA |
 			   MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED),
-	.clk_type	= S3C_SDHCI_CLK_DIV_INTERNAL,
 };
 
 struct platform_device s3c_device_hsmmc2 = {
@@ -446,7 +443,6 @@ struct s3c_sdhci_platdata s3c_hsmmc3_def_platdata = {
 	.max_width	= 4,
 	.host_caps	= (MMC_CAP_4_BIT_DATA |
 			   MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED),
-	.clk_type	= S3C_SDHCI_CLK_DIV_INTERNAL,
 };
 
 struct platform_device s3c_device_hsmmc3 = {
diff --git a/arch/arm/plat-samsung/include/plat/sdhci.h b/arch/arm/plat-samsung/include/plat/sdhci.h
index e00d35a..95a6e19 100644
--- a/arch/arm/plat-samsung/include/plat/sdhci.h
+++ b/arch/arm/plat-samsung/include/plat/sdhci.h
@@ -33,17 +33,11 @@ enum cd_types {
 	S3C_SDHCI_CD_PERMANENT,	/* no CD line, card permanently wired to host */
 };
 
-enum clk_types {
-	S3C_SDHCI_CLK_DIV_INTERNAL,	/* use mmc internal clock divider */
-	S3C_SDHCI_CLK_DIV_EXTERNAL,	/* use external clock divider */
-};
-
 /**
  * struct s3c_sdhci_platdata() - Platform device data for Samsung SDHCI
  * @max_width: The maximum number of data bits supported.
  * @host_caps: Standard MMC host capabilities bit field.
  * @cd_type: Type of Card Detection method (see cd_types enum above)
- * @clk_type: Type of clock divider method (see clk_types enum above)
  * @ext_cd_init: Initialize external card detect subsystem. Called on
  *		 sdhci-s3c driver probe when cd_type == S3C_SDHCI_CD_EXTERNAL.
  *		 notify_func argument is a callback to the sdhci-s3c driver
@@ -67,7 +61,6 @@ struct s3c_sdhci_platdata {
 	unsigned int	host_caps;
 	unsigned int	pm_caps;
 	enum cd_types	cd_type;
-	enum clk_types	clk_type;
 
 	int		ext_cd_gpio;
 	bool		ext_cd_gpio_invert;
diff --git a/arch/arm/plat-samsung/platformdata.c b/arch/arm/plat-samsung/platformdata.c
index 0f70718..dc55eb5 100644
--- a/arch/arm/plat-samsung/platformdata.c
+++ b/arch/arm/plat-samsung/platformdata.c
@@ -55,6 +55,4 @@ void s3c_sdhci_set_platdata(struct s3c_sdhci_platdata *pd,
 		set->host_caps |= pd->host_caps;
 	if (pd->pm_caps)
 		set->pm_caps |= pd->pm_caps;
-	if (pd->clk_type)
-		set->clk_type = pd->clk_type;
 }
-- 
1.6.6.rc2

[PATCH v3 4/6] mmc: sdhci-s3c: derive transfer width host capability from max_width in platform data

[Thomas Abraham]

max_width member in platform data can be used to derive the mmc bus transfer
width that can be supported by the controller.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 drivers/mmc/host/sdhci-s3c.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index e1b685d..a57d7a0 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -562,6 +562,14 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->mmc->caps = MMC_CAP_NONREMOVABLE;
 
+	switch (pdata->max_width) {
+	case 8:
+		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+	case 4:
+		host->mmc->caps |= MMC_CAP_4_BIT_DATA;
+		break;
+	}
+
 	if (pdata->host_caps)
 		host->mmc->caps |= pdata->host_caps;
 
-- 
1.6.6.rc2

Re: [PATCH v3 4/6] mmc: sdhci-s3c: derive transfer width host capability from max_width in platform data

[Sergei Shtylyov]

Hello.

On 01/31/2012 08:56 PM, Thomas Abraham wrote:

> max_width member in platform data can be used to derive the mmc bus transfer
> width that can be supported by the controller.

> Signed-off-by: Thomas Abraham<thomas.abraham@linaro.org>
> ---
>   drivers/mmc/host/sdhci-s3c.c |    8 ++++++++
>   1 files changed, 8 insertions(+), 0 deletions(-)

> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index e1b685d..a57d7a0 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -562,6 +562,14 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>   	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
>   		host->mmc->caps = MMC_CAP_NONREMOVABLE;
>
> +	switch (pdata->max_width) {
> +	case 8:
> +		host->mmc->caps |= MMC_CAP_8_BIT_DATA;

    A comment like /* FALL THRU */ is needed here.

> +	case 4:
> +		host->mmc->caps |= MMC_CAP_4_BIT_DATA;
> +		break;
> +	}
> +

WBR, Sergei

[PATCH v3 5/6] mmc: sdhci-s3c: Keep a copy of platform data and use it

[Thomas Abraham]

The platform data is copied into driver's private data and the copy is
used for all access to the platform data. This simpifies the addition
of device tree support for the sdhci-s3c driver.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 drivers/mmc/host/sdhci-s3c.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index a57d7a0..1760ba8 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -424,7 +424,7 @@ static inline struct sdhci_s3c_drv_data *sdhci_s3c_get_driver_data(
 
 static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 {
-	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
+	struct s3c_sdhci_platdata *pdata;
 	struct sdhci_s3c_drv_data *drv_data;
 	struct device *dev = &pdev->dev;
 	struct sdhci_host *host;
@@ -432,7 +432,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret, irq, ptr, clks;
 
-	if (!pdata) {
+	if (!pdev->dev.platform_data) {
 		dev_err(dev, "no device data specified\n");
 		return -ENOENT;
 	}
@@ -455,6 +455,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 		return PTR_ERR(host);
 	}
 
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		ret = -ENOMEM;
+		goto err_io_clk;
+	}
+	memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+
 	drv_data = sdhci_s3c_get_driver_data(pdev);
 	sc = sdhci_priv(host);
 
-- 
1.6.6.rc2

[PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Thomas Abraham]

Add device tree based discovery support for Samsung's sdhci controller

Cc: Ben Dooks <ben-linux@fluff.org>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 .../devicetree/bindings/mmc/samsung-sdhci.txt      |   70 +++++++++
 drivers/mmc/host/sdhci-s3c.c                       |  148 +++++++++++++++++++-
 2 files changed, 212 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung-sdhci.txt

diff --git a/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
new file mode 100644
index 0000000..8cbdd29
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
@@ -0,0 +1,70 @@
+* Samsung's SDHCI Controller device tree bindings
+
+Samsung's SDHCI controller is used as a connectivity interface with external
+MMC, SD and eMMC storage mediums.
+
+Required SoC Specific Properties:
+- compatible: should be one of the following
+  - "samsung,s3c6410-sdhci": For controllers compatible with s3c6410 sdhci
+    controller.
+  - "samsung,exynos4210-sdhci": For controller compatible with Exynos4 sdhci
+    controller.
+
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+- interrupts: The interrupt number to the cpu. The interrupt specifier format
+  depends on the interrupt controller.
+
+
+Required Board Specific Properties:
+- gpios: Should specify the gpios used for clock, command and data lines. The
+  gpio specifier format depends on the gpio controller. Note: There is no
+  particular order in which the gpio's have to be listed.
+
+
+Optional Board Specific Properties:
+- samsung,sdhci-bus-width: Number of data lines connected to the controller.
+  Note: This excludes the clock,command and card detect lines. If this property
+  is not specified, default value is 1.
+
+- samsung,cd-gpio-invert: If 'samsung,sdhci-cd-gpio' card detect method is
+  selected, this property can be optionally specified to invert the value of
+  external card detect gpio line.
+
+- One of the following properties for card detect type.
+  - samsung,sdhci-cd-internal: Card detect line from the card slot  is
+    connected to the card detect pad of the sdhci controller. A gpio is
+    used for this connection (with possible pin function settings).
+  - samsung,sdhci-cd-gpio: A gpio line (with possible pin function settings)
+    is used a card detect line. This gpio line is not connected to card detect
+    pad of the sdhci controller.
+  - samsung,sdhci-cd-none: There is no card detect line. Polling is used to
+    detect the presence of the card. (DEFAULT, if no card detect property
+    is specified).
+  - samsung,sdhci-cd-permanent: There is no card detect line. The card is
+    permanently connected to the sdhci controller.
+
+- gpio-cd: The gpio to be used as card detect line for
+  'samsung,sdhci-cd-internal' or 'samsung,sdhci-cd-gpio' card detection method.
+  The gpio specifier format depends on the gpio controller.
+
+Example:
+	sdhci@12530000 {
+		compatible = "samsung,exynos4210-sdhci";
+		reg = <0x12530000 0x100>;
+		interrupts = <139>;
+		samsung,sdhci-bus-width = <4>;
+		samsung,sdhci-cd-internal;
+		gpio-cd = <&gpk2 2 2 3 3>;
+		gpios = <&gpk2 0 2 0 3>,  /* clock line */
+			<&gpk2 1 2 0 3>,  /* command line */
+			<&gpk2 3 2 3 3>,  /* data line 0 */
+			<&gpk2 4 2 3 3>,  /* data line 1 */
+			<&gpk2 5 2 3 3>,  /* data line 2 */
+			<&gpk2 6 2 3 3>;  /* data line 3 */
+	};
+
+	Note: This example shows both SoC specific and board specific properties
+	in a single device node. The properties can be actually be seperated
+	into SoC specific node and board specific node.
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 1760ba8..6159542 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -20,6 +20,8 @@
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include <linux/mmc/host.h>
 
@@ -29,6 +31,8 @@
 #include "sdhci.h"
 
 #define MAX_BUS_CLK	(4)
+/* Number of gpio's used is max data bus width + command and clock lines */
+#define NUM_GPIOS(x)	(x + 2)
 
 /**
  * struct sdhci_s3c - S3C SDHCI instance
@@ -37,6 +41,7 @@
  * @ioarea: The resource created when we claimed the IO area.
  * @pdata: The platform data for this controller.
  * @cur_clk: The index of the current bus clock.
+ * @gpios: List of gpio numbers parsed from device tree.
  * @clk_io: The clock for the internal bus interface.
  * @clk_bus: The clocks that are available for the SD/MMC bus clock.
  */
@@ -48,6 +53,7 @@ struct sdhci_s3c {
 	unsigned int		cur_clk;
 	int			ext_cd_irq;
 	int			ext_cd_gpio;
+	int			*gpios;
 
 	struct clk		*clk_io;
 	struct clk		*clk_bus[MAX_BUS_CLK];
@@ -415,9 +421,110 @@ static void sdhci_s3c_setup_card_detect_gpio(struct sdhci_s3c *sc)
 	}
 }
 
+#ifdef CONFIG_OF
+static int __devinit sdhci_s3c_parse_dt(struct device *dev,
+		struct sdhci_host *host, struct s3c_sdhci_platdata *pdata)
+{
+	struct device_node *node = dev->of_node;
+	struct sdhci_s3c *ourhost = to_s3c(host);
+	u32 max_width;
+	int gpio, cnt, ret;
+
+	/* if the bus-width property is not specified, assume width as 1 */
+	if (of_property_read_u32(node, "samsung,sdhci-bus-width",
+				&max_width))
+		max_width = 1;
+	pdata->max_width = max_width;
+
+	ourhost->gpios = devm_kzalloc(dev, NUM_GPIOS(pdata->max_width) *
+				sizeof(int), GFP_KERNEL);
+	if (!ourhost->gpios)
+		return -ENOMEM;
+
+	/* get the card detection method */
+	if (of_get_property(node, "samsung,sdhci-cd-internal", NULL))
+		pdata->cd_type = S3C_SDHCI_CD_INTERNAL;
+	else if (of_get_property(node, "samsung,sdhci-cd-gpio", NULL))
+		pdata->cd_type = S3C_SDHCI_CD_GPIO;
+	else if (of_get_property(node, "samsung,sdhci-cd-none", NULL))
+		pdata->cd_type = S3C_SDHCI_CD_NONE;
+	else if (of_get_property(node, "samsung,sdhci-cd-permanent", NULL))
+		pdata->cd_type = S3C_SDHCI_CD_PERMANENT;
+	else
+		pdata->cd_type = S3C_SDHCI_CD_NONE;
+
+	/* get the gpio used for card detection */
+	if (pdata->cd_type == S3C_SDHCI_CD_GPIO ||
+			pdata->cd_type == S3C_SDHCI_CD_INTERNAL) {
+		gpio = of_get_named_gpio(node, "gpio-cd", 0);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(dev, "invalid card detect gpio specified\n");
+			return -EINVAL;
+		}
+	}
+
+	if (pdata->cd_type == S3C_SDHCI_CD_GPIO) {
+		pdata->ext_cd_gpio = gpio;
+		ourhost->ext_cd_gpio = -1; /* invalid gpio number */
+		if (of_get_property(node, "samsung,cd-gpio-invert", NULL))
+			pdata->ext_cd_gpio_invert = 1;
+	} else if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL) {
+		ret = gpio_request(gpio, "sdhci-cd");
+		if (ret) {
+			dev_err(dev, "card detect gpio request failed\n");
+			return -EINVAL;
+		}
+		ourhost->ext_cd_gpio = gpio;
+	}
+
+	/* get the gpios for command, clock and data lines */
+	for (cnt = 0; cnt < NUM_GPIOS(pdata->max_width); cnt++) {
+		gpio = of_get_gpio(node, cnt);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(dev, "invalid gpio[%d]\n", cnt);
+			goto err_free_dt_cd_gpio;
+		}
+		ourhost->gpios[cnt] = gpio;
+	}
+
+	for (cnt = 0; cnt < NUM_GPIOS(pdata->max_width); cnt++) {
+		ret = gpio_request(ourhost->gpios[cnt], "sdhci-gpio");
+		if (ret) {
+			dev_err(dev, "gpio[%d] request failed\n", cnt);
+			goto err_free_dt_gpios;
+		}
+	}
+
+	return 0;
+
+ err_free_dt_gpios:
+	while (--cnt >= 0)
+		gpio_free(ourhost->gpios[cnt]);
+ err_free_dt_cd_gpio:
+	if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL)
+		gpio_free(ourhost->ext_cd_gpio);
+	return -EINVAL;
+}
+#else
+static int __devinit sdhci_s3c_parse_dt(struct device *dev,
+		struct sdhci_host *host, struct s3c_sdhci_platdata *pdata)
+{
+	return -EINVAL;
+}
+#endif
+
+static const struct of_device_id sdhci_s3c_dt_match[];
+
 static inline struct sdhci_s3c_drv_data *sdhci_s3c_get_driver_data(
 			struct platform_device *pdev)
 {
+#ifdef CONFIG_OF
+	if (pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(sdhci_s3c_dt_match, pdev->dev.of_node);
+		return (struct sdhci_s3c_drv_data *)match->data;
+	}
+#endif
 	return (struct sdhci_s3c_drv_data *)
 			platform_get_device_id(pdev)->driver_data;
 }
@@ -432,7 +539,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret, irq, ptr, clks;
 
-	if (!pdev->dev.platform_data) {
+	if (!pdev->dev.platform_data && !pdev->dev.of_node) {
 		dev_err(dev, "no device data specified\n");
 		return -ENOENT;
 	}
@@ -454,21 +561,28 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 		dev_err(dev, "sdhci_alloc_host() failed\n");
 		return PTR_ERR(host);
 	}
+	sc = sdhci_priv(host);
 
 	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata) {
 		ret = -ENOMEM;
-		goto err_io_clk;
+		goto err_pdata;
+	}
+
+	if (pdev->dev.of_node) {
+		ret = sdhci_s3c_parse_dt(&pdev->dev, host, pdata);
+		if (ret)
+			goto err_pdata;
+	} else {
+		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+		sc->ext_cd_gpio = -1; /* invalid gpio number */
 	}
-	memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
 
 	drv_data = sdhci_s3c_get_driver_data(pdev);
-	sc = sdhci_priv(host);
 
 	sc->host = host;
 	sc->pdev = pdev;
 	sc->pdata = pdata;
-	sc->ext_cd_gpio = -1; /* invalid gpio number */
 
 	platform_set_drvdata(pdev, host);
 
@@ -637,6 +751,12 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	clk_put(sc->clk_io);
 
  err_io_clk:
+	for (ptr = 0; ptr < NUM_GPIOS(sc->pdata->max_width); ptr++)
+		gpio_free(sc->gpios[ptr]);
+	if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL)
+		gpio_free(sc->ext_cd_gpio);
+
+ err_pdata:
 	sdhci_free_host(host);
 
 	return ret;
@@ -644,9 +764,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 
 static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 {
-	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host =  platform_get_drvdata(pdev);
 	struct sdhci_s3c *sc = sdhci_priv(host);
+	struct s3c_sdhci_platdata *pdata = sc->pdata;
 	int ptr;
 
 	if (pdata->cd_type == S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup)
@@ -673,6 +793,11 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 	release_resource(sc->ioarea);
 	kfree(sc->ioarea);
 
+	if (pdev->dev.of_node) {
+		for (ptr = 0; ptr < NUM_GPIOS(sc->pdata->max_width); ptr++)
+			gpio_free(sc->gpios[ptr]);
+	}
+
 	sdhci_free_host(host);
 	platform_set_drvdata(pdev, NULL);
 
@@ -727,6 +852,16 @@ static struct platform_device_id sdhci_s3c_driver_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, sdhci_s3c_driver_ids);
 
+#ifdef CONFIG_OF
+static const struct of_device_id sdhci_s3c_dt_match[] = {
+	{ .compatible = "samsung,s3c6410-sdhci", },
+	{ .compatible = "samsung,exynos4210-sdhci",
+		.data = &exynos4_sdhci_drv_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
+#endif
+
 static struct platform_driver sdhci_s3c_driver = {
 	.probe		= sdhci_s3c_probe,
 	.remove		= __devexit_p(sdhci_s3c_remove),
@@ -734,6 +869,7 @@ static struct platform_driver sdhci_s3c_driver = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "s3c-sdhci",
+		.of_match_table = of_match_ptr(sdhci_s3c_dt_match),
 		.pm	= SDHCI_S3C_PMOPS,
 	},
 };
-- 
1.6.6.rc2

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Grant Likely]

On Tue, Jan 31, 2012 at 11:26:16PM +0530, Thomas Abraham wrote:
> Add device tree based discovery support for Samsung's sdhci controller
> 
> Cc: Ben Dooks <ben-linux@fluff.org>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  .../devicetree/bindings/mmc/samsung-sdhci.txt      |   70 +++++++++
>  drivers/mmc/host/sdhci-s3c.c                       |  148 +++++++++++++++++++-
>  2 files changed, 212 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/samsung-sdhci.txt

Looks okay to me on brief review.

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

> 
> diff --git a/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
> new file mode 100644
> index 0000000..8cbdd29
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
> @@ -0,0 +1,70 @@
> +* Samsung's SDHCI Controller device tree bindings
> +
> +Samsung's SDHCI controller is used as a connectivity interface with external
> +MMC, SD and eMMC storage mediums.
> +
> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> +  - "samsung,s3c6410-sdhci": For controllers compatible with s3c6410 sdhci
> +    controller.
> +  - "samsung,exynos4210-sdhci": For controller compatible with Exynos4 sdhci
> +    controller.
> +
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +- interrupts: The interrupt number to the cpu. The interrupt specifier format
> +  depends on the interrupt controller.
> +
> +
> +Required Board Specific Properties:
> +- gpios: Should specify the gpios used for clock, command and data lines. The
> +  gpio specifier format depends on the gpio controller. Note: There is no
> +  particular order in which the gpio's have to be listed.
> +
> +
> +Optional Board Specific Properties:
> +- samsung,sdhci-bus-width: Number of data lines connected to the controller.
> +  Note: This excludes the clock,command and card detect lines. If this property
> +  is not specified, default value is 1.
> +
> +- samsung,cd-gpio-invert: If 'samsung,sdhci-cd-gpio' card detect method is
> +  selected, this property can be optionally specified to invert the value of
> +  external card detect gpio line.
> +
> +- One of the following properties for card detect type.
> +  - samsung,sdhci-cd-internal: Card detect line from the card slot  is
> +    connected to the card detect pad of the sdhci controller. A gpio is
> +    used for this connection (with possible pin function settings).
> +  - samsung,sdhci-cd-gpio: A gpio line (with possible pin function settings)
> +    is used a card detect line. This gpio line is not connected to card detect
> +    pad of the sdhci controller.
> +  - samsung,sdhci-cd-none: There is no card detect line. Polling is used to
> +    detect the presence of the card. (DEFAULT, if no card detect property
> +    is specified).
> +  - samsung,sdhci-cd-permanent: There is no card detect line. The card is
> +    permanently connected to the sdhci controller.
> +
> +- gpio-cd: The gpio to be used as card detect line for
> +  'samsung,sdhci-cd-internal' or 'samsung,sdhci-cd-gpio' card detection method.
> +  The gpio specifier format depends on the gpio controller.
> +
> +Example:
> +	sdhci@12530000 {
> +		compatible = "samsung,exynos4210-sdhci";
> +		reg = <0x12530000 0x100>;
> +		interrupts = <139>;
> +		samsung,sdhci-bus-width = <4>;
> +		samsung,sdhci-cd-internal;
> +		gpio-cd = <&gpk2 2 2 3 3>;
> +		gpios = <&gpk2 0 2 0 3>,  /* clock line */
> +			<&gpk2 1 2 0 3>,  /* command line */
> +			<&gpk2 3 2 3 3>,  /* data line 0 */
> +			<&gpk2 4 2 3 3>,  /* data line 1 */
> +			<&gpk2 5 2 3 3>,  /* data line 2 */
> +			<&gpk2 6 2 3 3>;  /* data line 3 */
> +	};
> +
> +	Note: This example shows both SoC specific and board specific properties
> +	in a single device node. The properties can be actually be seperated
> +	into SoC specific node and board specific node.
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 1760ba8..6159542 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -20,6 +20,8 @@
>  #include <linux/io.h>
>  #include <linux/gpio.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -29,6 +31,8 @@
>  #include "sdhci.h"
>  
>  #define MAX_BUS_CLK	(4)
> +/* Number of gpio's used is max data bus width + command and clock lines */
> +#define NUM_GPIOS(x)	(x + 2)
>  
>  /**
>   * struct sdhci_s3c - S3C SDHCI instance
> @@ -37,6 +41,7 @@
>   * @ioarea: The resource created when we claimed the IO area.
>   * @pdata: The platform data for this controller.
>   * @cur_clk: The index of the current bus clock.
> + * @gpios: List of gpio numbers parsed from device tree.
>   * @clk_io: The clock for the internal bus interface.
>   * @clk_bus: The clocks that are available for the SD/MMC bus clock.
>   */
> @@ -48,6 +53,7 @@ struct sdhci_s3c {
>  	unsigned int		cur_clk;
>  	int			ext_cd_irq;
>  	int			ext_cd_gpio;
> +	int			*gpios;
>  
>  	struct clk		*clk_io;
>  	struct clk		*clk_bus[MAX_BUS_CLK];
> @@ -415,9 +421,110 @@ static void sdhci_s3c_setup_card_detect_gpio(struct sdhci_s3c *sc)
>  	}
>  }
>  
> +#ifdef CONFIG_OF
> +static int __devinit sdhci_s3c_parse_dt(struct device *dev,
> +		struct sdhci_host *host, struct s3c_sdhci_platdata *pdata)
> +{
> +	struct device_node *node = dev->of_node;
> +	struct sdhci_s3c *ourhost = to_s3c(host);
> +	u32 max_width;
> +	int gpio, cnt, ret;
> +
> +	/* if the bus-width property is not specified, assume width as 1 */
> +	if (of_property_read_u32(node, "samsung,sdhci-bus-width",
> +				&max_width))
> +		max_width = 1;
> +	pdata->max_width = max_width;
> +
> +	ourhost->gpios = devm_kzalloc(dev, NUM_GPIOS(pdata->max_width) *
> +				sizeof(int), GFP_KERNEL);
> +	if (!ourhost->gpios)
> +		return -ENOMEM;
> +
> +	/* get the card detection method */
> +	if (of_get_property(node, "samsung,sdhci-cd-internal", NULL))
> +		pdata->cd_type = S3C_SDHCI_CD_INTERNAL;
> +	else if (of_get_property(node, "samsung,sdhci-cd-gpio", NULL))
> +		pdata->cd_type = S3C_SDHCI_CD_GPIO;
> +	else if (of_get_property(node, "samsung,sdhci-cd-none", NULL))
> +		pdata->cd_type = S3C_SDHCI_CD_NONE;
> +	else if (of_get_property(node, "samsung,sdhci-cd-permanent", NULL))
> +		pdata->cd_type = S3C_SDHCI_CD_PERMANENT;
> +	else
> +		pdata->cd_type = S3C_SDHCI_CD_NONE;
> +
> +	/* get the gpio used for card detection */
> +	if (pdata->cd_type == S3C_SDHCI_CD_GPIO ||
> +			pdata->cd_type == S3C_SDHCI_CD_INTERNAL) {
> +		gpio = of_get_named_gpio(node, "gpio-cd", 0);
> +		if (!gpio_is_valid(gpio)) {
> +			dev_err(dev, "invalid card detect gpio specified\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (pdata->cd_type == S3C_SDHCI_CD_GPIO) {
> +		pdata->ext_cd_gpio = gpio;
> +		ourhost->ext_cd_gpio = -1; /* invalid gpio number */
> +		if (of_get_property(node, "samsung,cd-gpio-invert", NULL))
> +			pdata->ext_cd_gpio_invert = 1;
> +	} else if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL) {
> +		ret = gpio_request(gpio, "sdhci-cd");
> +		if (ret) {
> +			dev_err(dev, "card detect gpio request failed\n");
> +			return -EINVAL;
> +		}
> +		ourhost->ext_cd_gpio = gpio;
> +	}
> +
> +	/* get the gpios for command, clock and data lines */
> +	for (cnt = 0; cnt < NUM_GPIOS(pdata->max_width); cnt++) {
> +		gpio = of_get_gpio(node, cnt);
> +		if (!gpio_is_valid(gpio)) {
> +			dev_err(dev, "invalid gpio[%d]\n", cnt);
> +			goto err_free_dt_cd_gpio;
> +		}
> +		ourhost->gpios[cnt] = gpio;
> +	}
> +
> +	for (cnt = 0; cnt < NUM_GPIOS(pdata->max_width); cnt++) {
> +		ret = gpio_request(ourhost->gpios[cnt], "sdhci-gpio");
> +		if (ret) {
> +			dev_err(dev, "gpio[%d] request failed\n", cnt);
> +			goto err_free_dt_gpios;
> +		}
> +	}
> +
> +	return 0;
> +
> + err_free_dt_gpios:
> +	while (--cnt >= 0)
> +		gpio_free(ourhost->gpios[cnt]);
> + err_free_dt_cd_gpio:
> +	if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL)
> +		gpio_free(ourhost->ext_cd_gpio);
> +	return -EINVAL;
> +}
> +#else
> +static int __devinit sdhci_s3c_parse_dt(struct device *dev,
> +		struct sdhci_host *host, struct s3c_sdhci_platdata *pdata)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
> +static const struct of_device_id sdhci_s3c_dt_match[];
> +
>  static inline struct sdhci_s3c_drv_data *sdhci_s3c_get_driver_data(
>  			struct platform_device *pdev)
>  {
> +#ifdef CONFIG_OF
> +	if (pdev->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_node(sdhci_s3c_dt_match, pdev->dev.of_node);
> +		return (struct sdhci_s3c_drv_data *)match->data;
> +	}
> +#endif
>  	return (struct sdhci_s3c_drv_data *)
>  			platform_get_device_id(pdev)->driver_data;
>  }
> @@ -432,7 +539,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int ret, irq, ptr, clks;
>  
> -	if (!pdev->dev.platform_data) {
> +	if (!pdev->dev.platform_data && !pdev->dev.of_node) {
>  		dev_err(dev, "no device data specified\n");
>  		return -ENOENT;
>  	}
> @@ -454,21 +561,28 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  		dev_err(dev, "sdhci_alloc_host() failed\n");
>  		return PTR_ERR(host);
>  	}
> +	sc = sdhci_priv(host);
>  
>  	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata) {
>  		ret = -ENOMEM;
> -		goto err_io_clk;
> +		goto err_pdata;
> +	}
> +
> +	if (pdev->dev.of_node) {
> +		ret = sdhci_s3c_parse_dt(&pdev->dev, host, pdata);
> +		if (ret)
> +			goto err_pdata;
> +	} else {
> +		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
> +		sc->ext_cd_gpio = -1; /* invalid gpio number */
>  	}
> -	memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
>  
>  	drv_data = sdhci_s3c_get_driver_data(pdev);
> -	sc = sdhci_priv(host);
>  
>  	sc->host = host;
>  	sc->pdev = pdev;
>  	sc->pdata = pdata;
> -	sc->ext_cd_gpio = -1; /* invalid gpio number */
>  
>  	platform_set_drvdata(pdev, host);
>  
> @@ -637,6 +751,12 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  	clk_put(sc->clk_io);
>  
>   err_io_clk:
> +	for (ptr = 0; ptr < NUM_GPIOS(sc->pdata->max_width); ptr++)
> +		gpio_free(sc->gpios[ptr]);
> +	if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL)
> +		gpio_free(sc->ext_cd_gpio);
> +
> + err_pdata:
>  	sdhci_free_host(host);
>  
>  	return ret;
> @@ -644,9 +764,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  
>  static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  {
> -	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
>  	struct sdhci_host *host =  platform_get_drvdata(pdev);
>  	struct sdhci_s3c *sc = sdhci_priv(host);
> +	struct s3c_sdhci_platdata *pdata = sc->pdata;
>  	int ptr;
>  
>  	if (pdata->cd_type == S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup)
> @@ -673,6 +793,11 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  	release_resource(sc->ioarea);
>  	kfree(sc->ioarea);
>  
> +	if (pdev->dev.of_node) {
> +		for (ptr = 0; ptr < NUM_GPIOS(sc->pdata->max_width); ptr++)
> +			gpio_free(sc->gpios[ptr]);
> +	}
> +
>  	sdhci_free_host(host);
>  	platform_set_drvdata(pdev, NULL);
>  
> @@ -727,6 +852,16 @@ static struct platform_device_id sdhci_s3c_driver_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, sdhci_s3c_driver_ids);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id sdhci_s3c_dt_match[] = {
> +	{ .compatible = "samsung,s3c6410-sdhci", },
> +	{ .compatible = "samsung,exynos4210-sdhci",
> +		.data = &exynos4_sdhci_drv_data },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
> +#endif
> +
>  static struct platform_driver sdhci_s3c_driver = {
>  	.probe		= sdhci_s3c_probe,
>  	.remove		= __devexit_p(sdhci_s3c_remove),
> @@ -734,6 +869,7 @@ static struct platform_driver sdhci_s3c_driver = {
>  	.driver		= {
>  		.owner	= THIS_MODULE,
>  		.name	= "s3c-sdhci",
> +		.of_match_table = of_match_ptr(sdhci_s3c_dt_match),
>  		.pm	= SDHCI_S3C_PMOPS,
>  	},
>  };
> -- 
> 1.6.6.rc2
> 

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Karol Lewandowski]

On 31.01.2012 18:56, Thomas Abraham wrote:
> Add device tree based discovery support for Samsung's sdhci controller

Works fine on "nuri hardware" using exynos4-dt (with custom dts).

Thanks

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Arnd Bergmann]

On Tuesday 31 January 2012, Thomas Abraham wrote:
> +
> +Optional Board Specific Properties:
> +- samsung,sdhci-bus-width: Number of data lines connected to the controller.
> +  Note: This excludes the clock,command and card detect lines. If this property
> +  is not specified, default value is 1.
> +
> +- samsung,cd-gpio-invert: If 'samsung,sdhci-cd-gpio' card detect method is
> +  selected, this property can be optionally specified to invert the value of
> +  external card detect gpio line.
> +
> +- One of the following properties for card detect type.
> +  - samsung,sdhci-cd-internal: Card detect line from the card slot  is
> +    connected to the card detect pad of the sdhci controller. A gpio is
> +    used for this connection (with possible pin function settings).
> +  - samsung,sdhci-cd-gpio: A gpio line (with possible pin function settings)
> +    is used a card detect line. This gpio line is not connected to card detect
> +    pad of the sdhci controller.
> +  - samsung,sdhci-cd-none: There is no card detect line. Polling is used to
> +    detect the presence of the card. (DEFAULT, if no card detect property
> +    is specified).
> +  - samsung,sdhci-cd-permanent: There is no card detect line. The card is
> +    permanently connected to the sdhci controller.
> +
> +- gpio-cd: The gpio to be used as card detect line for
> +  'samsung,sdhci-cd-internal' or 'samsung,sdhci-cd-gpio' card detection method.
> +  The gpio specifier format depends on the gpio controller.

These bindings came up in a discussion IRC today. I think it's rather bad that
we can't agree on a common way to name the properties for mmc. We have
bindings being proposed or already included from Anton, Stephen, Shawn,
Rajendra, Viresh, Lee and Thomas. Almost all of them define GPIO pins
for card detect and write protect, as well properties to define the bus
width and high-speed modes, but we seem to have almost as many different
definitions of these as we have drivers.

Can we please come up with a common binding for these?

	Arnd

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Arnd Bergmann]

On Tuesday 31 January 2012, Thomas Abraham wrote:
> +
> +Optional Board Specific Properties:
> +- samsung,sdhci-bus-width: Number of data lines connected to the controller.
> +  Note: This excludes the clock,command and card detect lines. If this property
> +  is not specified, default value is 1.
> +
> +- samsung,cd-gpio-invert: If 'samsung,sdhci-cd-gpio' card detect method is
> +  selected, this property can be optionally specified to invert the value of
> +  external card detect gpio line.
> +
> +- One of the following properties for card detect type.
> +  - samsung,sdhci-cd-internal: Card detect line from the card slot  is
> +    connected to the card detect pad of the sdhci controller. A gpio is
> +    used for this connection (with possible pin function settings).
> +  - samsung,sdhci-cd-gpio: A gpio line (with possible pin function settings)
> +    is used a card detect line. This gpio line is not connected to card detect
> +    pad of the sdhci controller.
> +  - samsung,sdhci-cd-none: There is no card detect line. Polling is used to
> +    detect the presence of the card. (DEFAULT, if no card detect property
> +    is specified).
> +  - samsung,sdhci-cd-permanent: There is no card detect line. The card is
> +    permanently connected to the sdhci controller.
> +
> +- gpio-cd: The gpio to be used as card detect line for
> +  'samsung,sdhci-cd-internal' or 'samsung,sdhci-cd-gpio' card detection method.
> +  The gpio specifier format depends on the gpio controller.

These bindings came up in a discussion IRC today. I think it's rather bad that
we can't agree on a common way to name the properties for mmc. We have
bindings being proposed or already included from Anton, Stephen, Shawn,
Rajendra, Viresh, Lee and Thomas. Almost all of them define GPIO pins
for card detect and write protect, as well properties to define the bus
width and high-speed modes, but we seem to have almost as many different
definitions of these as we have drivers.

Can we please come up with a common binding for these?

	Arnd

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Viresh Kumar]

On 3/27/2012 9:49 PM, Arnd Bergmann wrote:
> These bindings came up in a discussion IRC today. I think it's rather bad that
> we can't agree on a common way to name the properties for mmc. We have
> bindings being proposed or already included from Anton, Stephen, Shawn,
> Rajendra, Viresh, Lee and Thomas. Almost all of them define GPIO pins
> for card detect and write protect, as well properties to define the bus
> width and high-speed modes, but we seem to have almost as many different
> definitions of these as we have drivers.
> 
> Can we please come up with a common binding for these?

Is there any progress on this? Sorry i wasn't following all mails.
How should i progress for sdhci-spear?

-- 
viresh

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Arnd Bergmann]

On Friday 30 March 2012, Viresh Kumar wrote:
> On 3/27/2012 9:49 PM, Arnd Bergmann wrote:
> > These bindings came up in a discussion IRC today. I think it's rather bad that
> > we can't agree on a common way to name the properties for mmc. We have
> > bindings being proposed or already included from Anton, Stephen, Shawn,
> > Rajendra, Viresh, Lee and Thomas. Almost all of them define GPIO pins
> > for card detect and write protect, as well properties to define the bus
> > width and high-speed modes, but we seem to have almost as many different
> > definitions of these as we have drivers.
> > 
> > Can we please come up with a common binding for these?
> 
> Is there any progress on this? Sorry i wasn't following all mails.
> How should i progress for sdhci-spear?

No progress so far. I would suggest we apply the patch below to unify
the bindings we have. I tried to minimize the impact by picking the most
common version for each property, but if we know about devices that would
get broken by this, we may have to be more careful.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -0,0 +1,25 @@
+These properties are common to multiple MMC host controllers. Any host
+that requires the respective functionality should implement them using
+these definitions.
+
+Required properties:
+- bus-width: Number of data lines, can be <1>, <4>, or <8>
+
+Optional properties:
+- cd-gpios : Specify GPIOs for card detection, see gpio binding
+- wp-gpios : Specify GPIOs for write protection, see gpio binding
+- cd-inverted: when present, polarity on the wp gpio line is inverted
+- wp-inverted: when present, polarity on the wp gpio line is inverted
+- non-removable: non-removable slot (like eMMC)
+
+Example:
+
+sdhci@ab000000 {
+	compatible = "sdhci";
+	reg = <0xab000000 0x200>;
+	interrupts = <23>;
+	bus-width = <4>;
+	cd-gpios = <&gpio 69 0>;
+	cd-inverted;
+	wp-gpios = <&gpio 70 0>;
+}
diff --git a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
index 64bcb8b..0d93b4b 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
@@ -11,9 +11,11 @@ Required properties:
   - interrupt-parent : interrupt source phandle.
   - clock-frequency : specifies eSDHC base clock frequency.
   - sdhci,wp-inverted : (optional) specifies that eSDHC controller
-    reports inverted write-protect state;
+    reports inverted write-protect state; New devices should use
+    the generic "wp-inverted" property.
   - sdhci,1-bit-only : (optional) specifies that a controller can
-    only handle 1-bit data transfers.
+    only handle 1-bit data transfers. New devices should use the
+    generic "bus-width = <1>" property.
   - sdhci,auto-cmd12: (optional) specifies that a controller can
     only handle auto CMD12.
 
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
index ab22fe6..c7e404b 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -9,7 +9,7 @@ Required properties:
 - interrupts : Should contain eSDHC interrupt
 
 Optional properties:
-- fsl,card-wired : Indicate the card is wired to host permanently
+- non-removable : 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
diff --git a/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt b/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
index 89a0084..d64aea5 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
@@ -10,7 +10,8 @@ Required properties:
 
 Optional properties:
 - gpios : may specify GPIOs in this order: Card-Detect GPIO,
-  Write-Protect GPIO.
+  Write-Protect GPIO. Note that this does not follow the
+  binding from mmc.txt, for historic reasons.
 - interrupts : the interrupt of a card detect interrupt.
 - interrupt-parent : the phandle for the interrupt controller that
   services interrupts for this device.
diff --git a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
index 7e51154..690a226 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
@@ -6,13 +6,13 @@ and SDIO types of memory cards.
 Required properties:
 - compatible : Should be "nvidia,<chip>-sdhci"
 - reg : Should contain SD/MMC registers location and length
-- interrupts : Should contain SD/MMC interrupt
+- interrupt  : Should contain SD/MMC interrupt
+- bus-width : Number of data lines, can be <1>, <4>, or <8>
 
 Optional properties:
 - cd-gpios : Specify GPIOs for card detection
 - wp-gpios : Specify GPIOs for write protection
 - power-gpios : Specify GPIOs for power control
-- support-8bit : Boolean, indicates if 8-bit mode should be used.
 
 Example:
 
@@ -23,5 +23,5 @@ sdhci@c8000200 {
 	cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 	wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 	power-gpios = <&gpio 155 0>; /* gpio PT3 */
-	support-8bit;
+	bus-width = <8>;
 };
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index dbd4368..90b86e5 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -15,10 +15,10 @@ Optional properties:
 ti,dual-volt: boolean, supports dual voltage cards
 <supply-name>-supply: phandle to the regulator device tree node
 "supply-name" examples are "vmmc", "vmmc_aux" etc
-ti,bus-width: Number of data lines, default assumed is 1 if the property is missing.
+bus-width: Number of data lines, default assumed is 1 if the property is missing.
 cd-gpios: GPIOs for card detection
 wp-gpios: GPIOs for write protection
-ti,non-removable: non-removable slot (like eMMC)
+non-removable: non-removable slot (like eMMC)
 ti,needs-special-reset: Requires a special softreset sequence
 
 Example:
@@ -27,7 +27,7 @@ Example:
 		reg = <0x4809c000 0x400>;
 		ti,hwmods = "mmc1";
 		ti,dual-volt;
-		ti,bus-width = <4>;
+		bus-width = <4>;
 		vmmc-supply = <&vmmc>; /* phandle to regulator node */
 		ti,non-removable;
 	};
diff --git a/arch/arm/boot/dts/imx53-smd.dts b/arch/arm/boot/dts/imx53-smd.dts
index c7ee86c..139138a 100644
--- a/arch/arm/boot/dts/imx53-smd.dts
+++ b/arch/arm/boot/dts/imx53-smd.dts
@@ -35,7 +35,7 @@
 				};
 
 				esdhc@50008000 { /* ESDHC2 */
-					fsl,card-wired;
+					non-removable;
 					status = "okay";
 				};
 
@@ -76,7 +76,7 @@
 				};
 
 				esdhc@50020000 { /* ESDHC3 */
-					fsl,card-wired;
+					non-removable;
 					status = "okay";
 				};
 			};
diff --git a/arch/arm/boot/dts/imx6q-arm2.dts b/arch/arm/boot/dts/imx6q-arm2.dts
index ce1c823..d2eaf52 100644
--- a/arch/arm/boot/dts/imx6q-arm2.dts
+++ b/arch/arm/boot/dts/imx6q-arm2.dts
@@ -41,7 +41,7 @@
 			};
 
 			usdhc@0219c000 { /* uSDHC4 */
-				fsl,card-wired;
+				non-removable;
 				vmmc-supply = <&reg_3p3v>;
 				status = "okay";
 			};
diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index 8c756be..5b4506c 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -57,7 +57,7 @@
 &mmc1 {
 	vmmc-supply = <&vmmc1>;
 	vmmc_aux-supply = <&vsim>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {
diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
index ea6f5bb..31fb421 100644
--- a/arch/arm/boot/dts/omap4-panda.dts
+++ b/arch/arm/boot/dts/omap4-panda.dts
@@ -55,7 +55,7 @@
 
 &mmc1 {
 	vmmc-supply = <&vmmc>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {
@@ -72,5 +72,5 @@
 
 &mmc5 {
 	ti,non-removable;
-	ti,bus-width = <4>;
+	bus-width = <4>;
 };
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 67b2e98..a1dd873 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -94,12 +94,12 @@
 
 &mmc1 {
 	vmmc-supply = <&vmmc>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {
 	vmmc-supply = <&vaux1>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 	ti,non-removable;
 };
 
@@ -112,6 +112,6 @@
 };
 
 &mmc5 {
-	ti,bus-width = <4>;
+	bus-width = <4>;
 	ti,non-removable;
 };
diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
index ac3fb75..67c6511 100644
--- a/arch/arm/boot/dts/tegra-cardhu.dts
+++ b/arch/arm/boot/dts/tegra-cardhu.dts
@@ -54,6 +54,7 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
 		power-gpios = <&gpio 31 0>; /* gpio PD7 */
+		bus-width = <4>;
 	};
 
 	sdhci@78000200 {
@@ -66,5 +67,6 @@
 
 	sdhci@78000400 {
 		support-8bit;
+		bus-width = <8>;
 	};
 };
diff --git a/arch/arm/boot/dts/tegra-harmony.dts b/arch/arm/boot/dts/tegra-harmony.dts
index 6e8447d..e9cae68 100644
--- a/arch/arm/boot/dts/tegra-harmony.dts
+++ b/arch/arm/boot/dts/tegra-harmony.dts
@@ -100,6 +100,7 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 155 0>; /* gpio PT3 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000400 {
@@ -111,5 +112,6 @@
 		wp-gpios = <&gpio 59 0>; /* gpio PH3 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
 		support-8bit;
+		bus-width = <8>;
 	};
 };
diff --git a/arch/arm/boot/dts/tegra-paz00.dts b/arch/arm/boot/dts/tegra-paz00.dts
index 6c02abb..03d3d79 100644
--- a/arch/arm/boot/dts/tegra-paz00.dts
+++ b/arch/arm/boot/dts/tegra-paz00.dts
@@ -97,6 +97,7 @@
 		cd-gpios = <&gpio 173 0>; /* gpio PV5 */
 		wp-gpios = <&gpio 57 0>;  /* gpio PH1 */
 		power-gpios = <&gpio 169 0>; /* gpio PV1 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000200 {
@@ -109,6 +110,7 @@
 
 	sdhci@c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	gpio-keys {
diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
index dbf1c5a..8decf72 100644
--- a/arch/arm/boot/dts/tegra-seaboard.dts
+++ b/arch/arm/boot/dts/tegra-seaboard.dts
@@ -104,10 +104,12 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	usb@c5000000 {
diff --git a/arch/arm/boot/dts/tegra-ventana.dts b/arch/arm/boot/dts/tegra-ventana.dts
index 2dcff87..29e54c7 100644
--- a/arch/arm/boot/dts/tegra-ventana.dts
+++ b/arch/arm/boot/dts/tegra-ventana.dts
@@ -100,9 +100,11 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 };
diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
index 7e283c8..fe0d609 100644
--- a/arch/powerpc/boot/dts/mpc8569mds.dts
+++ b/arch/powerpc/boot/dts/mpc8569mds.dts
@@ -119,6 +119,7 @@
 		sdhc@2e000 {
 			status = "disabled";
 			sdhci,1-bit-only;
+			bus-width = <1>;
 		};
 
 		par_io@e0100 {
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 47adb16..ae48fc7 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1766,7 +1766,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 		pdata->slots[0].nonremovable = true;
 		pdata->slots[0].no_regulator_off_init = true;
 	}
-	of_property_read_u32(np, "ti,bus-width", &bus_width);
+	of_property_read_u32(np, "bus-width", &bus_width);
 	if (bus_width == 4)
 		pdata->slots[0].caps |= MMC_CAP_4_BIT_DATA;
 	else if (bus_width == 8)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6193a0d..586a818 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -402,7 +402,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 	if (!np)
 		return -ENODEV;
 
-	if (of_get_property(np, "fsl,card-wired", NULL))
+	if (of_get_property(np, "non-removable", NULL))
 		boarddata->cd_type = ESDHC_CD_PERMANENT;
 
 	if (of_get_property(np, "fsl,cd-controller", NULL))
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index c5c2a48..a3858d0 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -42,7 +42,8 @@ static struct sdhci_ops sdhci_pltfm_ops = {
 #ifdef CONFIG_OF
 static bool sdhci_of_wp_inverted(struct device_node *np)
 {
-	if (of_get_property(np, "sdhci,wp-inverted", NULL))
+	if (of_get_property(np, "sdhci,wp-inverted", NULL) ||
+	    of_get_property(np, "wp-inverted", NULL))
 		return true;
 
 	/* Old device trees don't have the wp-inverted property. */
@@ -59,13 +60,16 @@ void sdhci_get_of_property(struct platform_device *pdev)
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	const __be32 *clk;
+	u32 bus_width;
 	int size;
 
 	if (of_device_is_available(np)) {
 		if (of_get_property(np, "sdhci,auto-cmd12", NULL))
 			host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
 
-		if (of_get_property(np, "sdhci,1-bit-only", NULL))
+		if (of_get_property(np, "sdhci,1-bit-only", NULL) ||
+		    (of_property_read_u32(np, "bus-width", &bus_width) == 0 &&
+		    bus_width = 1))
 			host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
 
 		if (sdhci_of_wp_inverted(np))

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Stephen Warren]

On 03/30/2012 05:36 AM, Arnd Bergmann wrote:
> On Friday 30 March 2012, Viresh Kumar wrote:
>> On 3/27/2012 9:49 PM, Arnd Bergmann wrote:
>>> These bindings came up in a discussion IRC today. I think it's rather bad that
>>> we can't agree on a common way to name the properties for mmc. We have
>>> bindings being proposed or already included from Anton, Stephen, Shawn,
>>> Rajendra, Viresh, Lee and Thomas. Almost all of them define GPIO pins
>>> for card detect and write protect, as well properties to define the bus
>>> width and high-speed modes, but we seem to have almost as many different
>>> definitions of these as we have drivers.
>>>
>>> Can we please come up with a common binding for these?
>>
>> Is there any progress on this? Sorry i wasn't following all mails.
>> How should i progress for sdhci-spear?
> 
> No progress so far. I would suggest we apply the patch below to unify
> the bindings we have. I tried to minimize the impact by picking the most
> common version for each property, but if we know about devices that would
> get broken by this, we may have to be more careful.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -0,0 +1,25 @@
> +These properties are common to multiple MMC host controllers. Any host
> +that requires the respective functionality should implement them using
> +these definitions.
> +
> +Required properties:
> +- bus-width: Number of data lines, can be <1>, <4>, or <8>

That property looks very reasonable.

Question: This would be a non-backwards-compatible change to the binding
definition. How should this be handled? In the past, I believe it's been
stated that new kernels need to run against old device trees, and hence
once a DT binding was defined and in use, it couldn't change except in a
backwards-compatible way. However, more recently, Grant has said that
his opinion is that (some or all?) bindings are currently considered
experimental and subject to change. And besides, the .dts files are
contained in the kernel tree at present... Some generally stated and
agreed upon policy here might be useful.

> +Optional properties:
> +- cd-gpios : Specify GPIOs for card detection, see gpio binding
> +- wp-gpios : Specify GPIOs for write protection, see gpio binding

> +- cd-inverted: when present, polarity on the wp gpio line is inverted
> +- wp-inverted: when present, polarity on the wp gpio line is inverted

I'm not sure about those two: Some of the GPIO bindings have flags in
the GPIO specifier (Tegra, ARM PL061, gpio.txt mentions the possibility
of polarity being in the specifier), and bit 0 of the flags is used to
indicate inversion. I think that either we should rely on GPIO
specifiers having such flags and remove these xxx-inverted properties
from the MMC binding, or remove that flag bit from the GPIO bindings.

Note that anything using of_gpio_simple_xlate() is going to end up using
the GPIO flag definitions from <linux/of_gpio.h> in their GPIO
specifier, and there a number of active users of this feature; grep for
OF_GPIO_ACTIVE_LOW.

The rather begs the question why of_get_named_gpio() exists; surely
of_get_named_gpio_flags() should always be used so that consumers know
whether the GPIO value should be inverted, or are the GPIO flags
supposed to be processed by the OF/GPIO core or GPIO driver somehow, and
act transparently to GPIO consumers?

> diff --git a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
...
> -- interrupts : Should contain SD/MMC interrupt
> +- interrupt  : Should contain SD/MMC interrupt

Isn't that usually pluralized, so interrupts?

> +- bus-width : Number of data lines, can be <1>, <4>, or <8>

For the device-specific binding documentation, rather than repeating the
core bindings, shouldn't we say something like:

This binding is based on the core MMC bindings documented in mmc.txt.
This file only documents additions or changes to those bindings.

... and then remove any of the common properties from the individual files?

> diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
...
> @@ -66,5 +67,6 @@
>  
>  	sdhci@78000400 {
>  		support-8bit;
> +		bus-width = <8>;
>  	};
>  };

Ah OK, so the first phase is to add all the new standardize properties,
then later remove all the legacy properties once the drivers have been
updated.

You've missed additions of "non-removable", but I can add them later. or
provide you an incremental patch or something.

> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c

This doesn't seem to decode cd-inverted, or do anything with the
bus-width property value that it reads. Was that intentional?

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Arnd Bergmann]

On Friday 30 March 2012, Stephen Warren wrote:
> That property looks very reasonable.
> 
> Question: This would be a non-backwards-compatible change to the binding
> definition. How should this be handled? In the past, I believe it's been
> stated that new kernels need to run against old device trees, and hence
> once a DT binding was defined and in use, it couldn't change except in a
> backwards-compatible way. However, more recently, Grant has said that
> his opinion is that (some or all?) bindings are currently considered
> experimental and subject to change. And besides, the .dts files are
> contained in the kernel tree at present... Some generally stated and
> agreed upon policy here might be useful.

I tried to leave the ones that look like they've been around for a while
backwards compatible (gpio, esdhc), while changing the relatively new
ones without backwards compatibility code.

> > +Optional properties:
> > +- cd-gpios : Specify GPIOs for card detection, see gpio binding
> > +- wp-gpios : Specify GPIOs for write protection, see gpio binding
> 
> > +- cd-inverted: when present, polarity on the wp gpio line is inverted
> > +- wp-inverted: when present, polarity on the wp gpio line is inverted
> 
> I'm not sure about those two: Some of the GPIO bindings have flags in
> the GPIO specifier (Tegra, ARM PL061, gpio.txt mentions the possibility
> of polarity being in the specifier), and bit 0 of the flags is used to
> indicate inversion. I think that either we should rely on GPIO
> specifiers having such flags and remove these xxx-inverted properties
> from the MMC binding, or remove that flag bit from the GPIO bindings.

Maybe the GPIO maintainer can comment on this. My understanding is
that not all gpio controllers support this in their bindings. If they
do, I agree that we can remove the extra properties here.

> Note that anything using of_gpio_simple_xlate() is going to end up using
> the GPIO flag definitions from <linux/of_gpio.h> in their GPIO
> specifier, and there a number of active users of this feature; grep for
> OF_GPIO_ACTIVE_LOW.
> 
> The rather begs the question why of_get_named_gpio() exists; surely
> of_get_named_gpio_flags() should always be used so that consumers know
> whether the GPIO value should be inverted, or are the GPIO flags
> supposed to be processed by the OF/GPIO core or GPIO driver somehow, and
> act transparently to GPIO consumers?
> 
> > diff --git a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
> ...
> > -- interrupts : Should contain SD/MMC interrupt
> > +- interrupt  : Should contain SD/MMC interrupt
> 
> Isn't that usually pluralized, so interrupts?

Right, my mistake.

> > +- bus-width : Number of data lines, can be <1>, <4>, or <8>
> 
> For the device-specific binding documentation, rather than repeating the
> core bindings, shouldn't we say something like:
> 
> This binding is based on the core MMC bindings documented in mmc.txt.
> This file only documents additions or changes to those bindings.
> 
> ... and then remove any of the common properties from the individual files?

yes, good idea.

> > diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
> ...
> > @@ -66,5 +67,6 @@
> >  
> >       sdhci@78000400 {
> >               support-8bit;
> > +             bus-width = <8>;
> >       };
> >  };
> 
> Ah OK, so the first phase is to add all the new standardize properties,
> then later remove all the legacy properties once the drivers have been
> updated.
> 
> You've missed additions of "non-removable", but I can add them later. or
> provide you an incremental patch or something.
> 
> > diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> 
> This doesn't seem to decode cd-inverted, or do anything with the
> bus-width property value that it reads. Was that intentional?

For now, I was trying to get the binding document right. I suppose as a follow-up,
we can actually add a common helper function to decode these attributes and set
the right flag in the mmc device, but that is not urgent. Right now, I mainly
want to make sure we gain no new users that have conflicting bindings.

	Arnd

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Mark Brown]

On Fri, Mar 30, 2012 at 06:45:07PM +0000, Arnd Bergmann wrote:
> On Friday 30 March 2012, Stephen Warren wrote:

> > > +- cd-inverted: when present, polarity on the wp gpio line is inverted
> > > +- wp-inverted: when present, polarity on the wp gpio line is inverted

> > I'm not sure about those two: Some of the GPIO bindings have flags in
> > the GPIO specifier (Tegra, ARM PL061, gpio.txt mentions the possibility
> > of polarity being in the specifier), and bit 0 of the flags is used to
> > indicate inversion. I think that either we should rely on GPIO
> > specifiers having such flags and remove these xxx-inverted properties
> > from the MMC binding, or remove that flag bit from the GPIO bindings.

> Maybe the GPIO maintainer can comment on this. My understanding is
> that not all gpio controllers support this in their bindings. If they
> do, I agree that we can remove the extra properties here.

Surely it'd be better to fix this at the GPIO controller end rather than
having a patchwork of approaches?

[PATCH v2] mmc: dt: Consolidate DT bindings

[Chris Ball]

Hi Arnd,

On Fri, Mar 30 2012, Stephen Warren wrote:
>> No progress so far. I would suggest we apply the patch below to unify
>> the bindings we have. I tried to minimize the impact by picking the most
>> common version for each property, but if we know about devices that would
>> get broken by this, we may have to be more careful.
>> 
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Here's a version of your MMC DT bindings patch that responds to Stephen
Warren's review comments, refreshed against latest linux-next -- I'd
like to get this merged for v3.5.  Since it touches many .dts files,
should it go via arm-soc?  Thanks.


From: Arnd Bergmann <arnd@arndb.de>
Subject: [PATCH] mmc: dt: Consolidate DT bindings

This patch unifies the current DT MMC bindings documentation and code,
adds generic MMC DT bindings documentation, and updates .dts files for
consistency.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[cjb: typo fixes, addition of max-frequency property]
Signed-off-by: Chris Ball <cjb@laptop.org>
---
Changes since Arnd's v1:
 * Typo fix from Stephen Warren: s/interrupt/interrupts/
 * Typo fix: s/non-removable/ti,non-removable/. This one's being
   preserved for backwards compatibility.
 * Add max-frequency property.

.../devicetree/bindings/mmc/fsl-esdhc.txt          |    6 +++--
 .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |    2 +-
 .../devicetree/bindings/mmc/mmc-spi-slot.txt       |    3 ++-
 Documentation/devicetree/bindings/mmc/mmc.txt      |   27 ++++++++++++++++++++
 .../devicetree/bindings/mmc/nvidia-sdhci.txt       |    4 +--
 .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    4 +--
 arch/arm/boot/dts/imx53-smd.dts                    |    4 +--
 arch/arm/boot/dts/imx6q-arm2.dts                   |    2 +-
 arch/arm/boot/dts/omap3-beagle.dts                 |    2 +-
 arch/arm/boot/dts/omap4-panda.dts                  |    4 +--
 arch/arm/boot/dts/omap4-sdp.dts                    |    6 ++---
 arch/arm/boot/dts/tegra-cardhu.dts                 |    2 ++
 arch/arm/boot/dts/tegra-harmony.dts                |    2 ++
 arch/arm/boot/dts/tegra-paz00.dts                  |    2 ++
 arch/arm/boot/dts/tegra-seaboard.dts               |    2 ++
 arch/arm/boot/dts/tegra-ventana.dts                |    2 ++
 arch/powerpc/boot/dts/mpc8569mds.dts               |    1 +
 drivers/mmc/host/omap_hsmmc.c                      |    2 +-
 drivers/mmc/host/sdhci-esdhc-imx.c                 |    2 +-
 drivers/mmc/host/sdhci-pltfm.c                     |    8 ++++--
 20 files changed, 66 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc.txt

diff --git a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
index 64bcb8b..0d93b4b 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
@@ -11,9 +11,11 @@ Required properties:
   - interrupt-parent : interrupt source phandle.
   - clock-frequency : specifies eSDHC base clock frequency.
   - sdhci,wp-inverted : (optional) specifies that eSDHC controller
-    reports inverted write-protect state;
+    reports inverted write-protect state; New devices should use
+    the generic "wp-inverted" property.
   - sdhci,1-bit-only : (optional) specifies that a controller can
-    only handle 1-bit data transfers.
+    only handle 1-bit data transfers. New devices should use the
+    generic "bus-width = <1>" property.
   - sdhci,auto-cmd12: (optional) specifies that a controller can
     only handle auto CMD12.
 
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
index ab22fe6..c7e404b 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -9,7 +9,7 @@ Required properties:
 - interrupts : Should contain eSDHC interrupt
 
 Optional properties:
-- fsl,card-wired : Indicate the card is wired to host permanently
+- non-removable : 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
diff --git a/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt b/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
index 89a0084..d64aea5 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
@@ -10,7 +10,8 @@ Required properties:
 
 Optional properties:
 - gpios : may specify GPIOs in this order: Card-Detect GPIO,
-  Write-Protect GPIO.
+  Write-Protect GPIO. Note that this does not follow the
+  binding from mmc.txt, for historic reasons.
 - interrupts : the interrupt of a card detect interrupt.
 - interrupt-parent : the phandle for the interrupt controller that
   services interrupts for this device.
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
new file mode 100644
index 0000000..6e70dcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -0,0 +1,27 @@
+These properties are common to multiple MMC host controllers. Any host
+that requires the respective functionality should implement them using
+these definitions.
+
+Required properties:
+- bus-width: Number of data lines, can be <1>, <4>, or <8>
+
+Optional properties:
+- cd-gpios : Specify GPIOs for card detection, see gpio binding
+- wp-gpios : Specify GPIOs for write protection, see gpio binding
+- cd-inverted: when present, polarity on the wp gpio line is inverted
+- wp-inverted: when present, polarity on the wp gpio line is inverted
+- non-removable: non-removable slot (like eMMC)
+- max-frequency: maximum operating clock frequency
+
+Example:
+
+sdhci@ab000000 {
+	compatible = "sdhci";
+	reg = <0xab000000 0x200>;
+	interrupts = <23>;
+	bus-width = <4>;
+	cd-gpios = <&gpio 69 0>;
+	cd-inverted;
+	wp-gpios = <&gpio 70 0>;
+	max-frequency = <50000000>;
+}
diff --git a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
index 7e51154..f77c303 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
@@ -7,12 +7,12 @@ Required properties:
 - compatible : Should be "nvidia,<chip>-sdhci"
 - reg : Should contain SD/MMC registers location and length
 - interrupts : Should contain SD/MMC interrupt
+- bus-width : Number of data lines, can be <1>, <4>, or <8>
 
 Optional properties:
 - cd-gpios : Specify GPIOs for card detection
 - wp-gpios : Specify GPIOs for write protection
 - power-gpios : Specify GPIOs for power control
-- support-8bit : Boolean, indicates if 8-bit mode should be used.
 
 Example:
 
@@ -23,5 +23,5 @@ sdhci@c8000200 {
 	cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 	wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 	power-gpios = <&gpio 155 0>; /* gpio PT3 */
-	support-8bit;
+	bus-width = <8>;
 };
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index dbd4368..8a53958 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -15,7 +15,7 @@ Optional properties:
 ti,dual-volt: boolean, supports dual voltage cards
 <supply-name>-supply: phandle to the regulator device tree node
 "supply-name" examples are "vmmc", "vmmc_aux" etc
-ti,bus-width: Number of data lines, default assumed is 1 if the property is missing.
+bus-width: Number of data lines, default assumed is 1 if the property is missing.
 cd-gpios: GPIOs for card detection
 wp-gpios: GPIOs for write protection
 ti,non-removable: non-removable slot (like eMMC)
@@ -27,7 +27,7 @@ Example:
 		reg = <0x4809c000 0x400>;
 		ti,hwmods = "mmc1";
 		ti,dual-volt;
-		ti,bus-width = <4>;
+		bus-width = <4>;
 		vmmc-supply = <&vmmc>; /* phandle to regulator node */
 		ti,non-removable;
 	};
diff --git a/arch/arm/boot/dts/imx53-smd.dts b/arch/arm/boot/dts/imx53-smd.dts
index c7ee86c..139138a 100644
--- a/arch/arm/boot/dts/imx53-smd.dts
+++ b/arch/arm/boot/dts/imx53-smd.dts
@@ -35,7 +35,7 @@
 				};
 
 				esdhc@50008000 { /* ESDHC2 */
-					fsl,card-wired;
+					non-removable;
 					status = "okay";
 				};
 
@@ -76,7 +76,7 @@
 				};
 
 				esdhc@50020000 { /* ESDHC3 */
-					fsl,card-wired;
+					non-removable;
 					status = "okay";
 				};
 			};
diff --git a/arch/arm/boot/dts/imx6q-arm2.dts b/arch/arm/boot/dts/imx6q-arm2.dts
index ce1c823..d2eaf52 100644
--- a/arch/arm/boot/dts/imx6q-arm2.dts
+++ b/arch/arm/boot/dts/imx6q-arm2.dts
@@ -41,7 +41,7 @@
 			};
 
 			usdhc@0219c000 { /* uSDHC4 */
-				fsl,card-wired;
+				non-removable;
 				vmmc-supply = <&reg_3p3v>;
 				status = "okay";
 			};
diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index 8c756be..5b4506c 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -57,7 +57,7 @@
 &mmc1 {
 	vmmc-supply = <&vmmc1>;
 	vmmc_aux-supply = <&vsim>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {
diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
index ea6f5bb..31fb421 100644
--- a/arch/arm/boot/dts/omap4-panda.dts
+++ b/arch/arm/boot/dts/omap4-panda.dts
@@ -55,7 +55,7 @@
 
 &mmc1 {
 	vmmc-supply = <&vmmc>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {
@@ -72,5 +72,5 @@
 
 &mmc5 {
 	ti,non-removable;
-	ti,bus-width = <4>;
+	bus-width = <4>;
 };
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 67b2e98..a1dd873 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -94,12 +94,12 @@
 
 &mmc1 {
 	vmmc-supply = <&vmmc>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {
 	vmmc-supply = <&vaux1>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 	ti,non-removable;
 };
 
@@ -112,6 +112,6 @@
 };
 
 &mmc5 {
-	ti,bus-width = <4>;
+	bus-width = <4>;
 	ti,non-removable;
 };
diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
index facb950..f00480a 100644
--- a/arch/arm/boot/dts/tegra-cardhu.dts
+++ b/arch/arm/boot/dts/tegra-cardhu.dts
@@ -129,6 +129,7 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
 		power-gpios = <&gpio 31 0>; /* gpio PD7 */
+		bus-width = <4>;
 	};
 
 	sdhci@78000200 {
@@ -141,6 +142,7 @@
 
 	sdhci@78000400 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	ahub@70080000 {
diff --git a/arch/arm/boot/dts/tegra-harmony.dts b/arch/arm/boot/dts/tegra-harmony.dts
index 6857cec..7cd513a 100644
--- a/arch/arm/boot/dts/tegra-harmony.dts
+++ b/arch/arm/boot/dts/tegra-harmony.dts
@@ -324,6 +324,7 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 155 0>; /* gpio PT3 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000400 {
@@ -335,6 +336,7 @@
 		wp-gpios = <&gpio 59 0>; /* gpio PH3 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	usb@c5004000 {
diff --git a/arch/arm/boot/dts/tegra-paz00.dts b/arch/arm/boot/dts/tegra-paz00.dts
index 34a9177..8d625e4 100644
--- a/arch/arm/boot/dts/tegra-paz00.dts
+++ b/arch/arm/boot/dts/tegra-paz00.dts
@@ -313,6 +313,7 @@
 		cd-gpios = <&gpio 173 0>; /* gpio PV5 */
 		wp-gpios = <&gpio 57 0>;  /* gpio PH1 */
 		power-gpios = <&gpio 169 0>; /* gpio PV1 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000200 {
@@ -325,6 +326,7 @@
 
 	sdhci@c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	gpio-keys {
diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
index c4d171e..38e40a1 100644
--- a/arch/arm/boot/dts/tegra-seaboard.dts
+++ b/arch/arm/boot/dts/tegra-seaboard.dts
@@ -380,10 +380,12 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	usb@c5000000 {
diff --git a/arch/arm/boot/dts/tegra-ventana.dts b/arch/arm/boot/dts/tegra-ventana.dts
index c20d5e9..149ab6d 100644
--- a/arch/arm/boot/dts/tegra-ventana.dts
+++ b/arch/arm/boot/dts/tegra-ventana.dts
@@ -338,10 +338,12 @@
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
+		bus-width = <4>;
 	};
 
 	sdhci@c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	usb@c5004000 {
diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
index 7e283c8..fe0d609 100644
--- a/arch/powerpc/boot/dts/mpc8569mds.dts
+++ b/arch/powerpc/boot/dts/mpc8569mds.dts
@@ -119,6 +119,7 @@
 		sdhc@2e000 {
 			status = "disabled";
 			sdhci,1-bit-only;
+			bus-width = <1>;
 		};
 
 		par_io@e0100 {
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index d15b149..ebaf62a 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1781,7 +1781,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 		pdata->slots[0].nonremovable = true;
 		pdata->slots[0].no_regulator_off_init = true;
 	}
-	of_property_read_u32(np, "ti,bus-width", &bus_width);
+	of_property_read_u32(np, "bus-width", &bus_width);
 	if (bus_width == 4)
 		pdata->slots[0].caps |= MMC_CAP_4_BIT_DATA;
 	else if (bus_width == 8)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 8abdaf6..0d2b082 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -402,7 +402,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 	if (!np)
 		return -ENODEV;
 
-	if (of_get_property(np, "fsl,card-wired", NULL))
+	if (of_get_property(np, "non-removable", NULL))
 		boarddata->cd_type = ESDHC_CD_PERMANENT;
 
 	if (of_get_property(np, "fsl,cd-controller", NULL))
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index c5c2a48..a3858d0 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -42,7 +42,8 @@ static struct sdhci_ops sdhci_pltfm_ops = {
 #ifdef CONFIG_OF
 static bool sdhci_of_wp_inverted(struct device_node *np)
 {
-	if (of_get_property(np, "sdhci,wp-inverted", NULL))
+	if (of_get_property(np, "sdhci,wp-inverted", NULL) ||
+	    of_get_property(np, "wp-inverted", NULL))
 		return true;
 
 	/* Old device trees don't have the wp-inverted property. */
@@ -59,13 +60,16 @@ void sdhci_get_of_property(struct platform_device *pdev)
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	const __be32 *clk;
+	u32 bus_width;
 	int size;
 
 	if (of_device_is_available(np)) {
 		if (of_get_property(np, "sdhci,auto-cmd12", NULL))
 			host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
 
-		if (of_get_property(np, "sdhci,1-bit-only", NULL))
+		if (of_get_property(np, "sdhci,1-bit-only", NULL) ||
+		    (of_property_read_u32(np, "bus-width", &bus_width) == 0 &&
+		    bus_width = 1))
 			host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
 
 		if (sdhci_of_wp_inverted(np))
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2] mmc: dt: Consolidate DT bindings

[Guennadi Liakhovetski]

Hi Chris

Good to see this happening! Is anyone also planning to submit a generic OF 
parser to convert OF properties into respective host capability flags? I 
didn't thoroughly review this patch, just a minor typo correction:

On Sun, 13 May 2012, Chris Ball wrote:

> Hi Arnd,
> 
> On Fri, Mar 30 2012, Stephen Warren wrote:
> >> No progress so far. I would suggest we apply the patch below to unify
> >> the bindings we have. I tried to minimize the impact by picking the most
> >> common version for each property, but if we know about devices that would
> >> get broken by this, we may have to be more careful.
> >> 
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Here's a version of your MMC DT bindings patch that responds to Stephen
> Warren's review comments, refreshed against latest linux-next -- I'd
> like to get this merged for v3.5.  Since it touches many .dts files,
> should it go via arm-soc?  Thanks.
> 
> 
> From: Arnd Bergmann <arnd@arndb.de>
> Subject: [PATCH] mmc: dt: Consolidate DT bindings
> 
> This patch unifies the current DT MMC bindings documentation and code,
> adds generic MMC DT bindings documentation, and updates .dts files for
> consistency.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> [cjb: typo fixes, addition of max-frequency property]
> Signed-off-by: Chris Ball <cjb@laptop.org>
> ---
> Changes since Arnd's v1:
>  * Typo fix from Stephen Warren: s/interrupt/interrupts/
>  * Typo fix: s/non-removable/ti,non-removable/. This one's being
>    preserved for backwards compatibility.
>  * Add max-frequency property.
> 
> .../devicetree/bindings/mmc/fsl-esdhc.txt          |    6 +++--
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.txt      |    2 +-
>  .../devicetree/bindings/mmc/mmc-spi-slot.txt       |    3 ++-
>  Documentation/devicetree/bindings/mmc/mmc.txt      |   27 ++++++++++++++++++++
>  .../devicetree/bindings/mmc/nvidia-sdhci.txt       |    4 +--
>  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    4 +--
>  arch/arm/boot/dts/imx53-smd.dts                    |    4 +--
>  arch/arm/boot/dts/imx6q-arm2.dts                   |    2 +-
>  arch/arm/boot/dts/omap3-beagle.dts                 |    2 +-
>  arch/arm/boot/dts/omap4-panda.dts                  |    4 +--
>  arch/arm/boot/dts/omap4-sdp.dts                    |    6 ++---
>  arch/arm/boot/dts/tegra-cardhu.dts                 |    2 ++
>  arch/arm/boot/dts/tegra-harmony.dts                |    2 ++
>  arch/arm/boot/dts/tegra-paz00.dts                  |    2 ++
>  arch/arm/boot/dts/tegra-seaboard.dts               |    2 ++
>  arch/arm/boot/dts/tegra-ventana.dts                |    2 ++
>  arch/powerpc/boot/dts/mpc8569mds.dts               |    1 +
>  drivers/mmc/host/omap_hsmmc.c                      |    2 +-
>  drivers/mmc/host/sdhci-esdhc-imx.c                 |    2 +-
>  drivers/mmc/host/sdhci-pltfm.c                     |    8 ++++--
>  20 files changed, 66 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/mmc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> index 64bcb8b..0d93b4b 100644
> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> @@ -11,9 +11,11 @@ Required properties:
>    - interrupt-parent : interrupt source phandle.
>    - clock-frequency : specifies eSDHC base clock frequency.
>    - sdhci,wp-inverted : (optional) specifies that eSDHC controller
> -    reports inverted write-protect state;
> +    reports inverted write-protect state; New devices should use
> +    the generic "wp-inverted" property.
>    - sdhci,1-bit-only : (optional) specifies that a controller can
> -    only handle 1-bit data transfers.
> +    only handle 1-bit data transfers. New devices should use the
> +    generic "bus-width = <1>" property.
>    - sdhci,auto-cmd12: (optional) specifies that a controller can
>      only handle auto CMD12.
>  
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> index ab22fe6..c7e404b 100644
> --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> @@ -9,7 +9,7 @@ Required properties:
>  - interrupts : Should contain eSDHC interrupt
>  
>  Optional properties:
> -- fsl,card-wired : Indicate the card is wired to host permanently
> +- non-removable : 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
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt b/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
> index 89a0084..d64aea5 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt
> @@ -10,7 +10,8 @@ Required properties:
>  
>  Optional properties:
>  - gpios : may specify GPIOs in this order: Card-Detect GPIO,
> -  Write-Protect GPIO.
> +  Write-Protect GPIO. Note that this does not follow the
> +  binding from mmc.txt, for historic reasons.
>  - interrupts : the interrupt of a card detect interrupt.
>  - interrupt-parent : the phandle for the interrupt controller that
>    services interrupts for this device.
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> new file mode 100644
> index 0000000..6e70dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -0,0 +1,27 @@
> +These properties are common to multiple MMC host controllers. Any host
> +that requires the respective functionality should implement them using
> +these definitions.
> +
> +Required properties:
> +- bus-width: Number of data lines, can be <1>, <4>, or <8>
> +
> +Optional properties:
> +- cd-gpios : Specify GPIOs for card detection, see gpio binding
> +- wp-gpios : Specify GPIOs for write protection, see gpio binding
> +- cd-inverted: when present, polarity on the wp gpio line is inverted

You certainly meant "cd gpio line is inverted." But in fact, as Olof 
proposed in his reply to my patch - cannot we just use the existing 
OF_GPIO_ACTIVE_LOW flag to invert CD and WP polarity?

Thanks
Guennadi

> +- wp-inverted: when present, polarity on the wp gpio line is inverted
> +- non-removable: non-removable slot (like eMMC)
> +- max-frequency: maximum operating clock frequency
> +
> +Example:
> +
> +sdhci@ab000000 {
> +	compatible = "sdhci";
> +	reg = <0xab000000 0x200>;
> +	interrupts = <23>;
> +	bus-width = <4>;
> +	cd-gpios = <&gpio 69 0>;
> +	cd-inverted;
> +	wp-gpios = <&gpio 70 0>;
> +	max-frequency = <50000000>;
> +}
> diff --git a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
> index 7e51154..f77c303 100644
> --- a/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt
> @@ -7,12 +7,12 @@ Required properties:
>  - compatible : Should be "nvidia,<chip>-sdhci"
>  - reg : Should contain SD/MMC registers location and length
>  - interrupts : Should contain SD/MMC interrupt
> +- bus-width : Number of data lines, can be <1>, <4>, or <8>
>  
>  Optional properties:
>  - cd-gpios : Specify GPIOs for card detection
>  - wp-gpios : Specify GPIOs for write protection
>  - power-gpios : Specify GPIOs for power control
> -- support-8bit : Boolean, indicates if 8-bit mode should be used.
>  
>  Example:
>  
> @@ -23,5 +23,5 @@ sdhci@c8000200 {
>  	cd-gpios = <&gpio 69 0>; /* gpio PI5 */
>  	wp-gpios = <&gpio 57 0>; /* gpio PH1 */
>  	power-gpios = <&gpio 155 0>; /* gpio PT3 */
> -	support-8bit;
> +	bus-width = <8>;
>  };
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index dbd4368..8a53958 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -15,7 +15,7 @@ Optional properties:
>  ti,dual-volt: boolean, supports dual voltage cards
>  <supply-name>-supply: phandle to the regulator device tree node
>  "supply-name" examples are "vmmc", "vmmc_aux" etc
> -ti,bus-width: Number of data lines, default assumed is 1 if the property is missing.
> +bus-width: Number of data lines, default assumed is 1 if the property is missing.
>  cd-gpios: GPIOs for card detection
>  wp-gpios: GPIOs for write protection
>  ti,non-removable: non-removable slot (like eMMC)
> @@ -27,7 +27,7 @@ Example:
>  		reg = <0x4809c000 0x400>;
>  		ti,hwmods = "mmc1";
>  		ti,dual-volt;
> -		ti,bus-width = <4>;
> +		bus-width = <4>;
>  		vmmc-supply = <&vmmc>; /* phandle to regulator node */
>  		ti,non-removable;
>  	};
> diff --git a/arch/arm/boot/dts/imx53-smd.dts b/arch/arm/boot/dts/imx53-smd.dts
> index c7ee86c..139138a 100644
> --- a/arch/arm/boot/dts/imx53-smd.dts
> +++ b/arch/arm/boot/dts/imx53-smd.dts
> @@ -35,7 +35,7 @@
>  				};
>  
>  				esdhc@50008000 { /* ESDHC2 */
> -					fsl,card-wired;
> +					non-removable;
>  					status = "okay";
>  				};
>  
> @@ -76,7 +76,7 @@
>  				};
>  
>  				esdhc@50020000 { /* ESDHC3 */
> -					fsl,card-wired;
> +					non-removable;
>  					status = "okay";
>  				};
>  			};
> diff --git a/arch/arm/boot/dts/imx6q-arm2.dts b/arch/arm/boot/dts/imx6q-arm2.dts
> index ce1c823..d2eaf52 100644
> --- a/arch/arm/boot/dts/imx6q-arm2.dts
> +++ b/arch/arm/boot/dts/imx6q-arm2.dts
> @@ -41,7 +41,7 @@
>  			};
>  
>  			usdhc@0219c000 { /* uSDHC4 */
> -				fsl,card-wired;
> +				non-removable;
>  				vmmc-supply = <&reg_3p3v>;
>  				status = "okay";
>  			};
> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
> index 8c756be..5b4506c 100644
> --- a/arch/arm/boot/dts/omap3-beagle.dts
> +++ b/arch/arm/boot/dts/omap3-beagle.dts
> @@ -57,7 +57,7 @@
>  &mmc1 {
>  	vmmc-supply = <&vmmc1>;
>  	vmmc_aux-supply = <&vsim>;
> -	ti,bus-width = <8>;
> +	bus-width = <8>;
>  };
>  
>  &mmc2 {
> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
> index ea6f5bb..31fb421 100644
> --- a/arch/arm/boot/dts/omap4-panda.dts
> +++ b/arch/arm/boot/dts/omap4-panda.dts
> @@ -55,7 +55,7 @@
>  
>  &mmc1 {
>  	vmmc-supply = <&vmmc>;
> -	ti,bus-width = <8>;
> +	bus-width = <8>;
>  };
>  
>  &mmc2 {
> @@ -72,5 +72,5 @@
>  
>  &mmc5 {
>  	ti,non-removable;
> -	ti,bus-width = <4>;
> +	bus-width = <4>;
>  };
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 67b2e98..a1dd873 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -94,12 +94,12 @@
>  
>  &mmc1 {
>  	vmmc-supply = <&vmmc>;
> -	ti,bus-width = <8>;
> +	bus-width = <8>;
>  };
>  
>  &mmc2 {
>  	vmmc-supply = <&vaux1>;
> -	ti,bus-width = <8>;
> +	bus-width = <8>;
>  	ti,non-removable;
>  };
>  
> @@ -112,6 +112,6 @@
>  };
>  
>  &mmc5 {
> -	ti,bus-width = <4>;
> +	bus-width = <4>;
>  	ti,non-removable;
>  };
> diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
> index facb950..f00480a 100644
> --- a/arch/arm/boot/dts/tegra-cardhu.dts
> +++ b/arch/arm/boot/dts/tegra-cardhu.dts
> @@ -129,6 +129,7 @@
>  		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
>  		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
>  		power-gpios = <&gpio 31 0>; /* gpio PD7 */
> +		bus-width = <4>;
>  	};
>  
>  	sdhci@78000200 {
> @@ -141,6 +142,7 @@
>  
>  	sdhci@78000400 {
>  		support-8bit;
> +		bus-width = <8>;
>  	};
>  
>  	ahub@70080000 {
> diff --git a/arch/arm/boot/dts/tegra-harmony.dts b/arch/arm/boot/dts/tegra-harmony.dts
> index 6857cec..7cd513a 100644
> --- a/arch/arm/boot/dts/tegra-harmony.dts
> +++ b/arch/arm/boot/dts/tegra-harmony.dts
> @@ -324,6 +324,7 @@
>  		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
>  		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
>  		power-gpios = <&gpio 155 0>; /* gpio PT3 */
> +		bus-width = <4>;
>  	};
>  
>  	sdhci@c8000400 {
> @@ -335,6 +336,7 @@
>  		wp-gpios = <&gpio 59 0>; /* gpio PH3 */
>  		power-gpios = <&gpio 70 0>; /* gpio PI6 */
>  		support-8bit;
> +		bus-width = <8>;
>  	};
>  
>  	usb@c5004000 {
> diff --git a/arch/arm/boot/dts/tegra-paz00.dts b/arch/arm/boot/dts/tegra-paz00.dts
> index 34a9177..8d625e4 100644
> --- a/arch/arm/boot/dts/tegra-paz00.dts
> +++ b/arch/arm/boot/dts/tegra-paz00.dts
> @@ -313,6 +313,7 @@
>  		cd-gpios = <&gpio 173 0>; /* gpio PV5 */
>  		wp-gpios = <&gpio 57 0>;  /* gpio PH1 */
>  		power-gpios = <&gpio 169 0>; /* gpio PV1 */
> +		bus-width = <4>;
>  	};
>  
>  	sdhci@c8000200 {
> @@ -325,6 +326,7 @@
>  
>  	sdhci@c8000600 {
>  		support-8bit;
> +		bus-width = <8>;
>  	};
>  
>  	gpio-keys {
> diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
> index c4d171e..38e40a1 100644
> --- a/arch/arm/boot/dts/tegra-seaboard.dts
> +++ b/arch/arm/boot/dts/tegra-seaboard.dts
> @@ -380,10 +380,12 @@
>  		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
>  		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
>  		power-gpios = <&gpio 70 0>; /* gpio PI6 */
> +		bus-width = <4>;
>  	};
>  
>  	sdhci@c8000600 {
>  		support-8bit;
> +		bus-width = <8>;
>  	};
>  
>  	usb@c5000000 {
> diff --git a/arch/arm/boot/dts/tegra-ventana.dts b/arch/arm/boot/dts/tegra-ventana.dts
> index c20d5e9..149ab6d 100644
> --- a/arch/arm/boot/dts/tegra-ventana.dts
> +++ b/arch/arm/boot/dts/tegra-ventana.dts
> @@ -338,10 +338,12 @@
>  		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
>  		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
>  		power-gpios = <&gpio 70 0>; /* gpio PI6 */
> +		bus-width = <4>;
>  	};
>  
>  	sdhci@c8000600 {
>  		support-8bit;
> +		bus-width = <8>;
>  	};
>  
>  	usb@c5004000 {
> diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
> index 7e283c8..fe0d609 100644
> --- a/arch/powerpc/boot/dts/mpc8569mds.dts
> +++ b/arch/powerpc/boot/dts/mpc8569mds.dts
> @@ -119,6 +119,7 @@
>  		sdhc@2e000 {
>  			status = "disabled";
>  			sdhci,1-bit-only;
> +			bus-width = <1>;
>  		};
>  
>  		par_io@e0100 {
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index d15b149..ebaf62a 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1781,7 +1781,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>  		pdata->slots[0].nonremovable = true;
>  		pdata->slots[0].no_regulator_off_init = true;
>  	}
> -	of_property_read_u32(np, "ti,bus-width", &bus_width);
> +	of_property_read_u32(np, "bus-width", &bus_width);
>  	if (bus_width == 4)
>  		pdata->slots[0].caps |= MMC_CAP_4_BIT_DATA;
>  	else if (bus_width == 8)
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 8abdaf6..0d2b082 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -402,7 +402,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
>  	if (!np)
>  		return -ENODEV;
>  
> -	if (of_get_property(np, "fsl,card-wired", NULL))
> +	if (of_get_property(np, "non-removable", NULL))
>  		boarddata->cd_type = ESDHC_CD_PERMANENT;
>  
>  	if (of_get_property(np, "fsl,cd-controller", NULL))
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index c5c2a48..a3858d0 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -42,7 +42,8 @@ static struct sdhci_ops sdhci_pltfm_ops = {
>  #ifdef CONFIG_OF
>  static bool sdhci_of_wp_inverted(struct device_node *np)
>  {
> -	if (of_get_property(np, "sdhci,wp-inverted", NULL))
> +	if (of_get_property(np, "sdhci,wp-inverted", NULL) ||
> +	    of_get_property(np, "wp-inverted", NULL))
>  		return true;
>  
>  	/* Old device trees don't have the wp-inverted property. */
> @@ -59,13 +60,16 @@ void sdhci_get_of_property(struct platform_device *pdev)
>  	struct sdhci_host *host = platform_get_drvdata(pdev);
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	const __be32 *clk;
> +	u32 bus_width;
>  	int size;
>  
>  	if (of_device_is_available(np)) {
>  		if (of_get_property(np, "sdhci,auto-cmd12", NULL))
>  			host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>  
> -		if (of_get_property(np, "sdhci,1-bit-only", NULL))
> +		if (of_get_property(np, "sdhci,1-bit-only", NULL) ||
> +		    (of_property_read_u32(np, "bus-width", &bus_width) == 0 &&
> +		    bus_width = 1))
>  			host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
>  
>  		if (sdhci_of_wp_inverted(np))
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

Re: [PATCH v2] mmc: dt: Consolidate DT bindings

[Arnd Bergmann]

On Sunday 13 May 2012, Chris Ball wrote:
> On Fri, Mar 30 2012, Stephen Warren wrote:
> >> No progress so far. I would suggest we apply the patch below to unify
> >> the bindings we have. I tried to minimize the impact by picking the most
> >> common version for each property, but if we know about devices that would
> >> get broken by this, we may have to be more careful.
> >> 
> >> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> Here's a version of your MMC DT bindings patch that responds to Stephen
> Warren's review comments, refreshed against latest linux-next -- I'd
> like to get this merged for v3.5.  Since it touches many .dts files,
> should it go via arm-soc?  Thanks.

Hi Chris,

Sorry for not getting to this sooner myself and many thanks for your
taking it on!

I've verified that there are no conflicts with anything we have in
arm-soc right now, so I think it should just go through your
tree.

	Arnd

Re: [PATCH v2] mmc: dt: Consolidate DT bindings

[Chris Ball]

Hi Arnd,

On Sun, May 13 2012, Arnd Bergmann wrote:
> I've verified that there are no conflicts with anything we have in
> arm-soc right now, so I think it should just go through your
> tree.

Ah, I based the patch on latest linux-next.  That's why there aren't
conflicts against arm-soc, but there are conflicts (in the omap and
tegra dts changes) between the patch and Linus master/mmc-next:

Applying: mmc: dt: Consolidate DT bindings
error: patch failed: arch/arm/boot/dts/omap3-beagle.dts:57
error: patch failed: arch/arm/boot/dts/omap4-panda.dts:55
error: patch failed: arch/arm/boot/dts/omap4-sdp.dts:94
error: patch failed: arch/arm/boot/dts/tegra-cardhu.dts:141
error: patch failed: arch/arm/boot/dts/tegra-harmony.dts:335
error: patch failed: arch/arm/boot/dts/tegra-ventana.dts:338
Patch failed at 0001 mmc: dt: Consolidate DT bindings

So I think merging via arm-soc is the only way to get it to Linus
without conflicts.  (Alternatively I can take it in mmc-next and
Linus can handle the conflicts.)

Thanks!

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

Re: [PATCH v2] mmc: dt: Consolidate DT bindings

[Arnd Bergmann]

On Sunday 13 May 2012, Chris Ball wrote:
> Hi Arnd,
> 
> On Sun, May 13 2012, Arnd Bergmann wrote:
> > I've verified that there are no conflicts with anything we have in
> > arm-soc right now, so I think it should just go through your
> > tree.
> 
> Ah, I based the patch on latest linux-next.  That's why there aren't
> conflicts against arm-soc, but there are conflicts (in the omap and
> tegra dts changes) between the patch and Linus master/mmc-next:
> 
> Applying: mmc: dt: Consolidate DT bindings
> error: patch failed: arch/arm/boot/dts/omap3-beagle.dts:57
> error: patch failed: arch/arm/boot/dts/omap4-panda.dts:55
> error: patch failed: arch/arm/boot/dts/omap4-sdp.dts:94
> error: patch failed: arch/arm/boot/dts/tegra-cardhu.dts:141
> error: patch failed: arch/arm/boot/dts/tegra-harmony.dts:335
> error: patch failed: arch/arm/boot/dts/tegra-ventana.dts:338
> Patch failed at 0001 mmc: dt: Consolidate DT bindings
> 
> So I think merging via arm-soc is the only way to get it to Linus
> without conflicts.  (Alternatively I can take it in mmc-next and
> Linus can handle the conflicts.)

I've created a drivers/mmc branch with this patch and its direct dependencies
now and pulled that into the next/drivers branch of arm-soc

Thanks,

	Arnd

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Chris Ball]

Hi Arnd,

On Fri, Mar 30 2012, Arnd Bergmann wrote:
> On Friday 30 March 2012, Viresh Kumar wrote:
>> On 3/27/2012 9:49 PM, Arnd Bergmann wrote:
>> > These bindings came up in a discussion IRC today. I think it's rather bad that
>> > we can't agree on a common way to name the properties for mmc. We have
>> > bindings being proposed or already included from Anton, Stephen, Shawn,
>> > Rajendra, Viresh, Lee and Thomas. Almost all of them define GPIO pins
>> > for card detect and write protect, as well properties to define the bus
>> > width and high-speed modes, but we seem to have almost as many different
>> > definitions of these as we have drivers.
>> > 
>> > Can we please come up with a common binding for these?
>> 
>> Is there any progress on this? Sorry i wasn't following all mails.
>> How should i progress for sdhci-spear?
>
> No progress so far. I would suggest we apply the patch below to unify
> the bindings we have. I tried to minimize the impact by picking the most
> common version for each property, but if we know about devices that would
> get broken by this, we may have to be more careful.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks very much for doing this -- I'll be happy to take the patch in
mmc-next once you get time to post a v2 covering Stephen Warren's review
comments (and adding "max-frequency" to the document, I guess).

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

Re: [PATCH v3 6/6] mmc: sdhci-s3c: Add device tree support

[Chris Ball]

Hi Arnd,

(Diff truncated to show relevant hunks.)

On Fri, Mar 30 2012, Arnd Bergmann wrote:
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index dbd4368..90b86e5 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -15,10 +15,10 @@ Optional properties:
>  ti,dual-volt: boolean, supports dual voltage cards
>  <supply-name>-supply: phandle to the regulator device tree node
>  "supply-name" examples are "vmmc", "vmmc_aux" etc
> -ti,bus-width: Number of data lines, default assumed is 1 if the property is missing.
> +bus-width: Number of data lines, default assumed is 1 if the property is missing.
>  cd-gpios: GPIOs for card detection
>  wp-gpios: GPIOs for write protection
> -ti,non-removable: non-removable slot (like eMMC)
> +non-removable: non-removable slot (like eMMC)
>  ti,needs-special-reset: Requires a special softreset sequence
>  
>  Example:
> @@ -27,7 +27,7 @@ Example:
>  		reg = <0x4809c000 0x400>;
>  		ti,hwmods = "mmc1";
>  		ti,dual-volt;
> -		ti,bus-width = <4>;
> +		bus-width = <4>;
>  		vmmc-supply = <&vmmc>; /* phandle to regulator node */
>  		ti,non-removable;
>  	};
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 47adb16..ae48fc7 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1766,7 +1766,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>  		pdata->slots[0].nonremovable = true;
>  		pdata->slots[0].no_regulator_off_init = true;
>  	}
> -	of_property_read_u32(np, "ti,bus-width", &bus_width);
> +	of_property_read_u32(np, "bus-width", &bus_width);
>  	if (bus_width == 4)
>  		pdata->slots[0].caps |= MMC_CAP_4_BIT_DATA;
>  	else if (bus_width == 8)

Here you change "ti,non-removable" to "non-removable" in the properties
section of ti-omap-hsmmc.txt without changing it in the example section
of that document, or in the code or shipped .dts files.

(Presumably you decided to preserve it for backwards compatibility, so
the bindings documentation shouldn't be changed.)

Thanks,

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

RE: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Kukjin Kim]

Thomas Abraham wrote:
> 
> This patchset removes all uses of 'clk_type' member from the platform data
> of sdhci-s3c driver and adds device tree support for sdhci-s3c driver.
> 
> In this patchset, all uses of 'clk_type' member from the platform data are
> removed from the sdhci-s3c driver and platform code. The clk_type is a SoC
> specific information and not a board/machine specific information. Hence,
> this
> information can be more aptly represented using SoC specific driver data
> in the
> sdhci-s3c driver.
> 
> Hence all uses of 'clk_type' member in sdhci-s3c driver's platform data is
> removed. In place of that, the sdhci host qurik
> 'SDHCI_QUIRK_NONSTANDARD_CLOCK'
> is used to handle controllers that do not have a standard sdclk division
> (like those in the exynos4 SoC's).
> 
> This is a pre-requisite for adding device tree support for sdhci-s3c
> driver.
> While migrating towards device tree support, retreving 'clk_type'
> information
> from device tree information does not seem correct and hence it has been
> added
> as SoC specific driver data. All this is handled in patches 1 to 3.
> 
> Patch 4 to 6 add device tree support for sdhci-s3c driver. The fourth
> patch
> derives the maximum transfer width host capability from the max_width
> member
> in platform data. The fifth patch modifies the sdhci-s3c driver to
> mainatain
> a local copy of the platform data, which makes it easier to add device
> tree
> support for the driver. The last patch adds device tree based discovery
> for
> the sdhci-s3c driver.
> 
> This patchset is based on the following tree.
>   http://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> [for-next]
> 
> Thomas Abraham (6):
>   mmc: sdhci-s3c: Remove usage of clk_type member in platform data
>   arm: exynos4: use 'exynos4-sdhci' as device name for sdhci controllers
>   arm: samsung: remove all uses of clk_type member in sdhci platform data
>   mmc: sdhci-s3c: derive transfer width host capability from max_width in
> platform data
>   mmc: sdhci-s3c: Keep a copy of platform data and use it
>   mmc: sdhci-s3c: Add device tree support
> 
>  .../devicetree/bindings/mmc/samsung-sdhci.txt      |   70 ++++++
>  arch/arm/mach-exynos/clock.c                       |   24 +-
>  arch/arm/mach-exynos/common.c                      |    5 +
>  arch/arm/mach-exynos/mach-armlex4210.c             |    3 -
>  arch/arm/mach-exynos/mach-nuri.c                   |    3 -
>  arch/arm/mach-exynos/mach-origen.c                 |    2 -
>  arch/arm/mach-exynos/mach-smdk4x12.c               |    2 -
>  arch/arm/mach-exynos/mach-smdkv310.c               |    4 -
>  arch/arm/mach-exynos/mach-universal_c210.c         |    2 -
>  arch/arm/plat-samsung/devs.c                       |    4 -
>  arch/arm/plat-samsung/include/plat/sdhci.h         |   35 +++-
>  arch/arm/plat-samsung/platformdata.c               |    2 -
>  drivers/mmc/host/sdhci-s3c.c                       |  235
+++++++++++++++++++-
>  13 files changed, 341 insertions(+), 50 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/samsung-
> sdhci.txt

Hi,

Looks ok to me and I'd like to pick this series in samsung tree with Chris'
ack.

Chris, is it ok? If you need to merge this in your tree, please let me know.
I will provide topic branch for it.

If any problems, please let me know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Chris Ball]

Hi,

On Thu, Feb 09 2012, Kukjin Kim wrote:
> Looks ok to me and I'd like to pick this series in samsung tree with Chris'
> ack.
>
> Chris, is it ok? If you need to merge this in your tree, please let me know.
> I will provide topic branch for it.
>
> If any problems, please let me know.

I don't have a very strong opinion, but I think it's probably better to
go via the mmc tree since the patch is so large and hence the chance of
conflict before merging is high.  Thanks,

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

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Kukjin Kim]

On 02/12/12 06:37, Chris Ball wrote:
> Hi,
>
> On Thu, Feb 09 2012, Kukjin Kim wrote:
>> Looks ok to me and I'd like to pick this series in samsung tree with Chris'
>> ack.
>>
>> Chris, is it ok? If you need to merge this in your tree, please let me know.
>> I will provide topic branch for it.
>>
>> If any problems, please let me know.
>
> I don't have a very strong opinion, but I think it's probably better to
> go via the mmc tree since the patch is so large and hence the chance of
> conflict before merging is high.  Thanks,
>
Hi Chris,

I mean if I or you create topic branch for this whole series, can be 
merged into both tree for avoiding conflicts with further working. Of 
course, the topic branch don't have to rebase after merging into each tree.

If you're ok on this and my opinion, let me create the topic branch.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Chris Ball]

Hi,

On Thu, Feb 16 2012, Kukjin Kim wrote:
> I mean if I or you create topic branch for this whole series, can be
> merged into both tree for avoiding conflicts with further working. Of
> course, the topic branch don't have to rebase after merging into each
> tree.
>
> If you're ok on this and my opinion, let me create the topic branch.

Okay, sure.  Thanks,

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

RE: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Kukjin Kim]

Chris Ball wrote:
> 
> Hi,
> 
Hi,

> On Thu, Feb 16 2012, Kukjin Kim wrote:
> > I mean if I or you create topic branch for this whole series, can be
> > merged into both tree for avoiding conflicts with further working. Of
> > course, the topic branch don't have to rebase after merging into each
> > tree.
> >
> > If you're ok on this and my opinion, let me create the topic branch.
> 
> Okay, sure.  Thanks,
> 
I created topic branch for this we talked. You can pull that following:
  git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
v3.4-for-cjb

If any problems, please kindly let me know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

The following changes since commit d65b4e98d7ea3038b767b70fe8be959b2913f16d:

  Linux 3.3-rc3 (2012-02-08 19:21:53 -0800)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
v3.4-for-cjb

Thomas Abraham (6):
      mmc: sdhci-s3c: Remove usage of clk_type member in platform data
      ARM: EXYNOS: use 'exynos4-sdhci' as device name for sdhci controllers
      ARM: SAMSUNG: remove all uses of clk_type member in sdhci platform
data
      mmc: sdhci-s3c: derive transfer width host capability from max_width
in platform data
      mmc: sdhci-s3c: Keep a copy of platform data and use it
      mmc: sdhci-s3c: Add device tree support

 .../devicetree/bindings/mmc/samsung-sdhci.txt      |   70 ++++++
 arch/arm/mach-exynos/clock.c                       |   24 +-
 arch/arm/mach-exynos/common.c                      |    5 +
 arch/arm/mach-exynos/mach-armlex4210.c             |    3 -
 arch/arm/mach-exynos/mach-nuri.c                   |    3 -
 arch/arm/mach-exynos/mach-origen.c                 |    2 -
 arch/arm/mach-exynos/mach-smdk4x12.c               |    2 -
 arch/arm/mach-exynos/mach-smdkv310.c               |    4 -
 arch/arm/mach-exynos/mach-universal_c210.c         |    2 -
 arch/arm/plat-samsung/devs.c                       |    4 -
 arch/arm/plat-samsung/include/plat/sdhci.h         |   35 +++-
 arch/arm/plat-samsung/platformdata.c               |    2 -
 drivers/mmc/host/sdhci-s3c.c                       |  235
+++++++++++++++++++-
 13 files changed, 341 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung-sdhci.txt

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Chris Ball]

Hi,

On Tue, Feb 21 2012, Kukjin Kim wrote:
> I created topic branch for this we talked. You can pull that following:
>   git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> v3.4-for-cjb
>
> If any problems, please kindly let me know.

Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
to Linus.)

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

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Mark Brown]

On Tue, Feb 21, 2012 at 08:17:41AM -0500, Chris Ball wrote:

> Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
> to Linus.)

I've been sending patches adding runtime PM support to this driver for a
while with no response - are there any issues with those?

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Chris Ball]

Hi,

On Wed, Feb 22 2012, Mark Brown wrote:
> On Tue, Feb 21, 2012 at 08:17:41AM -0500, Chris Ball wrote:
>
>> Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
>> to Linus.)
>
> I've been sending patches adding runtime PM support to this driver for a
> while with no response - are there any issues with those?

I don't have s3c hardware, so I've been waiting for a Tested-by or ACK
from someone who does -- Kukjin, any objection if I take this into
mmc-next with a plan to merge it, to provoke some testing?  Is it
possible for you to test/review the patch?

Thanks,

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

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Kukjin Kim]

On 03/03/12 05:40, Chris Ball wrote:
> Hi,
>
> On Wed, Feb 22 2012, Mark Brown wrote:
>> On Tue, Feb 21, 2012 at 08:17:41AM -0500, Chris Ball wrote:
>>
>>> Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
>>> to Linus.)
>>
>> I've been sending patches adding runtime PM support to this driver for a
>> while with no response - are there any issues with those?
>
> I don't have s3c hardware, so I've been waiting for a Tested-by or ACK
> from someone who does -- Kukjin, any objection if I take this into
> mmc-next with a plan to merge it, to provoke some testing?  Is it
> possible for you to test/review the patch?
>
Hi Chris,

Maybe I lost Mark's runtime PM supporting patches. I couldn't find it in 
my mail box. Sorry for that :(

Mark, could you please send it to me again?

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Mark Brown]

This matches current best practice as one can have runtime PM enabled
without system sleep and CONFIG_PM is defined for both.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/sdhci-s3c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index ea0767e..46152d6 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/pm.h>
 
 #include <linux/mmc/host.h>
 
@@ -807,8 +808,7 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
 static int sdhci_s3c_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
@@ -822,10 +822,11 @@ static int sdhci_s3c_resume(struct device *dev)
 
 	return sdhci_resume_host(host);
 }
+#endif
 
+#ifdef CONFIG_PM
 static const struct dev_pm_ops sdhci_s3c_pmops = {
-	.suspend	= sdhci_s3c_suspend,
-	.resume		= sdhci_s3c_resume,
+	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
 };
 
 #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)
-- 
1.7.9.1

[PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Mark Brown]

Since most of the work is already done by the core we just need to add
runtime suspend methods and tell the PM core that runtime PM is enabled
for this device.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 46152d6..6926ac9 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/host.h>
 
@@ -721,6 +722,11 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	if (pdata->host_caps2)
 		host->mmc->caps2 |= pdata->host_caps2;
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_suspend_ignore_children(&pdev->dev, 1);
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(dev, "sdhci_add_host() failed\n");
@@ -740,6 +746,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 
  err_add_host:
 	release_resource(sc->ioarea);
+	pm_runtime_forbid(&pdev->dev);
+	pm_runtime_get_noresume(&pdev->dev);
 	kfree(sc->ioarea);
 
  err_req_regs:
@@ -784,6 +792,8 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 
 	sdhci_remove_host(host, 1);
 
+	pm_runtime_disable(&pdev->dev);
+
 	for (ptr = 0; ptr < 3; ptr++) {
 		if (sc->clk_bus[ptr]) {
 			clk_disable(sc->clk_bus[ptr]);
@@ -824,9 +834,27 @@ static int sdhci_s3c_resume(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM_RUNTIME
+static int sdhci_s3c_runtime_suspend(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+
+	return sdhci_runtime_suspend_host(host);
+}
+
+static int sdhci_s3c_runtime_resume(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+
+	return sdhci_runtime_resume_host(host);
+}
+#endif
+
 #ifdef CONFIG_PM
 static const struct dev_pm_ops sdhci_s3c_pmops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
+	SET_RUNTIME_PM_OPS(sdhci_s3c_runtime_suspend, sdhci_s3c_runtime_resume,
+			   NULL)
 };
 
 #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)
-- 
1.7.9.1

Re: [PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Jaehoon Chung]

Hi Mark,

On 03/03/2012 09:46 AM, Mark Brown wrote:

> Since most of the work is already done by the core we just need to add
> runtime suspend methods and tell the PM core that runtime PM is enabled
> for this device.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 46152d6..6926ac9 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
>  #include <linux/pm.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -721,6 +722,11 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  	if (pdata->host_caps2)
>  		host->mmc->caps2 |= pdata->host_caps2;
>  
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);

Could you explain why use 50ms? 

> +	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_suspend_ignore_children(&pdev->dev, 1);

Is there reason that ignore_children use to set the true?

> +
>  	ret = sdhci_add_host(host);
>  	if (ret) {
>  		dev_err(dev, "sdhci_add_host() failed\n");
> @@ -740,6 +746,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  
>   err_add_host:
>  	release_resource(sc->ioarea);
> +	pm_runtime_forbid(&pdev->dev);
> +	pm_runtime_get_noresume(&pdev->dev);
>  	kfree(sc->ioarea);
>  
>   err_req_regs:
> @@ -784,6 +792,8 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  
>  	sdhci_remove_host(host, 1);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	for (ptr = 0; ptr < 3; ptr++) {
>  		if (sc->clk_bus[ptr]) {
>  			clk_disable(sc->clk_bus[ptr]);
> @@ -824,9 +834,27 @@ static int sdhci_s3c_resume(struct device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_PM_RUNTIME
> +static int sdhci_s3c_runtime_suspend(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	return sdhci_runtime_suspend_host(host);
> +}
> +
> +static int sdhci_s3c_runtime_resume(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	return sdhci_runtime_resume_host(host);
> +}
> +#endif
> +
>  #ifdef CONFIG_PM
>  static const struct dev_pm_ops sdhci_s3c_pmops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
> +	SET_RUNTIME_PM_OPS(sdhci_s3c_runtime_suspend, sdhci_s3c_runtime_resume,
> +			   NULL)
>  };
>  
>  #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)

Re: [PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Mon, Mar 05, 2012 at 07:48:42PM +0900, Jaehoon Chung wrote:
> On 03/03/2012 09:46 AM, Mark Brown wrote:

> > +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);

> Could you explain why use 50ms? 

It's essentially a random number, some other devices use the same one.
We're just trying to avoid suspending between back to back requests.

> > +	pm_runtime_use_autosuspend(&pdev->dev);
> > +	pm_suspend_ignore_children(&pdev->dev, 1);

> Is there reason that ignore_children use to set the true?

So that we can suspend the host without having to have the MMC card
suspended (the host suspend is not visible to the card).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Jaehoon Chung]

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
 
On 03/05/2012 08:52 PM, Mark Brown wrote:

> On Mon, Mar 05, 2012 at 07:48:42PM +0900, Jaehoon Chung wrote:
>> On 03/03/2012 09:46 AM, Mark Brown wrote:
> 
>>> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> 
>> Could you explain why use 50ms? 
> 
> It's essentially a random number, some other devices use the same one.
> We're just trying to avoid suspending between back to back requests.
> 
>>> +	pm_runtime_use_autosuspend(&pdev->dev);
>>> +	pm_suspend_ignore_children(&pdev->dev, 1);
> 
>> Is there reason that ignore_children use to set the true?
> 
> So that we can suspend the host without having to have the MMC card
> suspended (the host suspend is not visible to the card).
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Chris Ball]

Hi Mark,

On Fri, Mar 02 2012, Mark Brown wrote:
> Since most of the work is already done by the core we just need to add
> runtime suspend methods and tell the PM core that runtime PM is enabled
> for this device.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 46152d6..6926ac9 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
>  #include <linux/pm.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -721,6 +722,11 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  	if (pdata->host_caps2)
>  		host->mmc->caps2 |= pdata->host_caps2;
>  
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_suspend_ignore_children(&pdev->dev, 1);
> +
>  	ret = sdhci_add_host(host);
>  	if (ret) {
>  		dev_err(dev, "sdhci_add_host() failed\n");
> @@ -740,6 +746,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  
>   err_add_host:
>  	release_resource(sc->ioarea);
> +	pm_runtime_forbid(&pdev->dev);
> +	pm_runtime_get_noresume(&pdev->dev);
>  	kfree(sc->ioarea);
>  
>   err_req_regs:
> @@ -784,6 +792,8 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  
>  	sdhci_remove_host(host, 1);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	for (ptr = 0; ptr < 3; ptr++) {
>  		if (sc->clk_bus[ptr]) {
>  			clk_disable(sc->clk_bus[ptr]);
> @@ -824,9 +834,27 @@ static int sdhci_s3c_resume(struct device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_PM_RUNTIME
> +static int sdhci_s3c_runtime_suspend(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	return sdhci_runtime_suspend_host(host);
> +}
> +
> +static int sdhci_s3c_runtime_resume(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	return sdhci_runtime_resume_host(host);
> +}
> +#endif
> +
>  #ifdef CONFIG_PM
>  static const struct dev_pm_ops sdhci_s3c_pmops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
> +	SET_RUNTIME_PM_OPS(sdhci_s3c_runtime_suspend, sdhci_s3c_runtime_resume,
> +			   NULL)
>  };
>  
>  #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)

Thanks, pushed to mmc-next for 3.4 with Jaehoon's ACK.

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

Re: [PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Chris Ball]

Hi,

On Fri, Mar 02 2012, Mark Brown wrote:
> Since most of the work is already done by the core we just need to add
> runtime suspend methods and tell the PM core that runtime PM is enabled
> for this device.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 46152d6..6926ac9 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
>  #include <linux/pm.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -721,6 +722,11 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  	if (pdata->host_caps2)
>  		host->mmc->caps2 |= pdata->host_caps2;
>  
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_suspend_ignore_children(&pdev->dev, 1);
> +
>  	ret = sdhci_add_host(host);
>  	if (ret) {
>  		dev_err(dev, "sdhci_add_host() failed\n");
> @@ -740,6 +746,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  
>   err_add_host:
>  	release_resource(sc->ioarea);
> +	pm_runtime_forbid(&pdev->dev);
> +	pm_runtime_get_noresume(&pdev->dev);
>  	kfree(sc->ioarea);
>  
>   err_req_regs:
> @@ -784,6 +792,8 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  
>  	sdhci_remove_host(host, 1);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	for (ptr = 0; ptr < 3; ptr++) {
>  		if (sc->clk_bus[ptr]) {
>  			clk_disable(sc->clk_bus[ptr]);
> @@ -824,9 +834,27 @@ static int sdhci_s3c_resume(struct device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_PM_RUNTIME
> +static int sdhci_s3c_runtime_suspend(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	return sdhci_runtime_suspend_host(host);
> +}
> +
> +static int sdhci_s3c_runtime_resume(struct device *dev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	return sdhci_runtime_resume_host(host);
> +}
> +#endif
> +
>  #ifdef CONFIG_PM
>  static const struct dev_pm_ops sdhci_s3c_pmops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
> +	SET_RUNTIME_PM_OPS(sdhci_s3c_runtime_suspend, sdhci_s3c_runtime_resume,
> +			   NULL)
>  };
>  
>  #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)

I had to rework this patch to avoid a conflict with Julia Lawall's patch
"mmc: sdhci-s3c: use devm_ functions": https://lkml.org/lkml/2012/2/18/76

The reworked patch is below, please let me know if it looks incorrect:

From: Mark Brown <broonie@opensource.wolfsonmicro.com>
Subject: [PATCH] mmc: sdhci-s3c: Enable runtime power management

Since most of the work is already done by the core we just need to add
runtime suspend methods and tell the PM core that runtime PM is enabled
for this device.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Chris Ball <cjb@laptop.org>
---
 drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 9683322..312aaf4 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/host.h>
 
@@ -705,9 +706,16 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	if (pdata->host_caps)
 		host->mmc->caps |= pdata->host_caps;
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_suspend_ignore_children(&pdev->dev, 1);
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(dev, "sdhci_add_host() failed\n");
+		pm_runtime_forbid(&pdev->dev);
+		pm_runtime_get_noresume(&pdev->dev);
 		goto err_req_regs;
 	}
 
@@ -764,6 +772,8 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 
 	sdhci_remove_host(host, 1);
 
+	pm_runtime_disable(&pdev->dev);
+
 	for (ptr = 0; ptr < 3; ptr++) {
 		if (sc->clk_bus[ptr]) {
 			clk_disable(sc->clk_bus[ptr]);
@@ -800,9 +810,27 @@ static int sdhci_s3c_resume(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM_RUNTIME
+static int sdhci_s3c_runtime_suspend(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+
+	return sdhci_runtime_suspend_host(host);
+}
+
+static int sdhci_s3c_runtime_resume(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+
+	return sdhci_runtime_resume_host(host);
+}
+#endif
+
 #ifdef CONFIG_PM
 static const struct dev_pm_ops sdhci_s3c_pmops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
+	SET_RUNTIME_PM_OPS(sdhci_s3c_runtime_suspend, sdhci_s3c_runtime_resume,
+			   NULL)
 };
 
 #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: sdhci-s3c: Enable runtime power management

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

On Fri, Mar 09, 2012 at 12:08:58AM -0500, Chris Ball wrote:

> The reworked patch is below, please let me know if it looks incorrect:

That looks about right, I'll find out soon enough if it doesn't work in
-next as my main development system has the rootfs on this device :)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Jaehoon Chung]

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 03/03/2012 09:46 AM, Mark Brown wrote:

> This matches current best practice as one can have runtime PM enabled
> without system sleep and CONFIG_PM is defined for both.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index ea0767e..46152d6 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -22,6 +22,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
> +#include <linux/pm.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -807,8 +808,7 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -
> +#ifdef CONFIG_PM_SLEEP
>  static int sdhci_s3c_suspend(struct device *dev)
>  {
>  	struct sdhci_host *host = dev_get_drvdata(dev);
> @@ -822,10 +822,11 @@ static int sdhci_s3c_resume(struct device *dev)
>  
>  	return sdhci_resume_host(host);
>  }
> +#endif
>  
> +#ifdef CONFIG_PM
>  static const struct dev_pm_ops sdhci_s3c_pmops = {
> -	.suspend	= sdhci_s3c_suspend,
> -	.resume		= sdhci_s3c_resume,
> +	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
>  };
>  
>  #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)

Re: [PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Chris Ball]

Hi Mark,

On Fri, Mar 02 2012, Mark Brown wrote:
> This matches current best practice as one can have runtime PM enabled
> without system sleep and CONFIG_PM is defined for both.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index ea0767e..46152d6 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -22,6 +22,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
> +#include <linux/pm.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -807,8 +808,7 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -
> +#ifdef CONFIG_PM_SLEEP
>  static int sdhci_s3c_suspend(struct device *dev)
>  {
>  	struct sdhci_host *host = dev_get_drvdata(dev);
> @@ -822,10 +822,11 @@ static int sdhci_s3c_resume(struct device *dev)
>  
>  	return sdhci_resume_host(host);
>  }
> +#endif
>  
> +#ifdef CONFIG_PM
>  static const struct dev_pm_ops sdhci_s3c_pmops = {
> -	.suspend	= sdhci_s3c_suspend,
> -	.resume		= sdhci_s3c_resume,
> +	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
>  };
>  
>  #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)

Thanks, pushed to mmc-next for 3.4 with Jaehoon's ACK.

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

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Chris Ball]

Hi Kukjin,

On Tue, Feb 21 2012, Chris Ball wrote:
> On Tue, Feb 21 2012, Kukjin Kim wrote:
>> I created topic branch for this we talked. You can pull that following:
>>   git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
>> v3.4-for-cjb
>>
>> If any problems, please kindly let me know.
>
> Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
> to Linus.)

I was expecting you to merge these patches, but they aren't in the
arm-soc tree and haven't been sent to Linus, and we're in the last few
days of the merge window.

As a result I'm dropping this tree -- and all of the patches that I
have on top of it -- from mmc-next so that I can get a pull request
out.  This means that I'm dropping:

mmc: sdhci-s3c: Add device tree support
mmc: sdhci-s3c: Keep a copy of platform data and use it
mmc: sdhci-s3c: derive transfer width host capability from max_width in 
ARM: SAMSUNG: remove all uses of clk_type member in sdhci platform data
ARM: EXYNOS: use 'exynos4-sdhci' as device name for sdhci controllers
mmc: sdhci-s3c: Remove usage of clk_type member in platform data

.. which I was expecting you to merge, and also:

mmc: sdhci-s3c: fix broken compilation for non-Exynos SoCs
mmc: sdhci-s3c: Enable runtime power management
mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend
mmc: sdhci-s3c: use devm_ functions

.. which were merged on top of the above patches.

We'll have to try again with 3.5 for the new features, and post-rc1 for
the fixes.  Please let me know what happened with this merge.  Thanks,

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

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Mark Brown]

On Tue, Mar 27, 2012 at 11:50:24AM -0400, Chris Ball wrote:
> On Tue, Feb 21 2012, Chris Ball wrote:
> > On Tue, Feb 21 2012, Kukjin Kim wrote:

> >> I created topic branch for this we talked. You can pull that following:
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> >> v3.4-for-cjb

> >> If any problems, please kindly let me know.

> > Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
> > to Linus.)

> I was expecting you to merge these patches, but they aren't in the
> arm-soc tree and haven't been sent to Linus, and we're in the last few
> days of the merge window.

> As a result I'm dropping this tree -- and all of the patches that I
> have on top of it -- from mmc-next so that I can get a pull request
> out.  This means that I'm dropping:

If they're already in your tree why not just send them to Linus?  Given
that everything's in git I don't understand why you'd need Kukjin to
push them separately or what the benefit of that would be?

This is all extremely frustrating fromm the contributor point of view.

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Kukjin Kim]

On 03/28/12 02:54, Mark Brown wrote:
> On Tue, Mar 27, 2012 at 11:50:24AM -0400, Chris Ball wrote:
>> On Tue, Feb 21 2012, Chris Ball wrote:
>>> On Tue, Feb 21 2012, Kukjin Kim wrote:
>
>>>> I created topic branch for this we talked. You can pull that following:
>>>>    git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
>>>> v3.4-for-cjb
>
>>>> If any problems, please kindly let me know.
>
>>> Pushed to mmc-next, thanks.  (I'm expecting that you'll do the merge
>>> to Linus.)
>
>> I was expecting you to merge these patches, but they aren't in the
>> arm-soc tree and haven't been sent to Linus,

(Cc'ed Olof and Arnd)

Chris, Yeah, it is not in arm-soc tree now. Because I sent a pull 
request of samsung-mmc which is including this v3.4-for-cjb but Olof 
wanted to merge only arch/arm/ part for samsung mmc because they are not 
unrelated to the branch, v3.4-for-cjb sent to you. Since I thought the 
branch would be sent to upstream via your tree and you were included in 
the mailing loop, I agreed. So there is no it in arm-soc.

>> and we're in the last few
>> days of the merge window.
>
Yeah...it means not closed yet ;)

>> As a result I'm dropping this tree -- and all of the patches that I
>> have on top of it -- from mmc-next so that I can get a pull request
>> out.  This means that I'm dropping:
>
Hmm, I think, if you're ok, you can send a second pull request to Linus 
for it and actually, it is in linux-next for a long time via mmc and 
samsung tree.

Note, please don't rebase it because its resolution for conflicts is in 
linux-next and I think Linus will use it when happens conflicts...Or I 
can provide new tree on top of latest mainline. But I'm not sure about 
latter.

> If they're already in your tree why not just send them to Linus?  Given
> that everything's in git I don't understand why you'd need Kukjin to
> push them separately or what the benefit of that would be?
>
Yes, I already agreed its merging in the mmc tree :)

> This is all extremely frustrating fromm the contributor point of view.

If any my effort is required, feel free to contact me.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Chris Ball]

Hi Kukjin,

On Wed, Mar 28 2012, Kukjin Kim wrote:
>>> As a result I'm dropping this tree -- and all of the patches that I
>>> have on top of it -- from mmc-next so that I can get a pull request
>>> out.  This means that I'm dropping:
>>
> Hmm, I think, if you're ok, you can send a second pull request to
> Linus for it and actually, it is in linux-next for a long time via mmc
> and samsung tree.
>
> Note, please don't rebase it because its resolution for conflicts is
> in linux-next and I think Linus will use it when happens
> conflicts...Or I can provide new tree on top of latest mainline. But
> I'm not sure about latter.

I can't send the tree as it is to Linus now, because Arnd has asked us
to hold off on these device tree bindings and work with the unified
bindings he's proposing instead.  (Rebasing to drop that patch will
introduce new conflicts.)

I'm going to send Mark Brown's two patches to Linus now, even though it
will cause a conflict in -next.  The rest (other than the device tree
bindings) are mergable after -rc1, because they're fixes, so we'll
eventually get everything except DT in to 3.4.  I think you should
just drop this patchset from your tree in linux-next entirely now.

Thanks,

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

Re: [PATCH v3 0/6] mmc: sdhci-s3c: Rework platform data and add device tree support

[Kukjin Kim]

Chris Ball wrote:
> Hi Kukjin,
>
Hi,

Sorry for late response.

[...]

>
> I can't send the tree as it is to Linus now, because Arnd has asked us
> to hold off on these device tree bindings and work with the unified
> bindings he's proposing instead.  (Rebasing to drop that patch will
> introduce new conflicts.)
>
OK, I see.

> I'm going to send Mark Brown's two patches to Linus now, even though it
> will cause a conflict in -next.  The rest (other than the device tree
> bindings) are mergable after -rc1, because they're fixes, so we'll
> eventually get everything except DT in to 3.4.  I think you should
> just drop this patchset from your tree in linux-next entirely now.
>
Yep, I dropped it in my -next.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Mark Brown]

This matches current best practice as one can have runtime PM enabled
without system sleep and CONFIG_PM is defined for both.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/sdhci-s3c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index b19e7d4..84ea6c7 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
+#include <linux/pm.h>
 
 #include <linux/mmc/host.h>
 
@@ -620,8 +621,7 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
 static int sdhci_s3c_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
@@ -635,10 +635,11 @@ static int sdhci_s3c_resume(struct device *dev)
 
 	return sdhci_resume_host(host);
 }
+#endif
 
+#ifdef CONFIG_PM
 static const struct dev_pm_ops sdhci_s3c_pmops = {
-	.suspend	= sdhci_s3c_suspend,
-	.resume		= sdhci_s3c_resume,
+	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
 };
 
 #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)
-- 
1.7.9.rc1

[PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Mark Brown]

This matches current best practice as one can have runtime PM enabled
without system sleep and CONFIG_PM is defined for both.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/sdhci-s3c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 1af756e..df066b5 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
+#include <linux/pm.h>
 
 #include <linux/mmc/host.h>
 
@@ -620,8 +621,7 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
 static int sdhci_s3c_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
@@ -635,10 +635,11 @@ static int sdhci_s3c_resume(struct device *dev)
 
 	return sdhci_resume_host(host);
 }
+#endif
 
+#ifdef CONFIG_PM
 static const struct dev_pm_ops sdhci_s3c_pmops = {
-	.suspend	= sdhci_s3c_suspend,
-	.resume		= sdhci_s3c_resume,
+	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
 };
 
 #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)
-- 
1.7.7.3

Re: [PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Jaehoon Chung]

Hi Mark.

On 12/30/2011 11:24 AM, Mark Brown wrote:

> This matches current best practice as one can have runtime PM enabled
> without system sleep and CONFIG_PM is defined for both.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 1af756e..df066b5 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -20,6 +20,7 @@
>  #include <linux/io.h>
>  #include <linux/gpio.h>
>  #include <linux/module.h>
> +#include <linux/pm.h>

I think this include is unnecessary.

Thanks,
Jaehoon Chung

>  
>  #include <linux/mmc/host.h>
>  
> @@ -620,8 +621,7 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -
> +#ifdef CONFIG_PM_SLEEP
>  static int sdhci_s3c_suspend(struct device *dev)
>  {
>  	struct sdhci_host *host = dev_get_drvdata(dev);
> @@ -635,10 +635,11 @@ static int sdhci_s3c_resume(struct device *dev)
>  
>  	return sdhci_resume_host(host);
>  }
> +#endif
>  
> +#ifdef CONFIG_PM
>  static const struct dev_pm_ops sdhci_s3c_pmops = {
> -	.suspend	= sdhci_s3c_suspend,
> -	.resume		= sdhci_s3c_resume,
> +	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
>  };
>  
>  #define SDHCI_S3C_PMOPS (&sdhci_s3c_pmops)

Re: [PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Mark Brown]

On Mon, Jan 02, 2012 at 11:07:59AM +0900, Jaehoon Chung wrote:
> On 12/30/2011 11:24 AM, Mark Brown wrote:

> > +#include <linux/pm.h>

> I think this include is unnecessary.

I think you are wrong.  What makes you say that you think it is not
needed?

> > +#ifdef CONFIG_PM
> >  static const struct dev_pm_ops sdhci_s3c_pmops = {
> > -	.suspend	= sdhci_s3c_suspend,
> > -	.resume		= sdhci_s3c_resume,
> > +	SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)

Note that above.

Re: [PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Jae hoon Chung]

2012/1/2 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Mon, Jan 02, 2012 at 11:07:59AM +0900, Jaehoon Chung wrote:
>> On 12/30/2011 11:24 AM, Mark Brown wrote:
>
>> > +#include <linux/pm.h>
>
>> I think this include is unnecessary.
>
> I think you are wrong.  What makes you say that you think it is not
> needed?
If that didn't include,,what's happen?
I think that is already included...Anyway..this patch looks fine to me..
(if use with your patch[2/2]..)

Thanks,
Jaehoon Chung
>
>> > +#ifdef CONFIG_PM
>> >  static const struct dev_pm_ops sdhci_s3c_pmops = {
>> > -   .suspend        = sdhci_s3c_suspend,
>> > -   .resume         = sdhci_s3c_resume,
>> > +   SET_SYSTEM_SLEEP_PM_OPS(sdhci_s3c_suspend, sdhci_s3c_resume)
>
> Note that above.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mmc: sdhci-s3c: Use CONFIG_PM_SLEEP to ifdef system suspend

[Mark Brown]

On Tue, Jan 03, 2012 at 12:24:53AM +0900, Jae hoon Chung wrote:
> 2012/1/2 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> >> On 12/30/2011 11:24 AM, Mark Brown wrote:

> >> > +#include <linux/pm.h>
> >> I think this include is unnecessary.

> > I think you are wrong.  What makes you say that you think it is not
> > needed?

> If that didn't include,,what's happen?

The code won't compile as we're relying on a macro defined in the
header.

> I think that is already included...Anyway..this patch looks fine to me..
> (if use with your patch[2/2]..)

It's not included directly (or indirectly as far as I remember).