LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v3 0/4] Introduce the request_atomic() for the host
@ 2020-03-19 10:54 Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 1/4] mmc: host: " Baolin Wang
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Baolin Wang @ 2020-03-19 10:54 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: orsonzhai, zhang.lyra, baolin.wang7, arnd, linux-mmc, linux-kernel

This patch set introduces a new request_atomic() interface for the
MMC host controller, which is used to submit a request to host in
the atomic context, such as in the irq hard handler, to reduce the
request latency.

Any comments are welcome. Thanks.

Changes from v2:
 - Return busy flag if encountering unusual card busy state
 instead of polling in interrupt context.
 - Add a work for HSQ to try again in non-atomic context if the host
 returns busy flag.

Changes from v1:
 - Re-split the changes to make them more clear suggested by Ulf.
 - Factor out the auto CMD23 checking into a separate function.

Baolin Wang (4):
  mmc: host: Introduce the request_atomic() for the host
  mmc: host: sdhci: Implement the request_atomic() API
  mmc: host: hsq: Handle an unusual case of returing busy
  mmc: host: sdhci-sprd: Implement the request_atomic() API

 drivers/mmc/host/mmc_hsq.c    | 40 +++++++++++++++++++++++++-
 drivers/mmc/host/mmc_hsq.h    |  1 +
 drivers/mmc/host/sdhci-sprd.c | 23 +++++++++++++--
 drivers/mmc/host/sdhci.c      | 65 ++++++++++++++++++++++++++++++++-----------
 drivers/mmc/host/sdhci.h      |  4 ++-
 include/linux/mmc/host.h      |  3 ++
 6 files changed, 114 insertions(+), 22 deletions(-)

-- 
1.9.1


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

* [PATCH v3 1/4] mmc: host: Introduce the request_atomic() for the host
  2020-03-19 10:54 [PATCH v3 0/4] Introduce the request_atomic() for the host Baolin Wang
@ 2020-03-19 10:54 ` Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 2/4] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Baolin Wang @ 2020-03-19 10:54 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: orsonzhai, zhang.lyra, baolin.wang7, arnd, linux-mmc, linux-kernel

The SD host controller can process one request in the atomic context if
the card is nonremovable, which means we can submit next request in the
irq hard handler when using the MMC host software queue to reduce the
latency. Thus this patch adds a new API request_atomic() for the host
controller, as well as adding support for host software queue to submit
a request by the new request_atomic() API.

Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
 drivers/mmc/host/mmc_hsq.c | 5 ++++-
 include/linux/mmc/host.h   | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmc_hsq.c b/drivers/mmc/host/mmc_hsq.c
index 59d2776..fdbaa98 100644
--- a/drivers/mmc/host/mmc_hsq.c
+++ b/drivers/mmc/host/mmc_hsq.c
@@ -41,7 +41,10 @@ static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
 
 	spin_unlock_irqrestore(&hsq->lock, flags);
 
-	mmc->ops->request(mmc, hsq->mrq);
+	if (mmc->ops->request_atomic)
+		mmc->ops->request_atomic(mmc, hsq->mrq);
+	else
+		mmc->ops->request(mmc, hsq->mrq);
 }
 
 static void mmc_hsq_update_next_tag(struct mmc_hsq *hsq, int remains)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 562ed06..787a7b5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -92,6 +92,9 @@ struct mmc_host_ops {
 			    int err);
 	void	(*pre_req)(struct mmc_host *host, struct mmc_request *req);
 	void	(*request)(struct mmc_host *host, struct mmc_request *req);
+	/* Submit one request to host in atomic context. */
+	int	(*request_atomic)(struct mmc_host *host,
+				  struct mmc_request *req);
 
 	/*
 	 * Avoid calling the next three functions too often or in a "fast
-- 
1.9.1


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

* [PATCH v3 2/4] mmc: host: sdhci: Implement the request_atomic() API
  2020-03-19 10:54 [PATCH v3 0/4] Introduce the request_atomic() for the host Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 1/4] mmc: host: " Baolin Wang
@ 2020-03-19 10:54 ` Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 4/4] mmc: host: sdhci-sprd: Implement the request_atomic() API Baolin Wang
  3 siblings, 0 replies; 7+ messages in thread
From: Baolin Wang @ 2020-03-19 10:54 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: orsonzhai, zhang.lyra, baolin.wang7, arnd, linux-mmc, linux-kernel

Implement the request_atomic() ops for the sdhci driver to process
one request in the atomic context if the card is nonremovable.

