diff mbox series

dmaengine: sh: rcar-dmac: Should not stop the DMAC by rcar_dmac_sync_tcr()

Message ID 1532507224-4213-1-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Accepted
Headers show
Series dmaengine: sh: rcar-dmac: Should not stop the DMAC by rcar_dmac_sync_tcr() | expand

Commit Message

Yoshihiro Shimoda July 25, 2018, 8:27 a.m. UTC
rcar_dmac_chan_get_residue() should not stop the DMAC, because
the commit 538603c6026c ("dmaengine: sh: rcar-dmac: avoid to write
CHCR.TE to 1 if TCR is set to 0") had fixed unexpected re-transferring
issue. But it had caused the next issue which might stop the cyclic
mode transferring. Thus, for example R-Car sound might be stopped
suddenly.

According to the commit 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB
instead of TCR for residue"), the purpose of clearing CHCR.DE bit is
flushing buffered data to calculate the exact residue.

Such the "exact" residue had been required by sh-sci driver. sh-sci
driver is calling dmaengine_pause() to stop transferring, and get
"exact" residue. Otherwise, it might receive extra data during
getting residue without pausing.

In rx_timer_fn() of sh-sci driver:
	dmaengine_tx_status();		/* For checking roughly */
 	dmaengine_pause();
	dmaengine_tx_status();		/* For getting residue */
	dmaengine_terminate_all();

But, unfortunately the rcar-dmac driver didn't support dmaengine_pause()
at that time. So, the sh-sci driver cannot get the "exact" residue
without stopping the transferring, because rcar-dmac is buffering data
inside.

Because of these backgrounds, rcar-dmac had been cleared/set CHCR.DE
bit in rcar_dmac_chan_get_residue() to synchronizing data and getting
"exact" residue.

However, rcar-dmac driver has rcar_dmac_chan_pause() now, and clearing
CHCR.DE bit in rcar_dmac_chan_get_residue() doesn't need anymore.
So, this patch removes the rcar_dmac_sync_tcr().

Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 17 -----------------
 1 file changed, 17 deletions(-)

Comments

Geert Uytterhoeven July 30, 2018, 9:10 a.m. UTC | #1
On Wed, Jul 25, 2018 at 10:29 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> rcar_dmac_chan_get_residue() should not stop the DMAC, because
> the commit 538603c6026c ("dmaengine: sh: rcar-dmac: avoid to write
> CHCR.TE to 1 if TCR is set to 0") had fixed unexpected re-transferring
> issue. But it had caused the next issue which might stop the cyclic
> mode transferring. Thus, for example R-Car sound might be stopped
> suddenly.
>
> According to the commit 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB
> instead of TCR for residue"), the purpose of clearing CHCR.DE bit is
> flushing buffered data to calculate the exact residue.
>
> Such the "exact" residue had been required by sh-sci driver. sh-sci
> driver is calling dmaengine_pause() to stop transferring, and get
> "exact" residue. Otherwise, it might receive extra data during
> getting residue without pausing.
>
> In rx_timer_fn() of sh-sci driver:
>         dmaengine_tx_status();          /* For checking roughly */
>         dmaengine_pause();
>         dmaengine_tx_status();          /* For getting residue */
>         dmaengine_terminate_all();
>
> But, unfortunately the rcar-dmac driver didn't support dmaengine_pause()
> at that time. So, the sh-sci driver cannot get the "exact" residue
> without stopping the transferring, because rcar-dmac is buffering data
> inside.
>
> Because of these backgrounds, rcar-dmac had been cleared/set CHCR.DE
> bit in rcar_dmac_chan_get_residue() to synchronizing data and getting
> "exact" residue.
>
> However, rcar-dmac driver has rcar_dmac_chan_pause() now, and clearing
> CHCR.DE bit in rcar_dmac_chan_get_residue() doesn't need anymore.
> So, this patch removes the rcar_dmac_sync_tcr().
>
> Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>

Makes sense, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Vinod Koul July 31, 2018, 4:28 a.m. UTC | #2
On 25-07-18, 17:27, Yoshihiro Shimoda wrote:
> rcar_dmac_chan_get_residue() should not stop the DMAC, because
> the commit 538603c6026c ("dmaengine: sh: rcar-dmac: avoid to write
> CHCR.TE to 1 if TCR is set to 0") had fixed unexpected re-transferring
> issue. But it had caused the next issue which might stop the cyclic
> mode transferring. Thus, for example R-Car sound might be stopped
> suddenly.
> 
> According to the commit 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB
> instead of TCR for residue"), the purpose of clearing CHCR.DE bit is
> flushing buffered data to calculate the exact residue.
> 
> Such the "exact" residue had been required by sh-sci driver. sh-sci
> driver is calling dmaengine_pause() to stop transferring, and get
> "exact" residue. Otherwise, it might receive extra data during
> getting residue without pausing.
> 
> In rx_timer_fn() of sh-sci driver:
> 	dmaengine_tx_status();		/* For checking roughly */
>  	dmaengine_pause();
> 	dmaengine_tx_status();		/* For getting residue */
> 	dmaengine_terminate_all();
> 
> But, unfortunately the rcar-dmac driver didn't support dmaengine_pause()
> at that time. So, the sh-sci driver cannot get the "exact" residue
> without stopping the transferring, because rcar-dmac is buffering data
> inside.
> 
> Because of these backgrounds, rcar-dmac had been cleared/set CHCR.DE
> bit in rcar_dmac_chan_get_residue() to synchronizing data and getting
> "exact" residue.
> 
> However, rcar-dmac driver has rcar_dmac_chan_pause() now, and clearing
> CHCR.DE bit in rcar_dmac_chan_get_residue() doesn't need anymore.
> So, this patch removes the rcar_dmac_sync_tcr().

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index be82d69..48ee35e 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -770,20 +770,6 @@  static void rcar_dmac_clear_chcr_de(struct rcar_dmac_chan *chan)
 	rcar_dmac_chcr_de_barrier(chan);
 }
 
-static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
-{
-	u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
-
-	if (!(chcr & RCAR_DMACHCR_DE))
-		return;
-
-	rcar_dmac_clear_chcr_de(chan);
-
-	/* back DE if remain data exists */
-	if (rcar_dmac_chan_read(chan, RCAR_DMATCR))
-		rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr);
-}
-
 static void rcar_dmac_chan_halt(struct rcar_dmac_chan *chan)
 {
 	u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
@@ -1367,9 +1353,6 @@  static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan,
 		residue += chunk->size;
 	}
 
-	if (desc->direction == DMA_DEV_TO_MEM)
-		rcar_dmac_sync_tcr(chan);
-
 	/* Add the residue for the current chunk. */
 	residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift;