From: Adrian Hunter <adrian.hunter@intel.com>
To: Ulf Hansson <ulf.hansson@linaro.org>,
Baolin Wang <baolin.wang7@gmail.com>
Cc: linux-mmc <linux-mmc@vger.kernel.org>
Subject: [PATCH 5/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
Date: Sun, 12 Apr 2020 12:03:49 +0300 [thread overview]
Message-ID: <20200412090349.1607-6-adrian.hunter@intel.com> (raw)
In-Reply-To: <20200412090349.1607-1-adrian.hunter@intel.com>
Spending time under spinlock increases IRQ latencies and also
response times because preemption is disabled.
sdhci_send_command() waits up to 10 ms under spinlock for inhibit bits
to clear. In general inhibit bits will not be set, but there may be
corner cases, especially in the face of errors, where waiting helps.
There might also be dysfunctional hardware that needs the waiting. So
retain the legacy behaviour but do not wait for inhibit bits while under
spinlock. Instead adjust the logic to enable waiting while not under
spinlock. That is mostly straight forward, but in the interrupt handler
it requires deferring an "inhibited" command to the IRQ thread where
sleeping is allowed.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci.c | 126 ++++++++++++++++++++++++++++++---------
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b9ddc8edffe2..10b9570f48aa 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -50,7 +50,7 @@ static unsigned int debug_quirks2;
static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
-static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd);
+static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd);
void sdhci_dumpregs(struct sdhci_host *host)
{
@@ -1478,6 +1478,9 @@ static void __sdhci_finish_mrq(struct sdhci_host *host, struct mmc_request *mrq)
if (host->data_cmd && host->data_cmd->mrq == mrq)
host->data_cmd = NULL;
+ if (host->deferred_cmd && host->deferred_cmd->mrq == mrq)
+ host->deferred_cmd = NULL;
+
if (host->data && host->data->mrq == mrq)
host->data = NULL;
@@ -1499,7 +1502,7 @@ static void sdhci_finish_mrq(struct sdhci_host *host, struct mmc_request *mrq)
queue_work(host->complete_wq, &host->complete_work);
}
-static void sdhci_finish_data(struct sdhci_host *host)
+static void __sdhci_finish_data(struct sdhci_host *host, bool sw_data_timeout)
{
struct mmc_command *data_cmd = host->data_cmd;
struct mmc_data *data = host->data;
@@ -1551,14 +1554,31 @@ static void sdhci_finish_data(struct sdhci_host *host)
} else {
/* Avoid triggering warning in sdhci_send_command() */
host->cmd = NULL;
- sdhci_send_command(host, data->stop);
+ if (!sdhci_send_command(host, data->stop)) {
+ if (sw_data_timeout) {
+ /*
+ * This is anyway a sw data timeout, so
+ * give up now.
+ */
+ data->stop->error = -EIO;
+ __sdhci_finish_mrq(host, data->mrq);
+ } else {
+ WARN_ON(host->deferred_cmd);
+ host->deferred_cmd = data->stop;
+ }
+ }
}
} else {
__sdhci_finish_mrq(host, data->mrq);
}
}
-static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
+static void sdhci_finish_data(struct sdhci_host *host)
+{
+ __sdhci_finish_data(host, false);
+}
+
+static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
{
int flags;
u32 mask;
@@ -1573,9 +1593,6 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
cmd->opcode == MMC_STOP_TRANSMISSION)
cmd->flags |= MMC_RSP_BUSY;
- /* Wait max 10 ms */
- timeout = 10;
-
mask = SDHCI_CMD_INHIBIT;
if (sdhci_data_line_cmd(cmd))
mask |= SDHCI_DATA_INHIBIT;
@@ -1585,18 +1602,8 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
if (cmd->mrq->data && (cmd == cmd->mrq->data->stop))
mask &= ~SDHCI_DATA_INHIBIT;
- while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
- if (timeout == 0) {
- pr_err("%s: Controller never released inhibit bit(s).\n",
- mmc_hostname(host->mmc));
- sdhci_dumpregs(host);
- cmd->error = -EIO;
- sdhci_finish_mrq(host, cmd->mrq);
- return;
- }
- timeout--;
- mdelay(1);
- }
+ if (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask)
+ return false;
host->cmd = cmd;
host->data_timeout = 0;
@@ -1618,11 +1625,13 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
sdhci_set_transfer_mode(host, cmd);
if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
- pr_err("%s: Unsupported response type!\n",
- mmc_hostname(host->mmc));
- cmd->error = -EINVAL;
- sdhci_finish_mrq(host, cmd->mrq);
- return;
+ WARN_ONCE(1, "Unsupported response type!\n");
+ /*
+ * This does not happen in practice because 136-bit response
+ * commands never have busy waiting, so rather than complicate
+ * the error path, just remove busy waiting and continue.
+ */
+ cmd->flags &= ~MMC_RSP_BUSY;
}
if (!(cmd->flags & MMC_RSP_PRESENT))
@@ -1657,6 +1666,8 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
sdhci_external_dma_pre_transfer(host, cmd);
sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
+
+ return true;
}
static bool sdhci_present_error(struct sdhci_host *host,
@@ -1670,6 +1681,47 @@ static bool sdhci_present_error(struct sdhci_host *host,
return false;
}
+static bool sdhci_send_command_retry(struct sdhci_host *host,
+ struct mmc_command *cmd,
+ unsigned long flags)
+ __releases(host->lock)
+ __acquires(host->lock)
+{
+ struct mmc_command *deferred_cmd = host->deferred_cmd;
+ int timeout = 10; /* Approx. 10 ms */
+ bool present;
+
+ while (!sdhci_send_command(host, cmd)) {
+ if (!timeout--) {
+ pr_err("%s: Controller never released inhibit bit(s).\n",
+ mmc_hostname(host->mmc));
+ sdhci_dumpregs(host);
+ cmd->error = -EIO;
+ return false;
+ }
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ usleep_range(1000, 1250);
+
+ present = host->mmc->ops->get_cd(host->mmc);
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ /* A deferred command might disappear, handle that */
+ if (cmd == deferred_cmd && cmd != host->deferred_cmd)
+ return true;
+
+ if (sdhci_present_error(host, cmd, present))
+ return false;
+ }
+
+ if (cmd == host->deferred_cmd)
+ host->deferred_cmd = NULL;
+
+ return true;
+}
+
static void sdhci_read_rsp_136(struct sdhci_host *host, struct mmc_command *cmd)
{
int i, reg;
@@ -1729,7 +1781,10 @@ static void sdhci_finish_command(struct sdhci_host *host)
/* Finished CMD23, now send actual command. */
if (cmd == cmd->mrq->sbc) {
- sdhci_send_command(host, cmd->mrq->cmd);
+ if (!sdhci_send_command(host, cmd->mrq->cmd)) {
+ WARN_ON(host->deferred_cmd);
+ host->deferred_cmd = cmd->mrq->cmd;
+ }
} else {
/* Processed actual command. */
@@ -2076,7 +2131,8 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
- sdhci_send_command(host, cmd);
+ if (!sdhci_send_command_retry(host, cmd, flags))
+ goto out_finish;
spin_unlock_irqrestore(&host->lock, flags);
@@ -2624,7 +2680,11 @@ void sdhci_send_tuning(struct sdhci_host *host, u32 opcode)
*/
sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
- sdhci_send_command(host, &cmd);
+ if (!sdhci_send_command_retry(host, &cmd, flags)) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ host->tuning_done = 0;
+ return;
+ }
host->cmd = NULL;
@@ -3042,7 +3102,7 @@ static void sdhci_timeout_data_timer(struct timer_list *t)
if (host->data) {
host->data->error = -ETIMEDOUT;
- sdhci_finish_data(host);
+ __sdhci_finish_data(host, true);
queue_work(host->complete_wq, &host->complete_work);
} else if (host->data_cmd) {
host->data_cmd->error = -ETIMEDOUT;
@@ -3414,6 +3474,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
}
}
out:
+ if (host->deferred_cmd)
+ result = IRQ_WAKE_THREAD;
+
spin_unlock(&host->lock);
/* Process mrqs ready for immediate completion */
@@ -3439,6 +3502,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
{
struct sdhci_host *host = dev_id;
+ struct mmc_command *cmd;
unsigned long flags;
u32 isr;
@@ -3446,8 +3510,14 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
;
spin_lock_irqsave(&host->lock, flags);
+
isr = host->thread_isr;
host->thread_isr = 0;
+
+ cmd = host->deferred_cmd;
+ if (cmd && !sdhci_send_command_retry(host, cmd, flags))
+ sdhci_finish_mrq(host, cmd->mrq);
+
spin_unlock_irqrestore(&host->lock, flags);
if (isr & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 47324feeba86..a7e469c00617 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -540,6 +540,7 @@ struct sdhci_host {
struct mmc_request *mrqs_done[SDHCI_MAX_MRQS]; /* Requests done */
struct mmc_command *cmd; /* Current command */
struct mmc_command *data_cmd; /* Current data command */
+ struct mmc_command *deferred_cmd; /* Deferred command */
struct mmc_data *data; /* Current data request */
unsigned int data_early:1; /* Data finished before cmd */
--
2.17.1
next prev parent reply other threads:[~2020-04-12 9:04 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-12 9:03 [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
2020-04-12 9:03 ` [PATCH 1/5] mmc: sdhci: Add helpers for the auto-CMD23 flag Adrian Hunter
2020-04-12 9:03 ` [PATCH 2/5] mmc: sdhci: Stop exporting sdhci_send_command() Adrian Hunter
2020-04-12 9:03 ` [PATCH 3/5] mmc: sdhci: Remove unneeded forward declaration of sdhci_finish_data() Adrian Hunter
2020-04-12 9:03 ` [PATCH 4/5] mmc: sdhci: Tidy sdhci_request() a bit Adrian Hunter
2020-04-12 9:03 ` Adrian Hunter [this message]
2020-04-13 2:46 ` [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Baolin Wang
2020-04-17 11:29 ` Ulf Hansson
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=20200412090349.1607-6-adrian.hunter@intel.com \
--to=adrian.hunter@intel.com \
--cc=baolin.wang7@gmail.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.