From adfa57033d73dfc3449f3a9a2fe5450673b5aef3 Mon Sep 17 00:00:00 2001 From: Matteo Facchinetti Date: Tue, 30 Sep 2014 16:59:37 +0200 Subject: [PATCH] mmc: mxcmmc: fix race condition when dma finish a data transfer During a read of data block using dma, driver might have two ways to finish to read and free the resources: 1) checking STATUS_DATA_TRANS_DONE mask, in the mxcmci_irq() routine (pending to mmc irq) 2) mxmmc driver, registers also a mxcmci_dma_callback() and when transfer is finished, dma driver calls this callback. (pending to dma irq) Both ways are concurrent with each other. Race condition happens when following events occur: /* (1) mxcmci driver start data transfer */ 158.418970: mpc_dma_execute: mpc_dma_execute(): will_access_peripheral start cid=31 158.418976: mpc_dma_issue_pending <-mxcmci_request 158.418983: mxcmci_start_cmd <-mxcmci_request /* (2) mxcmci driver receive mmc irq */ 158.419656: mxcmci_irq <-handle_irq_event_percpu 158.419692: mxcmci_read_response <-mxcmci_irq /* (3) mxcmci driver checks that transfer is complete and call mxcmci_finish_data() */ 158.419726: mxcmci_data_done <-mxcmci_irq 158.419729: mxcmci_finish_data <-mxcmci_data_done 158.419733: dma_direct_unmap_sg <-mxcmci_finish_data 158.419736: mxcmci_swap_buffers.isra.24 <-mxcmci_finish_data 158.419762: mxcmci_read_response <-mxcmci_data_done /* (4) mxcmci driver (no dma): send stop command */ 158.419765: mxcmci_start_cmd <-mxcmci_data_done /* (5) mxcmci driver (no dma): receive the stop command irq response */ 158.419782: mxcmci_irq <-handle_irq_event_percpu 158.419812: mxcmci_read_response <-mxcmci_irq 158.419843: mxcmci_finish_request <-mxcmci_irq /* (6) dma driver: receive dma irq (finish data transfer) related by request on step 1 */ 158.419853: mpc_dma_irq <-handle_irq_event_percpu 158.420001: mpc_dma_irq_process <-mpc_dma_irq 158.420004: mpc_dma_irq_process <-mpc_dma_irq /* (7) dma driver: start dma tasklet to finish the dma irq handling */ 158.420008: mpc_dma_irq_process: mpc_dma_irq_process(): completed ch:31 /* (8) mxcmci driver: start next data transfer using dma */ 158.420174: mxcmci_request <-mmc_start_req 158.420182: dma_direct_map_sg <-mxcmci_request 158.420192: mpc_dma_prep_slave_sg <-mxcmci_request /* (9) dma driver: schedule irq tasklet and execute mxcmci dma driver callback */ 158.420250: mpc_dma_tasklet <-tasklet_action 158.420254: mpc_dma_process_completed <-tasklet_action 158.420267: mxcmci_dma_callback <-mpc_dma_process_completed /* ERROR!!! (10) mxcmci driver callback works on dma data related to the step 1 that is already finished */ 158.420271: mxcmci_data_done <-mpc_dma_process_completed 158.420273: mxcmci_finish_data <-mxcmci_data_done /* ERROR!!! (11) mxcmci driver: clear data that should be used by step 8 and send an other mmc stop command (already sended on step 4) */ 158.420276: dma_direct_unmap_sg <-mxcmci_finish_data 158.420279: mxcmci_swap_buffers.isra.24 <-mxcmci_finish_data 158.420330: mxcmci_read_response <-mxcmci_data_done 158.420333: mxcmci_start_cmd <-mxcmci_data_done 158.420338: dma_run_dependencies <-mpc_dma_process_completed ... ... ... 168.474223: mxcmci_watchdog <-call_timer_fn 168.474236: mxcmci_watchdog: mxcmci_watchdog 168.474397: mpc_dma_device_control <-mxcmci_watchdog In accordance with the other drivers that using the dma engine, fix it, leaving *only* to dma driver the complete control to ending the read operation. Removing STATUS_READ_OP_DONE event activation, has as effect to force mxcmci driver to handle the finish data transfer only by mxcmci dma callback. Signed-off-by: Matteo Facchinetti Acked-by: Sascha Hauer Signed-off-by: Ulf Hansson --- drivers/mmc/host/mxcmmc.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index ad11142..536a898 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -373,13 +373,9 @@ static void mxcmci_dma_callback(void *data) del_timer(&host->watchdog); stat = mxcmci_readl(host, MMC_REG_STATUS); - mxcmci_writel(host, stat & ~STATUS_DATA_TRANS_DONE, MMC_REG_STATUS); dev_dbg(mmc_dev(host->mmc), "%s: 0x%08x\n", __func__, stat); - if (stat & STATUS_READ_OP_DONE) - mxcmci_writel(host, STATUS_READ_OP_DONE, MMC_REG_STATUS); - mxcmci_data_done(host, stat); } @@ -743,10 +739,8 @@ static irqreturn_t mxcmci_irq(int irq, void *devid) sdio_irq = (stat & STATUS_SDIO_INT_ACTIVE) && host->use_sdio; spin_unlock_irqrestore(&host->lock, flags); - if (mxcmci_use_dma(host) && - (stat & (STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE))) - mxcmci_writel(host, STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE, - MMC_REG_STATUS); + if (mxcmci_use_dma(host) && (stat & (STATUS_WRITE_OP_DONE))) + mxcmci_writel(host, STATUS_WRITE_OP_DONE, MMC_REG_STATUS); if (sdio_irq) { mxcmci_writel(host, STATUS_SDIO_INT_ACTIVE, MMC_REG_STATUS); @@ -756,8 +750,7 @@ static irqreturn_t mxcmci_irq(int irq, void *devid) if (stat & STATUS_END_CMD_RESP) mxcmci_cmd_done(host, stat); - if (mxcmci_use_dma(host) && - (stat & (STATUS_DATA_TRANS_DONE | STATUS_WRITE_OP_DONE))) { + if (mxcmci_use_dma(host) && (stat & STATUS_WRITE_OP_DONE)) { del_timer(&host->watchdog); mxcmci_data_done(host, stat); } -- 2.7.4