linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jerome Brunet <jbrunet@baylibre.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Jerome Brunet <jbrunet@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-mmc@vger.kernel.org, Brad Harper <bjharper@gmail.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: [PATCH] mmc: meson-gx: remove IRQF_ONESHOT
Date: Fri,  2 Oct 2020 18:49:15 +0200	[thread overview]
Message-ID: <20201002164915.938217-1-jbrunet@baylibre.com> (raw)

IRQF_ONESHOT was added to this driver to make sure the irq was not enabled
again until the thread part of the irq had finished doing its job.

Doing so upsets RT because, under RT, the hardirq part of the irq handler
is not migrated to a thread if the irq is claimed with IRQF_ONESHOT.
In this case, it has been reported to eventually trigger a deadlock with
the led subsystem.

Preventing RT from doing this migration was certainly not the intent, the
description of IRQF_ONESHOT does not really reflect this constraint:

 > IRQF_ONESHOT - Interrupt is not reenabled after the hardirq handler finished.
 >              Used by threaded interrupts which need to keep the
 >              irq line disabled until the threaded handler has been run.

This is exactly what this driver was trying to acheive so I'm still a bit
confused whether this is a driver or an RT issue.

Anyway, this can be solved driver side by manually disabling the IRQs
instead of the relying on the IRQF_ONESHOT. IRQF_ONESHOT may then be removed
while still making sure the irq won't trigger until the threaded part of
the handler is done.

Fixes: eb4d81127746 ("mmc: meson-gx: correct irq flag")
Reported-by: Brad Harper <bjharper@gmail.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 47 ++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 08a3b1c05acb..effc356db904 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -101,8 +101,7 @@
 #define   IRQ_RESP_STATUS BIT(14)
 #define   IRQ_SDIO BIT(15)
 #define   IRQ_EN_MASK \
-	(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN | IRQ_RESP_STATUS |\
-	 IRQ_SDIO)
+	(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN)
 
 #define SD_EMMC_CMD_CFG 0x50
 #define SD_EMMC_CMD_ARG 0x54
@@ -170,6 +169,7 @@ struct meson_host {
 	dma_addr_t descs_dma_addr;
 
 	int irq;
+	u32 irq_en;
 
 	bool vqmmc_enabled;
 };
@@ -842,22 +842,24 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	struct meson_host *host = dev_id;
 	struct mmc_command *cmd;
 	struct mmc_data *data;
-	u32 irq_en, status, raw_status;
+	u32  status, raw_status;
 	irqreturn_t ret = IRQ_NONE;
 
-	irq_en = readl(host->regs + SD_EMMC_IRQ_EN);
+	/* Disable irqs */
+	writel(0, host->regs + SD_EMMC_IRQ_EN);
+
 	raw_status = readl(host->regs + SD_EMMC_STATUS);
-	status = raw_status & irq_en;
+	status = raw_status & host->irq_en;
 
 	if (!status) {
 		dev_dbg(host->dev,
 			"Unexpected IRQ! irq_en 0x%08x - status 0x%08x\n",
-			 irq_en, raw_status);
-		return IRQ_NONE;
+			 host->irq_en, raw_status);
+		goto none;
 	}
 
 	if (WARN_ON(!host) || WARN_ON(!host->cmd))
-		return IRQ_NONE;
+		goto none;
 
 	/* ack all raised interrupts */
 	writel(status, host->regs + SD_EMMC_STATUS);
@@ -908,6 +910,11 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	if (ret == IRQ_HANDLED)
 		meson_mmc_request_done(host->mmc, cmd->mrq);
 
+none:
+	/* Enable the irq again if the thread will not run */
+	if (ret != IRQ_WAKE_THREAD)
+		writel(host->irq_en, host->regs + SD_EMMC_IRQ_EN);
+
 	return ret;
 }
 
@@ -934,15 +941,17 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	struct mmc_command *next_cmd, *cmd = host->cmd;
 	struct mmc_data *data;
 	unsigned int xfer_bytes;
+	int ret = IRQ_HANDLED;
 
-	if (WARN_ON(!cmd))
-		return IRQ_NONE;
+	if (WARN_ON(!cmd)) {
+		ret = IRQ_NONE;
+		goto out;
+	}
 
 	if (cmd->error) {
 		meson_mmc_wait_desc_stop(host);
 		meson_mmc_request_done(host->mmc, cmd->mrq);
-
-		return IRQ_HANDLED;
+		goto out;
 	}
 
 	data = cmd->data;
@@ -959,7 +968,10 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	else
 		meson_mmc_request_done(host->mmc, cmd->mrq);
 
-	return IRQ_HANDLED;
+out:
+	/* Re-enable the irqs */
+	writel(host->irq_en, host->regs + SD_EMMC_IRQ_EN);
+	return ret;
 }
 
 /*
@@ -1133,13 +1145,12 @@ static int meson_mmc_probe(struct platform_device *pdev)
 
 	/* clear, ack and enable interrupts */
 	writel(0, host->regs + SD_EMMC_IRQ_EN);
-	writel(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN,
-	       host->regs + SD_EMMC_STATUS);
-	writel(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN,
-	       host->regs + SD_EMMC_IRQ_EN);
+	host->irq_en = IRQ_EN_MASK;
+	writel(host->irq_en, host->regs + SD_EMMC_STATUS);
+	writel(host->irq_en, host->regs + SD_EMMC_IRQ_EN);
 
 	ret = request_threaded_irq(host->irq, meson_mmc_irq,
-				   meson_mmc_irq_thread, IRQF_ONESHOT,
+				   meson_mmc_irq_thread, 0,
 				   dev_name(&pdev->dev), host);
 	if (ret)
 		goto err_init_clk;
-- 
2.25.4


             reply	other threads:[~2020-10-02 16:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 16:49 Jerome Brunet [this message]
2020-10-05  8:22 ` [PATCH] mmc: meson-gx: remove IRQF_ONESHOT Ulf Hansson
2020-10-05  8:55   ` Thomas Gleixner
2020-10-05 12:31     ` Jerome Brunet
2020-10-06 12:43     ` Thomas Gleixner
2020-10-06 13:45       ` Brad Harper
2020-10-06 15:33         ` Thomas Gleixner
2020-10-07 11:32           ` Jerome Brunet
2020-10-08  5:11             ` Brad Harper
2020-10-08  9:08           ` Ulf Hansson
2020-11-10 15:04             ` Jerome Brunet
2020-11-11 10:47               ` Ulf Hansson
2020-11-13 15:25                 ` Jerome Brunet

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=20201002164915.938217-1-jbrunet@baylibre.com \
    --to=jbrunet@baylibre.com \
    --cc=bigeasy@linutronix.de \
    --cc=bjharper@gmail.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=ulf.hansson@linaro.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).