Re: [PATCH 1/1] mmc:Support of PCI mode for the dw_mmc driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux