diff options
author | Matteo Facchinetti <matteo.facchinetti@sirius-es.it> | 2014-09-30 10:59:37 -0400 |
---|---|---|
committer | Ulf Hansson <ulf.hansson@linaro.org> | 2014-11-10 06:40:25 -0500 |
commit | adfa57033d73dfc3449f3a9a2fe5450673b5aef3 (patch) | |
tree | c73b34cc20feefe49a01b126709b83c2f4b89db0 | |
parent | 6380ea099cdd46d7377b6fbec0291cf2aa387bad (diff) |
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 <matteo.facchinetti@sirius-es.it>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
-rw-r--r-- | drivers/mmc/host/mxcmmc.c | 13 |
1 files changed, 3 insertions, 10 deletions
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index ad111422ad55..536a8983c3be 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c | |||
@@ -373,13 +373,9 @@ static void mxcmci_dma_callback(void *data) | |||
373 | del_timer(&host->watchdog); | 373 | del_timer(&host->watchdog); |
374 | 374 | ||
375 | stat = mxcmci_readl(host, MMC_REG_STATUS); | 375 | stat = mxcmci_readl(host, MMC_REG_STATUS); |
376 | mxcmci_writel(host, stat & ~STATUS_DATA_TRANS_DONE, MMC_REG_STATUS); | ||
377 | 376 | ||
378 | dev_dbg(mmc_dev(host->mmc), "%s: 0x%08x\n", __func__, stat); | 377 | dev_dbg(mmc_dev(host->mmc), "%s: 0x%08x\n", __func__, stat); |
379 | 378 | ||
380 | if (stat & STATUS_READ_OP_DONE) | ||
381 | mxcmci_writel(host, STATUS_READ_OP_DONE, MMC_REG_STATUS); | ||
382 | |||
383 | mxcmci_data_done(host, stat); | 379 | mxcmci_data_done(host, stat); |
384 | } | 380 | } |
385 | 381 | ||
@@ -743,10 +739,8 @@ static irqreturn_t mxcmci_irq(int irq, void *devid) | |||
743 | sdio_irq = (stat & STATUS_SDIO_INT_ACTIVE) && host->use_sdio; | 739 | sdio_irq = (stat & STATUS_SDIO_INT_ACTIVE) && host->use_sdio; |
744 | spin_unlock_irqrestore(&host->lock, flags); | 740 | spin_unlock_irqrestore(&host->lock, flags); |
745 | 741 | ||
746 | if (mxcmci_use_dma(host) && | 742 | if (mxcmci_use_dma(host) && (stat & (STATUS_WRITE_OP_DONE))) |
747 | (stat & (STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE))) | 743 | mxcmci_writel(host, STATUS_WRITE_OP_DONE, MMC_REG_STATUS); |
748 | mxcmci_writel(host, STATUS_READ_OP_DONE | STATUS_WRITE_OP_DONE, | ||
749 | MMC_REG_STATUS); | ||
750 | 744 | ||
751 | if (sdio_irq) { | 745 | if (sdio_irq) { |
752 | mxcmci_writel(host, STATUS_SDIO_INT_ACTIVE, MMC_REG_STATUS); | 746 | mxcmci_writel(host, STATUS_SDIO_INT_ACTIVE, MMC_REG_STATUS); |
@@ -756,8 +750,7 @@ static irqreturn_t mxcmci_irq(int irq, void *devid) | |||
756 | if (stat & STATUS_END_CMD_RESP) | 750 | if (stat & STATUS_END_CMD_RESP) |
757 | mxcmci_cmd_done(host, stat); | 751 | mxcmci_cmd_done(host, stat); |
758 | 752 | ||
759 | if (mxcmci_use_dma(host) && | 753 | if (mxcmci_use_dma(host) && (stat & STATUS_WRITE_OP_DONE)) { |
760 | (stat & (STATUS_DATA_TRANS_DONE | STATUS_WRITE_OP_DONE))) { | ||
761 | del_timer(&host->watchdog); | 754 | del_timer(&host->watchdog); |
762 | mxcmci_data_done(host, stat); | 755 | mxcmci_data_done(host, stat); |
763 | } | 756 | } |