[PATCH 0/4] Adding support for esdhc on mx35/51

[PATCH 0/4] Adding support for esdhc on mx35/51

[Wolfram Sang]

Hi,

this series adds basic support for the esdhc-controller found on mx35/51-cpus.
It first extends the sdhci-pltfm-driver to have a runtime-structure for data
(1/4), does a bit of cleanup (2/4), extracts a few parts which can be shared
with the OF-version (3/4) and finally adds the driver (4/4).

Note that the support is basic at the moment. Still to be done are:

* ADMA support (Richard Zhu mentioned it has issues and knows the details)

* 8-Bit bus width (needs more research; it breaks some cards for me :( )

* voltage switching using a regulator (I don't have such hardware, hopefully
  Richard can pick this up)

* write_protect using a GPIO (we need to retrieve board-specific data for that;
  an RFC follows after this series)

Platform resources could be posted after write_protect issue has been sorted
out.

Please review and/or give comments. Please let me also know if you intend to
work on a missing bit.

Thanks,

   Wolfram

===

Wolfram Sang (4):
  mmc: sdhci-pltfm: Add structure for host-specific data
  mmc: sdhci-pltfm: move .h-file into apropriate subdir
  mmc: sdhci-of-esdhc: factor out common stuff
  mmc: sdhci-pltfm: add pltfm-driver for imx35/51

 drivers/mmc/host/Kconfig          |    9 +++
 drivers/mmc/host/Makefile         |    1 +
 drivers/mmc/host/sdhci-cns3xxx.c  |    2 +-
 drivers/mmc/host/sdhci-esdhc.c    |  141 +++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-esdhc.h    |   81 +++++++++++++++++++++
 drivers/mmc/host/sdhci-of-esdhc.c |   70 ++----------------
 drivers/mmc/host/sdhci-pltfm.c    |   12 +++-
 drivers/mmc/host/sdhci-pltfm.h    |    8 ++-
 include/linux/mmc/sdhci-pltfm.h   |   35 +++++++++
 include/linux/sdhci-pltfm.h       |   35 ---------
 10 files changed, 292 insertions(+), 102 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-esdhc.c
 create mode 100644 drivers/mmc/host/sdhci-esdhc.h
 create mode 100644 include/linux/mmc/sdhci-pltfm.h
 delete mode 100644 include/linux/sdhci-pltfm.h

[PATCH 1/4] mmc: sdhci-pltfm: Add structure for host-specific data

[Wolfram Sang]

We need to carry some information per host, e.g. the clock. Add a
structure for it and initialize it in the generic part.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mmc/host/sdhci-pltfm.c |    7 +++++--
 drivers/mmc/host/sdhci-pltfm.h |    5 +++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index e045e3c..bf522a1 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -55,6 +55,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
 	const struct platform_device_id *platid = platform_get_device_id(pdev);
 	struct sdhci_host *host;
+	struct sdhci_pltfm_host *pltfm_host;
 	struct resource *iomem;
 	int ret;
 
@@ -72,15 +73,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 			"experience problems.\n");
 
 	if (pdev->dev.parent)
-		host = sdhci_alloc_host(pdev->dev.parent, 0);
+		host = sdhci_alloc_host(pdev->dev.parent, sizeof(*pltfm_host));
 	else
-		host = sdhci_alloc_host(&pdev->dev, 0);
+		host = sdhci_alloc_host(&pdev->dev, sizeof(*pltfm_host));
 
 	if (IS_ERR(host)) {
 		ret = PTR_ERR(host);
 		goto err;
 	}
 
+	pltfm_host = sdhci_priv(host);
+
 	host->hw_name = "platform";
 	if (pdata && pdata->ops)
 		host->ops = pdata->ops;
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 900f329..c393289 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -13,6 +13,11 @@
 
 #include <linux/sdhci-pltfm.h>
 
+struct sdhci_pltfm_host {
+	struct clk *clk;
+	u32 scratchpad; /* to handle quirks across io-accessor calls */
+};
+
 extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
 
 #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
-- 
1.7.1

[PATCH 2/4] mmc: sdhci-pltfm: move .h-file into apropriate subdir

[Wolfram Sang]

Make use of the mmc-directory.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mmc/host/sdhci-cns3xxx.c |    2 +-
 drivers/mmc/host/sdhci-pltfm.c   |    2 +-
 drivers/mmc/host/sdhci-pltfm.h   |    2 +-
 include/linux/mmc/sdhci-pltfm.h  |   35 +++++++++++++++++++++++++++++++++++
 include/linux/sdhci-pltfm.h      |   35 -----------------------------------
 5 files changed, 38 insertions(+), 38 deletions(-)
 create mode 100644 include/linux/mmc/sdhci-pltfm.h
 delete mode 100644 include/linux/sdhci-pltfm.h

diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
index b7050b3..9ebd1d7 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -15,7 +15,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/mmc/host.h>
-#include <linux/sdhci-pltfm.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-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index bf522a1..ff0271c 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -30,7 +30,7 @@
 #include <linux/mmc/host.h>
 
 #include <linux/io.h>
