[PATCH v2 0/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[PATCH v2 0/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Yoshihiro Shimoda]

This patch set is based on the lasest slave-dma / next branch.

This issue can be reproduced by the following commands on r8a7795
Salvator-XS with the fixed patch [1] and Windows Teraterm :) :
 # stty 921600
 (Change Teraterm baud rate)
 # cat > rx.txt
 (Send 5MiB file by Teraterm.)
 # cmp <somewhere the 5MiB file> rx.txt
 output "differ: byte NNNNN, line MMM"

[1] https://patchwork.kernel.org/patch/10500775/

Changes from v1:
 - Rebase the latest slave-dma / next branch.
 - Fix the subsystem name in each subject.
 - Minor fix the commit log in patch 2.

Yoshihiro Shimoda (2):
  dmaengine: sh: rcar-dmac: add a new function to clear CHCR.DE with
    barrier
  dmaengine: sh: rcar-dmac: Add dma_pause operation

 drivers/dma/sh/rcar-dmac.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

-- 
1.9.1

[v2,1/2] dmaengine: sh: rcar-dmac: add a new function to clear CHCR.DE with barrier

[Yoshihiro Shimoda]

This patch adds a new function rcar_dmac_clear_chcr_de() to simplify
adding pause function later.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 9906a9c..d3b7388 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -759,18 +759,25 @@ static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
 	dev_err(chan->chan.device->dev, "CHCR DE check error\n");
 }
 
-static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
+static void rcar_dmac_clear_chcr_de(struct rcar_dmac_chan *chan)
 {
 	u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
 
-	if (!(chcr & RCAR_DMACHCR_DE))
-		return;
-
 	/* set DE=0 and flush remaining data */
 	rcar_dmac_chan_write(chan, RCAR_DMACHCR, (chcr & ~RCAR_DMACHCR_DE));
 
 	/* make sure all remaining data was flushed */
 	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))

[PATCH v2 1/2] dmaengine: sh: rcar-dmac: add a new function to clear CHCR.DE with barrier

[Yoshihiro Shimoda]

This patch adds a new function rcar_dmac_clear_chcr_de() to simplify
adding pause function later.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 9906a9c..d3b7388 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -759,18 +759,25 @@ static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
 	dev_err(chan->chan.device->dev, "CHCR DE check error\n");
 }
 
-static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
+static void rcar_dmac_clear_chcr_de(struct rcar_dmac_chan *chan)
 {
 	u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
 
-	if (!(chcr & RCAR_DMACHCR_DE))
-		return;
-
 	/* set DE=0 and flush remaining data */
 	rcar_dmac_chan_write(chan, RCAR_DMACHCR, (chcr & ~RCAR_DMACHCR_DE));
 
 	/* make sure all remaining data was flushed */
 	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))
-- 
1.9.1

[v2,2/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Yoshihiro Shimoda]

This patch adds dma_pause operation. This patch is based on
Muhammad Hamza Farooq's patch.

After this patch applied, an issue that the sh-sci driver with
high baud rate might cause data lost disappeared because the DMAC
is possible to transmit between [1] and [2] below, and then
the residue of [1] is not true:

In rx_timer_fn() of the sh-sci.c:
	dmaengine_pause();
	...
	dmaengine_tx_status();		/* [1] */
	...
	dmaengine_terminate_all();	/* [2] */

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index d3b7388..be82d69 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -834,6 +834,17 @@ static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
 	}
 }
 
+static int rcar_dmac_chan_pause(struct dma_chan *chan)
+{
+	unsigned long flags;
+	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+	spin_lock_irqsave(&rchan->lock, flags);
+	rcar_dmac_clear_chcr_de(rchan);
+	spin_unlock_irqrestore(&rchan->lock, flags);
+
+	return 0;
+}
 
 /* -----------------------------------------------------------------------------
  * Descriptors preparation
@@ -1864,6 +1875,7 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 	engine->device_prep_slave_sg		= rcar_dmac_prep_slave_sg;
 	engine->device_prep_dma_cyclic		= rcar_dmac_prep_dma_cyclic;
 	engine->device_config			= rcar_dmac_device_config;
+	engine->device_pause			= rcar_dmac_chan_pause;
 	engine->device_terminate_all		= rcar_dmac_chan_terminate_all;
 	engine->device_tx_status		= rcar_dmac_tx_status;
 	engine->device_issue_pending		= rcar_dmac_issue_pending;

[PATCH v2 2/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Yoshihiro Shimoda]

This patch adds dma_pause operation. This patch is based on
Muhammad Hamza Farooq's patch.

After this patch applied, an issue that the sh-sci driver with
high baud rate might cause data lost disappeared because the DMAC
is possible to transmit between [1] and [2] below, and then
the residue of [1] is not true:

In rx_timer_fn() of the sh-sci.c:
	dmaengine_pause();
	...
	dmaengine_tx_status();		/* [1] */
	...
	dmaengine_terminate_all();	/* [2] */

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index d3b7388..be82d69 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -834,6 +834,17 @@ static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
 	}
 }
 
