All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
@ 2020-04-12  9:03 Adrian Hunter
  2020-04-12  9:03 ` [PATCH 1/5] mmc: sdhci: Add helpers for the auto-CMD23 flag Adrian Hunter
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Adrian Hunter @ 2020-04-12  9:03 UTC (permalink / raw)
  To: Ulf Hansson, Baolin Wang; +Cc: linux-mmc

Hi

Here are some patches to reduce maximum time under spinlock in
sdhci_send_command(), but also pave the way for an atomic request
function.

I haven't tried it, but with these patches, something like below
should work.



static int sdhci_atomic_request(struct mmc_host *mmc,
				struct mmc_request *mrq)
{
	struct sdhci_host *host = mmc_priv(mmc);
	struct mmc_command *cmd;
	unsigned long flags;
	int ret = 0;

	spin_lock_irqsave(&host->lock, flags);

	if (sdhci_present_error(host, mrq->cmd, true))
		goto out_finish;

	cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;

	if (sdhci_send_command(host, cmd))
		sdhci_led_activate(host);
	else
		ret = -EBUSY;

	spin_unlock_irqrestore(&host->lock, flags);

	return ret;

out_finish:
	sdhci_finish_mrq(host, mrq);
	spin_unlock_irqrestore(&host->lock, flags);
	return 0;
}



Adrian Hunter (5):
      mmc: sdhci: Add helpers for the auto-CMD23 flag
      mmc: sdhci: Stop exporting sdhci_send_command()
      mmc: sdhci: Remove unneeded forward declaration of sdhci_finish_data()
      mmc: sdhci: Tidy sdhci_request() a bit
      mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()

 drivers/mmc/host/sdhci.c | 182 +++++++++++++++++++++++++++++++++++------------
 drivers/mmc/host/sdhci.h |   2 +-
 2 files changed, 139 insertions(+), 45 deletions(-)



Regards
Adrian

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

* [PATCH 1/5] mmc: sdhci: Add helpers for the auto-CMD23 flag
  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 ` Adrian Hunter
  2020-04-12  9:03 ` [PATCH 2/5] mmc: sdhci: Stop exporting sdhci_send_command() Adrian Hunter
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2020-04-12  9:03 UTC (permalink / raw)
  To: Ulf Hansson, Baolin Wang; +Cc: linux-mmc

Add 2 helper functions to make the use of the auto-CMD23 flag more
readable.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3f716466fcfd..07548e5efd1f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1350,13 +1350,25 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
 	       !mrq->cap_cmd_during_tfr;
 }
 
+static inline bool sdhci_auto_cmd23(struct sdhci_host *host,
+				    struct mmc_request *mrq)
+{
+	return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
+}
+
+static inline bool sdhci_manual_cmd23(struct sdhci_host *host,
+				      struct mmc_request *mrq)
+{
+	return mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23);
+}
+
 static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
 					 struct mmc_command *cmd,
 					 u16 *mode)
 {
 	bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) &&
 			 (cmd->opcode != SD_IO_RW_EXTENDED);
-	bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
+	bool use_cmd23 = sdhci_auto_cmd23(host, cmd->mrq);
 	u16 ctrl2;
 
 	/*
@@ -1416,7 +1428,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
 		mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
 		sdhci_auto_cmd_select(host, cmd, &mode);
-		if (cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23))
+		if (sdhci_auto_cmd23(host, cmd->mrq))
 			sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
 	}
 
@@ -2054,7 +2066,7 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		mrq->cmd->error = -ENOMEDIUM;
 		sdhci_finish_mrq(host, mrq);
 	} else {
-		if (mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23))
+		if (sdhci_manual_cmd23(host, mrq))
 			sdhci_send_command(host, mrq->sbc);
 		else
 			sdhci_send_command(host, mrq->cmd);
-- 
2.17.1


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

* [PATCH 2/5] mmc: sdhci: Stop exporting sdhci_send_command()
  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 ` Adrian Hunter
  2020-04-12  9:03 ` [PATCH 3/5] mmc: sdhci: Remove unneeded forward declaration of sdhci_finish_data() Adrian Hunter
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2020-04-12  9:03 UTC (permalink / raw)
  To: Ulf Hansson, Baolin Wang; +Cc: linux-mmc

sdhci_send_command() has not been used outside of sdhci.c for many
years. Stop exporting it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 5 +++--
 drivers/mmc/host/sdhci.h | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 07548e5efd1f..429e5b80f2fb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -52,6 +52,8 @@ static void sdhci_finish_data(struct sdhci_host *);
 
 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);
+
 void sdhci_dumpregs(struct sdhci_host *host)
 {
 	SDHCI_DUMP("============ SDHCI REGISTER DUMP ===========\n");
@@ -1558,7 +1560,7 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	}
 }
 
-void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
+static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	int flags;
 	u32 mask;
@@ -1658,7 +1660,6 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
 }
-EXPORT_SYMBOL_GPL(sdhci_send_command);
 
 static void sdhci_read_rsp_136(struct sdhci_host *host, struct mmc_command *cmd)
 {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 79dffbb731d3..47324feeba86 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -757,7 +757,6 @@ void sdhci_cleanup_host(struct sdhci_host *host);
 int __sdhci_add_host(struct sdhci_host *host);
 int sdhci_add_host(struct sdhci_host *host);
 void sdhci_remove_host(struct sdhci_host *host, int dead);
-void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd);
 
 static inline void sdhci_read_caps(struct sdhci_host *host)
 {
-- 
2.17.1


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

* [PATCH 3/5] mmc: sdhci: Remove unneeded forward declaration of sdhci_finish_data()
  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 ` Adrian Hunter
  2020-04-12  9:03 ` [PATCH 4/5] mmc: sdhci: Tidy sdhci_request() a bit Adrian Hunter
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2020-04-12  9:03 UTC (permalink / raw)
  To: Ulf Hansson, Baolin Wang; +Cc: linux-mmc

sdhci_finish_data() is defined before it is referenced, so forward
declaration is not necessary.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 429e5b80f2fb..27be2152553f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -48,8 +48,6 @@
 static unsigned int debug_quirks = 0;
 static unsigned int debug_quirks2;
 
-static void sdhci_finish_data(struct sdhci_host *);
-
 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);
-- 
2.17.1


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

* [PATCH 4/5] mmc: sdhci: Tidy sdhci_request() a bit
  2020-04-12  9:03 [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
                   ` (2 preceding siblings ...)
  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 ` Adrian Hunter
  2020-04-12  9:03 ` [PATCH 5/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2020-04-12  9:03 UTC (permalink / raw)
  To: Ulf Hansson, Baolin Wang; +Cc: linux-mmc

In preparation for further changes, tidy sdhci_request() a bit.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 27be2152553f..b9ddc8edffe2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1659,6 +1659,17 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
 }
 
+static bool sdhci_present_error(struct sdhci_host *host,
+				struct mmc_command *cmd, bool present)
+{
+	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
+		cmd->error = -ENOMEDIUM;
+		return true;
+	}
+
+	return false;
+}
+
 static void sdhci_read_rsp_136(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	int i, reg;
@@ -2048,11 +2059,10 @@ EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);
 
 void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
-	struct sdhci_host *host;
-	int present;
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct mmc_command *cmd;
 	unsigned long flags;
-
-	host = mmc_priv(mmc);
+	bool present;
 
 	/* Firstly check card presence */
 	present = mmc->ops->get_cd(mmc);
@@ -2061,16 +2071,19 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	sdhci_led_activate(host);
 
-	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
-		mrq->cmd->error = -ENOMEDIUM;
-		sdhci_finish_mrq(host, mrq);
-	} else {
-		if (sdhci_manual_cmd23(host, mrq))
-			sdhci_send_command(host, mrq->sbc);
-		else
-			sdhci_send_command(host, mrq->cmd);
-	}
+	if (sdhci_present_error(host, mrq->cmd, present))
+		goto out_finish;
+
+	cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
+
+	sdhci_send_command(host, cmd);
+
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	return;
 
+out_finish:
+	sdhci_finish_mrq(host, mrq);
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 EXPORT_SYMBOL_GPL(sdhci_request);
-- 
2.17.1


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