-#include <linux/sdhci-pltfm.h>
+#include <linux/mmc/sdhci-pltfm.h>
 
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index c393289..2b43ed5 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -11,7 +11,7 @@
 #ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
 #define _DRIVERS_MMC_SDHCI_PLTFM_H
 
-#include <linux/sdhci-pltfm.h>
+#include <linux/mmc/sdhci-pltfm.h>
 
 struct sdhci_pltfm_host {
 	struct clk *clk;
diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
new file mode 100644
index 0000000..0239bd7
--- /dev/null
+++ b/include/linux/mmc/sdhci-pltfm.h
@@ -0,0 +1,35 @@
+/*
+ * 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_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);
+	void (*exit)(struct sdhci_host *host);
+};
+
+#endif /* _SDHCI_PLTFM_H */
diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
deleted file mode 100644
index 0239bd7..0000000
--- a/include/linux/sdhci-pltfm.h
+++ /dev/null
@@ -1,35 +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_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);
-	void (*exit)(struct sdhci_host *host);
-};
-
-#endif /* _SDHCI_PLTFM_H */
-- 
1.7.1

[PATCH 3/4] mmc: sdhci-of-esdhc: factor out common stuff

[Wolfram Sang]

Put everything which can be shared between the OF and platform version
of this driver into a local .h-file.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc.h    |   81 +++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-of-esdhc.c |   70 ++++----------------------------
 2 files changed, 89 insertions(+), 62 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-esdhc.h

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
new file mode 100644
index 0000000..cd30a63
--- /dev/null
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -0,0 +1,81 @@
+/*
+ * Freescale eSDHC controller driver generics for OF and pltfm.
+ *
+ * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Copyright (c) 2009 MontaVista Software, Inc.
+ * 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.
+ */
+
+#ifndef _DRIVERS_MMC_SDHCI_ESDHC_H
+#define _DRIVERS_MMC_SDHCI_ESDHC_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 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;
+}
+
+#endif /* _DRIVERS_MMC_SDHCI_ESDHC_H */
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index c8623de..fcd0e1f 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -18,23 +18,7 @@
 #include <linux/mmc/host.h>
 #include "sdhci-of.h"
 #include "sdhci.h"
-
-/*
- * Ops and quirks for the Freescale eSDHC controller.
- */
-
-#define ESDHC_DMA_SYSCTL	0x40c
-#define ESDHC_DMA_SNOOP		0x00000040
-
-#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
-
-#define ESDHC_HOST_CONTROL_RES	0x05
+#include "sdhci-esdhc.h"
 
 static u16 esdhc_readw(struct sdhci_host *host, int reg)
 {
@@ -68,51 +52,20 @@ static void esdhc_writeb(struct sdhci_host *host, u8 val, int reg)
 	sdhci_be32bs_writeb(host, val, reg);
 }
 
-static void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
-{
-	int pre_div = 2;
-	int div = 1;
-
-	clrbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL, ESDHC_CLOCK_IPGEN |
-		  ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN | ESDHC_CLOCK_MASK);
-
-	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--;
-
-	setbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL, ESDHC_CLOCK_IPGEN |
-		  ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN |
-		  div << ESDHC_DIVIDER_SHIFT | pre_div << ESDHC_PREDIV_SHIFT);
-	mdelay(100);
-out:
-	host->clock = clock;
-}
-
-static int esdhc_enable_dma(struct sdhci_host *host)
+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_get_max_clock(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);
 
 	return of_host->clock;
 }
 
-static unsigned int esdhc_get_min_clock(struct sdhci_host *host)
+static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 {
 	struct sdhci_of_host *of_host = sdhci_priv(host);
 
@@ -120,14 +73,7 @@ static unsigned int esdhc_get_min_clock(struct sdhci_host *host)
 }
 
 struct sdhci_of_data sdhci_esdhc = {
-	.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,
+	.quirks = ESDHC_DEFAULT_QUIRKS,
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,
@@ -136,8 +82,8 @@ struct sdhci_of_data sdhci_esdhc = {
 		.write_w = esdhc_writew,
 		.write_b = esdhc_writeb,
 		.set_clock = esdhc_set_clock,
-		.enable_dma = esdhc_enable_dma,
-		.get_max_clock = esdhc_get_max_clock,
-		.get_min_clock = esdhc_get_min_clock,
+		.enable_dma = esdhc_of_enable_dma,
+		.get_max_clock = esdhc_of_get_max_clock,
+		.get_min_clock = esdhc_of_get_min_clock,
 	},
 };
-- 
1.7.1

[PATCH 4/4] mmc: sdhci-pltfm: add pltfm-driver for imx35/51

[Wolfram Sang]

This driver adds basic support for the esdhc-core found on e.g.
imx35/51. It adds up to the pltfm-core.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mmc/host/Kconfig       |    9 +++
 drivers/mmc/host/Makefile      |    1 +
 drivers/mmc/host/sdhci-esdhc.c |  141 ++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-pltfm.c |    3 +
 drivers/mmc/host/sdhci-pltfm.h |    1 +
 5 files changed, 155 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-esdhc.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 6f12d5d..20d03f5 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -143,6 +143,15 @@ config MMC_SDHCI_MV
 
 	  If unsure, say N.
 
+config MMC_SDHCI_ESDHC
+	bool "SDHCI platform support for the Freescale eSDHC controller"
+	depends on MMC_SDHCI_PLTFM
+	select MMC_SDHCI_IO_ACCESSORS
+	help
+	  This selects the Freescale eSDHC controller support.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_S3C
 	tristate "SDHCI support on Samsung S3C SoC"
 	depends on MMC_SDHCI && PLAT_SAMSUNG
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 697bbfe..445d1e5 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -42,6 +42,7 @@ 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)	+= sdhci-esdhc.o
 
 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
new file mode 100644
index 0000000..132e9c9
--- /dev/null
+++ b/drivers/mmc/host/sdhci-esdhc.c
@@ -0,0 +1,141 @@
+/*
+ * Freescale eSDHC controller driver for the platform bus.
+ *
+ * derived from the OF-version.
+ *
+ * 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>
+#include "sdhci.h"
+#include "sdhci-esdhc.h"
+#include "sdhci-pltfm.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 int esdhc_pltfm_init(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct clk *clk;
+
+	clk = clk_get(NULL, "sdhc");
+	if (IS_ERR(clk)) {
+		dev_err(mmc_dev(host->mmc), "clk err\n");
+		return -ENODEV;
+	}
+	clk_enable(clk);
+	pltfm_host->clk = clk;
+
+	return 0;
+}
+
+static void esdhc_pltfm_exit(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+}
+
+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,
+};
+
+struct sdhci_pltfm_data sdhci_esdhc_pdata = {
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_MULTIBLOCK
+			| SDHCI_QUIRK_BROKEN_ADMA,
+	/* ADMA has issues. Might be fixable */
+	/* NO_MULTIBLOCK might be MX35 only (Errata: ENGcm07207) */
+	.ops = &sdhci_esdhc_ops,
+	.init = esdhc_pltfm_init,
+	.exit = esdhc_pltfm_exit,
+};
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index ff0271c..6ba3f3d 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -164,6 +164,9 @@ static const struct platform_device_id sdhci_pltfm_ids[] = {
 #ifdef CONFIG_MMC_SDHCI_CNS3XXX
 	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
 #endif
+#ifdef CONFIG_MMC_SDHCI_ESDHC
+	{ "sdhci-esdhc", (kernel_ulong_t)&sdhci_esdhc_pdata },
+#endif
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 2b43ed5..deb4d27 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -19,5 +19,6 @@ struct sdhci_pltfm_host {
 };
 
 extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
+extern struct sdhci_pltfm_data sdhci_esdhc_pdata;
 
 #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
-- 
1.7.1

[PATCH 3/4] mmc: sdhci-of-esdhc: factor out common stuff

[Anton Vorontsov]

On Tue, Sep 21, 2010 at 02:30:09PM +0200, Wolfram Sang wrote:
> Put everything which can be shared between the OF and platform version
> of this driver into a local .h-file.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
[...]
> +
> +static 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;
> +}

This function is too big for a header. How about moving it
to sdhci-esdhc.c, which would hold common bits for eSDHC?

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 2/4] mmc: sdhci-pltfm: move .h-file into apropriate subdir

[Anton Vorontsov]

On Tue, Sep 21, 2010 at 02:30:08PM +0200, Wolfram Sang wrote:
> Make use of the mmc-directory.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 4/4] mmc: sdhci-pltfm: add pltfm-driver for imx35/51

[Anton Vorontsov]

On Tue, Sep 21, 2010 at 02:30:10PM +0200, Wolfram Sang wrote:
> This driver adds basic support for the esdhc-core found on e.g.
> imx35/51. It adds up to the pltfm-core.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  drivers/mmc/host/Kconfig       |    9 +++
>  drivers/mmc/host/Makefile      |    1 +
>  drivers/mmc/host/sdhci-esdhc.c |  141 ++++++++++++++++++++++++++++++++++++++++

I'd reserve that file for esdhc common code, and move IMX
specific bits to sdhci-esdhc-imx.c.

>  drivers/mmc/host/sdhci-pltfm.c |    3 +
>  drivers/mmc/host/sdhci-pltfm.h |    1 +
>  5 files changed, 155 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/mmc/host/sdhci-esdhc.c
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 6f12d5d..20d03f5 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -143,6 +143,15 @@ config MMC_SDHCI_MV
>  
>  	  If unsure, say N.
>  
> +config MMC_SDHCI_ESDHC
> +	bool "SDHCI platform support for the Freescale eSDHC controller"

