[patch 2/2] mmc: add support of sdhci-pxa driver

[patch 2/2] mmc: add support of sdhci-pxa driver

[zhangfei gao]

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

Assume the sdhci-pltfm call back are accepted

>From f7e1cc9975cbed2d993d4209eae36f8f300fb18c Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Sun, 26 Sep 2010 17:13:43 -0400
Subject: [PATCH 2/2] mmc: add support of sdhci-pxa driver

	Support Marvell PXA168/PXA910/MMP2 SD Host Controller

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   32 +++++++
 drivers/mmc/host/Kconfig               |   11 +++
 drivers/mmc/host/Makefile              |    1 +
 drivers/mmc/host/sdhci-pxa.c           |  143 ++++++++++++++++++++++++++++++++
 4 files changed, 187 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-pxa/include/plat/sdhci.h
 create mode 100644 drivers/mmc/host/sdhci-pxa.c

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
b/arch/arm/plat-pxa/include/plat/sdhci.h
new file mode 100644
index 0000000..3d5f5ef
--- /dev/null
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -0,0 +1,32 @@
+/* arch/arm/plat-pxa/include/plat/sdhci.h
+ *
+ * Copyright 2010 Marvell
+ *	Zhangfei Gao <zhangfei.gao@marvell.com>
+ *
+ * PXA Platform - SDHCI platform data definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __PLAT_PXA_SDHCI_H
+#define __PLAT_PXA_SDHCI_H __FILE__
+
+/* pxa specific quirks */
+/* Card alwayes wired to host, like emmc */
+#define PXA_QUIRK_BROKEN_CARD_DETECTION (1<<0)
+/* Require clock free running */
+#define	PXA_QUIRK_DISABLE_CLOCK_GATING (1<<1)
+
+/**
+ * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
+ * @max_speed: The maximum speed supported.
+ * @pxa_quirks: specific quirk of pxa
+*/
+struct sdhci_pxa_platdata {
+	unsigned int	max_speed;
+	unsigned int	pxa_quirk;
+};
+
+#endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 6f12d5d..9510976 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -158,6 +158,17 @@ config MMC_SDHCI_S3C

 	  If unsure, say N.

+config MMC_SDHCI_PXA
+	tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
+	depends on ARCH_PXA || ARCH_MMP
+	depends on MMC_SDHCI_PLTFM
+	help
+	  This selects the Marvell(R) PXA168/PXA910/MMP2 SD Host Controller.
+	  If you have a PXA168/PXA910/MMP2 platform with SD Host Controller and a
+	  card slot,say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_SPEAR
 	tristate "SDHCI support on ST SPEAr platform"
 	depends on MMC_SDHCI && PLAT_SPEAR
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index ef32c32..d166493 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_PXA)		+= sdhci-pxa.o

 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
