[PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Shawn Guo]

Here are what the patch set does.

* Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
  a pure common helper function providers.
* Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
  sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
  registered with calling helper functions created above.
* Migrate the use of sdhci_of_host and sdhci_of_data to
  sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
  data structure works can be saved, and pltfm version works for both
  cases.
* Add OF common helper stuff into sdhci-pltfm.c, and make OF version
  sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
  registered as well, so that sdhci-of-core.c and sdhci-of.h can be
  removed.
* Consolidate the OF and pltfm esdhc drivers into one with sharing
  the same pair of .probe and .remove hooks.  As a result,
  sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
  sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
* Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
  drivers/mmc/host/sdhci-pltfm.h.

And the benefits we gain from the changes are:

* Get the sdhci device driver follow the Linux trend that driver
  makes the registration by its own.
* sdhci-pltfm.c becomes simple and clean as it only has common helper
  stuff there now.
* All sdhci device specific things are going back its own driver.
* The dt and non-dt drivers are consolidated to use the same pair of
  .probe and .remove hooks.
* SDHCI driver for Freescale eSDHC controller found on both MPCxxx
  and i.MX platforms is consolidated to use the same one .probe
  function.

The patch set works against the tree below, and was only tested on
i.mx51 babbage board, all other targets were build tested.

  git://git.secretlab.ca/git/linux-2.6.git devicetree/test

Comments are welcomed and appreciated.

Regards,
Shawn

PS: The first patch is a squashing of the patch set below, which was
posted for review a few days back.

  [PATCH 0/5] make sdhci device drivers self registered

Some patches in this series are relatively large, involving more
changes than expected, I chose to not split considering they are
logically integral, and doing so can reduce the patch quantity much,
and make bisect much easier.  But sorry for that it makes reviewers'
life harder.

Shawn Guo (5):
      mmc: sdhci: make sdhci-pltfm device drivers self registered
      mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data
      mmc: sdhci: make sdhci-of device drivers self registered
      mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx
      mmc: sdhci: merge two sdhci-pltfm.h into one

 drivers/mmc/host/Kconfig           |   71 ++++---
 drivers/mmc/host/Makefile          |   17 +-
 drivers/mmc/host/sdhci-cns3xxx.c   |   68 ++++++-
 drivers/mmc/host/sdhci-dove.c      |   69 ++++++-
 drivers/mmc/host/sdhci-esdhc-imx.c |  149 -------------
 drivers/mmc/host/sdhci-esdhc.c     |  412 ++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-of-core.c   |  247 ---------------------
 drivers/mmc/host/sdhci-of-esdhc.c  |   89 --------
 drivers/mmc/host/sdhci-of-hlwd.c   |   89 +++++++-
 drivers/mmc/host/sdhci-of.h        |   42 ----
 drivers/mmc/host/sdhci-pltfm.c     |  251 +++++++++-------------
 drivers/mmc/host/sdhci-pltfm.h     |   36 +++-
 drivers/mmc/host/sdhci-tegra.c     |  187 ++++++++++-------
 include/linux/mmc/sdhci-pltfm.h    |   35 ---
 14 files changed, 912 insertions(+), 850 deletions(-)

[PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered

[Shawn Guo]

The patch turns the common stuff in sdhci-pltfm.c into functions, and
add device drivers their own .probe and .remove which in turn call
into the common functions, so that those sdhci-pltfm device drivers
register itself and keep all device specific things away from common
sdhci-pltfm file.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/host/Kconfig           |   24 +++--
 drivers/mmc/host/Makefile          |   11 +-
 drivers/mmc/host/sdhci-cns3xxx.c   |   67 +++++++++++++-
 drivers/mmc/host/sdhci-dove.c      |   69 +++++++++++++-
 drivers/mmc/host/sdhci-esdhc-imx.c |   97 +++++++++++++++----
 drivers/mmc/host/sdhci-pltfm.c     |  165 ++-----------------------------
 drivers/mmc/host/sdhci-pltfm.h     |   14 ++-
 drivers/mmc/host/sdhci-tegra.c     |  187 +++++++++++++++++++++---------------
 include/linux/mmc/sdhci-pltfm.h    |    6 -
 9 files changed, 360 insertions(+), 280 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index afe8c6f..1db9347 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -113,20 +113,17 @@ config MMC_SDHCI_OF_HLWD
 	  If unsure, say N.
 
 config MMC_SDHCI_PLTFM
-	tristate "SDHCI support on the platform specific bus"
+	bool
 	depends on MMC_SDHCI
 	help
-	  This selects the platform specific bus support for Secure Digital Host
-	  Controller Interface.
-
-	  If you have a controller with this interface, say Y or M here.
-
-	  If unsure, say N.
+	  This selects the platform common function support for Secure Digital
+	  Host Controller Interface.
 
 config MMC_SDHCI_CNS3XXX
 	bool "SDHCI support on the Cavium Networks CNS3xxx SoC"
 	depends on ARCH_CNS3XXX
-	depends on MMC_SDHCI_PLTFM
+	depends on MMC_SDHCI
+	select MMC_SDHCI_PLTFM
 	help
 	  This selects the SDHCI support for CNS3xxx System-on-Chip devices.
 
@@ -134,7 +131,9 @@ config MMC_SDHCI_CNS3XXX
 
 config MMC_SDHCI_ESDHC_IMX
 	bool "SDHCI platform support for the Freescale eSDHC i.MX controller"
-	depends on MMC_SDHCI_PLTFM && (ARCH_MX25 || ARCH_MX35 || ARCH_MX5)
+	depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
+	depends on MMC_SDHCI
+	select MMC_SDHCI_PLTFM
 	select MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Freescale eSDHC controller support on the platform
@@ -145,7 +144,8 @@ config MMC_SDHCI_ESDHC_IMX
 config MMC_SDHCI_DOVE
 	bool "SDHCI support on Marvell's Dove SoC"
 	depends on ARCH_DOVE
-	depends on MMC_SDHCI_PLTFM
+	depends on MMC_SDHCI
+	select MMC_SDHCI_PLTFM
 	select MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Secure Digital Host Controller Interface in
@@ -155,7 +155,9 @@ config MMC_SDHCI_DOVE
 
 config MMC_SDHCI_TEGRA
 	tristate "SDHCI platform support for the Tegra SD/MMC Controller"
-	depends on MMC_SDHCI_PLTFM && ARCH_TEGRA
+	depends on ARCH_TEGRA
+	depends on MMC_SDHCI
+	select MMC_SDHCI_PLTFM
 	select MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Tegra SD/MMC controller. If you have a Tegra
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e834fb2..1d8e43d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -36,12 +36,11 @@ obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
 obj-$(CONFIG_MMC_USHC)		+= ushc.o
 
-obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
-sdhci-platform-y				:= sdhci-pltfm.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX)	+= sdhci-cns3xxx.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_TEGRA)	+= sdhci-tegra.o
+obj-$(CONFIG_MMC_SDHCI_PLTFM)		+= sdhci-pltfm.o
+obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
+obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
+obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
+obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= sdhci-tegra.o
 
 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
index 9ebd1d7..95b9080 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -86,7 +86,7 @@ static struct sdhci_ops sdhci_cns3xxx_ops = {
 	.set_clock	= sdhci_cns3xxx_set_clock,
 };
 
-struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
+static struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
 	.ops = &sdhci_cns3xxx_ops,
 	.quirks = SDHCI_QUIRK_BROKEN_DMA |
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
@@ -95,3 +95,68 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
 		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
 		  SDHCI_QUIRK_NONSTANDARD_CLOCK,
 };
+
+static int __devinit sdhci_cns3xxx_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	int ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_cns3xxx_pdata);
+	if (!host)
+		return -ENOMEM;
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
+	return 0;
+
+err_add_host:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int __devexit sdhci_cns3xxx_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	int dead = 0;
+	u32 scratch;
+
+	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (scratch == (u32)-1)
+		dead = 1;
+
+	sdhci_remove_host(host, dead);
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
+static struct platform_driver sdhci_cns3xxx_driver = {
+	.driver		= {
+		.name	= "sdhci-cns3xxx",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_cns3xxx_probe,
+	.remove		= __devexit_p(sdhci_cns3xxx_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
+#endif
+};
+
+static int __init sdhci_cns3xxx_init(void)
+{
+	return platform_driver_register(&sdhci_cns3xxx_driver);
+}
+module_init(sdhci_cns3xxx_init);
+
+static void __exit sdhci_cns3xxx_exit(void)
+{
+	platform_driver_unregister(&sdhci_cns3xxx_driver);
+}
+module_exit(sdhci_cns3xxx_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for CNS3xxx");
+MODULE_AUTHOR("Anton Vorontsov <avorontsov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index 2aeef4f..6926d4a 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -3,7 +3,7 @@
  *
  * Author: Saeed Bishara <saeed-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
  *	   Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
- * Based on sdhci-cns3xxx.c
+ * Based on sdhci-dove.c
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -61,10 +61,75 @@ static struct sdhci_ops sdhci_dove_ops = {
 	.read_l	= sdhci_dove_readl,
 };
 
-struct sdhci_pltfm_data sdhci_dove_pdata = {
+static struct sdhci_pltfm_data sdhci_dove_pdata = {
 	.ops	= &sdhci_dove_ops,
 	.quirks	= SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
 		  SDHCI_QUIRK_NO_BUSY_IRQ |
 		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
 		  SDHCI_QUIRK_FORCE_DMA,
 };
+
+static int __devinit sdhci_dove_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	int ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_dove_pdata);
+	if (!host)
+		return -ENOMEM;
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
+	return 0;
+
+err_add_host:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int __devexit sdhci_dove_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	int dead = 0;
+	u32 scratch;
+
+	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (scratch == (u32)-1)
+		dead = 1;
+
+	sdhci_remove_host(host, dead);
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
+static struct platform_driver sdhci_dove_driver = {
+	.driver		= {
+		.name	= "sdhci-dove",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_dove_probe,
+	.remove		= __devexit_p(sdhci_dove_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
+#endif
+};
+
+static int __init sdhci_dove_init(void)
+{
+	return platform_driver_register(&sdhci_dove_driver);
+}
+module_init(sdhci_dove_init);
+
+static void __exit sdhci_dove_exit(void)
+{
+	platform_driver_unregister(&sdhci_dove_driver);
+}
+module_exit(sdhci_dove_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for Dove");
+MODULE_AUTHOR("Saeed Bishara <saeed-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b82910..c9eb507 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -100,15 +100,39 @@ static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
 	return clk_get_rate(pltfm_host->clk) / 256 / 16;
 }
 
-static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pdata)
+static struct sdhci_ops sdhci_esdhc_ops = {
+	.read_w = esdhc_readw_le,
+	.write_w = esdhc_writew_le,
+	.write_b = esdhc_writeb_le,
+	.set_clock = esdhc_set_clock,
+	.get_max_clock = esdhc_pltfm_get_max_clock,
+	.get_min_clock = esdhc_pltfm_get_min_clock,
+};
+
+static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+	/* ADMA has issues. Might be fixable */
+	.ops = &sdhci_esdhc_ops,
+};
+
+static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 {
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_host *host;
 	struct clk *clk;
+	int ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_imx_pdata);
+	if (!host)
+		return -ENOMEM;
+
+	pltfm_host = sdhci_priv(host);
 
 	clk = clk_get(mmc_dev(host->mmc), NULL);
 	if (IS_ERR(clk)) {
 		dev_err(mmc_dev(host->mmc), "clk err\n");
-		return PTR_ERR(clk);
+		ret = PTR_ERR(clk);
+		goto err_clk_get;
 	}
 	clk_enable(clk);
 	pltfm_host->clk = clk;
@@ -120,30 +144,67 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
 	if (cpu_is_mx25() || cpu_is_mx35())
 		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
 
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
 	return 0;
+
+err_add_host:
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+err_clk_get:
+	sdhci_pltfm_free(pdev);
+	return ret;
 }
 
-static void esdhc_pltfm_exit(struct sdhci_host *host)
+static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
 {
+	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	int dead = 0;
+	u32 scratch;
+
+	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (scratch == (u32)-1)
+		dead = 1;
+
+	sdhci_remove_host(host, dead);
 
 	clk_disable(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
+
+	sdhci_pltfm_free(pdev);
+
+	return 0;
 }
 
-static struct sdhci_ops sdhci_esdhc_ops = {
-	.read_w = esdhc_readw_le,
-	.write_w = esdhc_writew_le,
-	.write_b = esdhc_writeb_le,
-	.set_clock = esdhc_set_clock,
-	.get_max_clock = esdhc_pltfm_get_max_clock,
-	.get_min_clock = esdhc_pltfm_get_min_clock,
+static struct platform_driver sdhci_esdhc_imx_driver = {
+	.driver		= {
+		.name	= "sdhci-esdhc-imx",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_esdhc_imx_probe,
+	.remove		= __devexit_p(sdhci_esdhc_imx_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
+#endif
 };
 
-struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
-	/* ADMA has issues. Might be fixable */
-	.ops = &sdhci_esdhc_ops,
-	.init = esdhc_pltfm_init,
-	.exit = esdhc_pltfm_exit,
-};
+static int __init sdhci_esdhc_imx_init(void)
+{
+	return platform_driver_register(&sdhci_esdhc_imx_driver);
+}
+
+static void __exit sdhci_esdhc_imx_exit(void)
+{
+	platform_driver_unregister(&sdhci_esdhc_imx_driver);
+}
+
+module_init(sdhci_esdhc_imx_init);
+module_exit(sdhci_esdhc_imx_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for i.MX eSDHC");
+MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index ccc04ac..63e45aa 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -22,70 +22,22 @@
  * Inspired by sdhci-pci.c, by Pierre Ossman
  */
 
-#include <linux/delay.h>
-#include <linux/highmem.h>
-#include <linux/mod_devicetable.h>
-#include <linux/platform_device.h>
-
-#include <linux/mmc/host.h>
-
-#include <linux/io.h>
-#include <linux/mmc/sdhci-pltfm.h>
+#include <linux/err.h>
 
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 
-/*****************************************************************************\
- *                                                                           *
- * SDHCI core callbacks                                                      *
- *                                                                           *
-\*****************************************************************************/
-
 static struct sdhci_ops sdhci_pltfm_ops = {
 };
 
-/*****************************************************************************\
- *                                                                           *
- * Device probing/removal                                                    *
- *                                                                           *
-\*****************************************************************************/
-#if defined(CONFIG_OF)
-#include <linux/of_device.h>
-static const struct of_device_id sdhci_dt_ids[] = {
-	{ .compatible = "nvidia,tegra250-sdhci", .data = &sdhci_tegra_dt_pdata },
-	{ }
-};
-MODULE_DEVICE_TABLE(platform, sdhci_dt_ids);
-
-static const struct of_device_id *sdhci_get_of_device_id(struct platform_device *pdev)
-{
-	return of_match_device(sdhci_dt_ids, &pdev->dev);
-}
-#else
-#define shdci_dt_ids NULL
-static inline struct of_device_id *sdhci_get_of_device_id(struct platform_device *pdev)
-{
-	return NULL;
-}
-#endif
-
-static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
+struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
+				    struct sdhci_pltfm_data *pdata)
 {
-	const struct platform_device_id *platid = platform_get_device_id(pdev);
-	const struct of_device_id *dtid = sdhci_get_of_device_id(pdev);
-	struct sdhci_pltfm_data *pdata;
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct resource *iomem;
 	int ret;
 
-	if (platid && platid->driver_data)
-		pdata = (void *)platid->driver_data;
-	else if (dtid && dtid->data)
-		pdata = dtid->data;
-	else
-		pdata = pdev->dev.platform_data;
-
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!iomem) {
 		ret = -ENOMEM;
@@ -93,8 +45,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 	}
 
 	if (resource_size(iomem) < 0x100)
-		dev_err(&pdev->dev, "Invalid iomem size. You may "
-			"experience problems.\n");
+		dev_err(&pdev->dev, "Invalid iomem size!\n");
 
 	/* Some PCI-based MFD need the parent here */
 	if (pdev->dev.parent != &platform_bus)
@@ -109,7 +60,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 
 	pltfm_host = sdhci_priv(host);
 
-	host->hw_name = "platform";
+	host->hw_name = dev_name(&pdev->dev);
 	if (pdata && pdata->ops)
 		host->ops = pdata->ops;
 	else
@@ -132,136 +83,42 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 		goto err_remap;
 	}
 
-	if (pdata && pdata->init) {
-		ret = pdata->init(host, pdata);
-		if (ret)
-			goto err_plat_init;
-	}
-
-	ret = sdhci_add_host(host);
-	if (ret)
-		goto err_add_host;
-
 	platform_set_drvdata(pdev, host);
 
-	return 0;
+	return host;
 
-err_add_host:
-	if (pdata && pdata->exit)
-		pdata->exit(host);
-err_plat_init:
-	iounmap(host->ioaddr);
 err_remap:
 	release_mem_region(iomem->start, resource_size(iomem));
 err_request:
 	sdhci_free_host(host);
 err:
-	printk(KERN_ERR"Probing of sdhci-pltfm failed: %d\n", ret);
-	return ret;
+	pr_err("%s failed %d\n", __func__, ret);
+	return NULL;
 }
 
-static int __devexit sdhci_pltfm_remove(struct platform_device *pdev)
+void sdhci_pltfm_free(struct platform_device *pdev)
 {
-	const struct platform_device_id *platid = platform_get_device_id(pdev);
-	const struct of_device_id *dtid = sdhci_get_of_device_id(pdev);
-	struct sdhci_pltfm_data *pdata;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	int dead;
-	u32 scratch;
 
-	if (platid && platid->driver_data)
-		pdata = (void *)platid->driver_data;
-	else if (dtid && dtid->data)
-		pdata = dtid->data;
-	else
-		pdata = pdev->dev.platform_data;
-
-	dead = 0;
-	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
-	if (scratch == (u32)-1)
-		dead = 1;
-
-	sdhci_remove_host(host, dead);
-	if (pdata && pdata->exit)
-		pdata->exit(host);
 	iounmap(host->ioaddr);
 	release_mem_region(iomem->start, resource_size(iomem));
 	sdhci_free_host(host);
 	platform_set_drvdata(pdev, NULL);
-
-	return 0;
 }
 
-static const struct platform_device_id sdhci_pltfm_ids[] = {
-	{ "sdhci", },
-#ifdef CONFIG_MMC_SDHCI_CNS3XXX
-	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
-#endif
-#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
-	{ "sdhci-esdhc-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata },
-#endif
-#ifdef CONFIG_MMC_SDHCI_DOVE
-	{ "sdhci-dove", (kernel_ulong_t)&sdhci_dove_pdata },
-#endif
-#ifdef CONFIG_MMC_SDHCI_TEGRA
-	{ "sdhci-tegra", (kernel_ulong_t)&sdhci_tegra_pdata },
-#endif
-	{ },
-};
-MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
-
 #ifdef CONFIG_PM
-static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state)
+int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct sdhci_host *host = platform_get_drvdata(dev);
 
 	return sdhci_suspend_host(host, state);
 }
 
-static int sdhci_pltfm_resume(struct platform_device *dev)
+int sdhci_pltfm_resume(struct platform_device *dev)
 {
 	struct sdhci_host *host = platform_get_drvdata(dev);
 
 	return sdhci_resume_host(host);
 }
-#else
-#define sdhci_pltfm_suspend	NULL
-#define sdhci_pltfm_resume	NULL
 #endif	/* CONFIG_PM */
-
-static struct platform_driver sdhci_pltfm_driver = {
-	.driver = {
-		.name	= "sdhci",
-		.owner	= THIS_MODULE,
-		.of_match_table = sdhci_dt_ids,
-	},
-	.probe		= sdhci_pltfm_probe,
-	.remove		= __devexit_p(sdhci_pltfm_remove),
-	.id_table	= sdhci_pltfm_ids,
-	.suspend	= sdhci_pltfm_suspend,
-	.resume		= sdhci_pltfm_resume,
-};
-
-/*****************************************************************************\
- *                                                                           *
- * Driver init/exit                                                          *
- *                                                                           *
-\*****************************************************************************/
-
-static int __init sdhci_drv_init(void)
-{
-	return platform_driver_register(&sdhci_pltfm_driver);
-}
-
-static void __exit sdhci_drv_exit(void)
-{
-	platform_driver_unregister(&sdhci_pltfm_driver);
-}
-
-module_init(sdhci_drv_init);
-module_exit(sdhci_drv_exit);
-
-MODULE_DESCRIPTION("Secure Digital Host Controller Interface platform driver");
-MODULE_AUTHOR("Mocean Laboratories <info-l7gf1WXxx3uGw+nKnLezzg@public.gmane.org>");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index c67523d..a3e4be0 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -13,6 +13,7 @@
 
 #include <linux/clk.h>
 #include <linux/types.h>
+#include <linux/platform_device.h>
 #include <linux/mmc/sdhci-pltfm.h>
 
 struct sdhci_pltfm_host {
@@ -20,10 +21,13 @@ struct sdhci_pltfm_host {
 	u32 scratchpad; /* to handle quirks across io-accessor calls */
 };
 
-extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
-extern struct sdhci_pltfm_data sdhci_esdhc_imx_pdata;
-extern struct sdhci_pltfm_data sdhci_dove_pdata;
-extern struct sdhci_pltfm_data sdhci_tegra_pdata;
-extern struct sdhci_pltfm_data sdhci_tegra_dt_pdata;
+extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
+					   struct sdhci_pltfm_data *pdata);
+extern void sdhci_pltfm_free(struct platform_device *pdev);
+
+#ifdef CONFIG_PM
+extern int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state);
+extern int sdhci_pltfm_resume(struct platform_device *dev);
+#endif
 
 #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index c3d6f83..cfcd521 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -119,19 +119,58 @@ static int tegra_sdhci_8bit(struct sdhci_host *host, int bus_width)
 }
 
 
-static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
-				  struct sdhci_pltfm_data *pdata)
+static struct sdhci_ops tegra_sdhci_ops = {
+	.get_ro     = tegra_sdhci_get_ro,
+	.read_l     = tegra_sdhci_readl,
+	.read_w     = tegra_sdhci_readw,
+	.write_l    = tegra_sdhci_writel,
+	.platform_8bit_width = tegra_sdhci_8bit,
+};
+
+static struct sdhci_pltfm_data sdhci_tegra_pdata = {
+	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
+		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
+		  SDHCI_QUIRK_NO_HISPD_BIT |
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+	.ops  = &tegra_sdhci_ops,
+};
+
+static int __devinit sdhci_tegra_probe(struct platform_device *pdev)
 {
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pltfm_host *pltfm_host;
 	struct tegra_sdhci_platform_data *plat;
+	struct sdhci_host *host;
 	struct clk *clk;
 	int rc;
 
+	host = sdhci_pltfm_init(pdev, &sdhci_tegra_pdata);
+	if (!host)
+		return -ENOMEM;
+
+	pltfm_host = sdhci_priv(host);
+
 	plat = pdev->dev.platform_data;
+
+#ifdef CONFIG_OF
+	plat = kzalloc(sizeof(*plat), GFP_KERNEL);
+	if (!plat) {
+		rc = -ENOMEM;
+		goto err_no_plat;
+	}
+	pdev->dev.platform_data = plat;
+
+	plat->cd_gpio = of_get_gpio(pdev->dev.of_node, 0);
+	plat->wp_gpio = of_get_gpio(pdev->dev.of_node, 1);
+	plat->power_gpio = of_get_gpio(pdev->dev.of_node, 2);
+
+	dev_info(&pdev->dev, "using gpios cd=%i, wp=%i power=%i\n",
+		plat->cd_gpio, plat->wp_gpio, plat->power_gpio);
+#endif
+
 	if (plat == NULL) {
 		dev_err(mmc_dev(host->mmc), "missing platform data\n");
-		return -ENXIO;
+		rc = -ENXIO;
+		goto err_no_plat;
 	}
 
 	if (gpio_is_valid(plat->power_gpio)) {
@@ -139,7 +178,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
 		if (rc) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to allocate power gpio\n");
-			goto out;
+			goto err_power_req;
 		}
 		tegra_gpio_enable(plat->power_gpio);
 		gpio_direction_output(plat->power_gpio, 1);
@@ -150,7 +189,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
 		if (rc) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to allocate cd gpio\n");
-			goto out_power;
+			goto err_cd_req;
 		}
 		tegra_gpio_enable(plat->cd_gpio);
 		gpio_direction_input(plat->cd_gpio);
@@ -161,7 +200,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
 
 		if (rc)	{
 			dev_err(mmc_dev(host->mmc), "request irq error\n");
-			goto out_cd;
+			goto err_cd_irq_req;
 		}
 
 	}
@@ -171,7 +210,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
 		if (rc) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to allocate wp gpio\n");
-			goto out_cd;
+			goto err_wp_req;
 		}
 		tegra_gpio_enable(plat->wp_gpio);
 		gpio_direction_input(plat->wp_gpio);
@@ -181,7 +220,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
 	if (IS_ERR(clk)) {
 		dev_err(mmc_dev(host->mmc), "clk err\n");
 		rc = PTR_ERR(clk);
-		goto out_wp;
+		goto err_clk_get;
 	}
 	clk_enable(clk);
 	pltfm_host->clk = clk;
@@ -189,62 +228,56 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
 	if (plat->is_8bit)
 		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
 
+	rc = sdhci_add_host(host);
+	if (rc)
+		goto err_add_host;
+
 	return 0;
 
-out_wp:
+err_add_host:
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+err_clk_get:
 	if (gpio_is_valid(plat->wp_gpio)) {
 		tegra_gpio_disable(plat->wp_gpio);
 		gpio_free(plat->wp_gpio);
 	}
-
-out_cd:
+err_wp_req:
+	if (gpio_is_valid(plat->cd_gpio)) {
+		free_irq(gpio_to_irq(plat->cd_gpio), host);
+	}
+err_cd_irq_req:
 	if (gpio_is_valid(plat->cd_gpio)) {
 		tegra_gpio_disable(plat->cd_gpio);
 		gpio_free(plat->cd_gpio);
 	}
-
-out_power:
+err_cd_req:
 	if (gpio_is_valid(plat->power_gpio)) {
 		tegra_gpio_disable(plat->power_gpio);
 		gpio_free(plat->power_gpio);
 	}
-
-out:
+err_power_req:
+#ifdef CONFIG_OF
+	kfree(plat);
+#endif
+err_no_plat:
+	sdhci_pltfm_free(pdev);
 	return rc;
 }
 
-static int tegra_sdhci_pltfm_dt_init(struct sdhci_host *host,
-				     struct sdhci_pltfm_data *pdata)
+static int __devexit sdhci_tegra_remove(struct platform_device *pdev)
 {
-	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct tegra_sdhci_platform_data *plat;
+	int dead = 0;
+	u32 scratch;
 
-	if (pdev->dev.platform_data) {
-		dev_err(&pdev->dev, "%s: platform_data not NULL; aborting\n",
-			__func__);
-		return -ENODEV;
-	}
+	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (scratch == (u32)-1)
+		dead = 1;
 
-	plat = kzalloc(sizeof(*plat), GFP_KERNEL);
-	if (!plat)
-		return -ENOMEM;
-	pdev->dev.platform_data = plat;
-
-	plat->cd_gpio = of_get_gpio(pdev->dev.of_node, 0);
-	plat->wp_gpio = of_get_gpio(pdev->dev.of_node, 1);
-	plat->power_gpio = of_get_gpio(pdev->dev.of_node, 2);
-
-	dev_info(&pdev->dev, "using gpios cd=%i, wp=%i power=%i\n",
-		plat->cd_gpio, plat->wp_gpio, plat->power_gpio);
-
-	return tegra_sdhci_pltfm_init(host, pdata);
-}
-
-static void tegra_sdhci_pltfm_exit(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
-	struct tegra_sdhci_platform_data *plat;
+	sdhci_remove_host(host, dead);
 
 	plat = pdev->dev.platform_data;
 
@@ -263,44 +296,44 @@ static void tegra_sdhci_pltfm_exit(struct sdhci_host *host)
 		gpio_free(plat->power_gpio);
 	}
 
+#ifdef CONFIG_OF
+	kfree(pdev->dev.platform_data);
+	pdev->dev.platform_data = NULL;
+#endif
+
 	clk_disable(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
-}
 
-static void tegra_sdhci_pltfm_dt_exit(struct sdhci_host *host)
-{
-	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
-
-	tegra_sdhci_pltfm_exit(host);
+	sdhci_pltfm_free(pdev);
 
-	kfree(pdev->dev.platform_data);
-	pdev->dev.platform_data = NULL;
+	return 0;
 }
 
-static struct sdhci_ops tegra_sdhci_ops = {
-	.get_ro     = tegra_sdhci_get_ro,
-	.read_l     = tegra_sdhci_readl,
-	.read_w     = tegra_sdhci_readw,
-	.write_l    = tegra_sdhci_writel,
-	.platform_8bit_width = tegra_sdhci_8bit,
+static struct platform_driver sdhci_tegra_driver = {
+	.driver		= {
+		.name	= "sdhci-tegra",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_tegra_probe,
+	.remove		= __devexit_p(sdhci_tegra_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
+#endif
 };
 
-struct sdhci_pltfm_data sdhci_tegra_pdata = {
-	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
-		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
-		  SDHCI_QUIRK_NO_HISPD_BIT |
-		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
-	.ops  = &tegra_sdhci_ops,
-	.init = tegra_sdhci_pltfm_init,
-	.exit = tegra_sdhci_pltfm_exit,
-};
+static int __init sdhci_tegra_init(void)
+{
+	return platform_driver_register(&sdhci_tegra_driver);
+}
+module_init(sdhci_tegra_init);
 
-struct sdhci_pltfm_data sdhci_tegra_dt_pdata = {
-	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
-		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
-		  SDHCI_QUIRK_NO_HISPD_BIT |
-		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
-	.ops  = &tegra_sdhci_ops,
-	.init = tegra_sdhci_pltfm_dt_init,
-	.exit = tegra_sdhci_pltfm_dt_exit,
-};
+static void __exit sdhci_tegra_exit(void)
+{
+	platform_driver_unregister(&sdhci_tegra_driver);
+}
+module_exit(sdhci_tegra_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for Tegra");
+MODULE_AUTHOR(" Google, Inc.");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
index 548d59d..f1c2ac3 100644
--- a/include/linux/mmc/sdhci-pltfm.h
+++ b/include/linux/mmc/sdhci-pltfm.h
@@ -15,21 +15,15 @@
 #define _SDHCI_PLTFM_H
 
 struct sdhci_ops;
-struct sdhci_host;
 
 /**
  * struct sdhci_pltfm_data - SDHCI platform-specific information & hooks
  * @ops: optional pointer to the platform-provided SDHCI ops
  * @quirks: optional SDHCI quirks
- * @init: optional hook that is called during device probe, before the
- *        driver tries to access any SDHCI registers
- * @exit: optional hook that is called during device removal
  */
 struct sdhci_pltfm_data {
 	struct sdhci_ops *ops;
 	unsigned int quirks;
-	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata);
-	void (*exit)(struct sdhci_host *host);
 };
 
 #endif /* _SDHCI_PLTFM_H */
-- 
1.7.1

Re: [PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered

[Grant Likely]

On Fri, Mar 25, 2011 at 04:48:47PM +0800, Shawn Guo wrote:
> The patch turns the common stuff in sdhci-pltfm.c into functions, and
> add device drivers their own .probe and .remove which in turn call
> into the common functions, so that those sdhci-pltfm device drivers
> register itself and keep all device specific things away from common
> sdhci-pltfm file.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Looks really good.  Relatively minor comments below, but you can add
this to the next version:

Reviewed-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

Thanks for doing this!
g.

> ---
>  drivers/mmc/host/Kconfig           |   24 +++--
>  drivers/mmc/host/Makefile          |   11 +-
>  drivers/mmc/host/sdhci-cns3xxx.c   |   67 +++++++++++++-
>  drivers/mmc/host/sdhci-dove.c      |   69 +++++++++++++-
>  drivers/mmc/host/sdhci-esdhc-imx.c |   97 +++++++++++++++----
>  drivers/mmc/host/sdhci-pltfm.c     |  165 ++-----------------------------
>  drivers/mmc/host/sdhci-pltfm.h     |   14 ++-
>  drivers/mmc/host/sdhci-tegra.c     |  187 +++++++++++++++++++++---------------
>  include/linux/mmc/sdhci-pltfm.h    |    6 -
>  9 files changed, 360 insertions(+), 280 deletions(-)
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index afe8c6f..1db9347 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -113,20 +113,17 @@ config MMC_SDHCI_OF_HLWD
>  	  If unsure, say N.
>  
>  config MMC_SDHCI_PLTFM
> -	tristate "SDHCI support on the platform specific bus"
> +	bool
>  	depends on MMC_SDHCI
>  	help
> -	  This selects the platform specific bus support for Secure Digital Host
> -	  Controller Interface.
> -
> -	  If you have a controller with this interface, say Y or M here.
> -
> -	  If unsure, say N.
> +	  This selects the platform common function support for Secure Digital
> +	  Host Controller Interface.
>  
>  config MMC_SDHCI_CNS3XXX
>  	bool "SDHCI support on the Cavium Networks CNS3xxx SoC"
>  	depends on ARCH_CNS3XXX
> -	depends on MMC_SDHCI_PLTFM
> +	depends on MMC_SDHCI
> +	select MMC_SDHCI_PLTFM
>  	help
>  	  This selects the SDHCI support for CNS3xxx System-on-Chip devices.
>  
> @@ -134,7 +131,9 @@ config MMC_SDHCI_CNS3XXX
>  
>  config MMC_SDHCI_ESDHC_IMX
>  	bool "SDHCI platform support for the Freescale eSDHC i.MX controller"
> -	depends on MMC_SDHCI_PLTFM && (ARCH_MX25 || ARCH_MX35 || ARCH_MX5)
> +	depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
> +	depends on MMC_SDHCI
> +	select MMC_SDHCI_PLTFM
>  	select MMC_SDHCI_IO_ACCESSORS
>  	help
>  	  This selects the Freescale eSDHC controller support on the platform
> @@ -145,7 +144,8 @@ config MMC_SDHCI_ESDHC_IMX
>  config MMC_SDHCI_DOVE
>  	bool "SDHCI support on Marvell's Dove SoC"
>  	depends on ARCH_DOVE
> -	depends on MMC_SDHCI_PLTFM
> +	depends on MMC_SDHCI
> +	select MMC_SDHCI_PLTFM
>  	select MMC_SDHCI_IO_ACCESSORS
>  	help
>  	  This selects the Secure Digital Host Controller Interface in
> @@ -155,7 +155,9 @@ config MMC_SDHCI_DOVE
>  
>  config MMC_SDHCI_TEGRA
>  	tristate "SDHCI platform support for the Tegra SD/MMC Controller"
> -	depends on MMC_SDHCI_PLTFM && ARCH_TEGRA
> +	depends on ARCH_TEGRA
> +	depends on MMC_SDHCI
> +	select MMC_SDHCI_PLTFM
>  	select MMC_SDHCI_IO_ACCESSORS
>  	help
>  	  This selects the Tegra SD/MMC controller. If you have a Tegra
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e834fb2..1d8e43d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -36,12 +36,11 @@ obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
>  obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
>  obj-$(CONFIG_MMC_USHC)		+= ushc.o
>  
> -obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
> -sdhci-platform-y				:= sdhci-pltfm.o
> -sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX)	+= sdhci-cns3xxx.o
> -sdhci-platform-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
> -sdhci-platform-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
> -sdhci-platform-$(CONFIG_MMC_SDHCI_TEGRA)	+= sdhci-tegra.o
> +obj-$(CONFIG_MMC_SDHCI_PLTFM)		+= sdhci-pltfm.o
> +obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
> +obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
> +obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
> +obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= sdhci-tegra.o
>  
>  obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
>  sdhci-of-y				:= sdhci-of-core.o
> diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
> index 9ebd1d7..95b9080 100644
> --- a/drivers/mmc/host/sdhci-cns3xxx.c
> +++ b/drivers/mmc/host/sdhci-cns3xxx.c
> @@ -86,7 +86,7 @@ static struct sdhci_ops sdhci_cns3xxx_ops = {
>  	.set_clock	= sdhci_cns3xxx_set_clock,
>  };
>  
> -struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
> +static struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
>  	.ops = &sdhci_cns3xxx_ops,
>  	.quirks = SDHCI_QUIRK_BROKEN_DMA |
>  		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> @@ -95,3 +95,68 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
>  		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
>  		  SDHCI_QUIRK_NONSTANDARD_CLOCK,
>  };
> +
> +static int __devinit sdhci_cns3xxx_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	int ret;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_cns3xxx_pdata);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		goto err_add_host;
> +
> +	return 0;
> +
> +err_add_host:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}

This pattern in this function is used by 2 drivers in this patch, and
2 more users (with the addition of an sdhci_get_of_property() call) in
the 3rd patch.  An sdchi_pltfm_register(pdev, &pdata) that does the
whole sequence would probably be valuable.  Same for the _remove hook.

Drivers that still need 2 stage registration, like the tegra driver,
would still be able to call sdhci_pltfm_init() and sdhci_add_host()
directly.

> +
> +static int __devexit sdhci_cns3xxx_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	int dead = 0;
> +	u32 scratch;
> +
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
> +
> +	sdhci_remove_host(host, dead);

The following would be equivalent to the above three lines:

	sdhci_remove_host(host, scratch == (u32)-1);

> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sdhci_cns3xxx_driver = {
> +	.driver		= {
> +		.name	= "sdhci-cns3xxx",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= sdhci_cns3xxx_probe,
> +	.remove		= __devexit_p(sdhci_cns3xxx_remove),
> +#ifdef CONFIG_PM
> +	.suspend	= sdhci_pltfm_suspend,
> +	.resume		= sdhci_pltfm_resume,
> +#endif
> +};
> +
> +static int __init sdhci_cns3xxx_init(void)
> +{
> +	return platform_driver_register(&sdhci_cns3xxx_driver);
> +}
> +module_init(sdhci_cns3xxx_init);
> +
> +static void __exit sdhci_cns3xxx_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_cns3xxx_driver);
> +}
> +module_exit(sdhci_cns3xxx_exit);
> +
> +MODULE_DESCRIPTION("SDHCI driver for CNS3xxx");
> +MODULE_AUTHOR("Anton Vorontsov <avorontsov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
> index 2aeef4f..6926d4a 100644
> --- a/drivers/mmc/host/sdhci-dove.c
> +++ b/drivers/mmc/host/sdhci-dove.c
> @@ -3,7 +3,7 @@
>   *
>   * Author: Saeed Bishara <saeed-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>   *	   Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
> - * Based on sdhci-cns3xxx.c
> + * Based on sdhci-dove.c

This file *is* sdhci-dove.c.  Did you intend this change?  :-)

>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -61,10 +61,75 @@ static struct sdhci_ops sdhci_dove_ops = {
>  	.read_l	= sdhci_dove_readl,
>  };
>  
> -struct sdhci_pltfm_data sdhci_dove_pdata = {
> +static struct sdhci_pltfm_data sdhci_dove_pdata = {
>  	.ops	= &sdhci_dove_ops,
>  	.quirks	= SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
>  		  SDHCI_QUIRK_NO_BUSY_IRQ |
>  		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
>  		  SDHCI_QUIRK_FORCE_DMA,
>  };
> +
> +static int __devinit sdhci_dove_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	int ret;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_dove_pdata);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		goto err_add_host;
> +
> +	return 0;
> +
> +err_add_host:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}
> +
> +static int __devexit sdhci_dove_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	int dead = 0;
> +	u32 scratch;
> +
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
> +
> +	sdhci_remove_host(host, dead);
> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sdhci_dove_driver = {
> +	.driver		= {
> +		.name	= "sdhci-dove",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= sdhci_dove_probe,
> +	.remove		= __devexit_p(sdhci_dove_remove),
> +#ifdef CONFIG_PM
> +	.suspend	= sdhci_pltfm_suspend,
> +	.resume		= sdhci_pltfm_resume,
> +#endif
> +};
> +
> +static int __init sdhci_dove_init(void)
> +{
> +	return platform_driver_register(&sdhci_dove_driver);
> +}
> +module_init(sdhci_dove_init);
> +
> +static void __exit sdhci_dove_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_dove_driver);
> +}
> +module_exit(sdhci_dove_exit);
> +
> +MODULE_DESCRIPTION("SDHCI driver for Dove");
> +MODULE_AUTHOR("Saeed Bishara <saeed-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 9b82910..c9eb507 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -100,15 +100,39 @@ static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
>  	return clk_get_rate(pltfm_host->clk) / 256 / 16;
>  }
>  
> -static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pdata)
> +static struct sdhci_ops sdhci_esdhc_ops = {
> +	.read_w = esdhc_readw_le,
> +	.write_w = esdhc_writew_le,
> +	.write_b = esdhc_writeb_le,
> +	.set_clock = esdhc_set_clock,
> +	.get_max_clock = esdhc_pltfm_get_max_clock,
> +	.get_min_clock = esdhc_pltfm_get_min_clock,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
> +	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
> +	/* ADMA has issues. Might be fixable */
> +	.ops = &sdhci_esdhc_ops,
> +};
> +
> +static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  {
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_host *host;
>  	struct clk *clk;
> +	int ret;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_imx_pdata);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	pltfm_host = sdhci_priv(host);
>  
>  	clk = clk_get(mmc_dev(host->mmc), NULL);
>  	if (IS_ERR(clk)) {
>  		dev_err(mmc_dev(host->mmc), "clk err\n");
> -		return PTR_ERR(clk);
> +		ret = PTR_ERR(clk);
> +		goto err_clk_get;
>  	}
>  	clk_enable(clk);
>  	pltfm_host->clk = clk;
> @@ -120,30 +144,67 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
>  	if (cpu_is_mx25() || cpu_is_mx35())
>  		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
>  
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		goto err_add_host;
> +
>  	return 0;
> +
> +err_add_host:
> +	clk_disable(pltfm_host->clk);
> +	clk_put(pltfm_host->clk);
> +err_clk_get:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
>  }
>  
> -static void esdhc_pltfm_exit(struct sdhci_host *host)
> +static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
>  {
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	int dead = 0;
> +	u32 scratch;
> +
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
> +
> +	sdhci_remove_host(host, dead);
>  
>  	clk_disable(pltfm_host->clk);
>  	clk_put(pltfm_host->clk);
> +
> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
>  }
>  
> -static struct sdhci_ops sdhci_esdhc_ops = {
> -	.read_w = esdhc_readw_le,
> -	.write_w = esdhc_writew_le,
> -	.write_b = esdhc_writeb_le,
> -	.set_clock = esdhc_set_clock,
> -	.get_max_clock = esdhc_pltfm_get_max_clock,
> -	.get_min_clock = esdhc_pltfm_get_min_clock,
> +static struct platform_driver sdhci_esdhc_imx_driver = {
> +	.driver		= {
> +		.name	= "sdhci-esdhc-imx",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= sdhci_esdhc_imx_probe,
> +	.remove		= __devexit_p(sdhci_esdhc_imx_remove),
> +#ifdef CONFIG_PM
> +	.suspend	= sdhci_pltfm_suspend,
> +	.resume		= sdhci_pltfm_resume,
> +#endif
>  };
>  
> -struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
> -	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
> -	/* ADMA has issues. Might be fixable */
> -	.ops = &sdhci_esdhc_ops,
> -	.init = esdhc_pltfm_init,
> -	.exit = esdhc_pltfm_exit,
> -};
> +static int __init sdhci_esdhc_imx_init(void)
> +{
> +	return platform_driver_register(&sdhci_esdhc_imx_driver);
> +}
> +
> +static void __exit sdhci_esdhc_imx_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_esdhc_imx_driver);
> +}
> +
> +module_init(sdhci_esdhc_imx_init);
> +module_exit(sdhci_esdhc_imx_exit);

Please move the module_init() line to be directly below the
sdchi_esdhc_imx_init() implementation.

> +
> +MODULE_DESCRIPTION("SDHCI driver for i.MX eSDHC");
> +MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index ccc04ac..63e45aa 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -22,70 +22,22 @@
>   * Inspired by sdhci-pci.c, by Pierre Ossman
>   */
>  
> -#include <linux/delay.h>
> -#include <linux/highmem.h>
> -#include <linux/mod_devicetable.h>
> -#include <linux/platform_device.h>
> -
> -#include <linux/mmc/host.h>
> -
> -#include <linux/io.h>
> -#include <linux/mmc/sdhci-pltfm.h>
> +#include <linux/err.h>
>  
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
>  
> -/*****************************************************************************\
> - *                                                                           *
> - * SDHCI core callbacks                                                      *
> - *                                                                           *
> -\*****************************************************************************/
> -
>  static struct sdhci_ops sdhci_pltfm_ops = {
>  };
>  
> -/*****************************************************************************\
> - *                                                                           *
> - * Device probing/removal                                                    *
> - *                                                                           *
> -\*****************************************************************************/
> -#if defined(CONFIG_OF)
> -#include <linux/of_device.h>
> -static const struct of_device_id sdhci_dt_ids[] = {
> -	{ .compatible = "nvidia,tegra250-sdhci", .data = &sdhci_tegra_dt_pdata },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(platform, sdhci_dt_ids);
> -
> -static const struct of_device_id *sdhci_get_of_device_id(struct platform_device *pdev)
> -{
> -	return of_match_device(sdhci_dt_ids, &pdev->dev);
> -}
> -#else
> -#define shdci_dt_ids NULL
> -static inline struct of_device_id *sdhci_get_of_device_id(struct platform_device *pdev)
> -{
> -	return NULL;
> -}
> -#endif
> -
> -static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
> +struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
> +				    struct sdhci_pltfm_data *pdata)
>  {
> -	const struct platform_device_id *platid = platform_get_device_id(pdev);
> -	const struct of_device_id *dtid = sdhci_get_of_device_id(pdev);
> -	struct sdhci_pltfm_data *pdata;
>  	struct sdhci_host *host;
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct resource *iomem;
>  	int ret;
>  
> -	if (platid && platid->driver_data)
> -		pdata = (void *)platid->driver_data;
> -	else if (dtid && dtid->data)
> -		pdata = dtid->data;
> -	else
> -		pdata = pdev->dev.platform_data;
> -
>  	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!iomem) {
>  		ret = -ENOMEM;
> @@ -93,8 +45,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  	}
>  
>  	if (resource_size(iomem) < 0x100)
> -		dev_err(&pdev->dev, "Invalid iomem size. You may "
> -			"experience problems.\n");
> +		dev_err(&pdev->dev, "Invalid iomem size!\n");
>  
>  	/* Some PCI-based MFD need the parent here */
>  	if (pdev->dev.parent != &platform_bus)
> @@ -109,7 +60,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  
>  	pltfm_host = sdhci_priv(host);
>  
> -	host->hw_name = "platform";
> +	host->hw_name = dev_name(&pdev->dev);
>  	if (pdata && pdata->ops)
>  		host->ops = pdata->ops;
>  	else
> @@ -132,136 +83,42 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  		goto err_remap;
>  	}
>  
> -	if (pdata && pdata->init) {
> -		ret = pdata->init(host, pdata);
> -		if (ret)
> -			goto err_plat_init;
> -	}
> -
> -	ret = sdhci_add_host(host);
> -	if (ret)
> -		goto err_add_host;
> -
>  	platform_set_drvdata(pdev, host);
>  
> -	return 0;
> +	return host;
>  
> -err_add_host:
> -	if (pdata && pdata->exit)
> -		pdata->exit(host);
> -err_plat_init:
> -	iounmap(host->ioaddr);
>  err_remap:
>  	release_mem_region(iomem->start, resource_size(iomem));
>  err_request:
>  	sdhci_free_host(host);
>  err:
> -	printk(KERN_ERR"Probing of sdhci-pltfm failed: %d\n", ret);
> -	return ret;
> +	pr_err("%s failed %d\n", __func__, ret);
> +	return NULL;
>  }
>  
> -static int __devexit sdhci_pltfm_remove(struct platform_device *pdev)
> +void sdhci_pltfm_free(struct platform_device *pdev)
>  {
> -	const struct platform_device_id *platid = platform_get_device_id(pdev);
> -	const struct of_device_id *dtid = sdhci_get_of_device_id(pdev);
> -	struct sdhci_pltfm_data *pdata;
>  	struct sdhci_host *host = platform_get_drvdata(pdev);
>  	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	int dead;
> -	u32 scratch;
>  
> -	if (platid && platid->driver_data)
> -		pdata = (void *)platid->driver_data;
> -	else if (dtid && dtid->data)
> -		pdata = dtid->data;
> -	else
> -		pdata = pdev->dev.platform_data;
> -
> -	dead = 0;
> -	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> -	if (scratch == (u32)-1)
> -		dead = 1;
> -
> -	sdhci_remove_host(host, dead);
> -	if (pdata && pdata->exit)
> -		pdata->exit(host);
>  	iounmap(host->ioaddr);
>  	release_mem_region(iomem->start, resource_size(iomem));
>  	sdhci_free_host(host);
>  	platform_set_drvdata(pdev, NULL);
> -
> -	return 0;
>  }
>  
> -static const struct platform_device_id sdhci_pltfm_ids[] = {
> -	{ "sdhci", },
> -#ifdef CONFIG_MMC_SDHCI_CNS3XXX
> -	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
> -#endif
> -#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
> -	{ "sdhci-esdhc-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata },
> -#endif
> -#ifdef CONFIG_MMC_SDHCI_DOVE
> -	{ "sdhci-dove", (kernel_ulong_t)&sdhci_dove_pdata },
> -#endif
> -#ifdef CONFIG_MMC_SDHCI_TEGRA
> -	{ "sdhci-tegra", (kernel_ulong_t)&sdhci_tegra_pdata },
> -#endif
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
> -
>  #ifdef CONFIG_PM
> -static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state)
> +int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state)
>  {
>  	struct sdhci_host *host = platform_get_drvdata(dev);
>  
>  	return sdhci_suspend_host(host, state);
>  }
>  
> -static int sdhci_pltfm_resume(struct platform_device *dev)
> +int sdhci_pltfm_resume(struct platform_device *dev)
>  {
>  	struct sdhci_host *host = platform_get_drvdata(dev);
>  
>  	return sdhci_resume_host(host);
>  }
> -#else
> -#define sdhci_pltfm_suspend	NULL
> -#define sdhci_pltfm_resume	NULL
>  #endif	/* CONFIG_PM */
> -
> -static struct platform_driver sdhci_pltfm_driver = {
> -	.driver = {
> -		.name	= "sdhci",
> -		.owner	= THIS_MODULE,
> -		.of_match_table = sdhci_dt_ids,
> -	},
> -	.probe		= sdhci_pltfm_probe,
> -	.remove		= __devexit_p(sdhci_pltfm_remove),
> -	.id_table	= sdhci_pltfm_ids,
> -	.suspend	= sdhci_pltfm_suspend,
> -	.resume		= sdhci_pltfm_resume,
> -};
> -
> -/*****************************************************************************\
> - *                                                                           *
> - * Driver init/exit                                                          *
> - *                                                                           *
> -\*****************************************************************************/
> -
> -static int __init sdhci_drv_init(void)
> -{
> -	return platform_driver_register(&sdhci_pltfm_driver);
> -}
> -
> -static void __exit sdhci_drv_exit(void)
> -{
> -	platform_driver_unregister(&sdhci_pltfm_driver);
> -}
> -
> -module_init(sdhci_drv_init);
> -module_exit(sdhci_drv_exit);
> -
> -MODULE_DESCRIPTION("Secure Digital Host Controller Interface platform driver");
> -MODULE_AUTHOR("Mocean Laboratories <info-l7gf1WXxx3uGw+nKnLezzg@public.gmane.org>");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index c67523d..a3e4be0 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -13,6 +13,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/types.h>
> +#include <linux/platform_device.h>
>  #include <linux/mmc/sdhci-pltfm.h>
>  
>  struct sdhci_pltfm_host {
> @@ -20,10 +21,13 @@ struct sdhci_pltfm_host {
>  	u32 scratchpad; /* to handle quirks across io-accessor calls */
>  };
>  
> -extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
> -extern struct sdhci_pltfm_data sdhci_esdhc_imx_pdata;
> -extern struct sdhci_pltfm_data sdhci_dove_pdata;
> -extern struct sdhci_pltfm_data sdhci_tegra_pdata;
> -extern struct sdhci_pltfm_data sdhci_tegra_dt_pdata;
> +extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
> +					   struct sdhci_pltfm_data *pdata);
> +extern void sdhci_pltfm_free(struct platform_device *pdev);
> +
> +#ifdef CONFIG_PM
> +extern int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t state);
> +extern int sdhci_pltfm_resume(struct platform_device *dev);
> +#endif
>  
>  #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index c3d6f83..cfcd521 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -119,19 +119,58 @@ static int tegra_sdhci_8bit(struct sdhci_host *host, int bus_width)
>  }
>  
>  
> -static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
> -				  struct sdhci_pltfm_data *pdata)
> +static struct sdhci_ops tegra_sdhci_ops = {
> +	.get_ro     = tegra_sdhci_get_ro,
> +	.read_l     = tegra_sdhci_readl,
> +	.read_w     = tegra_sdhci_readw,
> +	.write_l    = tegra_sdhci_writel,
> +	.platform_8bit_width = tegra_sdhci_8bit,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_tegra_pdata = {
> +	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> +		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
> +		  SDHCI_QUIRK_NO_HISPD_BIT |
> +		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
> +	.ops  = &tegra_sdhci_ops,
> +};
> +
> +static int __devinit sdhci_tegra_probe(struct platform_device *pdev)
>  {
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> +	struct sdhci_pltfm_host *pltfm_host;
>  	struct tegra_sdhci_platform_data *plat;
> +	struct sdhci_host *host;
>  	struct clk *clk;
>  	int rc;
>  
> +	host = sdhci_pltfm_init(pdev, &sdhci_tegra_pdata);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	pltfm_host = sdhci_priv(host);
> +
>  	plat = pdev->dev.platform_data;
> +
> +#ifdef CONFIG_OF
> +	plat = kzalloc(sizeof(*plat), GFP_KERNEL);
> +	if (!plat) {
> +		rc = -ENOMEM;
> +		goto err_no_plat;
> +	}
> +	pdev->dev.platform_data = plat;
> +
> +	plat->cd_gpio = of_get_gpio(pdev->dev.of_node, 0);
> +	plat->wp_gpio = of_get_gpio(pdev->dev.of_node, 1);
> +	plat->power_gpio = of_get_gpio(pdev->dev.of_node, 2);
> +
> +	dev_info(&pdev->dev, "using gpios cd=%i, wp=%i power=%i\n",
> +		plat->cd_gpio, plat->wp_gpio, plat->power_gpio);
> +#endif

This will need to be reworked for mainline since the Tegra device tree
support only exists in my git tree.  Basing it on devicetree/arm would
work.

> +
>  	if (plat == NULL) {
>  		dev_err(mmc_dev(host->mmc), "missing platform data\n");
> -		return -ENXIO;
> +		rc = -ENXIO;
> +		goto err_no_plat;
>  	}
>  
>  	if (gpio_is_valid(plat->power_gpio)) {
> @@ -139,7 +178,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
>  		if (rc) {
>  			dev_err(mmc_dev(host->mmc),
>  				"failed to allocate power gpio\n");
> -			goto out;
> +			goto err_power_req;
>  		}
>  		tegra_gpio_enable(plat->power_gpio);
>  		gpio_direction_output(plat->power_gpio, 1);
> @@ -150,7 +189,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
>  		if (rc) {
>  			dev_err(mmc_dev(host->mmc),
>  				"failed to allocate cd gpio\n");
> -			goto out_power;
> +			goto err_cd_req;
>  		}
>  		tegra_gpio_enable(plat->cd_gpio);
>  		gpio_direction_input(plat->cd_gpio);
> @@ -161,7 +200,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
>  
>  		if (rc)	{
>  			dev_err(mmc_dev(host->mmc), "request irq error\n");
> -			goto out_cd;
> +			goto err_cd_irq_req;
>  		}
>  
>  	}
> @@ -171,7 +210,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
>  		if (rc) {
>  			dev_err(mmc_dev(host->mmc),
>  				"failed to allocate wp gpio\n");
> -			goto out_cd;
> +			goto err_wp_req;
>  		}
>  		tegra_gpio_enable(plat->wp_gpio);
>  		gpio_direction_input(plat->wp_gpio);
> @@ -181,7 +220,7 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
>  	if (IS_ERR(clk)) {
>  		dev_err(mmc_dev(host->mmc), "clk err\n");
>  		rc = PTR_ERR(clk);
> -		goto out_wp;
> +		goto err_clk_get;
>  	}
>  	clk_enable(clk);
>  	pltfm_host->clk = clk;
> @@ -189,62 +228,56 @@ static int tegra_sdhci_pltfm_init(struct sdhci_host *host,
>  	if (plat->is_8bit)
>  		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>  
> +	rc = sdhci_add_host(host);
> +	if (rc)
> +		goto err_add_host;
> +
>  	return 0;
>  
> -out_wp:
> +err_add_host:
> +	clk_disable(pltfm_host->clk);
> +	clk_put(pltfm_host->clk);
> +err_clk_get:
>  	if (gpio_is_valid(plat->wp_gpio)) {
>  		tegra_gpio_disable(plat->wp_gpio);
>  		gpio_free(plat->wp_gpio);
>  	}
> -
> -out_cd:
> +err_wp_req:
> +	if (gpio_is_valid(plat->cd_gpio)) {
> +		free_irq(gpio_to_irq(plat->cd_gpio), host);
> +	}
> +err_cd_irq_req:
>  	if (gpio_is_valid(plat->cd_gpio)) {
>  		tegra_gpio_disable(plat->cd_gpio);
>  		gpio_free(plat->cd_gpio);
>  	}
> -
> -out_power:
> +err_cd_req:
>  	if (gpio_is_valid(plat->power_gpio)) {
>  		tegra_gpio_disable(plat->power_gpio);
>  		gpio_free(plat->power_gpio);
>  	}
> -
> -out:
> +err_power_req:
> +#ifdef CONFIG_OF
> +	kfree(plat);
> +#endif
> +err_no_plat:
> +	sdhci_pltfm_free(pdev);
>  	return rc;
>  }
>  
> -static int tegra_sdhci_pltfm_dt_init(struct sdhci_host *host,
> -				     struct sdhci_pltfm_data *pdata)
> +static int __devexit sdhci_tegra_remove(struct platform_device *pdev)
>  {
> -	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct tegra_sdhci_platform_data *plat;
> +	int dead = 0;
> +	u32 scratch;
>  
> -	if (pdev->dev.platform_data) {
> -		dev_err(&pdev->dev, "%s: platform_data not NULL; aborting\n",
> -			__func__);
> -		return -ENODEV;
> -	}
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
>  
> -	plat = kzalloc(sizeof(*plat), GFP_KERNEL);
> -	if (!plat)
> -		return -ENOMEM;
> -	pdev->dev.platform_data = plat;
> -
> -	plat->cd_gpio = of_get_gpio(pdev->dev.of_node, 0);
> -	plat->wp_gpio = of_get_gpio(pdev->dev.of_node, 1);
> -	plat->power_gpio = of_get_gpio(pdev->dev.of_node, 2);
> -
> -	dev_info(&pdev->dev, "using gpios cd=%i, wp=%i power=%i\n",
> -		plat->cd_gpio, plat->wp_gpio, plat->power_gpio);
> -
> -	return tegra_sdhci_pltfm_init(host, pdata);
> -}
> -
> -static void tegra_sdhci_pltfm_exit(struct sdhci_host *host)
> -{
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> -	struct tegra_sdhci_platform_data *plat;
> +	sdhci_remove_host(host, dead);
>  
>  	plat = pdev->dev.platform_data;
>  
> @@ -263,44 +296,44 @@ static void tegra_sdhci_pltfm_exit(struct sdhci_host *host)
>  		gpio_free(plat->power_gpio);
>  	}
>  
> +#ifdef CONFIG_OF
> +	kfree(pdev->dev.platform_data);
> +	pdev->dev.platform_data = NULL;
> +#endif
> +
>  	clk_disable(pltfm_host->clk);
>  	clk_put(pltfm_host->clk);
> -}
>  
> -static void tegra_sdhci_pltfm_dt_exit(struct sdhci_host *host)
> -{
> -	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> -
> -	tegra_sdhci_pltfm_exit(host);
> +	sdhci_pltfm_free(pdev);
>  
> -	kfree(pdev->dev.platform_data);
> -	pdev->dev.platform_data = NULL;
> +	return 0;
>  }
>  
> -static struct sdhci_ops tegra_sdhci_ops = {
> -	.get_ro     = tegra_sdhci_get_ro,
> -	.read_l     = tegra_sdhci_readl,
> -	.read_w     = tegra_sdhci_readw,
> -	.write_l    = tegra_sdhci_writel,
> -	.platform_8bit_width = tegra_sdhci_8bit,
> +static struct platform_driver sdhci_tegra_driver = {
> +	.driver		= {
> +		.name	= "sdhci-tegra",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= sdhci_tegra_probe,
> +	.remove		= __devexit_p(sdhci_tegra_remove),
> +#ifdef CONFIG_PM
> +	.suspend	= sdhci_pltfm_suspend,
> +	.resume		= sdhci_pltfm_resume,
> +#endif
>  };
>  
> -struct sdhci_pltfm_data sdhci_tegra_pdata = {
> -	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> -		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
> -		  SDHCI_QUIRK_NO_HISPD_BIT |
> -		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
> -	.ops  = &tegra_sdhci_ops,
> -	.init = tegra_sdhci_pltfm_init,
> -	.exit = tegra_sdhci_pltfm_exit,
> -};
> +static int __init sdhci_tegra_init(void)
> +{
> +	return platform_driver_register(&sdhci_tegra_driver);
> +}
> +module_init(sdhci_tegra_init);
>  
> -struct sdhci_pltfm_data sdhci_tegra_dt_pdata = {
> -	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> -		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
> -		  SDHCI_QUIRK_NO_HISPD_BIT |
> -		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
> -	.ops  = &tegra_sdhci_ops,
> -	.init = tegra_sdhci_pltfm_dt_init,
> -	.exit = tegra_sdhci_pltfm_dt_exit,
> -};
> +static void __exit sdhci_tegra_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_tegra_driver);
> +}
> +module_exit(sdhci_tegra_exit);
> +
> +MODULE_DESCRIPTION("SDHCI driver for Tegra");
> +MODULE_AUTHOR(" Google, Inc.");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
> index 548d59d..f1c2ac3 100644
> --- a/include/linux/mmc/sdhci-pltfm.h
> +++ b/include/linux/mmc/sdhci-pltfm.h
> @@ -15,21 +15,15 @@
>  #define _SDHCI_PLTFM_H
>  
>  struct sdhci_ops;
> -struct sdhci_host;
>  
>  /**
>   * struct sdhci_pltfm_data - SDHCI platform-specific information & hooks
>   * @ops: optional pointer to the platform-provided SDHCI ops
>   * @quirks: optional SDHCI quirks
> - * @init: optional hook that is called during device probe, before the
> - *        driver tries to access any SDHCI registers
> - * @exit: optional hook that is called during device removal
>   */
>  struct sdhci_pltfm_data {
>  	struct sdhci_ops *ops;
>  	unsigned int quirks;
> -	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata);
> -	void (*exit)(struct sdhci_host *host);
>  };
>  
>  #endif /* _SDHCI_PLTFM_H */
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered

[Shawn Guo]

On Thu, Mar 31, 2011 at 09:33:22AM -0600, Grant Likely wrote:
> On Fri, Mar 25, 2011 at 04:48:47PM +0800, Shawn Guo wrote:
> > The patch turns the common stuff in sdhci-pltfm.c into functions, and
> > add device drivers their own .probe and .remove which in turn call
> > into the common functions, so that those sdhci-pltfm device drivers
> > register itself and keep all device specific things away from common
> > sdhci-pltfm file.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Looks really good.  Relatively minor comments below, but you can add
> this to the next version:
> 
> Reviewed-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
Thanks for the review.

[...]
> > +
> > +static int __devinit sdhci_cns3xxx_probe(struct platform_device *pdev)
> > +{
> > +	struct sdhci_host *host;
> > +	int ret;
> > +
> > +	host = sdhci_pltfm_init(pdev, &sdhci_cns3xxx_pdata);
> > +	if (!host)
> > +		return -ENOMEM;
> > +
> > +	ret = sdhci_add_host(host);
> > +	if (ret)
> > +		goto err_add_host;
> > +
> > +	return 0;
> > +
> > +err_add_host:
> > +	sdhci_pltfm_free(pdev);
> > +	return ret;
> > +}
> 
> This pattern in this function is used by 2 drivers in this patch, and
> 2 more users (with the addition of an sdhci_get_of_property() call) in
> the 3rd patch.  An sdchi_pltfm_register(pdev, &pdata) that does the
> whole sequence would probably be valuable.  Same for the _remove hook.
> 
OK, one more pair of helper function will be added.

> Drivers that still need 2 stage registration, like the tegra driver,
> would still be able to call sdhci_pltfm_init() and sdhci_add_host()
> directly.
> 
> > +
> > +static int __devexit sdhci_cns3xxx_remove(struct platform_device *pdev)
> > +{
> > +	struct sdhci_host *host = platform_get_drvdata(pdev);
> > +	int dead = 0;
> > +	u32 scratch;
> > +
> > +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> > +	if (scratch == (u32)-1)
> > +		dead = 1;
> > +
> > +	sdhci_remove_host(host, dead);
> 
> The following would be equivalent to the above three lines:
> 
> 	sdhci_remove_host(host, scratch == (u32)-1);
> 
OK.

[...]
> > @@ -3,7 +3,7 @@
> >   *
> >   * Author: Saeed Bishara <saeed-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> >   *	   Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
> > - * Based on sdhci-cns3xxx.c
> > + * Based on sdhci-dove.c
> 
> This file *is* sdhci-dove.c.  Did you intend this change?  :-)
> 
My bad.

[...]
> > +#ifdef CONFIG_OF
> > +	plat = kzalloc(sizeof(*plat), GFP_KERNEL);
> > +	if (!plat) {
> > +		rc = -ENOMEM;
> > +		goto err_no_plat;
> > +	}
> > +	pdev->dev.platform_data = plat;
> > +
> > +	plat->cd_gpio = of_get_gpio(pdev->dev.of_node, 0);
> > +	plat->wp_gpio = of_get_gpio(pdev->dev.of_node, 1);
> > +	plat->power_gpio = of_get_gpio(pdev->dev.of_node, 2);
> > +
> > +	dev_info(&pdev->dev, "using gpios cd=%i, wp=%i power=%i\n",
> > +		plat->cd_gpio, plat->wp_gpio, plat->power_gpio);
> > +#endif
> 
> This will need to be reworked for mainline since the Tegra device tree
> support only exists in my git tree.  Basing it on devicetree/arm would
> work.
> 
OK. Will against mmc-next and test esdhc dt changes on devicetree/arm.

-- 
Regards,
Shawn

Re: [PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 1916 bytes --]


On Fri, Mar 25, 2011 at 04:48:47PM +0800, Shawn Guo wrote:
> The patch turns the common stuff in sdhci-pltfm.c into functions, and
> add device drivers their own .probe and .remove which in turn call
> into the common functions, so that those sdhci-pltfm device drivers
> register itself and keep all device specific things away from common
> sdhci-pltfm file.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

I'll second the comments from Grant (with one slight exception which is
noted below)

> +static int __devexit sdhci_dove_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	int dead = 0;
> +	u32 scratch;
> +
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;

I'd prefer

	dead = (readl() == 0xffffffff);

(or (u32)-1 if you prefer). But keeping a variable 'dead' is more
descriptive than keeping 'scratch'.

> +MODULE_LICENSE("GPL v2");

Just to be sure: Did you double-check if the original licenses were v2
or v2+?

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

[...]

> -err_add_host:
> -	if (pdata && pdata->exit)
> -		pdata->exit(host);
> -err_plat_init:
> -	iounmap(host->ioaddr);
>  err_remap:
>  	release_mem_region(iomem->start, resource_size(iomem));
>  err_request:
>  	sdhci_free_host(host);
>  err:
> -	printk(KERN_ERR"Probing of sdhci-pltfm failed: %d\n", ret);
> -	return ret;
> +	pr_err("%s failed %d\n", __func__, ret);

dev_err?

> +	return NULL;
>  }
>  

I didn't pay much attention to the OF version of the tegra driver, since
it still is not upstream yet, right?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 175 bytes --]

_______________________________________________
linaro-dev mailing list
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered

[Shawn Guo]

On Tue, Apr 19, 2011 at 12:20:42PM +0200, Wolfram Sang wrote:
> 
> On Fri, Mar 25, 2011 at 04:48:47PM +0800, Shawn Guo wrote:
> > The patch turns the common stuff in sdhci-pltfm.c into functions, and
> > add device drivers their own .probe and .remove which in turn call
> > into the common functions, so that those sdhci-pltfm device drivers
> > register itself and keep all device specific things away from common
> > sdhci-pltfm file.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> 
> I'll second the comments from Grant (with one slight exception which is
> noted below)
> 
> > +static int __devexit sdhci_dove_remove(struct platform_device *pdev)
> > +{
> > +	struct sdhci_host *host = platform_get_drvdata(pdev);
> > +	int dead = 0;
> > +	u32 scratch;
> > +
> > +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> > +	if (scratch == (u32)-1)
> > +		dead = 1;
> 
> I'd prefer
> 
> 	dead = (readl() == 0xffffffff);
> 
> (or (u32)-1 if you prefer). But keeping a variable 'dead' is more
> descriptive than keeping 'scratch'.
> 
Ok.

> > +MODULE_LICENSE("GPL v2");
> 
> Just to be sure: Did you double-check if the original licenses were v2
> or v2+?
> 
It seems to me that sdhci-cns3xxx.c, sdhci-dove.c, sdhci-esdhc-imx.c
and sdhci-tegra.c all use v2.

Actually I do not even know how v2+ is stated.  Do you have an example
for me to refer?

> > --- a/drivers/mmc/host/sdhci-pltfm.c
> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> 
> [...]
> 
> > -err_add_host:
> > -	if (pdata && pdata->exit)
> > -		pdata->exit(host);
> > -err_plat_init:
> > -	iounmap(host->ioaddr);
> >  err_remap:
> >  	release_mem_region(iomem->start, resource_size(iomem));
> >  err_request:
> >  	sdhci_free_host(host);
> >  err:
> > -	printk(KERN_ERR"Probing of sdhci-pltfm failed: %d\n", ret);
> > -	return ret;
> > +	pr_err("%s failed %d\n", __func__, ret);
> 
> dev_err?
> 
Good catch.

> > +	return NULL;
> >  }
> >  
> 
> I didn't pay much attention to the OF version of the tegra driver, since
> it still is not upstream yet, right?
> 
Do not worry about that.  You will not see that in the next version,
which will be based on mmc-next.  And tegra OF support should be in
another patch.

-- 
Regards,
Shawn

Re: [PATCH 1/5] mmc: sdhci: make sdhci-pltfm device drivers self registered

[Wolfram Sang]

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

> > > +MODULE_LICENSE("GPL v2");
> > 
> > Just to be sure: Did you double-check if the original licenses were v2
> > or v2+?
> > 
> It seems to me that sdhci-cns3xxx.c, sdhci-dove.c, sdhci-esdhc-imx.c
> and sdhci-tegra.c all use v2.
> 
> Actually I do not even know how v2+ is stated.  Do you have an example
> for me to refer?

Check davinci_mmc.c (or grep for 'any later version').

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[PATCH 2/5] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data

[Shawn Guo]

