[PATCH] ricoh_mmc: Use suspend/resume_noirq

[PATCH] ricoh_mmc: Use suspend/resume_noirq

[Philip Langdale]

If ricoh_mmc suspends before sdhci_pci, it will pull the card
out from under the controller, which could leave the system in
a very confused state.

Using suspend/resume_noirq ensures that sdhci_pci suspends first
and resumes second.

Signed-off-by: Philip Langdale <philipl@overt.org>
---

  ricoh_mmc.c |   20 ++++++++++++++++----
  1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/ricoh_mmc.c b/drivers/mmc/host/ricoh_mmc.c
index be9e7b3..34064d2 100644
--- a/drivers/mmc/host/ricoh_mmc.c
+++ b/drivers/mmc/host/ricoh_mmc.c
@@ -196,10 +196,14 @@ static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
  	pci_set_drvdata(pdev, NULL);
  }

-static int ricoh_mmc_suspend(struct pci_dev *pdev, pm_message_t state)
+static int ricoh_mmc_suspend_noirq(struct device *dev)
  {
+	struct pci_dev *pdev = NULL;
  	struct pci_dev *fw_dev = NULL;

+	pdev = to_pci_dev(dev);
+	BUG_ON(pdev == NULL);
+
  	fw_dev = pci_get_drvdata(pdev);
  	BUG_ON(fw_dev == NULL);

@@ -210,10 +214,14 @@ static int ricoh_mmc_suspend(struct pci_dev *pdev, pm_message_t state)
  	return 0;
  }

-static int ricoh_mmc_resume(struct pci_dev *pdev)
+static int ricoh_mmc_resume_noirq(struct device *dev)
  {
+	struct pci_dev *pdev = NULL;
  	struct pci_dev *fw_dev = NULL;

+	pdev = to_pci_dev(dev);
+	BUG_ON(pdev == NULL);
+
  	fw_dev = pci_get_drvdata(pdev);
  	BUG_ON(fw_dev == NULL);

@@ -224,13 +232,17 @@ static int ricoh_mmc_resume(struct pci_dev *pdev)
  	return 0;
  }

+static struct pm_ext_ops ricoh_mmc_pm_ext_opts = {
+	.suspend_noirq =	ricoh_mmc_suspend_noirq,
+	.resume_noirq =		ricoh_mmc_resume_noirq,
+};
+
  static struct pci_driver ricoh_mmc_driver = {
  	.name = 	DRIVER_NAME,
  	.id_table =	pci_ids,
  	.probe = 	ricoh_mmc_probe,
  	.remove =	__devexit_p(ricoh_mmc_remove),
-	.suspend =	ricoh_mmc_suspend,
-	.resume =	ricoh_mmc_resume,
+	.pm =		&ricoh_mmc_pm_ext_opts,
  };

  /*****************************************************************************\

I based this change on top of my previous fix for the new chip but it will apply
successfully to the original tree. There's still merit in considering the quirk
idea but this is the simplest fix and it works (for my machine, at least :-)

--phil

Re: [PATCH] ricoh_mmc: Use suspend/resume_noirq

[Frans Pop]

On Thursday 27 November 2008, Philip Langdale wrote:
> I based this change on top of my previous fix for the new chip but it
> will apply successfully to the original tree. There's still merit in
> considering the quirk idea but this is the simplest fix and it works
> (for my machine, at least :-)

It seems to work for me too.

I also see now what's wrong when I suspend/resume with a partition
mounted: after resume the device appears as /dev/mmcblk1 instead of
the /dev/mmcblk0 it was before suspending and of course that causes
errors when you try to access the mounted partition.

When I suspend/resume without a partition mounted it remains at
/dev/mmcblk0.

I'm not 100% sure this was also the case when I tested Matthew's patch
(I can't remember checking for that), but I would guess so.


With a partition mounted I get during suspend:
pci 0000:00:02.0: PCI INT A disabled
sd 0:0:0:0: [sda] Synchronizing SCSI cache
sd 0:0:0:0: [sda] Stopping disk
ACPI handle has no context!
mmc0: card 8879 removed
MMC: killing requests for dead queue
ACPI handle has no context!
sdhci-pci 0000:02:06.2: PCI INT C disabled
ACPI handle has no context!
ACPI handle has no context!
iwlagn 0000:10:00.0: PCI INT A disabled
[...]
ricoh-mmc: Suspending.
ricoh-mmc: Controller is now re-enabled.

And during resume:
Back to C!
Extended CMOS year: 2000
ricoh-mmc: Resuming.
ricoh-mmc: Controller is now disabled.
Enabling non-boot CPUs ...
[...]
sdhci-pci 0000:02:06.2: restoring config space at offset 0xf (was 0x300, writing 0x30a)
sdhci-pci 0000:02:06.2: restoring config space at offset 0x4 (was 0x0, writing 0xe0102000)
sdhci-pci 0000:02:06.2: restoring config space at offset 0x3 (was 0x800000, writing 0x804010)
sdhci-pci 0000:02:06.2: restoring config space at offset 0x1 (was 0x2100000, writing 0x2100006)
sdhci-pci 0000:02:06.2: PCI INT C -> GSI 20 (level, low) -> IRQ 20
mmc0: new SD card at address 8879
mmcblk1: mmc0:8879 SU02G 1.89 GiB
 mmcblk1: p1 p2 < p5 >

The messages "Controller is now re-enabled/disabled" are a bit confusing
during suspend/resume. Users may well wonder why a controller should get
enabled during suspend...

Feel free to add my:
Tested-by: Frans Pop <elendil@planet.nl>

Cheers,
FJP

Re: [PATCH] ricoh_mmc: Use suspend/resume_noirq

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

On Wed, 26 Nov 2008 17:43:21 -0800
Philip Langdale <philipl@overt.org> wrote:

> If ricoh_mmc suspends before sdhci_pci, it will pull the card
> out from under the controller, which could leave the system in
> a very confused state.
> 
> Using suspend/resume_noirq ensures that sdhci_pci suspends first
> and resumes second.
> 
> Signed-off-by: Philip Langdale <philipl@overt.org>
> ---

You get some odd whitespace damage on your patches. Please see if you
can get that fixed, or resend the patch as an attachment.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]