new file mode 100644
index 0000000..12ede18
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -0,0 +1,143 @@
+/* linux/drivers/mmc/host/sdhci-pxa.c
+ *
+ * Copyright 2010 Marvell
+ *      Zhangfei Gao <zhangfei.gao@marvell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/* Supports:
+ * SDHCI support for MMP2/PXA910/PXA168
+ *
+ * Based on  sdhci-pltfm.c
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/mmc/host.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <plat/sdhci.h>
+#include <linux/sdhci-pltfm.h>
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+#define SD_FIFO_PARAM		0x104
+#define DIS_PAD_SD_CLK_GATE	0x400
+
+struct sdhci_pxa {
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+
+	u32 quirks;
+	u8 clk_enable;
+};
+
+/*****************************************************************************\
+ *                                                                           *
+ * SDHCI core callbacks                                                      *
+ *                                                                           *
+\*****************************************************************************/
+static void sdhci_pxa_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+	u32 tmp = 0;
+
+	if (clock == 0) {
+		if (pxa->clk_enable) {
+			clk_disable(pxa->clk);
+			pxa->clk_enable = 0;
+		}
+	} else {
+		if (0 == pxa->clk_enable) {
+			if (pxa->pdata->pxa_quirk
+					& PXA_QUIRK_DISABLE_CLOCK_GATING) {
+				tmp = readl(host->ioaddr + SD_FIFO_PARAM);
+				tmp |= DIS_PAD_SD_CLK_GATE;
+				writel(tmp, host->ioaddr + SD_FIFO_PARAM);
+			}
+			clk_enable(pxa->clk);
+			pxa->clk_enable = 1;
+		}
+	}
+}
+
+static struct sdhci_ops sdhci_pxa_ops = {
+	.set_clock	= sdhci_pxa_set_clock,
+};
+
+/*****************************************************************************\
+ *                                                                           *
+ * sdhci-pltfm callbacks                                                      *
+ *                                                                           *
+\*****************************************************************************/
+static int sdhci_pxa_init(struct sdhci_host *host, struct
sdhci_pltfm_data *pdata, void* priv_pdata)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+	struct device *dev = mmc_dev(host->mmc);
+
+	pxa->pdata = priv_pdata;
+	pxa->clk_enable = 0;
+
+	pxa->clk = clk_get(dev, "PXA-SDHCLK");
+	if (IS_ERR(pxa->clk)) {
+		dev_err(dev, "clk err\n");
+		return -ENODEV;
+	}
+	clk_enable(pxa->clk);
+	dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(pxa->clk));
+
+	pxa->clk_enable = 0;
+	pxa->quirks = pdata->quirks;
+
+	if (pxa->pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
+		pxa->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
+	return 0;
+}
+
+static unsigned int sdhci_pxa_get_quirk(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	return pxa->quirks;
+}
+
+static void sdhci_pxa_set_max_speed(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	if (pxa->pdata->max_speed)
+		host->mmc->f_max = pxa->pdata->max_speed;
+}
+
+static void sdhci_pxa_exit(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	if (pxa->clk_enable)
+		clk_disable(pxa->clk);
+	clk_put(pxa->clk);
+}
+
+static struct sdhci_host *sdhci_pxa_alloc_host(struct device *dev)
+{
+	return sdhci_alloc_host(dev, sizeof(struct sdhci_pxa));
+}
+
+struct sdhci_pltfm_data sdhci_pxa_pdata = {
+	.ops	= &sdhci_pxa_ops,
+	.quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
+	.init	= sdhci_pxa_init,
+	.exit	= sdhci_pxa_exit,
+	.get_quirk	= sdhci_pxa_get_quirk,
+	.set_max_speed	= sdhci_pxa_set_max_speed,
+	.alloc_host	= sdhci_pxa_alloc_host,
+};
+
+MODULE_DESCRIPTION("SDH controller driver for PXA168/PXA910/MMP2");
+MODULE_AUTHOR("Zhangfei Gao <zhangfei.gao@marvell.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.7.0.4

[-- Attachment #2: 0002-mmc-add-support-of-sdhci-pxa-driver.patch --]
[-- Type: text/x-patch, Size: 7328 bytes --]

From f7e1cc9975cbed2d993d4209eae36f8f300fb18c Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Sun, 26 Sep 2010 17:13:43 -0400
Subject: [PATCH 2/2] mmc: add support of sdhci-pxa driver

	Support Marvell PXA168/PXA910/MMP2 SD Host Controller

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   32 +++++++
 drivers/mmc/host/Kconfig               |   11 +++
 drivers/mmc/host/Makefile              |    1 +
 drivers/mmc/host/sdhci-pxa.c           |  143 ++++++++++++++++++++++++++++++++
 4 files changed, 187 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-pxa/include/plat/sdhci.h
 create mode 100644 drivers/mmc/host/sdhci-pxa.c

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h b/arch/arm/plat-pxa/include/plat/sdhci.h
new file mode 100644
index 0000000..3d5f5ef
--- /dev/null
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -0,0 +1,32 @@
+/* arch/arm/plat-pxa/include/plat/sdhci.h
+ *
+ * Copyright 2010 Marvell
+ *	Zhangfei Gao <zhangfei.gao@marvell.com>
+ *
+ * PXA Platform - SDHCI platform data definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __PLAT_PXA_SDHCI_H
+#define __PLAT_PXA_SDHCI_H __FILE__
+
+/* pxa specific quirks */
+/* Card alwayes wired to host, like emmc */
+#define PXA_QUIRK_BROKEN_CARD_DETECTION (1<<0)
+/* Require clock free running */
+#define	PXA_QUIRK_DISABLE_CLOCK_GATING (1<<1)
+
+/**
+ * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
+ * @max_speed: The maximum speed supported.
+ * @pxa_quirks: specific quirk of pxa
+*/
+struct sdhci_pxa_platdata {
+	unsigned int	max_speed;
+	unsigned int	pxa_quirk;
+};
+
+#endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 6f12d5d..9510976 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -158,6 +158,17 @@ config MMC_SDHCI_S3C
 
 	  If unsure, say N.
 
+config MMC_SDHCI_PXA
+	tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
+	depends on ARCH_PXA || ARCH_MMP
+	depends on MMC_SDHCI_PLTFM
+	help
+	  This selects the Marvell(R) PXA168/PXA910/MMP2 SD Host Controller.
+	  If you have a PXA168/PXA910/MMP2 platform with SD Host Controller and a
+	  card slot,say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_SPEAR
 	tristate "SDHCI support on ST SPEAr platform"
 	depends on MMC_SDHCI && PLAT_SPEAR
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index ef32c32..d166493 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_PXA)		+= sdhci-pxa.o
 
 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
new file mode 100644
index 0000000..12ede18
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -0,0 +1,143 @@
+/* linux/drivers/mmc/host/sdhci-pxa.c
+ *
+ * Copyright 2010 Marvell
+ *      Zhangfei Gao <zhangfei.gao@marvell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/* Supports:
+ * SDHCI support for MMP2/PXA910/PXA168
+ *
+ * Based on  sdhci-pltfm.c
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/mmc/host.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <plat/sdhci.h>
+#include <linux/sdhci-pltfm.h>
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+#define SD_FIFO_PARAM		0x104
+#define DIS_PAD_SD_CLK_GATE	0x400
+
+struct sdhci_pxa {
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+
+	u32 quirks;
+	u8 clk_enable;
+};
+
+/*****************************************************************************\
+ *                                                                           *
+ * SDHCI core callbacks                                                      *
+ *                                                                           *
+\*****************************************************************************/
+static void sdhci_pxa_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+	u32 tmp = 0;
+
+	if (clock == 0) {
+		if (pxa->clk_enable) {
+			clk_disable(pxa->clk);
+			pxa->clk_enable = 0;
+		}
+	} else {
+		if (0 == pxa->clk_enable) {
+			if (pxa->pdata->pxa_quirk
+					& PXA_QUIRK_DISABLE_CLOCK_GATING) {
+				tmp = readl(host->ioaddr + SD_FIFO_PARAM);
+				tmp |= DIS_PAD_SD_CLK_GATE;
+				writel(tmp, host->ioaddr + SD_FIFO_PARAM);
+			}
+			clk_enable(pxa->clk);
+			pxa->clk_enable = 1;
+		}
+	}
+}
+
+static struct sdhci_ops sdhci_pxa_ops = {
+	.set_clock	= sdhci_pxa_set_clock,
+};
+
+/*****************************************************************************\
+ *                                                                           *
+ * sdhci-pltfm callbacks                                                      *
+ *                                                                           *
+\*****************************************************************************/
+static int sdhci_pxa_init(struct sdhci_host *host, struct sdhci_pltfm_data *pdata, void* priv_pdata)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+	struct device *dev = mmc_dev(host->mmc);
+
+	pxa->pdata = priv_pdata;
+	pxa->clk_enable = 0;
+
+	pxa->clk = clk_get(dev, "PXA-SDHCLK");
+	if (IS_ERR(pxa->clk)) {
+		dev_err(dev, "clk err\n");
+		return -ENODEV;
+	}
+	clk_enable(pxa->clk);
+	dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(pxa->clk));
+
+	pxa->clk_enable = 0;
+	pxa->quirks = pdata->quirks;
+
+	if (pxa->pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
+		pxa->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
+	return 0;
+}
+
+static unsigned int sdhci_pxa_get_quirk(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	return pxa->quirks;
+}
+
+static void sdhci_pxa_set_max_speed(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	if (pxa->pdata->max_speed)
+		host->mmc->f_max = pxa->pdata->max_speed;
+}
+
+static void sdhci_pxa_exit(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	if (pxa->clk_enable)
+		clk_disable(pxa->clk);
+	clk_put(pxa->clk);
+}
+
+static struct sdhci_host *sdhci_pxa_alloc_host(struct device *dev)
+{
+	return sdhci_alloc_host(dev, sizeof(struct sdhci_pxa));
+}
+
+struct sdhci_pltfm_data sdhci_pxa_pdata = {
+	.ops	= &sdhci_pxa_ops,
+	.quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
+	.init	= sdhci_pxa_init,
+	.exit	= sdhci_pxa_exit,
+	.get_quirk	= sdhci_pxa_get_quirk,
+	.set_max_speed	= sdhci_pxa_set_max_speed,
+	.alloc_host	= sdhci_pxa_alloc_host,
+};
+
+MODULE_DESCRIPTION("SDH controller driver for PXA168/PXA910/MMP2");
+MODULE_AUTHOR("Zhangfei Gao <zhangfei.gao@marvell.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.7.0.4

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Eric Miao]

On Sun, Sep 26, 2010 at 6:56 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
> Assume the sdhci-pltfm call back are accepted
>
> From f7e1cc9975cbed2d993d4209eae36f8f300fb18c Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Sun, 26 Sep 2010 17:13:43 -0400
> Subject: [PATCH 2/2] mmc: add support of sdhci-pxa driver
>
>        Support Marvell PXA168/PXA910/MMP2 SD Host Controller
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>

Zhangfei,

Patch looks good to me, except for a few minor issues as I commented
below. Please have a look again.

> ---
>  arch/arm/plat-pxa/include/plat/sdhci.h |   32 +++++++
>  drivers/mmc/host/Kconfig               |   11 +++
>  drivers/mmc/host/Makefile              |    1 +
>  drivers/mmc/host/sdhci-pxa.c           |  143 ++++++++++++++++++++++++++++++++

I prefer we rename it to something like sdhci-mmp.c and the 'pxa' in the source
code to 'mmp' to cause less confusion with pxamci.c?

>  4 files changed, 187 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/plat-pxa/include/plat/sdhci.h
>  create mode 100644 drivers/mmc/host/sdhci-pxa.c
>
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> new file mode 100644
> index 0000000..3d5f5ef
> --- /dev/null
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -0,0 +1,32 @@
> +/* arch/arm/plat-pxa/include/plat/sdhci.h
> + *
> + * Copyright 2010 Marvell
> + *     Zhangfei Gao <zhangfei.gao@marvell.com>
> + *
> + * PXA Platform - SDHCI platform data definitions
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +
> +#ifndef __PLAT_PXA_SDHCI_H
> +#define __PLAT_PXA_SDHCI_H __FILE__

typo

> +
> +/* pxa specific quirks */
> +/* Card alwayes wired to host, like emmc */
> +#define PXA_QUIRK_BROKEN_CARD_DETECTION (1<<0)
> +/* Require clock free running */
> +#define        PXA_QUIRK_DISABLE_CLOCK_GATING (1<<1)

tabs/space

> +
> +/**
> + * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
> + * @max_speed: The maximum speed supported.
> + * @pxa_quirks: specific quirk of pxa
> +*/
> +struct sdhci_pxa_platdata {
> +       unsigned int    max_speed;
> +       unsigned int    pxa_quirk;
> +};

'unsigned int quirk' should be enough here, no need for a pxa_ prefix.

> +
> +#endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 6f12d5d..9510976 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -158,6 +158,17 @@ config MMC_SDHCI_S3C
>
>          If unsure, say N.
>
> +config MMC_SDHCI_PXA
> +       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
> +       depends on ARCH_PXA || ARCH_MMP

Only on ARCH_MMP or is this also used in PXA950+?

> +       depends on MMC_SDHCI_PLTFM
> +       help
> +         This selects the Marvell(R) PXA168/PXA910/MMP2 SD Host Controller.
> +         If you have a PXA168/PXA910/MMP2 platform with SD Host Controller and a
> +         card slot,say Y or M here.
> +
> +         If unsure, say N.
> +
>  config MMC_SDHCI_SPEAR
>        tristate "SDHCI support on ST SPEAr platform"
>        depends on MMC_SDHCI && PLAT_SPEAR
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index ef32c32..d166493 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_PXA)         += sdhci-pxa.o
>
>  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
>  sdhci-of-y                             := sdhci-of-core.o
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> new file mode 100644
> index 0000000..12ede18
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -0,0 +1,143 @@
> +/* linux/drivers/mmc/host/sdhci-pxa.c
> + *
> + * Copyright 2010 Marvell
> + *      Zhangfei Gao <zhangfei.gao@marvell.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/* Supports:
> + * SDHCI support for MMP2/PXA910/PXA168
> + *
> + * Based on  sdhci-pltfm.c
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/mmc/host.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <plat/sdhci.h>
> +#include <linux/sdhci-pltfm.h>
> +#include "sdhci.h"
> +#include "sdhci-pltfm.h"
> +
> +#define SD_FIFO_PARAM          0x104
> +#define DIS_PAD_SD_CLK_GATE    0x400
> +
> +struct sdhci_pxa {
> +       struct sdhci_pxa_platdata       *pdata;
> +       struct clk                      *clk;
> +
> +       u32 quirks;
> +       u8 clk_enable;
> +};
> +
> +/*****************************************************************************\
> + *                                                                           *
> + * SDHCI core callbacks                                                      *
> + *                                                                           *
> +\*****************************************************************************/
> +static void sdhci_pxa_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +       u32 tmp = 0;
> +
> +       if (clock == 0) {
> +               if (pxa->clk_enable) {
> +                       clk_disable(pxa->clk);
> +                       pxa->clk_enable = 0;
> +               }
> +       } else {
> +               if (0 == pxa->clk_enable) {
> +                       if (pxa->pdata->pxa_quirk

Better copy this 'pxa_quirk' over to pxa->quirks at initialization?

> +                                       & PXA_QUIRK_DISABLE_CLOCK_GATING) {
> +                               tmp = readl(host->ioaddr + SD_FIFO_PARAM);
> +                               tmp |= DIS_PAD_SD_CLK_GATE;
> +                               writel(tmp, host->ioaddr + SD_FIFO_PARAM);
> +                       }
> +                       clk_enable(pxa->clk);
> +                       pxa->clk_enable = 1;
> +               }
> +       }
> +}
> +
> +static struct sdhci_ops sdhci_pxa_ops = {
> +       .set_clock      = sdhci_pxa_set_clock,
> +};
> +
> +/*****************************************************************************\
> + *                                                                           *
> + * sdhci-pltfm callbacks                                                      *
> + *                                                                           *
> +\*****************************************************************************/
> +static int sdhci_pxa_init(struct sdhci_host *host, struct
> sdhci_pltfm_data *pdata, void* priv_pdata)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +       struct device *dev = mmc_dev(host->mmc);
> +
> +       pxa->pdata = priv_pdata;
> +       pxa->clk_enable = 0;
> +
> +       pxa->clk = clk_get(dev, "PXA-SDHCLK");
> +       if (IS_ERR(pxa->clk)) {
> +               dev_err(dev, "clk err\n");
> +               return -ENODEV;
> +       }
> +       clk_enable(pxa->clk);
> +       dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(pxa->clk));
> +
> +       pxa->clk_enable = 0;

Shouldn't this be 1 as you've called clk_enable()? And if the .set_clock
logic is all right, we can simply leave the clock enabling/disabling to
that function.

Or if the power consumption difference doesn't differ that much, I'd
rather to see simpler solution to this: clk_enable() at probe, and clk_disable()
at remove?

> +       pxa->quirks = pdata->quirks;
> +
> +       if (pxa->pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
> +               pxa->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;

Why not just use SDHCI_QUIRK_BROKEN_CARD_DETECTION in
pdata->pxa_quirk?

> +
> +       return 0;
> +}
> +
> +static unsigned int sdhci_pxa_get_quirk(struct sdhci_host *host)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +
> +       return pxa->quirks;
> +}
> +
> +static void sdhci_pxa_set_max_speed(struct sdhci_host *host)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +
> +       if (pxa->pdata->max_speed)
> +               host->mmc->f_max = pxa->pdata->max_speed;
> +}
> +
> +static void sdhci_pxa_exit(struct sdhci_host *host)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +
> +       if (pxa->clk_enable)
> +               clk_disable(pxa->clk);
> +       clk_put(pxa->clk);
> +}
> +
> +static struct sdhci_host *sdhci_pxa_alloc_host(struct device *dev)
> +{
> +       return sdhci_alloc_host(dev, sizeof(struct sdhci_pxa));
> +}
> +
> +struct sdhci_pltfm_data sdhci_pxa_pdata = {
> +       .ops    = &sdhci_pxa_ops,
> +       .quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> +       .init   = sdhci_pxa_init,
> +       .exit   = sdhci_pxa_exit,
> +       .get_quirk      = sdhci_pxa_get_quirk,
> +       .set_max_speed  = sdhci_pxa_set_max_speed,
> +       .alloc_host     = sdhci_pxa_alloc_host,
> +};
> +
> +MODULE_DESCRIPTION("SDH controller driver for PXA168/PXA910/MMP2");
> +MODULE_AUTHOR("Zhangfei Gao <zhangfei.gao@marvell.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.0.4
>

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Haojian Zhuang]

On Sun, Sep 26, 2010 at 8:39 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Sun, Sep 26, 2010 at 6:56 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
>>
>> ---
>>  arch/arm/plat-pxa/include/plat/sdhci.h |   32 +++++++
>>  drivers/mmc/host/Kconfig               |   11 +++
>>  drivers/mmc/host/Makefile              |    1 +
>>  drivers/mmc/host/sdhci-pxa.c           |  143 ++++++++++++++++++++++++++++++++
>
> I prefer we rename it to something like sdhci-mmp.c and the 'pxa' in the source
> code to 'mmp' to cause less confusion with pxamci.c?
>
Maybe sdhci-pxa.c is better. pxamci.c isn't compatible with sdhci, so
it won't make people confusion.

>> +
>> +#endif /* __PLAT_PXA_SDHCI_H */
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 6f12d5d..9510976 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -158,6 +158,17 @@ config MMC_SDHCI_S3C
>>
>>          If unsure, say N.
>>
>> +config MMC_SDHCI_PXA
>> +       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
>> +       depends on ARCH_PXA || ARCH_MMP
>
> Only on ARCH_MMP or is this also used in PXA950+?
>
This SD controller IP is also used in PXA955. PXA955 (MG1) belongs to
ARCH_PXA. So
MMC_SDHCI_PXA should be depend on ARCH_PXA or ARCH_MMP.

Thanks
Haojian

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[zhangfei gao]

On Sun, Sep 26, 2010 at 8:39 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Sun, Sep 26, 2010 at 6:56 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
>> Assume the sdhci-pltfm call back are accepted
>>
>> From f7e1cc9975cbed2d993d4209eae36f8f300fb18c Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Sun, 26 Sep 2010 17:13:43 -0400
>> Subject: [PATCH 2/2] mmc: add support of sdhci-pxa driver
>>
>>        Support Marvell PXA168/PXA910/MMP2 SD Host Controller
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>
> Zhangfei,
>
> Patch looks good to me, except for a few minor issues as I commented
> below. Please have a look again.
>
>> ---
>>  arch/arm/plat-pxa/include/plat/sdhci.h |   32 +++++++
>>  drivers/mmc/host/Kconfig               |   11 +++
>>  drivers/mmc/host/Makefile              |    1 +
>>  drivers/mmc/host/sdhci-pxa.c           |  143 ++++++++++++++++++++++++++++++++
>
> I prefer we rename it to something like sdhci-mmp.c and the 'pxa' in the source
> code to 'mmp' to cause less confusion with pxamci.c?

pxamci.c is used for pxa25x/26x/27x, while sdhci-pxa.c is used for
pxa168/pxa910/mmp2.
>
>>  4 files changed, 187 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/plat-pxa/include/plat/sdhci.h
>>  create mode 100644 drivers/mmc/host/sdhci-pxa.c
>>
>> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
>> b/arch/arm/plat-pxa/include/plat/sdhci.h
>> new file mode 100644
>> index 0000000..3d5f5ef
>> --- /dev/null
>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>> @@ -0,0 +1,32 @@
>> +/* arch/arm/plat-pxa/include/plat/sdhci.h
>> + *
>> + * Copyright 2010 Marvell
>> + *     Zhangfei Gao <zhangfei.gao@marvell.com>
>> + *
>> + * PXA Platform - SDHCI platform data definitions
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> +*/
>> +
>> +#ifndef __PLAT_PXA_SDHCI_H
>> +#define __PLAT_PXA_SDHCI_H __FILE__
>
> typo
>
>> +
>> +/* pxa specific quirks */
>> +/* Card alwayes wired to host, like emmc */
>> +#define PXA_QUIRK_BROKEN_CARD_DETECTION (1<<0)
>> +/* Require clock free running */
>> +#define        PXA_QUIRK_DISABLE_CLOCK_GATING (1<<1)
>
> tabs/space
>
>> +
>> +/**
>> + * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>> + * @max_speed: The maximum speed supported.
>> + * @pxa_quirks: specific quirk of pxa
>> +*/
>> +struct sdhci_pxa_platdata {
>> +       unsigned int    max_speed;
>> +       unsigned int    pxa_quirk;
>> +};
>
> 'unsigned int quirk' should be enough here, no need for a pxa_ prefix.
 To distinguish the pxa->quirk, which used for transfer to sdhci,c
>
>> +
>> +#endif /* __PLAT_PXA_SDHCI_H */
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 6f12d5d..9510976 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -158,6 +158,17 @@ config MMC_SDHCI_S3C
>>
>>          If unsure, say N.
>>
>> +config MMC_SDHCI_PXA
>> +       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
>> +       depends on ARCH_PXA || ARCH_MMP
>
> Only on ARCH_MMP or is this also used in PXA950+?
>
>> +       depends on MMC_SDHCI_PLTFM
>> +       help
>> +         This selects the Marvell(R) PXA168/PXA910/MMP2 SD Host Controller.
>> +         If you have a PXA168/PXA910/MMP2 platform with SD Host Controller and a
>> +         card slot,say Y or M here.
>> +
>> +         If unsure, say N.
>> +
>>  config MMC_SDHCI_SPEAR
>>        tristate "SDHCI support on ST SPEAr platform"
>>        depends on MMC_SDHCI && PLAT_SPEAR
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index ef32c32..d166493 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_PXA)         += sdhci-pxa.o
>>
>>  obj-$(CONFIG_MMC_SDHCI_OF)     += sdhci-of.o
>>  sdhci-of-y                             := sdhci-of-core.o
>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>> new file mode 100644
>> index 0000000..12ede18
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-pxa.c
>> @@ -0,0 +1,143 @@
>> +/* linux/drivers/mmc/host/sdhci-pxa.c
>> + *
>> + * Copyright 2010 Marvell
>> + *      Zhangfei Gao <zhangfei.gao@marvell.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +/* Supports:
>> + * SDHCI support for MMP2/PXA910/PXA168
>> + *
>> + * Based on  sdhci-pltfm.c
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/err.h>
>> +#include <plat/sdhci.h>
>> +#include <linux/sdhci-pltfm.h>
>> +#include "sdhci.h"
>> +#include "sdhci-pltfm.h"
>> +
>> +#define SD_FIFO_PARAM          0x104
>> +#define DIS_PAD_SD_CLK_GATE    0x400
>> +
>> +struct sdhci_pxa {
>> +       struct sdhci_pxa_platdata       *pdata;
>> +       struct clk                      *clk;
>> +
>> +       u32 quirks;
>> +       u8 clk_enable;
>> +};
>> +
>> +/*****************************************************************************\
>> + *                                                                           *
>> + * SDHCI core callbacks                                                      *
>> + *                                                                           *
>> +\*****************************************************************************/
>> +static void sdhci_pxa_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>> +       u32 tmp = 0;
>> +
>> +       if (clock == 0) {
>> +               if (pxa->clk_enable) {
>> +                       clk_disable(pxa->clk);
>> +                       pxa->clk_enable = 0;
>> +               }
>> +       } else {
>> +               if (0 == pxa->clk_enable) {
>> +                       if (pxa->pdata->pxa_quirk
>
> Better copy this 'pxa_quirk' over to pxa->quirks at initialization?

Some confusion here.
Since quirk is limited resource, we should not over-used, however, the
quirk method is good design and could be used to distinguish different
device for different usage.
pxa_quirk is used by our own driver, transfered from platform data,
for example, driver may understand free-running requirement from emmc.
pxa->quirks is used for transfer to sdhci.c, which contains  not only
the init quirks, but also some other requirement, such as
SDHCI_QUIRK_BROKEN_CARD_DETECTION, which only effective to on-chip
non-removalbe device.


>
>> +                                       & PXA_QUIRK_DISABLE_CLOCK_GATING) {
>> +                               tmp = readl(host->ioaddr + SD_FIFO_PARAM);
>> +                               tmp |= DIS_PAD_SD_CLK_GATE;
>> +                               writel(tmp, host->ioaddr + SD_FIFO_PARAM);
>> +                       }
>> +                       clk_enable(pxa->clk);
>> +                       pxa->clk_enable = 1;
>> +               }
>> +       }
>> +}
>> +
>> +static struct sdhci_ops sdhci_pxa_ops = {
>> +       .set_clock      = sdhci_pxa_set_clock,
>> +};
>> +
>> +/*****************************************************************************\
>> + *                                                                           *
>> + * sdhci-pltfm callbacks                                                      *
>> + *                                                                           *
>> +\*****************************************************************************/
>> +static int sdhci_pxa_init(struct sdhci_host *host, struct
>> sdhci_pltfm_data *pdata, void* priv_pdata)
>> +{
>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>> +       struct device *dev = mmc_dev(host->mmc);
>> +
>> +       pxa->pdata = priv_pdata;
>> +       pxa->clk_enable = 0;
>> +
>> +       pxa->clk = clk_get(dev, "PXA-SDHCLK");
>> +       if (IS_ERR(pxa->clk)) {
>> +               dev_err(dev, "clk err\n");
>> +               return -ENODEV;
>> +       }
>> +       clk_enable(pxa->clk);
>> +       dev_dbg(dev, "SDHC clock:%lu\n", clk_get_rate(pxa->clk));
>> +
>> +       pxa->clk_enable = 0;
>
> Shouldn't this be 1 as you've called clk_enable()? And if the .set_clock
> logic is all right, we can simply leave the clock enabling/disabling to
> that function.

Thanks for finding, the clk_enable(pxa->clk) is planed to be removed
here, just use .set_clock to do clock issue, though one small issue is
mmc stack would never set_clock(host, 0) to disalbe clock, will submit
patch later.

>
> Or if the power consumption difference doesn't differ that much, I'd
> rather to see simpler solution to this: clk_enable() at probe, and clk_disable()
> at remove?
>
>> +       pxa->quirks = pdata->quirks;
>> +
>> +       if (pxa->pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
>> +               pxa->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>
> Why not just use SDHCI_QUIRK_BROKEN_CARD_DETECTION in
> pdata->pxa_quirk?

SDHCI_QUIRK_BROKEN_CARD_DETECTION is defined in sdhci.h, which in
driver/mmc/host instead of include, what's more, quirk is limited
resource, and we should not touch quirk as much as possible.

>
>> +
>> +       return 0;
>> +}
>> +
>> +static unsigned int sdhci_pxa_get_quirk(struct sdhci_host *host)
>> +{
>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>> +
>> +       return pxa->quirks;
>> +}
>> +
>> +static void sdhci_pxa_set_max_speed(struct sdhci_host *host)
>> +{
>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>> +
>> +       if (pxa->pdata->max_speed)
>> +               host->mmc->f_max = pxa->pdata->max_speed;
>> +}
>> +
>> +static void sdhci_pxa_exit(struct sdhci_host *host)
>> +{
>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>> +
>> +       if (pxa->clk_enable)
>> +               clk_disable(pxa->clk);
>> +       clk_put(pxa->clk);
>> +}
>> +
>> +static struct sdhci_host *sdhci_pxa_alloc_host(struct device *dev)
>> +{
>> +       return sdhci_alloc_host(dev, sizeof(struct sdhci_pxa));
>> +}
>> +
>> +struct sdhci_pltfm_data sdhci_pxa_pdata = {
>> +       .ops    = &sdhci_pxa_ops,
>> +       .quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
>> +       .init   = sdhci_pxa_init,
>> +       .exit   = sdhci_pxa_exit,
>> +       .get_quirk      = sdhci_pxa_get_quirk,
>> +       .set_max_speed  = sdhci_pxa_set_max_speed,
>> +       .alloc_host     = sdhci_pxa_alloc_host,
>> +};
>> +
>> +MODULE_DESCRIPTION("SDH controller driver for PXA168/PXA910/MMP2");
>> +MODULE_AUTHOR("Zhangfei Gao <zhangfei.gao@marvell.com>");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 1.7.0.4
>>
>

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Chris Ball]

