linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci: disable interrupt before free_irq
@ 2013-01-05  9:18 Kevin Liu
  2013-01-14 19:41 ` Chris Ball
  0 siblings, 1 reply; 2+ messages in thread
From: Kevin Liu @ 2013-01-05  9:18 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, Zhangfei Gao, Haojian Zhuang, Chao Xie,
	key

Current code missed disabling interrupts before free irq which is shared.

Notice below comments for function free_irq (kernel/irq/manage.c):
On a shared IRQ the caller must ensure the interrupt is disabled
on the card it drives before calling this function.

Original code has below issue during suspend/resume when multiple SD hosts
share the same IRQ:
1. Assume there are two hosts (host1 for emmc while host2 for sd) share
the same mmc irq.
2. When system suspend, host2 will be suspended before host1.
So the sequence is below:
	step1: irq handler for host2 removed ->
	step2: irq handler for host1 removed and irq disabled ->
	... system suspended ...
	... system resumed ...
	step3: irq enabled and the irq handler for host1 restored ->
	step4: irq handler for host2 restored
3. So there is the buggy time slot that the irq is enabled but the irq handler
for host2 is removed. Then host2 interrupt can be triggered but can't be handled
at that moment.

Signed-off-by: Jialing Fu <jlfu@marvell.com>
Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/host/sdhci.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6f0bfc0..089182e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2484,6 +2484,7 @@ int sdhci_suspend_host(struct sdhci_host *host)
 		return ret;
 	}
 
+	sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
 	free_irq(host->irq, host);
 
 	return ret;
@@ -3139,6 +3140,7 @@ int sdhci_add_host(struct sdhci_host *host)
 #ifdef SDHCI_USE_LEDS_CLASS
 reset:
 	sdhci_reset(host, SDHCI_RESET_ALL);
+	sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
 	free_irq(host->irq, host);
 #endif
 untasklet:
@@ -3181,6 +3183,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 	if (!dead)
 		sdhci_reset(host, SDHCI_RESET_ALL);
 
+	sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
 	free_irq(host->irq, host);
 
 	del_timer_sync(&host->timer);
-- 
1.7.9.5


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

* Re: [PATCH] mmc: sdhci: disable interrupt before free_irq
  2013-01-05  9:18 [PATCH] mmc: sdhci: disable interrupt before free_irq Kevin Liu
@ 2013-01-14 19:41 ` Chris Ball
  0 siblings, 0 replies; 2+ messages in thread
From: Chris Ball @ 2013-01-14 19:41 UTC (permalink / raw)
  To: Kevin Liu
  Cc: linux-mmc, 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, Zhangfei Gao,
	Haojian Zhuang, Chao

Hi,

On Sat, Jan 05 2013, Kevin Liu wrote:
> Current code missed disabling interrupts before free irq which is shared.
>
> Notice below comments for function free_irq (kernel/irq/manage.c):
> On a shared IRQ the caller must ensure the interrupt is disabled
> on the card it drives before calling this function.
>
> Original code has below issue during suspend/resume when multiple SD hosts
> share the same IRQ:
> 1. Assume there are two hosts (host1 for emmc while host2 for sd) share
> the same mmc irq.
> 2. When system suspend, host2 will be suspended before host1.
> So the sequence is below:
> 	step1: irq handler for host2 removed ->
> 	step2: irq handler for host1 removed and irq disabled ->
> 	... system suspended ...
> 	... system resumed ...
> 	step3: irq enabled and the irq handler for host1 restored ->
> 	step4: irq handler for host2 restored
> 3. So there is the buggy time slot that the irq is enabled but the irq handler
> for host2 is removed. Then host2 interrupt can be triggered but can't be handled
> at that moment.
>
> Signed-off-by: Jialing Fu <jlfu@marvell.com>
> Signed-off-by: Kevin Liu <kliu5@marvell.com>

Thanks, pushed to mmc-next for 3.9.

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

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

end of thread, other threads:[~2013-01-14 19:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-05  9:18 [PATCH] mmc: sdhci: disable interrupt before free_irq Kevin Liu
2013-01-14 19:41 ` Chris Ball

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