mmc: sdhci-s3c: Rework platform data and add device tree support

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<[email protected]>

---
  drivers/mmc/host/sdhci-s3c.c |    8 ++++++++
  1 files changed, 8 insertions(+), 0 deletions(-)

drivers/mmc/host/sdhci-s3c.c +0 -0

index e1b685d..a57d7a0 100644

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

Cc Ben Dooks <[email protected]>

Signed-off-by Thomas Abraham <[email protected]>

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

Documentation/devicetree/bindings/mmc/samsung-sdhci.txt new file +70 -0

new file mode 100644
index 0000000..8cbdd29
@@ -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 at 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.

drivers/mmc/host/sdhci-s3c.c +142 -6

index 1760ba8..6159542 100644
@@ -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,
 	},
 };

Signature · 1 line

1.6.6.rc2

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

Documentation/devicetree/bindings/mmc/samsung-sdhci.txt new file +16 -0

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

Documentation/devicetree/bindings/mmc/samsung-sdhci.txt new file +16 -0

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

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?

Quoted 8 lines · Arnd Bergmann

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.

Quoted 1 line

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?

Quoted 9 lines · Viresh Kumar

Quoted 8 lines · Arnd Bergmann

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.

Quoted 4 lines

Quoted 1 line

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 <[email protected]>

Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt +0 -0

-- interrupts : Should contain SD/MMC interrupt
+- interrupt : Should contain SD/MMC interrupt

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

arch/arm/boot/dts/tegra-cardhu.dts +0 -0

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.

Quoted 6 lines

+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?

Quoted 1 line

Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt +0 -0

...

Quoted 2 lines

-- interrupts : Should contain SD/MMC interrupt
+- interrupt : Should contain SD/MMC interrupt

Isn't that usually pluralized, so interrupts?

Quoted 1 line

+- 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?

Quoted 1 line

arch/arm/boot/dts/tegra-cardhu.dts +0 -0

...

arch/arm/boot/dts/tegra-cardhu.dts line 72

Quoted 7 lines

arch/arm/boot/dts/tegra-cardhu.dts +1 -0

@@ -66,5 +67,6 @@
 
      sdhci at 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.

drivers/mmc/host/sdhci-pltfm.c

Quoted 1 line

drivers/mmc/host/sdhci-pltfm.c +0 -0

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

Quoted 9 lines · Stephen Warren

Quoted 2 lines

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

Quoted 6 lines

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 <[email protected]>

Hi Arnd,

Quoted 6 lines · Stephen Warren

Quoted 6 lines

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 <[email protected]>

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 <[email protected]>
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 <[email protected]>

[cjb: typo fixes, addition of max-frequency property]

Signed-off-by Chris Ball <[email protected]>

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

Documentation/devicetree/bindings/mmc/fsl-esdhc.txt +4 -2

index 64bcb8b..0d93b4b 100644
@@ -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.
 

Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt +1 -1

index ab22fe6..c7e404b 100644
@@ -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

Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt +2 -1

index 89a0084..d64aea5 100644
@@ -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.

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

new file mode 100644
index 0000000..6e70dcd
@@ -0,0 +1,27 @@
+These properties are common to multiple MMC host controllers. Any host

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

+- 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 at 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>;
+}

Documentation/devicetree/bindings/mmc/nvidia-sdhci.txt +2 -2

index 7e51154..f77c303 100644
@@ -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 at 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>;
 };

Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt +2 -2

index dbd4368..8a53958 100644
@@ -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;
 	};

arch/arm/boot/dts/imx53-smd.dts +2 -2

index c7ee86c..139138a 100644
@@ -35,7 +35,7 @@
 				};
 
 				esdhc at 50008000 { /* ESDHC2 */
-					fsl,card-wired;
+					non-removable;
 					status = "okay";
 				};
 

@@ -76,7 +76,7 @@
 				};
 
 				esdhc at 50020000 { /* ESDHC3 */
-					fsl,card-wired;
+					non-removable;
 					status = "okay";
 				};
 			};

arch/arm/boot/dts/imx6q-arm2.dts +1 -1

index ce1c823..d2eaf52 100644
@@ -41,7 +41,7 @@
 			};
 
 			usdhc at 0219c000 { /* uSDHC4 */
-				fsl,card-wired;
+				non-removable;
 				vmmc-supply = <&reg_3p3v>;
 				status = "okay";
 			};

arch/arm/boot/dts/omap3-beagle.dts +1 -1

index 8c756be..5b4506c 100644
@@ -57,7 +57,7 @@
 &mmc1 {
 	vmmc-supply = <&vmmc1>;
 	vmmc_aux-supply = <&vsim>;
-	ti,bus-width = <8>;
+	bus-width = <8>;
 };
 
 &mmc2 {

arch/arm/boot/dts/omap4-panda.dts +2 -2

index ea6f5bb..31fb421 100644
@@ -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>;
 };

arch/arm/boot/dts/omap4-sdp.dts +3 -3

