* [patch 2/2] mmc: add support of sdhci-pxa driver
@ 2010-09-26 10:56 zhangfei gao
2010-09-26 12:39 ` Eric Miao
0 siblings, 1 reply; 10+ messages in thread
From: zhangfei gao @ 2010-09-26 10:56 UTC (permalink / raw)
To: linux-mmc; +Cc: Chris Ball, Wolfram Sang, eric.y.miao, Haojian Zhuang
[-- 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 10:56 [patch 2/2] mmc: add support of sdhci-pxa driver zhangfei gao
@ 2010-09-26 12:39 ` Eric Miao
2010-09-26 14:45 ` Haojian Zhuang
2010-09-26 15:05 ` zhangfei gao
0 siblings, 2 replies; 10+ messages in thread
From: Eric Miao @ 2010-09-26 12:39 UTC (permalink / raw)
To: zhangfei gao; +Cc: linux-mmc, Chris Ball, Wolfram Sang, Haojian Zhuang
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 12:39 ` Eric Miao
@ 2010-09-26 14:45 ` Haojian Zhuang
2010-09-26 15:59 ` Chris Ball
2010-09-26 15:05 ` zhangfei gao
1 sibling, 1 reply; 10+ messages in thread
From: Haojian Zhuang @ 2010-09-26 14:45 UTC (permalink / raw)
To: Eric Miao; +Cc: zhangfei gao, linux-mmc, Chris Ball, Wolfram Sang
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 14:45 ` Haojian Zhuang
@ 2010-09-26 15:59 ` Chris Ball
2010-09-26 16:09 ` Haojian Zhuang
0 siblings, 1 reply; 10+ messages in thread
From: Chris Ball @ 2010-09-26 15:59 UTC (permalink / raw)
To: Haojian Zhuang
Cc: Eric Miao, zhangfei gao, linux-mmc, Wolfram Sang, Saeed Bishara
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 15:59 ` Chris Ball
@ 2010-09-26 16:09 ` Haojian Zhuang
2010-09-26 17:48 ` Chris Ball
0 siblings, 1 reply; 10+ messages in thread
From: Haojian Zhuang @ 2010-09-26 16:09 UTC (permalink / raw)
To: Chris Ball
Cc: Eric Miao, zhangfei gao, linux-mmc, Wolfram Sang, Saeed Bishara
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 16:09 ` Haojian Zhuang
@ 2010-09-26 17:48 ` Chris Ball
2010-09-27 1:36 ` Haojian Zhuang
2010-10-03 8:15 ` Saeed Bishara
0 siblings, 2 replies; 10+ messages in thread
From: Chris Ball @ 2010-09-26 17:48 UTC (permalink / raw)
To: Haojian Zhuang
Cc: Eric Miao, zhangfei gao, linux-mmc, Wolfram Sang, Saeed Bishara
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 17:48 ` Chris Ball
@ 2010-09-27 1:36 ` Haojian Zhuang
2010-09-28 23:36 ` Chris Ball
2010-10-03 8:15 ` Saeed Bishara
1 sibling, 1 reply; 10+ messages in thread
From: Haojian Zhuang @ 2010-09-27 1:36 UTC (permalink / raw)
To: Chris Ball
Cc: Eric Miao, zhangfei gao, linux-mmc, Wolfram Sang, Saeed Bishara
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-27 1:36 ` Haojian Zhuang
@ 2010-09-28 23:36 ` Chris Ball
0 siblings, 0 replies; 10+ messages in thread
From: Chris Ball @ 2010-09-28 23:36 UTC (permalink / raw)
To: Saeed Bishara
Cc: Haojian Zhuang, Eric Miao, zhangfei gao, linux-mmc, Wolfram Sang
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 17:48 ` Chris Ball
2010-09-27 1:36 ` Haojian Zhuang
@ 2010-10-03 8:15 ` Saeed Bishara
1 sibling, 0 replies; 10+ messages in thread
From: Saeed Bishara @ 2010-10-03 8:15 UTC (permalink / raw)
To: Chris Ball, Haojian Zhuang
Cc: Eric Miao, zhangfei gao, linux-mmc, Wolfram Sang
>
>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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 2/2] mmc: add support of sdhci-pxa driver
2010-09-26 12:39 ` Eric Miao
2010-09-26 14:45 ` Haojian Zhuang
@ 2010-09-26 15:05 ` zhangfei gao
1 sibling, 0 replies; 10+ messages in thread
From: zhangfei gao @ 2010-09-26 15:05 UTC (permalink / raw)
To: Eric Miao; +Cc: linux-mmc, Chris Ball, Wolfram Sang, 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:
>> 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
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-10-03 8:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-26 10:56 [patch 2/2] mmc: add support of sdhci-pxa driver zhangfei gao
2010-09-26 12:39 ` Eric Miao
2010-09-26 14:45 ` Haojian Zhuang
2010-09-26 15:59 ` Chris Ball
2010-09-26 16:09 ` Haojian Zhuang
2010-09-26 17:48 ` Chris Ball
2010-09-27 1:36 ` Haojian Zhuang
2010-09-28 23:36 ` Chris Ball
2010-10-03 8:15 ` Saeed Bishara
2010-09-26 15:05 ` zhangfei gao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.