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.
Note, this patch set is based on Adrian's patch set:
https://www.spinics.net/lists/linux-mmc/msg58529.html
Any comments are welcome. Thanks.
Changes from v4:
- Remove redundant checking when warning the return value of request_atomic().
- Add acked tag from Adrian for patch 1.
- Re-implement the request_atomic() based on the Adrian's patch set.
Changes from v3:
- Move patch 3 of V3 patch set into patch 1.
- Add a warning for unexpected return value of request_atomic().
- Remove redundant checking of ops->request().
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 (3):
mmc: host: Introduce the request_atomic() for the host
mmc: host: sdhci: Implement the request_atomic() API
mmc: host: sdhci-sprd: Implement the request_atomic() API
drivers/mmc/host/mmc_hsq.c | 29 ++++++++++++++++++++++++++++-
drivers/mmc/host/mmc_hsq.h | 1 +
drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++---
drivers/mmc/host/sdhci.c | 34 ++++++++++++++++++++++++++++++++++
drivers/mmc/host/sdhci.h | 1 +
include/linux/mmc/host.h | 3 +++
6 files changed, 87 insertions(+), 4 deletions(-)
--
2.17.1
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 <[email protected]>
---
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 2ab42c59e4f8..bc7a8cb84862 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -406,7 +406,8 @@ static struct sdhci_ops sdhci_sprd_ops = {
.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;
--
2.17.1
Implement the request_atomic() ops for the sdhci driver to process
one request in the atomic context if the card is nonremovable.
Moreover, we should return BUSY flag if controller has not released
the inhibit bits to allow HSQ trying to send request again in non-atomic
context.
Suggested-by: Adrian Hunter <[email protected]>
Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci.c | 34 ++++++++++++++++++++++++++++++++++
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 35 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 10b9570f48aa..0baef595de26 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2144,6 +2144,40 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
}
EXPORT_SYMBOL_GPL(sdhci_request);
+int sdhci_request_atomic(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)) {
+ sdhci_finish_mrq(host, mrq);
+ goto out_finish;
+ }
+
+ cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
+
+ /*
+ * The 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.
+ */
+ if (!sdhci_send_command(host, cmd))
+ ret = -EBUSY;
+ else
+ sdhci_led_activate(host);
+
+out_finish:
+ spin_unlock_irqrestore(&host->lock, flags);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(sdhci_request_atomic);
+
void sdhci_set_bus_width(struct sdhci_host *host, int width)
{
u8 ctrl;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index a7e469c00617..4bd70da7aa00 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -776,6 +776,7 @@ void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
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);
--
2.17.1
On 13/04/20 5:46 am, Baolin Wang wrote:
> Implement the request_atomic() ops for the sdhci driver to process
> one request in the atomic context if the card is nonremovable.
>
> Moreover, we should return BUSY flag if controller has not released
> the inhibit bits to allow HSQ trying to send request again in non-atomic
> context.
>
> Suggested-by: Adrian Hunter <[email protected]>
> Signed-off-by: Baolin Wang <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 34 ++++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci.h | 1 +
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 10b9570f48aa..0baef595de26 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2144,6 +2144,40 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> }
> EXPORT_SYMBOL_GPL(sdhci_request);
>
> +int sdhci_request_atomic(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)) {
> + sdhci_finish_mrq(host, mrq);
> + goto out_finish;
> + }
> +
> + cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
> +
> + /*
> + * The 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.
> + */
> + if (!sdhci_send_command(host, cmd))
> + ret = -EBUSY;
> + else
> + sdhci_led_activate(host);
> +
> +out_finish:
> + spin_unlock_irqrestore(&host->lock, flags);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(sdhci_request_atomic);
> +
> void sdhci_set_bus_width(struct sdhci_host *host, int width)
> {
> u8 ctrl;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index a7e469c00617..4bd70da7aa00 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -776,6 +776,7 @@ void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
> 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);
>
On 13/04/20 5:46 am, Baolin Wang wrote:
> 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 <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> 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 2ab42c59e4f8..bc7a8cb84862 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -406,7 +406,8 @@ static struct sdhci_ops sdhci_sprd_ops = {
> .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;
>
On Mon, 13 Apr 2020 at 04:46, Baolin Wang <[email protected]> wrote:
>
> 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.
>
> Note, this patch set is based on Adrian's patch set:
> https://www.spinics.net/lists/linux-mmc/msg58529.html
>
> Any comments are welcome. Thanks.
>
> Changes from v4:
> - Remove redundant checking when warning the return value of request_atomic().
> - Add acked tag from Adrian for patch 1.
> - Re-implement the request_atomic() based on the Adrian's patch set.
>
> Changes from v3:
> - Move patch 3 of V3 patch set into patch 1.
> - Add a warning for unexpected return value of request_atomic().
> - Remove redundant checking of ops->request().
>
> 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 (3):
> mmc: host: Introduce the request_atomic() for the host
> mmc: host: sdhci: Implement the request_atomic() API
> mmc: host: sdhci-sprd: Implement the request_atomic() API
>
> drivers/mmc/host/mmc_hsq.c | 29 ++++++++++++++++++++++++++++-
> drivers/mmc/host/mmc_hsq.h | 1 +
> drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++---
> drivers/mmc/host/sdhci.c | 34 ++++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci.h | 1 +
> include/linux/mmc/host.h | 3 +++
> 6 files changed, 87 insertions(+), 4 deletions(-)
>
> --
> 2.17.1
>
Applied for next, thanks!
Kind regards
Uffe