Re: [PATCH] at_xdmac fix status read in cyclic mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ludovic,

Am 17.02.2016 um 16:31 schrieb Ludovic Desroches:
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.

No I don't see any lost data. But the pause/resume is only called for a very short time while doing the calculation and the interrupts are disabled right after calling pause. Nevertheless it is theoretical possible that we'll losing data but I didn't found a better solution.

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.

I tried that as well and the problem was not triggered that much but it was still there. This is because it may always happen when we're near the border between the two buffers.

Best regards
- David


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


--
Mit freundlichen Grüßen/Best regards,

David Engraf
Product Engineer

SYSGO AG
Office Mainz
Am Pfaffenstein 14 / D-55270 Klein-Winternheim / Germany
Phone: +49-6136-9948-0 / Fax: +49-6136-9948-10
VoIP: SIP:den@xxxxxxxxx
E-mail: david.engraf@xxxxxxxxx / Web: http://www.sysgo.com

Handelsregister/Commercial Registry: HRB Mainz 90 HRB 8066
Vorstand/Executive Board: Knut Degen (CEO), Kai Sablotny (COO)
Aufsichtsratsvorsitzender/Supervisory Board Chairman: Pascal Bouchiat
USt-Id-Nr./VAT-Id-No.: DE 149062328
--
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



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux