[PATCH v2 0/2] dmaengine: dw: fix cyclic transfers

[PATCH v2 0/2] dmaengine: dw: fix cyclic transfers

[Mans Rullgard]

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

Mans Rullgard (2):
  dmaengine: dw: fix cyclic transfer setup
  dmaengine: dw: fix cyclic transfer callbacks

 drivers/dma/dw/core.c | 44 ++++++++++++++++----------------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

-- 
2.7.0

[PATCH v2 1/2] dmaengine: dw: fix cyclic transfer setup

[Mans Rullgard]

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 <mans@mansr.com>
---
Changes:
- replace duplicated code by dwc_dostart() call
---
 drivers/dma/dw/core.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 7067b6ddc1db..af2b92f8501e 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1245,7 +1245,6 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 int dw_dma_cyclic_start(struct dma_chan *chan)
 {
 	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
-	struct dw_dma		*dw = to_dw_dma(dwc->chan.device);
 	unsigned long		flags;
 
 	if (!test_bit(DW_DMA_IS_CYCLIC, &dwc->flags)) {
@@ -1254,27 +1253,7 @@ int dw_dma_cyclic_start(struct dma_chan *chan)
 	}
 
 	spin_lock_irqsave(&dwc->lock, flags);
-
-	/* Assert channel is idle */
-	if (dma_readl(dw, CH_EN) & dwc->mask) {
-		dev_err(chan2dev(&dwc->chan),
-			"%s: BUG: Attempted to start non-idle channel\n",
-			__func__);
-		dwc_dump_chan_regs(dwc);
-		spin_unlock_irqrestore(&dwc->lock, flags);
-		return -EBUSY;
-	}
-
-	dma_writel(dw, CLEAR.ERROR, dwc->mask);
-	dma_writel(dw, CLEAR.XFER, dwc->mask);
-
-	/* Setup DMAC channel registers */
-	channel_writel(dwc, LLP, dwc->cdesc->desc[0]->txd.phys);
-	channel_writel(dwc, CTL_LO, DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN);
-	channel_writel(dwc, CTL_HI, 0);
-
-	channel_set_bit(dw, CH_EN, dwc->mask);
-
+	dwc_dostart(dwc, dwc->cdesc->desc[0]);
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	return 0;
-- 
2.7.0

Re: [PATCH v2 1/2] dmaengine: dw: fix cyclic transfer setup

[Viresh Kumar]

On 11-01-16, 13:04, Mans Rullgard wrote:
> 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 <mans@mansr.com>
> ---
> Changes:
> - replace duplicated code by dwc_dostart() call
> ---
>  drivers/dma/dw/core.c | 23 +----------------------
>  1 file changed, 1 insertion(+), 22 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[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 <mans@mansr.com>
---
Changes:
- new patch
---
 drivers/dma/dw/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index af2b92f8501e..b92662722404 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -156,6 +156,7 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
 
 	/* Enable interrupts */
 	channel_set_bit(dw, MASK.XFER, dwc->mask);
+	channel_set_bit(dw, MASK.BLOCK, dwc->mask);
 	channel_set_bit(dw, MASK.ERROR, dwc->mask);
 
 	dwc->initialized = true;
@@ -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,
-		u32 status_err, u32 status_xfer)
+		u32 status_block, u32 status_err, u32 status_xfer)
 {
 	unsigned long flags;
 
-	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));
+		dma_writel(dw, CLEAR.BLOCK, dwc->mask);
 
 		callback = dwc->cdesc->period_callback;
 		callback_param = dwc->cdesc->period_callback_param;
@@ -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);
 
+		dma_writel(dw, CLEAR.BLOCK, dwc->mask);
 		dma_writel(dw, CLEAR.ERROR, dwc->mask);
 		dma_writel(dw, CLEAR.XFER, dwc->mask);
 
@@ -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;
 
+	status_block = dma_readl(dw, RAW.BLOCK);
 	status_xfer = dma_readl(dw, RAW.XFER);
 	status_err = dma_readl(dw, RAW.ERROR);
 
@@ -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))
@@ -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);
 }
 
@@ -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);
@@ -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);
+		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);
@@ -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);
+	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);
@@ -1216,6 +1226,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 
 	/* Disable interrupts */
 	channel_clear_bit(dw, MASK.XFER, dwc->mask);
+	channel_clear_bit(dw, MASK.BLOCK, dwc->mask);
 	channel_clear_bit(dw, MASK.ERROR, dwc->mask);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
@@ -1458,6 +1469,7 @@ void dw_dma_cyclic_free(struct dma_chan *chan)
 
 	dwc_chan_disable(dw, dwc);
 
+	dma_writel(dw, CLEAR.BLOCK, dwc->mask);
 	dma_writel(dw, CLEAR.ERROR, dwc->mask);
 	dma_writel(dw, CLEAR.XFER, dwc->mask);
 
@@ -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);
 
