Re: [PATCH v4] dmaengine: omap-dma: add support for pause of non-cyclic transfers

[Vinod Koul]

On Fri, Oct 14, 2016 at 10:30:47AM +0530, Vignesh R wrote:
> From: Sebastian Andrzej Siewior 
> 
> This DMA driver is used by 8250-omap on DRA7-evm. There is one
> requirement that is to pause a transfer. This is currently used on the RX
> side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
> but the DMA controller starts the transfer shortly after.
> Before we can manually purge the FIFO we need to pause the transfer,
> check how many bytes it already received and terminate the transfer
> without it making any progress.
> 
> From testing on the TX side it seems that it is possible that we invoke
> pause once the transfer has completed which is indicated by the missing
> CCR_ENABLE bit but before the interrupt has been noticed. In that case the
> interrupt will come even after disabling it.
> 
> The AM572x manual says that we have to wait for the CCR_RD_ACTIVE &
> CCR_WR_ACTIVE bits to be gone before programming it again here is the
> drain loop. Also it looks like without the drain the TX-transfer makes
> sometimes progress.
> 
> One note: The pause + resume combo is broken because after resume the
> the complete transfer will be programmed again. That means the already
> transferred bytes (until the pause event) will be sent again. This is
> currently not important for my UART user because it does only pause +
> terminate.

Applied, thanks

-- 
~Vinod

Re: [PATCH v4] dmaengine: omap-dma: add support for pause of non-cyclic transfers

[Vinod Koul]

On Fri, Oct 14, 2016 at 10:30:47AM +0530, Vignesh R wrote:
> From: Sebastian Andrzej Siewior 
> 
> This DMA driver is used by 8250-omap on DRA7-evm. There is one
> requirement that is to pause a transfer. This is currently used on the RX
> side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
> but the DMA controller starts the transfer shortly after.
> Before we can manually purge the FIFO we need to pause the transfer,
> check how many bytes it already received and terminate the transfer
> without it making any progress.
> 
> From testing on the TX side it seems that it is possible that we invoke
> pause once the transfer has completed which is indicated by the missing
> CCR_ENABLE bit but before the interrupt has been noticed. In that case the
> interrupt will come even after disabling it.
> 
> The AM572x manual says that we have to wait for the CCR_RD_ACTIVE &
> CCR_WR_ACTIVE bits to be gone before programming it again here is the
> drain loop. Also it looks like without the drain the TX-transfer makes
> sometimes progress.
> 
> One note: The pause + resume combo is broken because after resume the
> the complete transfer will be programmed again. That means the already
> transferred bytes (until the pause event) will be sent again. This is
> currently not important for my UART user because it does only pause +
> terminate.

Applied, thanks

-- 
~Vinod

[PATCH v4] dmaengine: omap-dma: add support for pause of non-cyclic transfers

[Vignesh R]

From: Sebastian Andrzej Siewior 

This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.

>From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.

The AM572x manual says that we have to wait for the CCR_RD_ACTIVE &
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.

One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.

v3…v4:
 - update subject line.

v2…v3:
  - rephrase the comment based on Russell's information / feedback.

v1…v2:
  - move the drain loop into omap_dma_drain_chan() instead of having it
twice.
  - allow pause only for DMA_DEV_TO_MEM transfers if non-cyclic. Add a
comment why DMA_MEM_TO_DEV not allowed.
  - clear pause on terminate_all. Otherwise pause() + terminate_all()
will keep the pause bit set and we can't pause the following
transfer.

Signed-off-by: Sebastian Andrzej Siewior 
[vigne...@ti.com: drain channel only when buffering is on, rebase to v4.8]
Signed-off-by: Vignesh R 
Acked-by: Peter Ujfalusi 
---
 drivers/dma/omap-dma.c | 124 +++--
 1 file changed, 89 insertions(+), 35 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 7ca27d4b1c54..fd6b9e6834ad 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -422,7 +422,30 @@ static void omap_dma_start(struct omap_chan *c, struct 
omap_desc *d)
c->running = true;
 }
 
