All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] mmc: sdio: enhance irq handling during suspend/resume
@ 2012-12-19 12:12 Kevin Liu
  2012-12-19 12:12 ` [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended Kevin Liu
  2012-12-19 12:12 ` [PATCH 2/2] mmc: sdio: handle interrupt corner case during suspend Kevin Liu
  0 siblings, 2 replies; 11+ messages in thread
From: Kevin Liu @ 2012-12-19 12:12 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: Philip Rakity, Aaron Lu, Shawn Guo, Ulf Hansson, Johan Rudholm,
	Daniel Drake, Guennadi Liakhovetski, Adrian Hunter, Jerry Huang,
	Alexander Stein, Girish K S, Haijun Zhang, Viresh Kumar,
	Heiko Stuebner, Thomas Abraham, Chander Kashyap, Jaehoon Chung,
	Sebastian Hesselbarth, Al Cooper, Zhangfei Gao, Haojian Zhuang


This patchset aim to enhance sdio irq handling during suspend/resume so as to
wakeup host system with sdio card interrupt.

Kevin Liu (2):
 mmc: sdio: defer sdio irq handling when host suspended
 mmc: sdio: handle interrupt corner case during suspend

 drivers/mmc/core/sdio.c       |   59 +++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/core/sdio_irq.c   |    5 +++
 include/linux/mmc/host.h      |    5 +++
 include/linux/mmc/sdio_func.h |    6 ++++
 4 files changed, 73 insertions(+), 2 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2012-12-19 12:12 [PATCH 0/2] mmc: sdio: enhance irq handling during suspend/resume Kevin Liu
@ 2012-12-19 12:12 ` Kevin Liu
  2012-12-21 10:26   ` Ulf Hansson
  2012-12-19 12:12 ` [PATCH 2/2] mmc: sdio: handle interrupt corner case during suspend Kevin Liu
  1 sibling, 1 reply; 11+ messages in thread
From: Kevin Liu @ 2012-12-19 12:12 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: Philip Rakity, Aaron Lu, Shawn Guo, Ulf Hansson, Johan Rudholm,
	Daniel Drake, Guennadi Liakhovetski, Adrian Hunter, Jerry Huang,
	Alexander Stein, Girish K S, Haijun Zhang, Viresh Kumar,
	Heiko Stuebner, Thomas Abraham, Chander Kashyap, Jaehoon Chung,
	Sebastian Hesselbarth, Al Cooper, Zhangfei Gao, Haojian Zhuang

If sdio host can wakeup system, its interrupt will _NOT_ be disabled
during suspending. So when card interrupt happens, the sdio irq thread
will be woken up.
Claim the host  to avoid sdio irq thread handling the pending interrupt
while sdio host suspended. The pending interrupt will be handled after
the host released in resume when sdio host has been resumed.

Signed-off-by: Jialing Fu <jlfu@marvell.com>
Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 2273ce6..81140b9 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -11,6 +11,7 @@
 
 #include <linux/err.h>
 #include <linux/pm_runtime.h>
+#include <linux/pm_wakeup.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 		mmc_release_host(host);
 	}
 
+	/*
+	* If sdio host can wakeup system, its interrupt will _NOT_ be disabled
+	* during suspending. So the card interrupt may occur after host has
+	* suspended. Claim the host here to avoid sdio irq thread handling the
+	* pending interrupt while sdio host suspended. The pending interrupt
+	* will be handled after the host released in resume when sdio host has
+	* been resumed.
+	*/
+	if (!err && device_may_wakeup(mmc_dev(host)))
+		mmc_claim_host(host);
+
 	return err;
 }
 
@@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
-	/* Basic card reinitialization. */
-	mmc_claim_host(host);
+	/*
+	* Basic card reinitialization.
+	* if sdio host can wakeup system, the host has been claimed in suspend.
+	*/
+	if (!device_may_wakeup(mmc_dev(host)))
+		mmc_claim_host(host);
 
 	/* No need to reinitialize powered-resumed nonremovable cards */
 	if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] mmc: sdio: handle interrupt corner case during suspend
  2012-12-19 12:12 [PATCH 0/2] mmc: sdio: enhance irq handling during suspend/resume Kevin Liu
  2012-12-19 12:12 ` [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended Kevin Liu
@ 2012-12-19 12:12 ` Kevin Liu
  1 sibling, 0 replies; 11+ messages in thread
From: Kevin Liu @ 2012-12-19 12:12 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: Philip Rakity, Aaron Lu, Shawn Guo, Ulf Hansson, Johan Rudholm,
	Daniel Drake, Guennadi Liakhovetski, Adrian Hunter, Jerry Huang,
	Alexander Stein, Girish K S, Haijun Zhang, Viresh Kumar,
	Heiko Stuebner, Thomas Abraham, Chander Kashyap, Jaehoon Chung,
	Sebastian Hesselbarth, Al Cooper, Zhangfei Gao, Haojian Zhuang

Sdio device may trigger interrupt after its function suspended
while system not suspended yet.

This patch handle above cases by below two methods:
1. For multiple sdio_funcs, abort suspend if interrupt happen after
its func suspended while some other funcs not suspended yet.
2. If interrupt happens after all sdio_funcs suspended, wakeup the
system.

Signed-off-by: Jialing Fu <jlfu@marvell.com>
Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/core/sdio.c       |   57 ++++++++++++++++++++++++++++++++++------
 drivers/mmc/core/sdio_irq.c   |    5 +++
 include/linux/mmc/host.h      |    5 +++
 include/linux/mmc/sdio_func.h |    6 ++++
 4 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 81140b9..d9b4a11 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -888,6 +888,14 @@ out:
 	}
 }
 
+void mmc_sdio_irq_wakeup(struct mmc_host *host)
+{
+	pr_warning("%s: submit a wakeup event\n",
+		       mmc_hostname(host));
+	/* set the process time to 3 seconds */
+	pm_wakeup_event(mmc_dev(host), 3000);
+}
+
 /*
  * SDIO suspend.  We need to suspend all functions separately.
  * Therefore all registered functions must have drivers with suspend
@@ -897,8 +905,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 {
 	int i, err = 0;
 
+	atomic_set(&host->sdio_suspend_abort, 0);
 	for (i = 0; i < host->card->sdio_funcs; i++) {
 		struct sdio_func *func = host->card->sdio_func[i];
+
+		/* Abort suspend if irq come after its sdio_func suspended */
+		if (i && atomic_cmpxchg(&host->sdio_suspend_abort, 1, 0)) {
+			atomic_set(&host->sdio_suspend_abort, 0);
+			err = -EBUSY;
+			break;
+		}
+
 		if (func && sdio_func_present(func) && func->dev.driver) {
 			const struct dev_pm_ops *pmops = func->dev.driver->pm;
 			if (!pmops || !pmops->suspend || !pmops->resume) {
@@ -906,15 +923,20 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 				err = -ENOSYS;
 			} else
 				err = pmops->suspend(&func->dev);
+
 			if (err)
 				break;
+			else
+				func->func_status = FUNC_SUSPENDED;
 		}
 	}
+
 	while (err && --i >= 0) {
 		struct sdio_func *func = host->card->sdio_func[i];
 		if (func && sdio_func_present(func) && func->dev.driver) {
 			const struct dev_pm_ops *pmops = func->dev.driver->pm;
 			pmops->resume(&func->dev);
+			func->func_status = FUNC_RESUMED;
 		}
 	}
 
@@ -924,17 +946,30 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 		mmc_release_host(host);
 	}
 
-	/*
-	* If sdio host can wakeup system, its interrupt will _NOT_ be disabled
-	* during suspending. So the card interrupt may occur after host has
-	* suspended. Claim the host here to avoid sdio irq thread handling the
-	* pending interrupt while sdio host suspended. The pending interrupt
-	* will be handled after the host released in resume when sdio host has
-	* been resumed.
-	*/
-	if (!err && device_may_wakeup(mmc_dev(host)))
+	if (!err && device_may_wakeup(mmc_dev(host))) {
+		/*
+		 * All interrupts after this will wakeup system
+		 */
+		atomic_set(&host->sdio_suspended, 1);
+
+		/*
+		 * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
+		 * during suspending. So the card interrupt may occur after host has
+		 * suspended. Claim the host here to avoid sdio irq thread handling the
+		 * pending interrupt while sdio host suspended. The pending interrupt
+		 * will be handled after the host released in resume when sdio host has
+		 * been resumed.
+		 */
 		mmc_claim_host(host);
 
+		/*
+		 * Double check whether sdio IRQ happens after all func
+		 * suspended. If this case occur, wakeup system.
+		 */
+		if (atomic_cmpxchg(&host->sdio_suspend_abort, 1, 0))
+			mmc_sdio_irq_wakeup(host);
+	}
+
 	return err;
 }
 
@@ -967,6 +1002,8 @@ static int mmc_sdio_resume(struct mmc_host *host)
 
 	if (!err && host->sdio_irqs)
 		wake_up_process(host->sdio_irq_thread);
+
+	atomic_set(&host->sdio_suspended, 0);
 	mmc_release_host(host);
 
 	/*
@@ -982,8 +1019,10 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	for (i = 0; !err && i < host->card->sdio_funcs; i++) {
 		struct sdio_func *func = host->card->sdio_func[i];
 		if (func && sdio_func_present(func) && func->dev.driver) {
+
 			const struct dev_pm_ops *pmops = func->dev.driver->pm;
 			err = pmops->resume(&func->dev);
+			func->func_status = FUNC_RESUMED;
 		}
 	}
 
diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
index 3d8ceb4..cf03463 100644
--- a/drivers/mmc/core/sdio_irq.c
+++ b/drivers/mmc/core/sdio_irq.c
@@ -43,6 +43,9 @@ static int process_sdio_pending_irqs(struct mmc_host *host)
 	func = card->sdio_single_irq;
 	if (func && host->sdio_irq_pending) {
 		func->irq_handler(func);
+		if (func->func_status == FUNC_SUSPENDED)
+			atomic_set(&host->sdio_suspend_abort, 1);
+
 		return 1;
 	}
 
@@ -64,6 +67,8 @@ static int process_sdio_pending_irqs(struct mmc_host *host)
 				ret = -EINVAL;
 			} else if (func->irq_handler) {
 				func->irq_handler(func);
+				if (func->func_status == FUNC_SUSPENDED)
+					atomic_set(&host->sdio_suspend_abort, 1);
 				count++;
 			} else {
 				pr_warning("%s: pending IRQ with no handler\n",
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 61a10c1..72984ce 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -316,6 +316,8 @@ struct mmc_host {
 	struct task_struct	*sdio_irq_thread;
 	bool			sdio_irq_pending;
 	atomic_t		sdio_irq_thread_abort;
+	atomic_t		sdio_suspend_abort;
+	atomic_t		sdio_suspended;
 
 	mmc_pm_flag_t		pm_flags;	/* requested pm features */
 
@@ -367,11 +369,14 @@ extern void mmc_detect_change(struct mmc_host *, unsigned long delay);
 extern void mmc_request_done(struct mmc_host *, struct mmc_request *);
 
 extern int mmc_cache_ctrl(struct mmc_host *, u8);
+extern void mmc_sdio_irq_wakeup(struct mmc_host *host);
 
 static inline void mmc_signal_sdio_irq(struct mmc_host *host)
 {
 	host->ops->enable_sdio_irq(host, 0);
 	host->sdio_irq_pending = true;
+	if (atomic_read(&host->sdio_suspended))
+		mmc_sdio_irq_wakeup(host);
 	wake_up_process(host->sdio_irq_thread);
 }
 
diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
index 50f0bc9..59f4c23 100644
--- a/include/linux/mmc/sdio_func.h
+++ b/include/linux/mmc/sdio_func.h
@@ -32,6 +32,11 @@ struct sdio_func_tuple {
 	unsigned char data[0];
 };
 
+enum sdio_func_status {
+	FUNC_RESUMED = 0,	/* default value */
+	FUNC_SUSPENDED,
+};
+
 /*
  * SDIO function devices
  */
@@ -59,6 +64,7 @@ struct sdio_func {
 	const char		**info;		/* info strings */
 
 	struct sdio_func_tuple *tuples;
+	enum sdio_func_status	func_status;	/* SDIO function driver state */
 };
 
 #define sdio_func_present(f)	((f)->state & SDIO_STATE_PRESENT)
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2012-12-19 12:12 ` [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended Kevin Liu
@ 2012-12-21 10:26   ` Ulf Hansson
  2012-12-21 12:59     ` Kevin Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2012-12-21 10:26 UTC (permalink / raw)
  To: Kevin Liu
  Cc: linux-mmc, Chris Ball, Philip Rakity, Aaron Lu, Shawn Guo,
	Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao, Haojian Zhuang

On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
> during suspending. So when card interrupt happens, the sdio irq thread
> will be woken up.

Is that really needed to handle the irq?

I think you should instead wait on the system resume to be handled
properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
out if there is an irq to handle.

....
if (!err && host->sdio_irqs)
  wake_up_process(host->sdio_irq_thread);
....

Would that not solve your issue?

> Claim the host  to avoid sdio irq thread handling the pending interrupt
> while sdio host suspended. The pending interrupt will be handled after
> the host released in resume when sdio host has been resumed.
>
> Signed-off-by: Jialing Fu <jlfu@marvell.com>
> Signed-off-by: Kevin Liu <kliu5@marvell.com>
> ---
>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 2273ce6..81140b9 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -11,6 +11,7 @@
>
>  #include <linux/err.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pm_wakeup.h>
>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/card.h>
> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>                 mmc_release_host(host);
>         }
>
> +       /*
> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
> +       * during suspending. So the card interrupt may occur after host has
> +       * suspended. Claim the host here to avoid sdio irq thread handling the
> +       * pending interrupt while sdio host suspended. The pending interrupt
> +       * will be handled after the host released in resume when sdio host has
> +       * been resumed.
> +       */
> +       if (!err && device_may_wakeup(mmc_dev(host)))
> +               mmc_claim_host(host);
> +
>         return err;
>  }
>
> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>         BUG_ON(!host);
>         BUG_ON(!host->card);
>
> -       /* Basic card reinitialization. */
> -       mmc_claim_host(host);
> +       /*
> +       * Basic card reinitialization.
> +       * if sdio host can wakeup system, the host has been claimed in suspend.
> +       */
> +       if (!device_may_wakeup(mmc_dev(host)))
> +               mmc_claim_host(host);
>
>         /* No need to reinitialize powered-resumed nonremovable cards */
>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
> --
> 1.7.0.4
>

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2012-12-21 10:26   ` Ulf Hansson
@ 2012-12-21 12:59     ` Kevin Liu
  2013-01-07 10:59       ` Ulf Hansson
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Liu @ 2012-12-21 12:59 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>> during suspending. So when card interrupt happens, the sdio irq thread
>> will be woken up.
>
> Is that really needed to handle the irq?
>
> I think you should instead wait on the system resume to be handled
> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
> out if there is an irq to handle.
>
> ....
> if (!err && host->sdio_irqs)
>   wake_up_process(host->sdio_irq_thread);
> ....
>
> Would that not solve your issue?
>

With current code, if irq keeps on during suspend/resume, when
interrupt happen, sdio_irq_thread will be woken up and handle the
interrupt immediately while sdio host has suspended. It won't wait.

Just as you said, this patch just let sdio_irq_thread wait untill sdio
host resume back to handle the interrupt.

>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>> while sdio host suspended. The pending interrupt will be handled after
>> the host released in resume when sdio host has been resumed.
>>
>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>> ---
>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index 2273ce6..81140b9 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -11,6 +11,7 @@
>>
>>  #include <linux/err.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/pm_wakeup.h>
>>
>>  #include <linux/mmc/host.h>
>>  #include <linux/mmc/card.h>
>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>                 mmc_release_host(host);
>>         }
>>
>> +       /*
>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>> +       * during suspending. So the card interrupt may occur after host has
>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>> +       * pending interrupt while sdio host suspended. The pending interrupt
>> +       * will be handled after the host released in resume when sdio host has
>> +       * been resumed.
>> +       */
>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>> +               mmc_claim_host(host);
>> +
>>         return err;
>>  }
>>
>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>         BUG_ON(!host);
>>         BUG_ON(!host->card);
>>
>> -       /* Basic card reinitialization. */
>> -       mmc_claim_host(host);
>> +       /*
>> +       * Basic card reinitialization.
>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>> +       */
>> +       if (!device_may_wakeup(mmc_dev(host)))
>> +               mmc_claim_host(host);
>>
>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>> --
>> 1.7.0.4
>>
>
> Kind regards
> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2012-12-21 12:59     ` Kevin Liu
@ 2013-01-07 10:59       ` Ulf Hansson
  2013-01-07 11:16         ` Kevin Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2013-01-07 10:59 UTC (permalink / raw)
  To: Kevin Liu
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

On 21 December 2012 13:59, Kevin Liu <keyuan.liu@gmail.com> wrote:
> 2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>> during suspending. So when card interrupt happens, the sdio irq thread
>>> will be woken up.
>>
>> Is that really needed to handle the irq?
>>
>> I think you should instead wait on the system resume to be handled
>> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
>> out if there is an irq to handle.
>>
>> ....
>> if (!err && host->sdio_irqs)
>>   wake_up_process(host->sdio_irq_thread);
>> ....
>>
>> Would that not solve your issue?
>>
>
> With current code, if irq keeps on during suspend/resume, when
> interrupt happen, sdio_irq_thread will be woken up and handle the
> interrupt immediately while sdio host has suspended. It won't wait.

That is because the sdio host, when in a suspended state, does a
mmc_signal_sdio_irq call. Is that really what the host should do in
this state?

>
> Just as you said, this patch just let sdio_irq_thread wait untill sdio
> host resume back to handle the interrupt.
>
>>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>>> while sdio host suspended. The pending interrupt will be handled after
>>> the host released in resume when sdio host has been resumed.
>>>
>>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>> ---
>>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>> index 2273ce6..81140b9 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -11,6 +11,7 @@
>>>
>>>  #include <linux/err.h>
>>>  #include <linux/pm_runtime.h>
>>> +#include <linux/pm_wakeup.h>
>>>
>>>  #include <linux/mmc/host.h>
>>>  #include <linux/mmc/card.h>
>>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>>                 mmc_release_host(host);
>>>         }
>>>
>>> +       /*
>>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>> +       * during suspending. So the card interrupt may occur after host has
>>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>>> +       * pending interrupt while sdio host suspended. The pending interrupt
>>> +       * will be handled after the host released in resume when sdio host has
>>> +       * been resumed.
>>> +       */
>>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>>> +               mmc_claim_host(host);
>>> +
>>>         return err;
>>>  }
>>>
>>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>         BUG_ON(!host);
>>>         BUG_ON(!host->card);
>>>
>>> -       /* Basic card reinitialization. */
>>> -       mmc_claim_host(host);
>>> +       /*
>>> +       * Basic card reinitialization.
>>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>>> +       */
>>> +       if (!device_may_wakeup(mmc_dev(host)))
>>> +               mmc_claim_host(host);
>>>
>>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>>> --
>>> 1.7.0.4
>>>
>>
>> Kind regards
>> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2013-01-07 10:59       ` Ulf Hansson
@ 2013-01-07 11:16         ` Kevin Liu
  2013-01-07 12:01           ` Ulf Hansson
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Liu @ 2013-01-07 11:16 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
> On 21 December 2012 13:59, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
>>> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>>>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>> during suspending. So when card interrupt happens, the sdio irq thread
>>>> will be woken up.
>>>
>>> Is that really needed to handle the irq?
>>>
>>> I think you should instead wait on the system resume to be handled
>>> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
>>> out if there is an irq to handle.
>>>
>>> ....
>>> if (!err && host->sdio_irqs)
>>>   wake_up_process(host->sdio_irq_thread);
>>> ....
>>>
>>> Would that not solve your issue?
>>>
>>
>> With current code, if irq keeps on during suspend/resume, when
>> interrupt happen, sdio_irq_thread will be woken up and handle the
>> interrupt immediately while sdio host has suspended. It won't wait.
>
> That is because the sdio host, when in a suspended state, does a
> mmc_signal_sdio_irq call. Is that really what the host should do in
> this state?
>

Yes, the host should do this because we want the sdio to wakeup host
after system suspended.
So we must keep interrupt enabled after suspended.

>>
>> Just as you said, this patch just let sdio_irq_thread wait untill sdio
>> host resume back to handle the interrupt.
>>
>>>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>>>> while sdio host suspended. The pending interrupt will be handled after
>>>> the host released in resume when sdio host has been resumed.
>>>>
>>>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>>> ---
>>>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>> index 2273ce6..81140b9 100644
>>>> --- a/drivers/mmc/core/sdio.c
>>>> +++ b/drivers/mmc/core/sdio.c
>>>> @@ -11,6 +11,7 @@
>>>>
>>>>  #include <linux/err.h>
>>>>  #include <linux/pm_runtime.h>
>>>> +#include <linux/pm_wakeup.h>
>>>>
>>>>  #include <linux/mmc/host.h>
>>>>  #include <linux/mmc/card.h>
>>>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>>>                 mmc_release_host(host);
>>>>         }
>>>>
>>>> +       /*
>>>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>> +       * during suspending. So the card interrupt may occur after host has
>>>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>>>> +       * pending interrupt while sdio host suspended. The pending interrupt
>>>> +       * will be handled after the host released in resume when sdio host has
>>>> +       * been resumed.
>>>> +       */
>>>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>>>> +               mmc_claim_host(host);
>>>> +
>>>>         return err;
>>>>  }
>>>>
>>>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>         BUG_ON(!host);
>>>>         BUG_ON(!host->card);
>>>>
>>>> -       /* Basic card reinitialization. */
>>>> -       mmc_claim_host(host);
>>>> +       /*
>>>> +       * Basic card reinitialization.
>>>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>>>> +       */
>>>> +       if (!device_may_wakeup(mmc_dev(host)))
>>>> +               mmc_claim_host(host);
>>>>
>>>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>>>> --
>>>> 1.7.0.4
>>>>
>>>
>>> Kind regards
>>> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2013-01-07 11:16         ` Kevin Liu
@ 2013-01-07 12:01           ` Ulf Hansson
  2013-01-07 16:04             ` Kevin Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2013-01-07 12:01 UTC (permalink / raw)
  To: Kevin Liu
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

