diff mbox

mmc: support sdhci-mmp2

Message ID 1306156882-12124-1-git-send-email-zhangfei.gao@marvell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhangfei Gao May 23, 2011, 1:21 p.m. UTC
Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
 drivers/mmc/host/Kconfig               |    9 +-
 drivers/mmc/host/Makefile              |    2 +-
 drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
 5 files changed, 349 insertions(+), 312 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-mmp2.c
 delete mode 100644 drivers/mmc/host/sdhci-pxa.c

Comments

Arnd Bergmann May 23, 2011, 3:01 p.m. UTC | #1
On Monday 23 May 2011, Zhangfei Gao wrote:
>         Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
>  drivers/mmc/host/Kconfig               |    9 +-
>  drivers/mmc/host/Makefile              |    2 +-
>  drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
>  5 files changed, 349 insertions(+), 312 deletions(-)
>  create mode 100644 drivers/mmc/host/sdhci-mmp2.c
>  delete mode 100644 drivers/mmc/host/sdhci-pxa.c

Hi Zhangfei,

You should really try to improve your changelog texts. Instead of
the one-line summary that describes what you do 'Delete sdhci-pxa.c and
replace ...', please provide a paragraph explaining why you do it,
and what the bigger plan is here.

Also, when renaming a file, please use 'git diff -M' or 'git format-patch -M'
to detect renames and generate a diff for the changes you did to the file.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philip Rakity May 24, 2011, 6:10 a.m. UTC | #2
code is missing hooks for 74 clock generation.


On May 23, 2011, at 6:21 AM, Zhangfei Gao wrote:

>        Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
> arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
> drivers/mmc/host/Kconfig               |    9 +-
> drivers/mmc/host/Makefile              |    2 +-
> drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
> 5 files changed, 349 insertions(+), 312 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-mmp2.c
> delete 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
> index 1ab332e..92becc0 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -13,23 +13,58 @@
> #ifndef __PLAT_PXA_SDHCI_H
> #define __PLAT_PXA_SDHCI_H
>
> +#include <linux/mmc/sdhci.h>
> +#include <linux/platform_device.h>
> +
> /* pxa specific flag */
> /* Require clock free running */
> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> -
> +/* card alwayes wired to host, like on-chip emmc */
> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
> /* Board design supports 8-bit data on SD/SDIO BUS */
> #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
> +/* card not use wp, such as micro sd card */
> +#define PXA_FLAG_CARD_NO_WP    (3<<1)
>
> +struct sdhci_pxa;
> /*
>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
> - * @max_speed: the maximum speed supported
> - * @quirks: quirks of specific device
>  * @flags: flags for platform requirement
> + * @clk_delay_cycles:
> + *     mmp2: each step is roughly 100ps, 5bits width
> + *     pxa910: each step is 1ns, 4bits width
> + * @clk_delay_sel: select clk_delay, used on pxa910
> + *     0: choose feedback clk
> + *     1: choose feedback clk + delay value
> + *     2: choose internal clk
> + * @ext_cd_gpio: gpio pin used for external CD line
> + * @ext_cd_gpio_invert: invert values for external CD gpio line
> + * @clk_delay_enable: enable clk_delay or not, used on pxa910
> + * @max_speed: the maximum speed supported
> + * @host_caps: Standard MMC host capabilities bit field.
> + * @quirks: quirks of platfrom
> + * @pm_caps: pm_caps of platfrom
>  */
> struct sdhci_pxa_platdata {
> +       unsigned int    flags;
> +       unsigned int    clk_delay_cycles;
> +       unsigned int    clk_delay_sel;
> +       unsigned int    ext_cd_gpio;
> +       bool            ext_cd_gpio_invert;
> +       bool            clk_delay_enable;
>        unsigned int    max_speed;
> +       unsigned int    host_caps;
>        unsigned int    quirks;
> -       unsigned int    flags;
> +       unsigned int    pm_caps;
> +};
> +
> +struct sdhci_pxa {
> +       struct sdhci_pxa_platdata       *pdata;
> +       struct clk                      *clk;
> +       struct sdhci_ops                *ops;
> +
> +       u8      clk_enable;
> +       u8      power_mode;
> };
>
> #endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 862235e..1e520e3 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -181,14 +181,15 @@ config MMC_SDHCI_S3C
>
>          If unsure, say N.
>
> -config MMC_SDHCI_PXA
> -       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
> +config MMC_SDHCI_MMP2
> +       tristate "Marvell MMP2 SD Host Controller support"
>        depends on ARCH_PXA || ARCH_MMP
>        select MMC_SDHCI
> +       select MMC_SDHCI_PLTFM
>        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
> +         This selects the Marvell(R) MMP2 SD Host Controller.
> +         If you have a MMP2 platform with SD Host Controller
>          and a card slot, say Y or M here.
>
>          If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 4933004..f8650e0 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_MMC_MXC)           += mxcmmc.o
> obj-$(CONFIG_MMC_MXS)          += mxs-mmc.o
> obj-$(CONFIG_MMC_SDHCI)                += sdhci.o
> obj-$(CONFIG_MMC_SDHCI_PCI)    += sdhci-pci.o
> -obj-$(CONFIG_MMC_SDHCI_PXA)    += sdhci-pxa.o
> +obj-$(CONFIG_MMC_SDHCI_MMP2)   += sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
> new file mode 100644
> index 0000000..566d525
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-mmp2.c
> @@ -0,0 +1,304 @@
> +/*
> + * Copyright (C) 2010 Marvell International Ltd.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mmc/host.h>
> +#include <plat/sdhci.h>
> +#include <linux/slab.h>
> +#include "sdhci.h"
> +#include "sdhci-pltfm.h"
> +
> +#define MMP2_SD_FIFO_PARAM             0x104
> +#define MMP2_DIS_PAD_SD_CLK_GATE       0x400
> +
> +#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP             0x10A
> +#define MMP2_SDCLK_SEL 0x100
> +#define MMP2_SDCLK_DELAY_SHIFT 9
> +#define MMP2_SDCLK_DELAY_MASK  0x1f
> +
> +#define SD_CFG_FIFO_PARAM       0x100
> +#define SDCFG_GEN_PAD_CLK_ON   (1<<6)
> +#define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
> +#define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
> +
> +#define SD_SPI_MODE          0x108
> +#define SD_CE_ATA_1          0x10C
> +
> +#define SD_CE_ATA_2          0x10E
> +#define SDCE_MISC_INT          (1<<2)
> +#define SDCE_MISC_INT_EN       (1<<1)
> +
> +static void mmp2_soc_set_timing(struct sdhci_host *host,
> +                               struct sdhci_pxa_platdata *pdata)
> +{
> +       /*
> +        * tune timing of read data/command when crc error happen
> +        * no performance impact
> +        */
> +       if (pdata && 0 != pdata->clk_delay_cycles) {
> +               u16 tmp;
> +
> +               tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
> +               tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
> +                                       << MMP2_SDCLK_DELAY_SHIFT;
> +               tmp |= MMP2_SDCLK_SEL;
> +               writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
> +       }
> +
> +       /*
> +        * disable clock gating per some cards requirement
> +        */
> +
> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
> +               u32 tmp = 0;
> +
> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
> +       }
> +}
> +
> +static unsigned int mmp2_get_ro(struct sdhci_host *host)
> +{
> +       /* Micro SD does not support write-protect feature */
> +       return 0;
> +}
> +
> +static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +
> +       if (clock)
> +               mmp2_soc_set_timing(host, pxa->pdata);
> +}
> +
> +static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
> +{
> +       u16 ctrl_2;
> +
> +       /*
> +        * Set V18_EN -- UHS modes do not work without this.
> +        * does not change signaling voltage
> +        */
> +       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> +       /* Select Bus Speed Mode for host */
> +       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> +       switch (uhs) {
> +       case MMC_TIMING_UHS_SDR12:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> +               break;
> +       case MMC_TIMING_UHS_SDR25:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> +               break;
> +       case MMC_TIMING_UHS_SDR50:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
> +               break;
> +       case MMC_TIMING_UHS_SDR104:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
> +               break;
> +       case MMC_TIMING_UHS_DDR50:
> +               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
> +               break;
> +       }
> +
> +       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +       dev_dbg(mmc_dev(host->mmc),
> +               "%s:%s uhs = %d, ctrl_2 = %04X\n",
> +               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
> +
> +       return 0;
> +}
> +
> +static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
> +{
> +       struct sdhci_pltfm_host *pltfm_host;
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +       struct device *dev = &pdev->dev;
> +       struct sdhci_host *host = NULL;
> +       struct sdhci_pxa *pxa = NULL;
> +       int ret;
> +       struct clk *clk;
> +
> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
> +       if (!pxa) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
> +       if (!pxa->ops) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +       pxa->pdata = pdata;
> +
> +       clk = clk_get(dev, "PXA-SDHCLK");
> +       if (IS_ERR(clk)) {
> +               dev_err(dev, "failed to get io clock\n");
> +               ret = PTR_ERR(clk);
> +               goto out;
> +       }
> +       pxa->clk = clk;
> +       clk_enable(clk);
> +
> +       host = sdhci_pltfm_init(pdev, NULL);
> +       if (IS_ERR(host))
> +               return PTR_ERR(host);
> +
> +       pltfm_host = sdhci_priv(host);
> +       pltfm_host->priv = pxa;
> +
> +       host->quirks = SDHCI_QUIRK_BROKEN_ADMA
> +               | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
> +               | SDHCI_QUIRK_32BIT_DMA_ADDR
> +               | SDHCI_QUIRK_32BIT_DMA_SIZE
> +               | SDHCI_QUIRK_32BIT_ADMA_SIZE
> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
> +
> +       /* enable 1/8V DDR capable */
> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;
> +
> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
> +               /* on-chip device */
> +               host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +               host->mmc->caps |= MMC_CAP_NONREMOVABLE;
> +       }
> +
> +       /* If slot design supports 8 bit data, indicate this to MMC. */
> +       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> +
> +       if (pdata && pdata->quirks)
> +               host->quirks |= pdata->quirks;
> +       if (pdata && pdata->host_caps)
> +               host->mmc->caps |= pdata->host_caps;
> +       if (pdata && pdata->pm_caps)
> +               host->mmc->pm_caps |= pdata->pm_caps;
> +
> +       pxa->ops->set_clock = mmp2_set_clock;
> +       pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
> +       if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
> +               pxa->ops->get_ro = mmp2_get_ro;
> +
> +       host->ops = pxa->ops;
> +
> +       ret = sdhci_add_host(host);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to add host\n");
> +               goto out;
> +       }
> +
> +       platform_set_drvdata(pdev, host);
> +
> +       return 0;
> +out:
> +       if (host) {
> +               clk_disable(pltfm_host->clk);
> +               clk_put(pltfm_host->clk);
> +               sdhci_pltfm_free(pdev);
> +       }
> +
> +       if (pxa)
> +               kfree(pxa->ops);
> +       kfree(pxa);
> +
> +       return ret;
> +}
> +
> +static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
> +{
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +       struct sdhci_host *host = platform_get_drvdata(pdev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +       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);
> +
> +               clk_disable(pxa->clk);
> +               clk_put(pxa->clk);
> +               sdhci_pltfm_free(pdev);
> +
> +               platform_set_drvdata(pdev, NULL);
> +       }
> +
> +       if (pxa)
> +               kfree(pxa->ops);
> +       kfree(pxa);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(dev);
> +
> +       return sdhci_suspend_host(host, state);
> +}
> +
> +static int sdhci_mmp2_resume(struct platform_device *dev)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(dev);
> +
> +       return sdhci_resume_host(host);
> +}
> +#else
> +#define sdhci_mmp2_suspend     NULL
> +#define sdhci_mmp2_resume      NULL
> +#endif
> +
> +
> +static struct platform_driver sdhci_mmp2_driver = {
> +       .driver         = {
> +               .name   = "sdhci-mmp2",
> +               .owner  = THIS_MODULE,
> +       },
> +       .probe          = sdhci_mmp2_probe,
> +       .remove         = __devexit_p(sdhci_mmp2_remove),
> +#ifdef CONFIG_PM
> +       .suspend        = sdhci_mmp2_suspend,
> +       .resume         = sdhci_mmp2_resume,
> +#endif
> +};
> +static int __init sdhci_mmp2_init(void)
> +{
> +       return platform_driver_register(&sdhci_mmp2_driver);
> +}
> +
> +static void __exit sdhci_mmp2_exit(void)
> +{
> +       platform_driver_unregister(&sdhci_mmp2_driver);
> +}
> +
> +module_init(sdhci_mmp2_init);
> +module_exit(sdhci_mmp2_exit);
> +
> +MODULE_DESCRIPTION("SDHCI driver for mmp2");
> +MODULE_AUTHOR("Marvell International Ltd.");
> +MODULE_LICENSE("GPL v2");
> +
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> deleted file mode 100644
> index 089c9a6..0000000
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ /dev/null
> @@ -1,303 +0,0 @@
> -/* 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
> -{
> -       u16 ctrl_2;
> -
> -       /*
> -        * Set V18_EN -- UHS modes do not work without this.
> -        * does not change signaling voltage
> -        */
> -       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> -
> -       /* Select Bus Speed Mode for host */
> -       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> -       switch (uhs) {
> -       case MMC_TIMING_UHS_SDR12:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> -               break;
> -       case MMC_TIMING_UHS_SDR25:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> -               break;
> -       case MMC_TIMING_UHS_SDR50:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
> -               break;
> -       case MMC_TIMING_UHS_SDR104:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
> -               break;
> -       case MMC_TIMING_UHS_DDR50:
> -               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
> -               break;
> -       }
> -
> -       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> -       pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
> -               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
> -
> -       return 0;
> -}
> -
> -static struct sdhci_ops sdhci_pxa_ops = {
> -       .set_uhs_signaling = set_uhs_signaling,
> -       .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");
> -               return PTR_ERR(host);
> -       }
> -
> -       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
> -               | SDHCI_QUIRK_32BIT_DMA_ADDR
> -               | SDHCI_QUIRK_32BIT_DMA_SIZE
> -               | SDHCI_QUIRK_32BIT_ADMA_SIZE
> -               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
> -
> -       if (pdata->quirks)
> -               host->quirks |= pdata->quirks;
> -
> -       /* enable 1/8V DDR capable */
> -       host->mmc->caps |= MMC_CAP_1_8V_DDR;
> -
> -       /* If slot design supports 8 bit data, indicate this to MMC. */
> -       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
> -               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> -
> -       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) {
> -               clk_put(pxa->clk);
> -               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, pm_message_t state)
> -{
> -       struct sdhci_host *host = platform_get_drvdata(dev);
> -
> -       return sdhci_suspend_host(host, state);
> -}
> -
> -static int sdhci_pxa_resume(struct platform_device *dev)
> -{
> -       struct sdhci_host *host = platform_get_drvdata(dev);
> -
> -       return sdhci_resume_host(host);
> -}
> -#else
> -#define sdhci_pxa_suspend      NULL
> -#define sdhci_pxa_resume       NULL
> -#endif
> -
> -static struct platform_driver sdhci_pxa_driver = {
> -       .probe          = sdhci_pxa_probe,
> -       .remove         = __devexit_p(sdhci_pxa_remove),
> -       .suspend        = sdhci_pxa_suspend,
> -       .resume         = sdhci_pxa_resume,
> -       .driver         = {
> -               .name   = DRIVER_NAME,
> -               .owner  = THIS_MODULE,
> -       },
> -};
> -
> -/*****************************************************************************\
> - *                                                                           *
> - * 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
>

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao May 25, 2011, 8:41 a.m. UTC | #3
On Mon, May 23, 2011 at 11:01 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 23 May 2011, Zhangfei Gao wrote:
>>         Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
>>  drivers/mmc/host/Kconfig               |    9 +-
>>  drivers/mmc/host/Makefile              |    2 +-
>>  drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
>>  drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
>>  5 files changed, 349 insertions(+), 312 deletions(-)
>>  create mode 100644 drivers/mmc/host/sdhci-mmp2.c
>>  delete mode 100644 drivers/mmc/host/sdhci-pxa.c
>
> Hi Zhangfei,
>
> You should really try to improve your changelog texts. Instead of
> the one-line summary that describes what you do 'Delete sdhci-pxa.c and
> replace ...', please provide a paragraph explaining why you do it,
> and what the bigger plan is here.

Thanks Arnd, will add more change log for explanation.
>
> Also, when renaming a file, please use 'git diff -M' or 'git format-patch -M'
> to detect renames and generate a diff for the changes you did to the file.

Double checked same patch coming using  'git format-patch -M' or  'git
format-patch' , since the two files changes a lot.

>
>        Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao May 25, 2011, 8:50 a.m. UTC | #4
On Tue, May 24, 2011 at 2:10 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> code is missing hooks for 74 clock generation.

Could we add the hook later in another patch, or could you help submit
one patch to handle this.
We test on mmp2 and ttc_dkb with toshiba and sandisk emmc, do not find
issues without adding hook for 74 clock generation.

>
>
> On May 23, 2011, at 6:21 AM, Zhangfei Gao wrote:
>
>>        Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>> arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
>> drivers/mmc/host/Kconfig               |    9 +-
>> drivers/mmc/host/Makefile              |    2 +-
>> drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
>> drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
>> 5 files changed, 349 insertions(+), 312 deletions(-)
>> create mode 100644 drivers/mmc/host/sdhci-mmp2.c
>> delete 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
>> index 1ab332e..92becc0 100644
>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>> @@ -13,23 +13,58 @@
>> #ifndef __PLAT_PXA_SDHCI_H
>> #define __PLAT_PXA_SDHCI_H
>>
>> +#include <linux/mmc/sdhci.h>
>> +#include <linux/platform_device.h>
>> +
>> /* pxa specific flag */
>> /* Require clock free running */
>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>> -
>> +/* card alwayes wired to host, like on-chip emmc */
>> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
>> /* Board design supports 8-bit data on SD/SDIO BUS */
>> #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
>> +/* card not use wp, such as micro sd card */
>> +#define PXA_FLAG_CARD_NO_WP    (3<<1)
>>
>> +struct sdhci_pxa;
>> /*
>>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>> - * @max_speed: the maximum speed supported
>> - * @quirks: quirks of specific device
>>  * @flags: flags for platform requirement
>> + * @clk_delay_cycles:
>> + *     mmp2: each step is roughly 100ps, 5bits width
>> + *     pxa910: each step is 1ns, 4bits width
>> + * @clk_delay_sel: select clk_delay, used on pxa910
>> + *     0: choose feedback clk
>> + *     1: choose feedback clk + delay value
>> + *     2: choose internal clk
>> + * @ext_cd_gpio: gpio pin used for external CD line
>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910
>> + * @max_speed: the maximum speed supported
>> + * @host_caps: Standard MMC host capabilities bit field.
>> + * @quirks: quirks of platfrom
>> + * @pm_caps: pm_caps of platfrom
>>  */
>> struct sdhci_pxa_platdata {
>> +       unsigned int    flags;
>> +       unsigned int    clk_delay_cycles;
>> +       unsigned int    clk_delay_sel;
>> +       unsigned int    ext_cd_gpio;
>> +       bool            ext_cd_gpio_invert;
>> +       bool            clk_delay_enable;
>>        unsigned int    max_speed;
>> +       unsigned int    host_caps;
>>        unsigned int    quirks;
>> -       unsigned int    flags;
>> +       unsigned int    pm_caps;
>> +};
>> +
>> +struct sdhci_pxa {
>> +       struct sdhci_pxa_platdata       *pdata;
>> +       struct clk                      *clk;
>> +       struct sdhci_ops                *ops;
>> +
>> +       u8      clk_enable;
>> +       u8      power_mode;
>> };
>>
>> #endif /* __PLAT_PXA_SDHCI_H */
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 862235e..1e520e3 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -181,14 +181,15 @@ config MMC_SDHCI_S3C
>>
>>          If unsure, say N.
>>
>> -config MMC_SDHCI_PXA
>> -       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
>> +config MMC_SDHCI_MMP2
>> +       tristate "Marvell MMP2 SD Host Controller support"
>>        depends on ARCH_PXA || ARCH_MMP
>>        select MMC_SDHCI
>> +       select MMC_SDHCI_PLTFM
>>        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
>> +         This selects the Marvell(R) MMP2 SD Host Controller.
>> +         If you have a MMP2 platform with SD Host Controller
>>          and a card slot, say Y or M here.
>>
>>          If unsure, say N.
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 4933004..f8650e0 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -9,7 +9,7 @@ obj-$(CONFIG_MMC_MXC)           += mxcmmc.o
>> obj-$(CONFIG_MMC_MXS)          += mxs-mmc.o
>> obj-$(CONFIG_MMC_SDHCI)                += sdhci.o
>> obj-$(CONFIG_MMC_SDHCI_PCI)    += sdhci-pci.o
>> -obj-$(CONFIG_MMC_SDHCI_PXA)    += sdhci-pxa.o
>> +obj-$(CONFIG_MMC_SDHCI_MMP2)   += sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
>> new file mode 100644
>> index 0000000..566d525
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-mmp2.c
>> @@ -0,0 +1,304 @@
>> +/*
>> + * Copyright (C) 2010 Marvell International Ltd.
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/gpio.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/mmc/host.h>
>> +#include <plat/sdhci.h>
>> +#include <linux/slab.h>
>> +#include "sdhci.h"
>> +#include "sdhci-pltfm.h"
>> +
>> +#define MMP2_SD_FIFO_PARAM             0x104
>> +#define MMP2_DIS_PAD_SD_CLK_GATE       0x400
>> +
>> +#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP             0x10A
>> +#define MMP2_SDCLK_SEL 0x100
>> +#define MMP2_SDCLK_DELAY_SHIFT 9
>> +#define MMP2_SDCLK_DELAY_MASK  0x1f
>> +
>> +#define SD_CFG_FIFO_PARAM       0x100
>> +#define SDCFG_GEN_PAD_CLK_ON   (1<<6)
>> +#define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
>> +#define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
>> +
>> +#define SD_SPI_MODE          0x108
>> +#define SD_CE_ATA_1          0x10C
>> +
>> +#define SD_CE_ATA_2          0x10E
>> +#define SDCE_MISC_INT          (1<<2)
>> +#define SDCE_MISC_INT_EN       (1<<1)
>> +
>> +static void mmp2_soc_set_timing(struct sdhci_host *host,
>> +                               struct sdhci_pxa_platdata *pdata)
>> +{
>> +       /*
>> +        * tune timing of read data/command when crc error happen
>> +        * no performance impact
>> +        */
>> +       if (pdata && 0 != pdata->clk_delay_cycles) {
>> +               u16 tmp;
>> +
>> +               tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>> +               tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
>> +                                       << MMP2_SDCLK_DELAY_SHIFT;
>> +               tmp |= MMP2_SDCLK_SEL;
>> +               writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>> +       }
>> +
>> +       /*
>> +        * disable clock gating per some cards requirement
>> +        */
>> +
>> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
>> +               u32 tmp = 0;
>> +
>> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
>> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
>> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
>> +       }
>> +}
>> +
>> +static unsigned int mmp2_get_ro(struct sdhci_host *host)
>> +{
>> +       /* Micro SD does not support write-protect feature */
>> +       return 0;
>> +}
>> +
>> +static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>> +
>> +       if (clock)
>> +               mmp2_soc_set_timing(host, pxa->pdata);
>> +}
>> +
>> +static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>> +{
>> +       u16 ctrl_2;
>> +
>> +       /*
>> +        * Set V18_EN -- UHS modes do not work without this.
>> +        * does not change signaling voltage
>> +        */
>> +       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> +
>> +       /* Select Bus Speed Mode for host */
>> +       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> +       switch (uhs) {
>> +       case MMC_TIMING_UHS_SDR12:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>> +               break;
>> +       case MMC_TIMING_UHS_SDR25:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>> +               break;
>> +       case MMC_TIMING_UHS_SDR50:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>> +               break;
>> +       case MMC_TIMING_UHS_SDR104:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>> +               break;
>> +       case MMC_TIMING_UHS_DDR50:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>> +               break;
>> +       }
>> +
>> +       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> +       dev_dbg(mmc_dev(host->mmc),
>> +               "%s:%s uhs = %d, ctrl_2 = %04X\n",
>> +               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>> +
>> +       return 0;
>> +}
>> +
>> +static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host;
>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>> +       struct device *dev = &pdev->dev;
>> +       struct sdhci_host *host = NULL;
>> +       struct sdhci_pxa *pxa = NULL;
>> +       int ret;
>> +       struct clk *clk;
>> +
>> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
>> +       if (!pxa) {
>> +               ret = -ENOMEM;
>> +               goto out;
>> +       }
>> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>> +       if (!pxa->ops) {
>> +               ret = -ENOMEM;
>> +               goto out;
>> +       }
>> +       pxa->pdata = pdata;
>> +
>> +       clk = clk_get(dev, "PXA-SDHCLK");
>> +       if (IS_ERR(clk)) {
>> +               dev_err(dev, "failed to get io clock\n");
>> +               ret = PTR_ERR(clk);
>> +               goto out;
>> +       }
>> +       pxa->clk = clk;
>> +       clk_enable(clk);
>> +
>> +       host = sdhci_pltfm_init(pdev, NULL);
>> +       if (IS_ERR(host))
>> +               return PTR_ERR(host);
>> +
>> +       pltfm_host = sdhci_priv(host);
>> +       pltfm_host->priv = pxa;
>> +
>> +       host->quirks = SDHCI_QUIRK_BROKEN_ADMA
>> +               | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
>> +               | SDHCI_QUIRK_32BIT_DMA_ADDR
>> +               | SDHCI_QUIRK_32BIT_DMA_SIZE
>> +               | SDHCI_QUIRK_32BIT_ADMA_SIZE
>> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>> +
>> +       /* enable 1/8V DDR capable */
>> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>> +
>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
>> +               /* on-chip device */
>> +               host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>> +               host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>> +       }
>> +
>> +       /* If slot design supports 8 bit data, indicate this to MMC. */
>> +       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>> +
>> +       if (pdata && pdata->quirks)
>> +               host->quirks |= pdata->quirks;
>> +       if (pdata && pdata->host_caps)
>> +               host->mmc->caps |= pdata->host_caps;
>> +       if (pdata && pdata->pm_caps)
>> +               host->mmc->pm_caps |= pdata->pm_caps;
>> +
>> +       pxa->ops->set_clock = mmp2_set_clock;
>> +       pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
>> +               pxa->ops->get_ro = mmp2_get_ro;
>> +
>> +       host->ops = pxa->ops;
>> +
>> +       ret = sdhci_add_host(host);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "failed to add host\n");
>> +               goto out;
>> +       }
>> +
>> +       platform_set_drvdata(pdev, host);
>> +
>> +       return 0;
>> +out:
>> +       if (host) {
>> +               clk_disable(pltfm_host->clk);
>> +               clk_put(pltfm_host->clk);
>> +               sdhci_pltfm_free(pdev);
>> +       }
>> +
>> +       if (pxa)
>> +               kfree(pxa->ops);
>> +       kfree(pxa);
>> +
>> +       return ret;
>> +}
>> +
>> +static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
>> +{
>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>> +       struct sdhci_host *host = platform_get_drvdata(pdev);
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>> +       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);
>> +
>> +               clk_disable(pxa->clk);
>> +               clk_put(pxa->clk);
>> +               sdhci_pltfm_free(pdev);
>> +
>> +               platform_set_drvdata(pdev, NULL);
>> +       }
>> +
>> +       if (pxa)
>> +               kfree(pxa->ops);
>> +       kfree(pxa);
>> +
>> +       return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
>> +{
>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>> +
>> +       return sdhci_suspend_host(host, state);
>> +}
>> +
>> +static int sdhci_mmp2_resume(struct platform_device *dev)
>> +{
>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>> +
>> +       return sdhci_resume_host(host);
>> +}
>> +#else
>> +#define sdhci_mmp2_suspend     NULL
>> +#define sdhci_mmp2_resume      NULL
>> +#endif
>> +
>> +
>> +static struct platform_driver sdhci_mmp2_driver = {
>> +       .driver         = {
>> +               .name   = "sdhci-mmp2",
>> +               .owner  = THIS_MODULE,
>> +       },
>> +       .probe          = sdhci_mmp2_probe,
>> +       .remove         = __devexit_p(sdhci_mmp2_remove),
>> +#ifdef CONFIG_PM
>> +       .suspend        = sdhci_mmp2_suspend,
>> +       .resume         = sdhci_mmp2_resume,
>> +#endif
>> +};
>> +static int __init sdhci_mmp2_init(void)
>> +{
>> +       return platform_driver_register(&sdhci_mmp2_driver);
>> +}
>> +
>> +static void __exit sdhci_mmp2_exit(void)
>> +{
>> +       platform_driver_unregister(&sdhci_mmp2_driver);
>> +}
>> +
>> +module_init(sdhci_mmp2_init);
>> +module_exit(sdhci_mmp2_exit);
>> +
>> +MODULE_DESCRIPTION("SDHCI driver for mmp2");
>> +MODULE_AUTHOR("Marvell International Ltd.");
>> +MODULE_LICENSE("GPL v2");
>> +
>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>> deleted file mode 100644
>> index 089c9a6..0000000
>> --- a/drivers/mmc/host/sdhci-pxa.c
>> +++ /dev/null
>> @@ -1,303 +0,0 @@
>> -/* 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>> -{
>> -       u16 ctrl_2;
>> -
>> -       /*
>> -        * Set V18_EN -- UHS modes do not work without this.
>> -        * does not change signaling voltage
>> -        */
>> -       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> -
>> -       /* Select Bus Speed Mode for host */
>> -       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> -       switch (uhs) {
>> -       case MMC_TIMING_UHS_SDR12:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>> -               break;
>> -       case MMC_TIMING_UHS_SDR25:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>> -               break;
>> -       case MMC_TIMING_UHS_SDR50:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>> -               break;
>> -       case MMC_TIMING_UHS_SDR104:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>> -               break;
>> -       case MMC_TIMING_UHS_DDR50:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>> -               break;
>> -       }
>> -
>> -       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> -       pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
>> -               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>> -
>> -       return 0;
>> -}
>> -
>> -static struct sdhci_ops sdhci_pxa_ops = {
>> -       .set_uhs_signaling = set_uhs_signaling,
>> -       .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");
>> -               return PTR_ERR(host);
>> -       }
>> -
>> -       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
>> -               | SDHCI_QUIRK_32BIT_DMA_ADDR
>> -               | SDHCI_QUIRK_32BIT_DMA_SIZE
>> -               | SDHCI_QUIRK_32BIT_ADMA_SIZE
>> -               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>> -
>> -       if (pdata->quirks)
>> -               host->quirks |= pdata->quirks;
>> -
>> -       /* enable 1/8V DDR capable */
>> -       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>> -
>> -       /* If slot design supports 8 bit data, indicate this to MMC. */
>> -       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>> -               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>> -
>> -       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) {
>> -               clk_put(pxa->clk);
>> -               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, pm_message_t state)
>> -{
>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>> -
>> -       return sdhci_suspend_host(host, state);
>> -}
>> -
>> -static int sdhci_pxa_resume(struct platform_device *dev)
>> -{
>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>> -
>> -       return sdhci_resume_host(host);
>> -}
>> -#else
>> -#define sdhci_pxa_suspend      NULL
>> -#define sdhci_pxa_resume       NULL
>> -#endif
>> -
>> -static struct platform_driver sdhci_pxa_driver = {
>> -       .probe          = sdhci_pxa_probe,
>> -       .remove         = __devexit_p(sdhci_pxa_remove),
>> -       .suspend        = sdhci_pxa_suspend,
>> -       .resume         = sdhci_pxa_resume,
>> -       .driver         = {
>> -               .name   = DRIVER_NAME,
>> -               .owner  = THIS_MODULE,
>> -       },
>> -};
>> -
>> -/*****************************************************************************\
>> - *                                                                           *
>> - * 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
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philip Rakity May 28, 2011, 5 a.m. UTC | #5
Hi Zhangfei,


comments below. (based on V1 patch but change to V2 affect (3<<1) typo.

Philip and Mark.


On May 23, 2011, at 6:21 AM, Zhangfei Gao wrote:

>        Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
> arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
> drivers/mmc/host/Kconfig               |    9 +-
> drivers/mmc/host/Makefile              |    2 +-
> drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
> 5 files changed, 349 insertions(+), 312 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-mmp2.c
> delete 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
> index 1ab332e..92becc0 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -13,23 +13,58 @@
> #ifndef __PLAT_PXA_SDHCI_H
> #define __PLAT_PXA_SDHCI_H
>
> +#include <linux/mmc/sdhci.h>
> +#include <linux/platform_device.h>
> +
> /* pxa specific flag */
> /* Require clock free running */
> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> -
> +/* card alwayes wired to host, like on-chip emmc */
> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
> /* Board design supports 8-bit data on SD/SDIO BUS */
> #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
> +/* card not use wp, such as micro sd card */
> +#define PXA_FLAG_CARD_NO_WP    (3<<1)

do you mean (1<<3) ?
>
> +struct sdhci_pxa;
> /*
>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
> - * @max_speed: the maximum speed supported
> - * @quirks: quirks of specific device
>  * @flags: flags for platform requirement
> + * @clk_delay_cycles:
> + *     mmp2: each step is roughly 100ps, 5bits width
> + *     pxa910: each step is 1ns, 4bits width
> + * @clk_delay_sel: select clk_delay, used on pxa910
> + *     0: choose feedback clk
> + *     1: choose feedback clk + delay value
> + *     2: choose internal clk
> + * @ext_cd_gpio: gpio pin used for external CD line
> + * @ext_cd_gpio_invert: invert values for external CD gpio line
> + * @clk_delay_enable: enable clk_delay or not, used on pxa910
> + * @max_speed: the maximum speed supported
> + * @host_caps: Standard MMC host capabilities bit field.
> + * @quirks: quirks of platfrom
> + * @pm_caps: pm_caps of platfrom
>  */
> struct sdhci_pxa_platdata {
> +       unsigned int    flags;
> +       unsigned int    clk_delay_cycles;
> +       unsigned int    clk_delay_sel;
> +       unsigned int    ext_cd_gpio;
> +       bool            ext_cd_gpio_invert;
> +       bool            clk_delay_enable;

move up 2 lines to be next to other clk_ values.

>        unsigned int    max_speed;
> +       unsigned int    host_caps;
>        unsigned int    quirks;
> -       unsigned int    flags;
> +       unsigned int    pm_caps;
> +};
> +
> +struct sdhci_pxa {
> +       struct sdhci_pxa_platdata       *pdata;
> +       struct clk                      *clk;
> +       struct sdhci_ops                *ops;
> +
> +       u8      clk_enable;
> +       u8      power_mode;
> };
>
> #endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 862235e..1e520e3 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -181,14 +181,15 @@ config MMC_SDHCI_S3C
>
>          If unsure, say N.
>
> -config MMC_SDHCI_PXA
> -       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
> +config MMC_SDHCI_MMP2
> +       tristate "Marvell MMP2 SD Host Controller support"
>        depends on ARCH_PXA || ARCH_MMP
>        select MMC_SDHCI
> +       select MMC_SDHCI_PLTFM
>        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
> +         This selects the Marvell(R) MMP2 SD Host Controller.
> +         If you have a MMP2 platform with SD Host Controller
>          and a card slot, say Y or M here.
>

This needs to be specific to MMP2  -- depends on CPU_MMP2 ?
code can be selected for pxa9xx and pxa168 where this code cannot work.

deleting MMC_SDHCI_PXA breaks existing configs since it is renamed.
would it better to leave this and mark it as obsolete but let it still compile
mmp2 sd code?

In some sense all of this is backwards.  Once the SoC is known (pxa910 or mmp2),
it knows that SDHCI controller it supports.  It should set a config option
like USES_MRVL_V3_SD_CONTROLLER.  The Kconfig entry in mmc/host should
depend on THAT selection to allow the user to enable the SD option.

MMC_SDHCI_IO_ACCESSORS not needed


>


>          If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 4933004..f8650e0 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_MMC_MXC)           += mxcmmc.o
> obj-$(CONFIG_MMC_MXS)          += mxs-mmc.o
> obj-$(CONFIG_MMC_SDHCI)                += sdhci.o
> obj-$(CONFIG_MMC_SDHCI_PCI)    += sdhci-pci.o
> -obj-$(CONFIG_MMC_SDHCI_PXA)    += sdhci-pxa.o
> +obj-$(CONFIG_MMC_SDHCI_MMP2)   += sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
> new file mode 100644
> index 0000000..566d525
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-mmp2.c

It is probably better to NOT name the file sdhci-mmp2.c but
something like sdchi-pxaV3.c since other SoC's may use this controller.

> @@ -0,0 +1,304 @@
> +/*
> + * Copyright (C) 2010 Marvell International Ltd.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mmc/host.h>
> +#include <plat/sdhci.h>
> +#include <linux/slab.h>
> +#include "sdhci.h"
> +#include "sdhci-pltfm.h"
> +
> +#define MMP2_SD_FIFO_PARAM             0x104
> +#define MMP2_DIS_PAD_SD_CLK_GATE       0x400
> +
> +#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP             0x10A
> +#define MMP2_SDCLK_SEL 0x100
> +#define MMP2_SDCLK_DELAY_SHIFT 9
> +#define MMP2_SDCLK_DELAY_MASK  0x1f
> +
> +#define SD_CFG_FIFO_PARAM       0x100
> +#define SDCFG_GEN_PAD_CLK_ON   (1<<6)
> +#define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
> +#define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
> +
> +#define SD_SPI_MODE          0x108
> +#define SD_CE_ATA_1          0x10C
> +
> +#define SD_CE_ATA_2          0x10E
> +#define SDCE_MISC_INT          (1<<2)
> +#define SDCE_MISC_INT_EN       (1<<1)
> +
> +static void mmp2_soc_set_timing(struct sdhci_host *host,
> +                               struct sdhci_pxa_platdata *pdata)
> +{
> +       /*
> +        * tune timing of read data/command when crc error happen
> +        * no performance impact
> +        */
> +       if (pdata && 0 != pdata->clk_delay_cycles) {
> +               u16 tmp;
> +
> +               tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
> +               tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
> +                                       << MMP2_SDCLK_DELAY_SHIFT;
> +               tmp |= MMP2_SDCLK_SEL;
> +               writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
> +       }
> +
> +       /*
> +        * disable clock gating per some cards requirement
> +        */
> +
> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
> +               u32 tmp = 0;
> +
> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
> +       }
> +}

