dma: sh: rcar-dmac: avoid to write CHCR.TE to 1 if TCR is set to 0

dma: sh: rcar-dmac: avoid to write CHCR.TE to 1 if TCR is set to 0

[Vinod Koul]

On 02-07-18, 18:18, Yoshihiro Shimoda wrote:
> This patch fixes an issue that unexpected retransfering happens
> if TCR is set to 0 before rcar_dmac_sync_tcr() writes DE bit to
> the CHCR register. For example, sh-sci driver can reproduce this
> issue like below:
> 
>  In rx_timer_fn():		/* CHCR DE bit may be set to 1 */
>   dmaengine_tx_status()
>    rcar_dmac_tx_status()
>     rcar_dmac_chan_get_residue()
>      rcar_dmac_sync_tcr()	/* TCR is possible to be set to 0 */
> 
> According to the description of commit 73a47bd0da66 ("dmaengine:
> rcar-dmac: use TCRB instead of TCR for residue"), "this buffered data
> will be transferred if CHCR::DE bit was cleared". So, this patch
> doesn't need to check TCRB register.
> 
> Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  This patch is based on the latest slave-dma / fixes branch.
> 
>  This issue can be reproduced by the following commands on r8a7795
> Salvator-XS and Windows Teraterm :) :
>  # stty 921600
>  (Change Teraterm baud rate)
>  # cat > rx.txt
>  (Send 5MiB file by Teraterm. After a few minutes later, we cannot
>   input any commands by the serial console.)

Ack/tested-by ..?

> 
>  drivers/dma/sh/rcar-dmac.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 2a2ccd9..8305a1c 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -774,8 +774,9 @@ static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
>  	/* make sure all remaining data was flushed */
>  	rcar_dmac_chcr_de_barrier(chan);
>  
> -	/* back DE */
> -	rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr);
> +	/* 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)
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

dma: sh: rcar-dmac: avoid to write CHCR.TE to 1 if TCR is set to 0

[Geert Uytterhoeven]

On Mon, Jul 2, 2018 at 11:41 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This patch fixes an issue that unexpected retransfering happens
> if TCR is set to 0 before rcar_dmac_sync_tcr() writes DE bit to
> the CHCR register. For example, sh-sci driver can reproduce this
> issue like below:
>
>  In rx_timer_fn():              /* CHCR DE bit may be set to 1 */
>   dmaengine_tx_status()
>    rcar_dmac_tx_status()
>     rcar_dmac_chan_get_residue()
>      rcar_dmac_sync_tcr()       /* TCR is possible to be set to 0 */
>
> According to the description of commit 73a47bd0da66 ("dmaengine:
> rcar-dmac: use TCRB instead of TCR for residue"), "this buffered data
> will be transferred if CHCR::DE bit was cleared". So, this patch
> doesn't need to check TCRB register.
>
> Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Looks reasonable
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

dma: sh: rcar-dmac: avoid to write CHCR.TE to 1 if TCR is set to 0

[Vinod Koul]

On 02-07-18, 18:18, Yoshihiro Shimoda wrote:
> This patch fixes an issue that unexpected retransfering happens
> if TCR is set to 0 before rcar_dmac_sync_tcr() writes DE bit to
> the CHCR register. For example, sh-sci driver can reproduce this
> issue like below:
> 
>  In rx_timer_fn():		/* CHCR DE bit may be set to 1 */
>   dmaengine_tx_status()
>    rcar_dmac_tx_status()
>     rcar_dmac_chan_get_residue()
>      rcar_dmac_sync_tcr()	/* TCR is possible to be set to 0 */
> 
> According to the description of commit 73a47bd0da66 ("dmaengine:
> rcar-dmac: use TCRB instead of TCR for residue"), "this buffered data
> will be transferred if CHCR::DE bit was cleared". So, this patch
> doesn't need to check TCRB register.
> 
> Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  This patch is based on the latest slave-dma / fixes branch.
> 
>  This issue can be reproduced by the following commands on r8a7795
> Salvator-XS and Windows Teraterm :) :
>  # stty 921600
>  (Change Teraterm baud rate)
>  # cat > rx.txt
>  (Send 5MiB file by Teraterm. After a few minutes later, we cannot
>   input any commands by the serial console.)

Applied after fixing subsystem name

dma: sh: rcar-dmac: avoid to write CHCR.TE to 1 if TCR is set to 0

[Yoshihiro Shimoda]

This patch fixes an issue that unexpected retransfering happens
if TCR is set to 0 before rcar_dmac_sync_tcr() writes DE bit to
the CHCR register. For example, sh-sci driver can reproduce this
issue like below:

 In rx_timer_fn():		/* CHCR DE bit may be set to 1 */
  dmaengine_tx_status()
   rcar_dmac_tx_status()
    rcar_dmac_chan_get_residue()
     rcar_dmac_sync_tcr()	/* TCR is possible to be set to 0 */

According to the description of commit 73a47bd0da66 ("dmaengine:
rcar-dmac: use TCRB instead of TCR for residue"), "this buffered data
will be transferred if CHCR::DE bit was cleared". So, this patch
doesn't need to check TCRB register.

Fixes: 73a47bd0da66 ("dmaengine: rcar-dmac: use TCRB instead of TCR for residue")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 This patch is based on the latest slave-dma / fixes branch.

 This issue can be reproduced by the following commands on r8a7795
Salvator-XS and Windows Teraterm :) :
 # stty 921600
 (Change Teraterm baud rate)
 # cat > rx.txt
 (Send 5MiB file by Teraterm. After a few minutes later, we cannot
  input any commands by the serial console.)

 drivers/dma/sh/rcar-dmac.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 2a2ccd9..8305a1c 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -774,8 +774,9 @@ static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
 	/* make sure all remaining data was flushed */
 	rcar_dmac_chcr_de_barrier(chan);
 
-	/* back DE */
-	rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr);
+	/* 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)