From: vaibhav.hiremath@linaro.org (Vaibhav Hiremath)
To: linux-arm-kernel@lists.infradead.org
Subject: sdhci: runtime suspend/resume on card insert/removal
Date: Thu, 10 Sep 2015 12:40:48 +0530 [thread overview]
Message-ID: <55F12CF8.50003@linaro.org> (raw)
Hi,
During my testing of SDHCI-PXAV3 driver on Marvell's pxa1928
based platform, I observed that runtime PM suspend/resume is having
issues with card insertion and removal.
Let me try to explain it using execution sequence -
During boot:
MMC SD card gets detected as expected.
[ 2.431012] mmc1: new high speed SDHC card at address 1234
[ 2.437235] mmcblk1: mmc1:1234 SA04G 3.63 GiB
[ 2.444841] mmcblk1: p1
Now after coming to the linux prompt, if card removal event occurs
then the call sequence is -
sdhci_irq() -->
-> sdhci_thread_irq(): host->thread_isr - 0x80
-> sdhci_card_event()
-> mmc_detect_change()
--> _mmc_detect_change()
---> mmc_sd_detect()
mmc_sd_remove()
mmc_remove_card()
mmc_bus_remove()
mmc_power_off()
mmc_set_initial_state()
sdhci_set_ios()
...
sdhci_pxav3_runtime_suspend()
sdhci_runtime_suspend_host()
Till here everything looks perfect :) (if I got it right)
Now on card insertion again, the expectation is, runtime resume should
get called as part of interrupt trigger from the SDHCI controller on
card insertion.
But what I am observing here is, no interrupt is generated, as it is
not enabled at all. And the reason being sdhci_runtime_suspend_host()
int sdhci_runtime_suspend_host(struct sdhci_host *host)
{
....
spin_lock_irqsave(&host->lock, flags);
host->ier &= SDHCI_INT_CARD_INT;
sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
spin_unlock_irqrestore(&host->lock, flags);
...
}
In my case, I see SDHCI_INT_CARD_INT is not ON and with above step
we are not ensuring that it is enabled either.
Also we are not enabling card insertion and removal interrupts.
I have done following change to the code -
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 418f381..3129292 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2783,9 +2783,12 @@ int sdhci_runtime_suspend_host(struct sdhci_host
*host)
mmc_retune_needed(host->mmc);
spin_lock_irqsave(&host->lock, flags);
- host->ier &= SDHCI_INT_CARD_INT;
+
+ host->flags |= SDHCI_SDIO_IRQ_ENABLED;
+ host->ier |= SDHCI_INT_CARD_INT;
sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+
spin_unlock_irqrestore(&host->lock, flags);
synchronize_hardirq(host->irq);
And things started working as expected.
The execution sequence of irq and sdhci_runtime_resume
both looks ok to me.
Card insertion event =
sdhci_irq() -->
-> sdhci_thread_irq: host->thread_isr - 40
--> sdhci_card_event()
--> mmc_detect_change()
---> _mmc_detect_change()
...
-> sdhci_pxav3_runtime_resume()
--> sdhci_do_set_ios:1438
I see card getting enumerated as expected -
[ 15.116098] mmc1: mmc_rescan_try_freq: trying to init card at 400000 Hz
[ 15.356473] mmc1: new high speed SDHC card at address 1234
[ 15.363962] mmcblk1: mmc1:1234 SA04G 3.63 GiB
[ 15.371043] sdhci_thread_irq:2642 host->thread_isr - 100
[ 15.376492] mmcblk1: p1
I believe (based on my understanding) the change I have done is right.
I am not sure how other platforms using sdhci.c working so far,
probably no runtime_pm support? Not sure though
Please let me know if I am missing something here.
And not, and everyone is ok with this, I will submit the patch fixing
this issue.
Thanks,
Vaibhav
next reply other threads:[~2015-09-10 7:10 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-10 7:10 Vaibhav Hiremath [this message]
2015-09-10 7:31 ` sdhci: runtime suspend/resume on card insert/removal Jisheng Zhang
2015-09-10 7:51 ` Vaibhav Hiremath
2015-09-10 7:57 ` Jisheng Zhang
2015-09-10 8:02 ` Russell King - ARM Linux
2015-09-10 8:04 ` Jisheng Zhang
2015-09-14 6:25 ` Vaibhav Hiremath
2015-09-14 6:28 ` Jisheng Zhang
2015-09-14 8:13 ` Vaibhav Hiremath
2015-09-14 8:18 ` Jisheng Zhang
2015-09-14 9:19 ` Vaibhav Hiremath
2015-09-14 10:15 ` Vaibhav Hiremath
2015-09-14 10:50 ` Russell King - ARM Linux
2015-09-14 11:00 ` Russell King - ARM Linux
2015-09-14 12:33 ` Vaibhav Hiremath
2015-09-14 12:49 ` Russell King - ARM Linux
2015-09-14 13:05 ` Vaibhav Hiremath
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55F12CF8.50003@linaro.org \
--to=vaibhav.hiremath@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).