* [PATCH V2 1/1]MMC: add support of sdhci-pxa driver
@ 2010-09-29 3:23 zhangfei gao
2010-10-05 9:03 ` Matt Fleming
2010-10-07 19:45 ` Chris Ball
0 siblings, 2 replies; 4+ messages in thread
From: zhangfei gao @ 2010-09-29 3:23 UTC (permalink / raw)
To: linux-mmc; +Cc: Chris Ball, kmpark, eric.y.miao, Haojian Zhuang
[-- Attachment #1: Type: text/plain, Size: 10799 bytes --]
Hi, all
We strongly prefer using standalone host driver currently, like
sdhci-s3c.c, the scheme is more mature, more similar as our
controller, and more flexiable for sharing one controller in different
platform with different requirement.
We would rather pay more effort to enable new feature of silicon, to
enhance the sdhci.c, which benefit much more silicon vender, instead
of paying much effort in how to abstract probe and remove.
For the driver lever, we don't want too much dependence, the more
dependence, the less stability.
The sdhci-pltfm has good idea, however currently it does not meet our
requirement, not say in the future.
Update the patch, help review.
>From 06301de67ec50151fa07246bbbdd2f7400126b3c Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Mon, 20 Sep 2010 10:51:28 -0400
Subject: [PATCH 1/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 | 12 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-pxa.c | 259 ++++++++++++++++++++++++++++++++
4 files changed, 304 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..f7a0968
--- /dev/null
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -0,0 +1,32 @@
+/* linux/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
+
+/* 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..40aa3ba 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -158,6 +158,18 @@ 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
+ select MMC_SDHCI
+ select MMC_SDHCI_IO_ACCESSORS
+ 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..5cdc7c0 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_MMC_MXC) += mxcmmc.o
obj-$(CONFIG_MMC_SDHCI) += sdhci.o
obj-$(CONFIG_MMC_SDHCI_MV) += sdhci-mv.o
obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
+obj-$(CONFIG_MMC_SDHCI_PXA) += sdhci-pxa.o
obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o
obj-$(CONFIG_MMC_WBSD) += wbsd.o
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
new file mode 100644
index 0000000..3f68c67
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -0,0 +1,259 @@
+/* linux/drivers/mmc/host/sdhci-pxa.c
+ *
+ * Copyright (C) 2010 Marvell International Ltd.
+ * Zhangfei Gao <zhangfei.gao@marvell.com>
+ * Kevin Wang <dwang4@marvell.com>
+ * Mingwei Wang <mwwang@marvell.com>
+ * Philip Rakity <prakity@marvell.com>
+ * Mark Brown <markb@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
+ *
+ * Refer sdhci-s3c.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 "sdhci.h"
+
+#define DRIVER_NAME "sdhci-pxa"
+
+#define SD_FIFO_PARAM 0x104
+#define DIS_PAD_SD_CLK_GATE 0x400
+
+struct sdhci_pxa {
+ struct sdhci_host *host;
+ struct sdhci_pxa_platdata *pdata;
+ struct clk *clk;
+ struct resource *res;
+
+ u8 clk_enable;
+};
+
+/*****************************************************************************\
+ * *
+ * SDHCI core callbacks *
+ * *
+\*****************************************************************************/
+static void 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 = set_clock,
+};
+
+/*****************************************************************************\
+ * *
+ * Device probing/removal *
+ * *
+\*****************************************************************************/
+
+static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
+{
+ struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+ struct device *dev = &pdev->dev;
+ struct sdhci_host *host = NULL;
+ struct resource *iomem = NULL;
+ struct sdhci_pxa *pxa = NULL;
+ int ret, irq;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "no irq specified\n");
+ return irq;
+ }
+
+ iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!iomem) {
+ dev_err(dev, "no memory specified\n");
+ return -ENOENT;
+ }
+
+ host = sdhci_alloc_host(&pdev->dev, sizeof(struct sdhci_pxa));
+ if (IS_ERR(host)) {
+ dev_err(dev, "failed to alloc host\n");
+ ret = PTR_ERR(host);
+ goto out;
+ }
+
+ pxa = sdhci_priv(host);
+ pxa->host = host;
+ pxa->pdata = pdata;
+ pxa->clk_enable = 0;
+
+ pxa->clk = clk_get(dev, "PXA-SDHCLK");
+ if (IS_ERR(pxa->clk)) {
+ dev_err(dev, "failed to get io clock\n");
+ ret = PTR_ERR(pxa->clk);
+ goto out;
+ }
+
+ pxa->res = request_mem_region(iomem->start, resource_size(iomem),
+ mmc_hostname(host->mmc));
+ if (!pxa->res) {
+ dev_err(&pdev->dev, "cannot request region\n");
+ ret = -EBUSY;
+ goto out;
+ }
+
+ host->ioaddr = ioremap(iomem->start, resource_size(iomem));
+ if (!host->ioaddr) {
+ dev_err(&pdev->dev, "failed to remap registers\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ host->hw_name = "MMC";
+ host->ops = &sdhci_pxa_ops;
+ host->irq = irq;
+ host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+
+ if (pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
+ host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
+ ret = sdhci_add_host(host);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add host\n");
+ goto out;
+ }
+
+ if (pxa->pdata->max_speed)
+ host->mmc->f_max = pxa->pdata->max_speed;
+
+ platform_set_drvdata(pdev, host);
+
+ return 0;
+out:
+ if (host) {
+ if (host->ioaddr)
+ iounmap(host->ioaddr);
+ if (pxa->res)
+ release_mem_region(pxa->res->start,
+ resource_size(pxa->res));
+ sdhci_free_host(host);
+ }
+
+ return ret;
+}
+
+static int __devexit sdhci_pxa_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ struct sdhci_pxa *pxa = sdhci_priv(host);
+ int dead = 0;
+ u32 scratch;
+
+ if (host) {
+ scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+ if (scratch == (u32)-1)
+ dead = 1;
+
+ sdhci_remove_host(host, dead);
+
+ if (host->ioaddr)
+ iounmap(host->ioaddr);
+ if (pxa->res)
+ release_mem_region(pxa->res->start,
+ resource_size(pxa->res));
+ if (pxa->clk_enable) {
+ clk_disable(pxa->clk);
+ pxa->clk_enable = 0;
+ }
+ clk_put(pxa->clk);
+
+ sdhci_free_host(host);
+ platform_set_drvdata(pdev, NULL);
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int sdhci_pxa_suspend(struct platform_device *dev)
+{
+ struct sdhci_host *host = platform_get_drvdata(to_platform_device(dev));
+
+ return sdhci_suspend_host(host, state);
+}
+
+static int sdhci_pxa_resume(struct platform_device *dev)
+{
+ struct sdhci_host *host = platform_get_drvdata(to_platform_device(dev));
+
+ return sdhci_resume_host(host);
+}
+#else
+#define sdhci_pxa_suspend NULL
+#define sdhci_pxa_resume NULL
+#endif
+
+static const struct dev_pm_ops sdhci_pxa_pm_ops = {
+ .suspend = sdhci_pxa_suspend,
+ .resume = sdhci_pxa_resume,
+};
+
+
+static struct platform_driver sdhci_pxa_driver = {
+ .probe = sdhci_pxa_probe,
+ .remove = __devexit_p(sdhci_pxa_remove),
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .pm = &sdhci_pxa_pm_ops,
+ },
+};
+
+/*****************************************************************************\
+ * *
+ * Driver init/exit *
+ * *
+\*****************************************************************************/
+
+static int __init sdhci_pxa_init(void)
+{
+ return platform_driver_register(&sdhci_pxa_driver);
+}
+
+static void __exit sdhci_pxa_exit(void)
+{
+ platform_driver_unregister(&sdhci_pxa_driver);
+}
+
+module_init(sdhci_pxa_init);
+module_exit(sdhci_pxa_exit);
+
+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: 0001-mmc-add-support-of-sdhci-pxa-driver.patch --]
[-- Type: text/x-patch, Size: 10125 bytes --]
From 06301de67ec50151fa07246bbbdd2f7400126b3c Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Mon, 20 Sep 2010 10:51:28 -0400
Subject: [PATCH 1/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 | 12 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-pxa.c | 259 ++++++++++++++++++++++++++++++++
4 files changed, 304 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..f7a0968
--- /dev/null
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -0,0 +1,32 @@
+/* linux/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
+
+/* 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..40aa3ba 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -158,6 +158,18 @@ 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
+ select MMC_SDHCI
+ select MMC_SDHCI_IO_ACCESSORS
+ 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..5cdc7c0 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_MMC_MXC) += mxcmmc.o
obj-$(CONFIG_MMC_SDHCI) += sdhci.o
obj-$(CONFIG_MMC_SDHCI_MV) += sdhci-mv.o
obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
+obj-$(CONFIG_MMC_SDHCI_PXA) += sdhci-pxa.o
obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
obj-$(CONFIG_MMC_SDHCI_SPEAR) += sdhci-spear.o
obj-$(CONFIG_MMC_WBSD) += wbsd.o
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
new file mode 100644
index 0000000..3f68c67
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -0,0 +1,259 @@
+/* linux/drivers/mmc/host/sdhci-pxa.c
+ *
+ * Copyright (C) 2010 Marvell International Ltd.
+ * Zhangfei Gao <zhangfei.gao@marvell.com>
+ * Kevin Wang <dwang4@marvell.com>
+ * Mingwei Wang <mwwang@marvell.com>
+ * Philip Rakity <prakity@marvell.com>
+ * Mark Brown <markb@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
+ *
+ * Refer sdhci-s3c.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 "sdhci.h"
+
+#define DRIVER_NAME "sdhci-pxa"
+
+#define SD_FIFO_PARAM 0x104
+#define DIS_PAD_SD_CLK_GATE 0x400
+
+struct sdhci_pxa {
+ struct sdhci_host *host;
+ struct sdhci_pxa_platdata *pdata;
+ struct clk *clk;
+ struct resource *res;
+
+ u8 clk_enable;
+};
+
+/*****************************************************************************\
+ * *
+ * SDHCI core callbacks *
+ * *
+\*****************************************************************************/
+static void 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 = set_clock,
+};
+
+/*****************************************************************************\
+ * *
+ * Device probing/removal *
+ * *
+\*****************************************************************************/
+
+static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
+{
+ struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+ struct device *dev = &pdev->dev;
+ struct sdhci_host *host = NULL;
+ struct resource *iomem = NULL;
+ struct sdhci_pxa *pxa = NULL;
+ int ret, irq;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "no irq specified\n");
+ return irq;
+ }
+
+ iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!iomem) {
+ dev_err(dev, "no memory specified\n");
+ return -ENOENT;
+ }
+
+ host = sdhci_alloc_host(&pdev->dev, sizeof(struct sdhci_pxa));
+ if (IS_ERR(host)) {
+ dev_err(dev, "failed to alloc host\n");
+ ret = PTR_ERR(host);
+ goto out;
+ }
+
+ pxa = sdhci_priv(host);
+ pxa->host = host;
+ pxa->pdata = pdata;
+ pxa->clk_enable = 0;
+
+ pxa->clk = clk_get(dev, "PXA-SDHCLK");
+ if (IS_ERR(pxa->clk)) {
+ dev_err(dev, "failed to get io clock\n");
+ ret = PTR_ERR(pxa->clk);
+ goto out;
+ }
+
+ pxa->res = request_mem_region(iomem->start, resource_size(iomem),
+ mmc_hostname(host->mmc));
+ if (!pxa->res) {
+ dev_err(&pdev->dev, "cannot request region\n");
+ ret = -EBUSY;
+ goto out;
+ }
+
+ host->ioaddr = ioremap(iomem->start, resource_size(iomem));
+ if (!host->ioaddr) {
+ dev_err(&pdev->dev, "failed to remap registers\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ host->hw_name = "MMC";
+ host->ops = &sdhci_pxa_ops;
+ host->irq = irq;
+ host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+
+ if (pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
+ host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
+ ret = sdhci_add_host(host);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add host\n");
+ goto out;
+ }
+
+ if (pxa->pdata->max_speed)
+ host->mmc->f_max = pxa->pdata->max_speed;
+
+ platform_set_drvdata(pdev, host);
+
+ return 0;
+out:
+ if (host) {
+ if (host->ioaddr)
+ iounmap(host->ioaddr);
+ if (pxa->res)
+ release_mem_region(pxa->res->start,
+ resource_size(pxa->res));
+ sdhci_free_host(host);
+ }
+
+ return ret;
+}
+
+static int __devexit sdhci_pxa_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ struct sdhci_pxa *pxa = sdhci_priv(host);
+ int dead = 0;
+ u32 scratch;
+
+ if (host) {
+ scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+ if (scratch == (u32)-1)
+ dead = 1;
+
+ sdhci_remove_host(host, dead);
+
+ if (host->ioaddr)
+ iounmap(host->ioaddr);
+ if (pxa->res)
+ release_mem_region(pxa->res->start,
+ resource_size(pxa->res));
+ if (pxa->clk_enable) {
+ clk_disable(pxa->clk);
+ pxa->clk_enable = 0;
+ }
+ clk_put(pxa->clk);
+
+ sdhci_free_host(host);
+ platform_set_drvdata(pdev, NULL);
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int sdhci_pxa_suspend(struct platform_device *dev)
+{
+ struct sdhci_host *host = platform_get_drvdata(to_platform_device(dev));
+
+ return sdhci_suspend_host(host, state);
+}
+
+static int sdhci_pxa_resume(struct platform_device *dev)
+{
+ struct sdhci_host *host = platform_get_drvdata(to_platform_device(dev));
+
+ return sdhci_resume_host(host);
+}
+#else
+#define sdhci_pxa_suspend NULL
+#define sdhci_pxa_resume NULL
+#endif
+
+static const struct dev_pm_ops sdhci_pxa_pm_ops = {
+ .suspend = sdhci_pxa_suspend,
+ .resume = sdhci_pxa_resume,
+};
+
+
+static struct platform_driver sdhci_pxa_driver = {
+ .probe = sdhci_pxa_probe,
+ .remove = __devexit_p(sdhci_pxa_remove),
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .pm = &sdhci_pxa_pm_ops,
+ },
+};
+
+/*****************************************************************************\
+ * *
+ * Driver init/exit *
+ * *
+\*****************************************************************************/
+
+static int __init sdhci_pxa_init(void)
+{
+ return platform_driver_register(&sdhci_pxa_driver);
+}
+
+static void __exit sdhci_pxa_exit(void)
+{
+ platform_driver_unregister(&sdhci_pxa_driver);
+}
+
+module_init(sdhci_pxa_init);
+module_exit(sdhci_pxa_exit);
+
+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] 4+ messages in thread
* Re: [PATCH V2 1/1]MMC: add support of sdhci-pxa driver
2010-09-29 3:23 [PATCH V2 1/1]MMC: add support of sdhci-pxa driver zhangfei gao
@ 2010-10-05 9:03 ` Matt Fleming
2010-10-18 12:23 ` zhangfei gao
2010-10-07 19:45 ` Chris Ball
1 sibling, 1 reply; 4+ messages in thread
From: Matt Fleming @ 2010-10-05 9:03 UTC (permalink / raw)
To: zhangfei gao; +Cc: Chris Ball, kmpark, eric.y.miao, Haojian Zhuang, linux-mmc
On Tue, Sep 28, 2010 at 11:23:35PM -0400, zhangfei gao wrote:
> +
> + pxa->clk = clk_get(dev, "PXA-SDHCLK");
> + if (IS_ERR(pxa->clk)) {
> + dev_err(dev, "failed to get io clock\n");
> + ret = PTR_ERR(pxa->clk);
> + goto out;
> + }
> +
> + pxa->res = request_mem_region(iomem->start, resource_size(iomem),
> + mmc_hostname(host->mmc));
> + if (!pxa->res) {
> + dev_err(&pdev->dev, "cannot request region\n");
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + host->ioaddr = ioremap(iomem->start, resource_size(iomem));
> + if (!host->ioaddr) {
> + dev_err(&pdev->dev, "failed to remap registers\n");
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + host->hw_name = "MMC";
> + host->ops = &sdhci_pxa_ops;
> + host->irq = irq;
> + host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> +
> + if (pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
> + host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
I don't think there's a good reason to define
PXA_QUIRK_BROKEN_CARD_DETECTION because its only use is to turn on
SDHCI_QUIRK_BROKEN_CARD_DETECTION. As Eric pointed out, you should
just use SDHCI_QUIRK_BROKEN_CARD_DETECTION directly. While adding new
sdhci quirks should be avoided, using the existing ones is fine :-)
> +
> + ret = sdhci_add_host(host);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to add host\n");
> + goto out;
> + }
> +
> + if (pxa->pdata->max_speed)
> + host->mmc->f_max = pxa->pdata->max_speed;
> +
> + platform_set_drvdata(pdev, host);
> +
> + return 0;
> +out:
> + if (host) {
> + if (host->ioaddr)
> + iounmap(host->ioaddr);
> + if (pxa->res)
> + release_mem_region(pxa->res->start,
> + resource_size(pxa->res));
> + sdhci_free_host(host);
> + }
Aren't you missing a clk_put() here?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2 1/1]MMC: add support of sdhci-pxa driver
2010-09-29 3:23 [PATCH V2 1/1]MMC: add support of sdhci-pxa driver zhangfei gao
2010-10-05 9:03 ` Matt Fleming
@ 2010-10-07 19:45 ` Chris Ball
1 sibling, 0 replies; 4+ messages in thread
From: Chris Ball @ 2010-10-07 19:45 UTC (permalink / raw)
To: zhangfei gao; +Cc: linux-mmc, Wolfram Sang, kmpark, eric.y.miao, Haojian Zhuang
Hi Zhangfei, all,
On Tue, Sep 28, 2010 at 11:23:35PM -0400, zhangfei gao wrote:
> We strongly prefer using standalone host driver currently, like
> sdhci-s3c.c, the scheme is more mature, more similar as our
> controller, and more flexiable for sharing one controller in different
> platform with different requirement.
> We would rather pay more effort to enable new feature of silicon, to
> enhance the sdhci.c, which benefit much more silicon vender, instead
> of paying much effort in how to abstract probe and remove.
> For the driver lever, we don't want too much dependence, the more
> dependence, the less stability.
> The sdhci-pltfm has good idea, however currently it does not meet our
> requirement, not say in the future.
Sorry for taking so long to reply to this -- it's been difficult to
decide whether to take this as a non-pltfm driver.
I've decided to take it since our encouragement of platform drivers is
recent, and Eric is also in favor of keeping it separate, and you've
both argued that -pltfm isn't easily able to handle the shared clock
situation on your hardware. (I'm sure it could be extended to be.)
So, please update the patch in light of Matt Fleming's review comments
and re-submit.
I don't agree, however, with the argument that SDHCI drivers are more
stable if they don't depend on a common core; the common core will
receive more maintenance if more people are using it. Going forward,
I think that new SDHCI driver work should either use -pltfm or give
strong technical reasons why -pltfm isn't appropriate, and give us a
chance to improve -pltfm in response. I think we should also consider
converting existing drivers over to platform drivers, even though that's
clearly difficult to accomplish after a driver's been merged.
Hope that helps to explain the situation as I see it, and I'd be happy
to hear any more comments on the -pltfm approach in general.
Thanks!
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2 1/1]MMC: add support of sdhci-pxa driver
2010-10-05 9:03 ` Matt Fleming
@ 2010-10-18 12:23 ` zhangfei gao
0 siblings, 0 replies; 4+ messages in thread
From: zhangfei gao @ 2010-10-18 12:23 UTC (permalink / raw)
To: Matt Fleming; +Cc: Chris Ball, kmpark, eric.y.miao, Haojian Zhuang, linux-mmc
On Tue, Oct 5, 2010 at 5:03 AM, Matt Fleming <matt@console-pimps.org> wrote:
> On Tue, Sep 28, 2010 at 11:23:35PM -0400, zhangfei gao wrote:
>> +
>> + pxa->clk = clk_get(dev, "PXA-SDHCLK");
>> + if (IS_ERR(pxa->clk)) {
>> + dev_err(dev, "failed to get io clock\n");
>> + ret = PTR_ERR(pxa->clk);
>> + goto out;
>> + }
>> +
>> + pxa->res = request_mem_region(iomem->start, resource_size(iomem),
>> + mmc_hostname(host->mmc));
>> + if (!pxa->res) {
>> + dev_err(&pdev->dev, "cannot request region\n");
>> + ret = -EBUSY;
>> + goto out;
>> + }
>> +
>> + host->ioaddr = ioremap(iomem->start, resource_size(iomem));
>> + if (!host->ioaddr) {
>> + dev_err(&pdev->dev, "failed to remap registers\n");
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + host->hw_name = "MMC";
>> + host->ops = &sdhci_pxa_ops;
>> + host->irq = irq;
>> + host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>> +
>> + if (pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
>> + host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>
> I don't think there's a good reason to define
> PXA_QUIRK_BROKEN_CARD_DETECTION because its only use is to turn on
> SDHCI_QUIRK_BROKEN_CARD_DETECTION. As Eric pointed out, you should
> just use SDHCI_QUIRK_BROKEN_CARD_DETECTION directly. While adding new
> sdhci quirks should be avoided, using the existing ones is fine :-)
Thanks Matt for help review.
Will re-use the SDHCI_QUIRK_BROKEN_CARD_DETECTION, which already move
to include/ and could be used in arch/arm folder.
>
>> +
>> + ret = sdhci_add_host(host);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to add host\n");
>> + goto out;
>> + }
>> +
>> + if (pxa->pdata->max_speed)
>> + host->mmc->f_max = pxa->pdata->max_speed;
>> +
>> + platform_set_drvdata(pdev, host);
>> +
>> + return 0;
>> +out:
>> + if (host) {
>> + if (host->ioaddr)
>> + iounmap(host->ioaddr);
>> + if (pxa->res)
>> + release_mem_region(pxa->res->start,
>> + resource_size(pxa->res));
>> + sdhci_free_host(host);
>> + }
>
> Aren't you missing a clk_put() here?
Thanks, update in coming version.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-10-18 12:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-29 3:23 [PATCH V2 1/1]MMC: add support of sdhci-pxa driver zhangfei gao
2010-10-05 9:03 ` Matt Fleming
2010-10-18 12:23 ` zhangfei gao
2010-10-07 19:45 ` Chris Ball
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).