Re: [PATCHv2 1/4] dmaengine: rcar-dmac: use result of updated get_residue in tx_status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30.06.2016 17:15, Niklas Söderlund wrote:
From: Muhammad Hamza Farooq <mfarooq@xxxxxxxxxxx>

The hardware might have complete the transfer but the interrupt handler
might not have had a chance to run. If rcar_dmac_chan_get_residue()
which reads HW registers finds that there is no residue return
DMA_COMPLETE.

Signed-off-by: Muhammad Hamza Farooq <mfarooq@xxxxxxxxxxx>
Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
[Niklas: add explanation in commit message]
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
  drivers/dma/sh/rcar-dmac.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index dfb1792..791a064 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1202,6 +1202,10 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
  	residue = rcar_dmac_chan_get_residue(rchan, cookie);
  	spin_unlock_irqrestore(&rchan->lock, flags);
+ /* if there's no residue, the cookie is complete */
+	if (!residue)
+		return DMA_COMPLETE;
+
  	dma_set_residue(txstate, residue);
return status;


We have some doubts about this change (mainline commit [1]). Not being a DMA expert, let me try to explain:

We are configuring a cyclic, never ending DMA (dmaengine_prep_dma_cyclic()). For this cyclic DMA we continuously poll the residue (txstate->residue) via rcar_dmac_tx_status(). Having a cyclic, never ending DMA we think that residue == 0 is a valid value. However, with above change, a residue value of 0 is 'dropped' and never written via dma_set_residue() to txstate. So in case we continuously poll, this value is lost, resulting in wrong behavior of the caller.

In our case with cyclic, never ending DMA, changing this to

--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1388,7 +1388,7 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,

        /* if there's no residue, the cookie is complete */
        if (!residue)
-               return DMA_COMPLETE;
+               status = DMA_COMPLETE;

        dma_set_residue(txstate, residue);

seems to help. If we ignore the still wrong DMA_COMPLETE status (which in case of cyclic DMA doesn't make any sense?) the caller get the correct txstate->residue values (not dropping the 0).

So maybe we need anything like

--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1387,7 +1387,7 @@ static enum dma_status rcar_dmac_tx_status(struct dma_chan *chan,
        spin_unlock_irqrestore(&rchan->lock, flags);

        /* if there's no residue, the cookie is complete */
-       if (!residue)
+       if (!residue && !chan->desc.running->cyclic)
                return DMA_COMPLETE;

        dma_set_residue(txstate, residue);

?

Opinions?

Best regards

Dirk

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/dma/sh/rcar-dmac.c?h=v5.0-rc2&id=3544d2878817bd139dda238cdd86a15e1c03d037



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux