[PATCH 0/2 v2] dmaengine: rcar-dmac: use TCRB instead of TCR

[PATCH 0/2 v2] dmaengine: rcar-dmac: use TCRB instead of TCR

[Kuninori Morimoto]

Hi Vinod
Cc Geert, Laurent

These are v2 of dmaengine fixup for Renesas SoC.
v1 patch solved "sound noise" issue, but breaked "serial input".

The reason was that DMAC buffers read data until transferable size to write side.
This buffered data will be transfered if <1> it reached to transferable size,
<2> DE bit became 0.
In current driver, DE bit will be 0 if driver called dmaengine_terminate_all().

Thus, we need to flush this buffered data before getting residue.
And we need to check this DE bit was realy 0 when we cleared it.
Because DMAC will start to transfer bufferred data if DE was cleared, and then,
DE was 1 until finished.

I tested this patch with CONFIG_SERIAL_SH_SCI_DMA, and serial works correctly.

Kuninori Morimoto (2):
  dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear
  dmaengine: rcar-dmac: use TCRB instead of TCR for residue

-- 
1.9.1

[PATCH 1/2 v2] dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

DMAC reads data from source device, and buffered it until transferable
size for shink device. Because of this behavoir, DMAC is including
buffered data .

Now, CHCR DE bit is controlling DMA transfer enable/disable.

If DE bit was cleared during data transferring, or during buffering,
it will flush buffered data if source device was peripheral device
(The buffered data will be removed if source device was memory).
Because of this behavior, driver should ensure that DE bit is actually
0 after cleared.

This patch adds new rcar_dmac_chcr_de_barrier() and call it after CHCR
register access.

Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
Tested-by: Ryo Kodama <ryo.kodama.vz@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 2b2c7db..16ebd5d 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -10,6 +10,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/interrupt.h>
@@ -741,6 +742,24 @@ static int rcar_dmac_fill_hwdesc(struct rcar_dmac_chan *chan,
 /* -----------------------------------------------------------------------------
  * Stop and reset
  */
+static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
+{
+	u32 chcr;
+	int i;
+
+	/*
+	 * Ensure that the setting of the DE bit is actually 0 after
+	 * clearing it.
+	 */
+	for (i = 0; i < 1024; i++) {
+		chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
+		if (!(chcr & RCAR_DMACHCR_DE))
+			return;
+		udelay(1);
+	}
+
+	dev_err(chan->chan.device->dev, "CHCR DE check error\n");
+}
 
 static void rcar_dmac_chan_halt(struct rcar_dmac_chan *chan)
 {
@@ -749,6 +768,7 @@ static void rcar_dmac_chan_halt(struct rcar_dmac_chan *chan)
 	chcr &= ~(RCAR_DMACHCR_DSE | RCAR_DMACHCR_DSIE | RCAR_DMACHCR_IE |
 		  RCAR_DMACHCR_TE | RCAR_DMACHCR_DE);
 	rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr);
+	rcar_dmac_chcr_de_barrier(chan);
 }
 
 static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
@@ -1481,6 +1501,8 @@ static irqreturn_t rcar_dmac_isr_channel(int irq, void *dev)
 	if (chcr & RCAR_DMACHCR_TE)
 		mask |= RCAR_DMACHCR_DE;
 	rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr & ~mask);
+	if (mask & RCAR_DMACHCR_DE)
+		rcar_dmac_chcr_de_barrier(chan);
 
 	if (chcr & RCAR_DMACHCR_DSE)
 		ret |= rcar_dmac_isr_desc_stage_end(chan);
-- 
1.9.1

[PATCH 2/2 v2] dmaengine: rcar-dmac: use TCRB instead of TCR for residue

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

SYS/RT/Audio DMAC includes independent data buffers for reading
and writing. Therefore, the read transfer counter and write transfer
counter have different values.
TCR indicates read counter, and TCRB indicates write counter.
The relationship is like below.

	        TCR       TCRB
	[SOURCE] -> [DMAC] -> [SINK]

In the MEM_TO_DEV direction, what really matters is how much data has
been written to the device. If the DMA is interrupted between read and
write, then, the data doesn't end up in the destination, so shouldn't
be counted. TCRB is thus the register we should use in this cases.

In the DEV_TO_MEM direction, the situation is more complex. Both the
read and write side are important. What matters from a data consumer
point of view is how much data has been written to memory.
On the other hand, if the transfer is interrupted between read and
write, we'll end up losing data. It can also be important to report.

