[RT PATCH] mmc: sdhci: don't provide hard irq handler

[RT PATCH] mmc: sdhci: don't provide hard irq handler

[Sebastian Andrzej Siewior]

the sdhci code provides both irq handlers: the primary and the thread
handler. Initially it was meant for the primary handler to be very
short.
The result is not that on -RT we have the primrary handler grabing locks
and this isn't really working. As a hack for now I just push both
handler into the threaded mode.

Reported-By: Michal Šmucr <msmucr@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
The "same thing" was reported against the iwlwifi driver
(request_threaded_irq(…, iwl_pcie_isr, iwl_pcie_irq_handler, …) and they
managed to rework it and not do anything that would break -RT in their
primary handler. Besides sdhci there are a few others drivers in the
same tree doing similar things.
I'm not sure what to do here in general. Motivating upstream maintainer
to rework their code or introducing IRQF_RT_SAFE and for others doing
the conversation like in the patch below.

Michal: This is untested but should fix the issue, reported.

 drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 023c2010cd75..bcde53774bc9 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2565,6 +2565,31 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
 	return isr ? IRQ_HANDLED : IRQ_NONE;
 }
 
+#ifdef CONFIG_PREEMPT_RT_BASE
+static irqreturn_t sdhci_rt_irq(int irq, void *dev_id)
+{
+	irqreturn_t ret;
+
+	local_bh_disable();
+	ret = sdhci_irq(irq, dev_id);
+	local_bh_enable();
+	if (ret == IRQ_WAKE_THREAD)
+		ret = sdhci_thread_irq(irq, dev_id);
+	return ret;
+}
+#endif
+
+static int sdhci_req_irq(struct sdhci_host *host)
+{
+#ifdef CONFIG_PREEMPT_RT_BASE
+	return request_threaded_irq(host->irq, NULL, sdhci_rt_irq,
+				    IRQF_SHARED, mmc_hostname(host->mmc), host);
+#else
+	return request_threaded_irq(host->irq, sdhci_irq, sdhci_thread_irq,
+				    IRQF_SHARED, mmc_hostname(host->mmc), host);
+#endif
+}
+
 /*****************************************************************************\
  *                                                                           *
  * Suspend/resume                                                            *
@@ -2632,9 +2657,7 @@ int sdhci_resume_host(struct sdhci_host *host)
 	}
 
 	if (!device_may_wakeup(mmc_dev(host->mmc))) {
-		ret = request_threaded_irq(host->irq, sdhci_irq,
-					   sdhci_thread_irq, IRQF_SHARED,
-					   mmc_hostname(host->mmc), host);
+		ret = sdhci_req_irq(host);
 		if (ret)
 			return ret;
 	} else {
@@ -3253,8 +3276,7 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	sdhci_init(host, 0);
 
-	ret = request_threaded_irq(host->irq, sdhci_irq, sdhci_thread_irq,
-				   IRQF_SHARED,	mmc_hostname(mmc), host);
+	ret = sdhci_req_irq(host);
 	if (ret) {
 		pr_err("%s: Failed to request IRQ %d: %d\n",
 		       mmc_hostname(mmc), host->irq, ret);
-- 
2.1.4

Re: [RT PATCH] mmc: sdhci: don't provide hard irq handler

[Michal Šmucr]

On 26.2.2015 12:29, Sebastian Andrzej Siewior wrote:
> the sdhci code provides both irq handlers: the primary and the thread
> handler. Initially it was meant for the primary handler to be very
> short.
> The result is not that on -RT we have the primrary handler grabing locks
> and this isn't really working. As a hack for now I just push both
> handler into the threaded mode.
>
> Reported-By: Michal Šmucr <msmucr@gmail.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> The "same thing" was reported against the iwlwifi driver
> (request_threaded_irq(…, iwl_pcie_isr, iwl_pcie_irq_handler, …) and they
> managed to rework it and not do anything that would break -RT in their
> primary handler. Besides sdhci there are a few others drivers in the
> same tree doing similar things.
> I'm not sure what to do here in general. Motivating upstream maintainer
> to rework their code or introducing IRQF_RT_SAFE and for others doing
> the conversation like in the patch below.
>
> Michal: This is untested but should fix the issue, reported.
>

Sebastian,

thank you very much for the patch and explanation of what's going on. It 
is very handy to know for possible similar issues with other modules. 
The trick with forced threaded handlers seems to be working well and I 
haven't been able to crash it again during all of my tests.

Michal