From: James Hogan <james.hogan@imgtec.com>
To: Shashidhar Hiremath <shashidharh@vayavyalabs.com>
Cc: Will Newton <will.newton@imgtec.com>,
Jaehoon Chung <jh80.chung@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Shawn Guo <shawn.guo@linaro.org>, Chris Ball <cjb@laptop.org>,
Wolfram Sang <w.sang@pengutronix.de>,
linux-mmc@vger.kernel.org
Subject: Re: [PATCH 1/1] mmc: Support of PCI mode for the dw_mmc driver This Patch adds the support for the scenario where the Synopsys Designware IP is present on the PCI bus.The patch adds the minimal modifications necessary for the driver to work on PCI platform. The Driver has also been tested for on the PCI platform with single Card Slot.
Date: Wed, 28 Sep 2011 16:22:07 +0100 [thread overview]
Message-ID: <4E833B9F.8090105@imgtec.com> (raw)
In-Reply-To: <1317222139-5552-1-git-send-email-shashidharh@vayavyalabs.com>
Hi
On 09/28/2011 04:02 PM, Shashidhar Hiremath wrote:
> Signed-off-by: Shashidhar Hiremath <shashidharh@vayavyalabs.com>
Looks like your description ended up all in the subject.
> ---
> 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
> #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,
> + .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
> 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
> {
> struct dw_mci *host;
> struct resource *regs;
> @@ -1783,6 +1815,16 @@ static int dw_mci_probe(struct platform_device *pdev)
> int irq, ret, i, width;
> u32 fifo_size;
>
> +#ifdef CONFIG_MMC_DW_PCI
> + ret = pci_enable_device(pdev);
> + if (ret)
> + return ret;
> + if (pci_request_regions(pdev, "dw_mmc")) {
> + ret = -ENODEV;
> + goto err_freehost;
> + }
> + irq = pdev->irq;
> +#else
> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!regs)
> return -ENXIO;
> @@ -1790,19 +1832,26 @@ static int dw_mci_probe(struct platform_device *pdev)
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
> +#endif
>
> host = kzalloc(sizeof(struct dw_mci), GFP_KERNEL);
> if (!host)
> return -ENOMEM;
>
> host->pdev = pdev;
> +#ifdef CONFIG_MMC_DW_PCI
> + host->pdata = pdata = &pci_board_data;
> +#else
> host->pdata = pdata = pdev->dev.platform_data;
> +#endif
> +#ifndef CONFIG_MMC_DW_PCI
> if (!pdata || !pdata->init) {
> dev_err(&pdev->dev,
> "Platform data must supply init function\n");
> ret = -ENODEV;
> goto err_freehost;
> }
> +#endif
>
> if (!pdata->select_slot && pdata->num_slots > 1) {
> dev_err(&pdev->dev,
> @@ -1825,9 +1874,17 @@ static int dw_mci_probe(struct platform_device *pdev)
> INIT_LIST_HEAD(&host->queue);
>
> ret = -ENOMEM;
> +#ifdef CONFIG_MMC_DW_PCI
> + host->regs = pci_iomap(pdev, PCI_BAR_NO, COMPLETE_BAR);
> + if (!host->regs) {
> + ret = -EIO;
> + goto err_free_res;
> + }
> +#else
> host->regs = ioremap(regs->start, resource_size(regs));
> if (!host->regs)
> goto err_freehost;
> +#endif
>
> host->dma_ops = pdata->dma_ops;
> dw_mci_init_dma(host);
> @@ -1903,11 +1960,19 @@ static int dw_mci_probe(struct platform_device *pdev)
> goto err_dmaunmap;
> INIT_WORK(&host->card_work, dw_mci_work_routine_card);
>
> +#ifdef CONFIG_MMC_DW_PCI
> + ret = request_irq(irq, dw_mci_interrupt, IRQF_SHARED, "dw-mci", host);
> +#else
> ret = request_irq(irq, dw_mci_interrupt, 0, "dw-mci", host);
> +#endif
> if (ret)
> goto err_workqueue;
>
> +#ifdef CONFIG_MMC_DW_PCI
> + pci_set_drvdata(pdev, host);
> +#else
> platform_set_drvdata(pdev, host);
> +#endif
>
> if (host->pdata->num_slots)
> host->num_slots = host->pdata->num_slots;
> @@ -1966,21 +2031,38 @@ err_dmaunmap:
> regulator_put(host->vmmc);
> }
>
> +#ifdef CONFIG_MMC_DW_PCI
> +err_free_res:
> + pci_release_regions(pdev);
> +#endif
>
> err_freehost:
> kfree(host);
> return ret;
> }
>
> +#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
> {
> +
> +#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
>
> 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
I can't help thinking a lot of this could be abstracted better without
having to have quite so many #ifdefs. e.g. have generic
probe/remove/suspend/resume functions which just work with a struct
dw_mci, and handle any differences based on information in that struct,
or let the caller handle the differences and pass the relevant
information in. The PCI/platform specific code would then be nicely
grouped together in fewer ifdefs. Have you looked at how other drivers
handle this situation?
> }
>
> 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 */
In what way are they dummy?
> +#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
> +
> #endif /* _DW_MMC_H_ */
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 6b46819..87af5a6 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -147,7 +147,11 @@ struct dw_mci {
> u32 current_speed;
> u32 num_slots;
> u32 fifoth_val;
> +#ifdef CONFIG_MMC_DW_PCI
> + struct pci_dev *pdev;
> +#else
> struct platform_device *pdev;
> +#endif
> struct dw_mci_board *pdata;
> struct dw_mci_slot *slot[MAX_MCI_SLOTS];
>
Cheers
James
prev parent reply other threads:[~2011-09-28 15:22 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-28 15:02 [PATCH 1/1] mmc: Support of PCI mode for the dw_mmc driver This Patch adds the support for the scenario where the Synopsys Designware IP is present on the PCI bus.The patch adds the minimal modifications necessary for the driver to work on PCI platform. The Driver has also been tested for on the PCI platform with single Card Slot Shashidhar Hiremath
2011-09-28 15:22 ` James Hogan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E833B9F.8090105@imgtec.com \
--to=james.hogan@imgtec.com \
--cc=cjb@laptop.org \
--cc=jh80.chung@samsung.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-mmc@vger.kernel.org \
--cc=shashidharh@vayavyalabs.com \
--cc=shawn.guo@linaro.org \
--cc=w.sang@pengutronix.de \
--cc=will.newton@imgtec.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).