On Mon, 07 Jun 2010 03:37:32 +0300
Maxim Levitsky <[email protected]> wrote:

> On Sun, 2010-06-06 at 19:11 -0400, Philip Langdale wrote: 
> > On Mon,  7 Jun 2010 00:28:50 +0300, Maxim Levitsky
> > <[email protected]> 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'
> > > Most of the credit for this goes to Andrew de Quincey
> > > 
> > > 
> > > Signed-off-by: Maxim Levitsky <[email protected]>
> > > CC: Andrew de Quincey <[email protected]>
> > > CC: [email protected] <[email protected]>
> > 
> > ACK with one change to add a #define for the device ID.
> I am not sure about this.
> When I submitted a driver for xD part I was told that unless device is
> shared by several pieces of code, its not ok to add it to pci_ids.c.
> The device is I was told can be hardcoded in the driver.
> Nothing against changing it. 

Ok, then leave it as is:

Acked-by: Philip Langdale <[email protected]>

> > > ---
> > >  drivers/mmc/host/sdhci-pci.c |   31
> > > +++++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c     |
> > > 3 ++- drivers/mmc/host/sdhci.h     |    4 ++++
> > >  3 files changed, 37 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/mmc/host/sdhci-pci.c
> > > b/drivers/mmc/host/sdhci-pci.c index 65483fd..c4bcaeb 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>
> > >  
> > > @@ -84,7 +85,21 @@ static int ricoh_probe(struct sdhci_pci_chip
> > > *chip) if (chip->pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
> > > || chip->pdev->subsystem_vendor == PCI_VENDOR_ID_SONY)
> > >           chip->quirks |= SDHCI_QUIRK_NO_CARD_NO_RESET;
> > > + return 0;
> > > +}
> > > +
> > > +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;
> > >   return 0;
> > >  }
> > 
> > As we discussed, highspeed works. Of course, sdhci never sets the
> > MMC highspeed flag so the cap is irrelevant. We'd need another
> > quirk to indicate highspeed MMC is supported.
> > 
> > This can be done in a separate patch.
> Sure, but maybe we can enable this for SD/SDHC too?

Not sure what you mean. The proper SD interface reports the HISPD
cap already. Do you mean enabling high-speed MMC on a 'proper'
SDHCI controller. That's what Pierre didn't want to do blindly.

> > 
> > > @@ -95,6 +110,14 @@ 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_32BIT_DMA_ADDR |
> > > +                   SDHCI_QUIRK_CLOCK_BEFORE_RESET |
> > > +                   SDHCI_QUIRK_NO_CARD_NO_RESET |
> > > +                   SDHCI_QUIRK_MISSING_CAPS
> > > +};
> > > +
> > >  static const struct sdhci_pci_fixes sdhci_ene_712 = {
> > >   .quirks         = SDHCI_QUIRK_SINGLE_POWER_WRITE |
> > >                     SDHCI_QUIRK_BROKEN_DMA,
> > > @@ -374,6 +397,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,
> > > + },
> > > +
> > > + {
> > 
> > It seems we generally want to add a #define for the device ID.
> > The ENE device below has one.
> > 
> > >           .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..483b78e 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -1687,7 +1687,8 @@ int sdhci_add_host(struct sdhci_host *host)
> > >                   host->version);
> > >   }
> > >  
> > > - caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> > > + caps = (host->quirks & SDHCI_QUIRK_MISSING_CAPS) ?
> > > host->caps :
> > > +         sdhci_readl(host, SDHCI_CAPABILITIES);
> > >  
> > >   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..b1839a3 100644
> > > --- a/drivers/mmc/host/sdhci.h
> > > +++ b/drivers/mmc/host/sdhci.h
> > > @@ -240,6 +240,8 @@ struct sdhci_host {
> > >  #define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN                (1<<25)
> > >  /* Controller cannot support End Attribute in NOP ADMA
> > > descriptor */ #define
> > > SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC         (1<<26) +/*
> > > Controller is missing device caps. Use caps provided by host */
> > > +#define SDHCI_QUIRK_MISSING_CAPS                 (1<<27) 
> > >   int                     irq;            /* Device
> > > IRQ */ void __iomem *             ioaddr;         /*
> > > Mapped address */ @@ -292,6 +294,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
> 
> 

--phil
--
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

Reply via email to