* [PATCH v5 0/3] Introduce the request_atomic() for the host
@ 2020-04-13 2:46 Baolin Wang
2020-04-13 2:46 ` [PATCH v5 1/3] mmc: host: " Baolin Wang
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Baolin Wang @ 2020-04-13 2:46 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: arnd, baolin.wang7, orsonzhai, zhang.lyra, 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.
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/3] mmc: host: Introduce the request_atomic() for the host
2020-04-13 2:46 [PATCH v5 0/3] Introduce the request_atomic() for the host Baolin Wang
@ 2020-04-13 2:46 ` Baolin Wang
2020-04-13 2:46 ` [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-04-13 2:46 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: arnd, baolin.wang7, orsonzhai, zhang.lyra, 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.
Moreover 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.
Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
drivers/mmc/host/mmc_hsq.c | 29 ++++++++++++++++++++++++++++-
drivers/mmc/host/mmc_hsq.h | 1 +
include/linux/mmc/host.h | 3 +++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/mmc_hsq.c b/drivers/mmc/host/mmc_hsq.c
index b90b2c97b6cf..a5e05ed0fda3 100644
--- a/drivers/mmc/host/mmc_hsq.c
+++ b/drivers/mmc/host/mmc_hsq.c
@@ -16,11 +16,20 @@
#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;
+
+ mmc->ops->request(mmc, hsq->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,7 +51,24 @@ 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)
+ 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 just give a warning for other error cases, since the host
+ * driver will handle them.
+ */
+ if (ret == -EBUSY)
+ schedule_work(&hsq->retry_work);
+ else
+ WARN_ON_ONCE(ret);
}
static void mmc_hsq_update_next_tag(struct mmc_hsq *hsq, int remains)
@@ -325,6 +351,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 18b9cf55925f..ffdd9cd172c3 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;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c318fb5b6a94..d4a50e5dc111 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
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API
2020-04-13 2:46 [PATCH v5 0/3] Introduce the request_atomic() for the host Baolin Wang
2020-04-13 2:46 ` [PATCH v5 1/3] mmc: host: " Baolin Wang
@ 2020-04-13 2:46 ` Baolin Wang
2020-04-13 4:40 ` kbuild test robot
` (2 more replies)
2020-04-13 2:46 ` [PATCH v5 3/3] mmc: host: sdhci-sprd: " Baolin Wang
2020-04-17 11:29 ` [PATCH v5 0/3] Introduce the request_atomic() for the host Ulf Hansson
3 siblings, 3 replies; 10+ messages in thread
From: Baolin Wang @ 2020-04-13 2:46 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: arnd, baolin.wang7, orsonzhai, zhang.lyra, 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, 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 <adrian.hunter@intel.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/3] mmc: host: sdhci-sprd: Implement the request_atomic() API
2020-04-13 2:46 [PATCH v5 0/3] Introduce the request_atomic() for the host Baolin Wang
2020-04-13 2:46 ` [PATCH v5 1/3] mmc: host: " Baolin Wang
2020-04-13 2:46 ` [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
@ 2020-04-13 2:46 ` Baolin Wang
2020-04-14 13:49 ` Adrian Hunter
2020-04-17 11:29 ` [PATCH v5 0/3] Introduce the request_atomic() for the host Ulf Hansson
3 siblings, 1 reply; 10+ messages in thread
From: Baolin Wang @ 2020-04-13 2:46 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: arnd, baolin.wang7, orsonzhai, zhang.lyra, 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 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API
2020-04-13 2:46 ` [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
@ 2020-04-13 4:40 ` kbuild test robot
2020-04-13 5:42 ` kbuild test robot
2020-04-14 13:47 ` Adrian Hunter
2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2020-04-13 4:40 UTC (permalink / raw)
To: Baolin Wang
Cc: kbuild-all, ulf.hansson, adrian.hunter, arnd, baolin.wang7,
orsonzhai, zhang.lyra, linux-mmc, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3257 bytes --]
Hi Baolin,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc1 next-20200412]
[cannot apply to arm-soc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-the-request_atomic-for-the-host/20200413-105346
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8f3d9f354286745c751374f5f1fcafee6b3f3136
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci.c: In function 'sdhci_request_atomic':
>> drivers/mmc/host/sdhci.c:2076:6: error: implicit declaration of function 'sdhci_present_error' [-Werror=implicit-function-declaration]
2076 | if (sdhci_present_error(host, mrq->cmd, true)) {
| ^~~~~~~~~~~~~~~~~~~
>> drivers/mmc/host/sdhci.c:2081:8: error: implicit declaration of function 'sdhci_manual_cmd23'; did you mean 'sdhci_auto_cmd12'? [-Werror=implicit-function-declaration]
2081 | cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
| ^~~~~~~~~~~~~~~~~~
| sdhci_auto_cmd12
>> drivers/mmc/host/sdhci.c:2090:6: error: invalid use of void expression
2090 | if (!sdhci_send_command(host, cmd))
| ^
cc1: some warnings being treated as errors
vim +/sdhci_present_error +2076 drivers/mmc/host/sdhci.c
2066
2067 int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
2068 {
2069 struct sdhci_host *host = mmc_priv(mmc);
2070 struct mmc_command *cmd;
2071 unsigned long flags;
2072 int ret = 0;
2073
2074 spin_lock_irqsave(&host->lock, flags);
2075
> 2076 if (sdhci_present_error(host, mrq->cmd, true)) {
2077 sdhci_finish_mrq(host, mrq);
2078 goto out_finish;
2079 }
2080
> 2081 cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
2082
2083 /*
2084 * The HSQ may send a command in interrupt context without polling
2085 * the busy signaling, which means we should return BUSY if controller
2086 * has not released inhibit bits to allow HSQ trying to send request
2087 * again in non-atomic context. So we should not finish this request
2088 * here.
2089 */
> 2090 if (!sdhci_send_command(host, cmd))
2091 ret = -EBUSY;
2092 else
2093 sdhci_led_activate(host);
2094
2095 out_finish:
2096 spin_unlock_irqrestore(&host->lock, flags);
2097 return ret;
2098 }
2099 EXPORT_SYMBOL_GPL(sdhci_request_atomic);
2100
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63433 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API
2020-04-13 2:46 ` [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
2020-04-13 4:40 ` kbuild test robot
@ 2020-04-13 5:42 ` kbuild test robot
2020-04-13 6:52 ` Baolin Wang
2020-04-14 13:47 ` Adrian Hunter
2 siblings, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2020-04-13 5:42 UTC (permalink / raw)
To: Baolin Wang
Cc: kbuild-all, ulf.hansson, adrian.hunter, arnd, baolin.wang7,
orsonzhai, zhang.lyra, linux-mmc, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4422 bytes --]
Hi Baolin,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc1 next-20200412]
[cannot apply to arm-soc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-the-request_atomic-for-the-host/20200413-105346
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8f3d9f354286745c751374f5f1fcafee6b3f3136
config: x86_64-randconfig-s2-20200413 (attached as .config)
compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from include/linux/build_bug.h:5:0,
from include/linux/bitfield.h:10,
from drivers/mmc/host/sdhci.c:12:
drivers/mmc/host/sdhci.c: In function 'sdhci_request_atomic':
drivers/mmc/host/sdhci.c:2076:6: error: implicit declaration of function 'sdhci_present_error' [-Werror=implicit-function-declaration]
if (sdhci_present_error(host, mrq->cmd, true)) {
^
include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
>> drivers/mmc/host/sdhci.c:2076:2: note: in expansion of macro 'if'
if (sdhci_present_error(host, mrq->cmd, true)) {
^
drivers/mmc/host/sdhci.c:2081:8: error: implicit declaration of function 'sdhci_manual_cmd23' [-Werror=implicit-function-declaration]
cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
^
drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
if (!sdhci_send_command(host, cmd))
^
drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
In file included from include/linux/build_bug.h:5:0,
from include/linux/bitfield.h:10,
from drivers/mmc/host/sdhci.c:12:
>> include/linux/compiler.h:61:16: error: invalid use of void expression
static struct ftrace_branch_data \
^
include/linux/compiler.h:58:69: note: in expansion of macro '__trace_if_value'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/mmc/host/sdhci.c:2090:2: note: in expansion of macro 'if'
if (!sdhci_send_command(host, cmd))
^
cc1: some warnings being treated as errors
vim +/if +2076 drivers/mmc/host/sdhci.c
2066
2067 int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
2068 {
2069 struct sdhci_host *host = mmc_priv(mmc);
2070 struct mmc_command *cmd;
2071 unsigned long flags;
2072 int ret = 0;
2073
2074 spin_lock_irqsave(&host->lock, flags);
2075
> 2076 if (sdhci_present_error(host, mrq->cmd, true)) {
2077 sdhci_finish_mrq(host, mrq);
2078 goto out_finish;
2079 }
2080
2081 cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
2082
2083 /*
2084 * The HSQ may send a command in interrupt context without polling
2085 * the busy signaling, which means we should return BUSY if controller
2086 * has not released inhibit bits to allow HSQ trying to send request
2087 * again in non-atomic context. So we should not finish this request
2088 * here.
2089 */
2090 if (!sdhci_send_command(host, cmd))
2091 ret = -EBUSY;
2092 else
2093 sdhci_led_activate(host);
2094
2095 out_finish:
2096 spin_unlock_irqrestore(&host->lock, flags);
2097 return ret;
2098 }
2099 EXPORT_SYMBOL_GPL(sdhci_request_atomic);
2100
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31571 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API
2020-04-13 5:42 ` kbuild test robot
@ 2020-04-13 6:52 ` Baolin Wang
0 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-04-13 6:52 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Ulf Hansson, Adrian Hunter, Arnd Bergmann,
Orson Zhai, Chunyan Zhang, linux-mmc, LKML
Hi,
On Mon, Apr 13, 2020 at 1:42 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Baolin,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.7-rc1 next-20200412]
> [cannot apply to arm-soc/for-next]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-the-request_atomic-for-the-host/20200413-105346
> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8f3d9f354286745c751374f5f1fcafee6b3f3136
> config: x86_64-randconfig-s2-20200413 (attached as .config)
> compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>
This patch set is based on Adrian's patch set. Thanks.
https://www.spinics.net/lists/linux-mmc/msg58529.html
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from include/linux/build_bug.h:5:0,
> from include/linux/bitfield.h:10,
> from drivers/mmc/host/sdhci.c:12:
> drivers/mmc/host/sdhci.c: In function 'sdhci_request_atomic':
> drivers/mmc/host/sdhci.c:2076:6: error: implicit declaration of function 'sdhci_present_error' [-Werror=implicit-function-declaration]
> if (sdhci_present_error(host, mrq->cmd, true)) {
> ^
> include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
> #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
> ^
> >> drivers/mmc/host/sdhci.c:2076:2: note: in expansion of macro 'if'
> if (sdhci_present_error(host, mrq->cmd, true)) {
> ^
> drivers/mmc/host/sdhci.c:2081:8: error: implicit declaration of function 'sdhci_manual_cmd23' [-Werror=implicit-function-declaration]
> cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
> ^
> drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
> if (!sdhci_send_command(host, cmd))
> ^
> drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
> In file included from include/linux/build_bug.h:5:0,
> from include/linux/bitfield.h:10,
> from drivers/mmc/host/sdhci.c:12:
> >> include/linux/compiler.h:61:16: error: invalid use of void expression
> static struct ftrace_branch_data \
> ^
> include/linux/compiler.h:58:69: note: in expansion of macro '__trace_if_value'
> #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
> ^
> include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
> #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
> ^
> drivers/mmc/host/sdhci.c:2090:2: note: in expansion of macro 'if'
> if (!sdhci_send_command(host, cmd))
> ^
> cc1: some warnings being treated as errors
>
> vim +/if +2076 drivers/mmc/host/sdhci.c
>
> 2066
> 2067 int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
> 2068 {
> 2069 struct sdhci_host *host = mmc_priv(mmc);
> 2070 struct mmc_command *cmd;
> 2071 unsigned long flags;
> 2072 int ret = 0;
> 2073
> 2074 spin_lock_irqsave(&host->lock, flags);
> 2075
> > 2076 if (sdhci_present_error(host, mrq->cmd, true)) {
> 2077 sdhci_finish_mrq(host, mrq);
> 2078 goto out_finish;
> 2079 }
> 2080
> 2081 cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
> 2082
> 2083 /*
> 2084 * The HSQ may send a command in interrupt context without polling
> 2085 * the busy signaling, which means we should return BUSY if controller
> 2086 * has not released inhibit bits to allow HSQ trying to send request
> 2087 * again in non-atomic context. So we should not finish this request
> 2088 * here.
> 2089 */
> 2090 if (!sdhci_send_command(host, cmd))
> 2091 ret = -EBUSY;
> 2092 else
> 2093 sdhci_led_activate(host);
> 2094
> 2095 out_finish:
> 2096 spin_unlock_irqrestore(&host->lock, flags);
> 2097 return ret;
> 2098 }
> 2099 EXPORT_SYMBOL_GPL(sdhci_request_atomic);
> 2100
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
--
Baolin Wang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API
2020-04-13 2:46 ` [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
2020-04-13 4:40 ` kbuild test robot
2020-04-13 5:42 ` kbuild test robot
@ 2020-04-14 13:47 ` Adrian Hunter
2 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2020-04-14 13:47 UTC (permalink / raw)
To: Baolin Wang, ulf.hansson
Cc: arnd, orsonzhai, zhang.lyra, linux-mmc, linux-kernel
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 <adrian.hunter@intel.com>
> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 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);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] mmc: host: sdhci-sprd: Implement the request_atomic() API
2020-04-13 2:46 ` [PATCH v5 3/3] mmc: host: sdhci-sprd: " Baolin Wang
@ 2020-04-14 13:49 ` Adrian Hunter
0 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2020-04-14 13:49 UTC (permalink / raw)
To: Baolin Wang, ulf.hansson
Cc: arnd, orsonzhai, zhang.lyra, linux-mmc, linux-kernel
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 <baolin.wang7@gmail.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.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 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;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/3] Introduce the request_atomic() for the host
2020-04-13 2:46 [PATCH v5 0/3] Introduce the request_atomic() for the host Baolin Wang
` (2 preceding siblings ...)
2020-04-13 2:46 ` [PATCH v5 3/3] mmc: host: sdhci-sprd: " Baolin Wang
@ 2020-04-17 11:29 ` Ulf Hansson
3 siblings, 0 replies; 10+ messages in thread
From: Ulf Hansson @ 2020-04-17 11:29 UTC (permalink / raw)
To: Baolin Wang
Cc: Adrian Hunter, Arnd Bergmann, Orson Zhai, Chunyan Zhang,
linux-mmc, Linux Kernel Mailing List
On Mon, 13 Apr 2020 at 04:46, Baolin Wang <baolin.wang7@gmail.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-17 11:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 2:46 [PATCH v5 0/3] Introduce the request_atomic() for the host Baolin Wang
2020-04-13 2:46 ` [PATCH v5 1/3] mmc: host: " Baolin Wang
2020-04-13 2:46 ` [PATCH v5 2/3] mmc: host: sdhci: Implement the request_atomic() API Baolin Wang
2020-04-13 4:40 ` kbuild test robot
2020-04-13 5:42 ` kbuild test robot
2020-04-13 6:52 ` Baolin Wang
2020-04-14 13:47 ` Adrian Hunter
2020-04-13 2:46 ` [PATCH v5 3/3] mmc: host: sdhci-sprd: " Baolin Wang
2020-04-14 13:49 ` Adrian Hunter
2020-04-17 11:29 ` [PATCH v5 0/3] Introduce the request_atomic() for the host Ulf Hansson
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).