[PATCH 2/2] spi: sh-msiof: Fix leaking of unused DMA descriptors

From: Geert Uytterhoeven
Date: Fri Jul 11 2014 - 12:06:55 EST


If dmaengine_prep_slave_sg() or dmaengine_submit() fail, we may leak
unused DMA descriptors.

As per Documentation/dmaengine.txt, once a DMA descriptor has been
obtained, it must be submitted. Hence:
- First prepare and submit all DMA descriptors,
- Prepare the SPI controller for DMA,
- Start DMA by calling dma_async_issue_pending(),
- Make sure to call dmaengine_terminate_all() on all descriptors that
haven't completed.

Reported-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
---
drivers/spi/spi-sh-msiof.c | 71 +++++++++++++++++++++++++---------------------
1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 2a4354dcd661..887c2084130f 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -636,48 +636,38 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
dma_cookie_t cookie;
int ret;

- if (tx) {
- ier_bits |= IER_TDREQE | IER_TDMAE;
- dma_sync_single_for_device(p->master->dma_tx->device->dev,
- p->tx_dma_addr, len, DMA_TO_DEVICE);
- desc_tx = dmaengine_prep_slave_single(p->master->dma_tx,
- p->tx_dma_addr, len, DMA_TO_DEVICE,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
- if (!desc_tx)
- return -EAGAIN;
- }
-
+ /* First prepare and submit the DMA request(s), as this may fail */
if (rx) {
ier_bits |= IER_RDREQE | IER_RDMAE;
desc_rx = dmaengine_prep_slave_single(p->master->dma_rx,
p->rx_dma_addr, len, DMA_FROM_DEVICE,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
- if (!desc_rx)
- return -EAGAIN;
- }
-
- /* 1 stage FIFO watermarks for DMA */
- sh_msiof_write(p, FCTR, FCTR_TFWM_1 | FCTR_RFWM_1);
-
- /* setup msiof transfer mode registers (32-bit words) */
- sh_msiof_spi_set_mode_regs(p, tx, rx, 32, len / 4);
-
- sh_msiof_write(p, IER, ier_bits);
-
- reinit_completion(&p->done);
+ if (!desc_rx) {
+ ret = -EAGAIN;
+ goto no_dma_rx;
+ }

- if (rx) {
desc_rx->callback = sh_msiof_dma_complete;
desc_rx->callback_param = p;
cookie = dmaengine_submit(desc_rx);
if (dma_submit_error(cookie)) {
ret = cookie;
- goto stop_ier;
+ goto no_dma_rx;
}
- dma_async_issue_pending(p->master->dma_rx);
}

if (tx) {
+ ier_bits |= IER_TDREQE | IER_TDMAE;
+ dma_sync_single_for_device(p->master->dma_tx->device->dev,
+ p->tx_dma_addr, len, DMA_TO_DEVICE);
+ desc_tx = dmaengine_prep_slave_single(p->master->dma_tx,
+ p->tx_dma_addr, len, DMA_TO_DEVICE,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!desc_tx) {
+ ret = -EAGAIN;
+ goto no_dma_tx;
+ }
+
if (rx) {
/* No callback */
desc_tx->callback = NULL;
@@ -688,15 +678,30 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
cookie = dmaengine_submit(desc_tx);
if (dma_submit_error(cookie)) {
ret = cookie;
- goto stop_rx;
+ goto no_dma_tx;
}
- dma_async_issue_pending(p->master->dma_tx);
}

+ /* 1 stage FIFO watermarks for DMA */
+ sh_msiof_write(p, FCTR, FCTR_TFWM_1 | FCTR_RFWM_1);
+
+ /* setup msiof transfer mode registers (32-bit words) */
+ sh_msiof_spi_set_mode_regs(p, tx, rx, 32, len / 4);
+
+ sh_msiof_write(p, IER, ier_bits);
+
+ reinit_completion(&p->done);
+
+ /* Now start DMA */
+ if (tx)
+ dma_async_issue_pending(p->master->dma_rx);
+ if (rx)
+ dma_async_issue_pending(p->master->dma_tx);
+
ret = sh_msiof_spi_start(p, rx);
if (ret) {
dev_err(&p->pdev->dev, "failed to start hardware\n");
- goto stop_tx;
+ goto stop_dma;
}

/* wait for tx fifo to be emptied / rx fifo to be filled */
@@ -726,14 +731,14 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
stop_reset:
sh_msiof_reset_str(p);
sh_msiof_spi_stop(p, rx);
-stop_tx:
+stop_dma:
if (tx)
dmaengine_terminate_all(p->master->dma_tx);
-stop_rx:
+no_dma_tx:
if (rx)
dmaengine_terminate_all(p->master->dma_rx);
-stop_ier:
sh_msiof_write(p, IER, 0);
+no_dma_rx:
return ret;
}

--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/