Hi ,
Actually these are not really dummy Id's.The vendor Id ad Device Id
were given by Synopsys for the PCI platform.
On Mon, Oct 3, 2011 at 2:01 PM, James Hogan <james.ho...@imgtec.com> wrote:
> Hi
>
> On 10/03/2011 07:50 AM, Shashidhar Hiremath wrote:
>> Hi James,
>> Can I add separate files for PCI/Platform and from there call
>> common probe/remove api's similar to SDHCI driver in kernel ?
>
> yes, that sounds like a good idea.
>
> In what way are the PCI device and vendor ids dummy?
>
> Cheers
> James
>
>>
>> On Fri, Sep 30, 2011 at 2:12 PM, James Hogan <james.ho...@imgtec.com> wrote:
>>> Hi
>>>
>>> On 09/29/2011 06:40 PM, Shashidhar Hiremath wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Shashidhar Hiremath <shashidh...@vayavyalabs.com>
>>>> ---
>>>> v2:
>>>> *As per Suggestions by Will Newton and James Hogan
>>>> -Reduced the number of ifdefs
>>>>
>>>> drivers/mmc/host/Kconfig | 11 ++
>>>> drivers/mmc/host/dw_mmc.c | 352
>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>> drivers/mmc/host/dw_mmc.h | 13 ++
>>>> include/linux/mmc/dw_mmc.h | 4 +
>>>> 4 files changed, 380 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..0bd9e16 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -21,6 +21,7 @@
>>>> #include <linux/interrupt.h>
>>>> #include <linux/ioport.h>
>>>> #include <linux/module.h>
>>>> +#include <linux/pci.h>
>>>> #include <linux/platform_device.h>
>>>> #include <linux/scatterlist.h>
>>>> #include <linux/seq_file.h>
>>>> @@ -101,6 +102,7 @@ struct dw_mci_slot {
>>>> int last_detect_state;
>>>> };
>>>>
>>>> +
>>>> static struct workqueue_struct *dw_mci_card_workqueue;
>>>>
>>>> #if defined(CONFIG_DEBUG_FS)
>>>> @@ -682,6 +684,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc,
>>>> struct mmc_ios *ios)
>>>> {
>>>> struct dw_mci_slot *slot = mmc_priv(mmc);
>>>> u32 regs;
>>>> + u32 card_detect;
>>>>
>>>> /* set default 1 bit mode */
>>>> slot->ctype = SDMMC_CTYPE_1BIT;
>>>> @@ -716,6 +719,13 @@ 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);
>>>> + /* Enable Power to the card that has been detected */
>>>> + card_detect = mci_readl(slot->host, CDETECT);
>>>> + /* Enabling power for card 0 when PCI is the interface */
>>>> + mci_writel(slot->host, PWREN, ((~card_detect) & 0x1));
>>>> + break;
>>>> + case MMC_POWER_OFF:
>>>> + mci_writel(slot->host, PWREN, 0);
>>>> break;
>>>> default:
>>>> break;
>>>> @@ -1775,6 +1785,345 @@ static bool mci_wait_reset(struct device *dev,
>>>> struct dw_mci *host)
>>>> return false;
>>>> }
>>>>
>>>> +#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,
>>>> +};
>>>> +
>>>> +static int __devinit dw_mci_probe(struct pci_dev *pdev,
>>>> + const struct pci_device_id *entries)
>>>> +{
>>>> + struct dw_mci *host;
>>>> + struct resource *regs;
>>>> + struct dw_mci_board *pdata;
>>>> + int irq, ret, i, width;
>>>> + u32 fifo_size;
>>>> +
>>>> + ret = pci_enable_device(pdev);
>>>> + if (ret)
>>>> + return ret;
>>>> + if (pci_request_regions(pdev, "dw_mmc")) {
>>>> + ret = -ENODEV;
>>>> + goto err_freehost;
>>>> + }
>>>> + irq = pdev->irq;
>>>> +
>>>> + host = kzalloc(sizeof(struct dw_mci), GFP_KERNEL);
>>>> + if (!host)
>>>> + return -ENOMEM;
>>>> +
>>>> + host->pdev = pdev;
>>>> + host->pdata = pdata = &pci_board_data;
>>>> +
>>>> + if (!pdata->select_slot && pdata->num_slots > 1) {
>>>> + dev_err(&pdev->dev,
>>>> + "Platform data must supply select_slot function\n");
>>>> + ret = -ENODEV;
>>>> + goto err_freehost;
>>>> + }
>>>> +
>>>> + if (!pdata->bus_hz) {
>>>> + dev_err(&pdev->dev,
>>>> + "Platform data must supply bus speed\n");
>>>> + ret = -ENODEV;
>>>> + goto err_freehost;
>>>> + }
>>>> +
>>>> + host->bus_hz = pdata->bus_hz;
>>>> + host->quirks = pdata->quirks;
>>>> +
>>>> + spin_lock_init(&host->lock);
>>>> + INIT_LIST_HEAD(&host->queue);
>>>> +
>>>> + ret = -ENOMEM;
>>>> + host->regs = pci_iomap(pdev, PCI_BAR_NO, COMPLETE_BAR);
>>>> + if (!host->regs) {
>>>> + ret = -EIO;
>>>> + goto err_free_res;
>>>> + }
>>>> +
>>>> + host->dma_ops = pdata->dma_ops;
>>>> + dw_mci_init_dma(host);
>>>> +
>>>> + /*
>>>> + * Get the host data width - this assumes that HCON has been set with
>>>> + * the correct values.
>>>> + */
>>>> + i = (mci_readl(host, HCON) >> 7) & 0x7;
>>>> + if (!i) {
>>>> + host->push_data = dw_mci_push_data16;
>>>> + host->pull_data = dw_mci_pull_data16;
>>>> + width = 16;
>>>> + host->data_shift = 1;
>>>> + } else if (i == 2) {
>>>> + host->push_data = dw_mci_push_data64;
>>>> + host->pull_data = dw_mci_pull_data64;
>>>> + width = 64;
>>>> + host->data_shift = 3;
>>>> + } else {
>>>> + /* Check for a reserved value, and warn if it is */
>>>> + WARN((i != 1),
>>>> + "HCON reports a reserved host data width!\n"
>>>> + "Defaulting to 32-bit access.\n");
>>>> + host->push_data = dw_mci_push_data32;
>>>> + host->pull_data = dw_mci_pull_data32;
>>>> + width = 32;
>>>> + host->data_shift = 2;
>>>> + }
>>>> +
>>>> + /* Reset all blocks */
>>>> + if (!mci_wait_reset(&pdev->dev, host)) {
>>>> + ret = -ENODEV;
>>>> + goto err_dmaunmap;
>>>> + }
>>>> +
>>>> + /* Clear the interrupts for the host controller */
>>>> + mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> + mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>>>> +
>>>> + /* Put in max timeout */
>>>> + mci_writel(host, TMOUT, 0xFFFFFFFF);
>>>> +
>>>> + /*
>>>> + * FIFO threshold settings RxMark = fifo_size / 2 - 1,
>>>> + * Tx Mark = fifo_size / 2 DMA Size = 8
>>>> + */
>>>> + if (!host->pdata->fifo_depth) {
>>>> + /*
>>>> + * Power-on value of RX_WMark is FIFO_DEPTH-1, but this may
>>>> + * have been overwritten by the bootloader, just like we're
>>>> + * about to do, so if you know the value for your hardware,
>>>> you
>>>> + * should put it in the platform data.
>>>> + */
>>>> + fifo_size = mci_readl(host, FIFOTH);
>>>> + fifo_size = 1 + ((fifo_size >> 16) & 0x7ff);
>>>> + } else {
>>>> + fifo_size = host->pdata->fifo_depth;
>>>> + }
>>>> + host->fifo_depth = fifo_size;
>>>> + host->fifoth_val = ((0x2 << 28) | ((fifo_size/2 - 1) << 16) |
>>>> + ((fifo_size/2) << 0));
>>>> + mci_writel(host, FIFOTH, host->fifoth_val);
>>>> +
>>>> + /* disable clock to CIU */
>>>> + mci_writel(host, CLKENA, 0);
>>>> + mci_writel(host, CLKSRC, 0);
>>>> +
>>>> + tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned
>>>> long)host);
>>>> + dw_mci_card_workqueue = alloc_workqueue("dw-mci-card",
>>>> + WQ_MEM_RECLAIM | WQ_NON_REENTRANT, 1);
>>>> + if (!dw_mci_card_workqueue)
>>>> + goto err_dmaunmap;
>>>> + INIT_WORK(&host->card_work, dw_mci_work_routine_card);
>>>> +
>>>> + ret = request_irq(irq, dw_mci_interrupt, IRQF_SHARED, "dw-mci",
>>>> host);
>>>> + if (ret)
>>>> + goto err_workqueue;
>>>> +
>>>> + pci_set_drvdata(pdev, host);
>>>> +
>>>> + if (host->pdata->num_slots)
>>>> + host->num_slots = host->pdata->num_slots;
>>>> + else
>>>> + host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
>>>> +
>>>> + /* We need at least one slot to succeed */
>>>> + for (i = 0; i < host->num_slots; i++) {
>>>> + ret = dw_mci_init_slot(host, i);
>>>> + if (ret) {
>>>> + ret = -ENODEV;
>>>> + goto err_init_slot;
>>>> + }
>>>> + }
>>>> +
>>>> + /*
>>>> + * Enable interrupts for command done, data over, data empty, card
>>>> det,
>>>> + * receive ready and error such as transmit, receive timeout, crc
>>>> error
>>>> + */
>>>> + mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> + mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>>> + SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>>> + DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>>>> + mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE); /* Enable mci
>>>> interrupt */
>>>> +
>>>> + dev_info(&pdev->dev, "DW MMC controller at irq %d, "
>>>> + "%d bit host data width, "
>>>> + "%u deep fifo\n",
>>>> + irq, width, fifo_size);
>>>> + if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
>>>> + dev_info(&pdev->dev, "Internal DMAC interrupt fix
>>>> enabled.\n");
>>>> +
>>>> + return 0;
>>>> +
>>>> +err_init_slot:
>>>> + /* De-init any initialized slots */
>>>> + while (i > 0) {
>>>> + if (host->slot[i])
>>>> + dw_mci_cleanup_slot(host->slot[i], i);
>>>> + i--;
>>>> + }
>>>> + free_irq(irq, host);
>>>> +
>>>> +err_workqueue:
>>>> + destroy_workqueue(dw_mci_card_workqueue);
>>>> +
>>>> +err_dmaunmap:
>>>> + if (host->use_dma && host->dma_ops->exit)
>>>> + host->dma_ops->exit(host);
>>>> + dma_free_coherent(&host->pdev->dev, PAGE_SIZE,
>>>> + host->sg_cpu, host->sg_dma);
>>>> + iounmap(host->regs);
>>>> +
>>>> + if (host->vmmc) {
>>>> + regulator_disable(host->vmmc);
>>>> + regulator_put(host->vmmc);
>>>> + }
>>>> +
>>>> +err_free_res:
>>>> + pci_release_regions(pdev);
>>>> +
>>>> +err_freehost:
>>>> + kfree(host);
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static void __devexit dw_mci_remove(struct pci_dev *pdev)
>>>> +{
>>>> +
>>>> + struct dw_mci *host = pci_get_drvdata(pdev);
>>>> + int i;
>>>> +
>>>> + mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> + mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>>>> +
>>>> + pci_set_drvdata(pdev, NULL);
>>>> +
>>>> + for (i = 0; i < host->num_slots; i++) {
>>>> + dev_dbg(&pdev->dev, "remove slot %d\n", i);
>>>> + if (host->slot[i])
>>>> + dw_mci_cleanup_slot(host->slot[i], i);
>>>> + }
>>>> +
>>>> + /* disable clock to CIU */
>>>> + mci_writel(host, CLKENA, 0);
>>>> + mci_writel(host, CLKSRC, 0);
>>>> +
>>>> + free_irq(pdev->irq, host);
>>>> + destroy_workqueue(dw_mci_card_workqueue);
>>>> + dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma);
>>>> +
>>>> + if (host->use_dma && host->dma_ops->exit)
>>>> + host->dma_ops->exit(host);
>>>> +
>>>> + if (host->vmmc) {
>>>> + regulator_disable(host->vmmc);
>>>> + regulator_put(host->vmmc);
>>>> + }
>>>> +
>>>> + iounmap(host->regs);
>>>> +
>>>> + pci_release_regions(pdev);
>>>> + kfree(host);
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_PM
>>>> +/*
>>>> + * TODO: we should probably disable the clock to the card in the suspend
>>>> path.
>>>> + */
>>>> +static int dw_mci_suspend(struct pci_dev *pdev, pm_message_t mesg)
>>>> +{
>>>> + int i, ret;
>>>> + struct dw_mci *host = pci_get_drvdata(pdev);
>>>> +
>>>> + for (i = 0; i < host->num_slots; i++) {
>>>> + struct dw_mci_slot *slot = host->slot[i];
>>>> + if (!slot)
>>>> + continue;
>>>> + ret = mmc_suspend_host(slot->mmc);
>>>> + if (ret < 0) {
>>>> + while (--i >= 0) {
>>>> + slot = host->slot[i];
>>>> + if (slot)
>>>> + mmc_resume_host(host->slot[i]->mmc);
>>>> + }
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +
>>>> + if (host->vmmc)
>>>> + regulator_disable(host->vmmc);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int dw_mci_resume(struct pci_dev *pdev)
>>>> +{
>>>> + int i, ret;
>>>> + struct dw_mci *host = pci_get_drvdata(pdev);
>>>> +
>>>> + if (host->vmmc)
>>>> + regulator_enable(host->vmmc);
>>>> +
>>>> + if (host->dma_ops->init)
>>>> + host->dma_ops->init(host);
>>>> +
>>>> + if (!mci_wait_reset(&pdev->dev, host)) {
>>>> + ret = -ENODEV;
>>>> + return ret;
>>>> + }
>>>> +
>>>> + /* Restore the old value at FIFOTH register */
>>>> + mci_writel(host, FIFOTH, host->fifoth_val);
>>>> +
>>>> + mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> + mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>>> + SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>>> + DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>>>> + mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
>>>> +
>>>> + for (i = 0; i < host->num_slots; i++) {
>>>> + struct dw_mci_slot *slot = host->slot[i];
>>>> + if (!slot)
>>>> + continue;
>>>> + ret = mmc_resume_host(host->slot[i]->mmc);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +#else
>>>> +#define dw_mci_suspend NULL
>>>> +#define dw_mci_resume NULL
>>>> +#endif /* CONFIG_PM */
>>>> +
>>>> +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,
>>>> +};
>>>> +
>>>> +static int __init dw_mci_init(void)
>>>> +{
>>>> + return pci_register_driver(&dw_mci_driver);
>>>> +}
>>>> +
>>>> +static void __exit dw_mci_exit(void)
>>>> +{
>>>> + pci_unregister_driver(&dw_mci_driver);
>>>> +}
>>>> +#else
>>>> +
>>>
>>> Duplication isn't the same as abstraction, and it's much worse to
>>> maintain than lots of #ifdefs. What we meant was to abstract the common
>>> code into separate static functions which are used by both pci/platform
>>> probe/remove/suspend/resume functions (as Will has already described).
>>>
>>> These functions should also have pci/platform in their names to
>>> distinguish them more easily. Also I see no reason why pci and platform
>>> registration should be mutually exclusive, i.e. the platform driver can
>>> still be registered even if PCI is enabled.
>>>
>>>> static int dw_mci_probe(struct platform_device *pdev)
>>>> {
>>>> struct dw_mci *host;
>>>> @@ -1974,6 +2323,7 @@ err_freehost:
>>>>
>>>> static int __exit dw_mci_remove(struct platform_device *pdev)
>>>> {
>>>> +
>>>> struct dw_mci *host = platform_get_drvdata(pdev);
>>>> int i;
>>>>
>>>> @@ -2100,6 +2450,8 @@ static void __exit dw_mci_exit(void)
>>>> platform_driver_unregister(&dw_mci_driver);
>>>> }
>>>>
>>>> +#endif/* CONFIG_MMC_DW_PCI */
>>>> +
>>>> module_init(dw_mci_init);
>>>> module_exit(dw_mci_exit);
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>>> index 027d377..54bc604 100644
>>>> --- a/drivers/mmc/host/dw_mmc.h
>>>> +++ b/drivers/mmc/host/dw_mmc.h
>>>> @@ -164,4 +164,17 @@
>>>> (*(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
>>>
>>> Again, in what way are these values dummy? if somebody were to want to
>>> use this, would they have to change them for their specific hardware? Is
>>> that a common thing to have to do for other drivers like this?
>>>
>>>> +/* 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 /* CONFIG_MMC_DW_PCI */
>>>> +
>>>> #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
>>>
>>> The only time this is used is to find &host->pdev->dev for logging
>>> calls. It'd be better to replace these with a single struct device *,
>>> then it'll work with both platform and pci enabled.
>>>
>>>> struct dw_mci_board *pdata;
>>>> struct dw_mci_slot *slot[MAX_MCI_SLOTS];
>>>>
>>>
>>>
>>
>>
>>
>
>
--
regards,
Shashidhar Hiremath
--
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