-static void omap_dma_stop(struct omap_chan *c)
+static void omap_dma_drain_chan(struct omap_chan *c)
+{
+   int i;
+   u32 val;
+
+   /* Wait for sDMA FIFO to drain */
+   for (i = 0; ; i++) {
+   val = omap_dma_chan_read(c, CCR);
+   if (!(val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+   break;
+
+   if (i > 100)
+   break;
+
+   udelay(5);
+   }
+
+   if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
+   dev_err(c->vc.chan.device->dev,
+   "DMA drain did not complete on lch %d\n",
+   c->dma_ch);
+}
+
+static int omap_dma_stop(struct omap_chan *c)
 {
struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
uint32_t val;
@@ -435,7 +458,6 @@ static void omap_dma_stop(struct omap_chan *c)
val = omap_dma_chan_read(c, CCR);
if (od->plat->errata & DMA_ERRATA_i541 && val & CCR_TRIGGER_SRC) {
uint32_t sysconfig;
-   unsigned i;
 
sysconfig = omap_dma_glbl_read(od, OCP_SYSCONFIG);
val = sysconfig & ~DMA_SYSCONFIG_MIDLEMODE_MASK;
@@ -446,27 +468,19 @@ static void omap_dma_stop(struct omap_chan *c)
val &= ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
 
-   /* Wait for sDMA FIFO to drain */
-   for (i = 0; ; i++) {
-   val = omap_dma_chan_read(c, CCR);
-   if (!(val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
-   break;
-
-   if (i > 100)
-   break;
-
-   udelay(5);
-   }
-
-   if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
-   dev_err(c->vc.chan.device->dev,
-   "DMA drain did not complete on lch %d\n",
-   c->dma_ch);
+   if (!(c->ccr & CCR_BUFFERING_DISABLE))
+   omap_dma_drain_chan(c);
 
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+   if (!(val & CCR_ENABLE))
+   return -EINVAL;
+
val &= ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+
+   if (!(c->ccr & CCR_BUFFERING_DISABLE))
+   omap_dma_drain_chan(c);
}
 
mb();
@@ -481,8 +495,8 @@ static void omap_dma_stop(struct omap_chan *c)
 
omap_dma_chan_write(c, CLNK_CTRL, val);
}
-

[PATCH v4] dmaengine: omap-dma: add support for pause of non-cyclic transfers

[Vignesh R]

From: Sebastian Andrzej Siewior 

This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.

>From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.

The AM572x manual says that we have to wait for the CCR_RD_ACTIVE &
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.

One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.

v3…v4:
 - update subject line.

v2…v3:
  - rephrase the comment based on Russell's information / feedback.

v1…v2:
  - move the drain loop into omap_dma_drain_chan() instead of having it
twice.
  - allow pause only for DMA_DEV_TO_MEM transfers if non-cyclic. Add a
comment why DMA_MEM_TO_DEV not allowed.
  - clear pause on terminate_all. Otherwise pause() + terminate_all()
will keep the pause bit set and we can't pause the following
transfer.

Signed-off-by: Sebastian Andrzej Siewior 
[vigne...@ti.com: drain channel only when buffering is on, rebase to v4.8]
Signed-off-by: Vignesh R 
Acked-by: Peter Ujfalusi 
---
 drivers/dma/omap-dma.c | 124 +++--
 1 file changed, 89 insertions(+), 35 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 7ca27d4b1c54..fd6b9e6834ad 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -422,7 +422,30 @@ static void omap_dma_start(struct omap_chan *c, struct 
omap_desc *d)
c->running = true;
 }
 
-static void omap_dma_stop(struct omap_chan *c)
+static void omap_dma_drain_chan(struct omap_chan *c)
+{
+   int i;
+   u32 val;
+
+   /* Wait for sDMA FIFO to drain */
+   for (i = 0; ; i++) {
+   val = omap_dma_chan_read(c, CCR);
+   if (!(val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+   break;
+
+   if (i > 100)
+   break;
+
+   udelay(5);
+   }
+
+   if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
+   dev_err(c->vc.chan.device->dev,
+   "DMA drain did not complete on lch %d\n",
+   c->dma_ch);
+}
+
+static int omap_dma_stop(struct omap_chan *c)
 {
struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
uint32_t val;
@@ -435,7 +458,6 @@ static void omap_dma_stop(struct omap_chan *c)
val = omap_dma_chan_read(c, CCR);
if (od->plat->errata & DMA_ERRATA_i541 && val & CCR_TRIGGER_SRC) {
uint32_t sysconfig;
-   unsigned i;
 
sysconfig = omap_dma_glbl_read(od, OCP_SYSCONFIG);
val = sysconfig & ~DMA_SYSCONFIG_MIDLEMODE_MASK;
@@ -446,27 +468,19 @@ static void omap_dma_stop(struct omap_chan *c)
val &= ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
 
-   /* Wait for sDMA FIFO to drain */
-   for (i = 0; ; i++) {
-   val = omap_dma_chan_read(c, CCR);
-   if (!(val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
-   break;
-
-   if (i > 100)
-   break;
-
-   udelay(5);
-   }
-
-   if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
-   dev_err(c->vc.chan.device->dev,
-   "DMA drain did not complete on lch %d\n",
-   c->dma_ch);
+   if (!(c->ccr & CCR_BUFFERING_DISABLE))
+   omap_dma_drain_chan(c);
 
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+   if (!(val & CCR_ENABLE))
+   return -EINVAL;
+
val &= ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+
+   if (!(c->ccr & CCR_BUFFERING_DISABLE))
+   omap_dma_drain_chan(c);
}
 
mb();
@@ -481,8 +495,8 @@ static void omap_dma_stop(struct omap_chan *c)
 
omap_dma_chan_write(c, CLNK_CTRL, val);
}
-
c->running = false;
+   return 0;
 }
 
 static void omap_dma_start_sg(struct