2013/1/8 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: > On 7 January 2013 17:04, Kevin Liu <keyuan.liu@xxxxxxxxx> wrote: >> 2013/1/7 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: >>> On 7 January 2013 12:16, Kevin Liu <keyuan.liu@xxxxxxxxx> wrote: >>>> 2013/1/7 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: >>>>> On 21 December 2012 13:59, Kevin Liu <keyuan.liu@xxxxxxxxx> wrote: >>>>>> 2012/12/21 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: >>>>>>> On 19 December 2012 13:12, Kevin Liu <kliu5@xxxxxxxxxxx> 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@xxxxxxxxxxx> >>>>>>>> Signed-off-by: Kevin Liu <kliu5@xxxxxxxxxxx> >>>>>>>> --- >>>>>>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
