sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

PATCH New Bing Zhao 4 participants 7 messages Jul 22, 2010 – Sep 2, 2010 1 message not in the archive

Subsystems (2)

MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver

lore

Share & tools

lore

Bing Zhao

author

2010-07-22 01:20 UTC #0 lore raw

PATCH 1/1 sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

Marvell 8686/8688 device will not respond to re-probe attemps in powered resume, if the card is in sleep mode. The added MMC_PM_SKIP_RESUME_PROBE flag is checked so that call to mmc_sdio_init_card() can be skipped.

Signed-off-by Bing Zhao <[email protected]>

---
 drivers/mmc/core/sdio.c      |   18 +++++++++++++++---
 drivers/mmc/host/sdhci-pci.c |    3 ++-
 include/linux/mmc/pm.h       |    1 +
 3 files changed, 18 insertions(+), 4 deletions(-)

drivers/mmc/core/sdio.c +15 -3

index bd2755e..dc03939 100644
@@ -606,15 +606,27 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 
 static int mmc_sdio_resume(struct mmc_host *host)
 {
-	int i, err;
+	int i, err = 0;
 
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
 	/* Basic card reinitialization. */
 	mmc_claim_host(host);
-	err = mmc_sdio_init_card(host, host->ocr, host->card,
-				 (host->pm_flags & MMC_PM_KEEP_POWER));
+
+	/*
+	 * For a powered resume, Marvell 8686/8688 device will not respond
+	 * to re-initialization attemps if the card is in sleep mode.
+	 * The MMC_PM_SKIP_RESUME_PROBE flag is added as a workaround here.
+	 * This is a temporary measure until a fully thought-through
+	 * solution is elaborated.
+	 */
+	if (!(host->pm_flags & MMC_PM_KEEP_POWER) ||
+		!(host->pm_flags & MMC_PM_SKIP_RESUME_PROBE)) {
+			err = mmc_sdio_init_card(host, host->ocr, host->card,
+					host->pm_flags & MMC_PM_KEEP_POWER);
+	}
+
 	if (!err) {
 		/* We may have switched to 1-bit mode during suspend. */
 		err = sdio_enable_4bit_bus(host->card);

drivers/mmc/host/sdhci-pci.c +2 -1

index e021431..4f793f8 100644
@@ -703,7 +703,8 @@ static struct sdhci_pci_slot * __devinit sdhci_pci_probe_slot(
 			goto unmap;
 	}
 
-	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ;
+	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ
+				| MMC_PM_SKIP_RESUME_PROBE;
 
 	ret = sdhci_add_host(host);
 	if (ret)

include/linux/mmc/pm.h +1 -0

index d37aac4..7dbb630 100644
@@ -26,5 +26,6 @@ typedef unsigned int mmc_pm_flag_t;
 
 #define MMC_PM_KEEP_POWER	(1 << 0)	/* preserve card power during suspend */
 #define MMC_PM_WAKE_SDIO_IRQ	(1 << 1)	/* wake up host system on SDIO IRQ assertion */
+#define MMC_PM_SKIP_RESUME_PROBE	(1 << 2)	/* skip the attempt to reidentify the card in powered resume */
 
 #endif

Signature · 1 line

1.5.3.6

Chris Ball

2010-08-27 20:56 UTC #1 lore raw

PATCH 1/1 sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

Hi,

Any comments on this patch from the list?

From: Bing Zhao <[email protected]>
Date: Wed, 21 Jul 2010 18:46:55 -0700

Signed-off-by Bing Zhao <[email protected]>

---
 drivers/mmc/core/sdio.c      |   18 +++++++++++++++---
 drivers/mmc/host/sdhci-pci.c |    3 ++-
 include/linux/mmc/pm.h       |    1 +
 3 files changed, 18 insertions(+), 4 deletions(-)

drivers/mmc/core/sdio.c +15 -3

index bd2755e..dc03939 100644
@@ -606,15 +606,27 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 
 static int mmc_sdio_resume(struct mmc_host *host)
 {
-	int i, err;
+	int i, err = 0;
 
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
 	/* Basic card reinitialization. */
 	mmc_claim_host(host);
-	err = mmc_sdio_init_card(host, host->ocr, host->card,
-				 (host->pm_flags & MMC_PM_KEEP_POWER));
+
+	/*
+	 * For a powered resume, Marvell 8686/8688 device will not respond
+	 * to re-initialization attemps if the card is in sleep mode.
+	 * The MMC_PM_SKIP_RESUME_PROBE flag is added as a workaround here.
+	 * This is a temporary measure until a fully thought-through
+	 * solution is elaborated.
+	 */
+	if (!(host->pm_flags & MMC_PM_KEEP_POWER) ||
+		!(host->pm_flags & MMC_PM_SKIP_RESUME_PROBE)) {
+			err = mmc_sdio_init_card(host, host->ocr, host->card,
+					host->pm_flags & MMC_PM_KEEP_POWER);
+	}
+
 	if (!err) {
 		/* We may have switched to 1-bit mode during suspend. */
 		err = sdio_enable_4bit_bus(host->card);

drivers/mmc/host/sdhci-pci.c +2 -1

index e021431..4f793f8 100644
@@ -703,7 +703,8 @@ static struct sdhci_pci_slot * __devinit sdhci_pci_probe_slot(
 			goto unmap;
 	}
 
-	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ;
+	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ
+				| MMC_PM_SKIP_RESUME_PROBE;
 
 	ret = sdhci_add_host(host);
 	if (ret)

include/linux/mmc/pm.h +1 -0

index d37aac4..7dbb630 100644
@@ -26,5 +26,6 @@ typedef unsigned int mmc_pm_flag_t;
 
 #define MMC_PM_KEEP_POWER	(1 << 0)	/* preserve card power during suspend */
 #define MMC_PM_WAKE_SDIO_IRQ	(1 << 1)	/* wake up host system on SDIO IRQ assertion */
+#define MMC_PM_SKIP_RESUME_PROBE	(1 << 2)	/* skip the attempt to reidentify the card in powered resume */
 
 #endif

Signature · 6 lines

1.5.3.6

- Chris.
-- 
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child

Nicolas Pitre

2010-08-27 22:16 UTC #2 lore raw

Quoted 3 lines · Chris Ball

Hi,
Any comments on this patch from the list?

Looks rather hackish to me. There should be a more elegant way to achieve the same result. I have no time to look at it and propose an alternative solution though.

Quoted 84 lines

From: Bing Zhao <[email protected]>
Date: Wed, 21 Jul 2010 18:46:55 -0700

Signed-off-by Bing Zhao <[email protected]>

---
 drivers/mmc/core/sdio.c      |   18 +++++++++++++++---
 drivers/mmc/host/sdhci-pci.c |    3 ++-
 include/linux/mmc/pm.h       |    1 +
 3 files changed, 18 insertions(+), 4 deletions(-)

drivers/mmc/core/sdio.c +15 -3

index bd2755e..dc03939 100644
@@ -606,15 +606,27 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 
 static int mmc_sdio_resume(struct mmc_host *host)
 {
-	int i, err;
+	int i, err = 0;
 
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
 	/* Basic card reinitialization. */
 	mmc_claim_host(host);
-	err = mmc_sdio_init_card(host, host->ocr, host->card,
-				 (host->pm_flags & MMC_PM_KEEP_POWER));
+
+	/*
+	 * For a powered resume, Marvell 8686/8688 device will not respond
+	 * to re-initialization attemps if the card is in sleep mode.
+	 * The MMC_PM_SKIP_RESUME_PROBE flag is added as a workaround here.
+	 * This is a temporary measure until a fully thought-through
+	 * solution is elaborated.
+	 */
+	if (!(host->pm_flags & MMC_PM_KEEP_POWER) ||
+		!(host->pm_flags & MMC_PM_SKIP_RESUME_PROBE)) {
+			err = mmc_sdio_init_card(host, host->ocr, host->card,
+					host->pm_flags & MMC_PM_KEEP_POWER);
+	}
+
 	if (!err) {
 		/* We may have switched to 1-bit mode during suspend. */
 		err = sdio_enable_4bit_bus(host->card);

drivers/mmc/host/sdhci-pci.c +2 -1

index e021431..4f793f8 100644
@@ -703,7 +703,8 @@ static struct sdhci_pci_slot * __devinit sdhci_pci_probe_slot(
 			goto unmap;
 	}
 
-	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ;
+	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ
+				| MMC_PM_SKIP_RESUME_PROBE;
 
 	ret = sdhci_add_host(host);
 	if (ret)

include/linux/mmc/pm.h +1 -0

index d37aac4..7dbb630 100644
@@ -26,5 +26,6 @@ typedef unsigned int mmc_pm_flag_t;
 
 #define MMC_PM_KEEP_POWER	(1 << 0)	/* preserve card power during suspend */
 #define MMC_PM_WAKE_SDIO_IRQ	(1 << 1)	/* wake up host system on SDIO IRQ assertion */
+#define MMC_PM_SKIP_RESUME_PROBE	(1 << 2)	/* skip the attempt to reidentify the card in powered resume */
 
 #endif

Signature · 6 lines

1.5.3.6

- Chris.
-- 
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child

Nicolas Pitre

2010-08-27 22:19 UTC #3 lore raw

Quoted 9 lines · Nicolas Pitre

Quoted 3 lines · Chris Ball
Hi,
Any comments on this patch from the list?
Looks rather hackish to me. There should be a more elegant way to achieve the same result. I have no time to look at it and propose an alternative solution though.

... at the moment that is.

Nicolas

Michał Mirosław

2010-09-01 18:40 UTC #4 lore raw

In reply to Chris Ball

2010/8/27 Chris Ball <[email protected]>:

Quoted 9 lines

Any comments on this patch from the list?
From: Bing Zhao <[email protected]>
Date: Wed, 21 Jul 2010 18:46:55 -0700
Marvell 8686/8688 device will not respond to re-probe attemps in powered resume, if the card is in sleep mode. The added MMC_PM_SKIP_RESUME_PROBE flag is checked so that call to mmc_sdio_init_card() can be skipped.

[patch cut]

Simplified SDIO spec v.2.00 (section 6.14 - Bus State Diagram) suggests, that initialization commands (CMD5, CMD3) are not accepted in CMD state. As the card stays in that state on powered suspend (no resetting CMD52 nor power cycle is issued) then reinitialization should be entirely skipped on resume unless the power was lost between suspend and resume (or card was temporarily removed from the slot).

Best Regards,
Michał Mirosław

Bing Zhao

author

2010-09-02 00:00 UTC #5 lore raw

Hi Michal,

Thanks for your comment.

Quoted 17 lines

-----Original Message-----
From: Michał Mirosław [mailto:[email protected]]
Sent: Wednesday, September 01, 2010 11:40 AM
To: Chris Ball
Cc: Bing Zhao; [email protected]; Nicolas Pitre; Andrew Morton
Subject: Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume
2010/8/27 Chris Ball <[email protected]>:
Quoted 9 lines
Any comments on this patch from the list?
From: Bing Zhao <[email protected]>
Date: Wed, 21 Jul 2010 18:46:55 -0700
Marvell 8686/8688 device will not respond to re-probe attemps in powered resume, if the card is in sleep mode. The added MMC_PM_SKIP_RESUME_PROBE flag is checked so that call to mmc_sdio_init_card() can be skipped.

Quoted 8 lines

[patch cut]
Simplified SDIO spec v.2.00 (section 6.14 - Bus State Diagram) suggests, that initialization commands (CMD5, CMD3) are not accepted in CMD state. As the card stays in that state on powered suspend (no resetting CMD52 nor power cycle is issued) then reinitialization should be entirely skipped on resume unless the power was lost between suspend and resume (or card was temporarily removed from the slot).

With that said, skipping initialization on powered resume is needed in general. We can just check MMC_PM_KEEP_POWER flag to skip the card initialization.

I will send the updated patch.

Regards,

Bing

Quoted 3 lines

Best Regards,
Michał Mirosław

A message that is not in the archive (sent off-list or not yet received): [email protected]

Bing Zhao

author

2010-09-02 01:37 UTC #7 lore raw

Hi Wilson,

Quoted 10 lines

-----Original Message-----
From: Wilson Loi [mailto:[email protected]]
Sent: Wednesday, September 01, 2010 5:33 PM
To: Bing Zhao
Cc: Michał Mirosław; Chris Ball; [email protected]; Nicolas Pitre; Andrew Morton
Subject: Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume
It should be necessary for this flags.
Some WLAN card will put itself either into deep sleep or host wake up mode. They don't need to re-init after resume.

The MMC_PM_KEEP_POWER flag can be set in suspend handler before WLAN card is put into any power saving mode. When system resumes re-init attempt will be skipped as MMC_PM_KEEP_POWER is set.

Regards,

Bing

Quoted 63 lines

2010/9/2 Bing Zhao <[email protected]>
Hi Michal,
Thanks for your comment.
> -----Original Message-----
> From: Michał Mirosław [mailto:[email protected]]
> Sent: Wednesday, September 01, 2010 11:40 AM
> To: Chris Ball
> Cc: Bing Zhao; [email protected]; Nicolas Pitre; Andrew Morton > Subject: Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume >
> 2010/8/27 Chris Ball <[email protected]>:
> > Any comments on this patch from the list?
> >
> > From: Bing Zhao <[email protected]>
> > Date: Wed, 21 Jul 2010 18:46:55 -0700
> >
> > Marvell 8686/8688 device will not respond to re-probe attemps
> > in powered resume, if the card is in sleep mode.
> > The added MMC_PM_SKIP_RESUME_PROBE flag is checked so that
> > call to mmc_sdio_init_card() can be skipped.
> [patch cut]
>
> Simplified SDIO spec v.2.00 (section 6.14 - Bus State Diagram)
> suggests, that initialization commands (CMD5, CMD3) are not accepted > in CMD state. As the card stays in that state on powered suspend (no > resetting CMD52 nor power cycle is issued) then reinitialization
> should be entirely skipped on resume unless the power was lost between > suspend and resume (or card was temporarily removed from the slot).
With that said, skipping initialization on powered resume is needed in general. We can just
check MMC_PM_KEEP_POWER flag to skip the card initialization.
I will send the updated patch.
Regards,
Bing
>
> Best Regards,
> Michał Mirosław
--
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
--
Best regards,
Wilson
http://blog.roodo.com/wlsloi