Maybe Freescale IMX eSDHC? Let's keep MPC and IMX eSDHC
separate as it's quite possible that they may have different
bugs and limitations.

[...]
> +#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 "sdhci.h"
> +#include "sdhci-esdhc.h"
> +#include "sdhci-pltfm.h"
> +
> +static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
> +{
> +	void __iomem *base = host->ioaddr + (reg & ~0x3);

#include <linux/compiler.h> is needed for __iomem.

[...]
> +static unsigned int esdhc_pltfm_get_max_clock(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

Please put an empty line here.

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

Ditto.

> +	return clk_get_rate(pltfm_host->clk) / 256 / 16;
> +}

[...]
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -164,6 +164,9 @@ static const struct platform_device_id sdhci_pltfm_ids[] = {
>  #ifdef CONFIG_MMC_SDHCI_CNS3XXX
>  	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
>  #endif
> +#ifdef CONFIG_MMC_SDHCI_ESDHC
> +	{ "sdhci-esdhc", (kernel_ulong_t)&sdhci_esdhc_pdata },

sdhci-esdhc-imx, sdhci_esdhc_imx_pdata.

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/4] mmc: sdhci-pltfm: Add structure for host-specific data

[Anton Vorontsov]

On Tue, Sep 21, 2010 at 02:30:07PM +0200, Wolfram Sang wrote:
> We need to carry some information per host, e.g. the clock. Add a
> structure for it and initialize it in the generic part.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
[...]
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index 900f329..c393289 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -13,6 +13,11 @@
>  
>  #include <linux/sdhci-pltfm.h>
>  
> +struct sdhci_pltfm_host {
> +	struct clk *clk;

Forward decl for struct clk is needed.

> +	u32 scratchpad; /* to handle quirks across io-accessor calls */

#include <linux/types.h> is needed for u32.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 2/4] mmc: sdhci-pltfm: move .h-file into apropriate subdir

[Ben Dooks]

On Tue, Sep 21, 2010 at 02:30:08PM +0200, Wolfram Sang wrote:
> Make use of the mmc-directory.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  drivers/mmc/host/sdhci-cns3xxx.c |    2 +-
>  drivers/mmc/host/sdhci-pltfm.c   |    2 +-
>  drivers/mmc/host/sdhci-pltfm.h   |    2 +-
>  include/linux/mmc/sdhci-pltfm.h  |   35 +++++++++++++++++++++++++++++++++++
>  include/linux/sdhci-pltfm.h      |   35 -----------------------------------

hmm, you lost rename detection... best enable it to make these diffs clearer.

>  5 files changed, 38 insertions(+), 38 deletions(-)
>  create mode 100644 include/linux/mmc/sdhci-pltfm.h
>  delete mode 100644 include/linux/sdhci-pltfm.h
> 
> diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
> index b7050b3..9ebd1d7 100644
> --- a/drivers/mmc/host/sdhci-cns3xxx.c
> +++ b/drivers/mmc/host/sdhci-cns3xxx.c
> @@ -15,7 +15,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/mmc/host.h>
> -#include <linux/sdhci-pltfm.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-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index bf522a1..ff0271c 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -30,7 +30,7 @@
>  #include <linux/mmc/host.h>
>  
>  #include <linux/io.h>
> -#include <linux/sdhci-pltfm.h>
> +#include <linux/mmc/sdhci-pltfm.h>
>  
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index c393289..2b43ed5 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -11,7 +11,7 @@
>  #ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
>  #define _DRIVERS_MMC_SDHCI_PLTFM_H
>  
> -#include <linux/sdhci-pltfm.h>
> +#include <linux/mmc/sdhci-pltfm.h>
>  
>  struct sdhci_pltfm_host {
>  	struct clk *clk;
> diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
> new file mode 100644
> index 0000000..0239bd7
> --- /dev/null
> +++ b/include/linux/mmc/sdhci-pltfm.h
> @@ -0,0 +1,35 @@
> +/*
> + * 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_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);
> +	void (*exit)(struct sdhci_host *host);
> +};
> +
> +#endif /* _SDHCI_PLTFM_H */
> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
> deleted file mode 100644
> index 0239bd7..0000000
> --- a/include/linux/sdhci-pltfm.h
> +++ /dev/null
> @@ -1,35 +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_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);
> -	void (*exit)(struct sdhci_host *host);
> -};
> -
> -#endif /* _SDHCI_PLTFM_H */
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH 2/4] mmc: sdhci-pltfm: move .h-file into apropriate subdir

[Wolfram Sang]

On Wed, Sep 22, 2010 at 02:20:11AM +0100, Ben Dooks wrote:
> On Tue, Sep 21, 2010 at 02:30:08PM +0200, Wolfram Sang wrote:
> > Make use of the mmc-directory.
> > 
> > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > ---
> >  drivers/mmc/host/sdhci-cns3xxx.c |    2 +-
> >  drivers/mmc/host/sdhci-pltfm.c   |    2 +-
> >  drivers/mmc/host/sdhci-pltfm.h   |    2 +-
> >  include/linux/mmc/sdhci-pltfm.h  |   35 +++++++++++++++++++++++++++++++++++
> >  include/linux/sdhci-pltfm.h      |   35 -----------------------------------
> 
> hmm, you lost rename detection... best enable it to make these diffs clearer.