* [PATCH 5/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
  2020-04-12  9:03 [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
                   ` (3 preceding siblings ...)
  2020-04-12  9:03 ` [PATCH 4/5] mmc: sdhci: Tidy sdhci_request() a bit Adrian Hunter
@ 2020-04-12  9:03 ` Adrian Hunter
  2020-04-13  2:46 ` [PATCH 0/5] " Baolin Wang
  2020-04-17 11:29 ` Ulf Hansson
  6 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2020-04-12  9:03 UTC (permalink / raw)
  To: Ulf Hansson, Baolin Wang; +Cc: linux-mmc

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


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

* Re: [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
  2020-04-12  9:03 [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
                   ` (4 preceding siblings ...)
  2020-04-12  9:03 ` [PATCH 5/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
@ 2020-04-13  2:46 ` Baolin Wang
  2020-04-17 11:29 ` Ulf Hansson
  6 siblings, 0 replies; 8+ messages in thread
From: Baolin Wang @ 2020-04-13  2:46 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Ulf Hansson, linux-mmc

Hi Adrian,

On Sun, Apr 12, 2020 at 5:04 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Hi
>
> Here are some patches to reduce maximum time under spinlock in
> sdhci_send_command(), but also pave the way for an atomic request
> function.
>
> I haven't tried it, but with these patches, something like below
> should work.
>
>
>
> static int sdhci_atomic_request(struct mmc_host *mmc,
>                                 struct mmc_request *mrq)
> {
>         struct sdhci_host *host = mmc_priv(mmc);
>         struct mmc_command *cmd;
>         unsigned long flags;
>         int ret = 0;
>
>         spin_lock_irqsave(&host->lock, flags);
>
>         if (sdhci_present_error(host, mrq->cmd, true))
>                 goto out_finish;
>
>         cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
>
>         if (sdhci_send_command(host, cmd))
>                 sdhci_led_activate(host);
>         else
>                 ret = -EBUSY;
>
>         spin_unlock_irqrestore(&host->lock, flags);
>
>         return ret;
>
> out_finish:
>         sdhci_finish_mrq(host, mrq);
>         spin_unlock_irqrestore(&host->lock, flags);
>         return 0;
> }

Yes, this looks good to me, and I've tested on my platform with
re-implementing sdhci_request_atomic() based on your patch set, it
worked well. Thanks for your help.
Tested-by: Baolin Wang <baolin.wang7@gmail.com>

> Adrian Hunter (5):
>       mmc: sdhci: Add helpers for the auto-CMD23 flag
>       mmc: sdhci: Stop exporting sdhci_send_command()
>       mmc: sdhci: Remove unneeded forward declaration of sdhci_finish_data()
>       mmc: sdhci: Tidy sdhci_request() a bit
>       mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
>
>  drivers/mmc/host/sdhci.c | 182 +++++++++++++++++++++++++++++++++++------------
>  drivers/mmc/host/sdhci.h |   2 +-
>  2 files changed, 139 insertions(+), 45 deletions(-)
>
>
>
> Regards
> Adrian



-- 
Baolin Wang

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

* Re: [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
  2020-04-12  9:03 [PATCH 0/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
                   ` (5 preceding siblings ...)
  2020-04-13  2:46 ` [PATCH 0/5] " Baolin Wang
@ 2020-04-17 11:29 ` Ulf Hansson
  6 siblings, 0 replies; 8+ messages in thread
From: Ulf Hansson @ 2020-04-17 11:29 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Baolin Wang, linux-mmc

On Sun, 12 Apr 2020 at 11:04, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Hi
>
> Here are some patches to reduce maximum time under spinlock in
> sdhci_send_command(), but also pave the way for an atomic request
> function.
>
> I haven't tried it, but with these patches, something like below
> should work.
>
>
>
> static int sdhci_atomic_request(struct mmc_host *mmc,
>                                 struct mmc_request *mrq)
> {
>         struct sdhci_host *host = mmc_priv(mmc);
>         struct mmc_command *cmd;
>         unsigned long flags;
>         int ret = 0;
>
>         spin_lock_irqsave(&host->lock, flags);
>
>         if (sdhci_present_error(host, mrq->cmd, true))
>                 goto out_finish;
>
>         cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
>
>         if (sdhci_send_command(host, cmd))
>                 sdhci_led_activate(host);
>         else
>                 ret = -EBUSY;
>
>         spin_unlock_irqrestore(&host->lock, flags);
>
>         return ret;
>
> out_finish:
>         sdhci_finish_mrq(host, mrq);
>         spin_unlock_irqrestore(&host->lock, flags);
>         return 0;
> }
>
>
>
> Adrian Hunter (5):
>       mmc: sdhci: Add helpers for the auto-CMD23 flag
>       mmc: sdhci: Stop exporting sdhci_send_command()
>       mmc: sdhci: Remove unneeded forward declaration of sdhci_finish_data()
>       mmc: sdhci: Tidy sdhci_request() a bit
>       mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command()
>
>  drivers/mmc/host/sdhci.c | 182 +++++++++++++++++++++++++++++++++++------------
>  drivers/mmc/host/sdhci.h |   2 +-
>  2 files changed, 139 insertions(+), 45 deletions(-)
>

Applied for next, thanks!

Kind regards
Uffe

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

end of thread, other threads:[~2020-04-17 11:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 5/5] mmc: sdhci: Reduce maximum time under spinlock in sdhci_send_command() Adrian Hunter
2020-04-13  2:46 ` [PATCH 0/5] " Baolin Wang
2020-04-17 11:29 ` Ulf Hansson

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.