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

[PATCH 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. This
patchset has merged the following two patchsets submitted earlier into one
patchset.

[PATCH 0/3] mmc: sdhci-s3c: Remove 'clk_type' member from platform data
[PATCH 0/3] mmc: sdhci-s3c: Add device tree support for Samsung's sdhci controller driver

This patchset is rebased over the patches for sdhci clock lookup using
generic names.

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

Fifth patch adds support for parsing of mmc host controller capabilities from a
device node. This code would be reusable across other platforms as well. The
last patch adds device tree based discovery for the sdhci-s3c driver.

In this patchset, the comments from Rob Herring for the fifth patch in this
series has been addressed.

This patchset is based on the following tree.
  https://github.com/kgene/linux-samsung.git   branch: next-samsung-dt

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: Keep a copy of platform data and use it
  mmc: Add OF bindings support for mmc host controller capabilities
  mmc: sdhci-s3c: Add device tree support

 .../devicetree/bindings/mmc/linux-mmc-host.txt     |   13 +
 .../devicetree/bindings/mmc/samsung-sdhci.txt      |   75 +++++++
 arch/arm/mach-exynos4/clock.c                      |   24 +-
 arch/arm/mach-exynos4/cpu.c                        |    5 +
 arch/arm/mach-exynos4/mach-armlex4210.c            |    3 -
 arch/arm/mach-exynos4/mach-nuri.c                  |    3 -
 arch/arm/mach-exynos4/mach-origen.c                |    2 -
 arch/arm/mach-exynos4/mach-smdk4x12.c              |    2 -
 arch/arm/mach-exynos4/mach-smdkv310.c              |    4 -
 arch/arm/mach-exynos4/mach-universal_c210.c        |    2 -
 arch/arm/plat-samsung/devs.c                       |    4 -
 arch/arm/plat-samsung/include/plat/sdhci.h         |   34 +++-
 arch/arm/plat-samsung/platformdata.c               |    2 -
 drivers/mmc/core/host.c                            |   31 +++
 drivers/mmc/host/sdhci-s3c.c                       |  231 +++++++++++++++++++-
 include/linux/mmc/host.h                           |    1 +
 16 files changed, 386 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung-sdhci.txt

[PATCH 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 7551468..141fbbf 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
 
@@ -531,7 +575,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;
@@ -638,9 +682,31 @@ static int sdhci_s3c_resume(struct platform_device *dev)
 #define sdhci_s3c_resume 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,
 	.suspend	= sdhci_s3c_suspend,
 	.resume	        = sdhci_s3c_resume,
 	.driver		= {
-- 
1.6.6.rc2

[PATCH 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-exynos4/clock.c              |   24 ++++++++++++------------
 arch/arm/mach-exynos4/cpu.c                |    5 +++++
 arch/arm/plat-samsung/include/plat/sdhci.h |   27 +++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-exynos4/clock.c b/arch/arm/mach-exynos4/clock.c
index 6eeabdd..94fc337 100644
--- a/arch/arm/mach-exynos4/clock.c
+++ b/arch/arm/mach-exynos4/clock.c
@@ -495,25 +495,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),
@@ -1221,7 +1221,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),
@@ -1232,7 +1232,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),
@@ -1243,7 +1243,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),
@@ -1254,7 +1254,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),
@@ -1317,10 +1317,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),
 };
diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 6ac9baf..152c8b1 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -195,6 +195,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 dcff7dd..8daaa4d 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;
@@ -309,4 +311,29 @@ 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 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-exynos4/mach-armlex4210.c     |    3 ---
 arch/arm/mach-exynos4/mach-nuri.c           |    3 ---
 arch/arm/mach-exynos4/mach-origen.c         |    2 --
 arch/arm/mach-exynos4/mach-smdk4x12.c       |    2 --
 arch/arm/mach-exynos4/mach-smdkv310.c       |    4 ----
 arch/arm/mach-exynos4/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-exynos4/mach-armlex4210.c b/arch/arm/mach-exynos4/mach-armlex4210.c
index f0ca6c1..426b9d2 100644
--- a/arch/arm/mach-exynos4/mach-armlex4210.c
+++ b/arch/arm/mach-exynos4/mach-armlex4210.c
@@ -75,7 +75,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,
@@ -86,13 +85,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-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
index 236bbe1..4abf4e4 100644
--- a/arch/arm/mach-exynos4/mach-nuri.c
+++ b/arch/arm/mach-exynos4/mach-nuri.c
@@ -109,7 +109,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[] = {
@@ -151,7 +150,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 */
@@ -160,7 +158,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-exynos4/mach-origen.c b/arch/arm/mach-exynos4/mach-origen.c
index f80b563..47efb34 100644
--- a/arch/arm/mach-exynos4/mach-origen.c
+++ b/arch/arm/mach-exynos4/mach-origen.c
@@ -465,12 +465,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-exynos4/mach-smdk4x12.c b/arch/arm/mach-exynos4/mach-smdk4x12.c
index fcf2e0e..96c60df 100644
--- a/arch/arm/mach-exynos4/mach-smdk4x12.c
+++ b/arch/arm/mach-exynos4/mach-smdk4x12.c
@@ -83,7 +83,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,
@@ -92,7 +91,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-exynos4/mach-smdkv310.c b/arch/arm/mach-exynos4/mach-smdkv310.c
index cec2afa..f15ae7a 100644
--- a/arch/arm/mach-exynos4/mach-smdkv310.c
+++ b/arch/arm/mach-exynos4/mach-smdkv310.c
@@ -90,7 +90,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,
@@ -101,12 +100,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,
@@ -117,7 +114,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-exynos4/mach-universal_c210.c b/arch/arm/mach-exynos4/mach-universal_c210.c
index a2a177f..ab5eeb3 100644
--- a/arch/arm/mach-exynos4/mach-universal_c210.c
+++ b/arch/arm/mach-exynos4/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 4ca8b57..d009ad9 100644
--- a/arch/arm/plat-samsung/devs.c
+++ b/arch/arm/plat-samsung/devs.c
@@ -321,7 +321,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 = {
@@ -352,7 +351,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 = {
@@ -385,7 +383,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 = {
@@ -416,7 +413,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 8daaa4d..ae23555 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
@@ -66,7 +60,6 @@ struct s3c_sdhci_platdata {
 	unsigned int	max_width;
 	unsigned int	host_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 4c9a207..88c5e7c 100644
--- a/arch/arm/plat-samsung/platformdata.c
+++ b/arch/arm/plat-samsung/platformdata.c
@@ -52,6 +52,4 @@ void s3c_sdhci_set_platdata(struct s3c_sdhci_platdata *pd,
 		set->cfg_gpio = pd->cfg_gpio;
 	if (pd->host_caps)
 		set->host_caps |= pd->host_caps;
-	if (pd->clk_type)
-		set->clk_type = pd->clk_type;
 }
-- 
1.6.6.rc2

[PATCH 4/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 141fbbf..c77ec42 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 5/6] mmc: Add OF bindings support for mmc host controller capabilities

[Thomas Abraham]

Device nodes representing sd/mmc controllers in a device tree would include
mmc host controller capabilities. Add support for parsing of mmc host
controller capabilities included in device nodes.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 .../devicetree/bindings/mmc/linux-mmc-host.txt     |   13 ++++++++
 drivers/mmc/core/host.c                            |   31 ++++++++++++++++++++
 include/linux/mmc/host.h                           |    1 +
 3 files changed, 45 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt

diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
new file mode 100644
index 0000000..714b2b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
@@ -0,0 +1,13 @@
+* Linux MMC Host Controller Capabilities
+
+The following bindings can be used in a device node to specify any board
+specific mmc host controller capabilities.
+
+- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers
+- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing
+- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing
+- linux,mmc_cap_needs_poll - Host needs polling for card detection
+- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer
+- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power
+- linux,mmc_cap_nonremovable - Host is connected to nonremovable media
+- linux,mmc_cap_erase - Host allows erase/trim commands
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ca2e4f5..aabf440 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -19,6 +19,7 @@
 #include <linux/leds.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <linux/of.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -396,3 +397,33 @@ void mmc_free_host(struct mmc_host *host)
 }
 
 EXPORT_SYMBOL(mmc_free_host);
+
+#ifdef CONFIG_OF
+/**
+ *	mmc_of_parse_host_caps - parse mmc host capabilities from device node
+ *	@np: pointer to device node in device tree
+ *	@caps: pointer to host caps value to be returned
+ *
+ *	Search the device node in device tree for mmc host capabilities.
+ */
+void mmc_of_parse_host_caps(struct device_node *np, unsigned long *caps)
+{
+	if (of_find_property(np, "linux,mmc_cap_4_bit_data", NULL))
+		*caps |= MMC_CAP_4_BIT_DATA;
+	if (of_find_property(np, "linux,mmc_cap_mmc_highspeed", NULL))
+		*caps |= MMC_CAP_MMC_HIGHSPEED;
+	if (of_find_property(np, "linux,mmc_cap_sd_highspeed", NULL))
+		*caps |= MMC_CAP_SD_HIGHSPEED;
+	if (of_find_property(np, "linux,mmc_cap_needs_poll", NULL))
+		*caps |= MMC_CAP_NEEDS_POLL;
+	if (of_find_property(np, "linux,mmc_cap_8_bit_data", NULL))
+		*caps |= MMC_CAP_8_BIT_DATA;
+	if (of_find_property(np, "linux,mmc_cap_disable", NULL))
+		*caps |= MMC_CAP_DISABLE;
+	if (of_find_property(np, "linux,mmc_cap_nonremovable", NULL))
+		*caps |= MMC_CAP_NONREMOVABLE;
+	if (of_find_property(np, "linux,mmc_cap_erase", NULL))
+		*caps |= MMC_CAP_ERASE;
+}
+EXPORT_SYMBOL(mmc_of_parse_host_caps);
+#endif /* CONFIG_OF */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index a3ac9c4..c81c6e8 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -330,6 +330,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
 extern int mmc_add_host(struct mmc_host *);
 extern void mmc_remove_host(struct mmc_host *);
 extern void mmc_free_host(struct mmc_host *);
+extern void mmc_of_parse_host_caps(struct device_node *np, unsigned long *caps);
 
 static inline void *mmc_priv(struct mmc_host *host)
 {
-- 
1.6.6.rc2

[PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities

[Olof Johansson]

On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote:
> Device nodes representing sd/mmc controllers in a device tree would include
> mmc host controller capabilities. Add support for parsing of mmc host
> controller capabilities included in device nodes.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  .../devicetree/bindings/mmc/linux-mmc-host.txt     |   13 ++++++++
>  drivers/mmc/core/host.c                            |   31 ++++++++++++++++++++
>  include/linux/mmc/host.h                           |    1 +
>  3 files changed, 45 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
> new file mode 100644
> index 0000000..714b2b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
> @@ -0,0 +1,13 @@
> +* Linux MMC Host Controller Capabilities
> +
> +The following bindings can be used in a device node to specify any board
> +specific mmc host controller capabilities.
> +
> +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers
> +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing
> +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing
> +- linux,mmc_cap_needs_poll - Host needs polling for card detection
> +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer
> +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power
> +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media
> +- linux,mmc_cap_erase - Host allows erase/trim commands

linux-prefixed properties are a big red flag. The device tree should describe
the hardware, not what linux does with the hardware.

See previous comments about "support-8bit" for encoding exactly the same
hardware capability in a linux-agnostic way. What the sdhci driver chooses to
do with it is up to the driver, and in some cases it means it will set the
capabilities flag.

Same goes for the other properties. It should not go in as it is
implemented in this patch, it needs to be fixed up.


-Olof

[PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities

[Thomas Abraham]

On 5 November 2011 01:27, Olof Johansson <olof@lixom.net> wrote:
> On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote:
>> Device nodes representing sd/mmc controllers in a device tree would include
>> mmc host controller capabilities. Add support for parsing of mmc host
>> controller capabilities included in device nodes.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>> ?.../devicetree/bindings/mmc/linux-mmc-host.txt ? ? | ? 13 ++++++++
>> ?drivers/mmc/core/host.c ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 31 ++++++++++++++++++++
>> ?include/linux/mmc/host.h ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?1 +
>> ?3 files changed, 45 insertions(+), 0 deletions(-)
>> ?create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
>> new file mode 100644
>> index 0000000..714b2b1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
>> @@ -0,0 +1,13 @@
>> +* Linux MMC Host Controller Capabilities
>> +
>> +The following bindings can be used in a device node to specify any board
>> +specific mmc host controller capabilities.
>> +
>> +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers
>> +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing
>> +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing
>> +- linux,mmc_cap_needs_poll - Host needs polling for card detection
>> +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer
>> +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power
>> +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media
>> +- linux,mmc_cap_erase - Host allows erase/trim commands
>
> linux-prefixed properties are a big red flag. The device tree should describe
> the hardware, not what linux does with the hardware.

The vendor-prefixes documentation for device tree bindings includes
'linux' as an option. And I was trying to encode the linux specific
host controller capabilities using the above bindings.

>
> See previous comments about "support-8bit" for encoding exactly the same
> hardware capability in a linux-agnostic way. What the sdhci driver chooses to
> do with it is up to the driver, and in some cases it means it will set the
> capabilities flag.
>
> Same goes for the other properties. It should not go in as it is
> implemented in this patch, it needs to be fixed up.

Ok. I will remove all these linux specific bindings and replace with a
more generic ones. Bindings will be defined for all the linux defined
host capabilities. Non-linux platforms can add additional bindings as
required. A function to parse such properties from a controller device
node could actually be shared among all the mmc/sd host controller
drivers in linux. I will redo this patch and submit again.

Thanks Olof for your review and comments.

Regards,
Thomas.

>
>
> -Olof
>

[PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities

[Grant Likely]

On Mon, Nov 07, 2011 at 07:51:26PM +0530, Thomas Abraham wrote:
> On 5 November 2011 01:27, Olof Johansson <olof@lixom.net> wrote:
> > On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote:
> >> Device nodes representing sd/mmc controllers in a device tree would include
> >> mmc host controller capabilities. Add support for parsing of mmc host
> >> controller capabilities included in device nodes.
> >>
> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> >> ---
> >> ?.../devicetree/bindings/mmc/linux-mmc-host.txt ? ? | ? 13 ++++++++
> >> ?drivers/mmc/core/host.c ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 31 ++++++++++++++++++++
> >> ?include/linux/mmc/host.h ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?1 +
> >> ?3 files changed, 45 insertions(+), 0 deletions(-)
> >> ?create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
> >> new file mode 100644
> >> index 0000000..714b2b1
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
> >> @@ -0,0 +1,13 @@
> >> +* Linux MMC Host Controller Capabilities
> >> +
> >> +The following bindings can be used in a device node to specify any board
> >> +specific mmc host controller capabilities.
> >> +
> >> +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers
> >> +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing
> >> +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing
> >> +- linux,mmc_cap_needs_poll - Host needs polling for card detection
> >> +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer
> >> +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power
> >> +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media
> >> +- linux,mmc_cap_erase - Host allows erase/trim commands
> >
> > linux-prefixed properties are a big red flag. The device tree should describe
> > the hardware, not what linux does with the hardware.
> 
> The vendor-prefixes documentation for device tree bindings includes
> 'linux' as an option. And I was trying to encode the linux specific
> host controller capabilities using the above bindings.
> 
> >
> > See previous comments about "support-8bit" for encoding exactly the same
> > hardware capability in a linux-agnostic way. What the sdhci driver chooses to
> > do with it is up to the driver, and in some cases it means it will set the
> > capabilities flag.
> >
> > Same goes for the other properties. It should not go in as it is
> > implemented in this patch, it needs to be fixed up.
> 
> Ok. I will remove all these linux specific bindings and replace with a
> more generic ones. Bindings will be defined for all the linux defined
> host capabilities. Non-linux platforms can add additional bindings as
> required. A function to parse such properties from a controller device
> node could actually be shared among all the mmc/sd host controller
> drivers in linux. I will redo this patch and submit again.
> 
> Thanks Olof for your review and comments.

This patch appears to be conceptually wrong.  Many of these properties
are merely static capabilities of each individual device which the
driver should already have knowledge of, and be setting the capability
bits appropriately on its own.

So, you /don't/ want to simply create a 1:1 list between the current
linux capability bits (which are subject to change) and the device
tree properties (which ideally must not be changed after they are
defined).

What you need to do is figure out which properties are required to
describe the hardware about things that the driver wouldn't already
know.  For example, 'polling' is something the driver would already
know because it is an aspect of the specific device, but
'nonremovable' is a physical characteristic of some boards.  'disable'
and the speed timings are also something I would expect the driver to
know about and what to set itself.

So, only define properties for things that the device-specific driver
cannot know for itself; or are the sort of parameters that a
multi-device driver is already using for differentiation (ie. appears
in pdata instead of directly set by the driver).  You should be able
to justify the need for each property that gets defined.

g.

[PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities

[Thomas Abraham]

Hi Grant,

On 8 November 2011 02:45, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Nov 07, 2011 at 07:51:26PM +0530, Thomas Abraham wrote:
>> On 5 November 2011 01:27, Olof Johansson <olof@lixom.net> wrote:
>> > On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote:
>> >> Device nodes representing sd/mmc controllers in a device tree would include
>> >> mmc host controller capabilities. Add support for parsing of mmc host
>> >> controller capabilities included in device nodes.
>> >>
>> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> >> ---
>> >> ?.../devicetree/bindings/mmc/linux-mmc-host.txt ? ? | ? 13 ++++++++
>> >> ?drivers/mmc/core/host.c ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 31 ++++++++++++++++++++
>> >> ?include/linux/mmc/host.h ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?1 +
>> >> ?3 files changed, 45 insertions(+), 0 deletions(-)
>> >> ?create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
>> >> new file mode 100644
>> >> index 0000000..714b2b1
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt
>> >> @@ -0,0 +1,13 @@
>> >> +* Linux MMC Host Controller Capabilities
>> >> +
>> >> +The following bindings can be used in a device node to specify any board
>> >> +specific mmc host controller capabilities.
>> >> +
>> >> +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers
>> >> +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing
>> >> +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing
>> >> +- linux,mmc_cap_needs_poll - Host needs polling for card detection
>> >> +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer
>> >> +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power
>> >> +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media
>> >> +- linux,mmc_cap_erase - Host allows erase/trim commands
>> >
>> > linux-prefixed properties are a big red flag. The device tree should describe
>> > the hardware, not what linux does with the hardware.
>>
>> The vendor-prefixes documentation for device tree bindings includes
>> 'linux' as an option. And I was trying to encode the linux specific
>> host controller capabilities using the above bindings.
>>
>> >
>> > See previous comments about "support-8bit" for encoding exactly the same
>> > hardware capability in a linux-agnostic way. What the sdhci driver chooses to
>> > do with it is up to the driver, and in some cases it means it will set the
>> > capabilities flag.
>> >
>> > Same goes for the other properties. It should not go in as it is
>> > implemented in this patch, it needs to be fixed up.
>>
>> Ok. I will remove all these linux specific bindings and replace with a
>> more generic ones. Bindings will be defined for all the linux defined
>> host capabilities. Non-linux platforms can add additional bindings as
>> required. A function to parse such properties from a controller device
>> node could actually be shared among all the mmc/sd host controller
>> drivers in linux. I will redo this patch and submit again.
>>
>> Thanks Olof for your review and comments.
>
> This patch appears to be conceptually wrong. ?Many of these properties
> are merely static capabilities of each individual device which the
> driver should already have knowledge of, and be setting the capability
> bits appropriately on its own.
>
> So, you /don't/ want to simply create a 1:1 list between the current
> linux capability bits (which are subject to change) and the device
> tree properties (which ideally must not be changed after they are
> defined).

okay.

>
> What you need to do is figure out which properties are required to
> describe the hardware about things that the driver wouldn't already
> know. ?For example, 'polling' is something the driver would already
> know because it is an aspect of the specific device, but
> 'nonremovable' is a physical characteristic of some boards. ?'disable'
> and the speed timings are also something I would expect the driver to
> know about and what to set itself.

The host capability 'polling' seems to be more of board dependent
property than controller property. This property changes the way the
host stack detects the presence of the card. Some boards might have
broken card detection logic. So 'polling' cannot be generalized as a
controller property. Each host controller instance in a SoC would have
to be told if its card detection is broken. This is done currently
using the platform data.

Similarly, the host property 'disable' depends on the board. The
controller would be capable of handling the 'disable' feature but on
some boards, disabling to save power during idle times might not work
since the device connected to that host might have issues by doing so.
On a SoC with multiple host controllers, some controllers would want
to use 'disable' feature and others might not. So, 'disable' would
have to be board specific.

>
> So, only define properties for things that the device-specific driver
> cannot know for itself; or are the sort of parameters that a
> multi-device driver is already using for differentiation (ie. appears
> in pdata instead of directly set by the driver). ?You should be able
> to justify the need for each property that gets defined.

Okay. I agree with your viewpoint. I will try to redo this again.
Unfortunately, host controller drivers have used platform data to feed
host controller capabilities directly from platform code to the mmc
stack. And more and more of such changes are still being made. I will
attempt another version of this patch based on yours, Olof's and Rob's
comments.

Thanks Grant for your comments.

Regards,
Thomas.

>
> g.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[PATCH 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      |   75 ++++++++++
 drivers/mmc/host/sdhci-s3c.c                       |  152 +++++++++++++++++++-
 2 files changed, 221 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..a6dd6bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
@@ -0,0 +1,75 @@
+* 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.
+
+- One or more of the linux specific mmc host bindings.
+  See Documentation/devicetree/bindings/mmc/linux-mmc-host.txt for all the
+  linux mmc host controller specific bindings.
+
+Example:
+	sdhci at 12530000 {
+		compatible = "samsung,exynos4210-sdhci";
+		reg = <0x12530000 0x100>;
+		interrupts = <139>;
+		samsung,sdhci-bus-width = <4>;
+		linux,mmc_cap_4_bit_data;
+		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 c77ec42..e77e301 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
@@ -48,6 +52,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 +420,112 @@ 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;
+
+	mmc_of_parse_host_caps(node, (unsigned long *)&pdata->host_caps);
+
+	/* 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 +540,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 +562,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);
 
@@ -626,6 +741,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;
@@ -633,9 +754,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)
@@ -662,6 +783,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);
 
@@ -707,9 +833,22 @@ static struct platform_device_id sdhci_s3c_driver_ids[] = {
 		.name		= "exynos4-sdhci",
 		.driver_data	= EXYNOS4_SDHCI_DRV_DATA,
 	},
+	{},
 };
 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);
+#else
+#define sdhci_s3c_dt_match NULL
+#endif
+
 static struct platform_driver sdhci_s3c_driver = {
 	.probe		= sdhci_s3c_probe,
 	.remove		= __devexit_p(sdhci_s3c_remove),
@@ -719,6 +858,7 @@ static struct platform_driver sdhci_s3c_driver = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "s3c-sdhci",
+		.of_match_table = sdhci_s3c_dt_match,
 	},
 };
 
-- 
1.6.6.rc2

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

[Grant Likely]

On Thu, Nov 03, 2011 at 02:06:03AM +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>
> ---
> +Example:
> +	sdhci at 12530000 {
> +		compatible = "samsung,exynos4210-sdhci";
> +		reg = <0x12530000 0x100>;
> +		interrupts = <139>;
> +		samsung,sdhci-bus-width = <4>;
> +		linux,mmc_cap_4_bit_data;

Following on from my reply on patch 5, this is an example of exactly
what I'm talking about.  This node both sets bus-width to '4', and
sets the 4_bit_data flag.  Don't you think that the driver would be
smart enough to set the 4_bit_data flag when the bus width was set to
4?

g.

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

[Thomas Abraham]

Hi Grant,

On 8 November 2011 02:47, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Nov 03, 2011 at 02:06:03AM +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>
>> ---
>> +Example:
>> + ? ? sdhci at 12530000 {
>> + ? ? ? ? ? ? compatible = "samsung,exynos4210-sdhci";
>> + ? ? ? ? ? ? reg = <0x12530000 0x100>;
>> + ? ? ? ? ? ? interrupts = <139>;
>> + ? ? ? ? ? ? samsung,sdhci-bus-width = <4>;
>> + ? ? ? ? ? ? linux,mmc_cap_4_bit_data;
>
> Following on from my reply on patch 5, this is an example of exactly
> what I'm talking about. ?This node both sets bus-width to '4', and
> sets the 4_bit_data flag. ?Don't you think that the driver would be
> smart enough to set the 4_bit_data flag when the bus width was set to
> 4?

Yes, that is true. I will modify the driver based on your comments and
resubmit this patch.

Thanks for your comments.

Regards,
Thomas.

>
> g.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

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

[Sylwester Nawrocki]

Hi Kgene, Thomas

On 11/08/2011 04:23 PM, Thomas Abraham wrote:
> Hi Grant,
> 
> On 8 November 2011 02:47, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Thu, Nov 03, 2011 at 02:06:03AM +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>
>>> ---
>>> +Example:
>>> +     sdhci at 12530000 {
>>> +             compatible = "samsung,exynos4210-sdhci";
>>> +             reg = <0x12530000 0x100>;
>>> +             interrupts = <139>;
>>> +             samsung,sdhci-bus-width = <4>;
>>> +             linux,mmc_cap_4_bit_data;
>>
>> Following on from my reply on patch 5, this is an example of exactly
>> what I'm talking about.  This node both sets bus-width to '4', and
>> sets the 4_bit_data flag.  Don't you think that the driver would be
>> smart enough to set the 4_bit_data flag when the bus width was set to
>> 4?
> 
> Yes, that is true. I will modify the driver based on your comments and
> resubmit this patch.

Are we going to have those patches in 3.3-rc1 or only in 3.4 ?

--
Regards,
Sylwester

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

[Thomas Abraham]

Hi Sylwester,

On 4 January 2012 21:07, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> Hi Kgene, Thomas
>
> On 11/08/2011 04:23 PM, Thomas Abraham wrote:
>> Hi Grant,
>>
>> On 8 November 2011 02:47, Grant Likely <grant.likely@secretlab.ca> wrote:
>>> On Thu, Nov 03, 2011 at 02:06:03AM +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>
>>>> ---
>>>> +Example:
>>>> + ? ? sdhci at 12530000 {
>>>> + ? ? ? ? ? ? compatible = "samsung,exynos4210-sdhci";
>>>> + ? ? ? ? ? ? reg = <0x12530000 0x100>;
>>>> + ? ? ? ? ? ? interrupts = <139>;
>>>> + ? ? ? ? ? ? samsung,sdhci-bus-width = <4>;
>>>> + ? ? ? ? ? ? linux,mmc_cap_4_bit_data;
>>>
>>> Following on from my reply on patch 5, this is an example of exactly
>>> what I'm talking about. ?This node both sets bus-width to '4', and
>>> sets the 4_bit_data flag. ?Don't you think that the driver would be
>>> smart enough to set the 4_bit_data flag when the bus width was set to
>>> 4?
>>
>> Yes, that is true. I will modify the driver based on your comments and
>> resubmit this patch.
>
> Are we going to have those patches in 3.3-rc1 or only in 3.4 ?

Sorry for the delaying in completing this patchset. I will redo this
patchset and submit it soon. But 3.3-rc1 looks unlikely.

Thanks,
Thomas.

>
> --
> Regards,
> Sylwester

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

[Sylwester Nawrocki]

Hi Thomas,

On 01/05/2012 04:45 PM, Thomas Abraham wrote:
>>>> On Thu, Nov 03, 2011 at 02:06:03AM +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>
>>>>> ---
>>>>> +Example:
>>>>> +     sdhci at 12530000 {
>>>>> +             compatible = "samsung,exynos4210-sdhci";
>>>>> +             reg = <0x12530000 0x100>;
>>>>> +             interrupts = <139>;
>>>>> +             samsung,sdhci-bus-width = <4>;
>>>>> +             linux,mmc_cap_4_bit_data;
>>>>
>>>> Following on from my reply on patch 5, this is an example of exactly
>>>> what I'm talking about.  This node both sets bus-width to '4', and
>>>> sets the 4_bit_data flag.  Don't you think that the driver would be
>>>> smart enough to set the 4_bit_data flag when the bus width was set to
>>>> 4?
>>>
>>> Yes, that is true. I will modify the driver based on your comments and
>>> resubmit this patch.
>>
>> Are we going to have those patches in 3.3-rc1 or only in 3.4 ?
> 
> Sorry for the delaying in completing this patchset. I will redo this
> patchset and submit it soon. But 3.3-rc1 looks unlikely.

Ok, thanks. It's fine, there is no rush. I was just curious because we have
reused this sdhci driver update series and it seemed essential for initial
DT support on Exynos4 machines. Just FYI I have prepared initial DT support
patches for the S5P MIPI-CSI2 (s5p-mipi-csis) receiver driver.
But the whole camera subsystem needs more work, i.e. resource dependencies
(clock) between all involved devices (like image sensors and video capture
platform devices) need to be resolved and method of registering sub-devices
in the V4L2 core will likely need to be changed.


-- 
Thanks,
Sylwester

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

[Thomas Abraham]

Hi Sylwester,

On 5 January 2012 21:52, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
[...]

>> Sorry for the delaying in completing this patchset. I will redo this
>> patchset and submit it soon. But 3.3-rc1 looks unlikely.
>
> Ok, thanks. It's fine, there is no rush. I was just curious because we have
> reused this sdhci driver update series and it seemed essential for initial
> DT support on Exynos4 machines. Just FYI I have prepared initial DT support
> patches for the S5P MIPI-CSI2 (s5p-mipi-csis) receiver driver.
> But the whole camera subsystem needs more work, i.e. resource dependencies
> (clock) between all involved devices (like image sensors and video capture
> platform devices) need to be resolved and method of registering sub-devices
> in the V4L2 core will likely need to be changed.

Thanks for the information. It was helpful to know that you are working on this.

Regards,
Thomas.

>
>
> --
> Thanks,
> Sylwester

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

[Heiko Stübner]

Am Mittwoch, 2. November 2011, 21:36:03 schrieb Thomas Abraham:

Hi Thomas,

in patch 1/6:
> +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);


and in patch 6/6:
> +#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);

wouldn't it be better to keep the naming consistent between of and non-of?
I.e. s3c-sdhci vs. s3c6410-sdhci. Since the driver is used for all S3C SoCs 
containing hsmmc controllers I think s3c-sdhci would be preferable here.

Heiko

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

[Grant Likely]

On Mon, Jan 30, 2012 at 10:51:11AM +0100, Heiko St?bner wrote:
> Am Mittwoch, 2. November 2011, 21:36:03 schrieb Thomas Abraham:
> 
> Hi Thomas,
> 
> in patch 1/6:
> > +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);
> 
> 
> and in patch 6/6:
> > +#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);
> 
> wouldn't it be better to keep the naming consistent between of and non-of?
> I.e. s3c-sdhci vs. s3c6410-sdhci. Since the driver is used for all S3C SoCs 
> containing hsmmc controllers I think s3c-sdhci would be preferable here.

History has shown that future devices aren't always compatible with earlier
ones.  Compatible strings are expected to be specific to an exact device to
reduce the possibility of new hardware breaking assumptions.

Instead, new hardware can either claim compatibility with older
compatible strings (the compatible property in the DT is a list), or
can have the new string added to the match table in the driver;
whichever option makes the most sense.

g.

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

[Heiko Stübner]

Am Montag 30 Januar 2012, 20:01:14 schrieb Grant Likely:
> On Mon, Jan 30, 2012 at 10:51:11AM +0100, Heiko St?bner wrote:
> > Am Mittwoch, 2. November 2011, 21:36:03 schrieb Thomas Abraham:
> > 
> > Hi Thomas,
> > 
> > in patch 1/6:
> > > +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);
> > 
> > and in patch 6/6:
> > > +#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);
> > 
> > wouldn't it be better to keep the naming consistent between of and
> > non-of? I.e. s3c-sdhci vs. s3c6410-sdhci. Since the driver is used for
> > all S3C SoCs containing hsmmc controllers I think s3c-sdhci would be
> > preferable here.
> 
> History has shown that future devices aren't always compatible with earlier
> ones.  Compatible strings are expected to be specific to an exact device to
> reduce the possibility of new hardware breaking assumptions.
> 
> Instead, new hardware can either claim compatibility with older
> compatible strings (the compatible property in the DT is a list), or
> can have the new string added to the match table in the driver;
> whichever option makes the most sense.
ah, ok. Thanks for the explanation and I will keep that in mind.

Heiko