On Sun, Sep 26, 2010 at 10:45:02PM +0800, Haojian Zhuang wrote:
> > I prefer we rename it to something like sdhci-mmp.c and the 'pxa' in the
> > source code to 'mmp' to cause less confusion with pxamci.c?
>
> Maybe sdhci-pxa.c is better. pxamci.c isn't compatible with sdhci, so
> it won't make people confusion.

The confusion can still happen when people are wondering which driver to
build for their SoC.

It seems that both names are confusing -- sdhci-mmp would confuse this
driver with the one submitted by Saeed Bishara for Dove/MMP, and
sdhci-pxa confuses it with pxamci.c.

Have you considered sdhci-mmp for Saeed's driver, and sdhci-mmp2 for
this driver?  Or, sdhci-dove and sdhci-mmp2?  Perhaps a non-exhaustive
distinction is better than an incorrectly generalized one.

Thanks,

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

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Haojian Zhuang]

On Sun, Sep 26, 2010 at 11:59 PM, Chris Ball <cjb@laptop.org> wrote:
> On Sun, Sep 26, 2010 at 10:45:02PM +0800, Haojian Zhuang wrote:
>> > I prefer we rename it to something like sdhci-mmp.c and the 'pxa' in the
>> > source code to 'mmp' to cause less confusion with pxamci.c?
>>
>> Maybe sdhci-pxa.c is better. pxamci.c isn't compatible with sdhci, so
>> it won't make people confusion.
>
> The confusion can still happen when people are wondering which driver to
> build for their SoC.
>
> It seems that both names are confusing -- sdhci-mmp would confuse this
> driver with the one submitted by Saeed Bishara for Dove/MMP, and
> sdhci-pxa confuses it with pxamci.c.
>
> Have you considered sdhci-mmp for Saeed's driver, and sdhci-mmp2 for
> this driver?  Or, sdhci-dove and sdhci-mmp2?  Perhaps a non-exhaustive
> distinction is better than an incorrectly generalized one.
>
If the name is sdhci-mmp2, it's a big confusion that people will think
mmp(ttc/aspen) is using another IP.

sdhci-pxa is the best name.

Thanks
Haojian

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Chris Ball]

Hi Haojian,

On Mon, Sep 27, 2010 at 12:09:17AM +0800, Haojian Zhuang wrote:
> If the name is sdhci-mmp2, it's a big confusion that people will think
> mmp(ttc/aspen) is using another IP.
> 
> sdhci-pxa is the best name.

Okay.  What should Saeed's driver be called?

Thanks,

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

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Haojian Zhuang]

On Mon, Sep 27, 2010 at 1:48 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi Haojian,
>
> On Mon, Sep 27, 2010 at 12:09:17AM +0800, Haojian Zhuang wrote:
>> If the name is sdhci-mmp2, it's a big confusion that people will think
>> mmp(ttc/aspen) is using another IP.
>>
>> sdhci-pxa is the best name.
>
> Okay.  What should Saeed's driver be called?
>
> Thanks,
>
sdhci-dove / sdhci-mv or any other is acceptable. What's your opinion, saeed?

Thanks
Haojian

Re: [patch 2/2] mmc: add support of sdhci-pxa driver

[Chris Ball]

On Mon, Sep 27, 2010 at 09:36:01AM +0800, Haojian Zhuang wrote:
> > Okay.  What should Saeed's driver be called?
>
> sdhci-dove / sdhci-mv or any other is acceptable. What's your opinion, saeed?

sdhci-dove sounds good to me.  Saeed, if you agree, please could you
resend the patch?  (With an "ARCH_DOVE || ARCH_MMP" dependency and a
list of supported SoCs in the Kconfig help text.)

Thanks,

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

RE: [patch 2/2] mmc: add support of sdhci-pxa driver

[Saeed Bishara]

>
>On Mon, Sep 27, 2010 at 12:09:17AM +0800, Haojian Zhuang wrote:
>> If the name is sdhci-mmp2, it's a big confusion that people 
>will think
>> mmp(ttc/aspen) is using another IP.
>> 
>> sdhci-pxa is the best name.
>
>Okay.  What should Saeed's driver be called?
Chris,
	I've re-looked at the sdhci-pltfm.c driver, and I think it can serve the Dove device even without modifications. So please don't merge the shdci-mv driver.

saeed