[PATCH 0/7] fixes and update for at_xdmac

[PATCH 0/7] fixes and update for at_xdmac

[Ludovic Desroches]

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

[PATCH 1/7] dmaengine: at_xdmac: prefer usage of readl/writel_relaxed

[Ludovic Desroches]

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

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/dma/Kconfig    | 2 +-
 drivers/dma/at_xdmac.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index aef8b9d..f2b2c4e 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -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.
diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 89c43be..1f53d92 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -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

[PATCH 2/7] dmaengine: at_xdmac: remove chancnt affectation

[Ludovic Desroches]

Remove chancnt affectation since it is done in dma_async_device_regiser.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/dma/at_xdmac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 1f53d92..fcecbad 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -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

[PATCH 3/7] dmaengine: at_xdmac: fix software lockup at_xdmac_tx_status()

[Ludovic Desroches]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

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.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/dma/at_xdmac.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index fcecbad..fa9d75a 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -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

[PATCH 4/7] dmaengine: at_xdmac: fix a bug in transfer residue computation

[Ludovic Desroches]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

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

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/dma/at_xdmac.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index fa9d75a..bf2aacb 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -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

[PATCH 5/7] dmaengine: at_xdmac: fix missing spin_unlock

[Ludovic Desroches]

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

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/dma/at_xdmac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index bf2aacb..586275f 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -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

[PATCH 6/7] ARM: dts: at_xdmac: fix bad value of dma-cells in documentation

[Ludovic Desroches]

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

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 Documentation/devicetree/bindings/dma/atmel-xdma.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
index e75c128..0eb2b32 100644
--- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
+++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
@@ -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

[PATCH 7/7] dmaengine: at_xdmac: Add DMA_PRIVATE

[Ludovic Desroches]

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 */
"

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/dma/at_xdmac.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 586275f..b60d77a 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -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

[PATCH 1/7] dmaengine: at_xdmac: prefer usage of readl/writel_relaxed

[Arnd Bergmann]

On Thursday 13 November 2014 11:52:39 Ludovic Desroches wrote:
> _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.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
> 

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

[PATCH 1/7] dmaengine: at_xdmac: prefer usage of readl/writel_relaxed

[Ludovic Desroches]

On Thu, Nov 13, 2014 at 12:01:49PM +0100, Arnd Bergmann wrote:
> On Thursday 13 November 2014 11:52:39 Ludovic Desroches wrote:
> > _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.
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > ---
> > 
> 
> 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.

Good news.

Vinod, I can send a v2 if you want.


Ludovic

[PATCH 1/7] dmaengine: at_xdmac: prefer usage of readl/writel_relaxed

[Vinod Koul]

On Thu, Nov 13, 2014 at 12:01:49PM +0100, Arnd Bergmann wrote:
> On Thursday 13 November 2014 11:52:39 Ludovic Desroches wrote:
> > _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.
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > ---
> > 
> 
> 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.
And why is _relaxed() version required for thsi driver. Why cant readl(),
writel() with barriers do?

-- 
~Vinod

[PATCH 1/7] dmaengine: at_xdmac: prefer usage of readl/writel_relaxed

[Ludovic Desroches]

On Thu, Nov 13, 2014 at 09:04:49PM +0530, Vinod Koul wrote:
> On Thu, Nov 13, 2014 at 12:01:49PM +0100, Arnd Bergmann wrote:
> > On Thursday 13 November 2014 11:52:39 Ludovic Desroches wrote:
> > > _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.
> > > 
> > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > ---
> > > 
> > 
> > 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.
> And why is _relaxed() version required for thsi driver. Why cant readl(),
> writel() with barriers do?

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

> 
> -- 
> ~Vinod
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] dmaengine: at_xdmac: fix missing spin_unlock

[Vinod Koul]

On Thu, Nov 13, 2014 at 11:52:43AM +0100, Ludovic Desroches wrote:
> Lock taken when entering the function but unlock missing before it
> returns.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
I am adding reported to tag  here for Dan's report

-- 
~Vinod

[PATCH 1/7] dmaengine: at_xdmac: prefer usage of readl/writel_relaxed

[Vinod Koul]

On Thu, Nov 13, 2014 at 04:45:51PM +0100, Ludovic Desroches wrote:
> On Thu, Nov 13, 2014 at 09:04:49PM +0530, Vinod Koul wrote:
> > On Thu, Nov 13, 2014 at 12:01:49PM +0100, Arnd Bergmann wrote:
> > > On Thursday 13 November 2014 11:52:39 Ludovic Desroches wrote:
> > > > _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.
> > > > 
> > > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > > ---
> > > > 
> > > 
> > > 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.
> > And why is _relaxed() version required for thsi driver. Why cant readl(),
> > writel() with barriers do?
> 
> 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
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.
> - remove barrier since they are no more needed if using readl/writel
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