On 7 January 2013 12:16, Kevin Liu <keyuan.liu@gmail.com> wrote:
> 2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 21 December 2012 13:59, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>> 2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
>>>> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>>>>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>> during suspending. So when card interrupt happens, the sdio irq thread
>>>>> will be woken up.
>>>>
>>>> Is that really needed to handle the irq?
>>>>
>>>> I think you should instead wait on the system resume to be handled
>>>> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
>>>> out if there is an irq to handle.
>>>>
>>>> ....
>>>> if (!err && host->sdio_irqs)
>>>>   wake_up_process(host->sdio_irq_thread);
>>>> ....
>>>>
>>>> Would that not solve your issue?
>>>>
>>>
>>> With current code, if irq keeps on during suspend/resume, when
>>> interrupt happen, sdio_irq_thread will be woken up and handle the
>>> interrupt immediately while sdio host has suspended. It won't wait.
>>
>> That is because the sdio host, when in a suspended state, does a
>> mmc_signal_sdio_irq call. Is that really what the host should do in
>> this state?
>>
>
> Yes, the host should do this because we want the sdio to wakeup host
> after system suspended.
> So we must keep interrupt enabled after suspended.

I see that you need to keep the irqs enabled. But that does not mean
you need to call the mmc_signal_sdio_irq, when the host is in
suspended. You should only need to keep the irqs enabled to wake up
the system from suspend, that is enough, the rest could be handled
through the normal resume sequence. Don't you think?

>
>>>
>>> Just as you said, this patch just let sdio_irq_thread wait untill sdio
>>> host resume back to handle the interrupt.
>>>
>>>>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>>>>> while sdio host suspended. The pending interrupt will be handled after
>>>>> the host released in resume when sdio host has been resumed.
>>>>>
>>>>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>>>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>>>> ---
>>>>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>>>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>> index 2273ce6..81140b9 100644
>>>>> --- a/drivers/mmc/core/sdio.c
>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>> @@ -11,6 +11,7 @@
>>>>>
>>>>>  #include <linux/err.h>
>>>>>  #include <linux/pm_runtime.h>
>>>>> +#include <linux/pm_wakeup.h>
>>>>>
>>>>>  #include <linux/mmc/host.h>
>>>>>  #include <linux/mmc/card.h>
>>>>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>>>>                 mmc_release_host(host);
>>>>>         }
>>>>>
>>>>> +       /*
>>>>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>> +       * during suspending. So the card interrupt may occur after host has
>>>>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>>>>> +       * pending interrupt while sdio host suspended. The pending interrupt
>>>>> +       * will be handled after the host released in resume when sdio host has
>>>>> +       * been resumed.
>>>>> +       */
>>>>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>>>>> +               mmc_claim_host(host);
>>>>> +
>>>>>         return err;
>>>>>  }
>>>>>
>>>>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>>         BUG_ON(!host);
>>>>>         BUG_ON(!host->card);
>>>>>
>>>>> -       /* Basic card reinitialization. */
>>>>> -       mmc_claim_host(host);
>>>>> +       /*
>>>>> +       * Basic card reinitialization.
>>>>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>>>>> +       */
>>>>> +       if (!device_may_wakeup(mmc_dev(host)))
>>>>> +               mmc_claim_host(host);
>>>>>
>>>>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>>>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>>>>> --
>>>>> 1.7.0.4
>>>>>
>>>>
>>>> Kind regards
>>>> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2013-01-07 12:01           ` Ulf Hansson
@ 2013-01-07 16:04             ` Kevin Liu
  2013-01-07 16:38               ` Ulf Hansson
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Liu @ 2013-01-07 16:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
> On 7 January 2013 12:16, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>> On 21 December 2012 13:59, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>> 2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
>>>>> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>>>>>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>>> during suspending. So when card interrupt happens, the sdio irq thread
>>>>>> will be woken up.
>>>>>
>>>>> Is that really needed to handle the irq?
>>>>>
>>>>> I think you should instead wait on the system resume to be handled
>>>>> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
>>>>> out if there is an irq to handle.
>>>>>
>>>>> ....
>>>>> if (!err && host->sdio_irqs)
>>>>>   wake_up_process(host->sdio_irq_thread);
>>>>> ....
>>>>>
>>>>> Would that not solve your issue?
>>>>>
>>>>
>>>> With current code, if irq keeps on during suspend/resume, when
>>>> interrupt happen, sdio_irq_thread will be woken up and handle the
>>>> interrupt immediately while sdio host has suspended. It won't wait.
>>>
>>> That is because the sdio host, when in a suspended state, does a
>>> mmc_signal_sdio_irq call. Is that really what the host should do in
>>> this state?
>>>
>>
>> Yes, the host should do this because we want the sdio to wakeup host
>> after system suspended.
>> So we must keep interrupt enabled after suspended.
>
> I see that you need to keep the irqs enabled. But that does not mean
> you need to call the mmc_signal_sdio_irq, when the host is in
> suspended. You should only need to keep the irqs enabled to wake up
> the system from suspend, that is enough, the rest could be handled
> through the normal resume sequence. Don't you think?
>

mmc_signal_sdio_irq is called by sdhci_irq if card interrupt occur.
So if the irqs enabled, then mmc_signal_sdio_irq will be called when
card int from sdio occur even after host suspended and before resumed.

>>
>>>>
>>>> Just as you said, this patch just let sdio_irq_thread wait untill sdio
>>>> host resume back to handle the interrupt.
>>>>
>>>>>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>>>>>> while sdio host suspended. The pending interrupt will be handled after
>>>>>> the host released in resume when sdio host has been resumed.
>>>>>>
>>>>>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>>>>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>>>>> ---
>>>>>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>>>>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>>> index 2273ce6..81140b9 100644
>>>>>> --- a/drivers/mmc/core/sdio.c
>>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>>> @@ -11,6 +11,7 @@
>>>>>>
>>>>>>  #include <linux/err.h>
>>>>>>  #include <linux/pm_runtime.h>
>>>>>> +#include <linux/pm_wakeup.h>
>>>>>>
>>>>>>  #include <linux/mmc/host.h>
>>>>>>  #include <linux/mmc/card.h>
>>>>>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>>>>>                 mmc_release_host(host);
>>>>>>         }
>>>>>>
>>>>>> +       /*
>>>>>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>>> +       * during suspending. So the card interrupt may occur after host has
>>>>>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>>>>>> +       * pending interrupt while sdio host suspended. The pending interrupt
>>>>>> +       * will be handled after the host released in resume when sdio host has
>>>>>> +       * been resumed.
>>>>>> +       */
>>>>>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>>>>>> +               mmc_claim_host(host);
>>>>>> +
>>>>>>         return err;
>>>>>>  }
>>>>>>
>>>>>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>>>         BUG_ON(!host);
>>>>>>         BUG_ON(!host->card);
>>>>>>
>>>>>> -       /* Basic card reinitialization. */
>>>>>> -       mmc_claim_host(host);
>>>>>> +       /*
>>>>>> +       * Basic card reinitialization.
>>>>>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>>>>>> +       */
>>>>>> +       if (!device_may_wakeup(mmc_dev(host)))
>>>>>> +               mmc_claim_host(host);
>>>>>>
>>>>>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>>>>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>>>>>> --
>>>>>> 1.7.0.4
>>>>>>
>>>>>
>>>>> Kind regards
>>>>> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2013-01-07 16:04             ` Kevin Liu
@ 2013-01-07 16:38               ` Ulf Hansson
  2013-01-07 17:01                 ` Kevin Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2013-01-07 16:38 UTC (permalink / raw)
  To: Kevin Liu
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