+static int rcar_dmac_chan_pause(struct dma_chan *chan)
+{
+	unsigned long flags;
+	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+	spin_lock_irqsave(&rchan->lock, flags);
+	rcar_dmac_clear_chcr_de(rchan);
+	spin_unlock_irqrestore(&rchan->lock, flags);
+
+	return 0;
+}
 
 /* -----------------------------------------------------------------------------
  * Descriptors preparation
@@ -1864,6 +1875,7 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 	engine->device_prep_slave_sg		= rcar_dmac_prep_slave_sg;
 	engine->device_prep_dma_cyclic		= rcar_dmac_prep_dma_cyclic;
 	engine->device_config			= rcar_dmac_device_config;
+	engine->device_pause			= rcar_dmac_chan_pause;
 	engine->device_terminate_all		= rcar_dmac_chan_terminate_all;
 	engine->device_tx_status		= rcar_dmac_tx_status;
 	engine->device_issue_pending		= rcar_dmac_issue_pending;
-- 
1.9.1

Re: [PATCH v2 0/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Vinod]

On 11-07-18, 11:10, Yoshihiro Shimoda wrote:
> This patch set is based on the lasest slave-dma / next branch.
> 
> This issue can be reproduced by the following commands on r8a7795
> Salvator-XS with the fixed patch [1] and Windows Teraterm :) :
>  # stty 921600
>  (Change Teraterm baud rate)
>  # cat > rx.txt
>  (Send 5MiB file by Teraterm.)
>  # cmp <somewhere the 5MiB file> rx.txt
>  output "differ: byte NNNNN, line MMM"

Applied, thanks

-- 
~Vinod

[v2,1/2] dmaengine: sh: rcar-dmac: add a new function to clear CHCR.DE with barrier

[Geert Uytterhoeven]

On Wed, Jul 11, 2018 at 4:11 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This patch adds a new function rcar_dmac_clear_chcr_de() to simplify
> adding pause function later.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

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

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH v2 1/2] dmaengine: sh: rcar-dmac: add a new function to clear CHCR.DE with barrier

[Geert Uytterhoeven]

On Wed, Jul 11, 2018 at 4:11 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This patch adds a new function rcar_dmac_clear_chcr_de() to simplify
> adding pause function later.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 0/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Geert Uytterhoeven]

Hi Vinod,

On Wed, Jul 11, 2018 at 7:18 AM Vinod <vkoul@kernel.org> wrote:
> On 11-07-18, 11:10, Yoshihiro Shimoda wrote:
> > This patch set is based on the lasest slave-dma / next branch.
> >
> > This issue can be reproduced by the following commands on r8a7795
> > Salvator-XS with the fixed patch [1] and Windows Teraterm :) :
> >  # stty 921600
> >  (Change Teraterm baud rate)
> >  # cat > rx.txt
> >  (Send 5MiB file by Teraterm.)
> >  # cmp <somewhere the 5MiB file> rx.txt
> >  output "differ: byte NNNNN, line MMM"
>
> Applied, thanks

I can't seem to find this patch in today's next, nor in your next or
topic/renesas branches.

Am I looking at the wrong places?
Thanks, and have a nice weekend!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 0/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Vinod]

Hey Geert,

On 20-07-18, 15:06, Geert Uytterhoeven wrote:
> On Wed, Jul 11, 2018 at 7:18 AM Vinod <vkoul@kernel.org> wrote:
> > On 11-07-18, 11:10, Yoshihiro Shimoda wrote:
> > > This patch set is based on the lasest slave-dma / next branch.
> > >
> > > This issue can be reproduced by the following commands on r8a7795
> > > Salvator-XS with the fixed patch [1] and Windows Teraterm :) :
> > >  # stty 921600
> > >  (Change Teraterm baud rate)
> > >  # cat > rx.txt
> > >  (Send 5MiB file by Teraterm.)
> > >  # cmp <somewhere the 5MiB file> rx.txt
> > >  output "differ: byte NNNNN, line MMM"
> >
> > Applied, thanks
> 
> I can't seem to find this patch in today's next, nor in your next or
> topic/renesas branches.
> 
> Am I looking at the wrong places?

Yes you are looking at the right place.

I seem to have applied but missed to merge to next and push.

Done now
-- 
~Vinod

Re: [PATCH v2 0/2] dmaengine: sh: rcar-dmac: Add dma_pause operation

[Geert Uytterhoeven]

Hi Vinod,

On Fri, Jul 20, 2018 at 6:08 PM Vinod <vkoul@kernel.org> wrote:
> On 20-07-18, 15:06, Geert Uytterhoeven wrote:
> > On Wed, Jul 11, 2018 at 7:18 AM Vinod <vkoul@kernel.org> wrote:
> > > On 11-07-18, 11:10, Yoshihiro Shimoda wrote:
> > > > This patch set is based on the lasest slave-dma / next branch.
> > > >
> > > > This issue can be reproduced by the following commands on r8a7795
> > > > Salvator-XS with the fixed patch [1] and Windows Teraterm :) :
> > > >  # stty 921600
> > > >  (Change Teraterm baud rate)
> > > >  # cat > rx.txt
> > > >  (Send 5MiB file by Teraterm.)
> > > >  # cmp <somewhere the 5MiB file> rx.txt
> > > >  output "differ: byte NNNNN, line MMM"
> > >
> > > Applied, thanks
> >
> > I can't seem to find this patch in today's next, nor in your next or
> > topic/renesas branches.
> >
> > Am I looking at the wrong places?
>
> Yes you are looking at the right place.
>
> I seem to have applied but missed to merge to next and push.
>
> Done now

Confirmed, thanks again!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds