On Wed, Feb 17, 2016 at 02:55:43PM +0100, Ludovic Desroches wrote:
> Hi David,
>
> On Wed, Feb 17, 2016 at 02:12:19PM +0100, David Engraf wrote:
> > Hi,
> >
> > I encountered an issue when using atmel_serial.c driver with at_xdmac.c on a
> > SAMA5D4 board. The serial driver uses a cyclic DMA receive buffer. While
> > doing tests with a high baudrate and high data load, at_xdmac_tx_status()
> > sometimes returns an invalid value for residue. This happens when the serial
> > driver calls dmaengine_tx_status() while the DMA is still in progress
> > (DMA_IN_PROGRESS). In this case, going through the microblock list is not
> > save because the content might be modified. This may happen when
> > AT_XDMAC_CNDA has been changed while going through the list.
> >
> > The only reliable solution working in my test scenario was to pause and
> > resume the DMA channel when it is in progress.
> >
>
> Thanks for your patch. You're right, it sounds safer to pause dma to have an
> accurate residue.
>
> >
> > Signed-off-by: David Engraf <david.engraf@xxxxxxxxx>
> >
> Minor comment otherwise
> Acked-by: Ludovic Desroches <ludovic.desroches@xxxxxxxxx>
Well maybe I have answered too quickly. After discussing with Cyrille, it
reminds me that serial devices on sama5d4 have no fifo. If you pause the
channel you may lost data. I am surprised you don't notice it if you
have a high baudrate.
Did you try to increase the size of the dma buffer? If your high
baudrate is the root cause of your issue, we may fill the two parts of
the cyclic buffer too fast.
Regards
Ludovic
>
> > diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> > index 64f5d1b..295a06a 100644
> > --- a/drivers/dma/at_xdmac.c
> > +++ b/drivers/dma/at_xdmac.c
> > @@ -254,6 +254,9 @@ struct at_xdmac_desc {
> > struct list_head xfer_node;
> > };
> >
> > +static int at_xdmac_device_pause(struct dma_chan *chan);
> > +static int at_xdmac_device_resume(struct dma_chan *chan);
> > +
> > static inline void __iomem *at_xdmac_chan_reg_base(struct at_xdmac *atxdmac, unsigned int chan_nb)
> > {
> > return atxdmac->regs + (AT_XDMAC_CHAN_REG_BASE + chan_nb * 0x40);
> > @@ -1407,6 +1410,11 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> > if (!txstate)
> > return ret;
> >
> > + if (ret == DMA_IN_PROGRESS) {
> > + /* pause DMA while calculating residue */
> > + at_xdmac_device_pause(chan);
> > + }
>
> /* Pause DMA while calculating residue. */
> if (ret == DMA_IN_PROGRESS)
> at_xdmac_device_pause(chan);
>
> > +
> > spin_lock_irqsave(&atchan->lock, flags);
> >
> > desc = list_first_entry(&atchan->xfers_list, struct at_xdmac_desc, xfer_node);
> > @@ -1456,6 +1464,10 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> >
> > spin_unlock:
> > spin_unlock_irqrestore(&atchan->lock, flags);
> > +
> > + if (ret == DMA_IN_PROGRESS)
> > + at_xdmac_device_resume(chan);
> > +
> > return ret;
> > }
> >
>
> Regards
>
> Ludovic
>
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
