linux-kernel - Re: [PATCH] mmc: make sdhci work with ricoh mmc controller

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100603091113.7b1d6fc0@fido5>
Date:	Thu, 3 Jun 2010 09:11:13 -0700
From:	Philip Langdale <philipl@...rt.org>
To:	Maxim Levitsky <maximlevitsky@...il.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>, adq_dvb@...skialf.net,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>
Subject: Re: [PATCH] mmc: make sdhci work with ricoh mmc controller

On Thu,  3 Jun 2010 04:16:27 +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'

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.

> Signed-off-by: Maxim Levitsky <maximlevitsky@...il.com>
> CC: adq_dvb@...skialf.net
> CC: linux-mmc@...r.kernel.org <linux-mmc@...r.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.

> +
> +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.

> +
> +	/* 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.

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