The MMC block in octeontx2 uses architecture, where data lines are
shared between up to three supported cards. To keep the track of the
request hardware reports bus_id for the command complete interrupt.
At the same time the driver keeps information about the request on the
fly. This change combines these information to ensure proper mapping
between the request and the response.
Signed-off-by: Wojciech Bartczak <[email protected]>
---
drivers/mmc/host/cavium.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
index 95a41983c6c0..24ea03f5a7c0 100644
--- a/drivers/mmc/host/cavium.c
+++ b/drivers/mmc/host/cavium.c
@@ -438,6 +438,8 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
struct mmc_request *req;
u64 emm_int, rsp_sts;
bool host_done;
+ struct cvm_mmc_slot *slot;
+ int bus_id;
if (host->need_irq_handler_lock)
spin_lock(&host->irq_handler_lock);
@@ -455,7 +457,19 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
if (!req)
goto out;
+ slot = mmc_priv(req->host);
+
+ /* Get response */
rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
+ /* Map request to cvm_mmc_slot */
+ bus_id = FIELD_GET(MIO_EMM_RSP_STS_BUS_ID, rsp_sts);
+ if (bus_id != slot->bus_id) {
+ dev_err(mmc_classdev(slot->mmc),
+ "Remapping, request for slot %d, interrupt for %d!\n",
+ slot->bus_id, bus_id);
+ slot = host->slot[bus_id];
+ }
+
/*
* dma_val set means DMA is still in progress. Don't touch
* the request and wait for the interrupt indicating that
@@ -493,8 +507,9 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
(rsp_sts & MIO_EMM_RSP_STS_DMA_PEND))
cleanup_dma(host, rsp_sts);
+ if (mrq->done)
+ mrq->done(mrq);
host->current_req = NULL;
- req->done(req);
no_req_done:
if (host->dmar_fixup_done)
@@ -669,6 +684,7 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc,
set_wdog(slot, data->timeout_ns);
WARN_ON(host->current_req);
+ mrq->host = mmc;
host->current_req = mrq;
emm_dma = prepare_ext_dma(mmc, mrq);
@@ -776,6 +792,7 @@ static void cvm_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
mods = cvm_mmc_get_cr_mods(cmd);
WARN_ON(host->current_req);
+ mrq->host = mmc;
host->current_req = mrq;
if (cmd->data) {
--
2.17.1
Hi Wojciech,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on ulf-hansson-mmc-mirror/next v5.16-rc2 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Wojciech-Bartczak/mmc-cavium-Ensure-proper-mapping-between-request-and-interrupt/20211116-000649
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8ab774587903771821b59471cc723bba6d893942
config: arm64-buildonly-randconfig-r006-20211115 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/89ba622be0ed9957864526cf69be1b6070e9f117
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wojciech-Bartczak/mmc-cavium-Ensure-proper-mapping-between-request-and-interrupt/20211116-000649
git checkout 89ba622be0ed9957864526cf69be1b6070e9f117
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/mmc/host/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/mmc/host/cavium.c: In function 'cvm_mmc_interrupt':
>> drivers/mmc/host/cavium.c:510:13: error: 'mrq' undeclared (first use in this function); did you mean 'irq'?
510 | if (mrq->done)
| ^~~
| irq
drivers/mmc/host/cavium.c:510:13: note: each undeclared identifier is reported only once for each function it appears in
vim +510 drivers/mmc/host/cavium.c
434
435 irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
436 {
437 struct cvm_mmc_host *host = dev_id;
438 struct mmc_request *req;
439 u64 emm_int, rsp_sts;
440 bool host_done;
441 struct cvm_mmc_slot *slot;
442 int bus_id;
443
444 if (host->need_irq_handler_lock)
445 spin_lock(&host->irq_handler_lock);
446 else
447 __acquire(&host->irq_handler_lock);
448
449 /* Clear interrupt bits (write 1 clears ). */
450 emm_int = readq(host->base + MIO_EMM_INT(host));
451 writeq(emm_int, host->base + MIO_EMM_INT(host));
452
453 if (emm_int & MIO_EMM_INT_SWITCH_ERR)
454 check_switch_errors(host);
455
456 req = host->current_req;
457 if (!req)
458 goto out;
459
460 slot = mmc_priv(req->host);
461
462 /* Get response */
463 rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
464 /* Map request to cvm_mmc_slot */
465 bus_id = FIELD_GET(MIO_EMM_RSP_STS_BUS_ID, rsp_sts);
466 if (bus_id != slot->bus_id) {
467 dev_err(mmc_classdev(slot->mmc),
468 "Remapping, request for slot %d, interrupt for %d!\n",
469 slot->bus_id, bus_id);
470 slot = host->slot[bus_id];
471 }
472
473 /*
474 * dma_val set means DMA is still in progress. Don't touch
475 * the request and wait for the interrupt indicating that
476 * the DMA is finished.
477 */
478 if ((rsp_sts & MIO_EMM_RSP_STS_DMA_VAL) && host->dma_active)
479 goto out;
480
481 if (!host->dma_active && req->data &&
482 (emm_int & MIO_EMM_INT_BUF_DONE)) {
483 unsigned int type = (rsp_sts >> 7) & 3;
484
485 if (type == 1)
486 do_read(host, req, rsp_sts & MIO_EMM_RSP_STS_DBUF);
487 else if (type == 2)
488 do_write(req);
489 }
490
491 host_done = emm_int & MIO_EMM_INT_CMD_DONE ||
492 emm_int & MIO_EMM_INT_DMA_DONE ||
493 emm_int & MIO_EMM_INT_CMD_ERR ||
494 emm_int & MIO_EMM_INT_DMA_ERR;
495
496 if (!(host_done && req->done))
497 goto no_req_done;
498
499 req->cmd->error = check_status(rsp_sts);
500
501 if (host->dma_active && req->data)
502 if (!finish_dma(host, req->data))
503 goto no_req_done;
504
505 set_cmd_response(host, req, rsp_sts);
506 if ((emm_int & MIO_EMM_INT_DMA_ERR) &&
507 (rsp_sts & MIO_EMM_RSP_STS_DMA_PEND))
508 cleanup_dma(host, rsp_sts);
509
> 510 if (mrq->done)
511 mrq->done(mrq);
512 host->current_req = NULL;
513
514 no_req_done:
515 if (host->dmar_fixup_done)
516 host->dmar_fixup_done(host);
517 if (host_done)
518 host->release_bus(host);
519 out:
520 if (host->need_irq_handler_lock)
521 spin_unlock(&host->irq_handler_lock);
522 else
523 __release(&host->irq_handler_lock);
524 return IRQ_RETVAL(emm_int != 0);
525 }
526
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Mon, 15 Nov 2021 at 17:04, Wojciech Bartczak <[email protected]> wrote:
>
> The MMC block in octeontx2 uses architecture, where data lines are
> shared between up to three supported cards. To keep the track of the
> request hardware reports bus_id for the command complete interrupt.
> At the same time the driver keeps information about the request on the
> fly. This change combines these information to ensure proper mapping
> between the request and the response.
>
> Signed-off-by: Wojciech Bartczak <[email protected]>
> ---
> drivers/mmc/host/cavium.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
> index 95a41983c6c0..24ea03f5a7c0 100644
> --- a/drivers/mmc/host/cavium.c
> +++ b/drivers/mmc/host/cavium.c
> @@ -438,6 +438,8 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> struct mmc_request *req;
> u64 emm_int, rsp_sts;
> bool host_done;
> + struct cvm_mmc_slot *slot;
> + int bus_id;
>
> if (host->need_irq_handler_lock)
> spin_lock(&host->irq_handler_lock);
> @@ -455,7 +457,19 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> if (!req)
> goto out;
>
> + slot = mmc_priv(req->host);
> +
> + /* Get response */
> rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
> + /* Map request to cvm_mmc_slot */
> + bus_id = FIELD_GET(MIO_EMM_RSP_STS_BUS_ID, rsp_sts);
> + if (bus_id != slot->bus_id) {
> + dev_err(mmc_classdev(slot->mmc),
> + "Remapping, request for slot %d, interrupt for %d!\n",
> + slot->bus_id, bus_id);
> + slot = host->slot[bus_id];
> + }
> +
> /*
> * dma_val set means DMA is still in progress. Don't touch
> * the request and wait for the interrupt indicating that
> @@ -493,8 +507,9 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> (rsp_sts & MIO_EMM_RSP_STS_DMA_PEND))
> cleanup_dma(host, rsp_sts);
>
> + if (mrq->done)
> + mrq->done(mrq);
> host->current_req = NULL;
> - req->done(req);
This looks unrelated, please drop this from @subject patch.
>
> no_req_done:
> if (host->dmar_fixup_done)
> @@ -669,6 +684,7 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc,
> set_wdog(slot, data->timeout_ns);
>
> WARN_ON(host->current_req);
> + mrq->host = mmc;
Please, no. We don't want the host driver to update the mrq->host. If
that should be done, which is kind of questionable to me, that should
be managed by the core layer.
In this particular case, an option could be to store the bus-id that
corresponds to the slot for the request that is about to be started.
Along the lines of how we store host->current_req, below.
> host->current_req = mrq;
>
> emm_dma = prepare_ext_dma(mmc, mrq);
> @@ -776,6 +792,7 @@ static void cvm_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> mods = cvm_mmc_get_cr_mods(cmd);
>
> WARN_ON(host->current_req);
> + mrq->host = mmc;
> host->current_req = mrq;
>
> if (cmd->data) {
Kind regards
Uffe