On 7 January 2013 17:04, Kevin Liu <keyuan.liu@gmail.com> wrote:
> 2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 7 January 2013 12:16, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>> 2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>>> On 21 December 2012 13:59, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>>> 2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
>>>>>> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>>>>>>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>>>> during suspending. So when card interrupt happens, the sdio irq thread
>>>>>>> will be woken up.
>>>>>>
>>>>>> Is that really needed to handle the irq?
>>>>>>
>>>>>> I think you should instead wait on the system resume to be handled
>>>>>> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
>>>>>> out if there is an irq to handle.
>>>>>>
>>>>>> ....
>>>>>> if (!err && host->sdio_irqs)
>>>>>>   wake_up_process(host->sdio_irq_thread);
>>>>>> ....
>>>>>>
>>>>>> Would that not solve your issue?
>>>>>>
>>>>>
>>>>> With current code, if irq keeps on during suspend/resume, when
>>>>> interrupt happen, sdio_irq_thread will be woken up and handle the
>>>>> interrupt immediately while sdio host has suspended. It won't wait.
>>>>
>>>> That is because the sdio host, when in a suspended state, does a
>>>> mmc_signal_sdio_irq call. Is that really what the host should do in
>>>> this state?
>>>>
>>>
>>> Yes, the host should do this because we want the sdio to wakeup host
>>> after system suspended.
>>> So we must keep interrupt enabled after suspended.
>>
>> I see that you need to keep the irqs enabled. But that does not mean
>> you need to call the mmc_signal_sdio_irq, when the host is in
>> suspended. You should only need to keep the irqs enabled to wake up
>> the system from suspend, that is enough, the rest could be handled
>> through the normal resume sequence. Don't you think?
>>
>
> mmc_signal_sdio_irq is called by sdhci_irq if card interrupt occur.
> So if the irqs enabled, then mmc_signal_sdio_irq will be called when
> card int from sdio occur even after host suspended and before resumed.
>

Yes, but is that really correct? Should the host really do
mmc_signal_sdio_irq when the host is suspended? To me it feel like a
more proper way of dealing with this is to let the normal resume
sequence eventually handle it instead?

>>>
>>>>>
>>>>> Just as you said, this patch just let sdio_irq_thread wait untill sdio
>>>>> host resume back to handle the interrupt.
>>>>>
>>>>>>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>>>>>>> while sdio host suspended. The pending interrupt will be handled after
>>>>>>> the host released in resume when sdio host has been resumed.
>>>>>>>
>>>>>>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>>>>>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>>>>>> ---
>>>>>>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>>>>>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>>>> index 2273ce6..81140b9 100644
>>>>>>> --- a/drivers/mmc/core/sdio.c
>>>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>>>> @@ -11,6 +11,7 @@
>>>>>>>
>>>>>>>  #include <linux/err.h>
>>>>>>>  #include <linux/pm_runtime.h>
>>>>>>> +#include <linux/pm_wakeup.h>
>>>>>>>
>>>>>>>  #include <linux/mmc/host.h>
>>>>>>>  #include <linux/mmc/card.h>
>>>>>>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>>>>>>                 mmc_release_host(host);
>>>>>>>         }
>>>>>>>
>>>>>>> +       /*
>>>>>>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>>>> +       * during suspending. So the card interrupt may occur after host has
>>>>>>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>>>>>>> +       * pending interrupt while sdio host suspended. The pending interrupt
>>>>>>> +       * will be handled after the host released in resume when sdio host has
>>>>>>> +       * been resumed.
>>>>>>> +       */
>>>>>>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>>>>>>> +               mmc_claim_host(host);
>>>>>>> +
>>>>>>>         return err;
>>>>>>>  }
>>>>>>>
>>>>>>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>>>>         BUG_ON(!host);
>>>>>>>         BUG_ON(!host->card);
>>>>>>>
>>>>>>> -       /* Basic card reinitialization. */
>>>>>>> -       mmc_claim_host(host);
>>>>>>> +       /*
>>>>>>> +       * Basic card reinitialization.
>>>>>>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>>>>>>> +       */
>>>>>>> +       if (!device_may_wakeup(mmc_dev(host)))
>>>>>>> +               mmc_claim_host(host);
>>>>>>>
>>>>>>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>>>>>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>>>>>>> --
>>>>>>> 1.7.0.4
>>>>>>>
>>>>>>
>>>>>> Kind regards
>>>>>> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended
  2013-01-07 16:38               ` Ulf Hansson
@ 2013-01-07 17:01                 ` Kevin Liu
  0 siblings, 0 replies; 11+ messages in thread
From: Kevin Liu @ 2013-01-07 17:01 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Kevin Liu, linux-mmc, Chris Ball, Philip Rakity, Aaron Lu,
	Shawn Guo, Johan Rudholm, Daniel Drake, Guennadi Liakhovetski,
	Adrian Hunter, Jerry Huang, Alexander Stein, Girish K S,
	Haijun Zhang, Viresh Kumar, Heiko Stuebner, Thomas Abraham,
	Chander Kashyap, Jaehoon Chung, Sebastian Hesselbarth, Al Cooper,
	Zhangfei Gao