The patch is to migrate the use of sdhci_of_host and sdhci_of_data
to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can
be eliminated.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/host/sdhci-of-core.c  |   30 +++++++++++++++---------------
 drivers/mmc/host/sdhci-of-esdhc.c |   36 +++++++++++++++++++-----------------
 drivers/mmc/host/sdhci-of-hlwd.c  |   20 +++++++++++---------
 drivers/mmc/host/sdhci-of.h       |   15 +++------------
 drivers/mmc/host/sdhci-pltfm.h    |    4 ++++
 5 files changed, 52 insertions(+), 53 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index dd84124..a6c0132 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -59,7 +59,7 @@ void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
 
 void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
 {
-	struct sdhci_of_host *of_host = sdhci_priv(host);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	int base = reg & ~0x3;
 	int shift = (reg & 0x2) * 8;
 
@@ -69,10 +69,10 @@ void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
 		 * Postpone this write, we must do it together with a
 		 * command write that is down below.
 		 */
-		of_host->xfer_mode_shadow = val;
+		pltfm_host->xfer_mode_shadow = val;
 		return;
 	case SDHCI_COMMAND:
-		sdhci_be32bs_writel(host, val << 16 | of_host->xfer_mode_shadow,
+		sdhci_be32bs_writel(host, val << 16 | pltfm_host->xfer_mode_shadow,
 				    SDHCI_TRANSFER_MODE);
 		return;
 	}
@@ -128,9 +128,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
 				 const struct of_device_id *match)
 {
 	struct device_node *np = ofdev->dev.of_node;
-	struct sdhci_of_data *sdhci_of_data = match->data;
+	struct sdhci_pltfm_data *pdata = match->data;
 	struct sdhci_host *host;
-	struct sdhci_of_host *of_host;
+	struct sdhci_pltfm_host *pltfm_host;
 	const __be32 *clk;
 	int size;
 	int ret;
@@ -138,11 +138,11 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
 	if (!of_device_is_available(np))
 		return -ENODEV;
 
-	host = sdhci_alloc_host(&ofdev->dev, sizeof(*of_host));
+	host = sdhci_alloc_host(&ofdev->dev, sizeof(*pltfm_host));
 	if (IS_ERR(host))
 		return -ENOMEM;
 
-	of_host = sdhci_priv(host);
+	pltfm_host = sdhci_priv(host);
 	dev_set_drvdata(&ofdev->dev, host);
 
 	host->ioaddr = of_iomap(np, 0);
@@ -158,9 +158,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
 	}
 
 	host->hw_name = dev_name(&ofdev->dev);
-	if (sdhci_of_data) {
-		host->quirks = sdhci_of_data->quirks;
-		host->ops = &sdhci_of_data->ops;
+	if (pdata) {
+		host->quirks = pdata->quirks;
+		host->ops = &pdata->ops;
 	}
 
 	if (of_get_property(np, "sdhci,auto-cmd12", NULL))
@@ -175,7 +175,7 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
 
 	clk = of_get_property(np, "clock-frequency", &size);
 	if (clk && size == sizeof(*clk) && *clk)
-		of_host->clock = be32_to_cpup(clk);
+		pltfm_host->clock = be32_to_cpup(clk);
 
 	ret = sdhci_add_host(host);
 	if (ret)
@@ -205,12 +205,12 @@ static int __devexit sdhci_of_remove(struct platform_device *ofdev)
 
 static const struct of_device_id sdhci_of_match[] = {
 #ifdef CONFIG_MMC_SDHCI_OF_ESDHC
-	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc, },
-	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc, },
-	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc, },
+	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_pdata, },
+	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_pdata, },
+	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_pdata, },
 #endif
 #ifdef CONFIG_MMC_SDHCI_OF_HLWD
-	{ .compatible = "nintendo,hollywood-sdhci", .data = &sdhci_hlwd, },
+	{ .compatible = "nintendo,hollywood-sdhci", .data = &sdhci_hlwd_pdata, },
 #endif
 	{ .compatible = "generic-sdhci", },
 	{},
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fcd0e1f..702a98b 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -60,30 +60,32 @@ static int esdhc_of_enable_dma(struct sdhci_host *host)
 
 static unsigned int esdhc_of_get_max_clock(struct sdhci_host *host)
 {
-	struct sdhci_of_host *of_host = sdhci_priv(host);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 
-	return of_host->clock;
+	return pltfm_host->clock;
 }
 
 static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 {
-	struct sdhci_of_host *of_host = sdhci_priv(host);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 
-	return of_host->clock / 256 / 16;
+	return pltfm_host->clock / 256 / 16;
 }
 
-struct sdhci_of_data sdhci_esdhc = {
+static struct sdhci_ops sdhci_esdhc_ops = {
+	.read_l = sdhci_be32bs_readl,
+	.read_w = esdhc_readw,
+	.read_b = sdhci_be32bs_readb,
+	.write_l = sdhci_be32bs_writel,
+	.write_w = esdhc_writew,
+	.write_b = esdhc_writeb,
+	.set_clock = esdhc_set_clock,
+	.enable_dma = esdhc_of_enable_dma,
+	.get_max_clock = esdhc_of_get_max_clock,
+	.get_min_clock = esdhc_of_get_min_clock,
+};
+
+struct sdhci_pltfm_data sdhci_esdhc_pdata = {
 	.quirks = ESDHC_DEFAULT_QUIRKS,
-	.ops = {
-		.read_l = sdhci_be32bs_readl,
-		.read_w = esdhc_readw,
-		.read_b = sdhci_be32bs_readb,
-		.write_l = sdhci_be32bs_writel,
-		.write_w = esdhc_writew,
-		.write_b = esdhc_writeb,
-		.set_clock = esdhc_set_clock,
-		.enable_dma = esdhc_of_enable_dma,
-		.get_max_clock = esdhc_of_get_max_clock,
-		.get_min_clock = esdhc_of_get_min_clock,
-	},
+	.ops = &sdhci_esdhc_ops,
 };
diff --git a/drivers/mmc/host/sdhci-of-hlwd.c b/drivers/mmc/host/sdhci-of-hlwd.c
index 68ddb75..380e896 100644
--- a/drivers/mmc/host/sdhci-of-hlwd.c
+++ b/drivers/mmc/host/sdhci-of-hlwd.c
@@ -51,15 +51,17 @@ static void sdhci_hlwd_writeb(struct sdhci_host *host, u8 val, int reg)
 	udelay(SDHCI_HLWD_WRITE_DELAY);
 }
 
-struct sdhci_of_data sdhci_hlwd = {
+static struct sdhci_ops sdhci_hlwd_ops = {
+	.read_l = sdhci_be32bs_readl,
+	.read_w = sdhci_be32bs_readw,
+	.read_b = sdhci_be32bs_readb,
+	.write_l = sdhci_hlwd_writel,
+	.write_w = sdhci_hlwd_writew,
+	.write_b = sdhci_hlwd_writeb,
+};
+
+struct sdhci_pltfm_data sdhci_hlwd_pdata = {
 	.quirks = SDHCI_QUIRK_32BIT_DMA_ADDR |
 		  SDHCI_QUIRK_32BIT_DMA_SIZE,
-	.ops = {
-		.read_l = sdhci_be32bs_readl,
-		.read_w = sdhci_be32bs_readw,
-		.read_b = sdhci_be32bs_readb,
-		.write_l = sdhci_hlwd_writel,
-		.write_w = sdhci_hlwd_writew,
-		.write_b = sdhci_hlwd_writeb,
-	},
+	.ops = &sdhci_hlwd_ops,
 };
diff --git a/drivers/mmc/host/sdhci-of.h b/drivers/mmc/host/sdhci-of.h
index ad09ad9..5bdb5e7 100644
--- a/drivers/mmc/host/sdhci-of.h
+++ b/drivers/mmc/host/sdhci-of.h
@@ -18,16 +18,7 @@
 
 #include <linux/types.h>
 #include "sdhci.h"
-
-struct sdhci_of_data {
-	unsigned int quirks;
-	struct sdhci_ops ops;
-};
-
-struct sdhci_of_host {
-	unsigned int clock;
-	u16 xfer_mode_shadow;
-};
+#include "sdhci-pltfm.h"
 
 extern u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg);
 extern u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg);
@@ -36,7 +27,7 @@ extern void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg);
 extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
 extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
 
-extern struct sdhci_of_data sdhci_esdhc;
-extern struct sdhci_of_data sdhci_hlwd;
+extern struct sdhci_pltfm_data sdhci_esdhc_pdata;
+extern struct sdhci_pltfm_data sdhci_hlwd_pdata;
 
 #endif /* __SDHCI_OF_H */
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index a3e4be0..12afe86 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -19,6 +19,10 @@
 struct sdhci_pltfm_host {
 	struct clk *clk;
 	u32 scratchpad; /* to handle quirks across io-accessor calls */
+
+	/* migrate from sdhci_of_host */
+	unsigned int clock;
+	u16 xfer_mode_shadow;
 };
 
 extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
-- 
1.7.1

Re: [PATCH 2/5] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data

[Grant Likely]

On Fri, Mar 25, 2011 at 04:48:48PM +0800, Shawn Guo wrote:
> The patch is to migrate the use of sdhci_of_host and sdhci_of_data
> to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can
> be eliminated.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Reviewed-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

> ---
>  drivers/mmc/host/sdhci-of-core.c  |   30 +++++++++++++++---------------
>  drivers/mmc/host/sdhci-of-esdhc.c |   36 +++++++++++++++++++-----------------
>  drivers/mmc/host/sdhci-of-hlwd.c  |   20 +++++++++++---------
>  drivers/mmc/host/sdhci-of.h       |   15 +++------------
>  drivers/mmc/host/sdhci-pltfm.h    |    4 ++++
>  5 files changed, 52 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> index dd84124..a6c0132 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -59,7 +59,7 @@ void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
>  
>  void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
>  {
> -	struct sdhci_of_host *of_host = sdhci_priv(host);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	int base = reg & ~0x3;
>  	int shift = (reg & 0x2) * 8;
>  
> @@ -69,10 +69,10 @@ void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
>  		 * Postpone this write, we must do it together with a
>  		 * command write that is down below.
>  		 */
> -		of_host->xfer_mode_shadow = val;
> +		pltfm_host->xfer_mode_shadow = val;
>  		return;
>  	case SDHCI_COMMAND:
> -		sdhci_be32bs_writel(host, val << 16 | of_host->xfer_mode_shadow,
> +		sdhci_be32bs_writel(host, val << 16 | pltfm_host->xfer_mode_shadow,
>  				    SDHCI_TRANSFER_MODE);
>  		return;
>  	}
> @@ -128,9 +128,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
>  				 const struct of_device_id *match)
>  {
>  	struct device_node *np = ofdev->dev.of_node;
> -	struct sdhci_of_data *sdhci_of_data = match->data;
> +	struct sdhci_pltfm_data *pdata = match->data;
>  	struct sdhci_host *host;
> -	struct sdhci_of_host *of_host;
> +	struct sdhci_pltfm_host *pltfm_host;
>  	const __be32 *clk;
>  	int size;
>  	int ret;
> @@ -138,11 +138,11 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
>  	if (!of_device_is_available(np))
>  		return -ENODEV;
>  
> -	host = sdhci_alloc_host(&ofdev->dev, sizeof(*of_host));
> +	host = sdhci_alloc_host(&ofdev->dev, sizeof(*pltfm_host));
>  	if (IS_ERR(host))
>  		return -ENOMEM;
>  
> -	of_host = sdhci_priv(host);
> +	pltfm_host = sdhci_priv(host);
>  	dev_set_drvdata(&ofdev->dev, host);
>  
>  	host->ioaddr = of_iomap(np, 0);
> @@ -158,9 +158,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
>  	}
>  
>  	host->hw_name = dev_name(&ofdev->dev);
> -	if (sdhci_of_data) {
> -		host->quirks = sdhci_of_data->quirks;
> -		host->ops = &sdhci_of_data->ops;
> +	if (pdata) {
> +		host->quirks = pdata->quirks;
> +		host->ops = &pdata->ops;
>  	}
>  
>  	if (of_get_property(np, "sdhci,auto-cmd12", NULL))
> @@ -175,7 +175,7 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev,
>  
>  	clk = of_get_property(np, "clock-frequency", &size);
>  	if (clk && size == sizeof(*clk) && *clk)
> -		of_host->clock = be32_to_cpup(clk);
> +		pltfm_host->clock = be32_to_cpup(clk);
>  
>  	ret = sdhci_add_host(host);
>  	if (ret)
> @@ -205,12 +205,12 @@ static int __devexit sdhci_of_remove(struct platform_device *ofdev)
>  
>  static const struct of_device_id sdhci_of_match[] = {
>  #ifdef CONFIG_MMC_SDHCI_OF_ESDHC
> -	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc, },
> -	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc, },
> -	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc, },
> +	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_pdata, },
> +	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_pdata, },
> +	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_pdata, },
>  #endif
>  #ifdef CONFIG_MMC_SDHCI_OF_HLWD
> -	{ .compatible = "nintendo,hollywood-sdhci", .data = &sdhci_hlwd, },
> +	{ .compatible = "nintendo,hollywood-sdhci", .data = &sdhci_hlwd_pdata, },
>  #endif
>  	{ .compatible = "generic-sdhci", },
>  	{},
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index fcd0e1f..702a98b 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -60,30 +60,32 @@ static int esdhc_of_enable_dma(struct sdhci_host *host)
>  
>  static unsigned int esdhc_of_get_max_clock(struct sdhci_host *host)
>  {
> -	struct sdhci_of_host *of_host = sdhci_priv(host);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  
> -	return of_host->clock;
> +	return pltfm_host->clock;
>  }
>  
>  static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
>  {
> -	struct sdhci_of_host *of_host = sdhci_priv(host);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  
> -	return of_host->clock / 256 / 16;
> +	return pltfm_host->clock / 256 / 16;
>  }
>  
> -struct sdhci_of_data sdhci_esdhc = {
> +static struct sdhci_ops sdhci_esdhc_ops = {
> +	.read_l = sdhci_be32bs_readl,
> +	.read_w = esdhc_readw,
> +	.read_b = sdhci_be32bs_readb,
> +	.write_l = sdhci_be32bs_writel,
> +	.write_w = esdhc_writew,
> +	.write_b = esdhc_writeb,
> +	.set_clock = esdhc_set_clock,
> +	.enable_dma = esdhc_of_enable_dma,
> +	.get_max_clock = esdhc_of_get_max_clock,
> +	.get_min_clock = esdhc_of_get_min_clock,
> +};
> +
> +struct sdhci_pltfm_data sdhci_esdhc_pdata = {
>  	.quirks = ESDHC_DEFAULT_QUIRKS,
> -	.ops = {
> -		.read_l = sdhci_be32bs_readl,
> -		.read_w = esdhc_readw,
> -		.read_b = sdhci_be32bs_readb,
> -		.write_l = sdhci_be32bs_writel,
> -		.write_w = esdhc_writew,
> -		.write_b = esdhc_writeb,
> -		.set_clock = esdhc_set_clock,
> -		.enable_dma = esdhc_of_enable_dma,
> -		.get_max_clock = esdhc_of_get_max_clock,
> -		.get_min_clock = esdhc_of_get_min_clock,
> -	},
> +	.ops = &sdhci_esdhc_ops,
>  };
> diff --git a/drivers/mmc/host/sdhci-of-hlwd.c b/drivers/mmc/host/sdhci-of-hlwd.c
> index 68ddb75..380e896 100644
> --- a/drivers/mmc/host/sdhci-of-hlwd.c
> +++ b/drivers/mmc/host/sdhci-of-hlwd.c
> @@ -51,15 +51,17 @@ static void sdhci_hlwd_writeb(struct sdhci_host *host, u8 val, int reg)
>  	udelay(SDHCI_HLWD_WRITE_DELAY);
>  }
>  
> -struct sdhci_of_data sdhci_hlwd = {
> +static struct sdhci_ops sdhci_hlwd_ops = {
> +	.read_l = sdhci_be32bs_readl,
> +	.read_w = sdhci_be32bs_readw,
> +	.read_b = sdhci_be32bs_readb,
> +	.write_l = sdhci_hlwd_writel,
> +	.write_w = sdhci_hlwd_writew,
> +	.write_b = sdhci_hlwd_writeb,
> +};
> +
> +struct sdhci_pltfm_data sdhci_hlwd_pdata = {
>  	.quirks = SDHCI_QUIRK_32BIT_DMA_ADDR |
>  		  SDHCI_QUIRK_32BIT_DMA_SIZE,
> -	.ops = {
> -		.read_l = sdhci_be32bs_readl,
> -		.read_w = sdhci_be32bs_readw,
> -		.read_b = sdhci_be32bs_readb,
> -		.write_l = sdhci_hlwd_writel,
> -		.write_w = sdhci_hlwd_writew,
> -		.write_b = sdhci_hlwd_writeb,
> -	},
> +	.ops = &sdhci_hlwd_ops,
>  };
> diff --git a/drivers/mmc/host/sdhci-of.h b/drivers/mmc/host/sdhci-of.h
> index ad09ad9..5bdb5e7 100644
> --- a/drivers/mmc/host/sdhci-of.h
> +++ b/drivers/mmc/host/sdhci-of.h
> @@ -18,16 +18,7 @@
>  
>  #include <linux/types.h>
>  #include "sdhci.h"
> -
> -struct sdhci_of_data {
> -	unsigned int quirks;
> -	struct sdhci_ops ops;
> -};
> -
> -struct sdhci_of_host {
> -	unsigned int clock;
> -	u16 xfer_mode_shadow;
> -};
> +#include "sdhci-pltfm.h"
>  
>  extern u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg);
>  extern u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg);
> @@ -36,7 +27,7 @@ extern void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg);
>  extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
>  extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
>  
> -extern struct sdhci_of_data sdhci_esdhc;
> -extern struct sdhci_of_data sdhci_hlwd;
> +extern struct sdhci_pltfm_data sdhci_esdhc_pdata;
> +extern struct sdhci_pltfm_data sdhci_hlwd_pdata;
>  
>  #endif /* __SDHCI_OF_H */
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index a3e4be0..12afe86 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -19,6 +19,10 @@
>  struct sdhci_pltfm_host {
>  	struct clk *clk;
>  	u32 scratchpad; /* to handle quirks across io-accessor calls */
> +
> +	/* migrate from sdhci_of_host */
> +	unsigned int clock;
> +	u16 xfer_mode_shadow;
>  };
>  
>  extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 2/5] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 995 bytes --]

On Fri, Mar 25, 2011 at 04:48:48PM +0800, Shawn Guo wrote:
> The patch is to migrate the use of sdhci_of_host and sdhci_of_data
> to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can
> be eliminated.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

[...]

> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index a3e4be0..12afe86 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -19,6 +19,10 @@
>  struct sdhci_pltfm_host {
>  	struct clk *clk;
>  	u32 scratchpad; /* to handle quirks across io-accessor calls */
> +
> +	/* migrate from sdhci_of_host */
> +	unsigned int clock;
> +	u16 xfer_mode_shadow;

xfer_mode_shadow can be merged into scratchpad. They both fix the same
issue.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/5] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data

[Shawn Guo]

On Tue, Apr 19, 2011 at 12:20:53PM +0200, Wolfram Sang wrote:
> On Fri, Mar 25, 2011 at 04:48:48PM +0800, Shawn Guo wrote:
> > The patch is to migrate the use of sdhci_of_host and sdhci_of_data
> > to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can
> > be eliminated.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> 
> [...]
> 
> > diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> > index a3e4be0..12afe86 100644
> > --- a/drivers/mmc/host/sdhci-pltfm.h
> > +++ b/drivers/mmc/host/sdhci-pltfm.h
> > @@ -19,6 +19,10 @@
> >  struct sdhci_pltfm_host {
> >  	struct clk *clk;
> >  	u32 scratchpad; /* to handle quirks across io-accessor calls */
> > +
> > +	/* migrate from sdhci_of_host */
> > +	unsigned int clock;
> > +	u16 xfer_mode_shadow;
> 
> xfer_mode_shadow can be merged into scratchpad. They both fix the same
> issue.
> 
Yeah.

-- 
Regards,
Shawn

[PATCH 3/5] mmc: sdhci: make sdhci-of device drivers self registered

[Shawn Guo]

The patch turns the sdhci-of-core common stuff into helper functions
added into sdhci-pltfm.c, and makes sdhci-of device drviers self
registered using the same pair of .probe and .remove used by
sdhci-pltfm device drivers.

As a result, sdhci-of-core.c and sdhci-of.h can be eliminated with
those common things merged into sdhci-pltfm.c and sdhci-pltfm.h
respectively.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/host/Kconfig          |   15 +--
 drivers/mmc/host/Makefile         |    7 +-
 drivers/mmc/host/sdhci-of-core.c  |  247 -------------------------------------
 drivers/mmc/host/sdhci-of-esdhc.c |   75 +++++++++++-
 drivers/mmc/host/sdhci-of-hlwd.c  |   73 +++++++++++-
 drivers/mmc/host/sdhci-of.h       |   33 -----
 drivers/mmc/host/sdhci-pltfm.c    |  100 +++++++++++++++-
 drivers/mmc/host/sdhci-pltfm.h    |   14 ++
 8 files changed, 263 insertions(+), 301 deletions(-)
 delete mode 100644 drivers/mmc/host/sdhci-of-core.c
 delete mode 100644 drivers/mmc/host/sdhci-of.h

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1db9347..9f360b5 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -81,19 +81,11 @@ config MMC_RICOH_MMC
 
 	  If unsure, say Y.
 
-config MMC_SDHCI_OF
-	tristate "SDHCI support on OpenFirmware platforms"
-	depends on MMC_SDHCI && OF
-	help
-	  This selects the OF support for Secure Digital Host Controller
-	  Interfaces.
-
-	  If unsure, say N.
-
 config MMC_SDHCI_OF_ESDHC
 	bool "SDHCI OF support for the Freescale eSDHC controller"
-	depends on MMC_SDHCI_OF
+	depends on MMC_SDHCI
 	depends on PPC_OF
+	select MMC_SDHCI_PLTFM
 	select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
 	help
 	  This selects the Freescale eSDHC controller support.
@@ -102,8 +94,9 @@ config MMC_SDHCI_OF_ESDHC
 
 config MMC_SDHCI_OF_HLWD
 	bool "SDHCI OF support for the Nintendo Wii SDHCI controllers"
-	depends on MMC_SDHCI_OF
+	depends on MMC_SDHCI
 	depends on PPC_OF
+	select MMC_SDHCI_PLTFM
 	select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 1d8e43d..0ea8815 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -41,11 +41,8 @@ obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
 obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
 obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
 obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= sdhci-tegra.o
-
-obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
-sdhci-of-y				:= sdhci-of-core.o
-sdhci-of-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
-sdhci-of-$(CONFIG_MMC_SDHCI_OF_HLWD)	+= sdhci-of-hlwd.o
+obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
+obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
 	CFLAGS-cb710-mmc	+= -DDEBUG
diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
deleted file mode 100644
index a6c0132..0000000
--- a/drivers/mmc/host/sdhci-of-core.c
+++ /dev/null
@@ -1,247 +0,0 @@
-/*
- * OpenFirmware bindings for Secure Digital Host Controller Interface.
- *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
- * Copyright (c) 2009 MontaVista Software, Inc.
- *
- * Authors: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
- *	    Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- */
-
-#include <linux/err.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/interrupt.h>
-#include <linux/delay.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/mmc/host.h>
-#ifdef CONFIG_PPC
-#include <asm/machdep.h>
-#endif
-#include "sdhci-of.h"
-#include "sdhci.h"
-
-#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
-
-/*
- * These accessors are designed for big endian hosts doing I/O to
- * little endian controllers incorporating a 32-bit hardware byte swapper.
- */
-
-u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg)
-{
-	return in_be32(host->ioaddr + reg);
-}
-
-u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg)
-{
-	return in_be16(host->ioaddr + (reg ^ 0x2));
-}
-
-u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg)
-{
-	return in_8(host->ioaddr + (reg ^ 0x3));
-}
-
-void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
-{
-	out_be32(host->ioaddr + reg, val);
-}
-
-void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	int base = reg & ~0x3;
-	int shift = (reg & 0x2) * 8;
-
-	switch (reg) {
-	case SDHCI_TRANSFER_MODE:
-		/*
-		 * Postpone this write, we must do it together with a
-		 * command write that is down below.
-		 */
-		pltfm_host->xfer_mode_shadow = val;
-		return;
-	case SDHCI_COMMAND:
-		sdhci_be32bs_writel(host, val << 16 | pltfm_host->xfer_mode_shadow,
-				    SDHCI_TRANSFER_MODE);
-		return;
-	}
-	clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
-}
-
-void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg)
-{
-	int base = reg & ~0x3;
-	int shift = (reg & 0x3) * 8;
-
-	clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
-}
-#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */
-
-#ifdef CONFIG_PM
-
-static int sdhci_of_suspend(struct platform_device *ofdev, pm_message_t state)
-{
-	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
-
-	return mmc_suspend_host(host->mmc);
-}
-
-static int sdhci_of_resume(struct platform_device *ofdev)
-{
-	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
-
-	return mmc_resume_host(host->mmc);
-}
-
-#else
-
-#define sdhci_of_suspend NULL
-#define sdhci_of_resume NULL
-
-#endif
-
-static bool __devinit sdhci_of_wp_inverted(struct device_node *np)
-{
-	if (of_get_property(np, "sdhci,wp-inverted", NULL))
-		return true;
-
-	/* Old device trees don't have the wp-inverted property. */
-#ifdef CONFIG_PPC
-	return machine_is(mpc837x_rdb) || machine_is(mpc837x_mds);
-#else
-	return false;
-#endif
-}
-
-static int __devinit sdhci_of_probe(struct platform_device *ofdev,
-				 const struct of_device_id *match)
-{
-	struct device_node *np = ofdev->dev.of_node;
-	struct sdhci_pltfm_data *pdata = match->data;
-	struct sdhci_host *host;
-	struct sdhci_pltfm_host *pltfm_host;
-	const __be32 *clk;
-	int size;
-	int ret;
-
-	if (!of_device_is_available(np))
-		return -ENODEV;
-
-	host = sdhci_alloc_host(&ofdev->dev, sizeof(*pltfm_host));
-	if (IS_ERR(host))
-		return -ENOMEM;
-
-	pltfm_host = sdhci_priv(host);
-	dev_set_drvdata(&ofdev->dev, host);
-
-	host->ioaddr = of_iomap(np, 0);
-	if (!host->ioaddr) {
-		ret = -ENOMEM;
-		goto err_addr_map;
-	}
-
-	host->irq = irq_of_parse_and_map(np, 0);
-	if (!host->irq) {
-		ret = -EINVAL;
-		goto err_no_irq;
-	}
-
-	host->hw_name = dev_name(&ofdev->dev);
-	if (pdata) {
-		host->quirks = pdata->quirks;
-		host->ops = &pdata->ops;
-	}
-
-	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))
-		host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
-
-	if (sdhci_of_wp_inverted(np))
-		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
-
-	clk = of_get_property(np, "clock-frequency", &size);
-	if (clk && size == sizeof(*clk) && *clk)
-		pltfm_host->clock = be32_to_cpup(clk);
-
-	ret = sdhci_add_host(host);
-	if (ret)
-		goto err_add_host;
-
-	return 0;
-
-err_add_host:
-	irq_dispose_mapping(host->irq);
-err_no_irq:
-	iounmap(host->ioaddr);
-err_addr_map:
-	sdhci_free_host(host);
-	return ret;
-}
-
-static int __devexit sdhci_of_remove(struct platform_device *ofdev)
-{
-	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
-
-	sdhci_remove_host(host, 0);
-	sdhci_free_host(host);
-	irq_dispose_mapping(host->irq);
-	iounmap(host->ioaddr);
-	return 0;
-}
-
-static const struct of_device_id sdhci_of_match[] = {
-#ifdef CONFIG_MMC_SDHCI_OF_ESDHC
-	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_pdata, },
-	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_pdata, },
-	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_pdata, },
-#endif
-#ifdef CONFIG_MMC_SDHCI_OF_HLWD
-	{ .compatible = "nintendo,hollywood-sdhci", .data = &sdhci_hlwd_pdata, },
-#endif
-	{ .compatible = "generic-sdhci", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, sdhci_of_match);
-
-static struct of_platform_driver sdhci_of_driver = {
-	.driver = {
-		.name = "sdhci-of",
-		.owner = THIS_MODULE,
-		.of_match_table = sdhci_of_match,
-	},
-	.probe = sdhci_of_probe,
-	.remove = __devexit_p(sdhci_of_remove),
-	.suspend = sdhci_of_suspend,
-	.resume	= sdhci_of_resume,
-};
-
-static int __init sdhci_of_init(void)
-{
-	return of_register_platform_driver(&sdhci_of_driver);
-}
-module_init(sdhci_of_init);
-
-static void __exit sdhci_of_exit(void)
-{
-	of_unregister_platform_driver(&sdhci_of_driver);
-}
-module_exit(sdhci_of_exit);
-
-MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
-MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
-	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
-MODULE_LICENSE("GPL");
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 702a98b..ccd182f 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -16,7 +16,7 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/mmc/host.h>
-#include "sdhci-of.h"
+#include "sdhci-pltfm.h"
 #include "sdhci.h"
 #include "sdhci-esdhc.h"
 
@@ -85,7 +85,78 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.get_min_clock = esdhc_of_get_min_clock,
 };
 
-struct sdhci_pltfm_data sdhci_esdhc_pdata = {
+static struct sdhci_pltfm_data sdhci_esdhc_pdata = {
 	.quirks = ESDHC_DEFAULT_QUIRKS,
 	.ops = &sdhci_esdhc_ops,
 };
+
+static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	int ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata);
+	if (!host)
+		return -ENOMEM;
+
+	sdhci_get_of_property(pdev);
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
+	return 0;
+
+err_add_host:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int __devexit sdhci_esdhc_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+
+	sdhci_remove_host(host, 0);
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
+static const struct of_device_id sdhci_esdhc_of_match[] = {
+	{ .compatible = "fsl,mpc8379-esdhc" },
+	{ .compatible = "fsl,mpc8536-esdhc" },
+	{ .compatible = "fsl,esdhc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_esdhc_of_match);
+
+static struct platform_driver sdhci_esdhc_driver = {
+	.driver = {
+		.name = "sdhci-esdhc",
+		.owner = THIS_MODULE,
+		.of_match_table = sdhci_esdhc_of_match,
+	},
+	.probe = sdhci_esdhc_probe,
+	.remove = __devexit_p(sdhci_esdhc_remove),
+#ifdef CONFIG_PM
+	.suspend = sdhci_pltfm_suspend,
+	.resume = sdhci_pltfm_resume,
+#endif
+};
+
+static int __init sdhci_esdhc_init(void)
+{
+	return platform_driver_register(&sdhci_esdhc_driver);
+}
+module_init(sdhci_esdhc_init);
+
+static void __exit sdhci_esdhc_exit(void)
+{
+	platform_driver_unregister(&sdhci_esdhc_driver);
+}
+module_exit(sdhci_esdhc_exit);
+
+MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
+MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
+	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-of-hlwd.c b/drivers/mmc/host/sdhci-of-hlwd.c
index 380e896..52944d1 100644
--- a/drivers/mmc/host/sdhci-of-hlwd.c
+++ b/drivers/mmc/host/sdhci-of-hlwd.c
@@ -21,7 +21,7 @@
 
 #include <linux/delay.h>
 #include <linux/mmc/host.h>
-#include "sdhci-of.h"
+#include "sdhci-pltfm.h"
 #include "sdhci.h"
 
 /*
@@ -60,8 +60,77 @@ static struct sdhci_ops sdhci_hlwd_ops = {
 	.write_b = sdhci_hlwd_writeb,
 };
 
-struct sdhci_pltfm_data sdhci_hlwd_pdata = {
+static struct sdhci_pltfm_data sdhci_hlwd_pdata = {
 	.quirks = SDHCI_QUIRK_32BIT_DMA_ADDR |
 		  SDHCI_QUIRK_32BIT_DMA_SIZE,
 	.ops = &sdhci_hlwd_ops,
 };
+
+static int __devinit sdhci_hlwd_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	int ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_hlwd_pdata);
+	if (!host)
+		return -ENOMEM;
+
+	sdhci_get_of_property(pdev);
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
+	return 0;
+
+err_add_host:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int __devexit sdhci_hlwd_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+
+	sdhci_remove_host(host, 0);
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
+static const struct of_device_id sdhci_hlwd_of_match[] = {
+	{ .compatible = "nintendo,hollywood-sdhci" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_hlwd_of_match);
+
+static struct platform_driver sdhci_hlwd_driver = {
+	.driver = {
+		.name = "sdhci-hlwd",
+		.owner = THIS_MODULE,
+		.of_match_table = sdhci_hlwd_of_match,
+	},
+	.probe = sdhci_hlwd_probe,
+	.remove = __devexit_p(sdhci_hlwd_remove),
+#ifdef CONFIG_PM
+	.suspend = sdhci_pltfm_suspend,
+	.resume = sdhci_pltfm_resume,
+#endif
+};
+
+static int __init sdhci_hlwd_init(void)
+{
+	return platform_driver_register(&sdhci_hlwd_driver);
+}
+module_init(sdhci_hlwd_init);
+
+static void __exit sdhci_hlwd_exit(void)
+{
+	platform_driver_unregister(&sdhci_hlwd_driver);
+}
+module_exit(sdhci_hlwd_exit);
+
+MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
+MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
+	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-of.h b/drivers/mmc/host/sdhci-of.h
deleted file mode 100644
index 5bdb5e7..0000000
--- a/drivers/mmc/host/sdhci-of.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * OpenFirmware bindings for Secure Digital Host Controller Interface.
- *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
- * Copyright (c) 2009 MontaVista Software, Inc.
- *
- * Authors: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
- *	    Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- */
-
-#ifndef __SDHCI_OF_H
-#define __SDHCI_OF_H
-
-#include <linux/types.h>
-#include "sdhci.h"
-#include "sdhci-pltfm.h"
-
-extern u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg);
-extern u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg);
-extern u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg);
-extern void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg);
-extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
-extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
-
-extern struct sdhci_pltfm_data sdhci_esdhc_pdata;
-extern struct sdhci_pltfm_data sdhci_hlwd_pdata;
-
-#endif /* __SDHCI_OF_H */
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 63e45aa..4e998f1 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -23,13 +23,111 @@
  */
 
 #include <linux/err.h>
-
+#include <linux/of.h>
+#ifdef CONFIG_PPC
+#include <asm/machdep.h>
+#endif
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 
 static struct sdhci_ops sdhci_pltfm_ops = {
 };
 
+#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
+/*
+ * These accessors are designed for big endian hosts doing I/O to
+ * little endian controllers incorporating a 32-bit hardware byte swapper.
+ */
+u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg)
+{
+	return in_be32(host->ioaddr + reg);
+}
+
+u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg)
+{
+	return in_be16(host->ioaddr + (reg ^ 0x2));
+}
+
+u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg)
+{
+	return in_8(host->ioaddr + (reg ^ 0x3));
+}
+
+void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	out_be32(host->ioaddr + reg, val);
+}
+
+void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	int base = reg & ~0x3;
+	int shift = (reg & 0x2) * 8;
+
+	switch (reg) {
+	case SDHCI_TRANSFER_MODE:
+		/*
+		 * Postpone this write, we must do it together with a
+		 * command write that is down below.
+		 */
+		pltfm_host->xfer_mode_shadow = val;
+		return;
+	case SDHCI_COMMAND:
+		sdhci_be32bs_writel(host, val << 16 | pltfm_host->xfer_mode_shadow,
+				    SDHCI_TRANSFER_MODE);
+		return;
+	}
+	clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
+}
+
+void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	int base = reg & ~0x3;
+	int shift = (reg & 0x3) * 8;
+
+	clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
+}
+#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */
+
+#ifdef CONFIG_OF
+static bool sdhci_of_wp_inverted(struct device_node *np)
+{
+	if (of_get_property(np, "sdhci,wp-inverted", NULL))
+		return true;
+
+	/* Old device trees don't have the wp-inverted property. */
+#ifdef CONFIG_PPC
+	return machine_is(mpc837x_rdb) || machine_is(mpc837x_mds);
+#else
+	return false;
+#endif
+}
+
+void sdhci_get_of_property(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	const __be32 *clk;
+	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))
+			host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
+
+		if (sdhci_of_wp_inverted(np))
+			host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
+
+		clk = of_get_property(np, "clock-frequency", &size);
+		if (clk && size == sizeof(*clk) && *clk)
+			pltfm_host->clock = be32_to_cpup(clk);
+	}
+}
+#endif
+
 struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 				    struct sdhci_pltfm_data *pdata)
 {
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 12afe86..2f38056 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -15,6 +15,7 @@
 #include <linux/types.h>
 #include <linux/platform_device.h>
 #include <linux/mmc/sdhci-pltfm.h>
+#include <linux/mmc/sdhci.h>
 
 struct sdhci_pltfm_host {
 	struct clk *clk;
@@ -25,6 +26,19 @@ struct sdhci_pltfm_host {
 	u16 xfer_mode_shadow;
 };
 
+#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
+extern u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg);
+extern u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg);
+extern u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg);
+extern void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg);
+extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
+extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
+#endif
+
+#ifdef CONFIG_OF
+extern void sdhci_get_of_property(struct platform_device *pdev);
+#endif
+
 extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 					   struct sdhci_pltfm_data *pdata);
 extern void sdhci_pltfm_free(struct platform_device *pdev);
-- 
1.7.1

Re: [PATCH 3/5] mmc: sdhci: make sdhci-of device drivers self registered

[Grant Likely]

On Fri, Mar 25, 2011 at 04:48:49PM +0800, Shawn Guo wrote:
> The patch turns the sdhci-of-core common stuff into helper functions
> added into sdhci-pltfm.c, and makes sdhci-of device drviers self
> registered using the same pair of .probe and .remove used by
> sdhci-pltfm device drivers.
> 
> As a result, sdhci-of-core.c and sdhci-of.h can be eliminated with
> those common things merged into sdhci-pltfm.c and sdhci-pltfm.h
> respectively.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/mmc/host/Kconfig          |   15 +--
>  drivers/mmc/host/Makefile         |    7 +-
>  drivers/mmc/host/sdhci-of-core.c  |  247 -------------------------------------
>  drivers/mmc/host/sdhci-of-esdhc.c |   75 +++++++++++-
>  drivers/mmc/host/sdhci-of-hlwd.c  |   73 +++++++++++-
>  drivers/mmc/host/sdhci-of.h       |   33 -----
>  drivers/mmc/host/sdhci-pltfm.c    |  100 +++++++++++++++-
>  drivers/mmc/host/sdhci-pltfm.h    |   14 ++
>  8 files changed, 263 insertions(+), 301 deletions(-)
>  delete mode 100644 drivers/mmc/host/sdhci-of-core.c
>  delete mode 100644 drivers/mmc/host/sdhci-of.h
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1db9347..9f360b5 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -81,19 +81,11 @@ config MMC_RICOH_MMC
>  
>  	  If unsure, say Y.
>  
> -config MMC_SDHCI_OF
> -	tristate "SDHCI support on OpenFirmware platforms"
> -	depends on MMC_SDHCI && OF
> -	help
> -	  This selects the OF support for Secure Digital Host Controller
> -	  Interfaces.
> -
> -	  If unsure, say N.
> -
>  config MMC_SDHCI_OF_ESDHC
>  	bool "SDHCI OF support for the Freescale eSDHC controller"
> -	depends on MMC_SDHCI_OF
> +	depends on MMC_SDHCI
>  	depends on PPC_OF
> +	select MMC_SDHCI_PLTFM
>  	select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
>  	help
>  	  This selects the Freescale eSDHC controller support.
> @@ -102,8 +94,9 @@ config MMC_SDHCI_OF_ESDHC
>  
>  config MMC_SDHCI_OF_HLWD
>  	bool "SDHCI OF support for the Nintendo Wii SDHCI controllers"
> -	depends on MMC_SDHCI_OF
> +	depends on MMC_SDHCI
>  	depends on PPC_OF
> +	select MMC_SDHCI_PLTFM
>  	select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
>  	help
>  	  This selects the Secure Digital Host Controller Interface (SDHCI)
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 1d8e43d..0ea8815 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -41,11 +41,8 @@ obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
>  obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
>  obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
>  obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= sdhci-tegra.o
> -
> -obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
> -sdhci-of-y				:= sdhci-of-core.o
> -sdhci-of-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
> -sdhci-of-$(CONFIG_MMC_SDHCI_OF_HLWD)	+= sdhci-of-hlwd.o
> +obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
> +obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
>  
>  ifeq ($(CONFIG_CB710_DEBUG),y)
>  	CFLAGS-cb710-mmc	+= -DDEBUG
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> deleted file mode 100644
> index a6c0132..0000000
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ /dev/null
> @@ -1,247 +0,0 @@
> -/*
> - * OpenFirmware bindings for Secure Digital Host Controller Interface.
> - *
> - * Copyright (c) 2007 Freescale Semiconductor, Inc.
> - * Copyright (c) 2009 MontaVista Software, Inc.
> - *
> - * Authors: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> - *	    Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or (at
> - * your option) any later version.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/io.h>
> -#include <linux/interrupt.h>
> -#include <linux/delay.h>
> -#include <linux/of.h>
> -#include <linux/of_platform.h>
> -#include <linux/of_address.h>
> -#include <linux/of_irq.h>
> -#include <linux/mmc/host.h>
> -#ifdef CONFIG_PPC
> -#include <asm/machdep.h>
> -#endif
> -#include "sdhci-of.h"
> -#include "sdhci.h"
> -
> -#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
> -
> -/*
> - * These accessors are designed for big endian hosts doing I/O to
> - * little endian controllers incorporating a 32-bit hardware byte swapper.
> - */
> -
> -u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg)
> -{
> -	return in_be32(host->ioaddr + reg);
> -}
> -
> -u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg)
> -{
> -	return in_be16(host->ioaddr + (reg ^ 0x2));
> -}
> -
> -u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg)
> -{
> -	return in_8(host->ioaddr + (reg ^ 0x3));
> -}
> -
> -void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
> -{
> -	out_be32(host->ioaddr + reg, val);
> -}
> -
> -void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
> -{
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	int base = reg & ~0x3;
> -	int shift = (reg & 0x2) * 8;
> -
> -	switch (reg) {
> -	case SDHCI_TRANSFER_MODE:
> -		/*
> -		 * Postpone this write, we must do it together with a
> -		 * command write that is down below.
> -		 */
> -		pltfm_host->xfer_mode_shadow = val;
> -		return;
> -	case SDHCI_COMMAND:
> -		sdhci_be32bs_writel(host, val << 16 | pltfm_host->xfer_mode_shadow,
> -				    SDHCI_TRANSFER_MODE);
> -		return;
> -	}
> -	clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
> -}
> -
> -void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg)
> -{
> -	int base = reg & ~0x3;
> -	int shift = (reg & 0x3) * 8;
> -
> -	clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
> -}
> -#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */
> -
> -#ifdef CONFIG_PM
> -
> -static int sdhci_of_suspend(struct platform_device *ofdev, pm_message_t state)
> -{
> -	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
> -
> -	return mmc_suspend_host(host->mmc);
> -}
> -
> -static int sdhci_of_resume(struct platform_device *ofdev)
> -{
> -	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
> -
> -	return mmc_resume_host(host->mmc);
> -}
> -
> -#else
> -
> -#define sdhci_of_suspend NULL
> -#define sdhci_of_resume NULL
> -
> -#endif
> -
> -static bool __devinit sdhci_of_wp_inverted(struct device_node *np)
> -{
> -	if (of_get_property(np, "sdhci,wp-inverted", NULL))
> -		return true;
> -
> -	/* Old device trees don't have the wp-inverted property. */
> -#ifdef CONFIG_PPC
> -	return machine_is(mpc837x_rdb) || machine_is(mpc837x_mds);
> -#else
> -	return false;
> -#endif
> -}
> -
> -static int __devinit sdhci_of_probe(struct platform_device *ofdev,
> -				 const struct of_device_id *match)
> -{
> -	struct device_node *np = ofdev->dev.of_node;
> -	struct sdhci_pltfm_data *pdata = match->data;
> -	struct sdhci_host *host;
> -	struct sdhci_pltfm_host *pltfm_host;
> -	const __be32 *clk;
> -	int size;
> -	int ret;
> -
> -	if (!of_device_is_available(np))
> -		return -ENODEV;
> -
> -	host = sdhci_alloc_host(&ofdev->dev, sizeof(*pltfm_host));
> -	if (IS_ERR(host))
> -		return -ENOMEM;
> -
> -	pltfm_host = sdhci_priv(host);
> -	dev_set_drvdata(&ofdev->dev, host);
> -
> -	host->ioaddr = of_iomap(np, 0);
> -	if (!host->ioaddr) {
> -		ret = -ENOMEM;
> -		goto err_addr_map;
> -	}
> -
> -	host->irq = irq_of_parse_and_map(np, 0);
> -	if (!host->irq) {
> -		ret = -EINVAL;
> -		goto err_no_irq;
> -	}
> -
> -	host->hw_name = dev_name(&ofdev->dev);
> -	if (pdata) {
> -		host->quirks = pdata->quirks;
> -		host->ops = &pdata->ops;
> -	}
> -
> -	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))
> -		host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
> -
> -	if (sdhci_of_wp_inverted(np))
> -		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> -
> -	clk = of_get_property(np, "clock-frequency", &size);
> -	if (clk && size == sizeof(*clk) && *clk)
> -		pltfm_host->clock = be32_to_cpup(clk);
> -
> -	ret = sdhci_add_host(host);
> -	if (ret)
> -		goto err_add_host;
> -
> -	return 0;
> -
> -err_add_host:
> -	irq_dispose_mapping(host->irq);
> -err_no_irq:
> -	iounmap(host->ioaddr);
> -err_addr_map:
> -	sdhci_free_host(host);
> -	return ret;
> -}
> -
> -static int __devexit sdhci_of_remove(struct platform_device *ofdev)
> -{
> -	struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
> -
> -	sdhci_remove_host(host, 0);
> -	sdhci_free_host(host);
> -	irq_dispose_mapping(host->irq);
> -	iounmap(host->ioaddr);
> -	return 0;
> -}
> -
> -static const struct of_device_id sdhci_of_match[] = {
> -#ifdef CONFIG_MMC_SDHCI_OF_ESDHC
> -	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_pdata, },
> -	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_pdata, },
> -	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_pdata, },
> -#endif
> -#ifdef CONFIG_MMC_SDHCI_OF_HLWD
> -	{ .compatible = "nintendo,hollywood-sdhci", .data = &sdhci_hlwd_pdata, },
> -#endif
> -	{ .compatible = "generic-sdhci", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, sdhci_of_match);
> -
> -static struct of_platform_driver sdhci_of_driver = {
> -	.driver = {
> -		.name = "sdhci-of",
> -		.owner = THIS_MODULE,
> -		.of_match_table = sdhci_of_match,
> -	},
> -	.probe = sdhci_of_probe,
> -	.remove = __devexit_p(sdhci_of_remove),
> -	.suspend = sdhci_of_suspend,
> -	.resume	= sdhci_of_resume,
> -};
> -
> -static int __init sdhci_of_init(void)
> -{
> -	return of_register_platform_driver(&sdhci_of_driver);
> -}
> -module_init(sdhci_of_init);
> -
> -static void __exit sdhci_of_exit(void)
> -{
> -	of_unregister_platform_driver(&sdhci_of_driver);
> -}
> -module_exit(sdhci_of_exit);
> -
> -MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
> -MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
> -	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
> -MODULE_LICENSE("GPL");
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index 702a98b..ccd182f 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -16,7 +16,7 @@
>  #include <linux/io.h>
>  #include <linux/delay.h>
>  #include <linux/mmc/host.h>
> -#include "sdhci-of.h"
> +#include "sdhci-pltfm.h"
>  #include "sdhci.h"
>  #include "sdhci-esdhc.h"
>  
> @@ -85,7 +85,78 @@ static struct sdhci_ops sdhci_esdhc_ops = {
>  	.get_min_clock = esdhc_of_get_min_clock,
>  };
>  
> -struct sdhci_pltfm_data sdhci_esdhc_pdata = {
> +static struct sdhci_pltfm_data sdhci_esdhc_pdata = {
>  	.quirks = ESDHC_DEFAULT_QUIRKS,
>  	.ops = &sdhci_esdhc_ops,
>  };
> +
> +static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	int ret;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	sdhci_get_of_property(pdev);
> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		goto err_add_host;
> +
> +	return 0;
> +
> +err_add_host:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}
> +
> +static int __devexit sdhci_esdhc_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +
> +	sdhci_remove_host(host, 0);
> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sdhci_esdhc_of_match[] = {
> +	{ .compatible = "fsl,mpc8379-esdhc" },
> +	{ .compatible = "fsl,mpc8536-esdhc" },
> +	{ .compatible = "fsl,esdhc" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_esdhc_of_match);
> +
> +static struct platform_driver sdhci_esdhc_driver = {
> +	.driver = {
> +		.name = "sdhci-esdhc",
> +		.owner = THIS_MODULE,
> +		.of_match_table = sdhci_esdhc_of_match,
> +	},
> +	.probe = sdhci_esdhc_probe,
> +	.remove = __devexit_p(sdhci_esdhc_remove),
> +#ifdef CONFIG_PM
> +	.suspend = sdhci_pltfm_suspend,
> +	.resume = sdhci_pltfm_resume,
> +#endif
> +};
> +
> +static int __init sdhci_esdhc_init(void)
> +{
> +	return platform_driver_register(&sdhci_esdhc_driver);
> +}
> +module_init(sdhci_esdhc_init);
> +
> +static void __exit sdhci_esdhc_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_esdhc_driver);
> +}
> +module_exit(sdhci_esdhc_exit);
> +
> +MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
> +MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
> +	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-of-hlwd.c b/drivers/mmc/host/sdhci-of-hlwd.c
> index 380e896..52944d1 100644
> --- a/drivers/mmc/host/sdhci-of-hlwd.c
> +++ b/drivers/mmc/host/sdhci-of-hlwd.c
> @@ -21,7 +21,7 @@
>  
>  #include <linux/delay.h>
>  #include <linux/mmc/host.h>
> -#include "sdhci-of.h"
> +#include "sdhci-pltfm.h"
>  #include "sdhci.h"
>  
>  /*
> @@ -60,8 +60,77 @@ static struct sdhci_ops sdhci_hlwd_ops = {
>  	.write_b = sdhci_hlwd_writeb,
>  };
>  
> -struct sdhci_pltfm_data sdhci_hlwd_pdata = {
> +static struct sdhci_pltfm_data sdhci_hlwd_pdata = {
>  	.quirks = SDHCI_QUIRK_32BIT_DMA_ADDR |
>  		  SDHCI_QUIRK_32BIT_DMA_SIZE,
>  	.ops = &sdhci_hlwd_ops,
>  };
> +
> +static int __devinit sdhci_hlwd_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	int ret;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_hlwd_pdata);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	sdhci_get_of_property(pdev);
> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		goto err_add_host;
> +
> +	return 0;
> +
> +err_add_host:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}
> +
> +static int __devexit sdhci_hlwd_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +
> +	sdhci_remove_host(host, 0);
> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sdhci_hlwd_of_match[] = {
> +	{ .compatible = "nintendo,hollywood-sdhci" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_hlwd_of_match);
> +
> +static struct platform_driver sdhci_hlwd_driver = {
> +	.driver = {
> +		.name = "sdhci-hlwd",
> +		.owner = THIS_MODULE,
> +		.of_match_table = sdhci_hlwd_of_match,
> +	},
> +	.probe = sdhci_hlwd_probe,
> +	.remove = __devexit_p(sdhci_hlwd_remove),
> +#ifdef CONFIG_PM
> +	.suspend = sdhci_pltfm_suspend,
> +	.resume = sdhci_pltfm_resume,
> +#endif
> +};
> +
> +static int __init sdhci_hlwd_init(void)
> +{
> +	return platform_driver_register(&sdhci_hlwd_driver);
> +}
> +module_init(sdhci_hlwd_init);
> +
> +static void __exit sdhci_hlwd_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_hlwd_driver);
> +}
> +module_exit(sdhci_hlwd_exit);
> +
> +MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
> +MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
> +	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-of.h b/drivers/mmc/host/sdhci-of.h
> deleted file mode 100644
> index 5bdb5e7..0000000
> --- a/drivers/mmc/host/sdhci-of.h
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/*
> - * OpenFirmware bindings for Secure Digital Host Controller Interface.
> - *
> - * Copyright (c) 2007 Freescale Semiconductor, Inc.
> - * Copyright (c) 2009 MontaVista Software, Inc.
> - *
> - * Authors: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> - *	    Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or (at
> - * your option) any later version.
> - */
> -
> -#ifndef __SDHCI_OF_H
> -#define __SDHCI_OF_H
> -
> -#include <linux/types.h>
> -#include "sdhci.h"
> -#include "sdhci-pltfm.h"
> -
> -extern u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg);
> -extern u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg);
> -extern u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg);
> -extern void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg);
> -extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
> -extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
> -
> -extern struct sdhci_pltfm_data sdhci_esdhc_pdata;
> -extern struct sdhci_pltfm_data sdhci_hlwd_pdata;
> -
> -#endif /* __SDHCI_OF_H */
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index 63e45aa..4e998f1 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -23,13 +23,111 @@
>   */
>  
>  #include <linux/err.h>
> -
> +#include <linux/of.h>
> +#ifdef CONFIG_PPC
> +#include <asm/machdep.h>
> +#endif
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
>  
>  static struct sdhci_ops sdhci_pltfm_ops = {
>  };
>  
> +#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
> +/*
> + * These accessors are designed for big endian hosts doing I/O to
> + * little endian controllers incorporating a 32-bit hardware byte swapper.
> + */
> +u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg)
> +{
> +	return in_be32(host->ioaddr + reg);
> +}
> +
> +u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg)
> +{
> +	return in_be16(host->ioaddr + (reg ^ 0x2));
> +}
> +
> +u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg)
> +{
> +	return in_8(host->ioaddr + (reg ^ 0x3));
> +}
> +
> +void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
> +{
> +	out_be32(host->ioaddr + reg, val);
> +}
> +
> +void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	int base = reg & ~0x3;
> +	int shift = (reg & 0x2) * 8;
> +
> +	switch (reg) {
> +	case SDHCI_TRANSFER_MODE:
> +		/*
> +		 * Postpone this write, we must do it together with a
> +		 * command write that is down below.
> +		 */
> +		pltfm_host->xfer_mode_shadow = val;
> +		return;
> +	case SDHCI_COMMAND:
> +		sdhci_be32bs_writel(host, val << 16 | pltfm_host->xfer_mode_shadow,
> +				    SDHCI_TRANSFER_MODE);
> +		return;
> +	}
> +	clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
> +}
> +
> +void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> +	int base = reg & ~0x3;
> +	int shift = (reg & 0x3) * 8;
> +
> +	clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
> +}
> +#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */
> +
> +#ifdef CONFIG_OF
> +static bool sdhci_of_wp_inverted(struct device_node *np)
> +{
> +	if (of_get_property(np, "sdhci,wp-inverted", NULL))
> +		return true;
> +
> +	/* Old device trees don't have the wp-inverted property. */
> +#ifdef CONFIG_PPC
> +	return machine_is(mpc837x_rdb) || machine_is(mpc837x_mds);
> +#else
> +	return false;
> +#endif
> +}
> +
> +void sdhci_get_of_property(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	const __be32 *clk;
> +	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))
> +			host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
> +
> +		if (sdhci_of_wp_inverted(np))
> +			host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> +
> +		clk = of_get_property(np, "clock-frequency", &size);
> +		if (clk && size == sizeof(*clk) && *clk)
> +			pltfm_host->clock = be32_to_cpup(clk);
> +	}
> +}
> +#endif
> +
>  struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
>  				    struct sdhci_pltfm_data *pdata)
>  {
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index 12afe86..2f38056 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -15,6 +15,7 @@
>  #include <linux/types.h>
>  #include <linux/platform_device.h>
>  #include <linux/mmc/sdhci-pltfm.h>
> +#include <linux/mmc/sdhci.h>
>  
>  struct sdhci_pltfm_host {
>  	struct clk *clk;
> @@ -25,6 +26,19 @@ struct sdhci_pltfm_host {
>  	u16 xfer_mode_shadow;
>  };
>  
> +#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
> +extern u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg);
> +extern u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg);
> +extern u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg);
> +extern void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg);
> +extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
> +extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
> +#endif
> +
> +#ifdef CONFIG_OF
> +extern void sdhci_get_of_property(struct platform_device *pdev);

If you also implement an sdhci_pltfm_register() function as suggested
in my earlier comment, then You'll probably also want:

#else
static void sdhci_get_of_property(struct platform_device *pdev) {}



> +#endif
> +
>  extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
>  					   struct sdhci_pltfm_data *pdata);
>  extern void sdhci_pltfm_free(struct platform_device *pdev);
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 3/5] mmc: sdhci: make sdhci-of device drivers self registered

[Wolfram Sang]

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


> +static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	int ret;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata);
> +	if (!host)
> +		return -ENOMEM;

Just noticed: Since pltfm_init may fail due to various reasons, maybe
ERRPTR might be a good idea?

[...]

> +static int __init sdhci_hlwd_init(void)
> +{
> +	return platform_driver_register(&sdhci_hlwd_driver);
> +}
> +module_init(sdhci_hlwd_init);
> +
> +static void __exit sdhci_hlwd_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_hlwd_driver);
> +}
> +module_exit(sdhci_hlwd_exit);
> +
> +MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
> +MODULE_AUTHOR("Xiaobo Xie <X.Xie@freescale.com>, "
> +	      "Anton Vorontsov <avorontsov@ru.mvista.com>");
> +MODULE_LICENSE("GPL v2");

Please double check the authors. It is based on the fsl driver, but the
copyright should go to

 * Copyright (C) 2009 The GameCube Linux Team
 * Copyright (C) 2009 Albert Herranz

I think.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 3/5] mmc: sdhci: make sdhci-of device drivers self registered

[Shawn Guo]

On Tue, Apr 19, 2011 at 12:21:01PM +0200, Wolfram Sang wrote:
> 
> > +static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
> > +{
> > +	struct sdhci_host *host;
> > +	int ret;
> > +
> > +	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata);
> > +	if (!host)
> > +		return -ENOMEM;
> 
> Just noticed: Since pltfm_init may fail due to various reasons, maybe
> ERRPTR might be a good idea?
> 
Ok.

> [...]
> 
> > +static int __init sdhci_hlwd_init(void)
> > +{
> > +	return platform_driver_register(&sdhci_hlwd_driver);
> > +}
> > +module_init(sdhci_hlwd_init);
> > +
> > +static void __exit sdhci_hlwd_exit(void)
> > +{
> > +	platform_driver_unregister(&sdhci_hlwd_driver);
> > +}
> > +module_exit(sdhci_hlwd_exit);
> > +
> > +MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
> > +MODULE_AUTHOR("Xiaobo Xie <X.Xie@freescale.com>, "
> > +	      "Anton Vorontsov <avorontsov@ru.mvista.com>");
> > +MODULE_LICENSE("GPL v2");
> 
> Please double check the authors. It is based on the fsl driver, but the
> copyright should go to
> 
>  * Copyright (C) 2009 The GameCube Linux Team
>  * Copyright (C) 2009 Albert Herranz
> 
> I think.
> 
Sorry, I misread the current copyright in sdhci-of-hlwd.c.  You are
right.

-- 
Regards,
Shawn

[PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx

[Shawn Guo]

This patch is to consolidate SDHCI driver for Freescale eSDHC
controller found on both MPCxxx and i.MX platforms.  It turns
sdhci-of-esdhc.c and sdhci-esdhc-imx.c into one sdhci-esdhc.c,
which gets the same pair of .probe and .remove serving two cases.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/host/Kconfig           |   38 ++--
 drivers/mmc/host/Makefile          |    3 +-
 drivers/mmc/host/sdhci-esdhc-imx.c |  210 ------------------
 drivers/mmc/host/sdhci-esdhc.c     |  413 ++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-of-esdhc.c  |  162 --------------
 drivers/mmc/host/sdhci-pltfm.c     |    2 +
 drivers/mmc/host/sdhci-pltfm.h     |    2 -
 7 files changed, 439 insertions(+), 391 deletions(-)
 delete mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
 create mode 100644 drivers/mmc/host/sdhci-esdhc.c
 delete mode 100644 drivers/mmc/host/sdhci-of-esdhc.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 9f360b5..e39c145 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -81,17 +81,6 @@ config MMC_RICOH_MMC
 
 	  If unsure, say Y.
 
-config MMC_SDHCI_OF_ESDHC
-	bool "SDHCI OF support for the Freescale eSDHC controller"
-	depends on MMC_SDHCI
-	depends on PPC_OF
-	select MMC_SDHCI_PLTFM
-	select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
-	help
-	  This selects the Freescale eSDHC controller support.
-
-	  If unsure, say N.
-
 config MMC_SDHCI_OF_HLWD
 	bool "SDHCI OF support for the Nintendo Wii SDHCI controllers"
 	depends on MMC_SDHCI
@@ -122,15 +111,34 @@ config MMC_SDHCI_CNS3XXX
 
 	  If unsure, say N.
 
+config MMC_SDHCI_ESDHC
+	bool
+	depends on MMC_SDHCI
+	select MMC_SDHCI_PLTFM
+	help
+	  This selects SDHCI driver for Freescale eSDHC controller.
+
+config MMC_SDHCI_ESDHC_MPC
+	bool "SDHCI support for the Freescale eSDHC MPC controller"
+	depends on MMC_SDHCI
+	depends on PPC_OF
+	select MMC_SDHCI_ESDHC
+	select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
+	help
+	  This selects the Freescale eSDHC controller support on platforms
+	  like mpc8379/mpc8536.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_ESDHC_IMX
-	bool "SDHCI platform support for the Freescale eSDHC i.MX controller"
+	bool "SDHCI support for the Freescale eSDHC i.MX controller"
 	depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
 	depends on MMC_SDHCI
-	select MMC_SDHCI_PLTFM
+	select MMC_SDHCI_ESDHC
 	select MMC_SDHCI_IO_ACCESSORS
 	help
-	  This selects the Freescale eSDHC controller support on the platform
-	  bus, found on platforms like mx35/51.
+	  This selects the Freescale eSDHC controller support on platforms
+	  like mx35/51.
 
 	  If unsure, say N.
 
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 0ea8815..e6e3aaa 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -38,10 +38,9 @@ obj-$(CONFIG_MMC_USHC)		+= ushc.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)		+= sdhci-pltfm.o
 obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
-obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
+obj-$(CONFIG_MMC_SDHCI_ESDHC)		+= sdhci-esdhc.o
 obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
 obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= sdhci-tegra.o
-obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
 obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
deleted file mode 100644
index c9eb507..0000000
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ /dev/null
@@ -1,210 +0,0 @@
-/*
- * Freescale eSDHC i.MX controller driver for the platform bus.
- *
- * derived from the OF-version.
- *
- * Copyright (c) 2010 Pengutronix e.K.
- *   Author: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License.
- */
-
-#include <linux/io.h>
-#include <linux/delay.h>
-#include <linux/err.h>
-#include <linux/clk.h>
-#include <linux/mmc/host.h>
-#include <linux/mmc/sdhci-pltfm.h>
-#include <mach/hardware.h>
-#include "sdhci.h"
-#include "sdhci-pltfm.h"
-#include "sdhci-esdhc.h"
-
-static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
-{
-	void __iomem *base = host->ioaddr + (reg & ~0x3);
-	u32 shift = (reg & 0x3) * 8;
-
-	writel(((readl(base) & ~(mask << shift)) | (val << shift)), base);
-}
-
-static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
-{
-	if (unlikely(reg == SDHCI_HOST_VERSION))
-		reg ^= 2;
-
-	return readw(host->ioaddr + reg);
-}
-
-static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	switch (reg) {
-	case SDHCI_TRANSFER_MODE:
-		/*
-		 * Postpone this write, we must do it together with a
-		 * command write that is down below.
-		 */
-		pltfm_host->scratchpad = val;
-		return;
-	case SDHCI_COMMAND:
-		writel(val << 16 | pltfm_host->scratchpad,
-			host->ioaddr + SDHCI_TRANSFER_MODE);
-		return;
-	case SDHCI_BLOCK_SIZE:
-		val &= ~SDHCI_MAKE_BLKSZ(0x7, 0);
-		break;
-	}
-	esdhc_clrset_le(host, 0xffff, val, reg);
-}
-
-static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
-{
-	u32 new_val;
-
-	switch (reg) {
-	case SDHCI_POWER_CONTROL:
-		/*
-		 * FSL put some DMA bits here
-		 * If your board has a regulator, code should be here
-		 */
-		return;
-	case SDHCI_HOST_CONTROL:
-		/* FSL messed up here, so we can just keep those two */
-		new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS);
-		/* ensure the endianess */
-		new_val |= ESDHC_HOST_CONTROL_LE;
-		/* DMA mode bits are shifted */
-		new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
-
-		esdhc_clrset_le(host, 0xffff, new_val, reg);
-		return;
-	}
-	esdhc_clrset_le(host, 0xff, val, reg);
-}
-
-static unsigned int esdhc_pltfm_get_max_clock(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	return clk_get_rate(pltfm_host->clk);
-}
-
-static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	return clk_get_rate(pltfm_host->clk) / 256 / 16;
-}
-
-static struct sdhci_ops sdhci_esdhc_ops = {
-	.read_w = esdhc_readw_le,
-	.write_w = esdhc_writew_le,
-	.write_b = esdhc_writeb_le,
-	.set_clock = esdhc_set_clock,
-	.get_max_clock = esdhc_pltfm_get_max_clock,
-	.get_min_clock = esdhc_pltfm_get_min_clock,
-};
-
-static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
-	/* ADMA has issues. Might be fixable */
-	.ops = &sdhci_esdhc_ops,
-};
-
-static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
-{
-	struct sdhci_pltfm_host *pltfm_host;
-	struct sdhci_host *host;
-	struct clk *clk;
-	int ret;
-
-	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_imx_pdata);
-	if (!host)
-		return -ENOMEM;
-
-	pltfm_host = sdhci_priv(host);
-
-	clk = clk_get(mmc_dev(host->mmc), NULL);
-	if (IS_ERR(clk)) {
-		dev_err(mmc_dev(host->mmc), "clk err\n");
-		ret = PTR_ERR(clk);
-		goto err_clk_get;
-	}
-	clk_enable(clk);
-	pltfm_host->clk = clk;
-
-	if (cpu_is_mx35() || cpu_is_mx51())
-		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
-
-	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
-	if (cpu_is_mx25() || cpu_is_mx35())
-		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
-
-	ret = sdhci_add_host(host);
-	if (ret)
-		goto err_add_host;
-
-	return 0;
-
-err_add_host:
-	clk_disable(pltfm_host->clk);
-	clk_put(pltfm_host->clk);
-err_clk_get:
-	sdhci_pltfm_free(pdev);
-	return ret;
-}
-
-static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
-{
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	int dead = 0;
-	u32 scratch;
-
-	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
-	if (scratch == (u32)-1)
-		dead = 1;
-
-	sdhci_remove_host(host, dead);
-
-	clk_disable(pltfm_host->clk);
-	clk_put(pltfm_host->clk);
-
-	sdhci_pltfm_free(pdev);
-
-	return 0;
-}
-
-static struct platform_driver sdhci_esdhc_imx_driver = {
-	.driver		= {
-		.name	= "sdhci-esdhc-imx",
-		.owner	= THIS_MODULE,
-	},
-	.probe		= sdhci_esdhc_imx_probe,
-	.remove		= __devexit_p(sdhci_esdhc_imx_remove),
-#ifdef CONFIG_PM
-	.suspend	= sdhci_pltfm_suspend,
-	.resume		= sdhci_pltfm_resume,
-#endif
-};
-
-static int __init sdhci_esdhc_imx_init(void)
-{
-	return platform_driver_register(&sdhci_esdhc_imx_driver);
-}
-
-static void __exit sdhci_esdhc_imx_exit(void)
-{
-	platform_driver_unregister(&sdhci_esdhc_imx_driver);
-}
-
-module_init(sdhci_esdhc_imx_init);
-module_exit(sdhci_esdhc_imx_exit);
-
-MODULE_DESCRIPTION("SDHCI driver for i.MX eSDHC");
-MODULE_AUTHOR("Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
new file mode 100644
index 0000000..b3d1bc1
--- /dev/null
+++ b/drivers/mmc/host/sdhci-esdhc.c
@@ -0,0 +1,413 @@
+/*
+ * Freescale eSDHC controller driver for MPCxxx and i.MX.
+ *
+ * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ *   Author: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
+ *
+ * Copyright (c) 2009 MontaVista Software, Inc.
+ *   Author: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
+ *
+ * Copyright (c) 2010 Pengutronix e.K.
+ *   Author: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ */
+
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/sdhci-pltfm.h>
+#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
+#include <mach/hardware.h>
+#endif
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+/*
+ * Ops and quirks for the Freescale eSDHC controller.
+ */
+
+#define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
+				SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
+				SDHCI_QUIRK_NO_BUSY_IRQ | \
+				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
+				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
+				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
+				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
+				SDHCI_QUIRK_NO_CARD_NO_RESET)
+
+#define ESDHC_SYSTEM_CONTROL	0x2c
+#define ESDHC_CLOCK_MASK	0x0000fff0
+#define ESDHC_PREDIV_SHIFT	8
+#define ESDHC_DIVIDER_SHIFT	4
+#define ESDHC_CLOCK_PEREN	0x00000004
+#define ESDHC_CLOCK_HCKEN	0x00000002
+#define ESDHC_CLOCK_IPGEN	0x00000001
+
+/* pltfm-specific */
+#define ESDHC_HOST_CONTROL_LE	0x20
+
+/* OF-specific */
+#define ESDHC_DMA_SYSCTL	0x40c
+#define ESDHC_DMA_SNOOP		0x00000040
+
+#define ESDHC_HOST_CONTROL_RES	0x05
+
+static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	int pre_div = 2;
+	int div = 1;
+	u32 temp;
+
+	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
+	temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
+		| ESDHC_CLOCK_MASK);
+	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
+
+	if (clock == 0)
+		goto out;
+
+	while (host->max_clk / pre_div / 16 > clock && pre_div < 256)
+		pre_div *= 2;
+
+	while (host->max_clk / pre_div / div > clock && div < 16)
+		div++;
+
+	dev_dbg(mmc_dev(host->mmc), "desired SD clock: %d, actual: %d\n",
+		clock, host->max_clk / pre_div / div);
+
+	pre_div >>= 1;
+	div--;
+
+	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
+	temp |= (ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
+		| (div << ESDHC_DIVIDER_SHIFT)
+		| (pre_div << ESDHC_PREDIV_SHIFT));
+	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
+	mdelay(100);
+out:
+	host->clock = clock;
+}
+
+#ifdef CONFIG_MMC_SDHCI_ESDHC_MPC
+static u16 esdhc_readw(struct sdhci_host *host, int reg)
+{
+	u16 ret;
+
+	if (unlikely(reg == SDHCI_HOST_VERSION))
+		ret = in_be16(host->ioaddr + reg);
+	else
+		ret = sdhci_be32bs_readw(host, reg);
+	return ret;
+}
+
+static void esdhc_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	if (reg == SDHCI_BLOCK_SIZE) {
+		/*
+		 * Two last DMA bits are reserved, and first one is used for
+		 * non-standard blksz of 4096 bytes that we don't support
+		 * yet. So clear the DMA boundary bits.
+		 */
+		val &= ~SDHCI_MAKE_BLKSZ(0x7, 0);
+	}
+	sdhci_be32bs_writew(host, val, reg);
+}
+
+static void esdhc_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	/* Prevent SDHCI core from writing reserved bits (e.g. HISPD). */
+	if (reg == SDHCI_HOST_CONTROL)
+		val &= ~ESDHC_HOST_CONTROL_RES;
+	sdhci_be32bs_writeb(host, val, reg);
+}
+
+static int esdhc_mpc_enable_dma(struct sdhci_host *host)
+{
+	setbits32(host->ioaddr + ESDHC_DMA_SYSCTL, ESDHC_DMA_SNOOP);
+	return 0;
+}
+
+static unsigned int esdhc_mpc_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return pltfm_host->clock;
+}
+
+static unsigned int esdhc_mpc_get_min_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return pltfm_host->clock / 256 / 16;
+}
+
+static struct sdhci_ops sdhci_esdhc_mpc_ops = {
+	.read_l = sdhci_be32bs_readl,
+	.read_w = esdhc_readw,
+	.read_b = sdhci_be32bs_readb,
+	.write_l = sdhci_be32bs_writel,
+	.write_w = esdhc_writew,
+	.write_b = esdhc_writeb,
+	.set_clock = esdhc_set_clock,
+	.enable_dma = esdhc_mpc_enable_dma,
+	.get_max_clock = esdhc_mpc_get_max_clock,
+	.get_min_clock = esdhc_mpc_get_min_clock,
+};
+
+static struct sdhci_pltfm_data sdhci_esdhc_mpc_pdata = {
+	.quirks = ESDHC_DEFAULT_QUIRKS,
+	.ops = &sdhci_esdhc_mpc_ops,
+};
+#endif
+
+#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
+static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
+{
+	void __iomem *base = host->ioaddr + (reg & ~0x3);
+	u32 shift = (reg & 0x3) * 8;
+
+	writel(((readl(base) & ~(mask << shift)) | (val << shift)), base);
+}
+
+static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
+{
+	if (unlikely(reg == SDHCI_HOST_VERSION))
+		reg ^= 2;
+
+	return readw(host->ioaddr + reg);
+}
+
+static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	switch (reg) {
+	case SDHCI_TRANSFER_MODE:
+		/*
+		 * Postpone this write, we must do it together with a
+		 * command write that is down below.
+		 */
+		pltfm_host->scratchpad = val;
+		return;
+	case SDHCI_COMMAND:
+		writel(val << 16 | pltfm_host->scratchpad,
+			host->ioaddr + SDHCI_TRANSFER_MODE);
+		return;
+	case SDHCI_BLOCK_SIZE:
+		val &= ~SDHCI_MAKE_BLKSZ(0x7, 0);
+		break;
+	}
+	esdhc_clrset_le(host, 0xffff, val, reg);
+}
+
+static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
+{
+	u32 new_val;
+
+	switch (reg) {
+	case SDHCI_POWER_CONTROL:
+		/*
+		 * FSL put some DMA bits here
+		 * If your board has a regulator, code should be here
+		 */
+		return;
+	case SDHCI_HOST_CONTROL:
+		/* FSL messed up here, so we can just keep those two */
+		new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS);
+		/* ensure the endianess */
+		new_val |= ESDHC_HOST_CONTROL_LE;
+		/* DMA mode bits are shifted */
+		new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
+
+		esdhc_clrset_le(host, 0xffff, new_val, reg);
+		return;
+	}
+	esdhc_clrset_le(host, 0xff, val, reg);
+}
+
+static unsigned int esdhc_imx_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return clk_get_rate(pltfm_host->clk);
+}
+
+static unsigned int esdhc_imx_get_min_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return clk_get_rate(pltfm_host->clk) / 256 / 16;
+}
+
+static struct sdhci_ops sdhci_esdhc_imx_ops = {
+	.read_w = esdhc_readw_le,
+	.write_w = esdhc_writew_le,
+	.write_b = esdhc_writeb_le,
+	.set_clock = esdhc_set_clock,
+	.get_max_clock = esdhc_imx_get_max_clock,
+	.get_min_clock = esdhc_imx_get_min_clock,
+};
+
+static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+	/* ADMA has issues. Might be fixable */
+	.ops = &sdhci_esdhc_imx_ops,
+};
+#endif
+
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+static const struct of_device_id sdhci_esdhc_dt_ids[] = {
+#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
+	{ .compatible = "fsl,imx-esdhc", .data = &sdhci_esdhc_imx_pdata },
+#endif
+#ifdef CONFIG_MMC_SDHCI_ESDHC_MPC
+	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_mpc_pdata },
+	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_mpc_pdata },
+	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_mpc_pdata },
+#endif
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, sdhci_esdhc_dt_ids);
+
+static const struct of_device_id *
+sdhci_esdhc_get_of_device_id(struct platform_device *pdev)
+{
+	return of_match_device(sdhci_esdhc_dt_ids, &pdev->dev);
+}
+#else
+#define sdhci_esdhc_dt_ids NULL
+static inline struct of_device_id *
+sdhci_esdhc_get_of_device_id(struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
+static const struct platform_device_id sdhci_esdhc_pltfm_ids[] = {
+#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
+	{ "sdhci-esdhc-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata },
+#endif
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, sdhci_esdhc_pltfm_ids);
+
+#ifndef CONFIG_MMC_SDHCI_ESDHC_IMX
+#define cpu_is_mx25()	(0)
+#define cpu_is_mx35()	(0)
+#define cpu_is_mx51()	(0)
+#endif
+
+static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	const struct of_device_id *dtid = sdhci_esdhc_get_of_device_id(pdev);
+	struct sdhci_pltfm_data *pdata;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_host *host;
+	struct clk *clk;
+	int ret;
+
+	if (platid && platid->driver_data)
+		pdata = (void *)platid->driver_data;
+	else if (dtid && dtid->data)
+		pdata = dtid->data;
+	else
+		pdata = pdev->dev.platform_data;
+
+	host = sdhci_pltfm_init(pdev, pdata);
+	if (!host)
+		return -ENOMEM;
+
+	sdhci_get_of_property(pdev);
+
+	pltfm_host = sdhci_priv(host);
+
+	clk = clk_get(mmc_dev(host->mmc), NULL);
+	if (IS_ERR(clk)) {
+		dev_err(mmc_dev(host->mmc), "clk err\n");
+		ret = PTR_ERR(clk);
+		goto err_clk_get;
+	}
+	clk_enable(clk);
+	pltfm_host->clk = clk;
+
+	if (cpu_is_mx35() || cpu_is_mx51())
+		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+
+	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
+	if (cpu_is_mx25() || cpu_is_mx35())
+		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto err_add_host;
+
+	return 0;
+
+err_add_host:
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+err_clk_get:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static int __devexit sdhci_esdhc_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	int dead = 0;
+	u32 scratch;
+
+	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (scratch == (u32)-1)
+		dead = 1;
+
+	sdhci_remove_host(host, dead);
+
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
+static struct platform_driver sdhci_esdhc_driver = {
+	.driver = {
+		.name = "sdhci-esdhc",
+		.owner = THIS_MODULE,
+		.of_match_table = sdhci_esdhc_dt_ids,
+	},
+	.probe = sdhci_esdhc_probe,
+	.remove = __devexit_p(sdhci_esdhc_remove),
+	.id_table = sdhci_esdhc_pltfm_ids,
+#ifdef CONFIG_PM
+	.suspend = sdhci_pltfm_suspend,
+	.resume = sdhci_pltfm_resume,
+#endif
+};
+
+static int __init sdhci_esdhc_init(void)
+{
+	return platform_driver_register(&sdhci_esdhc_driver);
+}
+module_init(sdhci_esdhc_init);
+
+static void __exit sdhci_esdhc_exit(void)
+{
+	platform_driver_unregister(&sdhci_esdhc_driver);
+}
+module_exit(sdhci_esdhc_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for Freescale eSDHC");
+MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
+	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>, "
+	      "Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
deleted file mode 100644
index ccd182f..0000000
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ /dev/null
@@ -1,162 +0,0 @@
-/*
- * Freescale eSDHC controller driver.
- *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
- * Copyright (c) 2009 MontaVista Software, Inc.
- *
- * Authors: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
- *	    Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- */
-
-#include <linux/io.h>
-#include <linux/delay.h>
-#include <linux/mmc/host.h>
-#include "sdhci-pltfm.h"
-#include "sdhci.h"
-#include "sdhci-esdhc.h"
-
-static u16 esdhc_readw(struct sdhci_host *host, int reg)
-{
-	u16 ret;
-
-	if (unlikely(reg == SDHCI_HOST_VERSION))
-		ret = in_be16(host->ioaddr + reg);
-	else
-		ret = sdhci_be32bs_readw(host, reg);
-	return ret;
-}
-
-static void esdhc_writew(struct sdhci_host *host, u16 val, int reg)
-{
-	if (reg == SDHCI_BLOCK_SIZE) {
-		/*
-		 * Two last DMA bits are reserved, and first one is used for
-		 * non-standard blksz of 4096 bytes that we don't support
-		 * yet. So clear the DMA boundary bits.
-		 */
-		val &= ~SDHCI_MAKE_BLKSZ(0x7, 0);
-	}
-	sdhci_be32bs_writew(host, val, reg);
-}
-
-static void esdhc_writeb(struct sdhci_host *host, u8 val, int reg)
-{
-	/* Prevent SDHCI core from writing reserved bits (e.g. HISPD). */
-	if (reg == SDHCI_HOST_CONTROL)
-		val &= ~ESDHC_HOST_CONTROL_RES;
-	sdhci_be32bs_writeb(host, val, reg);
-}
-
-static int esdhc_of_enable_dma(struct sdhci_host *host)
-{
-	setbits32(host->ioaddr + ESDHC_DMA_SYSCTL, ESDHC_DMA_SNOOP);
-	return 0;
-}
-
-static unsigned int esdhc_of_get_max_clock(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	return pltfm_host->clock;
-}
-
-static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	return pltfm_host->clock / 256 / 16;
-}
-
-static struct sdhci_ops sdhci_esdhc_ops = {
-	.read_l = sdhci_be32bs_readl,
-	.read_w = esdhc_readw,
-	.read_b = sdhci_be32bs_readb,
-	.write_l = sdhci_be32bs_writel,
-	.write_w = esdhc_writew,
-	.write_b = esdhc_writeb,
-	.set_clock = esdhc_set_clock,
-	.enable_dma = esdhc_of_enable_dma,
-	.get_max_clock = esdhc_of_get_max_clock,
-	.get_min_clock = esdhc_of_get_min_clock,
-};
-
-static struct sdhci_pltfm_data sdhci_esdhc_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS,
-	.ops = &sdhci_esdhc_ops,
-};
-
-static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
-{
-	struct sdhci_host *host;
-	int ret;
-
-	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata);
-	if (!host)
-		return -ENOMEM;
-
-	sdhci_get_of_property(pdev);
-
-	ret = sdhci_add_host(host);
-	if (ret)
-		goto err_add_host;
-
-	return 0;
-
-err_add_host:
-	sdhci_pltfm_free(pdev);
-	return ret;
-}
-
-static int __devexit sdhci_esdhc_remove(struct platform_device *pdev)
-{
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-
-	sdhci_remove_host(host, 0);
-	sdhci_pltfm_free(pdev);
-
-	return 0;
-}
-
-static const struct of_device_id sdhci_esdhc_of_match[] = {
-	{ .compatible = "fsl,mpc8379-esdhc" },
-	{ .compatible = "fsl,mpc8536-esdhc" },
-	{ .compatible = "fsl,esdhc" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, sdhci_esdhc_of_match);
-
-static struct platform_driver sdhci_esdhc_driver = {
-	.driver = {
-		.name = "sdhci-esdhc",
-		.owner = THIS_MODULE,
-		.of_match_table = sdhci_esdhc_of_match,
-	},
-	.probe = sdhci_esdhc_probe,
-	.remove = __devexit_p(sdhci_esdhc_remove),
-#ifdef CONFIG_PM
-	.suspend = sdhci_pltfm_suspend,
-	.resume = sdhci_pltfm_resume,
-#endif
-};
-
-static int __init sdhci_esdhc_init(void)
-{
-	return platform_driver_register(&sdhci_esdhc_driver);
-}
-module_init(sdhci_esdhc_init);
-
-static void __exit sdhci_esdhc_exit(void)
-{
-	platform_driver_unregister(&sdhci_esdhc_driver);
-}
-module_exit(sdhci_esdhc_exit);
-
-MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
-MODULE_AUTHOR("Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, "
-	      "Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 4e998f1..004cc83 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -126,6 +126,8 @@ void sdhci_get_of_property(struct platform_device *pdev)
 			pltfm_host->clock = be32_to_cpup(clk);
 	}
 }
+#else
+void sdhci_get_of_property(struct platform_device *pdev) { }
 #endif
 
 struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 2f38056..05fe25d 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -35,9 +35,7 @@ extern void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg);
 extern void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg);
 #endif
 
-#ifdef CONFIG_OF
 extern void sdhci_get_of_property(struct platform_device *pdev);
-#endif
 
 extern struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 					   struct sdhci_pltfm_data *pdata);
-- 
1.7.1

Re: [PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx

[Grant Likely]

On Fri, Mar 25, 2011 at 04:48:50PM +0800, Shawn Guo wrote:
> This patch is to consolidate SDHCI driver for Freescale eSDHC
> controller found on both MPCxxx and i.MX platforms.  It turns
> sdhci-of-esdhc.c and sdhci-esdhc-imx.c into one sdhci-esdhc.c,
> which gets the same pair of .probe and .remove serving two cases.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/mmc/host/Kconfig           |   38 ++--
>  drivers/mmc/host/Makefile          |    3 +-
>  drivers/mmc/host/sdhci-esdhc-imx.c |  210 ------------------
>  drivers/mmc/host/sdhci-esdhc.c     |  413 ++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci-of-esdhc.c  |  162 --------------

This patch would be easier to review if it was split into two patches;
first rename sdhci-esdhc-imx.c to sdhci-esdhc.c without any changes to
the .c code, and then a second patch to merge the ppc bits into the
imx version.

> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +static const struct of_device_id sdhci_esdhc_dt_ids[] = {
> +#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
> +	{ .compatible = "fsl,imx-esdhc", .data = &sdhci_esdhc_imx_pdata },
> +#endif
> +#ifdef CONFIG_MMC_SDHCI_ESDHC_MPC
> +	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_mpc_pdata },
> +	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_mpc_pdata },
> +	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_mpc_pdata },
> +#endif
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, sdhci_esdhc_dt_ids);
> +
> +static const struct of_device_id *
> +sdhci_esdhc_get_of_device_id(struct platform_device *pdev)
> +{
> +	return of_match_device(sdhci_esdhc_dt_ids, &pdev->dev);

You can add an empty implementation of of_match_device() to
linux/of_device.h.  That would eliminate the need for this function.

g.

Re: [PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx

[Shawn Guo]

On Thu, Mar 31, 2011 at 09:53:12AM -0600, Grant Likely wrote:
> On Fri, Mar 25, 2011 at 04:48:50PM +0800, Shawn Guo wrote:
> > This patch is to consolidate SDHCI driver for Freescale eSDHC
> > controller found on both MPCxxx and i.MX platforms.  It turns
> > sdhci-of-esdhc.c and sdhci-esdhc-imx.c into one sdhci-esdhc.c,
> > which gets the same pair of .probe and .remove serving two cases.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  drivers/mmc/host/Kconfig           |   38 ++--
> >  drivers/mmc/host/Makefile          |    3 +-
> >  drivers/mmc/host/sdhci-esdhc-imx.c |  210 ------------------
> >  drivers/mmc/host/sdhci-esdhc.c     |  413 ++++++++++++++++++++++++++++++++++++
> >  drivers/mmc/host/sdhci-of-esdhc.c  |  162 --------------
> 
> This patch would be easier to review if it was split into two patches;
> first rename sdhci-esdhc-imx.c to sdhci-esdhc.c without any changes to
> the .c code, and then a second patch to merge the ppc bits into the
> imx version.
> 
OK.

> > +#if defined(CONFIG_OF)
> > +#include <linux/of_device.h>
> > +static const struct of_device_id sdhci_esdhc_dt_ids[] = {
> > +#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
> > +	{ .compatible = "fsl,imx-esdhc", .data = &sdhci_esdhc_imx_pdata },
> > +#endif
> > +#ifdef CONFIG_MMC_SDHCI_ESDHC_MPC
> > +	{ .compatible = "fsl,mpc8379-esdhc", .data = &sdhci_esdhc_mpc_pdata },
> > +	{ .compatible = "fsl,mpc8536-esdhc", .data = &sdhci_esdhc_mpc_pdata },
> > +	{ .compatible = "fsl,esdhc", .data = &sdhci_esdhc_mpc_pdata },
> > +#endif
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(platform, sdhci_esdhc_dt_ids);
> > +
> > +static const struct of_device_id *
> > +sdhci_esdhc_get_of_device_id(struct platform_device *pdev)
> > +{
> > +	return of_match_device(sdhci_esdhc_dt_ids, &pdev->dev);
> 
> You can add an empty implementation of of_match_device() to
> linux/of_device.h.  That would eliminate the need for this function.
> 
Will follow what you suggested later to use .of_match pointer newly
added into struct device.

-- 
Regards,
Shawn

Re: [PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 3256 bytes --]

>  config MMC_SDHCI_ESDHC_IMX
> -	bool "SDHCI platform support for the Freescale eSDHC i.MX controller"
> +	bool "SDHCI support for the Freescale eSDHC i.MX controller"
>  	depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
>  	depends on MMC_SDHCI
> -	select MMC_SDHCI_PLTFM
> +	select MMC_SDHCI_ESDHC
>  	select MMC_SDHCI_IO_ACCESSORS
>  	help
> -	  This selects the Freescale eSDHC controller support on the platform
> -	  bus, found on platforms like mx35/51.
> +	  This selects the Freescale eSDHC controller support on platforms
> +	  like mx35/51.

While we are at it, mx25 could be added and mx53 (and you know better
what else will be coming ;))

> diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
> new file mode 100644
> index 0000000..b3d1bc1
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-esdhc.c
> @@ -0,0 +1,413 @@
> +/*
> + * Freescale eSDHC controller driver for MPCxxx and i.MX.
> + *
> + * Copyright (c) 2007 Freescale Semiconductor, Inc.
> + *   Author: Xiaobo Xie <X.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> + *
> + * Copyright (c) 2009 MontaVista Software, Inc.
> + *   Author: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> + *
> + * Copyright (c) 2010 Pengutronix e.K.
> + *   Author: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/clk.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/sdhci-pltfm.h>
> +#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
> +#include <mach/hardware.h>
> +#endif
> +#include "sdhci.h"
> +#include "sdhci-pltfm.h"
> +
> +/*
> + * Ops and quirks for the Freescale eSDHC controller.
> + */
> +
> +#define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
> +				SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
> +				SDHCI_QUIRK_NO_BUSY_IRQ | \
> +				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
> +				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
> +				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
> +				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
> +				SDHCI_QUIRK_NO_CARD_NO_RESET)

These are not the current quirks. Meanwhile BROKEN_CARD_DETECTION is
gone as well as NO_CARD_NO_RESET (at least for imx). My additions to use
GPIOs for card detect and write protect are also not in this version.

[...]

> +static struct sdhci_pltfm_data sdhci_esdhc_mpc_pdata = {
> +	.quirks = ESDHC_DEFAULT_QUIRKS,
> +	.ops = &sdhci_esdhc_mpc_ops,
> +};
> +#endif

Please mark the #endif with comments of the expression they are
depending on, e.g.

#endif /* CONFIG_MMC_SDHCI_ESDHC_MPC */

if it is not immediately visible. That helps readability.

[...]

Phew, due to all these hardware bugs, the driver will get messy, even if
we try hard. Any chances that future revisions of the core will be
updated? :)

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx

[Shawn Guo]

On Tue, Apr 19, 2011 at 12:21:10PM +0200, Wolfram Sang wrote:
> >  config MMC_SDHCI_ESDHC_IMX
> > -	bool "SDHCI platform support for the Freescale eSDHC i.MX controller"
> > +	bool "SDHCI support for the Freescale eSDHC i.MX controller"
> >  	depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
> >  	depends on MMC_SDHCI
> > -	select MMC_SDHCI_PLTFM
> > +	select MMC_SDHCI_ESDHC
> >  	select MMC_SDHCI_IO_ACCESSORS
> >  	help
> > -	  This selects the Freescale eSDHC controller support on the platform
> > -	  bus, found on platforms like mx35/51.
> > +	  This selects the Freescale eSDHC controller support on platforms
> > +	  like mx35/51.
> 
> While we are at it, mx25 could be added and mx53 (and you know better
> what else will be coming ;))
> 
It's a direct copy.  Will update it properly.

> > diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
> > new file mode 100644
> > index 0000000..b3d1bc1
> > --- /dev/null
> > +++ b/drivers/mmc/host/sdhci-esdhc.c
> > @@ -0,0 +1,413 @@
> > +/*
> > + * Freescale eSDHC controller driver for MPCxxx and i.MX.
> > + *
> > + * Copyright (c) 2007 Freescale Semiconductor, Inc.
> > + *   Author: Xiaobo Xie <X.Xie@freescale.com>
> > + *
> > + * Copyright (c) 2009 MontaVista Software, Inc.
> > + *   Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> > + *
> > + * Copyright (c) 2010 Pengutronix e.K.
> > + *   Author: Wolfram Sang <w.sang@pengutronix.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/clk.h>
> > +#include <linux/mmc/host.h>
> > +#include <linux/mmc/sdhci-pltfm.h>
> > +#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
> > +#include <mach/hardware.h>
> > +#endif
> > +#include "sdhci.h"
> > +#include "sdhci-pltfm.h"
> > +
> > +/*
> > + * Ops and quirks for the Freescale eSDHC controller.
> > + */
> > +
> > +#define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
> > +				SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
> > +				SDHCI_QUIRK_NO_BUSY_IRQ | \
> > +				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
> > +				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
> > +				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
> > +				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
> > +				SDHCI_QUIRK_NO_CARD_NO_RESET)
> 
> These are not the current quirks. Meanwhile BROKEN_CARD_DETECTION is
> gone as well as NO_CARD_NO_RESET (at least for imx). My additions to use
> GPIOs for card detect and write protect are also not in this version.
> 
Next version will be based on mmc-next.

> [...]
> 
> > +static struct sdhci_pltfm_data sdhci_esdhc_mpc_pdata = {
> > +	.quirks = ESDHC_DEFAULT_QUIRKS,
> > +	.ops = &sdhci_esdhc_mpc_ops,
> > +};
> > +#endif
> 
> Please mark the #endif with comments of the expression they are
> depending on, e.g.
> 
> #endif /* CONFIG_MMC_SDHCI_ESDHC_MPC */
> 
> if it is not immediately visible. That helps readability.
> 
Ok, since you like it :)

> [...]
> 
> Phew, due to all these hardware bugs, the driver will get messy, even if
> we try hard. Any chances that future revisions of the core will be
> updated? :)
> 
I doubt all these quirks are design bugs but some designs not fully
compliant to SDHC specification, which you still can say bugs :)

-- 
Regards,
Shawn

[PATCH 5/5] mmc: sdhci: merge two sdhci-pltfm.h into one

[Shawn Guo]

The structure sdhci_pltfm_data is not necessarily to be in a public
header like include/linux/mmc/sdhci-pltfm.h, so the patch moves it
into drivers/mmc/host/sdhci-pltfm.h and eliminates the former one.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/host/sdhci-cns3xxx.c |    1 -
 drivers/mmc/host/sdhci-esdhc.c   |    1 -
 drivers/mmc/host/sdhci-pltfm.h   |    6 +++++-
 include/linux/mmc/sdhci-pltfm.h  |   29 -----------------------------
 4 files changed, 5 insertions(+), 32 deletions(-)
 delete mode 100644 include/linux/mmc/sdhci-pltfm.h

diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
index 95b9080..2238d34 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -15,7 +15,6 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/mmc/host.h>
-#include <linux/mmc/sdhci-pltfm.h>
 #include <mach/cns3xxx.h>
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
index b3d1bc1..fd041d9 100644
--- a/drivers/mmc/host/sdhci-esdhc.c
+++ b/drivers/mmc/host/sdhci-esdhc.c
@@ -20,7 +20,6 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/mmc/host.h>
-#include <linux/mmc/sdhci-pltfm.h>
 #ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
 #include <mach/hardware.h>
 #endif
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 05fe25d..e2d143c 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -14,9 +14,13 @@
 #include <linux/clk.h>
 #include <linux/types.h>
 #include <linux/platform_device.h>
-#include <linux/mmc/sdhci-pltfm.h>
 #include <linux/mmc/sdhci.h>
 
+struct sdhci_pltfm_data {
+	struct sdhci_ops *ops;
+	unsigned int quirks;
+};
+
 struct sdhci_pltfm_host {
 	struct clk *clk;
 	u32 scratchpad; /* to handle quirks across io-accessor calls */
diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
deleted file mode 100644
index f1c2ac3..0000000
--- a/include/linux/mmc/sdhci-pltfm.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * Platform data declarations for the sdhci-pltfm driver.
- *
- * Copyright (c) 2010 MontaVista Software, LLC.
- *
- * Author: Anton Vorontsov <avorontsov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- */
-
-#ifndef _SDHCI_PLTFM_H
-#define _SDHCI_PLTFM_H
-
-struct sdhci_ops;
-
-/**
- * struct sdhci_pltfm_data - SDHCI platform-specific information & hooks
- * @ops: optional pointer to the platform-provided SDHCI ops
- * @quirks: optional SDHCI quirks
- */
-struct sdhci_pltfm_data {
-	struct sdhci_ops *ops;
-	unsigned int quirks;
-};
-
-#endif /* _SDHCI_PLTFM_H */
-- 
1.7.1

Re: [PATCH 5/5] mmc: sdhci: merge two sdhci-pltfm.h into one

[Grant Likely]

On Fri, Mar 25, 2011 at 04:48:51PM +0800, Shawn Guo wrote:
> The structure sdhci_pltfm_data is not necessarily to be in a public
> header like include/linux/mmc/sdhci-pltfm.h, so the patch moves it
> into drivers/mmc/host/sdhci-pltfm.h and eliminates the former one.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

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

Looks good to me.

> ---
>  drivers/mmc/host/sdhci-cns3xxx.c |    1 -
>  drivers/mmc/host/sdhci-esdhc.c   |    1 -
>  drivers/mmc/host/sdhci-pltfm.h   |    6 +++++-
>  include/linux/mmc/sdhci-pltfm.h  |   29 -----------------------------
>  4 files changed, 5 insertions(+), 32 deletions(-)
>  delete mode 100644 include/linux/mmc/sdhci-pltfm.h
> 
> diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
> index 95b9080..2238d34 100644
> --- a/drivers/mmc/host/sdhci-cns3xxx.c
> +++ b/drivers/mmc/host/sdhci-cns3xxx.c
> @@ -15,7 +15,6 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/mmc/host.h>
> -#include <linux/mmc/sdhci-pltfm.h>
>  #include <mach/cns3xxx.h>
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
> diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
> index b3d1bc1..fd041d9 100644
> --- a/drivers/mmc/host/sdhci-esdhc.c
> +++ b/drivers/mmc/host/sdhci-esdhc.c
> @@ -20,7 +20,6 @@
>  #include <linux/err.h>
>  #include <linux/clk.h>
>  #include <linux/mmc/host.h>
> -#include <linux/mmc/sdhci-pltfm.h>
>  #ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
>  #include <mach/hardware.h>
>  #endif
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index 05fe25d..e2d143c 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -14,9 +14,13 @@
>  #include <linux/clk.h>
>  #include <linux/types.h>
>  #include <linux/platform_device.h>
> -#include <linux/mmc/sdhci-pltfm.h>
>  #include <linux/mmc/sdhci.h>
>  
> +struct sdhci_pltfm_data {
> +	struct sdhci_ops *ops;
> +	unsigned int quirks;
> +};
> +
>  struct sdhci_pltfm_host {
>  	struct clk *clk;
>  	u32 scratchpad; /* to handle quirks across io-accessor calls */
> diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
> deleted file mode 100644
> index f1c2ac3..0000000
> --- a/include/linux/mmc/sdhci-pltfm.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/*
> - * Platform data declarations for the sdhci-pltfm driver.
> - *
> - * Copyright (c) 2010 MontaVista Software, LLC.
> - *
> - * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or (at
> - * your option) any later version.
> - */
> -
> -#ifndef _SDHCI_PLTFM_H
> -#define _SDHCI_PLTFM_H
> -
> -struct sdhci_ops;
> -
> -/**
> - * struct sdhci_pltfm_data - SDHCI platform-specific information & hooks
> - * @ops: optional pointer to the platform-provided SDHCI ops
> - * @quirks: optional SDHCI quirks
> - */
> -struct sdhci_pltfm_data {
> -	struct sdhci_ops *ops;
> -	unsigned int quirks;
> -};
> -
> -#endif /* _SDHCI_PLTFM_H */
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 5/5] mmc: sdhci: merge two sdhci-pltfm.h into one

[Wolfram Sang]

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

On Fri, Mar 25, 2011 at 04:48:51PM +0800, Shawn Guo wrote:
> The structure sdhci_pltfm_data is not necessarily to be in a public
> header like include/linux/mmc/sdhci-pltfm.h, so the patch moves it
> into drivers/mmc/host/sdhci-pltfm.h and eliminates the former one.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Shawn Guo]

On Fri, Mar 25, 2011 at 04:48:46PM +0800, Shawn Guo wrote:
> Here are what the patch set does.
> 
> * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
>   a pure common helper function providers.
> * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
>   sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
>   registered with calling helper functions created above.
> * Migrate the use of sdhci_of_host and sdhci_of_data to
>   sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
>   data structure works can be saved, and pltfm version works for both
>   cases.
> * Add OF common helper stuff into sdhci-pltfm.c, and make OF version
>   sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
>   registered as well, so that sdhci-of-core.c and sdhci-of.h can be
>   removed.
> * Consolidate the OF and pltfm esdhc drivers into one with sharing
>   the same pair of .probe and .remove hooks.  As a result,
>   sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
>   sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
> * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
>   drivers/mmc/host/sdhci-pltfm.h.
> 
> And the benefits we gain from the changes are:
> 
> * Get the sdhci device driver follow the Linux trend that driver
>   makes the registration by its own.
> * sdhci-pltfm.c becomes simple and clean as it only has common helper
>   stuff there now.
> * All sdhci device specific things are going back its own driver.
> * The dt and non-dt drivers are consolidated to use the same pair of
>   .probe and .remove hooks.
> * SDHCI driver for Freescale eSDHC controller found on both MPCxxx
>   and i.MX platforms is consolidated to use the same one .probe
>   function.
> 
> The patch set works against the tree below, and was only tested on
> i.mx51 babbage board, all other targets were build tested.
> 
>   git://git.secretlab.ca/git/linux-2.6.git devicetree/test
> 
> Comments are welcomed and appreciated.
> 
> Regards,
> Shawn
> 
> PS: The first patch is a squashing of the patch set below, which was
> posted for review a few days back.
> 
>   [PATCH 0/5] make sdhci device drivers self registered
> 
> Some patches in this series are relatively large, involving more
> changes than expected, I chose to not split considering they are
> logically integral, and doing so can reduce the patch quantity much,
> and make bisect much easier.  But sorry for that it makes reviewers'
> life harder.
> 
> Shawn Guo (5):
>       mmc: sdhci: make sdhci-pltfm device drivers self registered
>       mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data
>       mmc: sdhci: make sdhci-of device drivers self registered
>       mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx
>       mmc: sdhci: merge two sdhci-pltfm.h into one
> 
>  drivers/mmc/host/Kconfig           |   71 ++++---
>  drivers/mmc/host/Makefile          |   17 +-
>  drivers/mmc/host/sdhci-cns3xxx.c   |   68 ++++++-
>  drivers/mmc/host/sdhci-dove.c      |   69 ++++++-
>  drivers/mmc/host/sdhci-esdhc-imx.c |  149 -------------
>  drivers/mmc/host/sdhci-esdhc.c     |  412 ++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci-of-core.c   |  247 ---------------------
>  drivers/mmc/host/sdhci-of-esdhc.c  |   89 --------
>  drivers/mmc/host/sdhci-of-hlwd.c   |   89 +++++++-
>  drivers/mmc/host/sdhci-of.h        |   42 ----
>  drivers/mmc/host/sdhci-pltfm.c     |  251 +++++++++-------------
>  drivers/mmc/host/sdhci-pltfm.h     |   36 +++-
>  drivers/mmc/host/sdhci-tegra.c     |  187 ++++++++++-------
>  include/linux/mmc/sdhci-pltfm.h    |   35 ---
>  14 files changed, 912 insertions(+), 850 deletions(-)
> --

Any comments?  Is this the right direction to move?

-- 
Regards,
Shawn

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Grant Likely]

[-- Attachment #1.1: Type: text/plain, Size: 4286 bytes --]

On Mar 30, 2011 10:23 PM, "Shawn Guo" <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>
> On Fri, Mar 25, 2011 at 04:48:46PM +0800, Shawn Guo wrote:
> > Here are what the patch set does.
> >
> > * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
> >   a pure common helper function providers.
> > * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
> >   sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
> >   registered with calling helper functions created above.
> > * Migrate the use of sdhci_of_host and sdhci_of_data to
> >   sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
> >   data structure works can be saved, and pltfm version works for both
> >   cases.
> > * Add OF common helper stuff into sdhci-pltfm.c, and make OF version
> >   sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
> >   registered as well, so that sdhci-of-core.c and sdhci-of.h can be
> >   removed.
> > * Consolidate the OF and pltfm esdhc drivers into one with sharing
> >   the same pair of .probe and .remove hooks.  As a result,
> >   sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
> >   sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
> > * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
> >   drivers/mmc/host/sdhci-pltfm.h.
> >
> > And the benefits we gain from the changes are:
> >
> > * Get the sdhci device driver follow the Linux trend that driver
> >   makes the registration by its own.
> > * sdhci-pltfm.c becomes simple and clean as it only has common helper
> >   stuff there now.
> > * All sdhci device specific things are going back its own driver.
> > * The dt and non-dt drivers are consolidated to use the same pair of
> >   .probe and .remove hooks.
> > * SDHCI driver for Freescale eSDHC controller found on both MPCxxx
> >   and i.MX platforms is consolidated to use the same one .probe
> >   function.
> >
> > The patch set works against the tree below, and was only tested on
> > i.mx51 babbage board, all other targets were build tested.
> >
> >   git://git.secretlab.ca/git/linux-2.6.git devicetree/test
> >
> > Comments are welcomed and appreciated.
> >
> > Regards,
> > Shawn
> >
> > PS: The first patch is a squashing of the patch set below, which was
> > posted for review a few days back.
> >
> >   [PATCH 0/5] make sdhci device drivers self registered
> >
> > Some patches in this series are relatively large, involving more
> > changes than expected, I chose to not split considering they are
> > logically integral, and doing so can reduce the patch quantity much,
> > and make bisect much easier.  But sorry for that it makes reviewers'
> > life harder.
> >
> > Shawn Guo (5):
> >       mmc: sdhci: make sdhci-pltfm device drivers self registered
> >       mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data
> >       mmc: sdhci: make sdhci-of device drivers self registered
> >       mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx
> >       mmc: sdhci: merge two sdhci-pltfm.h into one
> >
> >  drivers/mmc/host/Kconfig           |   71 ++++---
> >  drivers/mmc/host/Makefile          |   17 +-
> >  drivers/mmc/host/sdhci-cns3xxx.c   |   68 ++++++-
> >  drivers/mmc/host/sdhci-dove.c      |   69 ++++++-
> >  drivers/mmc/host/sdhci-esdhc-imx.c |  149 -------------
> >  drivers/mmc/host/sdhci-esdhc.c     |  412
++++++++++++++++++++++++++++++++++++
> >  drivers/mmc/host/sdhci-of-core.c   |  247 ---------------------
> >  drivers/mmc/host/sdhci-of-esdhc.c  |   89 --------
> >  drivers/mmc/host/sdhci-of-hlwd.c   |   89 +++++++-
> >  drivers/mmc/host/sdhci-of.h        |   42 ----
> >  drivers/mmc/host/sdhci-pltfm.c     |  251 +++++++++-------------
> >  drivers/mmc/host/sdhci-pltfm.h     |   36 +++-
> >  drivers/mmc/host/sdhci-tegra.c     |  187 ++++++++++-------
> >  include/linux/mmc/sdhci-pltfm.h    |   35 ---
> >  14 files changed, 912 insertions(+), 850 deletions(-)
> > --
>
> Any comments?  Is this the right direction to move?

I think it is, but I'll try to get you a full review tomorrow.

g.

>
> --
> Regards,
> Shawn
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

[-- Attachment #1.2: Type: text/html, Size: 5596 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Chris Ball]

Hi Wolfram,

On Fri, Mar 25 2011, Shawn Guo wrote:
> Here are what the patch set does.
>
> * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
>   a pure common helper function providers.
> * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
>   sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
>   registered with calling helper functions created above.
> * Migrate the use of sdhci_of_host and sdhci_of_data to
>   sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
>   data structure works can be saved, and pltfm version works for both
>   cases.
> * Add OF common helper stuff into sdhci-pltfm.c, and make OF version
>   sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
>   registered as well, so that sdhci-of-core.c and sdhci-of.h can be
>   removed.
> * Consolidate the OF and pltfm esdhc drivers into one with sharing
>   the same pair of .probe and .remove hooks.  As a result,
>   sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
>   sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
> * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
>   drivers/mmc/host/sdhci-pltfm.h.

If you get time, would appreciate getting an ACK from you on the patch
changes (including Grant's review).  Thanks very much!

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

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Shawn Guo]

Hi Wolfram,

On Thu, Mar 31, 2011 at 12:36:53PM -0400, Chris Ball wrote:
> Hi Wolfram,
> 
> On Fri, Mar 25 2011, Shawn Guo wrote:
> > Here are what the patch set does.
> >
> > * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
> >   a pure common helper function providers.
> > * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
> >   sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
> >   registered with calling helper functions created above.
> > * Migrate the use of sdhci_of_host and sdhci_of_data to
> >   sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
> >   data structure works can be saved, and pltfm version works for both
> >   cases.
> > * Add OF common helper stuff into sdhci-pltfm.c, and make OF version
> >   sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
> >   registered as well, so that sdhci-of-core.c and sdhci-of.h can be
> >   removed.
> > * Consolidate the OF and pltfm esdhc drivers into one with sharing
> >   the same pair of .probe and .remove hooks.  As a result,
> >   sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
> >   sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
> > * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
> >   drivers/mmc/host/sdhci-pltfm.h.
> 
> If you get time, would appreciate getting an ACK from you on the patch
> changes (including Grant's review).  Thanks very much!
> 
Can you please take some time to help review?  I know you are on ELC
this week :)  What about next couple of weeks?

-- 
Regards,
Shawn

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Wolfram Sang]

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

Hi Shawn,

On Fri, Mar 25, 2011 at 04:48:46PM +0800, Shawn Guo wrote:
> Here are what the patch set does.
> 
> * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
>   a pure common helper function providers.
> * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
>   sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
>   registered with calling helper functions created above.
> * Migrate the use of sdhci_of_host and sdhci_of_data to
>   sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
>   data structure works can be saved, and pltfm version works for both
>   cases.
> * Add OF common helper stuff into sdhci-pltfm.c, and make OF version
>   sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
>   registered as well, so that sdhci-of-core.c and sdhci-of.h can be
>   removed.
> * Consolidate the OF and pltfm esdhc drivers into one with sharing
>   the same pair of .probe and .remove hooks.  As a result,
>   sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
>   sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
> * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
>   drivers/mmc/host/sdhci-pltfm.h.
> 
> And the benefits we gain from the changes are:
> 
> * Get the sdhci device driver follow the Linux trend that driver
>   makes the registration by its own.
> * sdhci-pltfm.c becomes simple and clean as it only has common helper
>   stuff there now.
> * All sdhci device specific things are going back its own driver.
> * The dt and non-dt drivers are consolidated to use the same pair of
>   .probe and .remove hooks.
> * SDHCI driver for Freescale eSDHC controller found on both MPCxxx
>   and i.MX platforms is consolidated to use the same one .probe
>   function.
> 
> The patch set works against the tree below, and was only tested on
> i.mx51 babbage board, all other targets were build tested.
> 
>   git://git.secretlab.ca/git/linux-2.6.git devicetree/test
> 
> Comments are welcomed and appreciated.

First of all, thanks _a lot_ for doing this! Many people have promised
to do such an approach, but you finally made it!

Sorry for the long delay in reviewing it, I didn't want to rush a review
so I needed some time for it which I didn't have much in the last weeks.
Let's hope my battery will have enough power on my flight back to
Germany ;) So, this is for now a purely "visual" review. I hope I can
check V2 on real hardware then.

The approach seems sensible, so have a look at my (mostly minor)
comments inside the patches. However, there is one bigger piece missing.
You converted all the drivers which had a seperate source-file and
hooked into sdhci-pltfm.c. However, those are only those users which
need additional code to work around the quirks. There are also users
which can take the plain pltfm-driver with a properly set
platform_data (check the thread "[PATCH] mmc: add SDHCI driver for STM
platforms (V2)" for an example). Those have to be converted, too.
Now the discussion could be if every of those users gets its own
pltfm-<something>.c or if we create something similat to
sdhci-pltfm-generic, which can also be setup with platform_data like the
old driver (/me likes the latter a bit more. If we don't change the name
of the driver (not talking about the sourcefile) and keep it
"sdhci-pltfm", then you wouldn't need to change all those users if you
ensured it behaves the same.

Also, I think the next version of this series should have all makers of
a sdhci-pltfm user CCed so we give them a chance to report breakage. Or
donate acks or tested-by.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Kyungmin Park]

Hi,

BTW, Are there reason to omit the sdhci-s3c.c? Maybe Mr. Jung will handle it.

Thank you,
Kyungmin Park

On Tue, Apr 19, 2011 at 7:20 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Hi Shawn,
>
> On Fri, Mar 25, 2011 at 04:48:46PM +0800, Shawn Guo wrote:
>> Here are what the patch set does.
>>
>> * Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
>>   a pure common helper function providers.
>> * Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
>>   sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self
>>   registered with calling helper functions created above.
>> * Migrate the use of sdhci_of_host and sdhci_of_data to
>>   sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and
>>   data structure works can be saved, and pltfm version works for both
>>   cases.
>> * Add OF common helper stuff into sdhci-pltfm.c, and make OF version
>>   sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self
>>   registered as well, so that sdhci-of-core.c and sdhci-of.h can be
>>   removed.
>> * Consolidate the OF and pltfm esdhc drivers into one with sharing
>>   the same pair of .probe and .remove hooks.  As a result,
>>   sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while
>>   sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
>> * Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
>>   drivers/mmc/host/sdhci-pltfm.h.
>>
>> And the benefits we gain from the changes are:
>>
>> * Get the sdhci device driver follow the Linux trend that driver
>>   makes the registration by its own.
>> * sdhci-pltfm.c becomes simple and clean as it only has common helper
>>   stuff there now.
>> * All sdhci device specific things are going back its own driver.
>> * The dt and non-dt drivers are consolidated to use the same pair of
>>   .probe and .remove hooks.
>> * SDHCI driver for Freescale eSDHC controller found on both MPCxxx
>>   and i.MX platforms is consolidated to use the same one .probe
>>   function.
>>
>> The patch set works against the tree below, and was only tested on
>> i.mx51 babbage board, all other targets were build tested.
>>
>>   git://git.secretlab.ca/git/linux-2.6.git devicetree/test
>>
>> Comments are welcomed and appreciated.
>
> First of all, thanks _a lot_ for doing this! Many people have promised
> to do such an approach, but you finally made it!
>
> Sorry for the long delay in reviewing it, I didn't want to rush a review
> so I needed some time for it which I didn't have much in the last weeks.
> Let's hope my battery will have enough power on my flight back to
> Germany ;) So, this is for now a purely "visual" review. I hope I can
> check V2 on real hardware then.
>
> The approach seems sensible, so have a look at my (mostly minor)
> comments inside the patches. However, there is one bigger piece missing.
> You converted all the drivers which had a seperate source-file and
> hooked into sdhci-pltfm.c. However, those are only those users which
> need additional code to work around the quirks. There are also users
> which can take the plain pltfm-driver with a properly set
> platform_data (check the thread "[PATCH] mmc: add SDHCI driver for STM
> platforms (V2)" for an example). Those have to be converted, too.
> Now the discussion could be if every of those users gets its own
> pltfm-<something>.c or if we create something similat to
> sdhci-pltfm-generic, which can also be setup with platform_data like the
> old driver (/me likes the latter a bit more. If we don't change the name
> of the driver (not talking about the sourcefile) and keep it
> "sdhci-pltfm", then you wouldn't need to change all those users if you
> ensured it behaves the same.
>
> Also, I think the next version of this series should have all makers of
> a sdhci-pltfm user CCed so we give them a chance to report breakage. Or
> donate acks or tested-by.
>
> Regards,
>
>   Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk2tYe8ACgkQD27XaX1/VRsCRgCgkp+BmJJsLsKllKL0OI/BnO9F
> PRkAn1BpnXBv1exnkJFlqFAPe5O2Yt8w
> =GSqA
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
>

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Wolfram Sang]

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


> BTW, Are there reason to omit the sdhci-s3c.c? Maybe Mr. Jung will handle it.

The difference is that sdhci-s3c.c was more like a fork of sdhci-pltfm while
the others were users of it. With the new interface, s3c can be converted using
pltfm more easily. If somebody is willing to do that, this is very much
welcome. I'd suggest waiting for Shawn's interface to stabilize, though.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Kyungmin Park]

On Wed, Apr 20, 2011 at 2:47 AM, Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>
>> BTW, Are there reason to omit the sdhci-s3c.c? Maybe Mr. Jung will handle it.
>
> The difference is that sdhci-s3c.c was more like a fork of sdhci-pltfm while
> the others were users of it. With the new interface, s3c can be converted using
> pltfm more easily. If somebody is willing to do that, this is very much
> welcome. I'd suggest waiting for Shawn's interface to stabilize, though.
Okay Thanks, we will prepare it.
>
> Regards,
>
>   Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Shawn Guo]

Hi Wolfram,

Thanks for the review.

On Tue, Apr 19, 2011 at 12:20:31PM +0200, Wolfram Sang wrote:
[...]
> The approach seems sensible, so have a look at my (mostly minor)
> comments inside the patches. However, there is one bigger piece missing.
> You converted all the drivers which had a seperate source-file and
> hooked into sdhci-pltfm.c. However, those are only those users which
> need additional code to work around the quirks. There are also users
> which can take the plain pltfm-driver with a properly set
> platform_data (check the thread "[PATCH] mmc: add SDHCI driver for STM
> platforms (V2)" for an example). Those have to be converted, too.

Even those drivers need pltfm-<something>.c to accommodate the
platform_data, right?  I think sdhci-dove.c (sitting on mainline) is
also such an example.  So if I'm not mistaken, I did take care of the
drivers which can take the current plain pltfm-sdhci driver.

> Now the discussion could be if every of those users gets its own
> pltfm-<something>.c or if we create something similat to
> sdhci-pltfm-generic, which can also be setup with platform_data like the
> old driver (/me likes the latter a bit more. If we don't change the name
> of the driver (not talking about the sourcefile) and keep it
> "sdhci-pltfm", then you wouldn't need to change all those users if you
> ensured it behaves the same.
> 
Since there are already pltfm-<something>.c to hold platform_data for
those users anyway, it's not an argument here.

> Also, I think the next version of this series should have all makers of
> a sdhci-pltfm user CCed so we give them a chance to report breakage. Or
> donate acks or tested-by.
> 
Ok, will do.

-- 
Regards,
Shawn

Re: [PATCH 0/5] consolidate sdhci pltfm & OF drivers and get them self registered

[Wolfram Sang]

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

Hi Shawn,

> > The approach seems sensible, so have a look at my (mostly minor)
> > comments inside the patches. However, there is one bigger piece missing.
> > You converted all the drivers which had a seperate source-file and
> > hooked into sdhci-pltfm.c. However, those are only those users which
> > need additional code to work around the quirks. There are also users
> > which can take the plain pltfm-driver with a properly set
> > platform_data (check the thread "[PATCH] mmc: add SDHCI driver for STM
> > platforms (V2)" for an example). Those have to be converted, too.
> 
> Even those drivers need pltfm-<something>.c to accommodate the
> platform_data, right?  I think sdhci-dove.c (sitting on mainline) is
> also such an example.  So if I'm not mistaken, I did take care of the
> drivers which can take the current plain pltfm-sdhci driver.

I stand corrected. I assumed the STM platforms did hit mainline
meanwhile, but they did not. There really doesn't seem to be one user of
sdhci-pltfm without using workarounds. (will make sdhc v3 make it better
or worse? I get fears...) So, everything OK with your series.

> > Now the discussion could be if every of those users gets its own
> > pltfm-<something>.c or if we create something similat to
> > sdhci-pltfm-generic, which can also be setup with platform_data like the
> > old driver (/me likes the latter a bit more. If we don't change the name
> > of the driver (not talking about the sourcefile) and keep it
> > "sdhci-pltfm", then you wouldn't need to change all those users if you
> > ensured it behaves the same.
> > 
> Since there are already pltfm-<something>.c to hold platform_data for
> those users anyway, it's not an argument here.

sdhci-dove needs to overload readw/readl. If there is a user not
needings such, i.e. only plain quirks (or even nothing, what a dream!),
then a generic driver might be worthwhile. Can wait until we see such a
user, though.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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