In the MEM_TO_MEM direction, what matters is of course how much data
has been written to memory from data consumer point of view.
Here, because read and write have independent data buffers, it will
take a while for TCR and TCRB to become equal. Thus we should check
TCRB in this case, too.

Thus, all cases we should check TCRB instead of TCR.

Without this patch, Sound Capture has noise after PluseAudio support
(= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because
the recorder will use wrong residue counter which indicates transferred
from sound device, but in reality the data was not yet put to memory
and recorder will record it.

However, because DMAC is buffering data until it can be transferable
size, TCRB might not be updated.
For example, if consumer doesn't know how much data can be receaved,
it requests enough size to DMAC. But in reality, it might receave very
few data. In such case, DMAC just buffered it untile transferable size,
and no TCRB updated.

In such case, this buffered data will be transferred if CHCR::DE bit was
cleared, and this is happen if rcar_dmac_chan_halt(). In other word, it
happen when consumer called dmaengine_terminate_all().

Because of this behavior, it need to flush buffered data when it returns
"residue" (= dmaengine_tx_status()).
Otherwise, consumer might calculate wrong things if it called
dmaengine_tx_status() and dmaengine_terminate_all() consecutively.

Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
Tested-by: Ryo Kodama <ryo.kodama.vz@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/dma/sh/rcar-dmac.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 16ebd5d..9cb85b2 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -761,6 +761,23 @@ 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)
+{
+	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 fulshed */
+	rcar_dmac_chcr_de_barrier(chan);
+
+	/* back DE */
+	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);
@@ -1329,8 +1346,11 @@ 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_DMATCR) << desc->xfer_shift;
+	residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift;
 
 	return residue;
 }
-- 
1.9.1

Re: [PATCH 1/2 v2] dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear

[Geert Uytterhoeven]

Hi Morimoto-san,

On Thu, Nov 16, 2017 at 5:33 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> DMAC reads data from source device, and buffered it until transferable
> size for shink device. Because of this behavoir, DMAC is including

sink, behavior

> buffered data .
>
> Now, CHCR DE bit is controlling DMA transfer enable/disable.
>
> If DE bit was cleared during data transferring, or during buffering,
> it will flush buffered data if source device was peripheral device
> (The buffered data will be removed if source device was memory).
> Because of this behavior, driver should ensure that DE bit is actually
> 0 after cleared.

clearing

> This patch adds new rcar_dmac_chcr_de_barrier() and call it after CHCR
> register access.
>
> Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Tested-by: Ryo Kodama <ryo.kodama.vz@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/dma/sh/rcar-dmac.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 2b2c7db..16ebd5d 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -10,6 +10,7 @@
>   * published by the Free Software Foundation.
>   */
>
> +#include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #include <linux/interrupt.h>
> @@ -741,6 +742,24 @@ static int rcar_dmac_fill_hwdesc(struct rcar_dmac_chan *chan,
>  /* -----------------------------------------------------------------------------
>   * Stop and reset
>   */
> +static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
> +{
> +       u32 chcr;
> +       int i;

unsigned int

> +
> +       /*
> +        * Ensure that the setting of the DE bit is actually 0 after
> +        * clearing it.
> +        */
> +       for (i = 0; i < 1024; i++) {
> +               chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
> +               if (!(chcr & RCAR_DMACHCR_DE))
> +                       return;
> +               udelay(1);
> +       }

What's a typical number of loops needed before DE is really cleared?

> +
> +       dev_err(chan->chan.device->dev, "CHCR DE check error\n");
> +}

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 2/2 v2] dmaengine: rcar-dmac: use TCRB instead of TCR for residue

[Geert Uytterhoeven]

Hi Morimoto-san,

On Thu, Nov 16, 2017 at 5:34 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> SYS/RT/Audio DMAC includes independent data buffers for reading
> and writing. Therefore, the read transfer counter and write transfer
> counter have different values.
> TCR indicates read counter, and TCRB indicates write counter.
> The relationship is like below.
>
>                 TCR       TCRB
>         [SOURCE] -> [DMAC] -> [SINK]
>
> In the MEM_TO_DEV direction, what really matters is how much data has
> been written to the device. If the DMA is interrupted between read and
> write, then, the data doesn't end up in the destination, so shouldn't
> be counted. TCRB is thus the register we should use in this cases.
>
> In the DEV_TO_MEM direction, the situation is more complex. Both the
> read and write side are important. What matters from a data consumer
> point of view is how much data has been written to memory.
> On the other hand, if the transfer is interrupted between read and
> write, we'll end up losing data. It can also be important to report.
>
> In the MEM_TO_MEM direction, what matters is of course how much data
> has been written to memory from data consumer point of view.
> Here, because read and write have independent data buffers, it will
> take a while for TCR and TCRB to become equal. Thus we should check
> TCRB in this case, too.
>
> Thus, all cases we should check TCRB instead of TCR.
>
> Without this patch, Sound Capture has noise after PluseAudio support

