Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751856AbbHGJoa (ORCPT ); Fri, 7 Aug 2015 05:44:30 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:56452 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751640AbbHGJo1 (ORCPT ); Fri, 7 Aug 2015 05:44:27 -0400 Subject: Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers To: Sebastian Andrzej Siewior , Vinod Koul References: <1438936917-7254-1-git-send-email-bigeasy@linutronix.de> CC: Dan Williams , , , , , , , Russell King From: Peter Ujfalusi Message-ID: <55C47DE1.9020902@ti.com> Date: Fri, 7 Aug 2015 12:44:01 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1438936917-7254-1-git-send-email-bigeasy@linutronix.de> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5237 Lines: 174 On 08/07/2015 11:41 AM, Sebastian Andrzej Siewior wrote: > 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. with a short testing audio did not broke (the only user of pause/resume) Some comments embedded. > Cc: Why stable? This is not fixing any bugs since the PAUSE was not allowed for non cyclic transfers. > Signed-off-by: Sebastian Andrzej Siewior > --- > drivers/dma/omap-dma.c | 54 ++++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 41 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c > index 249445c8a4c6..6b8497203caf 100644 > --- a/drivers/dma/omap-dma.c > +++ b/drivers/dma/omap-dma.c > @@ -299,7 +299,7 @@ static void omap_dma_start(struct omap_chan *c, struct omap_desc *d) > omap_dma_chan_write(c, CCR, d->ccr | CCR_ENABLE); > } > > -static void omap_dma_stop(struct omap_chan *c) > +static int omap_dma_stop(struct omap_chan *c) > { > struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device); > uint32_t val; > @@ -342,8 +342,26 @@ static void omap_dma_stop(struct omap_chan *c) > > omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig); > } else { > + int i = 0; > + > + if (!(val & CCR_ENABLE)) > + return -EINVAL; > + > val &= ~CCR_ENABLE; > omap_dma_chan_write(c, CCR, val); > + do { > + val = omap_dma_chan_read(c, CCR); > + if (!(val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))) > + break; > + if (i > 100) if (++i > 100) break; to avoid infinite loop? > + break; > + udelay(5); > + } while (1); > + > + if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE)) if (i > 100) ? > + dev_err(c->vc.chan.device->dev, > + "DMA drain did not complete on lch %d\n", > + c->dma_ch); > } > > mb(); > @@ -358,6 +376,7 @@ static void omap_dma_stop(struct omap_chan *c) > > omap_dma_chan_write(c, CLNK_CTRL, val); > } > + return 0; > } > > static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d, > @@ -728,6 +747,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan *chan, > } else { > txstate->residue = 0; > } > + if (ret == DMA_IN_PROGRESS && c->paused) > + ret = DMA_PAUSED; > spin_unlock_irqrestore(&c->vc.lock, flags); > > return ret; > @@ -1053,28 +1074,33 @@ static int omap_dma_terminate_all(struct dma_chan *chan) > static int omap_dma_pause(struct dma_chan *chan) > { > struct omap_chan *c = to_omap_dma_chan(chan); > + struct omap_dmadev *od = to_omap_dma_dev(chan->device); > + unsigned long flags; > + int ret = -EINVAL; > > - /* Pause/Resume only allowed with cyclic mode */ > - if (!c->cyclic) > - return -EINVAL; > + spin_lock_irqsave(&od->irq_lock, flags); > > - if (!c->paused) { > - omap_dma_stop(c); > - c->paused = true; > + if (!c->paused && c->desc) { > + ret = omap_dma_stop(c); > + if (!ret) > + c->paused = true; > } > > - return 0; > + spin_unlock_irqrestore(&od->irq_lock, flags); > + > + return ret; > } > > static int omap_dma_resume(struct dma_chan *chan) > { > struct omap_chan *c = to_omap_dma_chan(chan); > + struct omap_dmadev *od = to_omap_dma_dev(chan->device); > + unsigned long flags; > + int ret = -EINVAL; > > - /* Pause/Resume only allowed with cyclic mode */ > - if (!c->cyclic) > - return -EINVAL; > + spin_lock_irqsave(&od->irq_lock, flags); > > - if (c->paused) { > + if (c->paused && c->desc) { > mb(); > > /* Restore channel link register */ > @@ -1082,9 +1108,11 @@ static int omap_dma_resume(struct dma_chan *chan) > > omap_dma_start(c, c->desc); > c->paused = false; > + ret = 0; > } > + spin_unlock_irqrestore(&od->irq_lock, flags); > > - return 0; > + return ret; > } > > static int omap_dma_chan_init(struct omap_dmadev *od) > -- P?ter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/