Moreover, add a boolean parameter for sdhci_send_command() function
to indicate if need poll the inhibit bits, which can avoid
time-consuming polling in interrupt context by returning BUSY if
we met this unusual case through request_atomic() API.

Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
 drivers/mmc/host/sdhci.c | 65 +++++++++++++++++++++++++++++++++++-------------
 drivers/mmc/host/sdhci.h |  4 ++-
 2 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9c37451..8f4b985 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1538,14 +1538,15 @@ 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);
+			sdhci_send_command(host, data->stop, true);
 		}
 	} else {
 		__sdhci_finish_mrq(host, data->mrq);
 	}
 }
 
-void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
+int sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd,
+		       bool polling)
 {
 	int flags;
 	u32 mask;
@@ -1573,13 +1574,26 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mask &= ~SDHCI_DATA_INHIBIT;
 
 	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
-		if (timeout == 0) {
+		if (!polling) {
+			pr_err("%s: Controller has not released inhibit bit(s), try again.\n",
+			       mmc_hostname(host->mmc));
+
+			/*
+			 * HSQ may send a command in interrupt context without
+			 * polling the busy signaling, which means we should
+			 * return BUSY if controller has not released inhibit
+			 * bits to allow HSQ trying to send request again in
+			 * non-atomic context. so we should not finish this
+			 * request here.
+			 */
+			return -EBUSY;
+		} else 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;
+			return -EIO;
 		}
 		timeout--;
 		mdelay(1);
@@ -1609,7 +1623,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 			mmc_hostname(host->mmc));
 		cmd->error = -EINVAL;
 		sdhci_finish_mrq(host, cmd->mrq);
-		return;
+		return -EINVAL;
 	}
 
 	if (!(cmd->flags & MMC_RSP_PRESENT))
@@ -1644,6 +1658,8 @@ 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 0;
 }
 EXPORT_SYMBOL_GPL(sdhci_send_command);
 
@@ -1706,7 +1722,7 @@ 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);
+		sdhci_send_command(host, cmd->mrq->cmd, true);
 	} else {
 
 		/* Processed actual command. */
@@ -2016,16 +2032,12 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
  *                                                                           *
 \*****************************************************************************/
 
-void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
+static int sdhci_start_request(struct mmc_host *mmc, struct mmc_request *mrq,
+				int present, bool polling)
 {
-	struct sdhci_host *host;
-	int present;
+	struct sdhci_host *host = mmc_priv(mmc);
 	unsigned long flags;
-
-	host = mmc_priv(mmc);
-
-	/* Firstly check card presence */
-	present = mmc->ops->get_cd(mmc);
+	int ret;
 
 	spin_lock_irqsave(&host->lock, flags);
 
@@ -2033,15 +2045,34 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		mrq->cmd->error = -ENOMEDIUM;
+		ret = -ENOMEDIUM;
 		sdhci_finish_mrq(host, mrq);
 	} else {
 		if (mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23))
-			sdhci_send_command(host, mrq->sbc);
+			ret = sdhci_send_command(host, mrq->sbc, polling);
 		else
-			sdhci_send_command(host, mrq->cmd);
+			ret = sdhci_send_command(host, mrq->cmd, polling);
 	}
 
 	spin_unlock_irqrestore(&host->lock, flags);
+
+	return ret;
+}
+
+int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	return sdhci_start_request(mmc, mrq, 1, false);
+}
+EXPORT_SYMBOL_GPL(sdhci_request_atomic);
+
+void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	int present;
+
+	/* Firstly check card presence */
+	present = mmc->ops->get_cd(mmc);
+
+	sdhci_start_request(mmc, mrq, present, true);
 }
 EXPORT_SYMBOL_GPL(sdhci_request);
 
@@ -2581,7 +2612,7 @@ void sdhci_send_tuning(struct sdhci_host *host, u32 opcode)
 	 */
 	sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
 
-	sdhci_send_command(host, &cmd);
+	sdhci_send_command(host, &cmd, true);
 
 	host->cmd = NULL;
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index cac2d97..73e15c5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -759,7 +759,8 @@ void __sdhci_read_caps(struct sdhci_host *host, const u16 *ver,
 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);
+int sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd,
+		       bool polling);
 
 static inline void sdhci_read_caps(struct sdhci_host *host)
 {
@@ -775,6 +776,7 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 			   unsigned short vdd);
 void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
