Hi, Thanks for the update. It's better, but there's still a few issues...
On Wed, Nov 30, 2011 at 12:00:37AM +0530, 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.Also added separate files for PCI and PLATFORM modes of operation Being picky, but full stop at end of sentences, and a space between sentences would be nice. Wrapping to 72 characters is nice too. :) > > Signed-off-by: Shashidhar Hiremath <[email protected]> > --- > v2: > *As per Suggestions by Will Newton and James Hogan > -Reduced the number of ifdefs > v3: > *As per Suggestions by Will Newton and James Hogan > -Added separate files for PCI and PLATFORM Modes similar to SDHCI driver > drivers/mmc/host/Kconfig | 23 ++++++ > drivers/mmc/host/Makefile | 2 + > drivers/mmc/host/dw_mmc-pci.c | 159 > +++++++++++++++++++++++++++++++++++++++ > drivers/mmc/host/dw_mmc-pltfm.c | 125 ++++++++++++++++++++++++++++++ > drivers/mmc/host/dw_mmc.c | 155 ++++++++++++++------------------------ > drivers/mmc/host/dw_mmc.h | 7 ++ > include/linux/mmc/dw_mmc.h | 4 +- > 7 files changed, 374 insertions(+), 101 deletions(-) > create mode 100644 drivers/mmc/host/dw_mmc-pci.c > create mode 100644 drivers/mmc/host/dw_mmc-pltfm.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 87d5067..6735fbe 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -534,6 +534,29 @@ config MMC_DW_IDMAC > Designware Mobile Storage IP block. This disables the external DMA > interface. > > +config MMC_DW_PCI > + tristate "Synopsys Designware MCI support on PCI bus" > + depends on MMC_DW && PCI > + help > + This selects the PCI bus for the Synopsys Designware Mobile Storage > IP. > + Select this option if the IP is present on PCI platform. > + > + If you have a controller with this interface, say Y or M here. > + > + If unsure, say N. > + > +config MMC_DW_PLTFM > + tristate "platform driver helper for Synopsys Designware Mobile Storage > IP" > + depends on MMC_DW > + help > + This selects the common helper functions support for Host Controller > + Interface based platform driver.Please select this option if the IP > + is present as a platform device. > + > + If you have a controller with this interface, say Y or M here. > + > + If unsure, say N. > + > config MMC_SH_MMCIF > tristate "SuperH Internal MMCIF support" > depends on MMC_BLOCK && (SUPERH || ARCH_SHMOBILE) > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > index b4b83f3..3aa2fa3 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -38,6 +38,8 @@ obj-$(CONFIG_MMC_CB710) += cb710-mmc.o > obj-$(CONFIG_MMC_VIA_SDMMC) += via-sdmmc.o > obj-$(CONFIG_SDH_BFIN) += bfin_sdh.o > obj-$(CONFIG_MMC_DW) += dw_mmc.o > +obj-$(CONFIG_MMC_DW_PLTFM) += dw_mmc-pltfm.o > +obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o > obj-$(CONFIG_MMC_SH_MMCIF) += sh_mmcif.o > obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o > obj-$(CONFIG_MMC_VUB300) += vub300.o > diff --git a/drivers/mmc/host/dw_mmc-pci.c b/drivers/mmc/host/dw_mmc-pci.c > new file mode 100644 > index 0000000..7d8fb5e > --- /dev/null > +++ b/drivers/mmc/host/dw_mmc-pci.c > @@ -0,0 +1,159 @@ > +/* > + * Synopsys DesignWare Multimedia Card PCI Interface driver > + * > + * Copyright (C) 2011 Vayavya Labs Pvt. Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/pci.h> > +#include <linux/mmc/host.h> > +#include <linux/mmc/mmc.h> > +#include <linux/mmc/dw_mmc.h> > +#include "dw_mmc.h" > + > +#define PCI_BAR_NO 2 > +#define COMPLETE_BAR 0 > +#define DW_MCI_VENDOR_ID 0x700 > +#define DW_MCI_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) > + > +static 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_pci_probe(struct pci_dev *pdev, > + const struct pci_device_id *entries) > +{ > + struct dw_mci *host; > + int ret; > + > + ret = pci_enable_device(pdev); > + if (ret) > + return ret; > + if (pci_request_regions(pdev, "dw_mmc_pci")) { > + ret = -ENODEV; > + goto err_freehost; This will kfree the as yet uninitialised host. Should this (and later error handlers) also undo the effects of pci_enable_device with pci_disable_device? > + } > + > + host = kzalloc(sizeof(struct dw_mci), GFP_KERNEL); > + if (!host) > + return -ENOMEM; > + > + host->irq = pdev->irq; > + host->dev = pdev->dev; > + host->pdata = &pci_board_data; > + > + ret = -ENOMEM; what's this for? > + host->regs = pci_iomap(pdev, PCI_BAR_NO, COMPLETE_BAR); > + if (!host->regs) { > + pci_release_regions(pdev); > + ret = -EIO; > + goto err_freehost; > + } > + > + > + pci_set_drvdata(pdev, host); > + ret = dw_mci_probe(host); > + if (ret) > +err_freehost: > + kfree(host); pci_iounmap? > + return ret; Since there are several things that need cleaning up on failure, it should really have the error handling labels after the return like a lot of other kernel drivers do (e.g. see ioapic_probe() in drivers/pci/ioapic.c). This makes the code a lot neater and easier to read. > +} > + > + > + > + > +static void __devexit dw_mci_pci_remove(struct pci_dev *pdev) > +{ > + no need for a newline here > + struct dw_mci *host = pci_get_drvdata(pdev); > + > + mci_writel(host, RINTSTS, 0xFFFFFFFF); > + mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */ > + > + pci_set_drvdata(pdev, NULL); > + dw_mci_remove(host); > + pci_release_regions(pdev); > + kfree(host); should this also be pci_iounmap'ing, and pci_disable_device'ing? > +} > + > +#ifdef CONFIG_PM > +static int dw_mci_pci_suspend(struct pci_dev *pdev, pm_message_t mesg) > +{ > + int ret; > + struct dw_mci *host = pci_get_drvdata(pdev); > + > + ret = dw_mci_suspend(host); > + if (ret) > + return ret; > + > + return 0; > +} > + > +/* > + * TODO: we should probably disable the clock to the card in the suspend > path. > + */ > + > + > +static int dw_mci_pci_resume(struct pci_dev *pdev) > +{ > + int ret; > + struct dw_mci *host = pci_get_drvdata(pdev); > + > + ret = dw_mci_resume(host); > + if (ret) > + return ret; > + > + return 0; > +} > + > +#else > +#define dw_mci_pci_suspend NULL > +#define dw_mci_pci_resume NULL > +#endif /* CONFIG_PM */ > + > +static DEFINE_PCI_DEVICE_TABLE(dw_mci_pci_id) = { > + { PCI_DEVICE(DW_MCI_DEVICE_ID, DW_MCI_VENDOR_ID) }, > + {} > +}; > +MODULE_DEVICE_TABLE(pci, dw_mci_pci_id); > + > +static struct pci_driver dw_mci_pci_driver = { > + .name = "dw_mmc_pci", > + .id_table = dw_mci_pci_id, > + .probe = dw_mci_pci_probe, > + .remove = dw_mci_pci_remove, > + .suspend = dw_mci_pci_suspend, > + .resume = dw_mci_pci_resume, > +}; > + > +static int __init dw_mci_init(void) > +{ > + return pci_register_driver(&dw_mci_pci_driver); > +} > + > +static void __exit dw_mci_exit(void) > +{ > + pci_unregister_driver(&dw_mci_pci_driver); > +} > + > +module_init(dw_mci_init); > +module_exit(dw_mci_exit); > + > +MODULE_DESCRIPTION("DW Multimedia Card PCI Interface driver"); > +MODULE_AUTHOR("Shashidhar Hiremath <[email protected]>"); > +MODULE_AUTHOR("VayavyaLabs Pvt. Ltd."); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > new file mode 100644 > index 0000000..0c6a95df > --- /dev/null > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -0,0 +1,125 @@ > +/* > + * Synopsys DesignWare Multimedia Card Interface driver > + * > + * Copyright (C) 2009 NXP Semiconductors > + * Copyright (C) 2009, 2010 Imagination Technologies Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/mmc/host.h> > +#include <linux/mmc/mmc.h> > +#include <linux/mmc/dw_mmc.h> > +#include "dw_mmc.h" > + > +static int dw_mci_pltfm_probe(struct platform_device *pdev) > +{ > + struct dw_mci *host; > + struct resource *regs; > + int ret; no need for the extra space before ret. > + > + host = kzalloc(sizeof(struct dw_mci), GFP_KERNEL); > + if (!host) > + return -ENOMEM; > + > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!regs) > + return -ENXIO; > + > + host->irq = platform_get_irq(pdev, 0); > + if (host->irq < 0) > + return host->irq; > + > + host->dev = pdev->dev; > + host->pdata = pdev->dev.platform_data; > + ret = -ENOMEM; > + host->regs = ioremap(regs->start, resource_size(regs)); > + if (!host->regs) > + goto err_freehost; > + platform_set_drvdata(pdev, host); > + ret = dw_mci_probe(host); > + if (ret) > +err_freehost: > + kfree(host); > + return ret; same stuff again, please fix the error handling. > +} > + > +static int __exit dw_mci_pltfm_remove(struct platform_device *pdev) > +{ > + struct dw_mci *host = platform_get_drvdata(pdev); > + > + mci_writel(host, RINTSTS, 0xFFFFFFFF); > + mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */ > + > + platform_set_drvdata(pdev, NULL); > + dw_mci_remove(host); > + kfree(host); iounmap (see below)? > + return 0; > +} > +#ifdef CONFIG_PM > +/* > + * TODO: we should probably disable the clock to the card in the suspend > path. > + */ > +static int dw_mci_pltfm_suspend(struct platform_device *pdev, pm_message_t > mesg) > +{ > + > + int ret; > + struct dw_mci *host = platform_get_drvdata(pdev); > + > + ret = dw_mci_suspend(host); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int dw_mci_pltfm_resume(struct platform_device *pdev) > +{ > + > + int ret; > + struct dw_mci *host = platform_get_drvdata(pdev); > + > + ret = dw_mci_resume(host); > + if (ret) > + return ret; > + > + return 0; > +} > +#else > +#define dw_mci_pltfm_suspend NULL > +#define dw_mci_pltfm_resume NULL > +#endif /* CONFIG_PM */ > + > +static struct platform_driver dw_mci_pltfm_driver = { > + .remove = __exit_p(dw_mci_pltfm_remove), > + .suspend = dw_mci_pltfm_suspend, > + .resume = dw_mci_pltfm_resume, > + .driver = { > + .name = "dw_mmc", > + }, > +}; > + > +static int __init dw_mci_init(void) > +{ > + return platform_driver_probe(&dw_mci_pltfm_driver, dw_mci_pltfm_probe); > +} > + > +static void __exit dw_mci_exit(void) > +{ > + platform_driver_unregister(&dw_mci_pltfm_driver); > +} > + > +module_init(dw_mci_init); > +module_exit(dw_mci_exit); > + > +MODULE_DESCRIPTION("DW Multimedia Card Interface driver"); > +MODULE_AUTHOR("NXP Semiconductor VietNam"); > +MODULE_AUTHOR("Imagination Technologies Ltd"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 3aaeb08..7d33626 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -269,7 +269,7 @@ static void dw_mci_start_command(struct dw_mci *host, > struct mmc_command *cmd, u32 cmd_flags) > { > host->cmd = cmd; > - dev_vdbg(&host->pdev->dev, > + dev_vdbg(&host->dev, > "start command: ARGR=0x%08x CMDR=0x%08x\n", > cmd->arg, cmd_flags); > > @@ -302,7 +302,7 @@ static void dw_mci_dma_cleanup(struct dw_mci *host) > struct mmc_data *data = host->data; > > if (data) > - dma_unmap_sg(&host->pdev->dev, data->sg, data->sg_len, > + dma_unmap_sg(&host->dev, data->sg, data->sg_len, > ((data->flags & MMC_DATA_WRITE) > ? DMA_TO_DEVICE : DMA_FROM_DEVICE)); > } > @@ -327,7 +327,7 @@ static void dw_mci_idmac_complete_dma(struct dw_mci *host) > { > struct mmc_data *data = host->data; > > - dev_vdbg(&host->pdev->dev, "DMA complete\n"); > + dev_vdbg(&host->dev, "DMA complete\n"); > > host->dma_ops->cleanup(host); > > @@ -463,10 +463,10 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, > struct mmc_data *data) > else > direction = DMA_TO_DEVICE; > > - sg_len = dma_map_sg(&host->pdev->dev, data->sg, data->sg_len, > + sg_len = dma_map_sg(&host->dev, data->sg, data->sg_len, > direction); > > - dev_vdbg(&host->pdev->dev, > + dev_vdbg(&host->dev, > "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n", > (unsigned long)host->sg_cpu, (unsigned long)host->sg_dma, > sg_len); > @@ -716,6 +716,7 @@ 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); > + mci_writel(slot->host, PWREN, (0x1 << (slot->id))); Was this something that the original driver should have been doing? (it might be best as a separate patch if so). Does it need to be undone somewhere? Won't it unintentionally power down all other slots by writing 0 to them? Do you "wait for regulator/switch ramp-up time before trying to initialize card" (quoting TRM) or is that handled by other code? > break; > default: > break; > @@ -804,12 +805,12 @@ static void dw_mci_request_end(struct dw_mci *host, > struct mmc_request *mrq) > slot = list_entry(host->queue.next, > struct dw_mci_slot, queue_node); > list_del(&slot->queue_node); > - dev_vdbg(&host->pdev->dev, "list not empty: %s is next\n", > + dev_vdbg(&host->dev, "list not empty: %s is next\n", > mmc_hostname(slot->mmc)); > host->state = STATE_SENDING_CMD; > dw_mci_start_request(host, slot); > } else { > - dev_vdbg(&host->pdev->dev, "list empty\n"); > + dev_vdbg(&host->dev, "list empty\n"); > host->state = STATE_IDLE; > } > > @@ -941,7 +942,7 @@ static void dw_mci_tasklet_func(unsigned long priv) > data->bytes_xfered = 0; > data->error = -ETIMEDOUT; > } else { > - dev_err(&host->pdev->dev, > + dev_err(&host->dev, > "data FIFO error " > "(status=%08x)\n", > status); > @@ -1651,7 +1652,7 @@ static int __init dw_mci_init_slot(struct dw_mci *host, > unsigned int id) > struct mmc_host *mmc; > struct dw_mci_slot *slot; > > - mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), &host->pdev->dev); > + mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), &host->dev); > if (!mmc) > return -ENOMEM; > > @@ -1757,10 +1758,10 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot > *slot, unsigned int id) > static void dw_mci_init_dma(struct dw_mci *host) > { > /* Alloc memory for sg translation */ > - host->sg_cpu = dma_alloc_coherent(&host->pdev->dev, PAGE_SIZE, > + host->sg_cpu = dma_alloc_coherent(&host->dev, PAGE_SIZE, > &host->sg_dma, GFP_KERNEL); > if (!host->sg_cpu) { > - dev_err(&host->pdev->dev, "%s: could not alloc DMA memory\n", > + dev_err(&host->dev, "%s: could not alloc DMA memory\n", > __func__); > goto no_dma; > } > @@ -1768,7 +1769,7 @@ static void dw_mci_init_dma(struct dw_mci *host) > /* Determine which DMA interface to use */ > #ifdef CONFIG_MMC_DW_IDMAC > host->dma_ops = &dw_mci_idmac_ops; > - dev_info(&host->pdev->dev, "Using internal DMA controller.\n"); > + dev_info(&host->dev, "Using internal DMA controller.\n"); > #endif > > if (!host->dma_ops) > @@ -1776,12 +1777,12 @@ static void dw_mci_init_dma(struct dw_mci *host) > > if (host->dma_ops->init) { > if (host->dma_ops->init(host)) { > - dev_err(&host->pdev->dev, "%s: Unable to initialize " > + dev_err(&host->dev, "%s: Unable to initialize " > "DMA Controller.\n", __func__); > goto no_dma; > } > } else { > - dev_err(&host->pdev->dev, "DMA initialization not found.\n"); > + dev_err(&host->dev, "DMA initialization not found.\n"); > goto no_dma; > } > > @@ -1789,7 +1790,7 @@ static void dw_mci_init_dma(struct dw_mci *host) > return; > > no_dma: > - dev_info(&host->pdev->dev, "Using PIO mode.\n"); > + dev_info(&host->dev, "Using PIO mode.\n"); > host->use_dma = 0; > return; > } > @@ -1815,61 +1816,37 @@ static bool mci_wait_reset(struct device *dev, struct > dw_mci *host) > return false; > } > > -static int dw_mci_probe(struct platform_device *pdev) > +int dw_mci_probe(struct dw_mci *host) > { > - struct dw_mci *host; > - struct resource *regs; > - struct dw_mci_board *pdata; > - int irq, ret, i, width; > + int width, i , ret; no need for a space after the i. > u32 fifo_size; > > - regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!regs) > - return -ENXIO; > - > - irq = platform_get_irq(pdev, 0); > - if (irq < 0) > - return irq; > - > - host = kzalloc(sizeof(struct dw_mci), GFP_KERNEL); > - if (!host) > - return -ENOMEM; > - > - host->pdev = pdev; > - host->pdata = pdata = pdev->dev.platform_data; > - if (!pdata || !pdata->init) { > - dev_err(&pdev->dev, > + if (!host->pdata || !host->pdata->init) { > + dev_err(&host->dev, > "Platform data must supply init function\n"); > - ret = -ENODEV; > - goto err_freehost; > + return -ENODEV; > } > > - if (!pdata->select_slot && pdata->num_slots > 1) { > - dev_err(&pdev->dev, > + if (!host->pdata->select_slot && host->pdata->num_slots > 1) { > + dev_err(&host->dev, > "Platform data must supply select_slot function\n"); > - ret = -ENODEV; > - goto err_freehost; > + return -ENODEV; > } > > - if (!pdata->bus_hz) { > - dev_err(&pdev->dev, > + if (!host->pdata->bus_hz) { > + dev_err(&host->dev, > "Platform data must supply bus speed\n"); > - ret = -ENODEV; > - goto err_freehost; > + return -ENODEV; > } > > - host->bus_hz = pdata->bus_hz; > - host->quirks = pdata->quirks; > + host->bus_hz = host->pdata->bus_hz; > + host->quirks = host->pdata->quirks; > > spin_lock_init(&host->lock); > INIT_LIST_HEAD(&host->queue); > > - ret = -ENOMEM; > - host->regs = ioremap(regs->start, resource_size(regs)); > - if (!host->regs) > - goto err_freehost; > > - host->dma_ops = pdata->dma_ops; > + host->dma_ops = host->pdata->dma_ops; > dw_mci_init_dma(host); > > /* > @@ -1899,7 +1876,7 @@ static int dw_mci_probe(struct platform_device *pdev) > } > > /* Reset all blocks */ > - if (!mci_wait_reset(&pdev->dev, host)) { > + if (!mci_wait_reset(&host->dev, host)) { > ret = -ENODEV; > goto err_dmaunmap; > } > @@ -1942,13 +1919,13 @@ static int dw_mci_probe(struct platform_device *pdev) > 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, 0, "dw-mci", host); > +#ifdef CONFIG_MMC_DW_PCI > + ret = request_irq(host->irq, dw_mci_interrupt, IRQF_SHARED, "dw-mci", > host); > +#else > + ret = request_irq(host->irq, dw_mci_interrupt, 0, "dw-mci", host); > +#endif is this necessary? perhaps it could pass in the irq flags from the caller somehow. > if (ret) > goto err_workqueue; > - > - platform_set_drvdata(pdev, host); > - > if (host->pdata->num_slots) > host->num_slots = host->pdata->num_slots; > else > @@ -1985,12 +1962,12 @@ static int dw_mci_probe(struct platform_device *pdev) > 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, " > + dev_info(&host->dev, "DW MMC controller at irq %d, " > "%d bit host data width, " > "%u deep fifo\n", > - irq, width, fifo_size); > + host->irq, width, fifo_size); > if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO) > - dev_info(&pdev->dev, "Internal DMAC interrupt fix enabled.\n"); > + dev_info(&host->dev, "Internal DMAC interrupt fix enabled.\n"); > > return 0; > > @@ -2001,7 +1978,7 @@ err_init_slot: > dw_mci_cleanup_slot(host->slot[i], i); > i--; > } > - free_irq(irq, host); > + free_irq(host->irq, host); > > err_workqueue: > destroy_workqueue(dw_mci_card_workqueue); > @@ -2009,7 +1986,7 @@ err_workqueue: > err_dmaunmap: > if (host->use_dma && host->dma_ops->exit) > host->dma_ops->exit(host); > - dma_free_coherent(&host->pdev->dev, PAGE_SIZE, > + dma_free_coherent(&host->dev, PAGE_SIZE, > host->sg_cpu, host->sg_dma); > iounmap(host->regs); this iounmap can be moved out to caller now that regs it initialised by caller. > > @@ -2017,25 +1994,16 @@ err_dmaunmap: > regulator_disable(host->vmmc); > regulator_put(host->vmmc); > } > - > - > -err_freehost: > - kfree(host); > return ret; > } > +EXPORT_SYMBOL(dw_mci_probe); > > -static int __exit dw_mci_remove(struct platform_device *pdev) > +void dw_mci_remove(struct dw_mci *host) > { > - struct dw_mci *host = platform_get_drvdata(pdev); > int i; > > - mci_writel(host, RINTSTS, 0xFFFFFFFF); > - mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */ This seems pretty independent of PCI/platform, and is done by both callers anyway. Perhaps it should stay where it is. > - > - platform_set_drvdata(pdev, NULL); > - > for (i = 0; i < host->num_slots; i++) { > - dev_dbg(&pdev->dev, "remove slot %d\n", i); > + dev_dbg(&host->dev, "remove slot %d\n", i); > if (host->slot[i]) > dw_mci_cleanup_slot(host->slot[i], i); > } > @@ -2044,9 +2012,9 @@ static int __exit dw_mci_remove(struct platform_device > *pdev) > mci_writel(host, CLKENA, 0); > mci_writel(host, CLKSRC, 0); > > - free_irq(platform_get_irq(pdev, 0), host); > + free_irq(host->irq, host); > destroy_workqueue(dw_mci_card_workqueue); > - dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); > + dma_free_coherent(&host->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); > > if (host->use_dma && host->dma_ops->exit) > host->dma_ops->exit(host); > @@ -2057,19 +2025,18 @@ static int __exit dw_mci_remove(struct > platform_device *pdev) > } > > iounmap(host->regs); same here (move out to caller) > - > - kfree(host); > - return 0; > } > +EXPORT_SYMBOL(dw_mci_remove); > + > + > > #ifdef CONFIG_PM > /* > * TODO: we should probably disable the clock to the card in the suspend > path. > */ > -static int dw_mci_suspend(struct platform_device *pdev, pm_message_t mesg) > +int dw_mci_suspend(struct dw_mci *host) > { > int i, ret; > - struct dw_mci *host = platform_get_drvdata(pdev); > > for (i = 0; i < host->num_slots; i++) { > struct dw_mci_slot *slot = host->slot[i]; > @@ -2091,19 +2058,18 @@ static int dw_mci_suspend(struct platform_device > *pdev, pm_message_t mesg) > > return 0; > } > +EXPORT_SYMBOL(dw_mci_suspend); > > -static int dw_mci_resume(struct platform_device *pdev) > +int dw_mci_resume(struct dw_mci *host) > { > int i, ret; > - struct dw_mci *host = platform_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)) { > + if (!mci_wait_reset(&host->dev, host)) { > ret = -ENODEV; > return ret; > } > @@ -2125,31 +2091,22 @@ static int dw_mci_resume(struct platform_device *pdev) > if (ret < 0) > return ret; > } > - > return 0; > } > +EXPORT_SYMBOL(dw_mci_resume); > #else > #define dw_mci_suspend NULL > #define dw_mci_resume NULL this else bit can probably go since the #defines aren't being used any more. > #endif /* CONFIG_PM */ > > -static struct platform_driver dw_mci_driver = { > - .remove = __exit_p(dw_mci_remove), > - .suspend = dw_mci_suspend, > - .resume = dw_mci_resume, > - .driver = { > - .name = "dw_mmc", > - }, > -}; > - > static int __init dw_mci_init(void) > { > - return platform_driver_probe(&dw_mci_driver, dw_mci_probe); > + printk(KERN_INFO "Synopsys Designware Multimedia Card Interface > Driver"); > + return 0; > } > > static void __exit dw_mci_exit(void) > { > - platform_driver_unregister(&dw_mci_driver); > } > > module_init(dw_mci_init); if the init and exit functions of the main driver no longer do anything, they can probably go too. > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > index 72c071f..70078f6 100644 > --- a/drivers/mmc/host/dw_mmc.h > +++ b/drivers/mmc/host/dw_mmc.h > @@ -175,4 +175,11 @@ > (*(volatile u64 __force *)((dev)->regs + SDMMC_##reg) = (value)) > #endif > > +extern int dw_mci_probe(struct dw_mci *host); > +extern void dw_mci_remove(struct dw_mci *host); > +#ifdef CONFIG_PM > +extern int dw_mci_suspend(struct dw_mci *host); > +extern int dw_mci_resume(struct dw_mci *host); > +#endif > + > #endif /* _DW_MMC_H_ */ > diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h > index 6dc9b80..5240446 100644 > --- a/include/linux/mmc/dw_mmc.h > +++ b/include/linux/mmc/dw_mmc.h > @@ -74,7 +74,7 @@ struct mmc_data; > * @num_slots: Number of slots available. > * @verid: Denote Version ID. > * @data_offset: Set the offset of DATA register according to VERID. > - * @pdev: Platform device associated with the MMC controller. > + * @dev: device associated with the MMC controller. I'm being picky here, but you could capitalise the d of device, for consistency with the rest of the descriptions. > * @pdata: Platform data associated with the MMC controller. > * @slot: Slots sharing this MMC controller. > * @fifo_depth: depth of FIFO. > @@ -151,7 +151,7 @@ struct dw_mci { > u32 fifoth_val; > u16 verid; > u16 data_offset; > - struct platform_device *pdev; > + struct device *dev; > struct dw_mci_board *pdata; > struct dw_mci_slot *slot[MAX_MCI_SLOTS]; > > -- > 1.7.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html Thanks James -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