PulseAudio

> (= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because
> the recorder will use wrong residue counter which indicates transferred
> from sound device, but in reality the data was not yet put to memory
> and recorder will record it.
>
> However, because DMAC is buffering data until it can be transferable
> size, TCRB might not be updated.
> For example, if consumer doesn't know how much data can be receaved,

received

> it requests enough size to DMAC. But in reality, it might receave very

receive

> few data. In such case, DMAC just buffered it untile transferable size,

until

> and no TCRB updated.
>
> In such case, this buffered data will be transferred if CHCR::DE bit was
> cleared, and this is happen if rcar_dmac_chan_halt(). In other word, it
> happen when consumer called dmaengine_terminate_all().
>
> Because of this behavior, it need to flush buffered data when it returns
> "residue" (= dmaengine_tx_status()).
> Otherwise, consumer might calculate wrong things if it called
> dmaengine_tx_status() and dmaengine_terminate_all() consecutively.
>
> Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Tested-by: Ryo Kodama <ryo.kodama.vz@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

My serial console is happy again (on Koelsch and Salvator-XS), so:
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -761,6 +761,23 @@ 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)
> +{
> +       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 fulshed */

flushed

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 1/2 v2] dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear

[Kuninori Morimoto]

Hi Geert

> > DMAC reads data from source device, and buffered it until transferable
> > size for shink device. Because of this behavoir, DMAC is including
> 
> sink, behavior
(snip)
> > 0 after cleared.
> 
> clearing

Grr, thanks.

> > +static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
> > +{
> > +       u32 chcr;
> > +       int i;
> 
> unsigned int
> 
> > +
> > +       /*
> > +        * Ensure that the setting of the DE bit is actually 0 after
> > +        * clearing it.
> > +        */
> > +       for (i = 0; i < 1024; i++) {
> > +               chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
> > +               if (!(chcr & RCAR_DMACHCR_DE))
> > +                       return;
> > +               udelay(1);
> > +       }
> 
> What's a typical number of loops needed before DE is really cleared?

It case by case, but I don't want to use while(1) loop

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/2 v2] dmaengine: rcar-dmac: use TCRB instead of TCR for residue

[Kuninori Morimoto]

Hi Geert

Thank you for your review, and test.
But where is my brown paper bag for English spell ?

> 
> Hi Morimoto-san,
> 
> On Thu, Nov 16, 2017 at 5:34 AM, Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Thanks for your patch!
> 
> > SYS/RT/Audio DMAC includes independent data buffers for reading
> > and writing. Therefore, the read transfer counter and write transfer
> > counter have different values.
> > TCR indicates read counter, and TCRB indicates write counter.
> > The relationship is like below.
> >
> >                 TCR       TCRB
> >         [SOURCE] -> [DMAC] -> [SINK]
> >
> > In the MEM_TO_DEV direction, what really matters is how much data has
> > been written to the device. If the DMA is interrupted between read and
> > write, then, the data doesn't end up in the destination, so shouldn't
> > be counted. TCRB is thus the register we should use in this cases.
> >
> > In the DEV_TO_MEM direction, the situation is more complex. Both the
> > read and write side are important. What matters from a data consumer
> > point of view is how much data has been written to memory.
> > On the other hand, if the transfer is interrupted between read and
> > write, we'll end up losing data. It can also be important to report.
> >
> > In the MEM_TO_MEM direction, what matters is of course how much data
> > has been written to memory from data consumer point of view.
> > Here, because read and write have independent data buffers, it will
> > take a while for TCR and TCRB to become equal. Thus we should check
> > TCRB in this case, too.
> >
> > Thus, all cases we should check TCRB instead of TCR.
> >
> > Without this patch, Sound Capture has noise after PluseAudio support
> 
> PulseAudio
> 
> > (= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because
> > the recorder will use wrong residue counter which indicates transferred
> > from sound device, but in reality the data was not yet put to memory
> > and recorder will record it.
> >
> > However, because DMAC is buffering data until it can be transferable
> > size, TCRB might not be updated.
> > For example, if consumer doesn't know how much data can be receaved,
> 
> received
> 
> > it requests enough size to DMAC. But in reality, it might receave very
> 
> receive
> 
> > few data. In such case, DMAC just buffered it untile transferable size,
> 
> until
> 
> > and no TCRB updated.
> >
> > In such case, this buffered data will be transferred if CHCR::DE bit was
> > cleared, and this is happen if rcar_dmac_chan_halt(). In other word, it
> > happen when consumer called dmaengine_terminate_all().
> >
> > Because of this behavior, it need to flush buffered data when it returns
> > "residue" (= dmaengine_tx_status()).
> > Otherwise, consumer might calculate wrong things if it called
> > dmaengine_tx_status() and dmaengine_terminate_all() consecutively.
> >
> > Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> > Tested-by: Ryo Kodama <ryo.kodama.vz@renesas.com>
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> My serial console is happy again (on Koelsch and Salvator-XS), so:
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/drivers/dma/sh/rcar-dmac.c
> > +++ b/drivers/dma/sh/rcar-dmac.c
> > @@ -761,6 +761,23 @@ 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)
> > +{
> > +       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 fulshed */
> 
> flushed
> 
> 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 1/2 v2] dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear

[Geert Uytterhoeven]

Hi Morimoto-san,

On Fri, Nov 17, 2017 at 1:10 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>> > +static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
>> > +{
>> > +       u32 chcr;
>> > +       int i;
>>
>> unsigned int
>>
>> > +
>> > +       /*
>> > +        * Ensure that the setting of the DE bit is actually 0 after
>> > +        * clearing it.
>> > +        */
>> > +       for (i = 0; i < 1024; i++) {
>> > +               chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
>> > +               if (!(chcr & RCAR_DMACHCR_DE))
>> > +                       return;
>> > +               udelay(1);
>> > +       }
>>
>> What's a typical number of loops needed before DE is really cleared?
>
> It case by case, but I don't want to use while(1) loop

I understand that, and I agree wholeheartedly with limiting the number
of cycles.

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 1/2 v2] dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear

[Laurent Pinchart]

On Friday, 17 November 2017 10:41:05 EET Geert Uytterhoeven wrote:
> On Fri, Nov 17, 2017 at 1:10 AM, Kuninori Morimoto wrote:
> >>> +static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
> >>> +{
> >>> +       u32 chcr;
> >>> +       int i;
> >> 
> >> unsigned int
> >> 
> >>> +
> >>> +       /*
> >>> +        * Ensure that the setting of the DE bit is actually 0 after
> >>> +        * clearing it.
> >>> +        */
> >>> +       for (i = 0; i < 1024; i++) {
> >>> +               chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
> >>> +               if (!(chcr & RCAR_DMACHCR_DE))
> >>> +                       return;
> >>> +               udelay(1);
> >>> +       }
> >> 
> >> What's a typical number of loops needed before DE is really cleared?
> > 
> > It case by case, but I don't want to use while(1) loop
> 
> I understand that, and I agree wholeheartedly with limiting the number
> of cycles.

So do I, but I'd still like to know what the typical values are :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2 v2] dmaengine: rcar-dmac: ensure CHCR DE bit is actually 0 after clear

[Kuninori Morimoto]

Hi Laurent

> > >>> +static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
> > >>> +{
> > >>> +       u32 chcr;
> > >>> +       int i;
> > >> 
> > >> unsigned int
> > >> 
> > >>> +
> > >>> +       /*
> > >>> +        * Ensure that the setting of the DE bit is actually 0 after
> > >>> +        * clearing it.
> > >>> +        */
> > >>> +       for (i = 0; i < 1024; i++) {
> > >>> +               chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
> > >>> +               if (!(chcr & RCAR_DMACHCR_DE))
> > >>> +                       return;
> > >>> +               udelay(1);
> > >>> +       }
> > >> 
> > >> What's a typical number of loops needed before DE is really cleared?
> > > 
> > > It case by case, but I don't want to use while(1) loop
> > 
> > I understand that, and I agree wholeheartedly with limiting the number
> > of cycles.
> 
> So do I, but I'd still like to know what the typical values are :-)

It can buffering max 8 requests.
1 request needs max 20000 cycle to transfer.

	20000 cycle x 8 request x [4ns/cycle] = 640000[ns] = 640usec

1024usec is enough :)

Best regards
---
Kuninori Morimoto