dmaengine: dw: fix cyclic transfers

Commit 61e183f83069 ("dmaengine/dw_dmac: Reconfigure interrupt and chan_cfg register on resume") moved some channel initialisation to a new function which must be called before starting a transfer.

This updates dw_dma_cyclic_start() to use dwc_dostart() like the other modes, thus ensuring dwc_initialize() gets called and removing some code duplication.

Fixes 61e183f83069 ("dmaengine/dw_dmac: Reconfigure interrupt and chan_cfg register on resume")

Signed-off-by Mans Rullgard <[email protected]>

---
Changes:
- replace duplicated code by dwc_dostart() call

---
 drivers/dma/dw/core.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

Cyclic transfer callbacks rely on block completion interrupts which were disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts"). This re-enables block interrupts so the cyclic callbacks can work. Other transfer types are not affected as they set the INT_EN bit only on the last block.

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts")

Signed-off-by Mans Rullgard <[email protected]>

---
Changes:
- new patch

---
 drivers/dma/dw/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Quoted 6 lines · Mans Rullgard

Cyclic transfer callbacks rely on block completion interrupts which were disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts"). This re-enables block interrupts so the cyclic callbacks can work. Other transfer types are not affected as they set the INT_EN bit only on the last block.

Quoted 8 lines

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts")

Signed-off-by Mans Rullgard <[email protected]>

---
Changes:
- new patch

---
 drivers/dma/dw/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Reviewed-by Viresh Kumar <[email protected]>

BTW, Shouldn't you mark these for stable trees as well ? :)

Quoted 16 lines · Viresh Kumar

Quoted 14 lines · Mans Rullgard

Cyclic transfer callbacks rely on block completion interrupts which were disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts"). This re-enables block interrupts so the cyclic callbacks can work. Other transfer types are not affected as they set the INT_EN bit only on the last block.

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts")

Signed-off-by Mans Rullgard <[email protected]>

---
Changes:
- new patch

---
 drivers/dma/dw/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Quoted 3 lines

Reviewed-by Viresh Kumar <[email protected]>

BTW, Shouldn't you mark these for stable trees as well ? :)

Probably. Some maintainers prefer to do that themselves, but apparently you're not one of them.

Quoted 18 lines · Måns Rullgård

Quoted 16 lines · Viresh Kumar

Quoted 14 lines · Mans Rullgard

Cyclic transfer callbacks rely on block completion interrupts which were disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts"). This re-enables block interrupts so the cyclic callbacks can work. Other transfer types are not affected as they set the INT_EN bit only on the last block.

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block interrupts")

Signed-off-by Mans Rullgard <[email protected]>

---
Changes:
- new patch

---
 drivers/dma/dw/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Quoted 6 lines

Quoted 3 lines

Reviewed-by Viresh Kumar <[email protected]>

BTW, Shouldn't you mark these for stable trees as well ? :)

Probably. Some maintainers prefer to do that themselves, but apparently you're not one of them.

Vinod is the maintainer who is going to apply the patch :)

Cyclic transfer callbacks rely on block completion interrupts which were
disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block
interrupts").  This re-enables block interrupts so the cyclic callbacks
can work.  Other transfer types are not affected as they set the INT_EN
bit only on the last block.

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block

interrupts")

Signed-off-by Mans Rullgard <[email protected]>

From my understanding the custom stuff that does cyclic interrupts

---
Changes:
- new patch
---
 drivers/dma/dw/core.c | 21 +++++++++++++++------

 1 file changed, 15 insertions(+), 6 deletions(-)

drivers/dma/dw/core.c +0 -0

index af2b92f8501e..b92662722404 100644
@@ -156,6 +156,7 @@ static void dwc_initialize(struct dw_dma_chan
 *dwc)

  /* Enable interrupts */ channel_set_bit(dw, MASK.XFER, dwc->mask);

drivers/dma/dw/core.c +1 -0

+	channel_set_bit(dw, MASK.BLOCK, dwc->mask);

  channel_set_bit(dw, MASK.ERROR, dwc->mask);
  dwc->initialized = true;

drivers/dma/dw/core.c +0 -0

@@ -536,16 +537,17 @@ EXPORT_SYMBOL(dw_dma_get_dst_addr);

 /* Called with dwc->lock held and all DMAC interrupts disabled */ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,

drivers/dma/dw/core.c +1 -1

