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.pinch...@ideasonboard.com>
Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
---
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to