[Intrepid, Jaunty] SRU: SD card insertion is not detected on HP EliteBook 6930p

[Andy Whitcroft]

On Mon, Mar 09, 2009 at 03:11:15PM +0100, Stefan Bader wrote:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/311932
>
> SRU justification:
>
> Impact: The MMC controller on the ricoh chipset has to be disabled in 
> order to enable the MMC cards to be detected by the generic driver. This 
> fails on newer chips since the wrong PCI function is used to perform the 
> operation.
>
> Fix: Patch from 2.6.29rc2 which adds the required restraint.
>
> Testcase: Card insertion without the patch results in no detection. With 
> that patch it is detected. (Got a test in the report with an older 
> chipset which still worked.)
>
> Stefan
> -- 
>
> When all other means of communication fail, try words!
>
>

> From 0527a60c2b6bd7ab20e82cc5e488659e20eaaacd Mon Sep 17 00:00:00 2001
> From: philipl at overt.org <philipl at overt.org>
> Date: Sun, 30 Nov 2008 20:27:50 -0500
> Subject: [PATCH] ricoh_mmc: Handle newer models of Ricoh controllers
> 
> Bug: #311932
> 
> commit 0527a60c2b6bd7ab20e82cc5e488659e20eaaacd upstream
> 
> The latest generation of laptops are shipping with a newer
> model of Ricoh chip where the firewire controller is the
> primary PCI function but a cardbus controller is also present.
> 
> The existing code assumes that if a cardbus controller is,
> present, then it must be the one to manipulate - but the real
> rule is that you manipulate PCI function 0. This patch adds an
> additional constraint that the target must be function 0.
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> Signed-off-by: Pierre Ossman <drzeus at drzeus.cx>
> Signed-off-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  drivers/mmc/host/ricoh_mmc.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/ricoh_mmc.c b/drivers/mmc/host/ricoh_mmc.c
> index a16d760..be9e7b3 100644
> --- a/drivers/mmc/host/ricoh_mmc.c
> +++ b/drivers/mmc/host/ricoh_mmc.c
> @@ -11,9 +11,10 @@
>  
>  /*
>   * This is a conceptually ridiculous driver, but it is required by the way
> - * the Ricoh multi-function R5C832 works. This chip implements firewire
> - * and four different memory card controllers. Two of those controllers are
> - * an SDHCI controller and a proprietary MMC controller. The linux SDHCI
> + * the Ricoh multi-function chips (R5CXXX) work. These chips implement
> + * the four main memory card controllers (SD, MMC, MS, xD) and one or both
> + * of cardbus or firewire. It happens that they implement SD and MMC
> + * support as separate controllers (and PCI functions). The linux SDHCI
>   * driver supports MMC cards but the chip detects MMC cards in hardware
>   * and directs them to the MMC controller - so the SDHCI driver never sees
>   * them. To get around this, we must disable the useless MMC controller.
> @@ -21,8 +22,10 @@
>   * a detection event occurs immediately, even if the MMC card is already
>   * in the reader.
>   *
> - * The relevant registers live on the firewire function, so this is unavoidably
> - * ugly. Such is life.
> + * It seems to be the case that the relevant PCI registers to deactivate the
> + * MMC controller live on PCI function 0, which might be the cardbus controller
> + * or the firewire controller, depending on the particular chip in question. As
> + * such, it makes what this driver has to do unavoidably ugly. Such is life.
>   */
>  
>  #include <linux/pci.h>
> @@ -143,6 +146,7 @@ static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
>  		pci_get_device(PCI_VENDOR_ID_RICOH,
>  			PCI_DEVICE_ID_RICOH_RL5C476, fw_dev))) {
>  		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
> +		    PCI_FUNC(fw_dev->devfn) == 0 &&
>  		    pdev->bus == fw_dev->bus) {
>  			if (ricoh_mmc_disable(fw_dev) != 0)
>  				return -ENODEV;
> @@ -160,6 +164,7 @@ static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
>  	    (fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
>  					PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
>  		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
> +		    PCI_FUNC(fw_dev->devfn) == 0 &&
>  		    pdev->bus == fw_dev->bus) {
>  			if (ricoh_mmc_disable(fw_dev) != 0)
>  				return -ENODEV;
> @@ -172,7 +177,7 @@ static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
>  
>  	if (!ctrlfound) {
>  		printk(KERN_WARNING DRIVER_NAME
> -		       ": Main firewire function not found. Cannot disable controller.\n");
> +		       ": Main Ricoh function not found. Cannot disable controller.\n");
>  		return -ENODEV;
>  	}
>  
> -- 
> 1.5.4.3
> 

That is unbelievably ugly and only really testable.  It appears to be
very specific to the RICOH device ID so if it tests well there it
appears to be safe for non-ricoh devices.

ACK i guess.

-apw