index 67b2e98..a1dd873 100644
@@ -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;
 };

arch/arm/boot/dts/tegra-cardhu.dts +2 -0

index facb950..f00480a 100644
@@ -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 at 78000200 {

@@ -141,6 +142,7 @@
 
 	sdhci at 78000400 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	ahub at 70080000 {

arch/arm/boot/dts/tegra-harmony.dts +2 -0

index 6857cec..7cd513a 100644
@@ -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 at 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 at c5004000 {

arch/arm/boot/dts/tegra-paz00.dts +2 -0

index 34a9177..8d625e4 100644
@@ -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 at c8000200 {

@@ -325,6 +326,7 @@
 
 	sdhci at c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	gpio-keys {

arch/arm/boot/dts/tegra-seaboard.dts +2 -0

index c4d171e..38e40a1 100644
@@ -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 at c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	usb at c5000000 {

arch/arm/boot/dts/tegra-ventana.dts +2 -0

index c20d5e9..149ab6d 100644
@@ -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 at c8000600 {
 		support-8bit;
+		bus-width = <8>;
 	};
 
 	usb at c5004000 {

arch/powerpc/boot/dts/mpc8569mds.dts +1 -0

index 7e283c8..fe0d609 100644
@@ -119,6 +119,7 @@
 		sdhc at 2e000 {
 			status = "disabled";
 			sdhci,1-bit-only;
+			bus-width = <1>;
 		};
 
 		par_io at e0100 {

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

index d15b149..ebaf62a 100644
@@ -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)

drivers/mmc/host/sdhci-esdhc-imx.c +1 -1

index 8abdaf6..0d2b082 100644
@@ -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))

drivers/mmc/host/sdhci-pltfm.c +6 -2

index c5c2a48..a3858d0 100644
@@ -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))

Signature · 6 lines

Chris Ball   <[email protected]>   <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 at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 5 lines · Stephen Warren

Quoted 5 lines

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.

Quoted 1 line

Quoted 1 line

Signed-off-by Arnd Bergmann <[email protected]>

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.

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.

Hi Arnd,

Quoted 3 lines · Arnd Bergmann

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

Quoted 9 lines · Viresh Kumar

Quoted 8 lines · Arnd Bergmann

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.

Quoted 4 lines

Quoted 1 line

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 <[email protected]>

Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt +3 -3

index dbd4368..90b86e5 100644
@@ -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;
 	};

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

index 47adb16..ae48fc7 100644

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

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.

Hi,

Quoted 3 lines · Kukjin Kim

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

Quoted 4 lines

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,

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.

Hi,

Quoted 6 lines

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.

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

Quoted 2 lines · Chris Ball

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?

Hi,

Quoted 5 lines · Mark Brown

Quoted 2 lines · Chris Ball

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

Quoted 2 lines

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?

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 <[email protected]>

---
 drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/mmc/host/sdhci-s3c.c +1 -0

index 46152d6..6926ac9 100644
@@ -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>
 

drivers/mmc/host/sdhci-s3c.c +23 -0

+
 	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)

drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 726

Quoted 1 line · Mark Brown

drivers/mmc/host/sdhci-s3c.c +1 -0

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

Could you explain why use 50ms?

drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 728

Quoted 2 lines

drivers/mmc/host/sdhci-s3c.c +2 -0

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

Is there reason that ignore_children use to set the true?

Quoted 5 lines · Jaehoon Chung

drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 726

Quoted 1 line · Mark Brown

drivers/mmc/host/sdhci-s3c.c +1 -0

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

Quoted 4 lines

drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 728

Quoted 2 lines

drivers/mmc/host/sdhci-s3c.c +2 -0

+	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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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 <[email protected]>

---
 drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/mmc/host/sdhci-s3c.c +24 -0

index 46152d6..6926ac9 100644
@@ -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);
+}

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 <[email protected]>

---
 drivers/mmc/host/sdhci-s3c.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/mmc/host/sdhci-s3c.c +24 -0

index 46152d6..6926ac9 100644
@@ -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);
+}

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

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 <[email protected]>

---
 drivers/mmc/host/sdhci-s3c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

drivers/mmc/host/sdhci-s3c.c +5 -4

index ea0767e..46152d6 100644
@@ -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)

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 <[email protected]>

---
 drivers/mmc/host/sdhci-s3c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

drivers/mmc/host/sdhci-s3c.c +2 -2

index ea0767e..46152d6 100644
@@ -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);
 }

Quoted 5 lines · Kukjin Kim

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

Quoted 10 lines · Chris Ball

Quoted 5 lines · Kukjin Kim

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:

Quoted 6 lines · Chris Ball

Quoted 5 lines · Chris Ball

Quoted 3 lines · Kukjin Kim

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

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

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,

Quoted 2 lines

and we're in the last few
days of the merge window.

Quoted 3 lines

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.

Quoted 4 lines

Quoted 3 lines

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.

Hi Kukjin,

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.