Do you mean just for the diffstat or for the whole patch? I am
reluctant to do the latter because you can't apply patches with plain
'patch' anymore. Or are git-patches considered standard meanwhile?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100922/2d2d5aa1/attachment.sig>

[PATCH 0/4] Adding support for esdhc on mx35/51

[Zhu Richard-R65037]

Hi Wolfram:
I have the following a few common comments about these patches.

* About the share between i.MX and PPC eSDHC.
Why configure a few parts be shared between the i.MX eSDHC and PPC
eSDHC? Can they be separated into two independent systems?
It would bring the conveniences to maintain stuff in the future if we
separate them.
And there is already one set of eSDHC driver for all the i.MX SOCs.
As we know that i.MX and PPC are the disparate SOC families.

* About the driver name of i.MX eSDHC driver, I think that use the imx
to identify it is better than esdhc.
As I know that the eSDHC name wouldn't be used anymore in next
generation SOCs of i.MX family.
So the name of imx would be much clear and exact.


Best Regards,
Richard Zhu
Freescale Semiconductor
Tel: +86-021-28937189
Email:Hong-Xing.Zhu at freescale.com 

-----Original Message-----
From: Wolfram Sang [mailto:w.sang at pengutronix.de] 
Sent: Tuesday, 21 September, 2010 20:30
To: linux-mmc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org; Anton Vorontsov; Zhu
Richard-R65037; Wolfram Sang
Subject: [PATCH 0/4] Adding support for esdhc on mx35/51

Hi,

this series adds basic support for the esdhc-controller found on
mx35/51-cpus.
It first extends the sdhci-pltfm-driver to have a runtime-structure for
data (1/4), does a bit of cleanup (2/4), extracts a few parts which can
be shared with the OF-version (3/4) and finally adds the driver (4/4).

Note that the support is basic at the moment. Still to be done are:

* ADMA support (Richard Zhu mentioned it has issues and knows the
details)

* 8-Bit bus width (needs more research; it breaks some cards for me :( )

* voltage switching using a regulator (I don't have such hardware,
hopefully
  Richard can pick this up)

* write_protect using a GPIO (we need to retrieve board-specific data
for that;
  an RFC follows after this series)

Platform resources could be posted after write_protect issue has been
sorted out.

Please review and/or give comments. Please let me also know if you
intend to work on a missing bit.

Thanks,

   Wolfram

===

Wolfram Sang (4):
  mmc: sdhci-pltfm: Add structure for host-specific data
  mmc: sdhci-pltfm: move .h-file into apropriate subdir
  mmc: sdhci-of-esdhc: factor out common stuff
  mmc: sdhci-pltfm: add pltfm-driver for imx35/51

 drivers/mmc/host/Kconfig          |    9 +++
 drivers/mmc/host/Makefile         |    1 +
 drivers/mmc/host/sdhci-cns3xxx.c  |    2 +-
 drivers/mmc/host/sdhci-esdhc.c    |  141
+++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-esdhc.h    |   81 +++++++++++++++++++++
 drivers/mmc/host/sdhci-of-esdhc.c |   70 ++----------------
 drivers/mmc/host/sdhci-pltfm.c    |   12 +++-
 drivers/mmc/host/sdhci-pltfm.h    |    8 ++-
 include/linux/mmc/sdhci-pltfm.h   |   35 +++++++++
 include/linux/sdhci-pltfm.h       |   35 ---------
 10 files changed, 292 insertions(+), 102 deletions(-)  create mode
100644 drivers/mmc/host/sdhci-esdhc.c  create mode 100644
drivers/mmc/host/sdhci-esdhc.h  create mode 100644
include/linux/mmc/sdhci-pltfm.h  delete mode 100644
include/linux/sdhci-pltfm.h

[PATCH 0/4] Adding support for esdhc on mx35/51

[Wolfram Sang]

Hi Richard,

> * About the share between i.MX and PPC eSDHC.
> Why configure a few parts be shared between the i.MX eSDHC and PPC
> eSDHC? Can they be separated into two independent systems?

The rule of thumb is to avoid code-duplication. Assume the PPC driver gets
fixed an off-by-one error in the freq-calculation, you surely want to have that
fix on imx, too.

> It would bring the conveniences to maintain stuff in the future if we
> separate them.

If you can name these conveniences and they are convincing, we can keep them
seperate. At the moment, I don't see them (what doesn't mean they don't exist)

> And there is already one set of eSDHC driver for all the i.MX SOCs.

I am confused: which set do you mean?

> As we know that i.MX and PPC are the disparate SOC families.

Well, if the eSDHC-core itself is the same or very similar, that doesn't really
matter IMO. Especially as the of-platform-bus might disappear somewhen in the
future, the PPC and imx-drivers could become more similar.

> * About the driver name of i.MX eSDHC driver, I think that use the imx
> to identify it is better than esdhc.

I picked up Anton's suggestion 'sdhci-esdhc-imx.c' so far. Although I am still
not sure if 'sdhci-esdhc-pltfm.c' would be more precise.

> As I know that the eSDHC name wouldn't be used anymore in next
> generation SOCs of i.MX family.
> So the name of imx would be much clear and exact.

Not really, because all _existing_ implementations are called eSDHC, no? :) If
some future incarnations have a different name, but behave the same or very
similar, this is usually no problem. Check for example the I2C eeprom driver
which is named 'at24' but support basically all EEPROMs and not only Atmel
ones. Same goes for a couple of RTC-drivers, etc...

Kind regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100924/b4783607/attachment.sig>

[PATCH 0/4] Adding support for esdhc on mx35/51

[Zhu Richard-R65037]

Hi Wolfram:
See my comments.

Best Regards,
Richard Zhu
Freescale Semiconductor
Tel: +86-021-28937189
Email:Hong-Xing.Zhu at freescale.com 


-----Original Message-----
From: Wolfram Sang [mailto:w.sang at pengutronix.de] 
Sent: Friday, 24 September, 2010 14:43
To: Zhu Richard-R65037
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
Anton Vorontsov
Subject: Re: [PATCH 0/4] Adding support for esdhc on mx35/51

Hi Richard,

> * About the share between i.MX and PPC eSDHC.
> Why configure a few parts be shared between the i.MX eSDHC and PPC 
> eSDHC? Can they be separated into two independent systems?

The rule of thumb is to avoid code-duplication. Assume the PPC driver
gets fixed an off-by-one error in the freq-calculation, you surely want
to have that fix on imx, too.

> It would bring the conveniences to maintain stuff in the future if we 
> separate them.

If you can name these conveniences and they are convincing, we can keep
them seperate. At the moment, I don't see them (what doesn't mean they
don't exist)
[<Zhu Richard-r65037>] As Anton's description different eSDHC IP on the
i.MX and the PPC may have the different IC bugs or limitations.
As I know that although in the i.MX SOC family, there are a few
differences between different SOCs. And the behaviors of SW driver may
be impacted by these differences.
I'm afraid that the differences of eSDHC IP module between i.MX and PPC
maybe bigger and bigger in future.

BTW, the block size of the i.MX eSDHC is not forced to 2K size. Up to
now, the default 512byes per sector is used in FSL i.MX Linux BSP.

> And there is already one set of eSDHC driver for all the i.MX SOCs.

I am confused: which set do you mean?
[<Zhu Richard-r65037>] The driver used for i.MX eSDHC. The one used by
i.MX35 in Linux kernel now, and the coming MX51 and so on.
It is better that one driver supports all i.MX SOC's eSDHC modules in
future (MX25, MX35, MX51, MX53...).

> As we know that i.MX and PPC are the disparate SOC families.

Well, if the eSDHC-core itself is the same or very similar, that doesn't
really matter IMO. Especially as the of-platform-bus might disappear
somewhen in the future, the PPC and imx-drivers could become more
similar.

> * About the driver name of i.MX eSDHC driver, I think that use the imx

> to identify it is better than esdhc.

I picked up Anton's suggestion 'sdhci-esdhc-imx.c' so far. Although I am
still not sure if 'sdhci-esdhc-pltfm.c' would be more precise.

> As I know that the eSDHC name wouldn't be used anymore in next 
> generation SOCs of i.MX family.
> So the name of imx would be much clear and exact.

Not really, because all _existing_ implementations are called eSDHC, no?
:) If some future incarnations have a different name, but behave the
same or very similar, this is usually no problem. Check for example the
I2C eeprom driver which is named 'at24' but support basically all
EEPROMs and not only Atmel ones. Same goes for a couple of RTC-drivers,
etc...
[<Zhu Richard-r65037>] :), ok.

Kind regards,

   Wolfram

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

[PATCH 0/4] Adding support for esdhc on mx35/51

[Wolfram Sang]

On Fri, Sep 24, 2010 at 03:08:52PM +0800, Zhu Richard-R65037 wrote:
> Hi Wolfram:
> See my comments.

BTW can't you get your mailer to quote like all others do? It is
inconvenient to read this way.

> > It would bring the conveniences to maintain stuff in the future if we 
> > separate them.
> 
> If you can name these conveniences and they are convincing, we can keep
> them seperate. At the moment, I don't see them (what doesn't mean they
> don't exist)
> [<Zhu Richard-r65037>] As Anton's description different eSDHC IP on the
> i.MX and the PPC may have the different IC bugs or limitations.

Sure thing. I don't see any problem with that as I don't want to merge
them completely, but just to share the common parts. Fixups can be done
in the specific part.

> As I know that although in the i.MX SOC family, there are a few
> differences between different SOCs. And the behaviors of SW driver may
> be impacted by these differences.

If you have even more incarnations of a similar core in the future to
come, that is another reason to share those parts which are in common.
Smaller differences can easily be handled in sdhci-esdhc-imx.c, I think.
For example, if stuff like

	if (cpu_is_mx35())
		quirks |= SDHCI_QUIRKS_NO_MULTIBLOCK;

will do, why would you want to duplicate all the code covering the
non-standard register layouts? Check the imx-spi driver how to handle
variations of a similar core.

> I'm afraid that the differences of eSDHC IP module between i.MX and PPC
> maybe bigger and bigger in future.

If the core of a future IMX99 might be too different, we can have a
custom driver then, but for now, I think a common driver is the way to
go.

> BTW, the block size of the i.MX eSDHC is not forced to 2K size. Up to
> now, the default 512byes per sector is used in FSL i.MX Linux BSP.

It is forced to 2K, because it would be otherwise set to 4K (according
to the cap-register) which is not conform to the spec 2.0. What is the
advantage of 512 byte?

> > And there is already one set of eSDHC driver for all the i.MX SOCs.
> 
> I am confused: which set do you mean?
> [<Zhu Richard-r65037>] The driver used for i.MX eSDHC. The one used by
> i.MX35 in Linux kernel now, and the coming MX51 and so on.
> It is better that one driver supports all i.MX SOC's eSDHC modules in
> future (MX25, MX35, MX51, MX53...).

? Now I am totally confused. That is what I am aiming for :D My driver
was mainly tested on MX35 back then, but was clearly intended to be for
MX51. It was clearly mentioned in the subject "[PATCH 0/4] Adding
support for esdhc on mx35/51". Frankly, I was hoping for a patchset from
you adding the stuff you need for you MX51-board on top of mine, and not
a completely new series. Will have a look at that now...

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100924/7479cd13/attachment-0001.sig>

[PATCH 0/4] Adding support for esdhc on mx35/51

[Zhu Richard-R65037]

Hi Wolfram:
Sorry to bring the confusions to you.
I will use your git repos and make up the missing bits (i.e:the
HOST_CONROL and so on) and verify the func on MX51 BBG boards later.
BTW, Is the URL of your git repos the following one?
git://git.pengutronix.de/git/wsa/linux-2.6.git

Best Regards,
Richard Zhu
Freescale Semiconductor
Tel: +86-021-28937189
Email:Hong-Xing.Zhu at freescale.com 


-----Original Message-----
From: Wolfram Sang [mailto:w.sang at pengutronix.de] 
Sent: Friday, 24 September, 2010 16:58
To: Zhu Richard-R65037
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
Anton Vorontsov
Subject: Re: [PATCH 0/4] Adding support for esdhc on mx35/51

On Fri, Sep 24, 2010 at 03:08:52PM +0800, Zhu Richard-R65037 wrote:
> Hi Wolfram:
> See my comments.

BTW can't you get your mailer to quote like all others do? It is
inconvenient to read this way.

> > It would bring the conveniences to maintain stuff in the future if 
> > we separate them.
> 
> If you can name these conveniences and they are convincing, we can 
> keep them seperate. At the moment, I don't see them (what doesn't mean

> they don't exist) [<Zhu Richard-r65037>] As Anton's description 
> different eSDHC IP on the i.MX and the PPC may have the different IC 
> bugs or limitations.

Sure thing. I don't see any problem with that as I don't want to merge
them completely, but just to share the common parts. Fixups can be done
in the specific part.

> As I know that although in the i.MX SOC family, there are a few 
> differences between different SOCs. And the behaviors of SW driver may

> be impacted by these differences.

If you have even more incarnations of a similar core in the future to
come, that is another reason to share those parts which are in common.
Smaller differences can easily be handled in sdhci-esdhc-imx.c, I think.
For example, if stuff like

	if (cpu_is_mx35())
		quirks |= SDHCI_QUIRKS_NO_MULTIBLOCK;

will do, why would you want to duplicate all the code covering the
non-standard register layouts? Check the imx-spi driver how to handle
variations of a similar core.

> I'm afraid that the differences of eSDHC IP module between i.MX and 
> PPC maybe bigger and bigger in future.

If the core of a future IMX99 might be too different, we can have a
custom driver then, but for now, I think a common driver is the way to
go.

> BTW, the block size of the i.MX eSDHC is not forced to 2K size. Up to 
> now, the default 512byes per sector is used in FSL i.MX Linux BSP.

It is forced to 2K, because it would be otherwise set to 4K (according
to the cap-register) which is not conform to the spec 2.0. What is the
advantage of 512 byte?

> > And there is already one set of eSDHC driver for all the i.MX SOCs.
> 
> I am confused: which set do you mean?
> [<Zhu Richard-r65037>] The driver used for i.MX eSDHC. The one used by
> i.MX35 in Linux kernel now, and the coming MX51 and so on.
> It is better that one driver supports all i.MX SOC's eSDHC modules in 
> future (MX25, MX35, MX51, MX53...).