One cannot know what card if being inserted or removed by a user
so the safest choice is to leave clock gating OFF by default.

PXA_FLAG_DISABLE_CLOCK_GATING should be renamed
to
PXA_FLAG_ENABLE_CLOCK_GATING.

If you have a card which is permanent then the flags should be used
to ENABLE clock gating.  Otherwise clock gating should be left OFF by default.

The above code should be called via the call back added to sdhci.c for
reset processing at the end of a reset and tested against a RESET_ALL.
The RESET_ALL will reset the private register space.


> +
> +static unsigned int mmp2_get_ro(struct sdhci_host *host)
> +{
> +       /* Micro SD does not support write-protect feature */
> +       return 0;
> +}
> +
> +static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +
> +       if (clock)
> +               mmp2_soc_set_timing(host, pxa->pdata);
> +}
> +


This code should be called via reset callback and mmp2_soc_set_timing added here
and prototype changed to
+static void mmp2_set_private_registers(struct sdhci_host *host, unsigned int reset_flag)

 and used as

        if (reset_flag == RESET_ALL)
                mmp2_soc_set_timing(host, pxa->pdata);
        }


> +static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
> +{
> +       u16 ctrl_2;
> +
> +       /*
> +        * Set V18_EN -- UHS modes do not work without this.
> +        * does not change signaling voltage
> +        */
> +       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> +       /* Select Bus Speed Mode for host */
> +       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> +       switch (uhs) {
> +       case MMC_TIMING_UHS_SDR12:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> +               break;
> +       case MMC_TIMING_UHS_SDR25:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> +               break;
> +       case MMC_TIMING_UHS_SDR50:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
> +               break;
> +       case MMC_TIMING_UHS_SDR104:
> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
> +               break;
> +       case MMC_TIMING_UHS_DDR50:
> +               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
> +               break;
> +       }
> +
> +       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +       dev_dbg(mmc_dev(host->mmc),
> +               "%s:%s uhs = %d, ctrl_2 = %04X\n",
> +               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
> +
> +       return 0;
> +}
> +
> +static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
> +{
> +       struct sdhci_pltfm_host *pltfm_host;
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +       struct device *dev = &pdev->dev;
> +       struct sdhci_host *host = NULL;
> +       struct sdhci_pxa *pxa = NULL;
> +       int ret;
> +       struct clk *clk;
> +
> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
> +       if (!pxa) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
why is  pxa->ops needed ?

> +       if (!pxa->ops) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +       pxa->pdata = pdata;
> +
> +       clk = clk_get(dev, "PXA-SDHCLK");
> +       if (IS_ERR(clk)) {
> +               dev_err(dev, "failed to get io clock\n");
> +               ret = PTR_ERR(clk);
> +               goto out;
> +       }
> +       pxa->clk = clk;
> +       clk_enable(clk);
> +
> +       host = sdhci_pltfm_init(pdev, NULL);
> +       if (IS_ERR(host))
> +               return PTR_ERR(host);
> +
> +       pltfm_host = sdhci_priv(host);
> +       pltfm_host->priv = pxa;
> +
> +       host->quirks = SDHCI_QUIRK_BROKEN_ADMA
> +               | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
> +               | SDHCI_QUIRK_32BIT_DMA_ADDR
> +               | SDHCI_QUIRK_32BIT_DMA_SIZE
> +               | SDHCI_QUIRK_32BIT_ADMA_SIZE
> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
> +
> +       /* enable 1/8V DDR capable */
> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;


missing cap for bus width testing CMD19 works on MMP2
ADMA is fine for SD/eMMC -- SDMA is best for SDIO.  SDHCI_QUIRK_BROKEN_ADMA
is best passed via pdata->quirks.


> +
> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
> +               /* on-chip device */
> +               host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +               host->mmc->caps |= MMC_CAP_NONREMOVABLE;
> +       }
> +
> +       /* If slot design supports 8 bit data, indicate this to MMC. */
> +       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> +
> +       if (pdata && pdata->quirks)
> +               host->quirks |= pdata->quirks;
> +       if (pdata && pdata->host_caps)
> +               host->mmc->caps |= pdata->host_caps;
> +       if (pdata && pdata->pm_caps)
> +               host->mmc->pm_caps |= pdata->pm_caps;
> +
> +       pxa->ops->set_clock = mmp2_set_clock;

see comment about using reset callback instead of set_clock.

> +       pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
> +       if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
> +               pxa->ops->get_ro = mmp2_get_ro;

need 74 clock code --- I pasted it at the end the code.  The reason the code is
needed is that SOME eMMC chips require 74 clocks.  The chips you have tested with
do not need the clocks but JEDEC  Spec JESD84-A441-1 requires the clocks be
generated.


> +
> +       host->ops = pxa->ops;
> +
> +       ret = sdhci_add_host(host);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to add host\n");
> +               goto out;
> +       }
> +
> +       platform_set_drvdata(pdev, host);
> +
> +       return 0;
> +out:
> +       if (host) {
> +               clk_disable(pltfm_host->clk);
> +               clk_put(pltfm_host->clk);
> +               sdhci_pltfm_free(pdev);
> +       }
> +
> +       if (pxa)
> +               kfree(pxa->ops);
> +       kfree(pxa);
> +
> +       return ret;
> +}
> +
> +static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
> +{
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +       struct sdhci_host *host = platform_get_drvdata(pdev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +       int dead = 0;
> +       u32 scratch;
> +
> +       if (host) {
> +               scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +               if (scratch == (u32)-1)
> +                       dead = 1;

not sure that this is the correct test -- will masked interrupts show
up as set in the INT_STATUS register ?  from the code in sdhci.c and from
my testing I have never seen -1 set.

The test in sdhci.c is for 0x0 or for 0xffffffff is for a
shared interrupt line to know if the interrupt is for this controller.

> +
> +               sdhci_remove_host(host, dead);
> +
> +               clk_disable(pxa->clk);
> +               clk_put(pxa->clk);
> +               sdhci_pltfm_free(pdev);
> +
> +               platform_set_drvdata(pdev, NULL);
> +       }
> +
> +       if (pxa)
> +               kfree(pxa->ops);
> +       kfree(pxa);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(dev);
> +
> +       return sdhci_suspend_host(host, state);
> +}
> +
> +static int sdhci_mmp2_resume(struct platform_device *dev)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(dev);
> +
> +       return sdhci_resume_host(host);
> +}
> +#else
> +#define sdhci_mmp2_suspend     NULL
> +#define sdhci_mmp2_resume      NULL
> +#endif
> +
> +
> +static struct platform_driver sdhci_mmp2_driver = {
> +       .driver         = {
> +               .name   = "sdhci-mmp2",
> +               .owner  = THIS_MODULE,
> +       },
> +       .probe          = sdhci_mmp2_probe,
> +       .remove         = __devexit_p(sdhci_mmp2_remove),
> +#ifdef CONFIG_PM
> +       .suspend        = sdhci_mmp2_suspend,
> +       .resume         = sdhci_mmp2_resume,
> +#endif
> +};
> +static int __init sdhci_mmp2_init(void)
> +{
> +       return platform_driver_register(&sdhci_mmp2_driver);
> +}
> +
> +static void __exit sdhci_mmp2_exit(void)
> +{
> +       platform_driver_unregister(&sdhci_mmp2_driver);
> +}
> +
> +module_init(sdhci_mmp2_init);
> +module_exit(sdhci_mmp2_exit);
> +
> +MODULE_DESCRIPTION("SDHCI driver for mmp2");
> +MODULE_AUTHOR("Marvell International Ltd.");
> +MODULE_LICENSE("GPL v2");
> +
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> deleted file mode 100644
> index 089c9a6..0000000
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ /dev/null
> @@ -1,303 +0,0 @@
> -/* 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>
> - *

Keep the authors.  They all contributed to the code.

> - * 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
> -{
> -       u16 ctrl_2;
> -
> -       /*
> -        * Set V18_EN -- UHS modes do not work without this.
> -        * does not change signaling voltage
> -        */
> -       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> -
> -       /* Select Bus Speed Mode for host */
> -       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> -       switch (uhs) {
> -       case MMC_TIMING_UHS_SDR12:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> -               break;
> -       case MMC_TIMING_UHS_SDR25:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> -               break;
> -       case MMC_TIMING_UHS_SDR50:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
> -               break;
> -       case MMC_TIMING_UHS_SDR104:
> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
> -               break;
> -       case MMC_TIMING_UHS_DDR50:
> -               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
> -               break;
> -       }
> -
> -       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> -       pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
> -               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
> -
> -       return 0;
> -}
> -
> -static struct sdhci_ops sdhci_pxa_ops = {
> -       .set_uhs_signaling = set_uhs_signaling,
> -       .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");
> -               return PTR_ERR(host);
> -       }
> -
> -       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
> -               | SDHCI_QUIRK_32BIT_DMA_ADDR
> -               | SDHCI_QUIRK_32BIT_DMA_SIZE
> -               | SDHCI_QUIRK_32BIT_ADMA_SIZE
> -               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
> -
> -       if (pdata->quirks)
> -               host->quirks |= pdata->quirks;
> -
> -       /* enable 1/8V DDR capable */
> -       host->mmc->caps |= MMC_CAP_1_8V_DDR;
> -
> -       /* If slot design supports 8 bit data, indicate this to MMC. */
> -       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
> -               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> -
> -       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) {
> -               clk_put(pxa->clk);
> -               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, pm_message_t state)
> -{
> -       struct sdhci_host *host = platform_get_drvdata(dev);
> -
> -       return sdhci_suspend_host(host, state);
> -}
> -
> -static int sdhci_pxa_resume(struct platform_device *dev)
> -{
> -       struct sdhci_host *host = platform_get_drvdata(dev);
> -
> -       return sdhci_resume_host(host);
> -}
> -#else
> -#define sdhci_pxa_suspend      NULL
> -#define sdhci_pxa_resume       NULL
> -#endif
> -
> -static struct platform_driver sdhci_pxa_driver = {
> -       .probe          = sdhci_pxa_probe,
> -       .remove         = __devexit_p(sdhci_pxa_remove),
> -       .suspend        = sdhci_pxa_suspend,
> -       .resume         = sdhci_pxa_resume,
> -       .driver         = {
> -               .name   = DRIVER_NAME,
> -               .owner  = THIS_MODULE,
> -       },
> -};
> -
> -/*****************************************************************************\
> - *                                                                           *
> - * 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




code to send 74 clocks when card is inserted.  Need to callback hook so
code is called from sdhci.c

from patch submitted to linux-mmc

from: Philip Rakity <prakity@marvell.com>
Date: February 13, 2011 11:24:18 PM PST
To: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
Cc: Mark Brown <markb@marvell.com>
Subject: [PATCH] sdhci: Add support PXA168, PXA910, and MMP2 controllers


+static void generate_initial_74_clocks(struct sdhci_host *host, u8 power_mode)
+{
+       struct sdhci_pxa *pxa = sdhci_priv(host);
+       u16 tmp;
+       int count;
+
+       if (pxa->power_mode == MMC_POWER_UP
+       && power_mode == MMC_POWER_ON) {
+
+               pr_debug("%s:%s ENTER: slot->power_mode = %d,"
+                       "ios->power_mode = %d\n",
+                       __func__,
+                       mmc_hostname(host->mmc),
+                       pxa->power_mode,
+                       power_mode);
+
+               /* set we want notice of when 74 clocks are sent */
+               tmp = readw(host->ioaddr + SD_CE_ATA_2);
+               tmp |= SDCE_MISC_INT_EN;
+               writew(tmp, host->ioaddr + SD_CE_ATA_2);
+
+               /* start sending the 74 clocks */
+               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
+               tmp |= SDCFG_GEN_PAD_CLK_ON;
+               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
+
+               /* slowest speed is about 100KHz or 10usec per clock */
+               udelay(740);
+               count = 0;
+#define MAX_WAIT_COUNT 5
+               while (count++ < MAX_WAIT_COUNT) {
+                       if ((readw(host->ioaddr + SD_CE_ATA_2)
+                                       & SDCE_MISC_INT) == 0)
+                               break;
+                       udelay(10);
+               }
+
+               if (count == MAX_WAIT_COUNT)
+                       printk(KERN_WARNING"%s: %s: 74 clock interrupt "
+                               "not cleared\n",
+                               __func__, mmc_hostname(host->mmc));
+               /* clear the interrupt bit if posted */
+               tmp = readw(host->ioaddr + SD_CE_ATA_2);
+               tmp |= SDCE_MISC_INT;
+               writew(tmp, host->ioaddr + SD_CE_ATA_2);
+       }
+       pxa->power_mode = power_mode;
+}
+

>

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann May 28, 2011, 8:52 a.m. UTC | #6
On Saturday 28 May 2011 07:00:38 Philip Rakity wrote:
> In some sense all of this is backwards.  Once the SoC is known (pxa910 or mmp2),
> it knows that SDHCI controller it supports.  It should set a config option
> like USES_MRVL_V3_SD_CONTROLLER.  The Kconfig entry in mmc/host should
> depend on THAT selection to allow the user to enable the SD option.
 
I would actually prefer in general if the Kconfig file listed only
the strictly necessary dependencies for building the driver.
If this driver can be built anywhere, I would list no dependency at
all. If it depends on something ARM specific, I'd make it depend
on CONFIG_ARM.

Then change the defconfig for the particular board to enable the
driver.

The main advantage of this is to increase build coverage on test
building machines doing an allyesconfig and randconfig once we
get there (right now, these have too many build errors, but we
have plans to work on that). 
 
> > --- /dev/null
> > +++ b/drivers/mmc/host/sdhci-mmp2.c
> 
> It is probably better to NOT name the file sdhci-mmp2.c but
> something like sdchi-pxaV3.c since other SoC's may use this controller.

Good point.

> > +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
> > +       if (!pxa) {
> > +               ret = -ENOMEM;
> > +               goto out;
> > +       }
> > +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
> why is  pxa->ops needed ?

I guess the idea was to be able to free the structure later. I already
commented that it should be statically allocation instead of kzalloc,
so that would make the pointer unnecessary.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philip Rakity May 28, 2011, 4:25 p.m. UTC | #7
On May 28, 2011, at 1:52 AM, Arnd Bergmann wrote:

> On Saturday 28 May 2011 07:00:38 Philip Rakity wrote:
>> In some sense all of this is backwards.  Once the SoC is known (pxa910 or mmp2),
>> it knows that SDHCI controller it supports.  It should set a config option
>> like USES_MRVL_V3_SD_CONTROLLER.  The Kconfig entry in mmc/host should
>> depend on THAT selection to allow the user to enable the SD option.
> 
> I would actually prefer in general if the Kconfig file listed only
> the strictly necessary dependencies for building the driver.
> If this driver can be built anywhere, I would list no dependency at
> all. If it depends on something ARM specific, I'd make it depend
> on CONFIG_ARM.
> 
> Then change the defconfig for the particular board to enable the
> driver.
> 
> The main advantage of this is to increase build coverage on test
> building machines doing an allyesconfig and randconfig once we
> get there (right now, these have too many build errors, but we
> have plans to work on that). 

The controller is built into the mmp2 SoC.  No build error could
occur if once the SoC is determined it selected the type of
controller available (in the arch/arm).  Like the patch you
helped me with a while ago (which never was accepted).

The Kconfig entry for MMP2 in drivers/mmc/host would
add the line 
depends on 

This is not a general as depending on ARM but at least
the code would work.

The best solution would be far more general and involve
generic probing and registration but that is a lot of work and
should be done for all of the arm/ directory. 

> 
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/sdhci-mmp2.c
>> 
>> It is probably better to NOT name the file sdhci-mmp2.c but
>> something like sdchi-pxaV3.c since other SoC's may use this controller.
> 
> Good point.
> 
>>> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
>>> +       if (!pxa) {
>>> +               ret = -ENOMEM;
>>> +               goto out;
>>> +       }
>>> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>> why is  pxa->ops needed ?
> 
> I guess the idea was to be able to free the structure later. I already
> commented that it should be statically allocation instead of kzalloc,
> so that would make the pointer unnecessary.

I do not understand why pxa->ops is needed at all.  More general 
question.

> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann May 28, 2011, 7:04 p.m. UTC | #8
On Saturday 28 May 2011 18:25:00 Philip Rakity wrote:
> On May 28, 2011, at 1:52 AM, Arnd Bergmann wrote:
> > I would actually prefer in general if the Kconfig file listed only
> > the strictly necessary dependencies for building the driver.
> > If this driver can be built anywhere, I would list no dependency at
> > all. If it depends on something ARM specific, I'd make it depend
> > on CONFIG_ARM.
> > 
> > Then change the defconfig for the particular board to enable the
> > driver.
> > 
> > The main advantage of this is to increase build coverage on test
> > building machines doing an allyesconfig and randconfig once we
> > get there (right now, these have too many build errors, but we
> > have plans to work on that). 
> 
> The controller is built into the mmp2 SoC.  No build error could
> occur if once the SoC is determined it selected the type of
> controller available (in the arch/arm).  Like the patch you
> helped me with a while ago (which never was accepted).

My point was not avoiding build errors in this driver, which is
fairly easy and obviously needs to happen. I want to enable the
driver (and most others) to be built in all cases that don't
cause a build error, instead of limiting them to very few
configurations.

> The Kconfig entry for MMP2 in drivers/mmc/host would
> add the line 
> depends on 
> 
> This is not a general as depending on ARM but at least
> the code would work.
> 
> The best solution would be far more general and involve
> generic probing and registration but that is a lot of work and
> should be done for all of the arm/ directory. 

We actually do all the generic probing based on the machine
type already. Any place where we don't do that and hardcode
the presence of a device based on a compile-time option should
be considered a bug.

> > I guess the idea was to be able to free the structure later. I already
> > commented that it should be statically allocation instead of kzalloc,
> > so that would make the pointer unnecessary.
> 
> I do not understand why pxa->ops is needed at all.  More general 
> question.