-	/* 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->dev,
 					 sizeof(struct dw_desc), 4, 0);
-- 
2.7.0

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Viresh Kumar]

On 11-01-16, 13:04, Mans Rullgard wrote:
> 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 <mans@mansr.com>
> ---
> Changes:
> - new patch
> ---
>  drivers/dma/dw/core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

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

-- 
viresh

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Måns Rullgård]

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 11-01-16, 13:04, Mans Rullgard wrote:
>> 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 <mans@mansr.com>
>> ---
>> Changes:
>> - new patch
>> ---
>>  drivers/dma/dw/core.c | 21 +++++++++++++++------
>>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> 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.

-- 
Måns Rullgård

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Viresh Kumar]

On 11-01-16, 14:14, Måns Rullgård wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> 
> > On 11-01-16, 13:04, Mans Rullgard wrote:
> >> 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 <mans@mansr.com>
> >> ---
> >> Changes:
> >> - new patch
> >> ---
> >>  drivers/dma/dw/core.c | 21 +++++++++++++++------
> >>  1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > 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 :)

-- 
viresh

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Vinod Koul]

On Mon, Jan 11, 2016 at 09:38:27PM +0530, Viresh Kumar wrote:
> On 11-01-16, 14:14, Måns Rullgård wrote:
> > Viresh Kumar <viresh.kumar@linaro.org> writes:
> > 
> > > On 11-01-16, 13:04, Mans Rullgard wrote:
> > >> 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 <mans@mansr.com>
> > >> ---
> > >> Changes:
> > >> - new patch
> > >> ---
> > >>  drivers/dma/dw/core.c | 21 +++++++++++++++------
> > >>  1 file changed, 15 insertions(+), 6 deletions(-)
> > >
> > > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> > >
> > > 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 :)

Letting know is fine too, either commenting after the SOB line or adding CC
line.. Maintainers can add/remove :)

-- 
~Vinod

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Andy Shevchenko]

On Mon, 2016-01-11 at 13:04 +0000, Mans Rullgard wrote:
> 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 <mans@mansr.com>

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.

> ---
> Changes:
> - new patch
> ---
>  drivers/dma/dw/core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index af2b92f8501e..b92662722404 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -156,6 +156,7 @@ static void dwc_initialize(struct dw_dma_chan
> *dwc)
>  
>  	/* Enable interrupts */
>  	channel_set_bit(dw, MASK.XFER, dwc->mask);
> +	channel_set_bit(dw, MASK.BLOCK, dwc->mask);
>  	channel_set_bit(dw, MASK.ERROR, dwc->mask);
>  
>  	dwc->initialized = true;
> @@ -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,
> -		u32 status_err, u32 status_xfer)
> +		u32 status_block, u32 status_err, u32 status_xfer)
>  {
>  	unsigned long flags;
>  
> -	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));
> +		dma_writel(dw, CLEAR.BLOCK, dwc->mask);
>  
>  		callback = dwc->cdesc->period_callback;
>  		callback_param = dwc->cdesc->period_callback_param;
> @@ -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);
>  
> +		dma_writel(dw, CLEAR.BLOCK, dwc->mask);
>  		dma_writel(dw, CLEAR.ERROR, dwc->mask);
>  		dma_writel(dw, CLEAR.XFER, dwc->mask);
>  
> @@ -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;
>  
> +	status_block = dma_readl(dw, RAW.BLOCK);
>  	status_xfer = dma_readl(dw, RAW.XFER);
>  	status_err = dma_readl(dw, RAW.ERROR);
>  
> @@ -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))
> @@ -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);
>  }
>  
> @@ -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);
> @@ -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);
> +		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);
> @@ -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);
> +	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);
> @@ -1216,6 +1226,7 @@ static void dwc_free_chan_resources(struct
> dma_chan *chan)
>  
>  	/* Disable interrupts */
>  	channel_clear_bit(dw, MASK.XFER, dwc->mask);
> +	channel_clear_bit(dw, MASK.BLOCK, dwc->mask);
>  	channel_clear_bit(dw, MASK.ERROR, dwc->mask);
>  
>  	spin_unlock_irqrestore(&dwc->lock, flags);
> @@ -1458,6 +1469,7 @@ void dw_dma_cyclic_free(struct dma_chan *chan)
>  
>  	dwc_chan_disable(dw, dwc);
>  
> +	dma_writel(dw, CLEAR.BLOCK, dwc->mask);
>  	dma_writel(dw, CLEAR.ERROR, dwc->mask);
>  	dma_writel(dw, CLEAR.XFER, dwc->mask);
>  
> @@ -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);
>  
> -	/* 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-
> >dev,
>  					 sizeof(struct dw_desc), 4,
> 0);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Måns Rullgård]

Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> On Mon, 2016-01-11 at 13:04 +0000, Mans Rullgard wrote:
>> 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 <mans@mansr.com>
>
> How did you test that?

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

> 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.

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.

-- 
Måns Rullgård

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Andy Shevchenko]

On Mon, Jan 11, 2016 at 5:09 PM, Måns Rullgård <mans@mansr.com> wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
>
>> On Mon, 2016-01-11 at 13:04 +0000, Mans Rullgard wrote:
>>> 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 <mans@mansr.com>
>>
>> How did you test that?
>
> With the ABDAC sound driver on the AVR32.  It fails rather miserably
> without these patches.
>
>> 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.
>
> 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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Måns Rullgård]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

> On Mon, Jan 11, 2016 at 5:09 PM, Måns Rullgård <mans@mansr.com> wrote:
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
>>
>>> On Mon, 2016-01-11 at 13:04 +0000, Mans Rullgard wrote:
>>>> 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 <mans@mansr.com>
>>>
>>> How did you test that?
>>
>> With the ABDAC sound driver on the AVR32.  It fails rather miserably
>> without these patches.
>>
>>> 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.
>>
>> 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,

Good idea.

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

Better to start from a working state.

-- 
Måns Rullgård

Re: [PATCH v2 2/2] dmaengine: dw: fix cyclic transfer callbacks

[Andy Shevchenko]

On Mon, Jan 11, 2016 at 5:22 PM, Måns Rullgård <mans@mansr.com> wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> writes:

>> thus, I would suggest to have this patch only for stable and as
>> interim for mainline.
>
> Better to start from a working state.

Agreed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] dmaengine: dw: fix cyclic transfers

[Vinod Koul]

On Mon, Jan 11, 2016 at 01:04:27PM +0000, Mans Rullgard wrote:
> These patches fix a couple of old regressions breaking cyclic transfers.

Applied both and marked stable

Thanks
-- 
~Vinod