[PATCH] mmc: cavium: Ensure proper mapping between request and interrupt

[PATCH] mmc: cavium: Ensure proper mapping between request and interrupt

[Wojciech Bartczak]

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 <wbartczak@marvell.com>
---
 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

Re: [PATCH] mmc: cavium: Ensure proper mapping between request and interrupt

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5100 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45436 bytes --]

Re: [PATCH] mmc: cavium: Ensure proper mapping between request and interrupt

[Ulf Hansson]

On Mon, 15 Nov 2021 at 17:04, Wojciech Bartczak <wbartczak@marvell.com> 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 <wbartczak@marvell.com>
> ---
>  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