[PATCH 0/6] Add driver for mx35/51-esdhc-controller (and update sdhci for that)

[PATCH 0/6] Add driver for mx35/51-esdhc-controller (and update sdhci for that)

[Wolfram Sang]

Okay, here is the next version of my patch series. The first four are
updates/improvements for sdhci and sdhci-pltfm and are of generic interest,
too. There were a few people asking for them, so I hope they will now donate
some Acked-by/Tested-by tags. Changes from the previous version are mentioned
in the seperate files. The corresponding branch can be found here:

git://git.pengutronix.de/git/wsa/linux-2.6.git pcm043-mmc

Patches are placed on top of mmc-next as of today.

Thanks,

   Wolfram

Wolfram Sang (6):
  mmc: sdhci-pltfm: Add structure for host-specific data
  mmc: sdhci-pltfm: move .h-file into apropriate subdir
  mmc: sdhci: introduce private get_ro
  mmc: sdhci_pltfm: pass platform_data on custom init-call
  mmc: sdhci-of-esdhc: factor out common stuff
  mmc: sdhci-pltfm: add pltfm-driver for imx35/51

 drivers/mmc/host/Kconfig           |   10 +++
 drivers/mmc/host/Makefile          |    1 +
 drivers/mmc/host/sdhci-cns3xxx.c   |    2 +-
 drivers/mmc/host/sdhci-esdhc-imx.c |  144 ++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-esdhc.h     |   81 ++++++++++++++++++++
 drivers/mmc/host/sdhci-of-esdhc.c  |   70 ++---------------
 drivers/mmc/host/sdhci-pltfm.c     |   23 ++++--
 drivers/mmc/host/sdhci-pltfm.h     |    9 ++-
 drivers/mmc/host/sdhci.c           |   11 ++-
 drivers/mmc/host/sdhci.h           |    1 +
 include/linux/mmc/sdhci-pltfm.h    |   36 +++++++++
 include/linux/sdhci-pltfm.h        |   35 ---------
 12 files changed, 314 insertions(+), 109 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.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/6] 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>
---

Change since last version:

* include clk.h

 drivers/mmc/host/sdhci-pltfm.c |    7 +++++--
 drivers/mmc/host/sdhci-pltfm.h |    6 ++++++
 2 files changed, 11 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..1f96864 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -11,8 +11,14 @@
 #ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
 #define _DRIVERS_MMC_SDHCI_PLTFM_H
 
+#include <linux/clk.h>
 #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

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

[Anton Vorontsov]

On Tue, Sep 28, 2010 at 02:36:41PM +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>
> ---
[...]
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -11,8 +11,14 @@
>  #ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
>  #define _DRIVERS_MMC_SDHCI_PLTFM_H
>  
> +#include <linux/clk.h>
>  #include <linux/sdhci-pltfm.h>
>  
> +struct sdhci_pltfm_host {
> +	struct clk *clk;
> +	u32 scratchpad; /* to handle quirks across io-accessor calls */

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

Otherwise, looks good.

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

Thanks,

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

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

[zhangfei gao]

On Tue, Sep 28, 2010 at 8:36 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>
> ---
>
> Change since last version:
>
> * include clk.h
>
>  drivers/mmc/host/sdhci-pltfm.c |    7 +++++--
>  drivers/mmc/host/sdhci-pltfm.h |    6 ++++++
>  2 files changed, 11 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..1f96864 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -11,8 +11,14 @@
>  #ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
>  #define _DRIVERS_MMC_SDHCI_PLTFM_H
>
> +#include <linux/clk.h>
>  #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
>
>

how about adding one call back of alloc_host, then specific driver
could alloc additional private member, clk and scratchpad may not meet
different device's requirement.

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

[Wolfram Sang]

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

On Tue, Sep 28, 2010 at 06:54:47AM -0700, Philip Rakity wrote:

> taking this off line for a bit (if you don't mind)

Actually, I do. Going back to the list.

> I really like what you are doing but it does not go far enough.  The
> controller I work on is somewhat funny and broken.  It needs to do
> either pre or post processing for some of the calls.

I never claimed that my current patchset should solve all issues. It
deals with some issues I encountered (and then some more, because there
were similar issues on the list), and tries to push things in the right
direction. Most of us are just working on one specific driver, so we
have to live with a slow, step-by-step procedure. I tried to get some
funding for general, large scale cleanup work, but did not get any
response, so far.

> For reset for example --- I need to check if the reset is RESET_DAT
> and handle that differently then RESET_CMD.  If RESET_ALL I need to do
> post processing to restore the controller private registers.

I know the reset-routine probably needs another hook. I think it was
Alan who also mentioned some requirements for it. But besides this, I am
not going to dive deeply into that topic (at least for now). It is too
specific for me at the moment.

Quite frankly, I am a bit overwhelmed by the massive request of guidance
for pltfm-specific drivers and I cannot provide this anymore at this
level as long as I have other work to do (which allows me to buy the
bread). As a result, I will continue reviewing and such but it will take
longer.

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

[PATCH 2/6] 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>
Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 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 1f96864..9a6f8a9 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -12,7 +12,7 @@
 #define _DRIVERS_MMC_SDHCI_PLTFM_H
 
 #include <linux/clk.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/6] mmc: sdhci: introduce private get_ro

[Wolfram Sang]

Some controllers handle their write-protection differently. Introduce a
callback to be able to handle it, ensuring the same locking takes place
for it.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 96c7f60..129958e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1229,14 +1229,17 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		present = 0;
+	else if (host->ops->get_ro)
+		present = host->ops->get_ro(host);
 	else
-		present = sdhci_readl(host, SDHCI_PRESENT_STATE);
+		present = !(sdhci_readl(host, SDHCI_PRESENT_STATE)
+				& SDHCI_WRITE_PROTECT);
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
-	if (host->quirks & SDHCI_QUIRK_INVERTED_WRITE_PROTECT)
-		return !!(present & SDHCI_WRITE_PROTECT);
-	return !(present & SDHCI_WRITE_PROTECT);
+	/* This quirk needs to be replaced by a callback-function later */
+	return host->quirks & SDHCI_QUIRK_INVERTED_WRITE_PROTECT ?
+		!present : present;
 }
 
 static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 112543a..66f83f4 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -336,6 +336,7 @@ struct sdhci_ops {
 	unsigned int	(*get_max_clock)(struct sdhci_host *host);
 	unsigned int	(*get_min_clock)(struct sdhci_host *host);
 	unsigned int	(*get_timeout_clock)(struct sdhci_host *host);
+	unsigned int    (*get_ro)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-- 
1.7.1

[PATCH 4/6] mmc: sdhci_pltfm: pass platform_data on custom init-call

[Wolfram Sang]

The custom init call may need more data to perform its job, so we pass
it a pointer to the platform_data, too. Now, for drivers extending
sdhci-pltfm (like sdhci-cns3xxx), a fixed platform_data is used
according to the platform device id. Because that one does not contain
board specific information, we simply name the board specific
information information priv_pdata and pass it to the init routine as
well. (Note: the old behaviour was that you could override the platform
id specific data with your own. However, one can still do this by using
the "sdhci" id instead of "sdhci-<something>".)

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

Changes since previous version:

Rebased and dropped imx-specific stuff as mentioned by zhangfei gao.

priv_pdata might not be needed as Giuseppe pointed out. Will update with
the next revision of this series, if needed.

 drivers/mmc/host/sdhci-pltfm.c  |   11 ++++++++---
 include/linux/mmc/sdhci-pltfm.h |    3 ++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index ff0271c..4076222 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -52,15 +52,20 @@ static struct sdhci_ops sdhci_pltfm_ops = {
 
 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_pltfm_data *pdata;
+	void *priv_pdata = NULL;
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct resource *iomem;
 	int ret;
 
-	if (!pdata && platid && platid->driver_data)
+	if (platid && platid->driver_data) {
 		pdata = (void *)platid->driver_data;
+		priv_pdata = pdev->dev.platform_data;
+	} else {
+		pdata = pdev->dev.platform_data;
+	}
 
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!iomem) {
@@ -108,7 +113,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 	}
 
 	if (pdata && pdata->init) {
-		ret = pdata->init(host);
+		ret = pdata->init(host, pdata, priv_pdata);
 		if (ret)
 			goto err_plat_init;
 	}
diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
index 0239bd7..cafb96b 100644
--- a/include/linux/mmc/sdhci-pltfm.h
+++ b/include/linux/mmc/sdhci-pltfm.h
@@ -28,7 +28,8 @@ struct sdhci_host;
 struct sdhci_pltfm_data {
 	struct sdhci_ops *ops;
 	unsigned int quirks;
-	int (*init)(struct sdhci_host *host);
+	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
+			void *priv_pdata);
 	void (*exit)(struct sdhci_host *host);
 };
 
-- 
1.7.1

Re: [PATCH 4/6] mmc: sdhci_pltfm: pass platform_data on custom init-call

[Anton Vorontsov]

On Tue, Sep 28, 2010 at 02:36:44PM +0200, Wolfram Sang wrote:
> The custom init call may need more data to perform its job, so we pass
> it a pointer to the platform_data, too. Now, for drivers extending
> sdhci-pltfm (like sdhci-cns3xxx), a fixed platform_data is used
> according to the platform device id. Because that one does not contain
> board specific information, we simply name the board specific
> information information priv_pdata and pass it to the init routine as
> well. (Note: the old behaviour was that you could override the platform
> id specific data with your own. However, one can still do this by using
> the "sdhci" id instead of "sdhci-<something>".)
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
> 
> Changes since previous version:
> 
> Rebased and dropped imx-specific stuff as mentioned by zhangfei gao.
> 
> priv_pdata might not be needed as Giuseppe pointed out. Will update with
> the next revision of this series, if needed.

Yeah, priv_pdata becomes just pdata. Please remove it
from the callback.

Thanks,

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

[PATCH 5/6] 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>
---

Note: Anton suggested to move the function out of the .h-file into a seperate 
.c-file. I tried this and it looked overly complicated to me; we ended up with
a tiny .c _and_ .h file with no gain but a lot of complication added IMHO.
So, while not being perfect, I hope we can live with this version.

 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..277fcb9 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.c"
 
 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

Re: [PATCH 5/6] mmc: sdhci-of-esdhc: factor out common stuff

[Anton Vorontsov]

On Tue, Sep 28, 2010 at 02:36:45PM +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>
> ---
> 
> Note: Anton suggested to move the function out of the .h-file into a seperate 
> .c-file. I tried this and it looked overly complicated to me; we ended up with
> a tiny .c _and_ .h file with no gain but a lot of complication added IMHO.
> So, while not being perfect, I hope we can live with this version.

I'd still like to see it as a C file.

[...]
> +static void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)

But if you do this in the .h file, you have to make it 'inline',
otherwise gcc might emit 'defined but not used' warning.

Thanks,

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

[PATCH 6/6] 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>
---

Changes since last version:

* some "-imx" suffixes added
* some cosmetic improvements

Thanks to Anton for those.

 drivers/mmc/host/Kconfig           |   10 +++
 drivers/mmc/host/Makefile          |    1 +
 drivers/mmc/host/sdhci-esdhc-imx.c |  144 ++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-pltfm.c     |    3 +
 drivers/mmc/host/sdhci-pltfm.h     |    1 +
 5 files changed, 159 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 6f12d5d..815bf0f 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -143,6 +143,16 @@ 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 on the platform
+	  bus, found on platforms like mx35/51.
+
+	  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 ef32c32..5294425 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -38,6 +38,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-imx.o
 
 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
new file mode 100644
index 0000000..c43d448
--- /dev/null
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -0,0 +1,144 @@
+/*
+ * 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-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 int esdhc_pltfm_init(struct sdhci_host *host,
+		struct sdhci_pltfm_data *pdata, void *priv_pdata)
+{
+	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_imx_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 4076222..3e892a8 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -169,6 +169,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-imx", (kernel_ulong_t)&sdhci_esdhc_imx_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 9a6f8a9..efe8ea0 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -20,5 +20,6 @@ struct sdhci_pltfm_host {
 };
 
 extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
+extern struct sdhci_pltfm_data sdhci_esdhc_imx_pdata;
 
 #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
-- 
1.7.1

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

[Anton Vorontsov]

On Tue, Sep 28, 2010 at 02:36:46PM +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>
> ---

Just a few more nits:

> +++ b/drivers/mmc/host/Kconfig
> @@ -143,6 +143,16 @@ config MMC_SDHCI_MV
>  
>  	  If unsure, say N.
>  
> +config MMC_SDHCI_ESDHC

MMC_SDHCI_ESDHC_IMX

> +	bool "SDHCI platform support for the Freescale eSDHC controller"

SDHCI platform support for the Freescale eSDHC i.MX controller

[...]
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -0,0 +1,144 @@
> +/*
> + * Freescale eSDHC controller driver for the platform bus.

eSDHC i.MX

Other than that, looks good.

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

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

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

[zhangfei gao]

On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang <w.sang@pengutronix.de> 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>
> ---
>
> Changes since last version:
>
> * some "-imx" suffixes added
> * some cosmetic improvements
>
> Thanks to Anton for those.
>
>  drivers/mmc/host/Kconfig           |   10 +++
>  drivers/mmc/host/Makefile          |    1 +
>  drivers/mmc/host/sdhci-esdhc-imx.c |  144 ++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci-pltfm.c     |    3 +
>  drivers/mmc/host/sdhci-pltfm.h     |    1 +
>  5 files changed, 159 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 6f12d5d..815bf0f 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -143,6 +143,16 @@ 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 on the platform
> +         bus, found on platforms like mx35/51.
> +
> +         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 ef32c32..5294425 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -38,6 +38,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-imx.o
>
>  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
>  sdhci-of-y                             := sdhci-of-core.o
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> new file mode 100644
> index 0000000..c43d448
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -0,0 +1,144 @@
> +/*
> + * 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-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 int esdhc_pltfm_init(struct sdhci_host *host,
> +               struct sdhci_pltfm_data *pdata, void *priv_pdata)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct clk *clk;
> +
> +       clk = clk_get(NULL, "sdhc");

Here only could support one device, how about several device, for
example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get the
first clock.
Then sdhc.1 sdhc.2 can not work.


> +       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_imx_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 4076222..3e892a8 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -169,6 +169,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-imx", (kernel_ulong_t)&sdhci_esdhc_imx_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 9a6f8a9..efe8ea0 100644
> --- a/drivers/mmc/host/sdhci-pltfm.h
> +++ b/drivers/mmc/host/sdhci-pltfm.h
> @@ -20,5 +20,6 @@ struct sdhci_pltfm_host {
>  };
>
>  extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
> +extern struct sdhci_pltfm_data sdhci_esdhc_imx_pdata;
>
>  #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
> --
> 1.7.1
>
>

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

[Wolfram Sang]

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

> > +static int esdhc_pltfm_init(struct sdhci_host *host,
> > +               struct sdhci_pltfm_data *pdata, void *priv_pdata)
> > +{
> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +       struct clk *clk;
> > +
> > +       clk = clk_get(NULL, "sdhc");
> 
> Here only could support one device, how about several device, for
> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get the
> first clock.
> Then sdhc.1 sdhc.2 can not work.
> 

Ouch, totally overlooked this one. Thanks for spotting it!

-- 
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 6/6] mmc: sdhci-pltfm: add pltfm-driver for imx35/51

[Ben Dooks]

On Tue, Sep 28, 2010 at 10:04:14AM -0400, zhangfei gao wrote:
> On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang <w.sang@pengutronix.de> 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>
> > ---
> >
> > Changes since last version:
> >
> > * some "-imx" suffixes added
> > * some cosmetic improvements
> >
> > Thanks to Anton for those.
> >
> >  drivers/mmc/host/Kconfig           |   10 +++
> >  drivers/mmc/host/Makefile          |    1 +
> >  drivers/mmc/host/sdhci-esdhc-imx.c |  144 ++++++++++++++++++++++++++++++++++++
> >  drivers/mmc/host/sdhci-pltfm.c     |    3 +
> >  drivers/mmc/host/sdhci-pltfm.h     |    1 +
> >  5 files changed, 159 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
> >
> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> > index 6f12d5d..815bf0f 100644
> > --- a/drivers/mmc/host/Kconfig
> > +++ b/drivers/mmc/host/Kconfig
> > @@ -143,6 +143,16 @@ 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 on the platform
> > +         bus, found on platforms like mx35/51.
> > +
> > +         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 ef32c32..5294425 100644
> > --- a/drivers/mmc/host/Makefile
> > +++ b/drivers/mmc/host/Makefile
> > @@ -38,6 +38,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-imx.o
> >
> >  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
> >  sdhci-of-y                             := sdhci-of-core.o
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > new file mode 100644
> > index 0000000..c43d448
> > --- /dev/null
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -0,0 +1,144 @@
> > +/*
> > + * 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-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;
> > +}

the above always confuses me, how about a / (b * c) or similar?

> > +static int esdhc_pltfm_init(struct sdhci_host *host,
> > +               struct sdhci_pltfm_data *pdata, void *priv_pdata)
> > +{
> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +       struct clk *clk;
> > +
> > +       clk = clk_get(NULL, "sdhc");
> 
> Here only could support one device, how about several device, for
> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get the
> first clock.
> Then sdhc.1 sdhc.2 can not work.

No, clk_get() should be passed the device and a NULL for preference.

The main match should be made on the device, and the name is an optional
distinguisher for devices that have multiple clock sources.

> > --- a/drivers/mmc/host/sdhci-pltfm.c
> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> > @@ -169,6 +169,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-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata },
> > +#endif
> >        { },

I'd much prefer to see these outside of the sdhci-pltfm.c, this sort of
#ifdef went out of other drivers with the end of the caveman.

My preference would be to rework the sdhci-pltfm.c to be the 'core'
implementation called by each driver. I'll have a look at posting
some patches to see what can be done to sort this out.

-- 
Ben

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

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

[zhangfei gao]

On Wed, Sep 29, 2010 at 4:57 AM, Ben Dooks <ben@trinity.fluff.org> wrote:
> On Tue, Sep 28, 2010 at 10:04:14AM -0400, zhangfei gao wrote:
>> On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang <w.sang@pengutronix.de> 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>
>> > ---
>> >
>> > Changes since last version:
>> >
>> > * some "-imx" suffixes added
>> > * some cosmetic improvements
>> >
>> > Thanks to Anton for those.
>> >
>> >  drivers/mmc/host/Kconfig           |   10 +++
>> >  drivers/mmc/host/Makefile          |    1 +
>> >  drivers/mmc/host/sdhci-esdhc-imx.c |  144 ++++++++++++++++++++++++++++++++++++
>> >  drivers/mmc/host/sdhci-pltfm.c     |    3 +
>> >  drivers/mmc/host/sdhci-pltfm.h     |    1 +
>> >  5 files changed, 159 insertions(+), 0 deletions(-)
>> >  create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
>> >
>> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> > index 6f12d5d..815bf0f 100644
>> > --- a/drivers/mmc/host/Kconfig
>> > +++ b/drivers/mmc/host/Kconfig
>> > @@ -143,6 +143,16 @@ 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 on the platform
>> > +         bus, found on platforms like mx35/51.
>> > +
>> > +         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 ef32c32..5294425 100644
>> > --- a/drivers/mmc/host/Makefile
>> > +++ b/drivers/mmc/host/Makefile
>> > @@ -38,6 +38,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-imx.o
>> >
>> >  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
>> >  sdhci-of-y                             := sdhci-of-core.o
>> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
>> > new file mode 100644
>> > index 0000000..c43d448
>> > --- /dev/null
>> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> > @@ -0,0 +1,144 @@
>> > +/*
>> > + * 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-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;
>> > +}
>
> the above always confuses me, how about a / (b * c) or similar?
>
>> > +static int esdhc_pltfm_init(struct sdhci_host *host,
>> > +               struct sdhci_pltfm_data *pdata, void *priv_pdata)
>> > +{
>> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> > +       struct clk *clk;
>> > +
>> > +       clk = clk_get(NULL, "sdhc");
>>
>> Here only could support one device, how about several device, for
>> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get the
>> first clock.
>> Then sdhc.1 sdhc.2 can not work.
>
> No, clk_get() should be passed the device and a NULL for preference.
>
> The main match should be made on the device, and the name is an optional
> distinguisher for devices that have multiple clock sources.

Sorry, not understand,
If three devices, sdhci,0, sdhci.1, sdhci.2, each device have
different clock, with different register.
Pass NULL will not distinguish which device, so can not choose which clk.

>
>> > --- a/drivers/mmc/host/sdhci-pltfm.c
>> > +++ b/drivers/mmc/host/sdhci-pltfm.c
>> > @@ -169,6 +169,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-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata },
>> > +#endif
>> >        { },
>
> I'd much prefer to see these outside of the sdhci-pltfm.c, this sort of
> #ifdef went out of other drivers with the end of the caveman.
>
> My preference would be to rework the sdhci-pltfm.c to be the 'core'
> implementation called by each driver. I'll have a look at posting
> some patches to see what can be done to sort this out.
>
> --
> Ben
>
> Q:      What's a light-year?
> A:      One-third less calories than a regular year.
>
>

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

[Zhu Richard-R65037]

> -----Original Message-----
> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
> Sent: Wednesday, September 29, 2010 10:00 AM
> To: Ben Dooks
> Cc: Wolfram Sang; linux-mmc@vger.kernel.org; Anton Vorontsov; Zhu
> Richard-R65037; Philip Rakity; Eric Miao; Haojian Zhuang; Saeed Bishara
> Subject: Re: [PATCH 6/6] mmc: sdhci-pltfm: add pltfm-driver for imx35/51
> 
> On Wed, Sep 29, 2010 at 4:57 AM, Ben Dooks <ben@trinity.fluff.org> wrote:
> > On Tue, Sep 28, 2010 at 10:04:14AM -0400, zhangfei gao wrote:
> >> On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang <w.sang@pengutronix.de>
> 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>
> >> > ---
> >> >
> >> > Changes since last version:
> >> >
> >> > * some "-imx" suffixes added
> >> > * some cosmetic improvements
> >> >
> >> > Thanks to Anton for those.
> >> >
> >> >  drivers/mmc/host/Kconfig           |   10 +++
> >> >  drivers/mmc/host/Makefile          |    1 +
> >> >  drivers/mmc/host/sdhci-esdhc-imx.c |  144
> >> > ++++++++++++++++++++++++++++++++++++
> >> >  drivers/mmc/host/sdhci-pltfm.c     |    3 +
> >> >  drivers/mmc/host/sdhci-pltfm.h     |    1 +
> >> >  5 files changed, 159 insertions(+), 0 deletions(-)
> >> >  create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
> >> >
> >> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> >> > index 6f12d5d..815bf0f 100644
> >> > --- a/drivers/mmc/host/Kconfig
> >> > +++ b/drivers/mmc/host/Kconfig
> >> > @@ -143,6 +143,16 @@ 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 on
> >> > +the platform
> >> > +         bus, found on platforms like mx35/51.
> >> > +
> >> > +         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
> >> > ef32c32..5294425 100644
> >> > --- a/drivers/mmc/host/Makefile
> >> > +++ b/drivers/mmc/host/Makefile
> >> > @@ -38,6 +38,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-imx.o
> >> >
> >> >  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
> >> >  sdhci-of-y                             := sdhci-of-core.o diff
> >> > --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > new file mode 100644
> >> > index 0000000..c43d448
> >> > --- /dev/null
> >> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > @@ -0,0 +1,144 @@
> >> > +/*
> >> > + * 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-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; }
> >
> > the above always confuses me, how about a / (b * c) or similar?
> >
> >> > +static int esdhc_pltfm_init(struct sdhci_host *host,
> >> > +               struct sdhci_pltfm_data *pdata, void *priv_pdata) {
> >> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >> > +       struct clk *clk;
> >> > +
> >> > +       clk = clk_get(NULL, "sdhc");
> >>
> >> Here only could support one device, how about several device, for
> >> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get
> >> the first clock.
> >> Then sdhc.1 sdhc.2 can not work.
> >
> > No, clk_get() should be passed the device and a NULL for preference.
> >
> > The main match should be made on the device, and the name is an
> > optional distinguisher for devices that have multiple clock sources.
> 
> Sorry, not understand,
> If three devices, sdhci,0, sdhci.1, sdhci.2, each device have different
> clock, with different register.
> Pass NULL will not distinguish which device, so can not choose which clk.
> 
Hi:
How about to add such kind of device_find_child func codes that can indicated
 which dev is requesting the clk.
static int __match_sdhci_imx(struct device *dev, void *data)
{
	return !strncmp(dev_name(dev), "imx-sdhci", 9);
}
static int sdhci_imx_init(struct sdhci_host *host) {
	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
	struct clk *clk;
	struct device *dev;

	dev = device_find_child(mmc_dev(host->mmc), NULL, __match_sdhci_imx);
	clk = clk_get(dev, "imx_sdhc_clk");
	if (IS_ERR(clk)) {
		dev_err(mmc_dev(host->mmc), "clk err\n");
		return -ENODEV;
	}
	clk_enable(clk);
	pltfm_host->clk = clk;
	dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(clk));

	return 0;
}
> >
> >> > --- a/drivers/mmc/host/sdhci-pltfm.c
> >> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> >> > @@ -169,6 +169,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-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata
> >> > +}, #endif
> >> >        { },
> >
> > I'd much prefer to see these outside of the sdhci-pltfm.c, this sort
> > of #ifdef went out of other drivers with the end of the caveman.
> >
> > My preference would be to rework the sdhci-pltfm.c to be the 'core'
> > implementation called by each driver. I'll have a look at posting some
> > patches to see what can be done to sort this out.
> >
> > --
> > Ben
> >
> > Q:      What's a light-year?
> > A:      One-third less calories than a regular year.
> >
> >

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

[zhangfei gao]

On Wed, Sep 29, 2010 at 10:27 AM, Zhu Richard-R65037
<r65037@freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
>> Sent: Wednesday, September 29, 2010 10:00 AM
>> To: Ben Dooks
>> Cc: Wolfram Sang; linux-mmc@vger.kernel.org; Anton Vorontsov; Zhu
>> Richard-R65037; Philip Rakity; Eric Miao; Haojian Zhuang; Saeed Bishara
>> Subject: Re: [PATCH 6/6] mmc: sdhci-pltfm: add pltfm-driver for imx35/51
>>
>> On Wed, Sep 29, 2010 at 4:57 AM, Ben Dooks <ben@trinity.fluff.org> wrote:
>> > On Tue, Sep 28, 2010 at 10:04:14AM -0400, zhangfei gao wrote:
>> >> On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang <w.sang@pengutronix.de>
>> 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>
>> >> > ---
>> >> >
>> >> > Changes since last version:
>> >> >
>> >> > * some "-imx" suffixes added
>> >> > * some cosmetic improvements
>> >> >
>> >> > Thanks to Anton for those.
>> >> >
>> >> >  drivers/mmc/host/Kconfig           |   10 +++
>> >> >  drivers/mmc/host/Makefile          |    1 +
>> >> >  drivers/mmc/host/sdhci-esdhc-imx.c |  144
>> >> > ++++++++++++++++++++++++++++++++++++
>> >> >  drivers/mmc/host/sdhci-pltfm.c     |    3 +
>> >> >  drivers/mmc/host/sdhci-pltfm.h     |    1 +
>> >> >  5 files changed, 159 insertions(+), 0 deletions(-)
>> >> >  create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
>> >> >
>> >> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> >> > index 6f12d5d..815bf0f 100644
>> >> > --- a/drivers/mmc/host/Kconfig
>> >> > +++ b/drivers/mmc/host/Kconfig
>> >> > @@ -143,6 +143,16 @@ 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 on
>> >> > +the platform
>> >> > +         bus, found on platforms like mx35/51.
>> >> > +
>> >> > +         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
>> >> > ef32c32..5294425 100644
>> >> > --- a/drivers/mmc/host/Makefile
>> >> > +++ b/drivers/mmc/host/Makefile
>> >> > @@ -38,6 +38,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-imx.o
>> >> >
>> >> >  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
>> >> >  sdhci-of-y                             := sdhci-of-core.o diff
>> >> > --git a/drivers/mmc/host/sdhci-esdhc-imx.c
>> >> > b/drivers/mmc/host/sdhci-esdhc-imx.c
>> >> > new file mode 100644
>> >> > index 0000000..c43d448
>> >> > --- /dev/null
>> >> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> >> > @@ -0,0 +1,144 @@
>> >> > +/*
>> >> > + * 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-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; }
>> >
>> > the above always confuses me, how about a / (b * c) or similar?
>> >
>> >> > +static int esdhc_pltfm_init(struct sdhci_host *host,
>> >> > +               struct sdhci_pltfm_data *pdata, void *priv_pdata) {
>> >> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> >> > +       struct clk *clk;
>> >> > +
>> >> > +       clk = clk_get(NULL, "sdhc");
>> >>
>> >> Here only could support one device, how about several device, for
>> >> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get
>> >> the first clock.
>> >> Then sdhc.1 sdhc.2 can not work.
>> >
>> > No, clk_get() should be passed the device and a NULL for preference.
>> >
>> > The main match should be made on the device, and the name is an
>> > optional distinguisher for devices that have multiple clock sources.
>>
>> Sorry, not understand,
>> If three devices, sdhci,0, sdhci.1, sdhci.2, each device have different
>> clock, with different register.
>> Pass NULL will not distinguish which device, so can not choose which clk.
>>
> Hi:
> How about to add such kind of device_find_child func codes that can indicated
>  which dev is requesting the clk.
> static int __match_sdhci_imx(struct device *dev, void *data)
> {
>        return !strncmp(dev_name(dev), "imx-sdhci", 9);
> }

They are same effect, what you get is only the first one, imx-sdhci.0

> static int sdhci_imx_init(struct sdhci_host *host) {
>        struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>        struct clk *clk;
>        struct device *dev;
>
>        dev = device_find_child(mmc_dev(host->mmc), NULL, __match_sdhci_imx);
>        clk = clk_get(dev, "imx_sdhc_clk");
>        if (IS_ERR(clk)) {
>                dev_err(mmc_dev(host->mmc), "clk err\n");
>                return -ENODEV;
>        }
>        clk_enable(clk);
>        pltfm_host->clk = clk;
>        dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(clk));
>
>        return 0;
> }
>> >
>> >> > --- a/drivers/mmc/host/sdhci-pltfm.c
>> >> > +++ b/drivers/mmc/host/sdhci-pltfm.c
>> >> > @@ -169,6 +169,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-imx", (kernel_ulong_t)&sdhci_esdhc_imx_pdata
>> >> > +}, #endif
>> >> >        { },
>> >
>> > I'd much prefer to see these outside of the sdhci-pltfm.c, this sort
>> > of #ifdef went out of other drivers with the end of the caveman.
>> >
>> > My preference would be to rework the sdhci-pltfm.c to be the 'core'
>> > implementation called by each driver. I'll have a look at posting some
>> > patches to see what can be done to sort this out.
>> >
>> > --
>> > Ben
>> >
>> > Q:      What's a light-year?
>> > A:      One-third less calories than a regular year.
>> >
>> >
>
>
>

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

[Zhu Richard-R65037]

> -----Original Message-----
> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
> Sent: Wednesday, September 29, 2010 11:22 AM
> To: Zhu Richard-R65037
> Cc: Ben Dooks; Wolfram Sang; linux-mmc@vger.kernel.org; Anton Vorontsov;
> Philip Rakity; Eric Miao; Haojian Zhuang; Saeed Bishara
> Subject: Re: [PATCH 6/6] mmc: sdhci-pltfm: add pltfm-driver for imx35/51
> 
> On Wed, Sep 29, 2010 at 10:27 AM, Zhu Richard-R65037
> <r65037@freescale.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
> >> Sent: Wednesday, September 29, 2010 10:00 AM
> >> To: Ben Dooks
> >> Cc: Wolfram Sang; linux-mmc@vger.kernel.org; Anton Vorontsov; Zhu
> >> Richard-R65037; Philip Rakity; Eric Miao; Haojian Zhuang; Saeed
> >> Bishara
> >> Subject: Re: [PATCH 6/6] mmc: sdhci-pltfm: add pltfm-driver for
> >> imx35/51
> >>
> >> On Wed, Sep 29, 2010 at 4:57 AM, Ben Dooks <ben@trinity.fluff.org>
> wrote:
> >> > On Tue, Sep 28, 2010 at 10:04:14AM -0400, zhangfei gao wrote:
> >> >> On Tue, Sep 28, 2010 at 8:36 AM, Wolfram Sang
> >> >> <w.sang@pengutronix.de>
> >> 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>
> >> >> > ---
> >> >> >
> >> >> > Changes since last version:
> >> >> >
> >> >> > * some "-imx" suffixes added
> >> >> > * some cosmetic improvements
> >> >> >
> >> >> > Thanks to Anton for those.
> >> >> >
> >> >> >  drivers/mmc/host/Kconfig           |   10 +++
> >> >> >  drivers/mmc/host/Makefile          |    1 +
> >> >> >  drivers/mmc/host/sdhci-esdhc-imx.c |  144
> >> >> > ++++++++++++++++++++++++++++++++++++
> >> >> >  drivers/mmc/host/sdhci-pltfm.c     |    3 +
> >> >> >  drivers/mmc/host/sdhci-pltfm.h     |    1 +
> >> >> >  5 files changed, 159 insertions(+), 0 deletions(-)
> >> >> >  create mode 100644 drivers/mmc/host/sdhci-esdhc-imx.c
> >> >> >
> >> >> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> >> >> > index 6f12d5d..815bf0f 100644
> >> >> > --- a/drivers/mmc/host/Kconfig
> >> >> > +++ b/drivers/mmc/host/Kconfig
> >> >> > @@ -143,6 +143,16 @@ 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 on
> >> >> > +the platform
> >> >> > +         bus, found on platforms like mx35/51.
> >> >> > +
> >> >> > +         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
> >> >> > ef32c32..5294425 100644
> >> >> > --- a/drivers/mmc/host/Makefile
> >> >> > +++ b/drivers/mmc/host/Makefile
> >> >> > @@ -38,6 +38,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-imx.o
> >> >> >
> >> >> >  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
> >> >> >  sdhci-of-y                             := sdhci-of-core.o diff
> >> >> > --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> >> >> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> >> > new file mode 100644
> >> >> > index 0000000..c43d448
> >> >> > --- /dev/null
> >> >> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> >> > @@ -0,0 +1,144 @@
> >> >> > +/*
> >> >> > + * 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-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; }
> >> >
> >> > the above always confuses me, how about a / (b * c) or similar?
> >> >
> >> >> > +static int esdhc_pltfm_init(struct sdhci_host *host,
> >> >> > +               struct sdhci_pltfm_data *pdata, void
> >> >> > +*priv_pdata) {
> >> >> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >> >> > +       struct clk *clk;
> >> >> > +
> >> >> > +       clk = clk_get(NULL, "sdhc");
> >> >>
> >> >> Here only could support one device, how about several device, for
> >> >> example sdhc.0, sdhc.1 sdhc.2, if using this method, they will get
> >> >> the first clock.
> >> >> Then sdhc.1 sdhc.2 can not work.
> >> >
> >> > No, clk_get() should be passed the device and a NULL for preference.
> >> >
> >> > The main match should be made on the device, and the name is an
> >> > optional distinguisher for devices that have multiple clock sources.
> >>
> >> Sorry, not understand,
> >> If three devices, sdhci,0, sdhci.1, sdhci.2, each device have
> >> different clock, with different register.
> >> Pass NULL will not distinguish which device, so can not choose which
> clk.
> >>
> > Hi:
> > How about to add such kind of device_find_child func codes that can
> > indicated
> >  which dev is requesting the clk.
> > static int __match_sdhci_imx(struct device *dev, void *data) {
> >        return !strncmp(dev_name(dev), "imx-sdhci", 9); }
> 
> They are same effect, what you get is only the first one, imx-sdhci.0

The clk name string can be set and transferred from the private board specified platform data.
We can just use the device_find_child func to find the different dev by dev_id.
(BTW, the dev.parent would be used when allocate the host.)
These example codes are used when I verify the functions on the i.MX51 BBG board based on the platform
Core sdhci-pltfm.c file.
I found that device can't get the proper clk when the first parameter of the clk_get func is NULL.
So I add these codes in my local branch to verify the basic functions on the i.MX51 board.

One more complementary, the different clocks should have the unified interfaces although they have the different
Registers or the configurations.

> 
> > static int sdhci_imx_init(struct sdhci_host *host) {
> >        struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >        struct clk *clk;
> >        struct device *dev;
> >
> >        dev = device_find_child(mmc_dev(host->mmc), NULL,
> > __match_sdhci_imx);
> >        clk = clk_get(dev, "imx_sdhc_clk");
> >        if (IS_ERR(clk)) {
> >                dev_err(mmc_dev(host->mmc), "clk err\n");
> >                return -ENODEV;
> >        }
> >        clk_enable(clk);
> >        pltfm_host->clk = clk;
> >        dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(clk));
> >
> >        return 0;
> > }
> >> >
> >> >> > --- a/drivers/mmc/host/sdhci-pltfm.c
> >> >> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> >> >> > @@ -169,6 +169,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-imx",
> >> >> > +(kernel_ulong_t)&sdhci_esdhc_imx_pdata
> >> >> > +}, #endif
> >> >> >        { },
> >> >
> >> > I'd much prefer to see these outside of the sdhci-pltfm.c, this
> >> > sort of #ifdef went out of other drivers with the end of the caveman.
> >> >
> >> > My preference would be to rework the sdhci-pltfm.c to be the 'core'
> >> > implementation called by each driver. I'll have a look at posting
> >> > some patches to see what can be done to sort this out.
> >> >
> >> > --
> >> > Ben
> >> >
> >> > Q:      What's a light-year?
> >> > A:      One-third less calories than a regular year.
> >> >
> >> >
> >
> >
> >

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

[Wolfram Sang]

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


> My preference would be to rework the sdhci-pltfm.c to be the 'core'
> implementation called by each driver. I'll have a look at posting
> some patches to see what can be done to sort this out.

Cool!

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

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