-		u32 status_err, u32 status_xfer)
+		u32 status_block, u32 status_err, u32 status_xfer)

 {
  unsigned long flags;

drivers/dma/dw/core.c +1 -1

-	if (dwc->mask) {
+	if (status_block & dwc->mask) {

  void (*callback)(void *param);
  void *callback_param;

  dev_vdbg(chan2dev(&dwc->chan), "new cyclic period llp 0x%08x\n",
  channel_readl(dwc, LLP));

drivers/dma/dw/core.c +1 -0

+		dma_writel(dw, CLEAR.BLOCK, dwc->mask);

  callback = dwc->cdesc->period_callback; callback_param = dwc->cdesc->period_callback_param;

drivers/dma/dw/core.c +0 -0

@@ -577,6 +579,7 @@ static void dwc_handle_cyclic(struct dw_dma *dw,
 struct dw_dma_chan *dwc,
  		channel_writel(dwc, CTL_LO, 0);
  		channel_writel(dwc, CTL_HI, 0);

drivers/dma/dw/core.c +1 -0

+		dma_writel(dw, CLEAR.BLOCK, dwc->mask);

  dma_writel(dw, CLEAR.ERROR, dwc->mask); dma_writel(dw, CLEAR.XFER, dwc->mask);

drivers/dma/dw/core.c +1 -0

@@ -593,10 +596,12 @@ static void dw_dma_tasklet(unsigned long data)
  {
  	struct dw_dma *dw = (struct dw_dma *)data;
  	struct dw_dma_chan *dwc;
+	u32 status_block;
  	u32 status_xfer;
  	u32 status_err;
  	int i;

drivers/dma/dw/core.c +1 -0

+	status_block = dma_readl(dw, RAW.BLOCK);

  status_xfer = dma_readl(dw, RAW.XFER);
  status_err = dma_readl(dw, RAW.ERROR);

drivers/dma/dw/core.c +2 -1

@@ -605,7 +610,8 @@ static void dw_dma_tasklet(unsigned long data)
  	for (i = 0; i < dw->dma.chancnt; i++) {
  		dwc = &dw->chan[i];
  		if (test_bit(DW_DMA_IS_CYCLIC, &dwc->flags))
-			dwc_handle_cyclic(dw, dwc, status_err,
 status_xfer);
+			dwc_handle_cyclic(dw, dwc, status_block,
 status_err,
+					status_xfer);
  		else if (status_err & (1 << i))

  dwc_handle_error(dw, dwc); else if (status_xfer & (1 << i))

drivers/dma/dw/core.c +1 -0

@@ -616,6 +622,7 @@ static void dw_dma_tasklet(unsigned long data)
  	 * Re-enable interrupts.
  	 */
  	channel_set_bit(dw, MASK.XFER, dw->all_chan_mask);
+	channel_set_bit(dw, MASK.BLOCK, dw->all_chan_mask);
  	channel_set_bit(dw, MASK.ERROR, dw->all_chan_mask);
  }

drivers/dma/dw/core.c +1 -0

@@ -635,6 +642,7 @@ static irqreturn_t dw_dma_interrupt(int irq, void
 *dev_id)
  	 * softirq handler.
  	 */
  	channel_clear_bit(dw, MASK.XFER, dw->all_chan_mask);
+	channel_clear_bit(dw, MASK.BLOCK, dw->all_chan_mask);
  	channel_clear_bit(dw, MASK.ERROR, dw->all_chan_mask);

  status = dma_readl(dw, STATUS_INT);

drivers/dma/dw/core.c +0 -0

@@ -645,6 +653,7 @@ static irqreturn_t dw_dma_interrupt(int irq, void
 *dev_id)

  /* Try to recover */ channel_clear_bit(dw, MASK.XFER, (1 << 8) - 1);

drivers/dma/dw/core.c +1 -0

+		channel_clear_bit(dw, MASK.BLOCK, (1 << 8) - 1);

  channel_clear_bit(dw, MASK.SRC_TRAN, (1 << 8) - 1); channel_clear_bit(dw, MASK.DST_TRAN, (1 << 8) - 1); channel_clear_bit(dw, MASK.ERROR, (1 << 8) - 1);

drivers/dma/dw/core.c +0 -0

@@ -1111,6 +1120,7 @@ static void dw_dma_off(struct dw_dma *dw)
  	dma_writel(dw, CFG, 0);

  channel_clear_bit(dw, MASK.XFER, dw->all_chan_mask);

drivers/dma/dw/core.c +1 -0

+	channel_clear_bit(dw, MASK.BLOCK, dw->all_chan_mask);

  channel_clear_bit(dw, MASK.SRC_TRAN, dw->all_chan_mask); channel_clear_bit(dw, MASK.DST_TRAN, dw->all_chan_mask); channel_clear_bit(dw, MASK.ERROR, dw->all_chan_mask);

drivers/dma/dw/core.c +0 -0

@@ -1216,6 +1226,7 @@ static void dwc_free_chan_resources(struct
 dma_chan *chan)

  /* Disable interrupts */ channel_clear_bit(dw, MASK.XFER, dwc->mask);

drivers/dma/dw/core.c +1 -0

+	channel_clear_bit(dw, MASK.BLOCK, dwc->mask);

  channel_clear_bit(dw, MASK.ERROR, dwc->mask);
  spin_unlock_irqrestore(&dwc->lock, flags);

drivers/dma/dw/core.c +0 -0

@@ -1458,6 +1469,7 @@ void dw_dma_cyclic_free(struct dma_chan *chan)

  dwc_chan_disable(dw, dwc);

drivers/dma/dw/core.c +1 -0

+	dma_writel(dw, CLEAR.BLOCK, dwc->mask);

  dma_writel(dw, CLEAR.ERROR, dwc->mask);
  dma_writel(dw, CLEAR.XFER, dwc->mask);

drivers/dma/dw/core.c +0 -0

@@ -1546,9 +1558,6 @@ int dw_dma_probe(struct dw_dma_chip *chip,
 struct dw_dma_platform_data *pdata)
  	/* Force dma off, just in case */
  	dw_dma_off(dw);

drivers/dma/dw/core.c +0 -3

-	/* Disable BLOCK interrupts as well */
-	channel_clear_bit(dw, MASK.BLOCK, dw->all_chan_mask);
-

  /* Create a pool of consistent memory blocks for hardware descriptors */
  dw->desc_pool = dmam_pool_create("dw_dmac_desc_pool", chip-

Quoted 1 line

dev,

   sizeof(struct dw_desc), 4, 0);

Quoted 10 lines · Mans Rullgard

Cyclic transfer callbacks rely on block completion interrupts which were
disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block
interrupts").  This re-enables block interrupts so the cyclic callbacks
can work.  Other transfer types are not affected as they set the INT_EN
bit only on the last block.

Quoted 3 lines

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block

interrupts")

Signed-off-by Mans Rullgard <[email protected]>

How did you test that?

From my understanding the custom stuff that does cyclic interrupts prepares a set of descriptors per period, which at the end of transfer will generate XFER interrupt. Next period will go in the same way.

Maybe I missed something.

Quoted 11 lines · Andy Shevchenko

Quoted 10 lines · Mans Rullgard

Cyclic transfer callbacks rely on block completion interrupts which were
disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block
interrupts"). This re-enables block interrupts so the cyclic callbacks
can work. Other transfer types are not affected as they set the INT_EN
bit only on the last block.

Quoted 5 lines

Quoted 3 lines

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block

interrupts")

Signed-off-by Mans Rullgard <[email protected]>

How did you test that?

With the ABDAC sound driver on the AVR32. It fails rather miserably without these patches.

Quoted 4 lines

From my understanding the custom stuff that does cyclic interrupts prepares a set of descriptors per period, which at the end of transfer will generate XFER interrupt. Next period will go in the same way.

Quoted 1 line

Maybe I missed something.

The cyclic DMA is done by setting up a set of descriptors, one per period, with the last linked back to the first. The chain never ends, so there is never an XFER interrupt.

Quoted 28 lines · Måns Rullgård

Quoted 11 lines · Andy Shevchenko

Quoted 10 lines · Mans Rullgard

Cyclic transfer callbacks rely on block completion interrupts which were
disabled in commit ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block
interrupts"). This re-enables block interrupts so the cyclic callbacks
can work. Other transfer types are not affected as they set the INT_EN
bit only on the last block.

Quoted 5 lines

Quoted 3 lines

Fixes ff7b05f29fd4 ("dmaengine/dw_dmac: Don't handle block

interrupts")

Signed-off-by Mans Rullgard <[email protected]>

How did you test that?

With the ABDAC sound driver on the AVR32. It fails rather miserably without these patches.

Quoted 5 lines

From my understanding the custom stuff that does cyclic interrupts prepares a set of descriptors per period, which at the end of transfer will generate XFER interrupt. Next period will go in the same way.

Maybe I missed something.

Quoted 3 lines

The cyclic DMA is done by setting up a set of descriptors, one per period, with the last linked back to the first. The chain never ends, so there is never an XFER interrupt.

Thanks for clarification.

Nevertheless, my plan is to get rid of custom implementation of the cyclic API in this driver,

thus, I would suggest to have this patch only for stable and as interim for mainline.

Quoted 2 lines · Andy Shevchenko

thus, I would suggest to have this patch only for stable and as interim for mainline.

Better to start from a working state.

These patches fix a couple of old regressions breaking cyclic transfers.