+int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq);
 void sdhci_set_bus_width(struct sdhci_host *host, int width);
 void sdhci_reset(struct sdhci_host *host, u8 mask);
 void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
-- 
1.9.1


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

* [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy
  2020-03-19 10:54 [PATCH v3 0/4] Introduce the request_atomic() for the host Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 1/4] mmc: host: " Baolin Wang
  2020-03-19 10:54 ` [PATCH v3 2/4] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
@ 2020-03-19 10:54 ` Baolin Wang
  2020-04-02 10:45   ` Adrian Hunter
  2020-03-19 10:54 ` [PATCH v3 4/4] mmc: host: sdhci-sprd: Implement the request_atomic() API Baolin Wang
  3 siblings, 1 reply; 7+ messages in thread
From: Baolin Wang @ 2020-03-19 10:54 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: orsonzhai, zhang.lyra, baolin.wang7, arnd, linux-mmc, linux-kernel

There is an unusual case that the card is busy when trying to send a
command, and we can not polling the card status in interrupt context
by using request_atomic() to dispatch requests.

Thus we should queue a work to try again in the non-atomic context
in case the host releases the busy signal later.

Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
 drivers/mmc/host/mmc_hsq.c | 37 ++++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/mmc_hsq.h |  1 +
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmc_hsq.c b/drivers/mmc/host/mmc_hsq.c
index fdbaa98..3edad11 100644
--- a/drivers/mmc/host/mmc_hsq.c
+++ b/drivers/mmc/host/mmc_hsq.c
@@ -15,11 +15,33 @@
 #define HSQ_NUM_SLOTS	64
 #define HSQ_INVALID_TAG	HSQ_NUM_SLOTS
 
+static void mmc_hsq_retry_handler(struct work_struct *work)
+{
+	struct mmc_hsq *hsq = container_of(work, struct mmc_hsq, retry_work);
+	struct mmc_host *mmc = hsq->mmc;
+	struct mmc_request *mrq = hsq->mrq;
+	struct mmc_data *data = mrq->data;
+
+	if (mmc->ops->request) {
+		mmc->ops->request(mmc, mrq);
+		return;
+	}
+
+	/*
+	 * If host does not supply the callback in normal context to
+	 * handle request, just finish this request.
+	 */
+	data->error = -EBUSY;
+	data->bytes_xfered = 0;
+	mmc_hsq_finalize_request(mmc, mrq);
+}
+
 static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
 {
 	struct mmc_host *mmc = hsq->mmc;
 	struct hsq_slot *slot;
 	unsigned long flags;
+	int ret = 0;
 
 	spin_lock_irqsave(&hsq->lock, flags);
 
@@ -42,9 +64,21 @@ static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
 	spin_unlock_irqrestore(&hsq->lock, flags);
 
 	if (mmc->ops->request_atomic)
-		mmc->ops->request_atomic(mmc, hsq->mrq);
+		ret = mmc->ops->request_atomic(mmc, hsq->mrq);
 	else
 		mmc->ops->request(mmc, hsq->mrq);
+
+	/*
+	 * If returning BUSY from request_atomic(), which means the card
+	 * may be busy now, and we should change to non-atomic context to
+	 * try again for this unusual case, to avoid time-consuming operations
+	 * in the atomic context.
+	 *
+	 * Note: we can ignore other error cases, since the host driver
+	 * will handle them.
+	 */
+	if (ret == -EBUSY)
+		schedule_work(&hsq->retry_work);
 }
 
 static void mmc_hsq_update_next_tag(struct mmc_hsq *hsq, int remains)
@@ -327,6 +361,7 @@ int mmc_hsq_init(struct mmc_hsq *hsq, struct mmc_host *mmc)
 	hsq->mmc->cqe_private = hsq;
 	mmc->cqe_ops = &mmc_hsq_ops;
 
+	INIT_WORK(&hsq->retry_work, mmc_hsq_retry_handler);
 	spin_lock_init(&hsq->lock);
 	init_waitqueue_head(&hsq->wait_queue);
 
diff --git a/drivers/mmc/host/mmc_hsq.h b/drivers/mmc/host/mmc_hsq.h
index d51beb7..81f6c4f 100644
--- a/drivers/mmc/host/mmc_hsq.h
+++ b/drivers/mmc/host/mmc_hsq.h
@@ -12,6 +12,7 @@ struct mmc_hsq {
 	wait_queue_head_t wait_queue;
 	struct hsq_slot *slot;
 	spinlock_t lock;
+	struct work_struct retry_work;
 
 	int next_tag;
 	int num_slots;
-- 
1.9.1


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

* [PATCH v3 4/4] mmc: host: sdhci-sprd: Implement the request_atomic() API
  2020-03-19 10:54 [PATCH v3 0/4] Introduce the request_atomic() for the host Baolin Wang
                   ` (2 preceding siblings ...)
  2020-03-19 10:54 ` [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy Baolin Wang
@ 2020-03-19 10:54 ` Baolin Wang
  3 siblings, 0 replies; 7+ messages in thread
From: Baolin Wang @ 2020-03-19 10:54 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: orsonzhai, zhang.lyra, baolin.wang7, arnd, linux-mmc, linux-kernel

Implement the request_atomic() API for nonremovable cards, that means
we can submit next request in the irq hard handler context to reduce
latency.

Moreover factor out the AUTO CMD23 checking into a separate function
to reduce duplicate code.

Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
 drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index 2ab42c5..bc7a8cb 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -406,7 +406,8 @@ static void sdhci_sprd_request_done(struct sdhci_host *host,
 	.request_done = sdhci_sprd_request_done,
 };
 
-static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
+static void sdhci_sprd_check_auto_cmd23(struct mmc_host *mmc,
+					struct mmc_request *mrq)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
 	struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
@@ -422,10 +423,23 @@ static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	    mrq->sbc && (mrq->sbc->arg & SDHCI_SPRD_ARG2_STUFF) &&
 	    (host->flags & SDHCI_AUTO_CMD23))
 		host->flags &= ~SDHCI_AUTO_CMD23;
+}
+
+static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	sdhci_sprd_check_auto_cmd23(mmc, mrq);
 
 	sdhci_request(mmc, mrq);
 }
 
+static int sdhci_sprd_request_atomic(struct mmc_host *mmc,
+				      struct mmc_request *mrq)
+{
+	sdhci_sprd_check_auto_cmd23(mmc, mrq);
+
+	return sdhci_request_atomic(mmc, mrq);
+}
+
 static int sdhci_sprd_voltage_switch(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
@@ -561,6 +575,11 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
 	if (ret)
 		goto pltfm_free;
 
+	if (!mmc_card_is_removable(host->mmc))
+		host->mmc_host_ops.request_atomic = sdhci_sprd_request_atomic;
+	else
+		host->always_defer_done = true;
+
 	sprd_host = TO_SPRD_HOST(host);
 	sdhci_sprd_phy_param_parse(sprd_host, pdev->dev.of_node);
 
@@ -654,8 +673,6 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_cleanup_host;
 
-	host->always_defer_done = true;
-
 	ret = __sdhci_add_host(host);
 	if (ret)
 		goto err_cleanup_host;
-- 
1.9.1


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

* Re: [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy
  2020-03-19 10:54 ` [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy Baolin Wang
@ 2020-04-02 10:45   ` Adrian Hunter
  2020-04-03  1:21     ` Baolin Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2020-04-02 10:45 UTC (permalink / raw)
  To: Baolin Wang, ulf.hansson
  Cc: orsonzhai, zhang.lyra, arnd, linux-mmc, linux-kernel

On 19/03/20 12:54 pm, Baolin Wang wrote:
> There is an unusual case that the card is busy when trying to send a
> command, and we can not polling the card status in interrupt context
> by using request_atomic() to dispatch requests.
> 
> Thus we should queue a work to try again in the non-atomic context
> in case the host releases the busy signal later.

I think this should be part of patch 1

> 
> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>

Sorry for the slow reply.

> ---
>  drivers/mmc/host/mmc_hsq.c | 37 ++++++++++++++++++++++++++++++++++++-
>  drivers/mmc/host/mmc_hsq.h |  1 +
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/mmc_hsq.c b/drivers/mmc/host/mmc_hsq.c
> index fdbaa98..3edad11 100644
> --- a/drivers/mmc/host/mmc_hsq.c
> +++ b/drivers/mmc/host/mmc_hsq.c
> @@ -15,11 +15,33 @@
>  #define HSQ_NUM_SLOTS	64
>  #define HSQ_INVALID_TAG	HSQ_NUM_SLOTS
>  
> +static void mmc_hsq_retry_handler(struct work_struct *work)
> +{
> +	struct mmc_hsq *hsq = container_of(work, struct mmc_hsq, retry_work);
> +	struct mmc_host *mmc = hsq->mmc;
> +	struct mmc_request *mrq = hsq->mrq;
> +	struct mmc_data *data = mrq->data;
> +
> +	if (mmc->ops->request) {

->request() is not an optional mmc operation so checking it is not necessary.

> +		mmc->ops->request(mmc, mrq);
> +		return;
> +	}
> +
> +	/*
> +	 * If host does not supply the callback in normal context to
> +	 * handle request, just finish this request.
> +	 */
> +	data->error = -EBUSY;
> +	data->bytes_xfered = 0;
> +	mmc_hsq_finalize_request(mmc, mrq);
> +}
> +
>  static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
>  {
>  	struct mmc_host *mmc = hsq->mmc;
>  	struct hsq_slot *slot;
>  	unsigned long flags;
> +	int ret = 0;
>  
>  	spin_lock_irqsave(&hsq->lock, flags);
>  
> @@ -42,9 +64,21 @@ static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
>  	spin_unlock_irqrestore(&hsq->lock, flags);
>  
>  	if (mmc->ops->request_atomic)
> -		mmc->ops->request_atomic(mmc, hsq->mrq);
> +		ret = mmc->ops->request_atomic(mmc, hsq->mrq);
>  	else
>  		mmc->ops->request(mmc, hsq->mrq);
> +
> +	/*
> +	 * If returning BUSY from request_atomic(), which means the card
> +	 * may be busy now, and we should change to non-atomic context to
> +	 * try again for this unusual case, to avoid time-consuming operations
> +	 * in the atomic context.
> +	 *
> +	 * Note: we can ignore other error cases, since the host driver
> +	 * will handle them.
> +	 */
> +	if (ret == -EBUSY)
> +		schedule_work(&hsq->retry_work);

Let's add a warning for unexpected return values i.e.

	WARN_ON_ONCE(ret && ret != -EBUSY);


>  }
>  
>  static void mmc_hsq_update_next_tag(struct mmc_hsq *hsq, int remains)
> @@ -327,6 +361,7 @@ int mmc_hsq_init(struct mmc_hsq *hsq, struct mmc_host *mmc)
>  	hsq->mmc->cqe_private = hsq;
>  	mmc->cqe_ops = &mmc_hsq_ops;
>  
> +	INIT_WORK(&hsq->retry_work, mmc_hsq_retry_handler);
>  	spin_lock_init(&hsq->lock);
>  	init_waitqueue_head(&hsq->wait_queue);
>  
> diff --git a/drivers/mmc/host/mmc_hsq.h b/drivers/mmc/host/mmc_hsq.h
> index d51beb7..81f6c4f 100644
> --- a/drivers/mmc/host/mmc_hsq.h
> +++ b/drivers/mmc/host/mmc_hsq.h
> @@ -12,6 +12,7 @@ struct mmc_hsq {
>  	wait_queue_head_t wait_queue;
>  	struct hsq_slot *slot;
>  	spinlock_t lock;
> +	struct work_struct retry_work;
>  
>  	int next_tag;
>  	int num_slots;
> 


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

* Re: [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy
  2020-04-02 10:45   ` Adrian Hunter
@ 2020-04-03  1:21     ` Baolin Wang
  0 siblings, 0 replies; 7+ messages in thread
From: Baolin Wang @ 2020-04-03  1:21 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Ulf Hansson, Orson Zhai, Chunyan Zhang, Arnd Bergmann, linux-mmc, LKML

Hi Adrian,

On Thu, Apr 2, 2020 at 6:45 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 19/03/20 12:54 pm, Baolin Wang wrote:
> > There is an unusual case that the card is busy when trying to send a
> > command, and we can not polling the card status in interrupt context
> > by using request_atomic() to dispatch requests.
> >
> > Thus we should queue a work to try again in the non-atomic context
> > in case the host releases the busy signal later.
>
> I think this should be part of patch 1

OK. Will move these changes into patch 1.

>
> >
> > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
>
> Sorry for the slow reply.
>
> > ---
> >  drivers/mmc/host/mmc_hsq.c | 37 ++++++++++++++++++++++++++++++++++++-
> >  drivers/mmc/host/mmc_hsq.h |  1 +
> >  2 files changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/mmc_hsq.c b/drivers/mmc/host/mmc_hsq.c
> > index fdbaa98..3edad11 100644
> > --- a/drivers/mmc/host/mmc_hsq.c
> > +++ b/drivers/mmc/host/mmc_hsq.c
> > @@ -15,11 +15,33 @@
> >  #define HSQ_NUM_SLOTS        64
> >  #define HSQ_INVALID_TAG      HSQ_NUM_SLOTS
> >
> > +static void mmc_hsq_retry_handler(struct work_struct *work)
> > +{
> > +     struct mmc_hsq *hsq = container_of(work, struct mmc_hsq, retry_work);
> > +     struct mmc_host *mmc = hsq->mmc;
> > +     struct mmc_request *mrq = hsq->mrq;
> > +     struct mmc_data *data = mrq->data;
> > +
> > +     if (mmc->ops->request) {
>
> ->request() is not an optional mmc operation so checking it is not necessary.

Yes, will remove the checking.

>
> > +             mmc->ops->request(mmc, mrq);
> > +             return;
> > +     }
> > +
> > +     /*
> > +      * If host does not supply the callback in normal context to
> > +      * handle request, just finish this request.
> > +      */
> > +     data->error = -EBUSY;
> > +     data->bytes_xfered = 0;
> > +     mmc_hsq_finalize_request(mmc, mrq);
> > +}
> > +
> >  static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
> >  {
> >       struct mmc_host *mmc = hsq->mmc;
> >       struct hsq_slot *slot;
> >       unsigned long flags;
> > +     int ret = 0;
> >
> >       spin_lock_irqsave(&hsq->lock, flags);
> >
> > @@ -42,9 +64,21 @@ static void mmc_hsq_pump_requests(struct mmc_hsq *hsq)
> >       spin_unlock_irqrestore(&hsq->lock, flags);
> >
> >       if (mmc->ops->request_atomic)
> > -             mmc->ops->request_atomic(mmc, hsq->mrq);
> > +             ret = mmc->ops->request_atomic(mmc, hsq->mrq);
> >       else
> >               mmc->ops->request(mmc, hsq->mrq);
> > +
> > +     /*
> > +      * If returning BUSY from request_atomic(), which means the card
> > +      * may be busy now, and we should change to non-atomic context to
> > +      * try again for this unusual case, to avoid time-consuming operations
> > +      * in the atomic context.
> > +      *
> > +      * Note: we can ignore other error cases, since the host driver
> > +      * will handle them.
> > +      */
> > +     if (ret == -EBUSY)
> > +             schedule_work(&hsq->retry_work);
>
> Let's add a warning for unexpected return values i.e.
>
>         WARN_ON_ONCE(ret && ret != -EBUSY);

Sure. Thanks for your comments.

>
>
> >  }
> >
> >  static void mmc_hsq_update_next_tag(struct mmc_hsq *hsq, int remains)
> > @@ -327,6 +361,7 @@ int mmc_hsq_init(struct mmc_hsq *hsq, struct mmc_host *mmc)
> >       hsq->mmc->cqe_private = hsq;
> >       mmc->cqe_ops = &mmc_hsq_ops;
> >
> > +     INIT_WORK(&hsq->retry_work, mmc_hsq_retry_handler);
> >       spin_lock_init(&hsq->lock);
> >       init_waitqueue_head(&hsq->wait_queue);
> >
> > diff --git a/drivers/mmc/host/mmc_hsq.h b/drivers/mmc/host/mmc_hsq.h
> > index d51beb7..81f6c4f 100644
> > --- a/drivers/mmc/host/mmc_hsq.h
> > +++ b/drivers/mmc/host/mmc_hsq.h
> > @@ -12,6 +12,7 @@ struct mmc_hsq {
> >       wait_queue_head_t wait_queue;
> >       struct hsq_slot *slot;
> >       spinlock_t lock;
> > +     struct work_struct retry_work;
> >
> >       int next_tag;
> >       int num_slots;
> >
>


-- 
Baolin Wang

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

end of thread, other threads:[~2020-04-03  1:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 10:54 [PATCH v3 0/4] Introduce the request_atomic() for the host Baolin Wang
2020-03-19 10:54 ` [PATCH v3 1/4] mmc: host: " Baolin Wang
2020-03-19 10:54 ` [PATCH v3 2/4] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
2020-03-19 10:54 ` [PATCH v3 3/4] mmc: host: hsq: Handle an unusual case of returing busy Baolin Wang
2020-04-02 10:45   ` Adrian Hunter
2020-04-03  1:21     ` Baolin Wang
2020-03-19 10:54 ` [PATCH v3 4/4] mmc: host: sdhci-sprd: Implement the request_atomic() API Baolin Wang

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