fixes and update for at_xdmac

Hi,

This set of patches is mainly about fixes and an update to remove the chancnt affection since xdmac was not in the tree when Maxime managed that.

Cyrille Pitchen (2):
dmaengine: at_xdmac: fix software lockup at_xdmac_tx_status()
dmaengine: at_xdmac: fix a bug in transfer residue computation

Ludovic Desroches (5):
dmaengine: at_xdmac: prefer usage of readl/writel_relaxed
dmaengine: at_xdmac: remove chancnt affectation
dmaengine: at_xdmac: fix missing spin_unlock
ARM: dts: at_xdmac: fix bad value of dma-cells in documentation
dmaengine: at_xdmac: Add DMA_PRIVATE

 .../devicetree/bindings/dma/atmel-xdma.txt         |  2 +-
 drivers/dma/Kconfig                                |  2 +-
 drivers/dma/at_xdmac.c                             | 36 +++++++++++++++-------
 3 files changed, 27 insertions(+), 13 deletions(-)

2.0.3

_relaxed version of readl and writel are not implemented on all architecture so COMPILE_TEST has to be removed in order to not cause some build failures.

---
 drivers/dma/Kconfig    | 2 +-
 drivers/dma/at_xdmac.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

index aef8b9d..f2b2c4e 100644
@@ -109,7 +109,7 @@ config AT_HDMAC
 
 config AT_XDMAC
 	tristate "Atmel XDMA support"
-	depends on (ARCH_AT91 || COMPILE_TEST)
+	depends on ARCH_AT91
 	select DMA_ENGINE
 	help
 	  Support the Atmel XDMA controller.

index 89c43be..1f53d92 100644
@@ -248,12 +248,12 @@ static inline void __iomem *at_xdmac_chan_reg_base(struct at_xdmac *atxdmac, uns
 	return atxdmac->regs + (AT_XDMAC_CHAN_REG_BASE + chan_nb * 0x40);
 }
 
-#define at_xdmac_read(atxdmac, reg) readl((atxdmac)->regs + (reg))
+#define at_xdmac_read(atxdmac, reg) readl_relaxed((atxdmac)->regs + (reg))
 #define at_xdmac_write(atxdmac, reg, value) \
-	writel((value), (atxdmac)->regs + (reg))
+	writel_relaxed((value), (atxdmac)->regs + (reg))
 
-#define at_xdmac_chan_read(atchan, reg) readl((atchan)->ch_regs + (reg))
-#define at_xdmac_chan_write(atchan, reg, value) writel((value), (atchan)->ch_regs + (reg))
+#define at_xdmac_chan_read(atchan, reg) readl_relaxed((atchan)->ch_regs + (reg))
+#define at_xdmac_chan_write(atchan, reg, value) writel_relaxed((value), (atchan)->ch_regs + (reg))
 
 static inline struct at_xdmac_chan *to_at_xdmac_chan(struct dma_chan *dchan)
 {

2.0.3

I've queued up Will Deacon's series to enable the relaxed accessors on all architectures, so the Kconfig change should no longer be required in 3.19.

Arnd

Good news.

Vinod, I can send a v2 if you want.

Ludovic

And why is _relaxed() version required for thsi driver. Why cant readl(), writel() with barriers do?

~Vinod

Required not but preferred to my mind. I don't need a barrier for all read and write I am performing. Barriers have been added when needed.

When you have fixed my code because of the compilation breakage on other architecture, you only change the read and write macros. Currently, there are some redundancies because of the barriers added in the code.

To be honest, I have no idea about the performance impact between the use of read/write and read/write_relaxed. Then two choices:
- go back to relaxed version, good new can keep COMPILE_TEST thanks to Will Deacon
- remove barrier since they are no more needed if using readl/writel

Ludovic

But that would merged after merge window. This was also reported on next. So lets keep this patch for now and you can enabled COMPILE_TEST after next merge window.

I think that should be right fix along with checking performance of driver and optimize/stress it

So for now, I am applying this whole series.

~Vinod

Remove chancnt affectation since it is done in dma_async_device_regiser.

---
 drivers/dma/at_xdmac.c | 1 -
 1 file changed, 1 deletion(-)

index 1f53d92..fcecbad 100644
@@ -1393,7 +1393,6 @@ static int at_xdmac_probe(struct platform_device *pdev)
 	atxdmac->dma.device_prep_dma_memcpy		= at_xdmac_prep_dma_memcpy;
 	atxdmac->dma.device_prep_slave_sg		= at_xdmac_prep_slave_sg;
 	atxdmac->dma.device_control			= at_xdmac_control;
-	atxdmac->dma.chancnt				= nr_channels;
 	atxdmac->dma.device_slave_caps			= at_xdmac_device_slave_caps;
 
 	/* Disable all chans and interrupts. */

2.0.3

From: Cyrille Pitchen <[email protected]>

According to the Atmel eXtended DMA controller datasheet, requesting a DMA transfer flush for a channel is only revelant when this transfer is source peripheral synchronized.

So we have to check this condition before requesting a channel flush by writing the channel bit into the Global channel SoftWare Flush (GSWF) register then waiting for flush to complete by monitoring the end of Flush Interrupt Status (FIS) bit in the Channel Interrupt Status (CIS) register.

Indeed, for non source peripheral synchronized transfer, writing the channel bit into the GSWF register does nothing. Especially, the FIS bit is never set into the CIS register. The former code looped forever waiting for this bit to be set.

---
 drivers/dma/at_xdmac.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

index fcecbad..fa9d75a 100644
@@ -879,7 +879,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 	struct list_head	*descs_list;
 	enum dma_status		ret;
 	int			residue;
-	u32			cur_nda;
+	u32			cur_nda, mask, value;
 	u8			dwidth = at_xdmac_get_dwidth(atchan->cfg[AT_XDMAC_CUR_CFG]);
 
 	ret = dma_cookie_status(chan, cookie, txstate);

@@ -903,10 +903,17 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 	}
 
 	residue = desc->xfer_size;
-	/* Flush FIFO. */
-	at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
-	while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
-		cpu_relax();
+	/*
+	 * Flush FIFO: only relevant when the transfer is source peripheral
+	 * synchronized.
+	 */
+	mask = AT_XDMAC_CC_TYPE | AT_XDMAC_CC_DSYNC;
+	value = AT_XDMAC_CC_TYPE_PER_TRAN | AT_XDMAC_CC_DSYNC_PER2MEM;
+	if ((atchan->cfg[AT_XDMAC_CUR_CFG] & mask) == value) {
+		at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
+		while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
+			cpu_relax();
+	}
 
 	cur_nda = at_xdmac_chan_read(atchan, AT_XDMAC_CNDA) & 0xfffffffc;
 	/*

2.0.3

From: Cyrille Pitchen <[email protected]>

The total size of the transfer was wrong in at_xdmac_prep_slave_sg() resulting in bad computation of the transfer residue by at_xdmac_tx_status().

---
 drivers/dma/at_xdmac.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index fa9d75a..bf2aacb 100644
@@ -562,6 +562,7 @@ at_xdmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	struct scatterlist	*sg;
 	int			i;
 	u32			cfg;
+	unsigned int		xfer_size = 0;
 
 	if (!sgl)
 		return NULL;

@@ -637,12 +638,13 @@ at_xdmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		dev_dbg(chan2dev(chan), "%s: add desc 0x%p to descs_list 0x%p\n",
 			 __func__, desc, first);
 		list_add_tail(&desc->desc_node, &first->descs_list);
+		xfer_size += len;
 	}
 
 	spin_unlock_bh(&atchan->lock);
 
 	first->tx_dma_desc.flags = flags;
-	first->xfer_size = sg_len;
+	first->xfer_size = xfer_size;
 	first->direction = direction;
 
 	return &first->tx_dma_desc;

2.0.3

Lock taken when entering the function but unlock missing before it returns.

---
 drivers/dma/at_xdmac.c | 1 +
 1 file changed, 1 insertion(+)

index bf2aacb..586275f 100644
@@ -901,6 +901,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 	 */
 	if (!desc->active_xfer) {
 		dma_set_residue(txstate, desc->xfer_size);
+		spin_unlock_bh(&atchan->lock);
 		return ret;
 	}
 

2.0.3

I am adding reported to tag here for Dan's report

~Vinod

The dma-cells value in the example was 2 instead of 1.

---
 Documentation/devicetree/bindings/dma/atmel-xdma.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e75c128..0eb2b32 100644
@@ -22,7 +22,7 @@ dma1: dma-controller at f0004000 {
 	compatible = "atmel,sama5d4-dma";
 	reg = <0xf0004000 0x200>;
 	interrupts = <50 4 0>;
-	#dma-cells = <2>;
+	#dma-cells = <1>;
 };
 
 

2.0.3

same issue as commit 7f5ae3553685:
"Without DMA_PRIVATE the driver is not able to allocate more than one channel. Since it uses dma_get_any_slave_channel that calls private_candidate, the second allocation fails at
/* some channels are already publicly allocated */
"

---
 drivers/dma/at_xdmac.c | 5 +++++
 1 file changed, 5 insertions(+)

index 586275f..b60d77a 100644
@@ -1394,6 +1394,11 @@ static int at_xdmac_probe(struct platform_device *pdev)
 	dma_cap_set(DMA_CYCLIC, atxdmac->dma.cap_mask);
 	dma_cap_set(DMA_MEMCPY, atxdmac->dma.cap_mask);
 	dma_cap_set(DMA_SLAVE, atxdmac->dma.cap_mask);
+	/*
+	 * Without DMA_PRIVATE the driver is not able to allocate more than
+	 * one channel, second allocation fails in private_candidate.
+	 */
+	dma_cap_set(DMA_PRIVATE, atxdmac->dma.cap_mask);
 	atxdmac->dma.dev				= &pdev->dev;
 	atxdmac->dma.device_alloc_chan_resources	= at_xdmac_alloc_chan_resources;
 	atxdmac->dma.device_free_chan_resources		= at_xdmac_free_chan_resources;

2.0.3