pxa->ops is the same as host->ops, which is required by the base
sdhci driver.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao May 30, 2011, 1:15 p.m. UTC | #9
On Sat, May 28, 2011 at 1:00 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> Hi Zhangfei,
>
>
> comments below. (based on V1 patch but change to V2 affect (3<<1) typo.
>
> Philip and Mark.

Thanks for review.
>
>
> On May 23, 2011, at 6:21 AM, Zhangfei Gao wrote:
>
>>        Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>> arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
>> drivers/mmc/host/Kconfig               |    9 +-
>> drivers/mmc/host/Makefile              |    2 +-
>> drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
>> drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
>> 5 files changed, 349 insertions(+), 312 deletions(-)
>> create mode 100644 drivers/mmc/host/sdhci-mmp2.c
>> delete 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
>> index 1ab332e..92becc0 100644
>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>> @@ -13,23 +13,58 @@
>> #ifndef __PLAT_PXA_SDHCI_H
>> #define __PLAT_PXA_SDHCI_H
>>
>> +#include <linux/mmc/sdhci.h>
>> +#include <linux/platform_device.h>
>> +
>> /* pxa specific flag */
>> /* Require clock free running */
>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>> -
>> +/* card alwayes wired to host, like on-chip emmc */
>> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
>> /* Board design supports 8-bit data on SD/SDIO BUS */
>> #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
>> +/* card not use wp, such as micro sd card */
>> +#define PXA_FLAG_CARD_NO_WP    (3<<1)
>
> do you mean (1<<3) ?
already update in v2.
>>
>> +struct sdhci_pxa;
>> /*
>>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>> - * @max_speed: the maximum speed supported
>> - * @quirks: quirks of specific device
>>  * @flags: flags for platform requirement
>> + * @clk_delay_cycles:
>> + *     mmp2: each step is roughly 100ps, 5bits width
>> + *     pxa910: each step is 1ns, 4bits width
>> + * @clk_delay_sel: select clk_delay, used on pxa910
>> + *     0: choose feedback clk
>> + *     1: choose feedback clk + delay value
>> + *     2: choose internal clk
>> + * @ext_cd_gpio: gpio pin used for external CD line
>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910
>> + * @max_speed: the maximum speed supported
>> + * @host_caps: Standard MMC host capabilities bit field.
>> + * @quirks: quirks of platfrom
>> + * @pm_caps: pm_caps of platfrom
>>  */
>> struct sdhci_pxa_platdata {
>> +       unsigned int    flags;
>> +       unsigned int    clk_delay_cycles;
>> +       unsigned int    clk_delay_sel;
>> +       unsigned int    ext_cd_gpio;
>> +       bool            ext_cd_gpio_invert;
>> +       bool            clk_delay_enable;
>
> move up 2 lines to be next to other clk_ values.
fine.
>
>>        unsigned int    max_speed;
>> +       unsigned int    host_caps;
>>        unsigned int    quirks;
>> -       unsigned int    flags;
>> +       unsigned int    pm_caps;
>> +};
>> +
>> +struct sdhci_pxa {
>> +       struct sdhci_pxa_platdata       *pdata;
>> +       struct clk                      *clk;
>> +       struct sdhci_ops                *ops;
>> +
>> +       u8      clk_enable;
>> +       u8      power_mode;
>> };
>>
>> #endif /* __PLAT_PXA_SDHCI_H */
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 862235e..1e520e3 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -181,14 +181,15 @@ config MMC_SDHCI_S3C
>>
>>          If unsure, say N.
>>
>> -config MMC_SDHCI_PXA
>> -       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
>> +config MMC_SDHCI_MMP2
>> +       tristate "Marvell MMP2 SD Host Controller support"
>>        depends on ARCH_PXA || ARCH_MMP
>>        select MMC_SDHCI
>> +       select MMC_SDHCI_PLTFM
>>        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
>> +         This selects the Marvell(R) MMP2 SD Host Controller.
>> +         If you have a MMP2 platform with SD Host Controller
>>          and a card slot, say Y or M here.
>>
>
> This needs to be specific to MMP2  -- depends on CPU_MMP2 ?
> code can be selected for pxa9xx and pxa168 where this code cannot work.
>
> deleting MMC_SDHCI_PXA breaks existing configs since it is renamed.
> would it better to leave this and mark it as obsolete but let it still compile
> mmp2 sd code?
>
> In some sense all of this is backwards.  Once the SoC is known (pxa910 or mmp2),
> it knows that SDHCI controller it supports.  It should set a config option
> like USES_MRVL_V3_SD_CONTROLLER.  The Kconfig entry in mmc/host should
> depend on THAT selection to allow the user to enable the SD option.
>
> MMC_SDHCI_IO_ACCESSORS not needed
>
Will take Arnd advice, remove dependency and only select MMC_SDHCI and
MMC_SDHCI_PLTFM
>
>>
>
>
>>          If unsure, say N.
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 4933004..f8650e0 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -9,7 +9,7 @@ obj-$(CONFIG_MMC_MXC)           += mxcmmc.o
>> obj-$(CONFIG_MMC_MXS)          += mxs-mmc.o
>> obj-$(CONFIG_MMC_SDHCI)                += sdhci.o
>> obj-$(CONFIG_MMC_SDHCI_PCI)    += sdhci-pci.o
>> -obj-$(CONFIG_MMC_SDHCI_PXA)    += sdhci-pxa.o
>> +obj-$(CONFIG_MMC_SDHCI_MMP2)   += sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
>> new file mode 100644
>> index 0000000..566d525
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-mmp2.c
>
> It is probably better to NOT name the file sdhci-mmp2.c but
> something like sdchi-pxaV3.c since other SoC's may use this controller.
fine
>
>> @@ -0,0 +1,304 @@
>> +/*
>> + * Copyright (C) 2010 Marvell International Ltd.
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/gpio.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/mmc/host.h>
>> +#include <plat/sdhci.h>
>> +#include <linux/slab.h>
>> +#include "sdhci.h"
>> +#include "sdhci-pltfm.h"
>> +
>> +#define MMP2_SD_FIFO_PARAM             0x104
>> +#define MMP2_DIS_PAD_SD_CLK_GATE       0x400
>> +
>> +#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP             0x10A
>> +#define MMP2_SDCLK_SEL 0x100
>> +#define MMP2_SDCLK_DELAY_SHIFT 9
>> +#define MMP2_SDCLK_DELAY_MASK  0x1f
>> +
>> +#define SD_CFG_FIFO_PARAM       0x100
>> +#define SDCFG_GEN_PAD_CLK_ON   (1<<6)
>> +#define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
>> +#define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
>> +
>> +#define SD_SPI_MODE          0x108
>> +#define SD_CE_ATA_1          0x10C
>> +
>> +#define SD_CE_ATA_2          0x10E
>> +#define SDCE_MISC_INT          (1<<2)
>> +#define SDCE_MISC_INT_EN       (1<<1)
>> +
>> +static void mmp2_soc_set_timing(struct sdhci_host *host,
>> +                               struct sdhci_pxa_platdata *pdata)
>> +{
>> +       /*
>> +        * tune timing of read data/command when crc error happen
>> +        * no performance impact
>> +        */
>> +       if (pdata && 0 != pdata->clk_delay_cycles) {
>> +               u16 tmp;
>> +
>> +               tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>> +               tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
>> +                                       << MMP2_SDCLK_DELAY_SHIFT;
>> +               tmp |= MMP2_SDCLK_SEL;
>> +               writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>> +       }
>> +
>> +       /*
>> +        * disable clock gating per some cards requirement
>> +        */
>> +
>> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
>> +               u32 tmp = 0;
>> +
>> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
>> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
>> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
>> +       }
>> +}
>
> One cannot know what card if being inserted or removed by a user
> so the safest choice is to leave clock gating OFF by default.
>
> PXA_FLAG_DISABLE_CLOCK_GATING should be renamed
> to
> PXA_FLAG_ENABLE_CLOCK_GATING.
>
> If you have a card which is permanent then the flags should be used
> to ENABLE clock gating.  Otherwise clock gating should be left OFF by default.
Still in check.

>
> The above code should be called via the call back added to sdhci.c for
> reset processing at the end of a reset and tested against a RESET_ALL.
> The RESET_ALL will reset the private register space.
OK, it also workable to put in platform_reset_exit.
>
>
>> +
>> +static unsigned int mmp2_get_ro(struct sdhci_host *host)
>> +{
>> +       /* Micro SD does not support write-protect feature */
>> +       return 0;
>> +}
>> +
>> +static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>> +
>> +       if (clock)
>> +               mmp2_soc_set_timing(host, pxa->pdata);
>> +}
>> +
>
>
> This code should be called via reset callback and mmp2_soc_set_timing added here
> and prototype changed to
> +static void mmp2_set_private_registers(struct sdhci_host *host, unsigned int reset_flag)
>
>  and used as
>
>        if (reset_flag == RESET_ALL)
>                mmp2_soc_set_timing(host, pxa->pdata);
>        }
>
>
>> +static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>> +{
>> +       u16 ctrl_2;
>> +
>> +       /*
>> +        * Set V18_EN -- UHS modes do not work without this.
>> +        * does not change signaling voltage
>> +        */
>> +       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> +
>> +       /* Select Bus Speed Mode for host */
>> +       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> +       switch (uhs) {
>> +       case MMC_TIMING_UHS_SDR12:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>> +               break;
>> +       case MMC_TIMING_UHS_SDR25:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>> +               break;
>> +       case MMC_TIMING_UHS_SDR50:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>> +               break;
>> +       case MMC_TIMING_UHS_SDR104:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>> +               break;
>> +       case MMC_TIMING_UHS_DDR50:
>> +               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>> +               break;
>> +       }
>> +
>> +       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> +       dev_dbg(mmc_dev(host->mmc),
>> +               "%s:%s uhs = %d, ctrl_2 = %04X\n",
>> +               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>> +
>> +       return 0;
>> +}
>> +
>> +static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host;
>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>> +       struct device *dev = &pdev->dev;
>> +       struct sdhci_host *host = NULL;
>> +       struct sdhci_pxa *pxa = NULL;
>> +       int ret;
>> +       struct clk *clk;
>> +
>> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
>> +       if (!pxa) {
>> +               ret = -ENOMEM;
>> +               goto out;
>> +       }
>> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
> why is  pxa->ops needed ?
>
>> +       if (!pxa->ops) {
>> +               ret = -ENOMEM;
>> +               goto out;
>> +       }
>> +       pxa->pdata = pdata;
>> +
>> +       clk = clk_get(dev, "PXA-SDHCLK");
>> +       if (IS_ERR(clk)) {
>> +               dev_err(dev, "failed to get io clock\n");
>> +               ret = PTR_ERR(clk);
>> +               goto out;
>> +       }
>> +       pxa->clk = clk;
>> +       clk_enable(clk);
>> +
>> +       host = sdhci_pltfm_init(pdev, NULL);
>> +       if (IS_ERR(host))
>> +               return PTR_ERR(host);
>> +
>> +       pltfm_host = sdhci_priv(host);
>> +       pltfm_host->priv = pxa;
>> +
>> +       host->quirks = SDHCI_QUIRK_BROKEN_ADMA
>> +               | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL

>> +               | SDHCI_QUIRK_32BIT_DMA_ADDR
>> +               | SDHCI_QUIRK_32BIT_DMA_SIZE
>> +               | SDHCI_QUIRK_32BIT_ADMA_SIZE
my understand is mmp2 do not have such limitation?

>> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
This quirk also not must?
>> +
>> +       /* enable 1/8V DDR capable */
>> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>
>
> missing cap for bus width testing CMD19 works on MMP2
Will do the test of CMD19.

> ADMA is fine for SD/eMMC -- SDMA is best for SDIO.  SDHCI_QUIRK_BROKEN_ADMA
> is best passed via pdata->quirks.
>
>
>> +
>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
>> +               /* on-chip device */
>> +               host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>> +               host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>> +       }
>> +
>> +       /* If slot design supports 8 bit data, indicate this to MMC. */
>> +       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>> +
>> +       if (pdata && pdata->quirks)
>> +               host->quirks |= pdata->quirks;
>> +       if (pdata && pdata->host_caps)
>> +               host->mmc->caps |= pdata->host_caps;
>> +       if (pdata && pdata->pm_caps)
>> +               host->mmc->pm_caps |= pdata->pm_caps;
>> +
>> +       pxa->ops->set_clock = mmp2_set_clock;
>
> see comment about using reset callback instead of set_clock.
fine.
>
>> +       pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
>> +               pxa->ops->get_ro = mmp2_get_ro;
>
> need 74 clock code --- I pasted it at the end the code.  The reason the code is
> needed is that SOME eMMC chips require 74 clocks.  The chips you have tested with
> do not need the clocks but JEDEC  Spec JESD84-A441-1 requires the clocks be
> generated.
>
>
>> +
>> +       host->ops = pxa->ops;
>> +
>> +       ret = sdhci_add_host(host);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "failed to add host\n");
>> +               goto out;
>> +       }
>> +
>> +       platform_set_drvdata(pdev, host);
>> +
>> +       return 0;
>> +out:
>> +       if (host) {
>> +               clk_disable(pltfm_host->clk);
>> +               clk_put(pltfm_host->clk);
>> +               sdhci_pltfm_free(pdev);
>> +       }
>> +
>> +       if (pxa)
>> +               kfree(pxa->ops);
>> +       kfree(pxa);
>> +
>> +       return ret;
>> +}
>> +
>> +static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
>> +{
>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>> +       struct sdhci_host *host = platform_get_drvdata(pdev);
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>> +       int dead = 0;
>> +       u32 scratch;
>> +
>> +       if (host) {
>> +               scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>> +               if (scratch == (u32)-1)
>> +                       dead = 1;
>
> not sure that this is the correct test -- will masked interrupts show
> up as set in the INT_STATUS register ?  from the code in sdhci.c and from
> my testing I have never seen -1 set.
>
> The test in sdhci.c is for 0x0 or for 0xffffffff is for a
> shared interrupt line to know if the interrupt is for this controller.
(u32)-1 is same as 0xffffffff
>
>> +
>> +               sdhci_remove_host(host, dead);
>> +
>> +               clk_disable(pxa->clk);
>> +               clk_put(pxa->clk);
>> +               sdhci_pltfm_free(pdev);
>> +
>> +               platform_set_drvdata(pdev, NULL);
>> +       }
>> +
>> +       if (pxa)
>> +               kfree(pxa->ops);
>> +       kfree(pxa);
>> +
>> +       return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
>> +{
>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>> +
>> +       return sdhci_suspend_host(host, state);
>> +}
>> +
>> +static int sdhci_mmp2_resume(struct platform_device *dev)
>> +{
>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>> +
>> +       return sdhci_resume_host(host);
>> +}
>> +#else
>> +#define sdhci_mmp2_suspend     NULL
>> +#define sdhci_mmp2_resume      NULL
>> +#endif
>> +
>> +
>> +static struct platform_driver sdhci_mmp2_driver = {
>> +       .driver         = {
>> +               .name   = "sdhci-mmp2",
>> +               .owner  = THIS_MODULE,
>> +       },
>> +       .probe          = sdhci_mmp2_probe,
>> +       .remove         = __devexit_p(sdhci_mmp2_remove),
>> +#ifdef CONFIG_PM
>> +       .suspend        = sdhci_mmp2_suspend,
>> +       .resume         = sdhci_mmp2_resume,
>> +#endif
>> +};
>> +static int __init sdhci_mmp2_init(void)
>> +{
>> +       return platform_driver_register(&sdhci_mmp2_driver);
>> +}
>> +
>> +static void __exit sdhci_mmp2_exit(void)
>> +{
>> +       platform_driver_unregister(&sdhci_mmp2_driver);
>> +}
>> +
>> +module_init(sdhci_mmp2_init);
>> +module_exit(sdhci_mmp2_exit);
>> +
>> +MODULE_DESCRIPTION("SDHCI driver for mmp2");
>> +MODULE_AUTHOR("Marvell International Ltd.");
>> +MODULE_LICENSE("GPL v2");
>> +
>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>> deleted file mode 100644
>> index 089c9a6..0000000
>> --- a/drivers/mmc/host/sdhci-pxa.c
>> +++ /dev/null
>> @@ -1,303 +0,0 @@
>> -/* 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>
>> - *
>
> Keep the authors.  They all contributed to the code.
fine
>
>> - * 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>> -{
>> -       u16 ctrl_2;
>> -
>> -       /*
>> -        * Set V18_EN -- UHS modes do not work without this.
>> -        * does not change signaling voltage
>> -        */
>> -       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> -
>> -       /* Select Bus Speed Mode for host */
>> -       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> -       switch (uhs) {
>> -       case MMC_TIMING_UHS_SDR12:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>> -               break;
>> -       case MMC_TIMING_UHS_SDR25:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>> -               break;
>> -       case MMC_TIMING_UHS_SDR50:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>> -               break;
>> -       case MMC_TIMING_UHS_SDR104:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>> -               break;
>> -       case MMC_TIMING_UHS_DDR50:
>> -               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>> -               break;
>> -       }
>> -
>> -       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> -       pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
>> -               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>> -
>> -       return 0;
>> -}
>> -
>> -static struct sdhci_ops sdhci_pxa_ops = {
>> -       .set_uhs_signaling = set_uhs_signaling,
>> -       .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");
>> -               return PTR_ERR(host);
>> -       }
>> -
>> -       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
>> -               | SDHCI_QUIRK_32BIT_DMA_ADDR
>> -               | SDHCI_QUIRK_32BIT_DMA_SIZE
>> -               | SDHCI_QUIRK_32BIT_ADMA_SIZE
>> -               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>> -
>> -       if (pdata->quirks)
>> -               host->quirks |= pdata->quirks;
>> -
>> -       /* enable 1/8V DDR capable */
>> -       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>> -
>> -       /* If slot design supports 8 bit data, indicate this to MMC. */
>> -       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>> -               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>> -
>> -       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) {
>> -               clk_put(pxa->clk);
>> -               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, pm_message_t state)
>> -{
>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>> -
>> -       return sdhci_suspend_host(host, state);
>> -}
>> -
>> -static int sdhci_pxa_resume(struct platform_device *dev)
>> -{
>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>> -
>> -       return sdhci_resume_host(host);
>> -}
>> -#else
>> -#define sdhci_pxa_suspend      NULL
>> -#define sdhci_pxa_resume       NULL
>> -#endif
>> -
>> -static struct platform_driver sdhci_pxa_driver = {
>> -       .probe          = sdhci_pxa_probe,
>> -       .remove         = __devexit_p(sdhci_pxa_remove),
>> -       .suspend        = sdhci_pxa_suspend,
>> -       .resume         = sdhci_pxa_resume,
>> -       .driver         = {
>> -               .name   = DRIVER_NAME,
>> -               .owner  = THIS_MODULE,
>> -       },
>> -};
>> -
>> -/*****************************************************************************\
>> - *                                                                           *
>> - * 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
>
>
>
>
> code to send 74 clocks when card is inserted.  Need to callback hook so
> code is called from sdhci.c
>
> from patch submitted to linux-mmc
>
> from: Philip Rakity <prakity@marvell.com>
> Date: February 13, 2011 11:24:18 PM PST
> To: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
> Cc: Mark Brown <markb@marvell.com>
> Subject: [PATCH] sdhci: Add support PXA168, PXA910, and MMP2 controllers
>
>
> +static void generate_initial_74_clocks(struct sdhci_host *host, u8 power_mode)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +       u16 tmp;
> +       int count;
> +
> +       if (pxa->power_mode == MMC_POWER_UP
> +       && power_mode == MMC_POWER_ON) {
> +
> +               pr_debug("%s:%s ENTER: slot->power_mode = %d,"
> +                       "ios->power_mode = %d\n",
> +                       __func__,
> +                       mmc_hostname(host->mmc),
> +                       pxa->power_mode,
> +                       power_mode);
> +
> +               /* set we want notice of when 74 clocks are sent */
> +               tmp = readw(host->ioaddr + SD_CE_ATA_2);
> +               tmp |= SDCE_MISC_INT_EN;
> +               writew(tmp, host->ioaddr + SD_CE_ATA_2);
> +
> +               /* start sending the 74 clocks */
> +               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
> +               tmp |= SDCFG_GEN_PAD_CLK_ON;
> +               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);

Will merge the code together?
How about simplifying the code to
 if (pxa->power_mode == MMC_POWER_UP
      && power_mode == MMC_POWER_ON) {
               /* start sending the 74 clocks */
               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
               tmp |= SDCFG_GEN_PAD_CLK_ON;
               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
       }
       pxa->power_mode = power_mode;
}
Since core.c already consider the time for 74 clocks.
static void mmc_power_up(struct mmc_host *host)
{
~~
         /*
         * This delay must be at least 74 clock sizes, or 1 ms, or the
         * time required to reach a stable voltage.
         */
        mmc_delay(10);
}
The issue of our controller is clock is not start until there is cmd or data.
While 10ms is enough for 74 clock, so just need to start and no need
to wait finish.


> +
> +               /* slowest speed is about 100KHz or 10usec per clock */
> +               udelay(740);
> +               count = 0;
> +#define MAX_WAIT_COUNT 5
> +               while (count++ < MAX_WAIT_COUNT) {
> +                       if ((readw(host->ioaddr + SD_CE_ATA_2)
> +                                       & SDCE_MISC_INT) == 0)
> +                               break;
> +                       udelay(10);
> +               }
> +
> +               if (count == MAX_WAIT_COUNT)
> +                       printk(KERN_WARNING"%s: %s: 74 clock interrupt "
> +                               "not cleared\n",
> +                               __func__, mmc_hostname(host->mmc));
> +               /* clear the interrupt bit if posted */
> +               tmp = readw(host->ioaddr + SD_CE_ATA_2);
> +               tmp |= SDCE_MISC_INT;
> +               writew(tmp, host->ioaddr + SD_CE_ATA_2);
> +       }
> +       pxa->power_mode = power_mode;
> +}
> +
>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philip Rakity May 30, 2011, 8:47 p.m. UTC | #10
On May 30, 2011, at 6:15 AM, zhangfei gao wrote:

> On Sat, May 28, 2011 at 1:00 AM, Philip Rakity <prakity@marvell.com> wrote:
>>
>> Hi Zhangfei,
>>
>>
>> comments below. (based on V1 patch but change to V2 affect (3<<1) typo.
>>
>> Philip and Mark.
>
> Thanks for review.
>>
>>
>> On May 23, 2011, at 6:21 AM, Zhangfei Gao wrote:
>>
>>>       Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>>>
>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>>> ---
>>> arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
>>> drivers/mmc/host/Kconfig               |    9 +-
>>> drivers/mmc/host/Makefile              |    2 +-
>>> drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
>>> drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
>>> 5 files changed, 349 insertions(+), 312 deletions(-)
>>> create mode 100644 drivers/mmc/host/sdhci-mmp2.c
>>> delete 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
>>> index 1ab332e..92becc0 100644
>>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>>> @@ -13,23 +13,58 @@
>>> #ifndef __PLAT_PXA_SDHCI_H
>>> #define __PLAT_PXA_SDHCI_H
>>>
>>> +#include <linux/mmc/sdhci.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> /* pxa specific flag */
>>> /* Require clock free running */
>>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>>> -
>>> +/* card alwayes wired to host, like on-chip emmc */
>>> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
>>> /* Board design supports 8-bit data on SD/SDIO BUS */
>>> #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
>>> +/* card not use wp, such as micro sd card */
>>> +#define PXA_FLAG_CARD_NO_WP    (3<<1)
>>
>> do you mean (1<<3) ?
> already update in v2.
>>>
>>> +struct sdhci_pxa;
>>> /*
>>> * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>>> - * @max_speed: the maximum speed supported
>>> - * @quirks: quirks of specific device
>>> * @flags: flags for platform requirement
>>> + * @clk_delay_cycles:
>>> + *     mmp2: each step is roughly 100ps, 5bits width
>>> + *     pxa910: each step is 1ns, 4bits width
>>> + * @clk_delay_sel: select clk_delay, used on pxa910
>>> + *     0: choose feedback clk
>>> + *     1: choose feedback clk + delay value
>>> + *     2: choose internal clk
>>> + * @ext_cd_gpio: gpio pin used for external CD line
>>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910
>>> + * @max_speed: the maximum speed supported
>>> + * @host_caps: Standard MMC host capabilities bit field.
>>> + * @quirks: quirks of platfrom
>>> + * @pm_caps: pm_caps of platfrom
>>> */
>>> struct sdhci_pxa_platdata {
>>> +       unsigned int    flags;
>>> +       unsigned int    clk_delay_cycles;
>>> +       unsigned int    clk_delay_sel;
>>> +       unsigned int    ext_cd_gpio;
>>> +       bool            ext_cd_gpio_invert;
>>> +       bool            clk_delay_enable;
>>
>> move up 2 lines to be next to other clk_ values.
> fine.
>>
>>>       unsigned int    max_speed;
>>> +       unsigned int    host_caps;
>>>       unsigned int    quirks;
>>> -       unsigned int    flags;
>>> +       unsigned int    pm_caps;
>>> +};
>>> +
>>> +struct sdhci_pxa {
>>> +       struct sdhci_pxa_platdata       *pdata;
>>> +       struct clk                      *clk;
>>> +       struct sdhci_ops                *ops;
>>> +
>>> +       u8      clk_enable;
>>> +       u8      power_mode;
>>> };
>>>
>>> #endif /* __PLAT_PXA_SDHCI_H */
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 862235e..1e520e3 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -181,14 +181,15 @@ config MMC_SDHCI_S3C
>>>
>>>         If unsure, say N.
>>>
>>> -config MMC_SDHCI_PXA
>>> -       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
>>> +config MMC_SDHCI_MMP2
>>> +       tristate "Marvell MMP2 SD Host Controller support"
>>>       depends on ARCH_PXA || ARCH_MMP
>>>       select MMC_SDHCI
>>> +       select MMC_SDHCI_PLTFM
>>>       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
>>> +         This selects the Marvell(R) MMP2 SD Host Controller.
>>> +         If you have a MMP2 platform with SD Host Controller
>>>         and a card slot, say Y or M here.
>>>
>>
>> This needs to be specific to MMP2  -- depends on CPU_MMP2 ?
>> code can be selected for pxa9xx and pxa168 where this code cannot work.
>>
>> deleting MMC_SDHCI_PXA breaks existing configs since it is renamed.
>> would it better to leave this and mark it as obsolete but let it still compile
>> mmp2 sd code?
>>
>> In some sense all of this is backwards.  Once the SoC is known (pxa910 or mmp2),
>> it knows that SDHCI controller it supports.  It should set a config option
>> like USES_MRVL_V3_SD_CONTROLLER.  The Kconfig entry in mmc/host should
>> depend on THAT selection to allow the user to enable the SD option.
>>
>> MMC_SDHCI_IO_ACCESSORS not needed
>>
> Will take Arnd advice, remove dependency and only select MMC_SDHCI and
> MMC_SDHCI_PLTFM
>>
>>>
>>
>>
>>>         If unsure, say N.
>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>> index 4933004..f8650e0 100644
>>> --- a/drivers/mmc/host/Makefile
>>> +++ b/drivers/mmc/host/Makefile
>>> @@ -9,7 +9,7 @@ obj-$(CONFIG_MMC_MXC)           += mxcmmc.o
>>> obj-$(CONFIG_MMC_MXS)          += mxs-mmc.o
>>> obj-$(CONFIG_MMC_SDHCI)                += sdhci.o
>>> obj-$(CONFIG_MMC_SDHCI_PCI)    += sdhci-pci.o
>>> -obj-$(CONFIG_MMC_SDHCI_PXA)    += sdhci-pxa.o
>>> +obj-$(CONFIG_MMC_SDHCI_MMP2)   += sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
>>> new file mode 100644
>>> index 0000000..566d525
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/sdhci-mmp2.c
>>
>> It is probably better to NOT name the file sdhci-mmp2.c but
>> something like sdchi-pxaV3.c since other SoC's may use this controller.
> fine

The same renaming change is needed for the pxa9xx patch.

>>
>>> @@ -0,0 +1,304 @@
>>> +/*
>>> + * Copyright (C) 2010 Marvell International Ltd.
>>> + *
>>> + * This software is licensed under the terms of the GNU General Public
>>> + * License version 2, as published by the Free Software Foundation, and
>>> + * may be copied, distributed, and modified under those terms.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + */
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/init.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/io.h>
>>> +#include <linux/gpio.h>
>>> +#include <linux/mmc/card.h>
>>> +#include <linux/mmc/host.h>
>>> +#include <plat/sdhci.h>
>>> +#include <linux/slab.h>
>>> +#include "sdhci.h"
>>> +#include "sdhci-pltfm.h"
>>> +
>>> +#define MMP2_SD_FIFO_PARAM             0x104
>>> +#define MMP2_DIS_PAD_SD_CLK_GATE       0x400
>>> +
>>> +#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP             0x10A
>>> +#define MMP2_SDCLK_SEL 0x100
>>> +#define MMP2_SDCLK_DELAY_SHIFT 9
>>> +#define MMP2_SDCLK_DELAY_MASK  0x1f
>>> +
>>> +#define SD_CFG_FIFO_PARAM       0x100
>>> +#define SDCFG_GEN_PAD_CLK_ON   (1<<6)
>>> +#define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
>>> +#define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
>>> +
>>> +#define SD_SPI_MODE          0x108
>>> +#define SD_CE_ATA_1          0x10C
>>> +
>>> +#define SD_CE_ATA_2          0x10E
>>> +#define SDCE_MISC_INT          (1<<2)
>>> +#define SDCE_MISC_INT_EN       (1<<1)
>>> +
>>> +static void mmp2_soc_set_timing(struct sdhci_host *host,
>>> +                               struct sdhci_pxa_platdata *pdata)
>>> +{
>>> +       /*
>>> +        * tune timing of read data/command when crc error happen
>>> +        * no performance impact
>>> +        */
>>> +       if (pdata && 0 != pdata->clk_delay_cycles) {
>>> +               u16 tmp;
>>> +
>>> +               tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>>> +               tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
>>> +                                       << MMP2_SDCLK_DELAY_SHIFT;
>>> +               tmp |= MMP2_SDCLK_SEL;
>>> +               writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>>> +       }
>>> +
>>> +       /*
>>> +        * disable clock gating per some cards requirement
>>> +        */
>>> +
>>> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
>>> +               u32 tmp = 0;
>>> +
>>> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
>>> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
>>> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
>>> +       }
>>> +}
>>
>> One cannot know what card if being inserted or removed by a user
>> so the safest choice is to leave clock gating OFF by default.
>>
>> PXA_FLAG_DISABLE_CLOCK_GATING should be renamed
>> to
>> PXA_FLAG_ENABLE_CLOCK_GATING.
>>
>> If you have a card which is permanent then the flags should be used
>> to ENABLE clock gating.  Otherwise clock gating should be left OFF by default.
> Still in check.
>
>>
>> The above code should be called via the call back added to sdhci.c for
>> reset processing at the end of a reset and tested against a RESET_ALL.
>> The RESET_ALL will reset the private register space.
> OK, it also workable to put in platform_reset_exit.
>>
>>
>>> +
>>> +static unsigned int mmp2_get_ro(struct sdhci_host *host)
>>> +{
>>> +       /* Micro SD does not support write-protect feature */
>>> +       return 0;
>>> +}
>>> +
>>> +static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +{
>>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>>> +
>>> +       if (clock)
>>> +               mmp2_soc_set_timing(host, pxa->pdata);
>>> +}
>>> +
>>
>>
>> This code should be called via reset callback and mmp2_soc_set_timing added here
>> and prototype changed to
>> +static void mmp2_set_private_registers(struct sdhci_host *host, unsigned int reset_flag)
>>
>> and used as
>>
>>       if (reset_flag == RESET_ALL)
>>               mmp2_soc_set_timing(host, pxa->pdata);
>>       }
>>
>>
>>> +static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>>> +{
>>> +       u16 ctrl_2;
>>> +
>>> +       /*
>>> +        * Set V18_EN -- UHS modes do not work without this.
>>> +        * does not change signaling voltage
>>> +        */
>>> +       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> +
>>> +       /* Select Bus Speed Mode for host */
>>> +       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>>> +       switch (uhs) {
>>> +       case MMC_TIMING_UHS_SDR12:
>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>>> +               break;
>>> +       case MMC_TIMING_UHS_SDR25:
>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>>> +               break;
>>> +       case MMC_TIMING_UHS_SDR50:
>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>>> +               break;
>>> +       case MMC_TIMING_UHS_SDR104:
>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>>> +               break;
>>> +       case MMC_TIMING_UHS_DDR50:
>>> +               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>>> +               break;
>>> +       }
>>> +
>>> +       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>> +       dev_dbg(mmc_dev(host->mmc),
>>> +               "%s:%s uhs = %d, ctrl_2 = %04X\n",
>>> +               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
>>> +{
>>> +       struct sdhci_pltfm_host *pltfm_host;
>>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>> +       struct device *dev = &pdev->dev;
>>> +       struct sdhci_host *host = NULL;
>>> +       struct sdhci_pxa *pxa = NULL;
>>> +       int ret;
>>> +       struct clk *clk;
>>> +
>>> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
>>> +       if (!pxa) {
>>> +               ret = -ENOMEM;
>>> +               goto out;
>>> +       }
>>> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>> why is  pxa->ops needed ?
>>
>>> +       if (!pxa->ops) {
>>> +               ret = -ENOMEM;
>>> +               goto out;
>>> +       }
>>> +       pxa->pdata = pdata;
>>> +
>>> +       clk = clk_get(dev, "PXA-SDHCLK");
>>> +       if (IS_ERR(clk)) {
>>> +               dev_err(dev, "failed to get io clock\n");
>>> +               ret = PTR_ERR(clk);
>>> +               goto out;
>>> +       }
>>> +       pxa->clk = clk;
>>> +       clk_enable(clk);
>>> +
>>> +       host = sdhci_pltfm_init(pdev, NULL);
>>> +       if (IS_ERR(host))
>>> +               return PTR_ERR(host);
>>> +
>>> +       pltfm_host = sdhci_priv(host);
>>> +       pltfm_host->priv = pxa;
>>> +
>>> +       host->quirks = SDHCI_QUIRK_BROKEN_ADMA
>>> +               | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
>
>>> +               | SDHCI_QUIRK_32BIT_DMA_ADDR
>>> +               | SDHCI_QUIRK_32BIT_DMA_SIZE
>>> +               | SDHCI_QUIRK_32BIT_ADMA_SIZE
> my understand is mmp2 do not have such limitation?

The silicon designer indicated we should set 32bit access.  64 bit is not supported.

>
>>> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
> This quirk also not must?

It is NOT a must but it saves a needless memory access per ADMA transfer to fetch a
descriptor that says ADMA is done.  Since you are concerned about  the time 74 clocks
takes this quirk makes everything faster.

>>> +
>>> +       /* enable 1/8V DDR capable */
>>> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>>
>>
>> missing cap for bus width testing CMD19 works on MMP2
> Will do the test of CMD19.
>
>> ADMA is fine for SD/eMMC -- SDMA is best for SDIO.  SDHCI_QUIRK_BROKEN_ADMA
>> is best passed via pdata->quirks.
>>
>>
>>> +
>>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
>>> +               /* on-chip device */
>>> +               host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>>> +               host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>>> +       }
>>> +
>>> +       /* If slot design supports 8 bit data, indicate this to MMC. */
>>> +       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>> +
>>> +       if (pdata && pdata->quirks)
>>> +               host->quirks |= pdata->quirks;
>>> +       if (pdata && pdata->host_caps)
>>> +               host->mmc->caps |= pdata->host_caps;
>>> +       if (pdata && pdata->pm_caps)
>>> +               host->mmc->pm_caps |= pdata->pm_caps;
>>> +
>>> +       pxa->ops->set_clock = mmp2_set_clock;
>>
>> see comment about using reset callback instead of set_clock.
> fine.
>>
>>> +       pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
>>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
>>> +               pxa->ops->get_ro = mmp2_get_ro;
>>
>> need 74 clock code --- I pasted it at the end the code.  The reason the code is
>> needed is that SOME eMMC chips require 74 clocks.  The chips you have tested with
>> do not need the clocks but JEDEC  Spec JESD84-A441-1 requires the clocks be
>> generated.
>>
>>
>>> +
>>> +       host->ops = pxa->ops;
>>> +
>>> +       ret = sdhci_add_host(host);
>>> +       if (ret) {
>>> +               dev_err(&pdev->dev, "failed to add host\n");
>>> +               goto out;
>>> +       }
>>> +
>>> +       platform_set_drvdata(pdev, host);
>>> +
>>> +       return 0;
>>> +out:
>>> +       if (host) {
>>> +               clk_disable(pltfm_host->clk);
>>> +               clk_put(pltfm_host->clk);
>>> +               sdhci_pltfm_free(pdev);
>>> +       }
>>> +
>>> +       if (pxa)
>>> +               kfree(pxa->ops);
>>> +       kfree(pxa);
>>> +
>>> +       return ret;
>>> +}
>>> +
>>> +static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
>>> +{
>>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>> +       struct sdhci_host *host = platform_get_drvdata(pdev);
>>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>>> +       int dead = 0;
>>> +       u32 scratch;
>>> +
>>> +       if (host) {
>>> +               scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>>> +               if (scratch == (u32)-1)
>>> +                       dead = 1;
>>
>> not sure that this is the correct test -- will masked interrupts show
>> up as set in the INT_STATUS register ?  from the code in sdhci.c and from
>> my testing I have never seen -1 set.
>>
>> The test in sdhci.c is for 0x0 or for 0xffffffff is for a
>> shared interrupt line to know if the interrupt is for this controller.
> (u32)-1 is same as 0xffffffff

not clear in my question.  Have you confirmed that when the controller is DEAD that
-1 is read from the INT STATUS register ?  I have never seen this.
Maybe there is a private register that can help.

>>
>>> +
>>> +               sdhci_remove_host(host, dead);
>>> +
>>> +               clk_disable(pxa->clk);
>>> +               clk_put(pxa->clk);
>>> +               sdhci_pltfm_free(pdev);
>>> +
>>> +               platform_set_drvdata(pdev, NULL);
>>> +       }
>>> +
>>> +       if (pxa)
>>> +               kfree(pxa->ops);
>>> +       kfree(pxa);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM
>>> +static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
>>> +{
>>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>>> +
>>> +       return sdhci_suspend_host(host, state);
>>> +}
>>> +
>>> +static int sdhci_mmp2_resume(struct platform_device *dev)
>>> +{
>>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>>> +
>>> +       return sdhci_resume_host(host);
>>> +}
>>> +#else
>>> +#define sdhci_mmp2_suspend     NULL
>>> +#define sdhci_mmp2_resume      NULL
>>> +#endif
>>> +
>>> +
>>> +static struct platform_driver sdhci_mmp2_driver = {
>>> +       .driver         = {
>>> +               .name   = "sdhci-mmp2",
>>> +               .owner  = THIS_MODULE,
>>> +       },
>>> +       .probe          = sdhci_mmp2_probe,
>>> +       .remove         = __devexit_p(sdhci_mmp2_remove),
>>> +#ifdef CONFIG_PM
>>> +       .suspend        = sdhci_mmp2_suspend,
>>> +       .resume         = sdhci_mmp2_resume,
>>> +#endif
>>> +};
>>> +static int __init sdhci_mmp2_init(void)
>>> +{
>>> +       return platform_driver_register(&sdhci_mmp2_driver);
>>> +}
>>> +
>>> +static void __exit sdhci_mmp2_exit(void)
>>> +{
>>> +       platform_driver_unregister(&sdhci_mmp2_driver);
>>> +}
>>> +
>>> +module_init(sdhci_mmp2_init);
>>> +module_exit(sdhci_mmp2_exit);
>>> +
>>> +MODULE_DESCRIPTION("SDHCI driver for mmp2");
>>> +MODULE_AUTHOR("Marvell International Ltd.");
>>> +MODULE_LICENSE("GPL v2");
>>> +
>>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>>> deleted file mode 100644
>>> index 089c9a6..0000000
>>> --- a/drivers/mmc/host/sdhci-pxa.c
>>> +++ /dev/null
>>> @@ -1,303 +0,0 @@
>>> -/* 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>
>>> - *
>>
>> Keep the authors.  They all contributed to the code.
> fine
>>
>>> - * 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>>> -{
>>> -       u16 ctrl_2;
>>> -
>>> -       /*
>>> -        * Set V18_EN -- UHS modes do not work without this.
>>> -        * does not change signaling voltage
>>> -        */
>>> -       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> -
>>> -       /* Select Bus Speed Mode for host */
>>> -       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>>> -       switch (uhs) {
>>> -       case MMC_TIMING_UHS_SDR12:
>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>>> -               break;
>>> -       case MMC_TIMING_UHS_SDR25:
>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>>> -               break;
>>> -       case MMC_TIMING_UHS_SDR50:
>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>>> -               break;
>>> -       case MMC_TIMING_UHS_SDR104:
>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>>> -               break;
>>> -       case MMC_TIMING_UHS_DDR50:
>>> -               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>>> -               break;
>>> -       }
>>> -
>>> -       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>> -       pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
>>> -               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>>> -
>>> -       return 0;
>>> -}
>>> -
>>> -static struct sdhci_ops sdhci_pxa_ops = {
>>> -       .set_uhs_signaling = set_uhs_signaling,
>>> -       .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");
>>> -               return PTR_ERR(host);
>>> -       }
>>> -
>>> -       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
>>> -               | SDHCI_QUIRK_32BIT_DMA_ADDR
>>> -               | SDHCI_QUIRK_32BIT_DMA_SIZE
>>> -               | SDHCI_QUIRK_32BIT_ADMA_SIZE
>>> -               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>>> -
>>> -       if (pdata->quirks)
>>> -               host->quirks |= pdata->quirks;
>>> -
>>> -       /* enable 1/8V DDR capable */
>>> -       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>>> -
>>> -       /* If slot design supports 8 bit data, indicate this to MMC. */
>>> -       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>> -               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>> -
>>> -       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) {
>>> -               clk_put(pxa->clk);
>>> -               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, pm_message_t state)
>>> -{
>>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>>> -
>>> -       return sdhci_suspend_host(host, state);
>>> -}
>>> -
>>> -static int sdhci_pxa_resume(struct platform_device *dev)
>>> -{
>>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>>> -
>>> -       return sdhci_resume_host(host);
>>> -}
>>> -#else
>>> -#define sdhci_pxa_suspend      NULL
>>> -#define sdhci_pxa_resume       NULL
>>> -#endif
>>> -
>>> -static struct platform_driver sdhci_pxa_driver = {
>>> -       .probe          = sdhci_pxa_probe,
>>> -       .remove         = __devexit_p(sdhci_pxa_remove),
>>> -       .suspend        = sdhci_pxa_suspend,
>>> -       .resume         = sdhci_pxa_resume,
>>> -       .driver         = {
>>> -               .name   = DRIVER_NAME,
>>> -               .owner  = THIS_MODULE,
>>> -       },
>>> -};
>>> -
>>> -/*****************************************************************************\
>>> - *                                                                           *
>>> - * 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
>>
>>
>>
>>
>> code to send 74 clocks when card is inserted.  Need to callback hook so
>> code is called from sdhci.c
>>
>> from patch submitted to linux-mmc
>>
>> from: Philip Rakity <prakity@marvell.com>
>> Date: February 13, 2011 11:24:18 PM PST
>> To: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
>> Cc: Mark Brown <markb@marvell.com>
>> Subject: [PATCH] sdhci: Add support PXA168, PXA910, and MMP2 controllers
>>
>>
>> +static void generate_initial_74_clocks(struct sdhci_host *host, u8 power_mode)
>> +{
>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>> +       u16 tmp;
>> +       int count;
>> +
>> +       if (pxa->power_mode == MMC_POWER_UP
>> +       && power_mode == MMC_POWER_ON) {
>> +
>> +               pr_debug("%s:%s ENTER: slot->power_mode = %d,"
>> +                       "ios->power_mode = %d\n",
>> +                       __func__,
>> +                       mmc_hostname(host->mmc),
>> +                       pxa->power_mode,
>> +                       power_mode);
>> +
>> +               /* set we want notice of when 74 clocks are sent */
>> +               tmp = readw(host->ioaddr + SD_CE_ATA_2);
>> +               tmp |= SDCE_MISC_INT_EN;
>> +               writew(tmp, host->ioaddr + SD_CE_ATA_2);
>> +
>> +               /* start sending the 74 clocks */
>> +               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
>> +               tmp |= SDCFG_GEN_PAD_CLK_ON;
>> +               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
>
> Will merge the code together?
> How about simplifying the code to
> if (pxa->power_mode == MMC_POWER_UP
>      && power_mode == MMC_POWER_ON) {
>               /* start sending the 74 clocks */
>               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
>               tmp |= SDCFG_GEN_PAD_CLK_ON;
>               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
>       }
>       pxa->power_mode = power_mode;
> }

see comment below.

> Since core.c already consider the time for 74 clocks.
> static void mmc_power_up(struct mmc_host *host)
> {
> ~~
>         /*
>         * This delay must be at least 74 clock sizes, or 1 ms, or the
>         * time required to reach a stable voltage.
>         */
>        mmc_delay(10);
> }
> The issue of our controller is clock is not start until there is cmd or data.
> While 10ms is enough for 74 clock, so just need to start and no need
> to wait finish.
>
>
>> +
>> +               /* slowest speed is about 100KHz or 10usec per clock */
>> +               udelay(740);
>> +               count = 0;
>> +#define MAX_WAIT_COUNT 5
>> +               while (count++ < MAX_WAIT_COUNT) {
>> +                       if ((readw(host->ioaddr + SD_CE_ATA_2)
>> +                                       & SDCE_MISC_INT) == 0)
>> +                               break;
>> +                       udelay(10);
>> +               }
>> +
>> +               if (count == MAX_WAIT_COUNT)
>> +                       printk(KERN_WARNING"%s: %s: 74 clock interrupt "
>> +                               "not cleared\n",
>> +                               __func__, mmc_hostname(host->mmc));
>> +               /* clear the interrupt bit if posted */
>> +               tmp = readw(host->ioaddr + SD_CE_ATA_2);
>> +               tmp |= SDCE_MISC_INT;
>> +               writew(tmp, host->ioaddr + SD_CE_ATA_2);
>> +       }
>> +       pxa->power_mode = power_mode;
>> +}
>> +
>>

is the 740 us delay important  once per card insert ?  The code works as submitted and was tested
with the silicon design team.  The other option would be to enable the interrupt rather then wait but
in that case you will need to plumb the interrupt in.  Did not want to touch base code and
was not concerned about 740 us.

The mmc_delay value is WAY too high and should be lowered.

The 74 clock code is needed for any slot where a card can be plugged in.   You cannot assume it is
SD.  It could be mmc (or sdio) or combo.

>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao May 31, 2011, 12:07 p.m. UTC | #11
On Tue, May 31, 2011 at 4:47 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> On May 30, 2011, at 6:15 AM, zhangfei gao wrote:
>
>> On Sat, May 28, 2011 at 1:00 AM, Philip Rakity <prakity@marvell.com> wrote:
>>>
>>> Hi Zhangfei,
>>>
>>>
>>> comments below. (based on V1 patch but change to V2 affect (3<<1) typo.
>>>
>>> Philip and Mark.
>>
>> Thanks for review.
>>>
>>>
>>> On May 23, 2011, at 6:21 AM, Zhangfei Gao wrote:
>>>
>>>>       Delete sdhci-pxa.c and replace with sdhci-mmp2.c specificlly for mmp2
>>>>
>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>>>> ---
>>>> arch/arm/plat-pxa/include/plat/sdhci.h |   43 ++++-
>>>> drivers/mmc/host/Kconfig               |    9 +-
>>>> drivers/mmc/host/Makefile              |    2 +-
>>>> drivers/mmc/host/sdhci-mmp2.c          |  304 ++++++++++++++++++++++++++++++++
>>>> drivers/mmc/host/sdhci-pxa.c           |  303 -------------------------------
>>>> 5 files changed, 349 insertions(+), 312 deletions(-)
>>>> create mode 100644 drivers/mmc/host/sdhci-mmp2.c
>>>> delete 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
>>>> index 1ab332e..92becc0 100644
>>>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>>>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>>>> @@ -13,23 +13,58 @@
>>>> #ifndef __PLAT_PXA_SDHCI_H
>>>> #define __PLAT_PXA_SDHCI_H
>>>>
>>>> +#include <linux/mmc/sdhci.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> /* pxa specific flag */
>>>> /* Require clock free running */
>>>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>>>> -
>>>> +/* card alwayes wired to host, like on-chip emmc */
>>>> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
>>>> /* Board design supports 8-bit data on SD/SDIO BUS */
>>>> #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
>>>> +/* card not use wp, such as micro sd card */
>>>> +#define PXA_FLAG_CARD_NO_WP    (3<<1)
>>>
>>> do you mean (1<<3) ?
>> already update in v2.
>>>>
>>>> +struct sdhci_pxa;
>>>> /*
>>>> * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>>>> - * @max_speed: the maximum speed supported
>>>> - * @quirks: quirks of specific device
>>>> * @flags: flags for platform requirement
>>>> + * @clk_delay_cycles:
>>>> + *     mmp2: each step is roughly 100ps, 5bits width
>>>> + *     pxa910: each step is 1ns, 4bits width
>>>> + * @clk_delay_sel: select clk_delay, used on pxa910
>>>> + *     0: choose feedback clk
>>>> + *     1: choose feedback clk + delay value
>>>> + *     2: choose internal clk
>>>> + * @ext_cd_gpio: gpio pin used for external CD line
>>>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>>>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910
>>>> + * @max_speed: the maximum speed supported
>>>> + * @host_caps: Standard MMC host capabilities bit field.
>>>> + * @quirks: quirks of platfrom
>>>> + * @pm_caps: pm_caps of platfrom
>>>> */
>>>> struct sdhci_pxa_platdata {
>>>> +       unsigned int    flags;
>>>> +       unsigned int    clk_delay_cycles;
>>>> +       unsigned int    clk_delay_sel;
>>>> +       unsigned int    ext_cd_gpio;
>>>> +       bool            ext_cd_gpio_invert;
>>>> +       bool            clk_delay_enable;
>>>
>>> move up 2 lines to be next to other clk_ values.
>> fine.
>>>
>>>>       unsigned int    max_speed;
>>>> +       unsigned int    host_caps;
>>>>       unsigned int    quirks;
>>>> -       unsigned int    flags;
>>>> +       unsigned int    pm_caps;
>>>> +};
>>>> +
>>>> +struct sdhci_pxa {
>>>> +       struct sdhci_pxa_platdata       *pdata;
>>>> +       struct clk                      *clk;
>>>> +       struct sdhci_ops                *ops;
>>>> +
>>>> +       u8      clk_enable;
>>>> +       u8      power_mode;
>>>> };
>>>>
>>>> #endif /* __PLAT_PXA_SDHCI_H */
>>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>>> index 862235e..1e520e3 100644
>>>> --- a/drivers/mmc/host/Kconfig
>>>> +++ b/drivers/mmc/host/Kconfig
>>>> @@ -181,14 +181,15 @@ config MMC_SDHCI_S3C
>>>>
>>>>         If unsure, say N.
>>>>
>>>> -config MMC_SDHCI_PXA
>>>> -       tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
>>>> +config MMC_SDHCI_MMP2
>>>> +       tristate "Marvell MMP2 SD Host Controller support"
>>>>       depends on ARCH_PXA || ARCH_MMP
>>>>       select MMC_SDHCI
>>>> +       select MMC_SDHCI_PLTFM
>>>>       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
>>>> +         This selects the Marvell(R) MMP2 SD Host Controller.
>>>> +         If you have a MMP2 platform with SD Host Controller
>>>>         and a card slot, say Y or M here.
>>>>
>>>
>>> This needs to be specific to MMP2  -- depends on CPU_MMP2 ?
>>> code can be selected for pxa9xx and pxa168 where this code cannot work.
>>>
>>> deleting MMC_SDHCI_PXA breaks existing configs since it is renamed.
>>> would it better to leave this and mark it as obsolete but let it still compile
>>> mmp2 sd code?
>>>
>>> In some sense all of this is backwards.  Once the SoC is known (pxa910 or mmp2),
>>> it knows that SDHCI controller it supports.  It should set a config option
>>> like USES_MRVL_V3_SD_CONTROLLER.  The Kconfig entry in mmc/host should
>>> depend on THAT selection to allow the user to enable the SD option.
>>>
>>> MMC_SDHCI_IO_ACCESSORS not needed
>>>
>> Will take Arnd advice, remove dependency and only select MMC_SDHCI and
>> MMC_SDHCI_PLTFM
>>>
>>>>
>>>
>>>
>>>>         If unsure, say N.
>>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>>> index 4933004..f8650e0 100644
>>>> --- a/drivers/mmc/host/Makefile
>>>> +++ b/drivers/mmc/host/Makefile
>>>> @@ -9,7 +9,7 @@ obj-$(CONFIG_MMC_MXC)           += mxcmmc.o
>>>> obj-$(CONFIG_MMC_MXS)          += mxs-mmc.o
>>>> obj-$(CONFIG_MMC_SDHCI)                += sdhci.o
>>>> obj-$(CONFIG_MMC_SDHCI_PCI)    += sdhci-pci.o
>>>> -obj-$(CONFIG_MMC_SDHCI_PXA)    += sdhci-pxa.o
>>>> +obj-$(CONFIG_MMC_SDHCI_MMP2)   += sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
>>>> new file mode 100644
>>>> index 0000000..566d525
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/host/sdhci-mmp2.c
>>>
>>> It is probably better to NOT name the file sdhci-mmp2.c but
>>> something like sdchi-pxaV3.c since other SoC's may use this controller.
>> fine
>
> The same renaming change is needed for the pxa9xx patch.
Do you mean use sdhci-pxa9xx.c to replace sdhci-pxa910.c, pxa955 do
not use the same driver, but it does not matter.

>
>>>
>>>> @@ -0,0 +1,304 @@
>>>> +/*
>>>> + * Copyright (C) 2010 Marvell International Ltd.
>>>> + *
>>>> + * This software is licensed under the terms of the GNU General Public
>>>> + * License version 2, as published by the Free Software Foundation, and
>>>> + * may be copied, distributed, and modified under those terms.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/err.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/clk.h>
>>>> +#include <linux/io.h>
>>>> +#include <linux/gpio.h>
>>>> +#include <linux/mmc/card.h>
>>>> +#include <linux/mmc/host.h>
>>>> +#include <plat/sdhci.h>
>>>> +#include <linux/slab.h>
>>>> +#include "sdhci.h"
>>>> +#include "sdhci-pltfm.h"
>>>> +
>>>> +#define MMP2_SD_FIFO_PARAM             0x104
>>>> +#define MMP2_DIS_PAD_SD_CLK_GATE       0x400
>>>> +
>>>> +#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP             0x10A
>>>> +#define MMP2_SDCLK_SEL 0x100
>>>> +#define MMP2_SDCLK_DELAY_SHIFT 9
>>>> +#define MMP2_SDCLK_DELAY_MASK  0x1f
>>>> +
>>>> +#define SD_CFG_FIFO_PARAM       0x100
>>>> +#define SDCFG_GEN_PAD_CLK_ON   (1<<6)
>>>> +#define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
>>>> +#define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
>>>> +
>>>> +#define SD_SPI_MODE          0x108
>>>> +#define SD_CE_ATA_1          0x10C
>>>> +
>>>> +#define SD_CE_ATA_2          0x10E
>>>> +#define SDCE_MISC_INT          (1<<2)
>>>> +#define SDCE_MISC_INT_EN       (1<<1)
>>>> +
>>>> +static void mmp2_soc_set_timing(struct sdhci_host *host,
>>>> +                               struct sdhci_pxa_platdata *pdata)
>>>> +{
>>>> +       /*
>>>> +        * tune timing of read data/command when crc error happen
>>>> +        * no performance impact
>>>> +        */
>>>> +       if (pdata && 0 != pdata->clk_delay_cycles) {
>>>> +               u16 tmp;
>>>> +
>>>> +               tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>>>> +               tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
>>>> +                                       << MMP2_SDCLK_DELAY_SHIFT;
>>>> +               tmp |= MMP2_SDCLK_SEL;
>>>> +               writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
>>>> +       }
>>>> +
>>>> +       /*
>>>> +        * disable clock gating per some cards requirement
>>>> +        */
>>>> +
>>>> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
>>>> +               u32 tmp = 0;
>>>> +
>>>> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
>>>> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
>>>> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
>>>> +       }
>>>> +}
>>>
>>> One cannot know what card if being inserted or removed by a user
>>> so the safest choice is to leave clock gating OFF by default.
>>>
>>> PXA_FLAG_DISABLE_CLOCK_GATING should be renamed
>>> to
>>> PXA_FLAG_ENABLE_CLOCK_GATING.
>>>
>>> If you have a card which is permanent then the flags should be used
>>> to ENABLE clock gating.  Otherwise clock gating should be left OFF by default.
Fine,
My understanding is it is not a question, user have to know which slot
should open/close clock gating, and set flag accordingly.
Currently only 8787 require clock free running.

>> Still in check.
>>
>>>
>>> The above code should be called via the call back added to sdhci.c for
>>> reset processing at the end of a reset and tested against a RESET_ALL.
>>> The RESET_ALL will reset the private register space.
>> OK, it also workable to put in platform_reset_exit.
>>>
>>>
>>>> +
>>>> +static unsigned int mmp2_get_ro(struct sdhci_host *host)
>>>> +{
>>>> +       /* Micro SD does not support write-protect feature */
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
>>>> +{
>>>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>>>> +
>>>> +       if (clock)
>>>> +               mmp2_soc_set_timing(host, pxa->pdata);
>>>> +}
>>>> +
>>>
>>>
>>> This code should be called via reset callback and mmp2_soc_set_timing added here
>>> and prototype changed to
>>> +static void mmp2_set_private_registers(struct sdhci_host *host, unsigned int reset_flag)
>>>
>>> and used as
>>>
>>>       if (reset_flag == RESET_ALL)
>>>               mmp2_soc_set_timing(host, pxa->pdata);
>>>       }
>>>
>>>
>>>> +static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>>>> +{
>>>> +       u16 ctrl_2;
>>>> +
>>>> +       /*
>>>> +        * Set V18_EN -- UHS modes do not work without this.
>>>> +        * does not change signaling voltage
>>>> +        */
>>>> +       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>> +
>>>> +       /* Select Bus Speed Mode for host */
>>>> +       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>>>> +       switch (uhs) {
>>>> +       case MMC_TIMING_UHS_SDR12:
>>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>>>> +               break;
>>>> +       case MMC_TIMING_UHS_SDR25:
>>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>>>> +               break;
>>>> +       case MMC_TIMING_UHS_SDR50:
>>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>>>> +               break;
>>>> +       case MMC_TIMING_UHS_SDR104:
>>>> +               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>>>> +               break;
>>>> +       case MMC_TIMING_UHS_DDR50:
>>>> +               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>>>> +               break;
>>>> +       }
>>>> +
>>>> +       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>> +       dev_dbg(mmc_dev(host->mmc),
>>>> +               "%s:%s uhs = %d, ctrl_2 = %04X\n",
>>>> +               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
>>>> +{
>>>> +       struct sdhci_pltfm_host *pltfm_host;
>>>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>>> +       struct device *dev = &pdev->dev;
>>>> +       struct sdhci_host *host = NULL;
>>>> +       struct sdhci_pxa *pxa = NULL;
>>>> +       int ret;
>>>> +       struct clk *clk;
>>>> +
>>>> +       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
>>>> +       if (!pxa) {
>>>> +               ret = -ENOMEM;
>>>> +               goto out;
>>>> +       }
>>>> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>>> why is  pxa->ops needed ?
>>>
>>>> +       if (!pxa->ops) {
>>>> +               ret = -ENOMEM;
>>>> +               goto out;
>>>> +       }
>>>> +       pxa->pdata = pdata;
>>>> +
>>>> +       clk = clk_get(dev, "PXA-SDHCLK");
>>>> +       if (IS_ERR(clk)) {
>>>> +               dev_err(dev, "failed to get io clock\n");
>>>> +               ret = PTR_ERR(clk);
>>>> +               goto out;
>>>> +       }
>>>> +       pxa->clk = clk;
>>>> +       clk_enable(clk);
>>>> +
>>>> +       host = sdhci_pltfm_init(pdev, NULL);
>>>> +       if (IS_ERR(host))
>>>> +               return PTR_ERR(host);
>>>> +
>>>> +       pltfm_host = sdhci_priv(host);
>>>> +       pltfm_host->priv = pxa;
>>>> +
>>>> +       host->quirks = SDHCI_QUIRK_BROKEN_ADMA
>>>> +               | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
>>
>>>> +               | SDHCI_QUIRK_32BIT_DMA_ADDR
>>>> +               | SDHCI_QUIRK_32BIT_DMA_SIZE
>>>> +               | SDHCI_QUIRK_32BIT_ADMA_SIZE
>> my understand is mmp2 do not have such limitation?
>
> The silicon designer indicated we should set 32bit access.  64 bit is not supported.
But these 32BIT QUIRK does not mean whether 64 bit is supported or
not, but whether address is 32-bit aligned, and whether size is
multiple of 32 bits,
mmp2 do not have such limitation?
>
>>
>>>> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>> This quirk also not must?
>
> It is NOT a must but it saves a needless memory access per ADMA transfer to fetch a
> descriptor that says ADMA is done.  Since you are concerned about  the time 74 clocks
> takes this quirk makes everything faster.
For pxa955, this quirk must be set, otherwise it is not work properly.
For mmp2, ADMA work well regardless of this quirk, it is OK if you
think add this quirk would be efficiently,
One general question is quirk should be added only for chip limitation?
>
>>>> +
>>>> +       /* enable 1/8V DDR capable */
>>>> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>>>
>>>
>>> missing cap for bus width testing CMD19 works on MMP2
>> Will do the test of CMD19.
Test CMD19 on mmp2 A2 with MMC_CAP_BUS_WIDTH_TEST, timeout occurs here.
Since you have add one workaround to use mmc_compare_ext_csds, so same
result adding caps or not, timeout and use 1bit mode.

>>
>>> ADMA is fine for SD/eMMC -- SDMA is best for SDIO.  SDHCI_QUIRK_BROKEN_ADMA
>>> is best passed via pdata->quirks.
>>>
>>>
>>>> +
>>>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
>>>> +               /* on-chip device */
>>>> +               host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>>>> +               host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>>>> +       }
>>>> +
>>>> +       /* If slot design supports 8 bit data, indicate this to MMC. */
>>>> +       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>>> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>>> +
>>>> +       if (pdata && pdata->quirks)
>>>> +               host->quirks |= pdata->quirks;
>>>> +       if (pdata && pdata->host_caps)
>>>> +               host->mmc->caps |= pdata->host_caps;
>>>> +       if (pdata && pdata->pm_caps)
>>>> +               host->mmc->pm_caps |= pdata->pm_caps;
>>>> +
>>>> +       pxa->ops->set_clock = mmp2_set_clock;
>>>
>>> see comment about using reset callback instead of set_clock.
>> fine.
>>>
>>>> +       pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
>>>> +       if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
>>>> +               pxa->ops->get_ro = mmp2_get_ro;
>>>
>>> need 74 clock code --- I pasted it at the end the code.  The reason the code is
>>> needed is that SOME eMMC chips require 74 clocks.  The chips you have tested with
>>> do not need the clocks but JEDEC  Spec JESD84-A441-1 requires the clocks be
>>> generated.
>>>
>>>
>>>> +
>>>> +       host->ops = pxa->ops;
>>>> +
>>>> +       ret = sdhci_add_host(host);
>>>> +       if (ret) {
>>>> +               dev_err(&pdev->dev, "failed to add host\n");
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       platform_set_drvdata(pdev, host);
>>>> +
>>>> +       return 0;
>>>> +out:
>>>> +       if (host) {
>>>> +               clk_disable(pltfm_host->clk);
>>>> +               clk_put(pltfm_host->clk);
>>>> +               sdhci_pltfm_free(pdev);
>>>> +       }
>>>> +
>>>> +       if (pxa)
>>>> +               kfree(pxa->ops);
>>>> +       kfree(pxa);
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +
>>>> +static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
>>>> +{
>>>> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>>> +       struct sdhci_host *host = platform_get_drvdata(pdev);
>>>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> +       struct sdhci_pxa *pxa = pltfm_host->priv;
>>>> +       int dead = 0;
>>>> +       u32 scratch;
>>>> +
>>>> +       if (host) {
>>>> +               scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>>>> +               if (scratch == (u32)-1)
>>>> +                       dead = 1;
>>>
>>> not sure that this is the correct test -- will masked interrupts show
>>> up as set in the INT_STATUS register ?  from the code in sdhci.c and from
>>> my testing I have never seen -1 set.
>>>
>>> The test in sdhci.c is for 0x0 or for 0xffffffff is for a
>>> shared interrupt line to know if the interrupt is for this controller.
>> (u32)-1 is same as 0xffffffff
>
> not clear in my question.  Have you confirmed that when the controller is DEAD that
> -1 is read from the INT STATUS register ?  I have never seen this.
> Maybe there is a private register that can help.
not see -1 either, not impact function.
if you want to make sure, we can make the controller DEAD to have a try.

>
>>>
>>>> +
>>>> +               sdhci_remove_host(host, dead);
>>>> +
>>>> +               clk_disable(pxa->clk);
>>>> +               clk_put(pxa->clk);
>>>> +               sdhci_pltfm_free(pdev);
>>>> +
>>>> +               platform_set_drvdata(pdev, NULL);
>>>> +       }
>>>> +
>>>> +       if (pxa)
>>>> +               kfree(pxa->ops);
>>>> +       kfree(pxa);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_PM
>>>> +static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
>>>> +{
>>>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>>>> +
>>>> +       return sdhci_suspend_host(host, state);
>>>> +}
>>>> +
>>>> +static int sdhci_mmp2_resume(struct platform_device *dev)
>>>> +{
>>>> +       struct sdhci_host *host = platform_get_drvdata(dev);
>>>> +
>>>> +       return sdhci_resume_host(host);
>>>> +}
>>>> +#else
>>>> +#define sdhci_mmp2_suspend     NULL
>>>> +#define sdhci_mmp2_resume      NULL
>>>> +#endif
>>>> +
>>>> +
>>>> +static struct platform_driver sdhci_mmp2_driver = {
>>>> +       .driver         = {
>>>> +               .name   = "sdhci-mmp2",
>>>> +               .owner  = THIS_MODULE,
>>>> +       },
>>>> +       .probe          = sdhci_mmp2_probe,
>>>> +       .remove         = __devexit_p(sdhci_mmp2_remove),
>>>> +#ifdef CONFIG_PM
>>>> +       .suspend        = sdhci_mmp2_suspend,
>>>> +       .resume         = sdhci_mmp2_resume,
>>>> +#endif
>>>> +};
>>>> +static int __init sdhci_mmp2_init(void)
>>>> +{
>>>> +       return platform_driver_register(&sdhci_mmp2_driver);
>>>> +}
>>>> +
>>>> +static void __exit sdhci_mmp2_exit(void)
>>>> +{
>>>> +       platform_driver_unregister(&sdhci_mmp2_driver);
>>>> +}
>>>> +
>>>> +module_init(sdhci_mmp2_init);
>>>> +module_exit(sdhci_mmp2_exit);
>>>> +
>>>> +MODULE_DESCRIPTION("SDHCI driver for mmp2");
>>>> +MODULE_AUTHOR("Marvell International Ltd.");
>>>> +MODULE_LICENSE("GPL v2");
>>>> +
>>>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>>>> deleted file mode 100644
>>>> index 089c9a6..0000000
>>>> --- a/drivers/mmc/host/sdhci-pxa.c
>>>> +++ /dev/null
>>>> @@ -1,303 +0,0 @@
>>>> -/* 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>
>>>> - *
>>>
>>> Keep the authors.  They all contributed to the code.
>> fine
>>>
>>>> - * 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>>>> -{
>>>> -       u16 ctrl_2;
>>>> -
>>>> -       /*
>>>> -        * Set V18_EN -- UHS modes do not work without this.
>>>> -        * does not change signaling voltage
>>>> -        */
>>>> -       ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>>> -
>>>> -       /* Select Bus Speed Mode for host */
>>>> -       ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>>>> -       switch (uhs) {
>>>> -       case MMC_TIMING_UHS_SDR12:
>>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>>>> -               break;
>>>> -       case MMC_TIMING_UHS_SDR25:
>>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>>>> -               break;
>>>> -       case MMC_TIMING_UHS_SDR50:
>>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
>>>> -               break;
>>>> -       case MMC_TIMING_UHS_SDR104:
>>>> -               ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
>>>> -               break;
>>>> -       case MMC_TIMING_UHS_DDR50:
>>>> -               ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
>>>> -               break;
>>>> -       }
>>>> -
>>>> -       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>> -       pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
>>>> -               __func__, mmc_hostname(host->mmc), uhs, ctrl_2);
>>>> -
>>>> -       return 0;
>>>> -}
>>>> -
>>>> -static struct sdhci_ops sdhci_pxa_ops = {
>>>> -       .set_uhs_signaling = set_uhs_signaling,
>>>> -       .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");
>>>> -               return PTR_ERR(host);
>>>> -       }
>>>> -
>>>> -       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
>>>> -               | SDHCI_QUIRK_32BIT_DMA_ADDR
>>>> -               | SDHCI_QUIRK_32BIT_DMA_SIZE
>>>> -               | SDHCI_QUIRK_32BIT_ADMA_SIZE
>>>> -               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>>>> -
>>>> -       if (pdata->quirks)
>>>> -               host->quirks |= pdata->quirks;
>>>> -
>>>> -       /* enable 1/8V DDR capable */
>>>> -       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>>>> -
>>>> -       /* If slot design supports 8 bit data, indicate this to MMC. */
>>>> -       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>>> -               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>>> -
>>>> -       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) {
>>>> -               clk_put(pxa->clk);
>>>> -               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, pm_message_t state)
>>>> -{
>>>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>>>> -
>>>> -       return sdhci_suspend_host(host, state);
>>>> -}
>>>> -
>>>> -static int sdhci_pxa_resume(struct platform_device *dev)
>>>> -{
>>>> -       struct sdhci_host *host = platform_get_drvdata(dev);
>>>> -
>>>> -       return sdhci_resume_host(host);
>>>> -}
>>>> -#else
>>>> -#define sdhci_pxa_suspend      NULL
>>>> -#define sdhci_pxa_resume       NULL
>>>> -#endif
>>>> -
>>>> -static struct platform_driver sdhci_pxa_driver = {
>>>> -       .probe          = sdhci_pxa_probe,
>>>> -       .remove         = __devexit_p(sdhci_pxa_remove),
>>>> -       .suspend        = sdhci_pxa_suspend,
>>>> -       .resume         = sdhci_pxa_resume,
>>>> -       .driver         = {
>>>> -               .name   = DRIVER_NAME,
>>>> -               .owner  = THIS_MODULE,
>>>> -       },
>>>> -};
>>>> -
>>>> -/*****************************************************************************\
>>>> - *                                                                           *
>>>> - * 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
>>>
>>>
>>>
>>>
>>> code to send 74 clocks when card is inserted.  Need to callback hook so
>>> code is called from sdhci.c
>>>
>>> from patch submitted to linux-mmc
>>>
>>> from: Philip Rakity <prakity@marvell.com>
>>> Date: February 13, 2011 11:24:18 PM PST
>>> To: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
>>> Cc: Mark Brown <markb@marvell.com>
>>> Subject: [PATCH] sdhci: Add support PXA168, PXA910, and MMP2 controllers
>>>
>>>
>>> +static void generate_initial_74_clocks(struct sdhci_host *host, u8 power_mode)
>>> +{
>>> +       struct sdhci_pxa *pxa = sdhci_priv(host);
>>> +       u16 tmp;
>>> +       int count;
>>> +
>>> +       if (pxa->power_mode == MMC_POWER_UP
>>> +       && power_mode == MMC_POWER_ON) {
>>> +
>>> +               pr_debug("%s:%s ENTER: slot->power_mode = %d,"
>>> +                       "ios->power_mode = %d\n",
>>> +                       __func__,
>>> +                       mmc_hostname(host->mmc),
>>> +                       pxa->power_mode,
>>> +                       power_mode);
>>> +
>>> +               /* set we want notice of when 74 clocks are sent */
>>> +               tmp = readw(host->ioaddr + SD_CE_ATA_2);
>>> +               tmp |= SDCE_MISC_INT_EN;
>>> +               writew(tmp, host->ioaddr + SD_CE_ATA_2);
>>> +
>>> +               /* start sending the 74 clocks */
>>> +               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
>>> +               tmp |= SDCFG_GEN_PAD_CLK_ON;
>>> +               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
>>
>> Will merge the code together?
>> How about simplifying the code to
>> if (pxa->power_mode == MMC_POWER_UP
>>      && power_mode == MMC_POWER_ON) {
>>               /* start sending the 74 clocks */
>>               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
>>               tmp |= SDCFG_GEN_PAD_CLK_ON;
>>               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
>>       }
>>       pxa->power_mode = power_mode;
>> }
>
> see comment below.
>
>> Since core.c already consider the time for 74 clocks.
>> static void mmc_power_up(struct mmc_host *host)
>> {
>> ~~
>>         /*
>>         * This delay must be at least 74 clock sizes, or 1 ms, or the
>>         * time required to reach a stable voltage.
>>         */
>>        mmc_delay(10);
>> }
>> The issue of our controller is clock is not start until there is cmd or data.
>> While 10ms is enough for 74 clock, so just need to start and no need
>> to wait finish.
>>
>>
>>> +
>>> +               /* slowest speed is about 100KHz or 10usec per clock */
>>> +               udelay(740);
>>> +               count = 0;
>>> +#define MAX_WAIT_COUNT 5
>>> +               while (count++ < MAX_WAIT_COUNT) {
>>> +                       if ((readw(host->ioaddr + SD_CE_ATA_2)
>>> +                                       & SDCE_MISC_INT) == 0)
>>> +                               break;
>>> +                       udelay(10);
>>> +               }
>>> +
>>> +               if (count == MAX_WAIT_COUNT)
>>> +                       printk(KERN_WARNING"%s: %s: 74 clock interrupt "
>>> +                               "not cleared\n",
>>> +                               __func__, mmc_hostname(host->mmc));
>>> +               /* clear the interrupt bit if posted */
>>> +               tmp = readw(host->ioaddr + SD_CE_ATA_2);
>>> +               tmp |= SDCE_MISC_INT;
>>> +               writew(tmp, host->ioaddr + SD_CE_ATA_2);
>>> +       }
>>> +       pxa->power_mode = power_mode;
>>> +}
>>> +
>>>
>
> is the 740 us delay important  once per card insert ?  The code works as submitted and was tested
> with the silicon design team.  The other option would be to enable the interrupt rather then wait but
> in that case you will need to plumb the interrupt in.  Did not want to touch base code and
> was not concerned about 740 us.
>
> The mmc_delay value is WAY too high and should be lowered.
mmc_delay will schedule out, so the value has no defect.
Finish 74 clocks during mmc_delay time could make the code simple, and
solve the silicon issue.
It is OK if you want to use polling, function is OK.

