On Wed, Sep 28, 2011 at 4:02 PM, Shashidhar Hiremath
<shashidh...@vayavyalabs.com> wrote:
>
> Signed-off-by: Shashidhar Hiremath <shashidh...@vayavyalabs.com>
> ---
>  drivers/mmc/host/Kconfig   |   11 ++++
>  drivers/mmc/host/dw_mmc.c  |  126 
> ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/dw_mmc.h  |   12 ++++
>  include/linux/mmc/dw_mmc.h |    4 ++
>  4 files changed, 153 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 8c87096..84d8908 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -526,6 +526,17 @@ config MMC_DW
>          block, this provides host support for SD and MMC interfaces, in both
>          PIO and external DMA modes.
>
> +config MMC_DW_PCI
> +       bool "MMC_DW Support On PCI bus"
> +       depends on MMC_DW && PCI
> +       help
> +         This selects the PCI for the Synopsys Designware Mobile Storage IP.
> +
> +         If you have a controller with this interface, say Y or M here.
> +
> +         If unsure, say N.
> +
> +
>  config MMC_DW_IDMAC
>        bool "Internal DMAC interface"
>        depends on MMC_DW
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ff0f714..74f28a5 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -21,7 +21,11 @@
>  #include <linux/interrupt.h>
>  #include <linux/ioport.h>
>  #include <linux/module.h>
> +#ifdef CONFIG_MMC_DW_PCI
> +#include <linux/pci.h>
> +#else
>  #include <linux/platform_device.h>
> +#endif

This doesn't need an #ifdef.

>  #include <linux/scatterlist.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> @@ -101,6 +105,18 @@ struct dw_mci_slot {
>        int                     last_detect_state;
>  };
>
> +#ifdef CONFIG_MMC_DW_PCI
> +static struct pci_device_id dw_mci_id  = {  PCI_DEVICE(DEVICE_ID, VENDOR_ID) 
> };
> +
> +struct dw_mci_board pci_board_data = {
> +       .num_slots                      = 1,
> +       .caps                           = DW_MCI_CAPABILITIES,
> +       .bus_hz                         = 33 * 1000 * 1000,

Is this value correct? The bus this refers to is the SD bus, so 33MHz
is not fast.

> +       .detect_delay_ms                = 200,
> +       .fifo_depth                     = 32,
> +};
> +#endif
> +
>  static struct workqueue_struct *dw_mci_card_workqueue;
>
>  #if defined(CONFIG_DEBUG_FS)
> @@ -682,6 +698,9 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
> mmc_ios *ios)
>  {
>        struct dw_mci_slot *slot = mmc_priv(mmc);
>        u32 regs;
> +#ifdef CONFIG_MMC_DW_PCI
> +       u32 card_detect;
> +#endif
>
>        /* set default 1 bit mode */
>        slot->ctype = SDMMC_CTYPE_1BIT;
> @@ -716,7 +735,15 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
> mmc_ios *ios)
>        switch (ios->power_mode) {
>        case MMC_POWER_UP:
>                set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
> +#ifdef CONFIG_MMC_DW_PCI
> +               /* Enable Power to the card that has been detected */
> +               card_detect = mci_readl(slot->host, CDETECT);
> +               mci_writel(slot->host, PWREN, ((~card_detect) & 0x1));
> +               break;
> +       case MMC_POWER_OFF:
> +               mci_writel(slot->host, PWREN, 0);
>                break;
> +#endif

It may be safe to not #ifdef this, all the hardware I have uses an
external regulator so the PWREN bits do nothing. I could imagine
hardware that was non-PCI that could use PWREN however.

>        default:
>                break;
>        }
> @@ -1775,7 +1802,12 @@ static bool mci_wait_reset(struct device *dev, struct 
> dw_mci *host)
>        return false;
>  }
>
> +#ifdef CONFIG_MMC_DW_PCI
> +static int __devinit dw_mci_probe(struct pci_dev *pdev,
> +                                 const struct pci_device_id *entries)
> +#else
>  static int dw_mci_probe(struct platform_device *pdev)
> +#endif

Possibly it would be better to split this into three functions:

dw_mci_pci_probe
dw_mci_platform_probe
dw_mci_probe

And then the pci/platform probe functions can call the generic probe
function to do the common parts of the setup.

> +#ifdef CONFIG_MMC_DW_PCI
> +static void __devexit dw_mci_remove(struct pci_dev *pdev)
> +#else
>  static int __exit dw_mci_remove(struct platform_device *pdev)
> +#endif

Again this should probably be split into separate functions to reduce
the number of #ifdefs.

>  {
> +
> +#ifdef CONFIG_MMC_DW_PCI
> +       struct dw_mci *host = pci_get_drvdata(pdev);
> +#else
>        struct dw_mci *host = platform_get_drvdata(pdev);
> +#endif
>        int i;
>
>        mci_writel(host, RINTSTS, 0xFFFFFFFF);
>        mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>
> +#ifdef CONFIG_MMC_DW_PCI
> +       pci_set_drvdata(pdev, NULL);
> +#else
>        platform_set_drvdata(pdev, NULL);
> +#endif
>
>        for (i = 0; i < host->num_slots; i++) {
>                dev_dbg(&pdev->dev, "remove slot %d\n", i);
> @@ -1992,7 +2074,11 @@ static int __exit dw_mci_remove(struct platform_device 
> *pdev)
>        mci_writel(host, CLKENA, 0);
>        mci_writel(host, CLKSRC, 0);
>
> +#ifdef CONFIG_MMC_DW_PCI
> +       free_irq(pdev->irq, host);
> +#else
>        free_irq(platform_get_irq(pdev, 0), host);
> +#endif
>        destroy_workqueue(dw_mci_card_workqueue);
>        dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma);
>
> @@ -2006,18 +2092,31 @@ static int __exit dw_mci_remove(struct 
> platform_device *pdev)
>
>        iounmap(host->regs);
>
> +#ifdef CONFIG_MMC_DW_PCI
> +       pci_release_regions(pdev);
> +#endif
>        kfree(host);
> +#ifndef CONFIG_MMC_DW_PCI
>        return 0;
> +#endif
>  }
>
>  #ifdef CONFIG_PM
>  /*
>  * TODO: we should probably disable the clock to the card in the suspend path.
>  */
> +#ifdef CONFIG_MMC_DW_PCI
> +static int dw_mci_suspend(struct pci_dev *pdev, pm_message_t mesg)
> +#else
>  static int dw_mci_suspend(struct platform_device *pdev, pm_message_t mesg)
> +#endif
>  {
>        int i, ret;
> +#ifdef CONFIG_MMC_DW_PCI
> +       struct dw_mci *host = pci_get_drvdata(pdev);
> +#else
>        struct dw_mci *host = platform_get_drvdata(pdev);
> +#endif
>
>        for (i = 0; i < host->num_slots; i++) {
>                struct dw_mci_slot *slot = host->slot[i];
> @@ -2040,10 +2139,18 @@ static int dw_mci_suspend(struct platform_device 
> *pdev, pm_message_t mesg)
>        return 0;
>  }
>
> +#ifdef CONFIG_MMC_DW_PCI
> +static int dw_mci_resume(struct pci_dev *pdev)
> +#else
>  static int dw_mci_resume(struct platform_device *pdev)
> +#endif
>  {
>        int i, ret;
> +#ifdef CONFIG_MMC_DW_PCI
> +       struct dw_mci *host = pci_get_drvdata(pdev);
> +#else
>        struct dw_mci *host = platform_get_drvdata(pdev);
> +#endif

Even though these functions are quite simple it would be better to code them as

dw_mci_platform_resume(struct platform_device *pdev)
 calling dw_mci_resume(struct dw_mci *host)

dw_mci_pci_resume(struct pci_dev *pdev)
 calling dw_mci_resume(struct dw_mci *host)

To reduce the #ifdef overhead.

>
>        if (host->vmmc)
>                regulator_enable(host->vmmc);
> @@ -2081,6 +2188,16 @@ static int dw_mci_resume(struct platform_device *pdev)
>  #define dw_mci_resume  NULL
>  #endif /* CONFIG_PM */
>
> +#ifdef CONFIG_MMC_DW_PCI
> +static struct pci_driver dw_mci_driver = {
> +       .name           = "dw_mmc",
> +       .id_table       = &dw_mci_id,
> +       .probe          = dw_mci_probe,
> +       .remove         = dw_mci_remove,
> +       .suspend        = dw_mci_suspend,
> +       .resume         = dw_mci_resume,
> +};
> +#else
>  static struct platform_driver dw_mci_driver = {
>        .remove         = __exit_p(dw_mci_remove),
>        .suspend        = dw_mci_suspend,
> @@ -2089,15 +2206,24 @@ static struct platform_driver dw_mci_driver = {
>                .name           = "dw_mmc",
>        },
>  };
> +#endif
>
>  static int __init dw_mci_init(void)
>  {
> +#ifdef CONFIG_MMC_DW_PCI
> +       return pci_register_driver(&dw_mci_driver);
> +#else
>        return platform_driver_probe(&dw_mci_driver, dw_mci_probe);
> +#endif
>  }
>
>  static void __exit dw_mci_exit(void)
>  {
> +#ifdef CONFIG_MMC_DW_PCI
> +       pci_unregister_driver(&dw_mci_driver);
> +#else
>        platform_driver_unregister(&dw_mci_driver);
> +#endif
>  }
>
>  module_init(dw_mci_init);
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 027d377..3b1e459 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -164,4 +164,16 @@
>        (*(volatile u64 __force *)((dev)->regs + SDMMC_##reg) = (value))
>  #endif
>
> +/* PCI Specific macros */
> +#ifdef CONFIG_MMC_DW_PCI
> +#define PCI_BAR_NO 2
> +#define COMPLETE_BAR 0
> +/* Dummy Device Id and Vendor Id */
> +#define VENDOR_ID 0x700
> +#define DEVICE_ID 0x1107
> +/* Defining the Capabilities */
> +#define DW_MCI_CAPABILITIES (MMC_CAP_4_BIT_DATA | MMC_CAP_MMC_HIGHSPEED |\
> +                        MMC_CAP_SD_HIGHSPEED | MMC_CAP_8_BIT_DATA | 
> MMC_CAP_SDIO_IRQ)
> +#endif
> +

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

Reply via email to