2013/1/8 Ulf Hansson <ulf.hansson@linaro.org>:
> On 7 January 2013 17:04, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>> On 7 January 2013 12:16, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>> 2013/1/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>>>> On 21 December 2012 13:59, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>>>> 2012/12/21 Ulf Hansson <ulf.hansson@linaro.org>:
>>>>>>> On 19 December 2012 13:12, Kevin Liu <kliu5@marvell.com> wrote:
>>>>>>>> If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>>>>> during suspending. So when card interrupt happens, the sdio irq thread
>>>>>>>> will be woken up.
>>>>>>>
>>>>>>> Is that really needed to handle the irq?
>>>>>>>
>>>>>>> I think you should instead wait on the system resume to be handled
>>>>>>> properly. In the mmc_sdio_resume, sdio_irq thread is woken up to find
>>>>>>> out if there is an irq to handle.
>>>>>>>
>>>>>>> ....
>>>>>>> if (!err && host->sdio_irqs)
>>>>>>>   wake_up_process(host->sdio_irq_thread);
>>>>>>> ....
>>>>>>>
>>>>>>> Would that not solve your issue?
>>>>>>>
>>>>>>
>>>>>> With current code, if irq keeps on during suspend/resume, when
>>>>>> interrupt happen, sdio_irq_thread will be woken up and handle the
>>>>>> interrupt immediately while sdio host has suspended. It won't wait.
>>>>>
>>>>> That is because the sdio host, when in a suspended state, does a
>>>>> mmc_signal_sdio_irq call. Is that really what the host should do in
>>>>> this state?
>>>>>
>>>>
>>>> Yes, the host should do this because we want the sdio to wakeup host
>>>> after system suspended.
>>>> So we must keep interrupt enabled after suspended.
>>>
>>> I see that you need to keep the irqs enabled. But that does not mean
>>> you need to call the mmc_signal_sdio_irq, when the host is in
>>> suspended. You should only need to keep the irqs enabled to wake up
>>> the system from suspend, that is enough, the rest could be handled
>>> through the normal resume sequence. Don't you think?
>>>
>>
>> mmc_signal_sdio_irq is called by sdhci_irq if card interrupt occur.
>> So if the irqs enabled, then mmc_signal_sdio_irq will be called when
>> card int from sdio occur even after host suspended and before resumed.
>>
>
> Yes, but is that really correct? Should the host really do
> mmc_signal_sdio_irq when the host is suspended? To me it feel like a
> more proper way of dealing with this is to let the normal resume
> sequence eventually handle it instead?
>

Yes, it seems more reasonable...
Just skip the wake_up if suspended...I will update the patch. Thanks!

>>>>
>>>>>>
>>>>>> Just as you said, this patch just let sdio_irq_thread wait untill sdio
>>>>>> host resume back to handle the interrupt.
>>>>>>
>>>>>>>> Claim the host  to avoid sdio irq thread handling the pending interrupt
>>>>>>>> while sdio host suspended. The pending interrupt will be handled after
>>>>>>>> the host released in resume when sdio host has been resumed.
>>>>>>>>
>>>>>>>> Signed-off-by: Jialing Fu <jlfu@marvell.com>
>>>>>>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>>>>>>> ---
>>>>>>>>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++--
>>>>>>>>  1 files changed, 18 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>>>>> index 2273ce6..81140b9 100644
>>>>>>>> --- a/drivers/mmc/core/sdio.c
>>>>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>>>>> @@ -11,6 +11,7 @@
>>>>>>>>
>>>>>>>>  #include <linux/err.h>
>>>>>>>>  #include <linux/pm_runtime.h>
>>>>>>>> +#include <linux/pm_wakeup.h>
>>>>>>>>
>>>>>>>>  #include <linux/mmc/host.h>
>>>>>>>>  #include <linux/mmc/card.h>
>>>>>>>> @@ -923,6 +924,17 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>>>>>>>                 mmc_release_host(host);
>>>>>>>>         }
>>>>>>>>
>>>>>>>> +       /*
>>>>>>>> +       * If sdio host can wakeup system, its interrupt will _NOT_ be disabled
>>>>>>>> +       * during suspending. So the card interrupt may occur after host has
>>>>>>>> +       * suspended. Claim the host here to avoid sdio irq thread handling the
>>>>>>>> +       * pending interrupt while sdio host suspended. The pending interrupt
>>>>>>>> +       * will be handled after the host released in resume when sdio host has
>>>>>>>> +       * been resumed.
>>>>>>>> +       */
>>>>>>>> +       if (!err && device_may_wakeup(mmc_dev(host)))
>>>>>>>> +               mmc_claim_host(host);
>>>>>>>> +
>>>>>>>>         return err;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> @@ -933,8 +945,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>>>>>         BUG_ON(!host);
>>>>>>>>         BUG_ON(!host->card);
>>>>>>>>
>>>>>>>> -       /* Basic card reinitialization. */
>>>>>>>> -       mmc_claim_host(host);
>>>>>>>> +       /*
>>>>>>>> +       * Basic card reinitialization.
>>>>>>>> +       * if sdio host can wakeup system, the host has been claimed in suspend.
>>>>>>>> +       */
>>>>>>>> +       if (!device_may_wakeup(mmc_dev(host)))
>>>>>>>> +               mmc_claim_host(host);
>>>>>>>>
>>>>>>>>         /* No need to reinitialize powered-resumed nonremovable cards */
>>>>>>>>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
>>>>>>>> --
>>>>>>>> 1.7.0.4
>>>>>>>>
>>>>>>>
>>>>>>> Kind regards
>>>>>>> Ulf Hansson

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-01-07 17:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-19 12:12 [PATCH 0/2] mmc: sdio: enhance irq handling during suspend/resume Kevin Liu
2012-12-19 12:12 ` [PATCH 1/2] mmc: sdio: defer sdio irq handling when host suspended Kevin Liu
2012-12-21 10:26   ` Ulf Hansson
2012-12-21 12:59     ` Kevin Liu
2013-01-07 10:59       ` Ulf Hansson
2013-01-07 11:16         ` Kevin Liu
2013-01-07 12:01           ` Ulf Hansson
2013-01-07 16:04             ` Kevin Liu
2013-01-07 16:38               ` Ulf Hansson
2013-01-07 17:01                 ` Kevin Liu
2012-12-19 12:12 ` [PATCH 2/2] mmc: sdio: handle interrupt corner case during suspend Kevin Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.