? Now I am totally confused. That is what I am aiming for :D My driver
was mainly tested on MX35 back then, but was clearly intended to be for
MX51. It was clearly mentioned in the subject "[PATCH 0/4] Adding
support for esdhc on mx35/51". Frankly, I was hoping for a patchset from
you adding the stuff you need for you MX51-board on top of mine, and not
a completely new series. Will have a look at that now...

Regards,

   Wolfram

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

[PATCH 0/4] Adding support for esdhc on mx35/51

[Wolfram Sang]

On Fri, Sep 24, 2010 at 05:40:15PM +0800, Zhu Richard-R65037 wrote:

> Hi Wolfram:
> Sorry to bring the confusions to you.

Well, no worries, maybe I was not clear as well. Hope we have everything
clear now :)

> I will use your git repos and make up the missing bits (i.e:the
> HOST_CONROL and so on) and verify the func on MX51 BBG boards later.
> BTW, Is the URL of your git repos the following one?
> git://git.pengutronix.de/git/wsa/linux-2.6.git

Yes. I pushed out the current status of my branch, but I haven't
implemented all requested changes from Anton yet. Will do on Monday.
If you can't afford to wait for this updated branch, please keep your
changes well separated to reduce merge conflicts.

Thanks!

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100924/7e0bbaf8/attachment.sig>

[PATCH 1/4] mmc: sdhci-pltfm: Add structure for host-specific data

[zhangfei gao]

On Tue, Sep 21, 2010 at 8:30 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> We need to carry some information per host, e.g. the clock. Add a
> structure for it and initialize it in the generic part.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
> ?drivers/mmc/host/sdhci-pltfm.c | ? ?7 +++++--
> ?drivers/mmc/host/sdhci-pltfm.h | ? ?5 +++++
> ?2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index e045e3c..bf522a1 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -55,6 +55,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
> ? ? ? ?struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
> ? ? ? ?const struct platform_device_id *platid = platform_get_device_id(pdev);
> ? ? ? ?struct sdhci_host *host;
> + ? ? ? struct sdhci_pltfm_host *pltfm_host;
> ? ? ? ?struct resource *iomem;
> ? ? ? ?int ret;
>
> @@ -72,15 +73,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
> ? ? ? ? ? ? ? ? ? ? ? ?"experience problems.\n");
>
> ? ? ? ?if (pdev->dev.parent)
> - ? ? ? ? ? ? ? host = sdhci_alloc_host(pdev->dev.parent, 0);
> + ? ? ? ? ? ? ? host = sdhci_alloc_host(pdev->dev.parent, sizeof(*pltfm_host));

Could you help clarify why pdev->dev.parent here, then
mmc_dev(host->mmc)  in child driver is platform, instead of specific
driver itself, is this the design.
> ? ? ? ?else
> - ? ? ? ? ? ? ? host = sdhci_alloc_host(&pdev->dev, 0);
> + ? ? ? ? ? ? ? host = sdhci_alloc_host(&pdev->dev, sizeof(*pltfm_host));
>

> ? ? ? ?if (IS_ERR(host)) {
> ? ? ? ? ? ? ? ?ret = PTR_ERR(host);
> ? ? ? ? ? ? ? ?goto err;
> ? ? ? ?}
>
> + ? ? ? pltfm_host = sdhci_priv(host);
> +
> ? ? ? ?host->hw_name = "platform";
> ? ? ? ?if (pdata && pdata->ops)
> ? ? ? ? ? ? ? ?host->ops = pdata->ops;
> diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
> index 900f329..c393289 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -13,6 +13,11 @@
>
> ?#include <linux/sdhci-pltfm.h>
>
> +struct sdhci_pltfm_host {
> + ? ? ? struct clk *clk;
> + ? ? ? u32 scratchpad; /* to handle quirks across io-accessor calls */
> +};
> +
How about move private data to child driver, and one call back
function alloc_host, then child driver could alloc private data,
including clk.

> ?extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
>
> ?#endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[PATCH 1/4] mmc: sdhci-pltfm: Add structure for host-specific data

[Anton Vorontsov]

Hello,

On Sun, Sep 26, 2010 at 05:02:29AM -0400, zhangfei gao wrote:
> On Tue, Sep 21, 2010 at 8:30 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> > +++ b/drivers/mmc/host/sdhci-pltfm.h
> > @@ -13,6 +13,11 @@
> >
> > ?#include <linux/sdhci-pltfm.h>
> >
> > +struct sdhci_pltfm_host {
> > + ? ? ? struct clk *clk;
> > + ? ? ? u32 scratchpad; /* to handle quirks across io-accessor calls */
> > +};
> > +
> How about move private data to child driver, and one call back
> function alloc_host, then child driver could alloc private data,
> including clk.

IMHO, this sounds like a bit of overdesign/overcomplication.

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2