| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1275871052.24091.7.camel@maxim-laptop>
Date: Mon, 07 Jun 2010 03:37:32 +0300
From: Maxim Levitsky <maximlevitsky@...il.com>
To: Philip Langdale <philipl@...rt.org>
Cc: linux-mmc@...r.kernel.org,
linux-kernel <linux-kernel@...r.kernel.org>,
Andrew de Quincey <adq_dvb@...skialf.net>
Subject: Re: [PATCH 1/2] mmc: make sdhci work with ricoh mmc controller
On Sun, 2010-06-06 at 19:11 -0400, Philip Langdale wrote:
> On Mon, 7 Jun 2010 00:28:50 +0300, Maxim Levitsky
> <maximlevitsky@...il.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'
> > Most of the credit for this goes to Andrew de Quincey
> >
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky@...il.com>
> > CC: Andrew de Quincey <adq_dvb@...skialf.net>
> > CC: linux-mmc@...r.kernel.org <linux-mmc@...r.kernel.org>
>
> 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.
>
> > ---
> > 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?
>
> > @@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists