[PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Bing Zhao]

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 <bzhao@marvell.com>
---
 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(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index bd2755e..dc03939 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -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);
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index e021431..4f793f8 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -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)
diff --git a/include/linux/mmc/pm.h b/include/linux/mmc/pm.h
index d37aac4..7dbb630 100644
--- a/include/linux/mmc/pm.h
+++ b/include/linux/mmc/pm.h
@@ -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
-- 
1.5.3.6

[PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Chris Ball]

Hi,

Any comments on this patch from the list?

From: Bing Zhao <bzhao@marvell.com>
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.

Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 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(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index bd2755e..dc03939 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -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);
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index e021431..4f793f8 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -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)
diff --git a/include/linux/mmc/pm.h b/include/linux/mmc/pm.h
index d37aac4..7dbb630 100644
--- a/include/linux/mmc/pm.h
+++ b/include/linux/mmc/pm.h
@@ -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
-- 
1.5.3.6

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Nicolas Pitre]

On Fri, 27 Aug 2010, Chris Ball wrote:

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


> 
> From: Bing Zhao <bzhao@marvell.com>
> 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.
> 
> Signed-off-by: Bing Zhao <bzhao@marvell.com>
> ---
>  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(-)
> 
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index bd2755e..dc03939 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -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);
> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index e021431..4f793f8 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -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)
> diff --git a/include/linux/mmc/pm.h b/include/linux/mmc/pm.h
> index d37aac4..7dbb630 100644
> --- a/include/linux/mmc/pm.h
> +++ b/include/linux/mmc/pm.h
> @@ -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
> -- 
> 1.5.3.6
> 
> - Chris.
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> 

Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Nicolas Pitre]

On Fri, 27 Aug 2010, Nicolas Pitre wrote:

> On Fri, 27 Aug 2010, Chris Ball wrote:
> 
> > 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

Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Michał Mirosław]

2010/8/27 Chris Ball <cjb@laptop.org>:
> Any comments on this patch from the list?
>
> From: Bing Zhao <bzhao@marvell.com>
> 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

RE: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Bing Zhao]

Hi Michal,

Thanks for your comment.

> -----Original Message-----
> From: Michał Mirosław [mailto:mirqus@gmail.com]
> Sent: Wednesday, September 01, 2010 11:40 AM
> To: Chris Ball
> Cc: Bing Zhao; linux-mmc@vger.kernel.org; Nicolas Pitre; Andrew Morton
> Subject: Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume
> 
> 2010/8/27 Chris Ball <cjb@laptop.org>:
> > Any comments on this patch from the list?
> >
> > From: Bing Zhao <bzhao@marvell.com>
> > 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

RE: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume

[Bing Zhao]

Hi Wilson,

> -----Original Message-----
> From: Wilson Loi [mailto:wlsloi@gmail.com]
> Sent: Wednesday, September 01, 2010 5:33 PM
> To: Bing Zhao
> Cc: Michał Mirosław; Chris Ball; linux-mmc@vger.kernel.org; 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

> 
> 
> 2010/9/2 Bing Zhao <bzhao@marvell.com>
> 
> 
> 	Hi Michal,
> 
> 	Thanks for your comment.
> 
> 
> 	> -----Original Message-----
> 	> From: Michał Mirosław [mailto:mirqus@gmail.com]
> 	> Sent: Wednesday, September 01, 2010 11:40 AM
> 	> To: Chris Ball
> 	> Cc: Bing Zhao; linux-mmc@vger.kernel.org; Nicolas Pitre; Andrew Morton
> 	> Subject: Re: [PATCH] sdio: add MMC_PM_SKIP_RESUME_PROBE to workaround powered resume
> 	>
> 	> 2010/8/27 Chris Ball <cjb@laptop.org>:
> 	> > Any comments on this patch from the list?
> 	> >
> 	> > From: Bing Zhao <bzhao@marvell.com>
> 	> > 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 majordomo@vger.kernel.org
> 	More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> 
> 
> --
> Best regards,
> Wilson
> http://blog.roodo.com/wlsloi