Hi James,
Sorry for the minor alignment issues.Will fix them . I actually did
compile the code but as separate driver not along with the kernel. In
fact, I have also tested the PCI patch ,but not as separate files but
with the earlier #ifdef method I had sent in initial patch .
On Wed, Nov 30, 2011 at 7:36 PM, James Hogan <james.hogan@xxxxxxxxxx> wrote:
> Hi,
>
> *sigh* You haven't actually tried compiling this have you? How do you
> know whether it works or not? You really really should at least make
> sure it compiles (for simple changes) and works without breaking
> anything (for anything non-trivial like adding support for a new bus
> like PCI)!
>
> Couple of other minor things below.
>
> On 11/30/2011 12:36 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. Also added separate
>> files for PCI and PLATFORM modes of operation.
>>
>> Signed-off-by: Shashidhar Hiremath <shashidharh@xxxxxxxxxxxxxxx>
>> ---
>> 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
>> v4:
>> *As per Suggestions by James Hogan
>> -Fixed Indentation Issue.
>> -Added Proper error Handling for probe and remove sequences.
>> -Modified location of some code.
>> -Added isr_flags to dw_mmc.h and removed the ifdef PCI from driver.
>>
>> drivers/mmc/host/Kconfig | 23 ++++++
>> drivers/mmc/host/dw_mmc-pci.c | 158 +++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/host/dw_mmc-pltfm.c | 134 +++++++++++++++++++++++++++++++++
>> drivers/mmc/host/dw_mmc.c | 151 +++++++++++++------------------------
>> drivers/mmc/host/dw_mmc.h | 7 ++
>> include/linux/mmc/dw_mmc.h | 6 +-
>> 6 files changed, 377 insertions(+), 102 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..2d8fad6 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"
>
> It would be nice if this followed the same pattern as MMC_DW_PCI, i.e.
> something like "Synopsys DesignWare MCI support as platform device"
> (makes it look nicer on the menu :) ).
>
> Presuming this is the more common case, it might make sense for it to be
> default y too (the driver cannot be used by default until one or both of
> these options has been selected).
>
>> + 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)
>
> <snip>
>
>> +static int dw_mci_pltfm_probe(struct platform_device *pdev)
>> +{
>> + struct dw_mci *host;
>> + struct resource *regs;
>> + int ret;
>> +
>> + host = kzalloc(sizeof(struct dw_mci), GFP_KERNEL);
>> + if (!host)
>> + return -ENOMEM;
>> +
>> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!regs) {
>> + ret = -ENXIO;
>> + goto err_free;
>> + }
>> +
>> + host->irq = platform_get_irq(pdev, 0);
>> + if (host->irq < 0) {
>> + ret = host->irq;
>> + goto err_free;
>> + }
>> +
>> + host->dev = pdev->dev;
>> + host->irq_flags = 0;
>> + host->pdata = pdev->dev.platform_data;
>> + ret = -ENOMEM;
>> + host->regs = ioremap(regs->start, resource_size(regs));
>> + if (!host->regs)
>> + goto err_free;
>> + platform_set_drvdata(pdev, host);
>> + ret = dw_mci_probe(host);
>> + if (ret)
>> + goto err_out;
>> + else
>> + return ret;
>> +err_out:
>> + iounmap(host->regs);
>> +err_free:
>> + kfree(host);
>
> alignment needs fixing here. There's no need for the else either.
>
> Thanks
> James
>
--
regards,
Shashidhar Hiremath
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
