lkml.org 
[lkml]   [2010]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] mmc: make sdhci work with ricoh mmc controller
    From
    Date
    On Thu, 2010-06-03 at 09:11 -0700, Philip Langdale wrote: 
    > On Thu, 3 Jun 2010 04:16:27 +0300
    > Maxim Levitsky <maximlevitsky@gmail.com> wrote:
    >
    > > The current way of disabling it is not well tested by vendor
    > > and has all kinds of bugs that show up on resume from ram/disk.
    > >
    > > Old way of disabling is still supported by
    > > continuing to use CONFIG_MMC_RICOH_MMC.
    > >
    > > Based on
    > > 'http://list.drzeus.cx/pipermail/sdhci-devel/2007-December/002085.html'
    >
    > As long as this doesn't limit the performance of MMC cards, I can't
    > complain.
    >
    > BTW, the new PCIe native controllers from Ricoh don't require the
    > MMC controller to be disabled - the SD controller sees the cards by
    > default; I guess some bit has to be twiddled by the MMC driver.
    Can I distinguish between this and newer controllers.
    Could you help me with that?


    >
    > > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
    > > CC: adq_dvb@lidskialf.net
    > > CC: linux-mmc@vger.kernel.org <linux-mmc@vger.kernel.org>
    > > ---
    > > drivers/mmc/host/sdhci-pci.c | 34
    > > ++++++++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c |
    > > 5 ++++- drivers/mmc/host/sdhci.h | 2 ++
    > > 3 files changed, 40 insertions(+), 1 deletions(-)
    > >
    > > diff --git a/drivers/mmc/host/sdhci-pci.c
    > > b/drivers/mmc/host/sdhci-pci.c index 65483fd..3843780 100644
    > > --- a/drivers/mmc/host/sdhci-pci.c
    > > +++ b/drivers/mmc/host/sdhci-pci.c
    > > @@ -17,6 +17,7 @@
    > > #include <linux/pci.h>
    > > #include <linux/dma-mapping.h>
    > > #include <linux/slab.h>
    > > +#include <linux/device.h>
    > >
    > > #include <linux/mmc/host.h>
    > >
    > > @@ -85,6 +86,26 @@ static int ricoh_probe(struct sdhci_pci_chip *chip)
    > > chip->pdev->subsystem_vendor == PCI_VENDOR_ID_SONY)
    > > chip->quirks |= SDHCI_QUIRK_NO_CARD_NO_RESET;
    > >
    > > + /* To disable hardware races against MMC part */
    > > + device_disable_async_suspend(&chip->pci_dev->dev);
    > > + return 0;
    > > +}
    >
    > It would be nice if this could be more selective so it only happens
    > if it's really needed.
    Same as above

    >
    > > +
    > > +static int ricoh_mmc_probe_slot(struct sdhci_pci_slot *slot)
    > > +{
    > > + slot->host->caps =
    > > + ((0x21 << SDHCI_TIMEOUT_CLK_SHIFT)
    > > + & SDHCI_TIMEOUT_CLK_MASK) |
    > > +
    > > + ((0x21 << SDHCI_CLOCK_BASE_SHIFT)
    > > + & SDHCI_CLOCK_BASE_MASK) |
    > > +
    > > + SDHCI_TIMEOUT_CLK_UNIT |
    > > + SDHCI_CAN_VDD_330 |
    > > + SDHCI_CAN_DO_SDMA ;
    >
    > Have you been able to establish if 4bit and high-speed operations
    > work correctly through the MMC controller? I note that you didn't
    > set SDHCI_CAN_DO_HISPD.
    Didn't test that yet, will do.
    I hope my MMCPlus card can do high-speed.

    >
    > > +
    > > + /* To disable hardware races against SDHC part */
    > > + device_disable_async_suspend(&slot->chip->pci_dev->dev);
    > > return 0;
    > > }
    > >
    > > @@ -95,6 +116,11 @@ static const struct sdhci_pci_fixes sdhci_ricoh =
    > > { SDHCI_QUIRK_CLOCK_BEFORE_RESET,
    > > };
    > >
    > > +static const struct sdhci_pci_fixes sdhci_ricoh_mmc = {
    > > + .probe_slot = ricoh_mmc_probe_slot,
    > > + .quirks = SDHCI_QUIRK_NO_CARD_NO_RESET,
    > > +};
    > > +
    > > static const struct sdhci_pci_fixes sdhci_ene_712 = {
    > > .quirks = SDHCI_QUIRK_SINGLE_POWER_WRITE |
    > > SDHCI_QUIRK_BROKEN_DMA,
    > > @@ -374,6 +400,14 @@ static const struct pci_device_id pci_ids[]
    > > __devinitdata = { },
    > >
    > > {
    > > + .vendor = PCI_VENDOR_ID_RICOH,
    > > + .device = 0x843,
    > > + .subvendor = PCI_ANY_ID,
    > > + .subdevice = PCI_ANY_ID,
    > > + .driver_data = (kernel_ulong_t)&sdhci_ricoh_mmc,
    > > + },
    > > +
    > > + {
    > > .vendor = PCI_VENDOR_ID_ENE,
    > > .device = PCI_DEVICE_ID_ENE_CB712_SD,
    > > .subvendor = PCI_ANY_ID,
    > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
    > > index c6d1bd8..dbd9367 100644
    > > --- a/drivers/mmc/host/sdhci.c
    > > +++ b/drivers/mmc/host/sdhci.c
    > > @@ -1687,7 +1687,10 @@ int sdhci_add_host(struct sdhci_host *host)
    > > host->version);
    > > }
    > >
    > > - caps = sdhci_readl(host, SDHCI_CAPABILITIES);
    > > + if (host->caps)
    > > + caps = host->caps;
    > > + else
    > > + caps = sdhci_readl(host, SDHCI_CAPABILITIES);
    >
    > I'd prefer keying this off an explicit quirk.
    Could you explain?

    Do you mean I should add SDHCI_QUIRK_MISSING_CAPS ?
    This is fine.

    If you mean that I should create a SDHCI_QUIRK_RICOH_MMC_CAPS,
    and hardcode caps in sdhci, I kind of disagree, too ugly :-)


    Btw, suspend/resume races seem to disappear. Maybe I didn't install the
    kernel (will test more)
    > >
    > > if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
    > > host->flags |= SDHCI_USE_SDMA;
    > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
    > > index c846813..b41581a 100644
    > > --- a/drivers/mmc/host/sdhci.h
    > > +++ b/drivers/mmc/host/sdhci.h
    > > @@ -292,6 +292,8 @@ struct sdhci_host {
    > >
    > > struct timer_list timer; /* Timer for
    > > timeouts */
    > > + unsigned int caps; /*
    > > Alternative capabilities */ +
    > > unsigned long private[0]
    > > ____cacheline_aligned; };
    > >
    >
    > --phil


    Best regards,
    Maxim Levitsky



    \
     
     \ /
      Last update: 2010-06-03 18:35    [W:0.031 / U:121.868 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site