>
> The 74 clock code is needed for any slot where a card can be plugged in.   You cannot assume it is
> SD.  It could be mmc (or sdio) or combo.
>
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philip Rakity May 31, 2011, 3:57 p.m. UTC | #12
Hi Zhangfei,

<Snip>
>>>> 
>>>> It is probably better to NOT name the file sdhci-mmp2.c but
>>>> something like sdchi-pxaV3.c since other SoC's may use this controller.
>>> fine
>> 
>> The same renaming change is needed for the pxa9xx patch.
> Do you mean use sdhci-pxa9xx.c to replace sdhci-pxa910.c, pxa955 do
> not use the same driver, but it does not matter.

Give it a name that is not chip specific so the driver can be reused.
the name in the probe definition should be changed as well.

>>>>> +static struct platform_driver sdhci_mmp2_driver = {
>>>>> +       .driver         = {
>>>>> +               .name   = "sdhci-mmp2",

change to sdhci-pxaV3

>>>>> +               .owner  = THIS_MODULE,
>>>>> +       },
>>>>> +       .probe          = sdhci_mmp2_probe,
>>>>> +       .remove         = __devexit_p(sdhci_mmp2_remove),
>>>>> +#ifdef CONFIG_PM
>>>>> +       .suspend        = sdhci_mmp2_suspend,
>>>>> +       .resume         = sdhci_mmp2_resume,
>>>>> +#endif
>>>>> +};

for pxa910 and for pxa168 they use a sdhci-pxav2 controller and the code should be updated accordingly.

>>>>> \
>> 
>>> 
>>>>> +               | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>>> This quirk also not must?
>> 
>> It is NOT a must but it saves a needless memory access per ADMA transfer to fetch a
>> descriptor that says ADMA is done.  Since you are concerned about  the time 74 clocks
>> takes this quirk makes everything faster.
> For pxa955, this quirk must be set, otherwise it is not work properly.
> For mmp2, ADMA work well regardless of this quirk, it is OK if you
> think add this quirk would be efficiently,
> One general question is quirk should be added only for chip limitation?

The code is backwards in sdhci.c.  The original code assumed a extra descriptor was needed.
The quirk removed the limitation.  Strange use for a quirk to make things work as they should 
but preserved original behavior.

>> 
>>>>> +
>>>>> +       /* enable 1/8V DDR capable */
>>>>> +       host->mmc->caps |= MMC_CAP_1_8V_DDR;
>>>> 
>>>> 
>>>> missing cap for bus width testing CMD19 works on MMP2
>>> Will do the test of CMD19.
> Test CMD19 on mmp2 A2 with MMC_CAP_BUS_WIDTH_TEST, timeout occurs here.

check the chip revision that you have.  This is fixed in newer silicon.

>>>>> +       if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
>>>>> +               u32 tmp = 0;
>>>>> +
>>>>> +               tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
>>>>> +               tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
>>>>> +               writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
>>>>> +       }
>>>>> +}
>>>> 
>>>> One cannot know what card if being inserted or removed by a user
>>>> so the safest choice is to leave clock gating OFF by default.
>>>> 
>>>> PXA_FLAG_DISABLE_CLOCK_GATING should be renamed
>>>> to
>>>> PXA_FLAG_ENABLE_CLOCK_GATING.
>>>> 
>>>> If you have a card which is permanent then the flags should be used
>>>> to ENABLE clock gating.  Otherwise clock gating should be left OFF by default.
> Fine,
> My understanding is it is not a question, user have to know which slot
> should open/close clock gating, and set flag accordingly.
> Currently only 8787 require clock free running.

NO USER can know if a card requires clock gating or not.  There are other cards
that require no clock gating 8688 for example.



> Since you have add one workaround to use mmc_compare_ext_csds, so same
> result adding caps or not, timeout and use 1bit mode.

CMD19 maps the standard.  If no ext_csd they bus width test will make card work in 1 bit mode
when CMD19 might make it run in 4 or 8 bit mode.  CMD19 is best if supported.

>> \
>> not clear in my question.  Have you confirmed that when the controller is DEAD that
>> -1 is read from the INT STATUS register ?  I have never seen this.
>> Maybe there is a private register that can help.
> not see -1 either, not impact function.
> if you want to make sure, we can make the controller DEAD to have a try.

Thank You

>>>> +}
>>>> +
>>>> 
>> 
>> is the 740 us delay important  once per card insert ?  The code works as submitted and was tested
>> with the silicon design team.  The other option would be to enable the interrupt rather then wait but
>> in that case you will need to plumb the interrupt in.  Did not want to touch base code and
>> was not concerned about 740 us.
>> 
>> The mmc_delay value is WAY too high and should be lowered.
> mmc_delay will schedule out, so the value has no defect.

Not really -- it delays the start up time for the device with a value that is this high.  It does not effect the other
components.

> Finish 74 clocks during mmc_delay time could make the code simple, and
> solve the silicon issue.
> It is OK if you want to use polling, function is OK.

thank you -- in practice it just delays and never polls.  The code completes.  If you are concerned about the
time you can try lowering the 74 uSec delay.  at 400KHz the code takes 185 uSec rather than 740.  I wanted
to be safe with the value.  
> 
>> 
>> The 74 clock code is needed for any slot where a card can be plugged in.   You cannot assume it is
>> SD.  It could be mmc (or sdio) or combo.




>> 
>>>>> 
>>>> 
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> 
>> 
>> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h b/arch/arm/plat-pxa/include/plat/sdhci.h
index 1ab332e..92becc0 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -13,23 +13,58 @@ 
 #ifndef __PLAT_PXA_SDHCI_H
 #define __PLAT_PXA_SDHCI_H
 
+#include <linux/mmc/sdhci.h>
+#include <linux/platform_device.h>
+
 /* pxa specific flag */
 /* Require clock free running */
 #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
-
+/* card alwayes wired to host, like on-chip emmc */
+#define PXA_FLAG_CARD_PERMANENT	(1<<1)
 /* Board design supports 8-bit data on SD/SDIO BUS */
 #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
+/* card not use wp, such as micro sd card */
+#define PXA_FLAG_CARD_NO_WP	(3<<1)
 
+struct sdhci_pxa;
 /*
  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
- * @max_speed: the maximum speed supported
- * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay_cycles:
+ *	mmp2: each step is roughly 100ps, 5bits width
+ *	pxa910: each step is 1ns, 4bits width
+ * @clk_delay_sel: select clk_delay, used on pxa910
+ *	0: choose feedback clk
+ *	1: choose feedback clk + delay value
+ *	2: choose internal clk
+ * @ext_cd_gpio: gpio pin used for external CD line
+ * @ext_cd_gpio_invert: invert values for external CD gpio line
+ * @clk_delay_enable: enable clk_delay or not, used on pxa910
+ * @max_speed: the maximum speed supported
+ * @host_caps: Standard MMC host capabilities bit field.
+ * @quirks: quirks of platfrom
+ * @pm_caps: pm_caps of platfrom
  */
 struct sdhci_pxa_platdata {
+	unsigned int	flags;
+	unsigned int	clk_delay_cycles;
+	unsigned int	clk_delay_sel;
+	unsigned int	ext_cd_gpio;
+	bool		ext_cd_gpio_invert;
+	bool		clk_delay_enable;
 	unsigned int	max_speed;
+	unsigned int	host_caps;
 	unsigned int	quirks;
-	unsigned int	flags;
+	unsigned int	pm_caps;
+};
+
+struct sdhci_pxa {
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+	struct sdhci_ops		*ops;
+
+	u8	clk_enable;
+	u8	power_mode;
 };
 
 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 862235e..1e520e3 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -181,14 +181,15 @@  config MMC_SDHCI_S3C
 
 	  If unsure, say N.
 
-config MMC_SDHCI_PXA
-	tristate "Marvell PXA168/PXA910/MMP2 SD Host Controller support"
+config MMC_SDHCI_MMP2
+	tristate "Marvell MMP2 SD Host Controller support"
 	depends on ARCH_PXA || ARCH_MMP
 	select MMC_SDHCI
+	select MMC_SDHCI_PLTFM
 	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
+	  This selects the Marvell(R) MMP2 SD Host Controller.
+	  If you have a MMP2 platform with SD Host Controller
 	  and a card slot, say Y or M here.
 
 	  If unsure, say N.
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 4933004..f8650e0 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -9,7 +9,7 @@  obj-$(CONFIG_MMC_MXC)		+= mxcmmc.o
 obj-$(CONFIG_MMC_MXS)		+= mxs-mmc.o
 obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
-obj-$(CONFIG_MMC_SDHCI_PXA)	+= sdhci-pxa.o
+obj-$(CONFIG_MMC_SDHCI_MMP2)	+= sdhci-mmp2.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-mmp2.c b/drivers/mmc/host/sdhci-mmp2.c
new file mode 100644
index 0000000..566d525
--- /dev/null
+++ b/drivers/mmc/host/sdhci-mmp2.c
@@ -0,0 +1,304 @@ 
+/*
+ * Copyright (C) 2010 Marvell International Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <plat/sdhci.h>
+#include <linux/slab.h>
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+#define MMP2_SD_FIFO_PARAM		0x104
+#define MMP2_DIS_PAD_SD_CLK_GATE	0x400
+
+#define MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP		0x10A
+#define MMP2_SDCLK_SEL	0x100
+#define MMP2_SDCLK_DELAY_SHIFT	9
+#define MMP2_SDCLK_DELAY_MASK	0x1f
+
+#define SD_CFG_FIFO_PARAM       0x100
+#define SDCFG_GEN_PAD_CLK_ON	(1<<6)
+#define SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
+#define SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
+
+#define SD_SPI_MODE          0x108
+#define SD_CE_ATA_1          0x10C
+
+#define SD_CE_ATA_2          0x10E
+#define SDCE_MISC_INT		(1<<2)
+#define SDCE_MISC_INT_EN	(1<<1)
+
+static void mmp2_soc_set_timing(struct sdhci_host *host,
+				struct sdhci_pxa_platdata *pdata)
+{
+	/*
+	 * tune timing of read data/command when crc error happen
+	 * no performance impact
+	 */
+	if (pdata && 0 != pdata->clk_delay_cycles) {
+		u16 tmp;
+
+		tmp = readw(host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
+		tmp |= (pdata->clk_delay_cycles & MMP2_SDCLK_DELAY_MASK)
+					<< MMP2_SDCLK_DELAY_SHIFT;
+		tmp |= MMP2_SDCLK_SEL;
+		writew(tmp, host->ioaddr + MMP2_SD_CLOCK_AND_BURST_SIZE_SETUP);
+	}
+
+	/*
+	 * disable clock gating per some cards requirement
+	 */
+
+	if (pdata && pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
+		u32 tmp = 0;
+
+		tmp = readl(host->ioaddr + MMP2_SD_FIFO_PARAM);
+		tmp |= MMP2_DIS_PAD_SD_CLK_GATE;
+		writel(tmp, host->ioaddr + MMP2_SD_FIFO_PARAM);
+	}
+}
+
+static unsigned int mmp2_get_ro(struct sdhci_host *host)
+{
+	/* Micro SD does not support write-protect feature */
+	return 0;
+}
+
+static void mmp2_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+
+	if (clock)
+		mmp2_soc_set_timing(host, pxa->pdata);
+}
+
+static int mmp2_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
+{
+	u16 ctrl_2;
+
+	/*
+	 * Set V18_EN -- UHS modes do not work without this.
+	 * does not change signaling voltage
+	 */
+	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+
+	/* Select Bus Speed Mode for host */
+	ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
+	switch (uhs) {
+	case MMC_TIMING_UHS_SDR12:
+		ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
+		break;
+	case MMC_TIMING_UHS_SDR25:
+		ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
+		break;
+	case MMC_TIMING_UHS_SDR50:
+		ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
+		break;
+	case MMC_TIMING_UHS_SDR104:
+		ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
+		break;
+	case MMC_TIMING_UHS_DDR50:
+		ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
+		break;
+	}
+
+	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+	dev_dbg(mmc_dev(host->mmc),
+		"%s:%s uhs = %d, ctrl_2 = %04X\n",
+		__func__, mmc_hostname(host->mmc), uhs, ctrl_2);
+
+	return 0;
+}
+
+static int __devinit sdhci_mmp2_probe(struct platform_device *pdev)
+{
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sdhci_host *host = NULL;
+	struct sdhci_pxa *pxa = NULL;
+	int ret;
+	struct clk *clk;
+
+	pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
+	if (!pxa) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
+	if (!pxa->ops) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	pxa->pdata = pdata;
+
+	clk = clk_get(dev, "PXA-SDHCLK");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "failed to get io clock\n");
+		ret = PTR_ERR(clk);
+		goto out;
+	}
+	pxa->clk = clk;
+	clk_enable(clk);
+
+	host = sdhci_pltfm_init(pdev, NULL);
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	pltfm_host = sdhci_priv(host);
+	pltfm_host->priv = pxa;
+
+	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
+		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
+		| SDHCI_QUIRK_32BIT_DMA_ADDR
+		| SDHCI_QUIRK_32BIT_DMA_SIZE
+		| SDHCI_QUIRK_32BIT_ADMA_SIZE
+		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
+
+	/* enable 1/8V DDR capable */
+	host->mmc->caps |= MMC_CAP_1_8V_DDR;
+
+	if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
+		/* on-chip device */
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+	}
+
+	/* If slot design supports 8 bit data, indicate this to MMC. */
+	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
+		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+
+	if (pdata && pdata->quirks)
+		host->quirks |= pdata->quirks;
+	if (pdata && pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+	if (pdata && pdata->pm_caps)
+		host->mmc->pm_caps |= pdata->pm_caps;
+
+	pxa->ops->set_clock = mmp2_set_clock;
+	pxa->ops->set_uhs_signaling = mmp2_set_uhs_signaling;
+	if (pdata && pdata->flags & PXA_FLAG_CARD_NO_WP)
+		pxa->ops->get_ro = mmp2_get_ro;
+
+	host->ops = pxa->ops;
+
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add host\n");
+		goto out;
+	}
+
+	platform_set_drvdata(pdev, host);
+
+	return 0;
+out:
+	if (host) {
+		clk_disable(pltfm_host->clk);
+		clk_put(pltfm_host->clk);
+		sdhci_pltfm_free(pdev);
+	}
+
+	if (pxa)
+		kfree(pxa->ops);
+	kfree(pxa);
+
+	return ret;
+}
+
+static int __devexit sdhci_mmp2_remove(struct platform_device *pdev)
+{
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+	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);
+
+		clk_disable(pxa->clk);
+		clk_put(pxa->clk);
+		sdhci_pltfm_free(pdev);
+
+		platform_set_drvdata(pdev, NULL);
+	}
+
+	if (pxa)
+		kfree(pxa->ops);
+	kfree(pxa);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int sdhci_mmp2_suspend(struct platform_device *dev, pm_message_t state)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+
+	return sdhci_suspend_host(host, state);
+}
+
+static int sdhci_mmp2_resume(struct platform_device *dev)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+
+	return sdhci_resume_host(host);
+}
+#else
+#define sdhci_mmp2_suspend	NULL
+#define sdhci_mmp2_resume	NULL
+#endif
+
+
+static struct platform_driver sdhci_mmp2_driver = {
+	.driver		= {
+		.name	= "sdhci-mmp2",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_mmp2_probe,
+	.remove		= __devexit_p(sdhci_mmp2_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_mmp2_suspend,
+	.resume		= sdhci_mmp2_resume,
+#endif
+};
+static int __init sdhci_mmp2_init(void)
+{
+	return platform_driver_register(&sdhci_mmp2_driver);
+}
+
+static void __exit sdhci_mmp2_exit(void)
+{
+	platform_driver_unregister(&sdhci_mmp2_driver);
+}
+
+module_init(sdhci_mmp2_init);
+module_exit(sdhci_mmp2_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for mmp2");
+MODULE_AUTHOR("Marvell International Ltd.");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
deleted file mode 100644
index 089c9a6..0000000
--- a/drivers/mmc/host/sdhci-pxa.c
+++ /dev/null
@@ -1,303 +0,0 @@ 
-/* 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 to 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->flags & PXA_FLAG_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 int set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
-{
-	u16 ctrl_2;
-
-	/*
-	 * Set V18_EN -- UHS modes do not work without this.
-	 * does not change signaling voltage
-	 */
-	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-
-	/* Select Bus Speed Mode for host */
-	ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
-	switch (uhs) {
-	case MMC_TIMING_UHS_SDR12:
-		ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
-		break;
-	case MMC_TIMING_UHS_SDR25:
-		ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
-		break;
-	case MMC_TIMING_UHS_SDR50:
-		ctrl_2 |= SDHCI_CTRL_UHS_SDR50 | SDHCI_CTRL_VDD_180;
-		break;
-	case MMC_TIMING_UHS_SDR104:
-		ctrl_2 |= SDHCI_CTRL_UHS_SDR104 | SDHCI_CTRL_VDD_180;
-		break;
-	case MMC_TIMING_UHS_DDR50:
-		ctrl_2 |= SDHCI_CTRL_UHS_DDR50 | SDHCI_CTRL_VDD_180;
-		break;
-	}
-
-	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
-	pr_debug("%s:%s uhs = %d, ctrl_2 = %04X\n",
-		__func__, mmc_hostname(host->mmc), uhs, ctrl_2);
-
-	return 0;
-}
-
-static struct sdhci_ops sdhci_pxa_ops = {
-	.set_uhs_signaling = set_uhs_signaling,
-	.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");
-		return PTR_ERR(host);
-	}
-
-	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
-		| SDHCI_QUIRK_32BIT_DMA_ADDR
-		| SDHCI_QUIRK_32BIT_DMA_SIZE
-		| SDHCI_QUIRK_32BIT_ADMA_SIZE
-		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
-
-	if (pdata->quirks)
-		host->quirks |= pdata->quirks;
-
-	/* enable 1/8V DDR capable */
-	host->mmc->caps |= MMC_CAP_1_8V_DDR;
-
-	/* If slot design supports 8 bit data, indicate this to MMC. */
-	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
-		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
-
-	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) {
-		clk_put(pxa->clk);
-		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, pm_message_t state)
-{
-	struct sdhci_host *host = platform_get_drvdata(dev);
-
-	return sdhci_suspend_host(host, state);
-}
-
-static int sdhci_pxa_resume(struct platform_device *dev)
-{
-	struct sdhci_host *host = platform_get_drvdata(dev);
-
-	return sdhci_resume_host(host);
-}
-#else
-#define sdhci_pxa_suspend	NULL
-#define sdhci_pxa_resume	NULL
-#endif
-
-static struct platform_driver sdhci_pxa_driver = {
-	.probe		= sdhci_pxa_probe,
-	.remove		= __devexit_p(sdhci_pxa_remove),
-	.suspend	= sdhci_pxa_suspend,
-	.resume		= sdhci_pxa_resume,
-	.driver		= {
-		.name	= DRIVER_NAME,
-		.owner	= THIS_MODULE,
-	},
-};
-
-/*****************************************************************************\
- *                                                                           *
- * 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");