On Mon, 15 Nov 2021 at 22:54, Wojciech Bartczak <wbartczak@xxxxxxxxxxx> wrote:
>
> The driver for cavium/marvell platforms uses directly mrq->done() callback
> to signalize the request completion. This method to finalize request
> processing is not correct.
>
> Following fix introduces proper use of mmc_request_done() API for
> all paths involved into handling MMC core requests.
>
> Changes v1 => v2:
> - Added missing variable slot and functionality to retrive
> slot base on bus_id contained in response status register.
Version history is great, but should come outside the actual commit
message. See below.
>
> Signed-off-by: Wojciech Bartczak <wbartczak@xxxxxxxxxxx>
> ---
Version history should come here along with other information. Then
add the below three dashes to end the section.
---
> drivers/mmc/host/cavium.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
> index 95a41983c6c0..674cfaf5d64e 100644
> --- a/drivers/mmc/host/cavium.c
> +++ b/drivers/mmc/host/cavium.c
> @@ -435,8 +435,10 @@ static void cleanup_dma(struct cvm_mmc_host *host, u64 rsp_sts)
> irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> {
> struct cvm_mmc_host *host = dev_id;
> + struct cvm_mmc_slot *slot;
> struct mmc_request *req;
> u64 emm_int, rsp_sts;
> + int bus_id;
> bool host_done;
>
> if (host->need_irq_handler_lock)
> @@ -456,6 +458,8 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> goto out;
>
> rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
> + bus_id = get_bus_id(rsp_sts);
> + slot = host->slot[bus_id]; /* bus_id is in a range 0..2 */
> /*
> * dma_val set means DMA is still in progress. Don't touch
> * the request and wait for the interrupt indicating that
> @@ -493,8 +497,8 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> (rsp_sts & MIO_EMM_RSP_STS_DMA_PEND))
> cleanup_dma(host, rsp_sts);
>
> + mmc_request_done(slot->mmc, req);
> host->current_req = NULL;
> - req->done(req);
Flipping the order doesn't really matter here, as cvm_mmc_request() is
protected with the ->acquire_bus() lock.
However, I think it's good practise from the mmc core point of view,
to clear host->current_req prior to calling mmc_request_done(). Can
you please change this.
>
> no_req_done:
> if (host->dmar_fixup_done)
> @@ -699,8 +703,7 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc,
>
> error:
> mrq->cmd->error = -EINVAL;
> - if (mrq->done)
> - mrq->done(mrq);
> + mmc_request_done(mmc, mrq);
> host->release_bus(host);
> }
>
Kind regards
Uffe
