dma: pdma: some patches needed for PXA DT environments

PATCH Superseded Daniel Mack 5 participants 26 messages Aug 7, 2013 – Aug 16, 2013 dmaengine

Share & tools

Daniel Mack

author

2013-08-07 10:19 UTC #0 lore raw

cover dma: pdma: some patches needed for PXA DT environments

I've been working on the PXA DMA transition for some days now, and tweaked the mmp-pdma driver in order to make it work in pure DT environments.

I also ported over most PXA peripheral drivers, but that will take some more time to settle. I'll post those in a separate series. Up front, here are some patches for the mmp-pdma that can be reviewed and merged independently.

With these patches applied, I can successfully use pxa3xx-nand, pxamci and audio on a custom pxa3xx board.

I'm including two patches from Xiang Wang which have been posted but not yet merged, as far as I can tell.

Many thanks,
Daniel

Daniel Mack (10):
dma: mmp_pdma: factor out DRCMR register calculation
dma: mmp_pdma: fix maximum transfer length
dma: mmp_pdma: add filter function
dma: mmp_pdma: make the controller a DMA provider
dma: mmp_pdma: print the number of channels at probe time
dma: mmp_pdma: remove duplicate assignment
dma: mmp_pdma: add support for byte-aligned transfers
dma: mmp_pdma: implement DMA_PAUSE and DMA_RESUME
dma: mmp_pdma: add support for residue reporting
dma: mmp_pdma: add support for cyclic DMA descriptors

Xiang Wang (2):
dma: mmp_pdma: add protect when alloc/free phy channels
dma: mmp_pdma: clear DRCMR when free a phy channel

 drivers/dma/mmp_pdma.c       | 274 +++++++++++++++++++++++++++++++++++++++----
 include/linux/dma/mmp-pdma.h |  15 +++
 2 files changed, 263 insertions(+), 26 deletions(-)
 create mode 100644 include/linux/dma/mmp-pdma.h

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #1 lore raw

PATCH 1/12 dma: mmp_pdma: add protect when alloc/free phy channels

From: Xiang Wang <[email protected]>

In mmp pdma, phy channels are allocated/freed dynamically and frequently. But no proper protection is added. Conflict will happen when multi-users are requesting phy channels at the same time. Use spinlock to protect.

Signed-off-by Xiang Wang <[email protected]>

---
 drivers/dma/mmp_pdma.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

drivers/dma/mmp_pdma.c +26 ~~-16~~

index c26699f..226158d 100644
@@ -121,6 +121,7 @@ struct mmp_pdma_device {
 	struct device			*dev;
 	struct dma_device		device;
 	struct mmp_pdma_phy		*phy;
+	spinlock_t phy_lock; /* protect alloc/free phy channels */
 };
 
 #define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx)

@@ -219,6 +220,7 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	int prio, i;
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	struct mmp_pdma_phy *phy;
+	unsigned long flags;
 
 	/*
 	 * dma channel priorities

@@ -227,6 +229,8 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	 * ch 8 - 11, 24 - 27  <--> (2)
 	 * ch 12 - 15, 28 - 31  <--> (3)
 	 */
+
+	spin_lock_irqsave(&pdev->phy_lock, flags);
 	for (prio = 0; prio <= (((pdev->dma_channels - 1) & 0xf) >> 2); prio++) {
 		for (i = 0; i < pdev->dma_channels; i++) {
 			if (prio != ((i & 0xf) >> 2))

@@ -234,14 +238,30 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 			phy = &pdev->phy[i];
 			if (!phy->vchan) {
 				phy->vchan = pchan;
+				spin_unlock_irqrestore(&pdev->phy_lock, flags);
 				return phy;
 			}
 		}
 	}
 
+	spin_unlock_irqrestore(&pdev->phy_lock, flags);
 	return NULL;
 }
 
+static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
+{
+	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
+	unsigned long flags;
+
+	if (!pchan->phy)
+		return;
+
+	spin_lock_irqsave(&pdev->phy_lock, flags);
+	pchan->phy->vchan = NULL;
+	pchan->phy = NULL;
+	spin_unlock_irqrestore(&pdev->phy_lock, flags);
+}
+
 /* desc->tx_list ==> pending list */
 static void append_pending_queue(struct mmp_pdma_chan *chan,
 					struct mmp_pdma_desc_sw *desc)

@@ -277,10 +297,7 @@ static void start_pending_queue(struct mmp_pdma_chan *chan)
 
 	if (list_empty(&chan->chain_pending)) {
 		/* chance to re-fetch phy channel with higher prio */
-		if (chan->phy) {
-			chan->phy->vchan = NULL;
-			chan->phy = NULL;
-		}
+		mmp_pdma_free_phy(chan);
 		dev_dbg(chan->dev, "no pending list\n");
 		return;
 	}

@@ -377,10 +394,7 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan)
 		dev_err(chan->dev, "unable to allocate descriptor pool\n");
 		return -ENOMEM;
 	}
-	if (chan->phy) {
-		chan->phy->vchan = NULL;
-		chan->phy = NULL;
-	}
+	mmp_pdma_free_phy(chan);
 	chan->idle = true;
 	chan->dev_addr = 0;
 	return 1;

@@ -411,10 +425,7 @@ static void mmp_pdma_free_chan_resources(struct dma_chan *dchan)
 	chan->desc_pool = NULL;
 	chan->idle = true;
 	chan->dev_addr = 0;
-	if (chan->phy) {
-		chan->phy->vchan = NULL;
-		chan->phy = NULL;
-	}
+	mmp_pdma_free_phy(chan);
 	return;
 }

@@ -581,10 +592,7 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
 	switch (cmd) {
 	case DMA_TERMINATE_ALL:
 		disable_chan(chan->phy);
-		if (chan->phy) {
-			chan->phy->vchan = NULL;
-			chan->phy = NULL;
-		}
+		mmp_pdma_free_phy(chan);
 		spin_lock_irqsave(&chan->desc_lock, flags);
 		mmp_pdma_free_desc_list(chan, &chan->chain_pending);
 		mmp_pdma_free_desc_list(chan, &chan->chain_running);

@@ -777,6 +785,8 @@ static int mmp_pdma_probe(struct platform_device *op)
 		return -ENOMEM;
 	pdev->dev = &op->dev;
 
+	spin_lock_init(&pdev->phy_lock);
+
 	iores = platform_get_resource(op, IORESOURCE_MEM, 0);
 	if (!iores)
 		return -EINVAL;

Signature · 1 line

1.8.3.1

Ezequiel Garcia

2013-08-07 12:40 UTC #2 lore raw

PATCH 1/12 dma: mmp_pdma: add protect when alloc/free phy channels

Daniel,

It's good to see this effort is moving forward!

I just have a minor nitpick.

Quoted 20 lines · Daniel Mack

#define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx)

drivers/dma/mmp_pdma.c +3 -0

@@ -219,6 +220,7 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	int prio, i;
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	struct mmp_pdma_phy *phy;
+	unsigned long flags;
 
 	/*
 	 * dma channel priorities

@@ -227,6 +229,8 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	 * ch 8 - 11, 24 - 27  <--> (2)
 	 * ch 12 - 15, 28 - 31  <--> (3)
 	 */
+
+	spin_lock_irqsave(&pdev->phy_lock, flags);
 	for (prio = 0; prio <= (((pdev->dma_channels - 1) & 0xf) >> 2); prio++) {
 		for (i = 0; i < pdev->dma_channels; i++) {
 			if (prio != ((i & 0xf) >> 2))

drivers/dma/mmp_pdma.c static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) · line 241

Quoted 4 lines

drivers/dma/mmp_pdma.c +1 -0

@@ -234,14 +238,30 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 			phy = &pdev->phy[i];
 			if (!phy->vchan) {
 				phy->vchan = pchan;
+				spin_unlock_irqrestore(&pdev->phy_lock, flags);

Isn't it better to goto ...

drivers/dma/mmp_pdma.c static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) · line 246

Quoted 5 lines

drivers/dma/mmp_pdma.c +0 -0

 				return phy;
 			}
 		}
 	}

... here?

I think it's generally cleaner and less error-prone.

Quoted 4 lines

drivers/dma/mmp_pdma.c +1 -0

+	spin_unlock_irqrestore(&pdev->phy_lock, flags);
 	return NULL;
 }

Signature · 3 lines

Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Daniel Mack

author

2013-08-07 12:42 UTC #3 lore raw

PATCH 1/12 dma: mmp_pdma: add protect when alloc/free phy channels

Quoted 34 lines · Ezequiel Garcia

Quoted 20 lines · Daniel Mack

#define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx)

drivers/dma/mmp_pdma.c +3 -0

@@ -219,6 +220,7 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	int prio, i;
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	struct mmp_pdma_phy *phy;
+	unsigned long flags;
 
 	/*
 	 * dma channel priorities

@@ -227,6 +229,8 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	 * ch 8 - 11, 24 - 27  <--> (2)
 	 * ch 12 - 15, 28 - 31  <--> (3)
 	 */
+
+	spin_lock_irqsave(&pdev->phy_lock, flags);
 	for (prio = 0; prio <= (((pdev->dma_channels - 1) & 0xf) >> 2); prio++) {
 		for (i = 0; i < pdev->dma_channels; i++) {
 			if (prio != ((i & 0xf) >> 2))

drivers/dma/mmp_pdma.c static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) · line 241

Quoted 4 lines

drivers/dma/mmp_pdma.c +1 -0

@@ -234,14 +238,30 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 			phy = &pdev->phy[i];
 			if (!phy->vchan) {
 				phy->vchan = pchan;
+				spin_unlock_irqrestore(&pdev->phy_lock, flags);

Isn't it better to goto ...

drivers/dma/mmp_pdma.c static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) · line 246

Quoted 5 lines

drivers/dma/mmp_pdma.c +0 -0

 				return phy;
 			}
 		}
 	}

Quoted 3 lines

... here?
I think it's generally cleaner and less error-prone.

It's not my patch, but I can do that, unless Xiang Wang has objections.

Thanks,
Daniel

Daniel Mack

author

2013-08-08 08:35 UTC #4 lore raw

PATCH 1/12 dma: mmp_pdma: add protect when alloc/free phy channels

In reply to Ezequiel Garcia

Quoted 5 lines · Ezequiel Garcia

Daniel,
It's good to see this effort is moving forward!
I just have a minor nitpick.

I just realized that Vinod already commited this patch to his tree, so I won't be sending it again.

Daniel

Quoted 44 lines · Daniel Mack

Quoted 20 lines
#define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx)
drivers/dma/mmp_pdma.c +3 -0
@@ -219,6 +220,7 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	int prio, i;
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	struct mmp_pdma_phy *phy;
+	unsigned long flags;
 
 	/*
 	 * dma channel priorities
@@ -227,6 +229,8 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 	 * ch 8 - 11, 24 - 27  <--> (2)
 	 * ch 12 - 15, 28 - 31  <--> (3)
 	 */
+
+	spin_lock_irqsave(&pdev->phy_lock, flags);
 	for (prio = 0; prio <= (((pdev->dma_channels - 1) & 0xf) >> 2); prio++) {
 		for (i = 0; i < pdev->dma_channels; i++) {
 			if (prio != ((i & 0xf) >> 2))
drivers/dma/mmp_pdma.c static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) · line 241
Quoted 4 lines
drivers/dma/mmp_pdma.c +1 -0
@@ -234,14 +238,30 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 			phy = &pdev->phy[i];
 			if (!phy->vchan) {
 				phy->vchan = pchan;
+				spin_unlock_irqrestore(&pdev->phy_lock, flags);
Isn't it better to goto ...
drivers/dma/mmp_pdma.c static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) · line 246
Quoted 5 lines
drivers/dma/mmp_pdma.c +0 -0
 				return phy;
 			}
 		}
 	}
 
... here?
I think it's generally cleaner and less error-prone.
Quoted 4 lines
drivers/dma/mmp_pdma.c +1 -0
+	spin_unlock_irqrestore(&pdev->phy_lock, flags);
 	return NULL;
 }
 

Daniel Mack

author

2013-08-07 10:19 UTC #5 lore raw

PATCH 2/12 dma: mmp_pdma: clear DRCMR when free a phy channel

In reply to Daniel Mack

From: Xiang Wang <[email protected]>

In mmp pdma, phy channels are allocated/freed dynamically. The mapping from DMA request to DMA channel number in DRCMR should be cleared when a phy channel is freed. Otherwise conflicts will happen when:
1. A is using channel 2 and free it after finished, but A still maps to channel 2 in DRCMR of A.
2. Now another one B gets channel 2. So B maps to channel 2 too in DRCMR of B.
In the datasheet, it is described that "Do not map two active requests to the same channel since it produces unpredictable results" and we can observe that during test.

Signed-off-by Xiang Wang <[email protected]>

---
 drivers/dma/mmp_pdma.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/dma/mmp_pdma.c +6 -0

index 226158d..2844eaf 100644
@@ -252,10 +252,16 @@ static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
 {
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	unsigned long flags;
+	u32 reg;
 
 	if (!pchan->phy)
 		return;
 
+	/* clear the channel mapping in DRCMR */
+	reg = pchan->phy->vchan->drcmr;
+	reg = ((reg < 64) ? 0x0100 : 0x1100) + ((reg & 0x3f) << 2);
+	writel(0, pchan->phy->base + reg);
+
 	spin_lock_irqsave(&pdev->phy_lock, flags);
 	pchan->phy->vchan = NULL;
 	pchan->phy = NULL;

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #6 lore raw

PATCH 3/12 dma: mmp_pdma: factor out DRCMR register calculation

In reply to Daniel Mack

The exact same calculation is done twice, so let's factor it out to a macro.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/dma/mmp_pdma.c +4 -4

index 2844eaf..8d6aae3 100644
@@ -47,6 +47,8 @@
 #define DCSR_CMPST	(1 << 10)       /* The Descriptor Compare Status */
 #define DCSR_EORINTR	(1 << 9)        /* The end of Receive */
 
+#define DRCMR(n)	((((n) < 64) ? 0x0100 : 0x1100) + \
+				 (((n) & 0x3f) << 2))
 #define DRCMR_MAPVLD	(1 << 7)	/* Map Valid (read / write) */
 #define DRCMR_CHLNUM	0x1f		/* mask for Channel Number (read / write) */

@@ -143,8 +145,7 @@ static void enable_chan(struct mmp_pdma_phy *phy)
 	if (!phy->vchan)
 		return;
 
-	reg = phy->vchan->drcmr;
-	reg = (((reg) < 64) ? 0x0100 : 0x1100) + (((reg) & 0x3f) << 2);
+	reg = DRCMR(phy->vchan->drcmr);
 	writel(DRCMR_MAPVLD | phy->idx, phy->base + reg);
 
 	reg = (phy->idx << 2) + DCSR;

@@ -258,8 +259,7 @@ static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
 		return;
 
 	/* clear the channel mapping in DRCMR */
-	reg = pchan->phy->vchan->drcmr;
-	reg = ((reg < 64) ? 0x0100 : 0x1100) + ((reg & 0x3f) << 2);
+	reg = DRCMR(pchan->phy->vchan->drcmr);
 	writel(0, pchan->phy->base + reg);
 
 	spin_lock_irqsave(&pdev->phy_lock, flags);

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #7 lore raw

PATCH 4/12 dma: mmp_pdma: fix maximum transfer length

In reply to Daniel Mack

There's no reason for limiting the maximum transfer length to 0x1000. Take the actual bit mask instead; the PDMA is able to transfer chunks of up to SZ_8K - 1.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/dma/mmp_pdma.c +1 -1

index 8d6aae3..1fe78d8 100644
@@ -71,7 +71,7 @@
 #define DCMD_LENGTH	0x01fff		/* length mask (max = 8K - 1) */
 
 #define PDMA_ALIGNMENT		3
-#define PDMA_MAX_DESC_BYTES	0x1000
+#define PDMA_MAX_DESC_BYTES	DCMD_LENGTH
 
 struct mmp_pdma_desc_hw {
 	u32 ddadr;	/* Points to the next descriptor + flags */

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #8 lore raw

PATCH 5/12 dma: mmp_pdma: add filter function

In reply to Daniel Mack

PXA peripherals need to obtain specific DMA request ids which will eventually be stored in the DRCMR register.

Currently, clients are expected to store that number inside the slave config block as slave_id, which is unfortunately incompatible with the way DMA resources are handled in DT environments.

This patch adds a filter function which stores the filter parameter passed in by of-dma.c into the channel's drcmr register.

For backward compatability, cfg->slave_id is still used if set to a non-zero value.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c       | 21 ++++++++++++++++++++-
 include/linux/dma/mmp-pdma.h | 15 +++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/dma/mmp-pdma.h

drivers/dma/mmp_pdma.c +20 -1

index 1fe78d8..60a1410 100644
@@ -19,6 +19,7 @@
 #include <linux/dmapool.h>
 #include <linux/of_device.h>
 #include <linux/of.h>
+#include <linux/dma/mmp-pdma.h>
 
 #include "dmaengine.h"

@@ -633,8 +634,13 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
 			chan->dcmd |= DCMD_BURST32;
 
 		chan->dir = cfg->direction;
-		chan->drcmr = cfg->slave_id;
 		chan->dev_addr = addr;
+		/* FIXME: drivers should be ported over to use the filter
+		 * function. Once that's done, the following two lines can
+		 * be removed.
+		 */
+		if (cfg->slave_id)
+			chan->drcmr = cfg->slave_id;
 		break;
 	default:
 		return -ENOSYS;

@@ -883,6 +889,19 @@ static struct platform_driver mmp_pdma_driver = {
 	.remove		= mmp_pdma_remove,
 };
 
+bool mmp_pdma_filter_fn(struct dma_chan *chan, void *param)
+{
+	struct mmp_pdma_chan *c = to_mmp_pdma_chan(chan);
+
+	if (chan->device->dev->driver != &mmp_pdma_driver.driver)
+		return false;
+
+	c->drcmr = *(unsigned int *) param;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(mmp_pdma_filter_fn);
+
 module_platform_driver(mmp_pdma_driver);
 
 MODULE_DESCRIPTION("MARVELL MMP Periphera DMA Driver");

include/linux/dma/mmp-pdma.h new file +15 -0

new file mode 100644
index 0000000..2dc9b2b
@@ -0,0 +1,15 @@
+#ifndef _MMP_PDMA_H_
+#define _MMP_PDMA_H_
+
+struct dma_chan;
+
+#ifdef CONFIG_MMP_PDMA
+bool mmp_pdma_filter_fn(struct dma_chan *chan, void *param);
+#else
+static inline bool mmp_pdma_filter_fn(struct dma_chan *chan, void *param)
+{
+	return false;
+}
+#endif
+
+#endif /* _MMP_PDMA_H_ */

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #9 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

In reply to Daniel Mack

This patch makes the mmp_pdma controller able to provide DMA resources in DT environments.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/dma/mmp_pdma.c +18 -0

index 60a1410..d60217a 100644
@@ -18,6 +18,7 @@
 #include <linux/platform_data/mmp_dma.h>
 #include <linux/dmapool.h>
 #include <linux/of_device.h>
+#include <linux/of_dma.h>
 #include <linux/of.h>
 #include <linux/dma/mmp-pdma.h>

@@ -783,6 +784,10 @@ static struct of_device_id mmp_pdma_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids);
 
+static struct of_dma_filter_info mmp_pdma_info = {
+	.filter_fn = mmp_pdma_filter_fn,
+};
+
 static int mmp_pdma_probe(struct platform_device *op)
 {
 	struct mmp_pdma_device *pdev;

@@ -869,6 +874,19 @@ static int mmp_pdma_probe(struct platform_device *op)
 		return ret;
 	}
 
+	if (op->dev.of_node) {
+		mmp_pdma_info.dma_cap = pdev->device.cap_mask;
+
+		/* Device-tree DMA controller registration */
+		ret = of_dma_controller_register(op->dev.of_node,
+						 of_dma_simple_xlate,
+						 &mmp_pdma_info);
+		if (ret < 0) {
+			dev_err(&op->dev, "of_dma_controller_register failed\n");
+			return ret;
+		}
+	}
+
 	dev_info(pdev->device.dev, "initialized\n");
 	return 0;
 }

Signature · 1 line

1.8.3.1

Arnd Bergmann

2013-08-07 16:12 UTC #10 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

Quoted 32 lines · Daniel Mack

This patch makes the mmp_pdma controller able to provide DMA resources in DT environments.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/dma/mmp_pdma.c +5 -0

index 60a1410..d60217a 100644
@@ -18,6 +18,7 @@
 #include <linux/platform_data/mmp_dma.h>
 #include <linux/dmapool.h>
 #include <linux/of_device.h>
+#include <linux/of_dma.h>
 #include <linux/of.h>
 #include <linux/dma/mmp-pdma.h>

@@ -783,6 +784,10 @@ static struct of_device_id mmp_pdma_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids);
 
+static struct of_dma_filter_info mmp_pdma_info = {
+	.filter_fn = mmp_pdma_filter_fn,
+};
+
 static int mmp_pdma_probe(struct platform_device *op)
 {
 	struct mmp_pdma_device *pdev;

drivers/dma/mmp_pdma.c static int mmp_pdma_probe(struct platform_device *op) · line 883

Quoted 10 lines

drivers/dma/mmp_pdma.c +7 -0

@@ -869,6 +874,19 @@ static int mmp_pdma_probe(struct platform_device *op)
 		return ret;
 	}
 
+	if (op->dev.of_node) {
+		mmp_pdma_info.dma_cap = pdev->device.cap_mask;
+
+		/* Device-tree DMA controller registration */
+		ret = of_dma_controller_register(op->dev.of_node,
+						 of_dma_simple_xlate,
+						 &mmp_pdma_info);

of_dma_simple_xlate can not be used if there is a chance that multiple instances of the same dma engine, or multiple different DMA engines are present in the system. I generally advise against using it.

Please have a look at the changes that Zhangfei Gao proposed in
http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
and see if you can do the same here.

Arnd

Daniel Mack

author

2013-08-07 16:17 UTC #11 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

Quoted 13 lines · Arnd Bergmann

drivers/dma/mmp_pdma.c static int mmp_pdma_probe(struct platform_device *op) · line 883
Quoted 7 lines · Daniel Mack
drivers/dma/mmp_pdma.c +7 -0
+	if (op->dev.of_node) {
+		mmp_pdma_info.dma_cap = pdev->device.cap_mask;
+
+		/* Device-tree DMA controller registration */
+		ret = of_dma_controller_register(op->dev.of_node,
+						 of_dma_simple_xlate,
+						 &mmp_pdma_info);
of_dma_simple_xlate can not be used if there is a chance that multiple instances of the same dma engine, or multiple different DMA engines are present in the system.

Both can't be the case really for PXA, but I see your point.

Quoted 3 lines

Please have a look at the changes that Zhangfei Gao proposed in
http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
and see if you can do the same here.

Ok, I can do the same. As I can directly access dma_spec->args[0] from that context, that approach would also solve the problem with the hard-coded filter function, right?

Thanks,
Daniel

Arnd Bergmann

2013-08-07 20:17 UTC #12 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

Quoted 7 lines · Daniel Mack

Quoted 3 lines
Please have a look at the changes that Zhangfei Gao proposed in
http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
and see if you can do the same here.
Ok, I can do the same. As I can directly access dma_spec->args[0] from that context, that approach would also solve the problem with the hard-coded filter function, right?

You mean the problem of using the exported filter function pointer in other drivers? No, since the filter function is used only for the non-DT path, while the xlate function is only used for DT.

Arnd

Daniel Mack

author

2013-08-08 08:38 UTC #13 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

In reply to Arnd Bergmann

Quoted 18 lines · Arnd Bergmann

drivers/dma/mmp_pdma.c static int mmp_pdma_probe(struct platform_device *op) · line 883
Quoted 11 lines · Daniel Mack
drivers/dma/mmp_pdma.c +7 -0
@@ -869,6 +874,19 @@ static int mmp_pdma_probe(struct platform_device *op)
 		return ret;
 	}
 
+	if (op->dev.of_node) {
+		mmp_pdma_info.dma_cap = pdev->device.cap_mask;
+
+		/* Device-tree DMA controller registration */
+		ret = of_dma_controller_register(op->dev.of_node,
+						 of_dma_simple_xlate,
+						 &mmp_pdma_info);
of_dma_simple_xlate can not be used if there is a chance that multiple instances of the same dma engine, or multiple different DMA engines are present in the system. I generally advise against using it.

Quoted 3 lines

Please have a look at the changes that Zhangfei Gao proposed in
http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
and see if you can do the same here.

Ok, I'll rebase my series on top of that one, hoping that Zhangfei's patch will make it upstream before mine.

Daniel

Daniel Mack

author

2013-08-09 13:10 UTC #14 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

In reply to Arnd Bergmann

Quoted 13 lines · Arnd Bergmann

drivers/dma/mmp_pdma.c static int mmp_pdma_probe(struct platform_device *op) · line 883
Quoted 7 lines · Daniel Mack
drivers/dma/mmp_pdma.c +7 -0
+	if (op->dev.of_node) {
+		mmp_pdma_info.dma_cap = pdev->device.cap_mask;
+
+		/* Device-tree DMA controller registration */
+		ret = of_dma_controller_register(op->dev.of_node,
+						 of_dma_simple_xlate,
+						 &mmp_pdma_info);
of_dma_simple_xlate can not be used if there is a chance that multiple instances of the same dma engine, or multiple different DMA engines are present in the system. I generally advise against using it.

Quoted 3 lines

Please have a look at the changes that Zhangfei Gao proposed in
http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
and see if you can do the same here.

I had another look at that and Zhangfei's case is not really applicable to mine, unfortunately.

In his case, one specific out of many channels has to be used, depending on the first argument of the phandle. In my case though, the pdma controller may just take any of its channels, and just assign the correct DMA request to it.

So if I provide a private xlate function, I need a way to obtain *any* of the channels in my instance. Open-coding that is not easily possible, as I need to hold the dmaengine's local dma_list_mutex for that.

I have to dig deeper here, but if anyone has a hint, please let me know :)

Daniel

zhangfei gao

2013-08-09 14:32 UTC #15 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

Quoted 18 lines · Daniel Mack

Quoted 9 lines · Arnd Bergmann
drivers/dma/mmp_pdma.c static int mmp_pdma_probe(struct platform_device *op) · line 883
Quoted 7 lines · Daniel Mack
drivers/dma/mmp_pdma.c +7 -0
+    if (op->dev.of_node) {
+            mmp_pdma_info.dma_cap = pdev->device.cap_mask;
+
+            /* Device-tree DMA controller registration */
+            ret = of_dma_controller_register(op->dev.of_node,
+                                             of_dma_simple_xlate,
+                                             &mmp_pdma_info);
Quoted 7 lines
of_dma_simple_xlate can not be used if there is a chance that multiple instances of the same dma engine, or multiple different DMA engines are present in the system. I generally advise against using it.
Please have a look at the changes that Zhangfei Gao proposed in
http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
and see if you can do the same here.

Quoted 7 lines

I had another look at that and Zhangfei's case is not really applicable to mine, unfortunately.
In his case, one specific out of many channels has to be used, depending on the first argument of the phandle. In my case though, the pdma controller may just take any of its channels, and just assign the correct DMA request to it.

Dear Daniel

Though any physical channel is workable, the virtual channel does not. Each device has to set specific request line.

pdma.c
chan->drcmr = cfg->slave_id;
nand.c
conf.slave_id = info->drcmr_dat;

The specific virtual channel can be directly specificied by request line. While pdma.c choose the free physical channel inside, which is transparent to client.

It should be same.

Thanks

Quoted 15 lines

So if I provide a private xlate function, I need a way to obtain *any* of the channels in my instance. Open-coding that is not easily possible, as I need to hold the dmaengine's local dma_list_mutex for that.
I have to dig deeper here, but if anyone has a hint, please let me know :)
Daniel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Arnd Bergmann

2013-08-09 21:08 UTC #16 lore raw

PATCH 6/12 dma: mmp_pdma: make the controller a DMA provider

Quoted 11 lines · zhangfei gao

Quoted 10 lines · Daniel Mack
On 07.08.2013 18:12, Arnd Bergmann wrote:
I had another look at that and Zhangfei's case is not really applicable to mine, unfortunately.
In his case, one specific out of many channels has to be used, depending on the first argument of the phandle. In my case though, the pdma controller may just take any of its channels, and just assign the correct DMA request to it.

This should still be fine since your driver can keep track of which channels are currently in use. The locking can be done inside of the dmaengine core, but you might have to retry if the channel is getting acquired between your lookup and the lock.

Quoted 11 lines

Though any physical channel is workable, the virtual channel does not. Each device has to set specific request line.
pdma.c
chan->drcmr = cfg->slave_id;
nand.c
conf.slave_id = info->drcmr_dat;
The specific virtual channel can be directly specificied by request line. While pdma.c choose the free physical channel inside, which is transparent to client.

It is a bug to override the slave_id value from the dma slave driver when using dma_request_slave_channel, and the dmaengine driver should not allow that. The slave_config function should only be used to set the configuration parts that the slave driver knows about, which does not include the slave_id in this case (since there is no platform data).

Arnd

Daniel Mack

author

2013-08-07 10:19 UTC #17 lore raw

PATCH 7/12 dma: mmp_pdma: print the number of channels at probe time

In reply to Daniel Mack

That helps check the provided runtime information.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/dma/mmp_pdma.c +1 -1

index d60217a..2d48f31 100644
@@ -887,7 +887,7 @@ static int mmp_pdma_probe(struct platform_device *op)
 		}
 	}
 
-	dev_info(pdev->device.dev, "initialized\n");
+	dev_info(pdev->device.dev, "initialized %d channels\n", dma_channels);
 	return 0;
 }

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #18 lore raw

PATCH 8/12 dma: mmp_pdma: remove duplicate assignment

In reply to Daniel Mack

The DMA_SLAVE is currently set twice.

Signed-off-by Daniel Mack <[email protected]>

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

drivers/dma/mmp_pdma.c +0 -1

index 2d48f31..1c2c00b 100644
@@ -852,7 +852,6 @@ static int mmp_pdma_probe(struct platform_device *op)
 
 	dma_cap_set(DMA_SLAVE, pdev->device.cap_mask);
 	dma_cap_set(DMA_MEMCPY, pdev->device.cap_mask);
-	dma_cap_set(DMA_SLAVE, pdev->device.cap_mask);
 	pdev->device.dev = &op->dev;
 	pdev->device.device_alloc_chan_resources = mmp_pdma_alloc_chan_resources;
 	pdev->device.device_free_chan_resources = mmp_pdma_free_chan_resources;

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #19 lore raw

PATCH 9/12 dma: mmp_pdma: add support for byte-aligned transfers

In reply to Daniel Mack

The PXA DMA controller has a DALGN register which allows for byte-aligned DMA transfers. Use it in case any of the transfer descriptors is not aligned to a mask of ~0x7.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/dma/mmp_pdma.c +16 -1

index 1c2c00b..7eb235b 100644
@@ -109,6 +109,7 @@ struct mmp_pdma_chan {
 	struct list_head chain_pending;	/* Link descriptors queue for pending */
 	struct list_head chain_running;	/* Link descriptors queue for running */
 	bool idle;			/* channel statue machine */
+	bool byte_align;
 
 	struct dma_pool *desc_pool;	/* Descriptors pool */
 };

@@ -142,7 +143,7 @@ static void set_desc(struct mmp_pdma_phy *phy, dma_addr_t addr)
 
 static void enable_chan(struct mmp_pdma_phy *phy)
 {
-	u32 reg;
+	u32 reg, dalgn;
 
 	if (!phy->vchan)
 		return;

@@ -150,6 +151,13 @@ static void enable_chan(struct mmp_pdma_phy *phy)
 	reg = DRCMR(phy->vchan->drcmr);
 	writel(DRCMR_MAPVLD | phy->idx, phy->base + reg);
 
+	dalgn = readl(phy->base + DALGN);
+	if (phy->vchan->byte_align)
+		dalgn |= 1 << phy->idx;
+	else
+		dalgn &= ~(1 << phy->idx);
+	writel(dalgn, phy->base + DALGN);
+
 	reg = (phy->idx << 2) + DCSR;
 	writel(readl(phy->base + reg) | DCSR_RUN,
 					phy->base + reg);

@@ -453,6 +461,7 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		return NULL;
 
 	chan = to_mmp_pdma_chan(dchan);
+	chan->byte_align = false;
 
 	if (!chan->dir) {
 		chan->dir = DMA_MEM_TO_MEM;

@@ -469,6 +478,8 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		}
 
 		copy = min_t(size_t, len, PDMA_MAX_DESC_BYTES);
+		if (dma_src & 0x7 || dma_dst & 0x7)
+			chan->byte_align = true;
 
 		new->desc.dcmd = chan->dcmd | (DCMD_LENGTH & copy);
 		new->desc.dsadr = dma_src;

@@ -528,12 +539,16 @@ mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 	if ((sgl == NULL) || (sg_len == 0))
 		return NULL;
 
+	chan->byte_align = false;
+
 	for_each_sg(sgl, sg, sg_len, i) {
 		addr = sg_dma_address(sg);
 		avail = sg_dma_len(sgl);
 
 		do {
 			len = min_t(size_t, avail, PDMA_MAX_DESC_BYTES);
+			if (addr & 0x7)
+				chan->byte_align = true;
 
 			/* allocate and populate the descriptor */
 			new = mmp_pdma_alloc_descriptor(chan);

Signature · 1 line

1.8.3.1

Xiang Wang

2013-08-08 09:04 UTC #20 lore raw

PATCH 9/12 dma: mmp_pdma: add support for byte-aligned transfers

Quoted 81 lines

Subject: [PATCH 09/12] dma: mmp_pdma: add support for byte-aligned transfers

The PXA DMA controller has a DALGN register which allows for byte-aligned DMA transfers. Use it in case any of the transfer descriptors is not aligned to a mask of ~0x7.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/dma/mmp_pdma.c +16 -2

index 1c2c00b..7eb235b 100644
@@ -109,6 +109,7 @@ struct mmp_pdma_chan {
 	struct list_head chain_pending;	/* Link descriptors queue for pending */
 	struct list_head chain_running;	/* Link descriptors queue for running */
 	bool idle;			/* channel statue machine */
+	bool byte_align;
 
 	struct dma_pool *desc_pool;	/* Descriptors pool */
 };

@@ -142,7 +143,7 @@ static void set_desc(struct mmp_pdma_phy *phy, dma_addr_t addr)
 
 static void enable_chan(struct mmp_pdma_phy *phy)
 {
-	u32 reg;
+	u32 reg, dalgn;
 
 	if (!phy->vchan)
 		return;

@@ -150,6 +151,13 @@ static void enable_chan(struct mmp_pdma_phy *phy)
 	reg = DRCMR(phy->vchan->drcmr);
 	writel(DRCMR_MAPVLD | phy->idx, phy->base + reg);
 
+	dalgn = readl(phy->base + DALGN);
+	if (phy->vchan->byte_align)
+		dalgn |= 1 << phy->idx;
+	else
+		dalgn &= ~(1 << phy->idx);
+	writel(dalgn, phy->base + DALGN);
+
 	reg = (phy->idx << 2) + DCSR;
 	writel(readl(phy->base + reg) | DCSR_RUN,
 					phy->base + reg);

@@ -453,6 +461,7 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		return NULL;
 
 	chan = to_mmp_pdma_chan(dchan);
+	chan->byte_align = false;
 
 	if (!chan->dir) {
 		chan->dir = DMA_MEM_TO_MEM;

@@ -469,6 +478,8 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		}
 
 		copy = min_t(size_t, len, PDMA_MAX_DESC_BYTES);
+		if (dma_src & 0x7 || dma_dst & 0x7)
+			chan->byte_align = true;
 
 		new->desc.dcmd = chan->dcmd | (DCMD_LENGTH & copy);
 		new->desc.dsadr = dma_src;

@@ -528,12 +539,16 @@ mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 	if ((sgl == NULL) || (sg_len == 0))
 		return NULL;
 
+	chan->byte_align = false;
+
 	for_each_sg(sgl, sg, sg_len, i) {
 		addr = sg_dma_address(sg);
 		avail = sg_dma_len(sgl);
 
 		do {
 			len = min_t(size_t, avail, PDMA_MAX_DESC_BYTES);
+			if (addr & 0x7)
+				chan->byte_align = true;
 
 			/* allocate and populate the descriptor */
 			new = mmp_pdma_alloc_descriptor(chan);
--

Quoted 1 line

1.8.3.1

We do need to set DALGN bit in some of our drivers. But we cannot configure this via standard dma engine API.
In this patch, dma address is used to determine whether or not to set DALGN. But what if we need to use 1-byte-aligned mode when addresses are 8-byte-aligned? Is it proper to always use 1-byte-aligned mode?
Thanks.

Regards,
Xiang

Daniel Mack

author

2013-08-08 09:11 UTC #21 lore raw

PATCH 9/12 dma: mmp_pdma: add support for byte-aligned transfers

Hi Xiang,

Quoted 6 lines · Xiang Wang

Quoted 6 lines
Subject: [PATCH 09/12] dma: mmp_pdma: add support for byte-aligned transfers
The PXA DMA controller has a DALGN register which allows for byte-aligned DMA transfers. Use it in case any of the transfer descriptors is not aligned to a mask of ~0x7.

[...]

Quoted 1 line

We do need to set DALGN bit in some of our drivers.

Which ones, and how do you currently do that? I didn't find any code to support this yet in mmp-pdma.

Quoted 5 lines

But we cannot
configure this via standard dma engine API. In this patch, dma address is used to determine whether or not to set DALGN. But what if we need to use 1-byte-aligned mode when addresses are 8-byte-aligned?

Hmm, why would you need that? What's the constraint for this driver that they have to rely on that?

Quoted 1 line

Is it proper to always use 1-byte-aligned mode?

As far as I understand the datasheet, this bit has performance implications and should only be used if really needed.

I think if you have that constraint in drivers, and the dmaengine implementation can't determine that automatically, we should introduce a flag or something in the dma_slave_config struct.

Daniel

Xiang Wang

2013-08-16 08:05 UTC #22 lore raw

PATCH 9/12 dma: mmp_pdma: add support for byte-aligned transfers

Quoted 10 lines · Daniel Mack

Hi Xiang,
Quoted 6 lines · Xiang Wang
Quoted 6 lines
Subject: [PATCH 09/12] dma: mmp_pdma: add support for byte-aligned transfers
The PXA DMA controller has a DALGN register which allows for byte-aligned DMA transfers. Use it in case any of the transfer descriptors is not aligned to a mask of ~0x7.

Quoted 6 lines

[...]
Quoted 1 line
We do need to set DALGN bit in some of our drivers.
Which ones, and how do you currently do that? I didn't find any code to support this yet in mmp-pdma.

Hi, Daniel
When we use DMA in UART and let DMA handle the UART trailing bytes, we should set the DALGN bit. Otherwise, DMA controller cannot move the bytes from UART FIFO to memory correctly because they are not 8-bytes-alligned.

Quoted 22 lines

Quoted 6 lines
But we cannot
configure this via standard dma engine API. In this patch, dma address is used to determine whether or not to set DALGN. But what if we need to use 1-byte-aligned mode when addresses are 8-byte-aligned?
Hmm, why would you need that? What's the constraint for this driver that they have to rely on that?
Quoted 1 line
Is it proper to always use 1-byte-aligned mode?
As far as I understand the datasheet, this bit has performance implications and should only be used if really needed.
I think if you have that constraint in drivers, and the dmaengine implementation can't determine that automatically, we should introduce a flag or something in the dma_slave_config struct.
Daniel

Signature · 2 lines

Regards,
Xiang

Daniel Mack

author

2013-08-07 10:19 UTC #23 lore raw

PATCH 10/12 dma: mmp_pdma: implement DMA_PAUSE and DMA_RESUME

In reply to Daniel Mack

This is needed at least for audio operation.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/dma/mmp_pdma.c +6 -0

index 7eb235b..c56da7c 100644
@@ -658,6 +658,12 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
 		if (cfg->slave_id)
 			chan->drcmr = cfg->slave_id;
 		break;
+	case DMA_PAUSE:
+		disable_chan(chan->phy);
+		break;
+	case DMA_RESUME:
+		start_pending_queue(chan);
+		break;
 	default:
 		return -ENOSYS;
 	}

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #24 lore raw

PATCH 11/12 dma: mmp_pdma: add support for residue reporting

In reply to Daniel Mack

In order to report the channel's residue, we have to memorize the first dma buffer position when the channel is configured.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

drivers/dma/mmp_pdma.c +37 -2

index c56da7c..0c4a2d5 100644
@@ -28,8 +28,8 @@
 #define DALGN		0x00a0
 #define DINT		0x00f0
 #define DDADR		0x0200
-#define DSADR		0x0204
-#define DTADR		0x0208
+#define DSADR(n)	(0x0204 + ((n) << 4))
+#define DTADR(n)	(0x0208 + ((n) << 4))
 #define DCMD		0x020c
 
 #define DCSR_RUN	(1 << 31)	/* Run Bit (read / write) */

@@ -97,6 +97,13 @@ struct mmp_pdma_chan {
 	struct dma_async_tx_descriptor desc;
 	struct mmp_pdma_phy *phy;
 	enum dma_transfer_direction dir;
+	/*
+	 * We memorize the original start address of the first descriptor as
+	 * well as the original total length so we can later determine the
+	 * channel's residue.
+	 */
+	dma_addr_t start_addr;
+	u32 total_len;
 
 	/* channel's basic info */
 	struct tasklet_struct tasklet;

@@ -469,6 +476,13 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		chan->dcmd |= DCMD_BURST32;
 	}
 
+	if (chan->dir == DMA_MEM_TO_DEV)
+		chan->start_addr = dma_src;
+	else
+		chan->start_addr = dma_dst;
+
+	chan->total_len = len;
+
 	do {
 		/* Allocate the link descriptor from DMA pool */
 		new = mmp_pdma_alloc_descriptor(chan);

@@ -540,11 +554,17 @@ mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 		return NULL;
 
 	chan->byte_align = false;
+	chan->total_len = 0;
 
 	for_each_sg(sgl, sg, sg_len, i) {
 		addr = sg_dma_address(sg);
 		avail = sg_dma_len(sgl);
 
+		if (!first)
+			chan->start_addr = addr;
+
+		chan->total_len += avail;
+
 		do {
 			len = min_t(size_t, avail, PDMA_MAX_DESC_BYTES);
 			if (addr & 0x7)

@@ -671,6 +691,20 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
 	return ret;
 }
 
+static unsigned int mmp_pdma_residue(struct mmp_pdma_chan *chan)
+{
+	u32 curr, done;
+
+	if (chan->dir == DMA_DEV_TO_MEM)
+		curr = readl(chan->phy->base + DTADR(chan->phy->idx));
+	else
+		curr = readl(chan->phy->base + DSADR(chan->phy->idx));
+
+	done = curr - chan->start_addr;
+
+	return chan->total_len - done;
+}
+
 static enum dma_status mmp_pdma_tx_status(struct dma_chan *dchan,
 			dma_cookie_t cookie, struct dma_tx_state *txstate)
 {

@@ -680,6 +714,7 @@ static enum dma_status mmp_pdma_tx_status(struct dma_chan *dchan,
 
 	spin_lock_irqsave(&chan->desc_lock, flags);
 	ret = dma_cookie_status(dchan, cookie, txstate);
+	txstate->residue = mmp_pdma_residue(chan);
 	spin_unlock_irqrestore(&chan->desc_lock, flags);
 
 	return ret;

Signature · 1 line

1.8.3.1

Daniel Mack

author

2013-08-07 10:19 UTC #25 lore raw

PATCH 12/12 dma: mmp_pdma: add support for cyclic DMA descriptors

In reply to Daniel Mack

Provide a callback to prepare cyclic DMA transfers. This is for instance needed for audio channel transport.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/dma/mmp_pdma.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 115 insertions(+), 1 deletion(-)

drivers/dma/mmp_pdma.c +115 -1

index 0c4a2d5..e95a685 100644
@@ -97,6 +97,8 @@ struct mmp_pdma_chan {
 	struct dma_async_tx_descriptor desc;
 	struct mmp_pdma_phy *phy;
 	enum dma_transfer_direction dir;
+	struct mmp_pdma_desc_sw *cyclic_first;	/* first desc_sw if channel
+						 * is in cyclic mode */
 	/*
 	 * We memorize the original start address of the first descriptor as
 	 * well as the original total length so we can later determine the

@@ -530,6 +532,8 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 	new->desc.ddadr = DDADR_STOP;
 	new->desc.dcmd |= DCMD_ENDIRQEN;
 
+	chan->cyclic_first = NULL;
+
 	return &first->async_tx;
 
 fail:

@@ -611,6 +615,96 @@ mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 	new->desc.ddadr = DDADR_STOP;
 	new->desc.dcmd |= DCMD_ENDIRQEN;
 
+	chan->dir = dir;
+	chan->cyclic_first = NULL;
+
+	return &first->async_tx;
+
+fail:
+	if (first)
+		mmp_pdma_free_desc_list(chan, &first->tx_list);
+	return NULL;
+}
+
+static struct dma_async_tx_descriptor *mmp_pdma_prep_dma_cyclic(
+	struct dma_chan *dchan, dma_addr_t buf_addr, size_t len,
+	size_t period_len, enum dma_transfer_direction direction,
+	unsigned long flags, void *context)
+{
+	struct mmp_pdma_chan *chan;
+	struct mmp_pdma_desc_sw *first = NULL, *prev = NULL, *new;
+	dma_addr_t dma_src, dma_dst;
+
+	if (!dchan || !len || !period_len)
+		return NULL;
+
+	/* the buffer length must be a multiple of period_len */
+	if (len % period_len != 0)
+		return NULL;
+
+	if (period_len > PDMA_MAX_DESC_BYTES)
+		return NULL;
+
+	chan = to_mmp_pdma_chan(dchan);
+
+	switch (direction) {
+	case DMA_MEM_TO_DEV:
+		dma_src = buf_addr;
+		dma_dst = chan->dev_addr;
+		break;
+	case DMA_DEV_TO_MEM:
+		dma_dst = buf_addr;
+		dma_src = chan->dev_addr;
+		break;
+	default:
+		dev_err(chan->dev, "Unsupported direction for cyclic DMA\n");
+		return NULL;
+	}
+
+	chan->start_addr = buf_addr;
+	chan->total_len = len;
+	chan->dir = direction;
+
+	do {
+		/* Allocate the link descriptor from DMA pool */
+		new = mmp_pdma_alloc_descriptor(chan);
+		if (!new) {
+			dev_err(chan->dev, "no memory for desc\n");
+			goto fail;
+		}
+
+		new->desc.dcmd = chan->dcmd | DCMD_ENDIRQEN |
+					(DCMD_LENGTH & period_len);
+		new->desc.dsadr = dma_src;
+		new->desc.dtadr = dma_dst;
+
+		if (!first)
+			first = new;
+		else
+			prev->desc.ddadr = new->async_tx.phys;
+
+		new->async_tx.cookie = 0;
+		async_tx_ack(&new->async_tx);
+
+		prev = new;
+		len -= period_len;
+
+		if (chan->dir == DMA_MEM_TO_DEV)
+			dma_src += period_len;
+		else
+			dma_dst += period_len;
+
+		/* Insert the link descriptor to the LD ring */
+		list_add_tail(&new->node, &first->tx_list);
+	} while (len);
+
+	first->async_tx.flags = flags; /* client is in control of this ack */
+	first->async_tx.cookie = -EBUSY;
+
+	/* make the cyclic link */
+	new->desc.ddadr = first->async_tx.phys;
+	chan->cyclic_first = first;
+
 	return &first->async_tx;
 
 fail:

@@ -746,8 +840,25 @@ static void dma_do_tasklet(unsigned long data)
 	LIST_HEAD(chain_cleanup);
 	unsigned long flags;
 
-	/* submit pending list; callback for each desc; free desc */
+	if (chan->cyclic_first) {
+		dma_async_tx_callback cb = NULL;
+		void *cb_data = NULL;
+
+		spin_lock_irqsave(&chan->desc_lock, flags);
+		desc = chan->cyclic_first;
+		cb = desc->async_tx.callback;
+		cb_data = desc->async_tx.callback_param;
+		spin_unlock_irqrestore(&chan->desc_lock, flags);
+
+		start_pending_queue(chan);
 
+		if (cb)
+			cb(cb_data);
+
+		return;
+	}
+
+	/* submit pending list; callback for each desc; free desc */
 	spin_lock_irqsave(&chan->desc_lock, flags);
 
 	/* update the cookie if we have some descriptors to cleanup */

@@ -780,6 +891,7 @@ static void dma_do_tasklet(unsigned long data)
 
 		/* Remove from the list of transactions */
 		list_del(&desc->node);
+
 		/* Run the link descriptor callback function */
 		if (txd->callback)
 			txd->callback(txd->callback_param);

@@ -908,12 +1020,14 @@ static int mmp_pdma_probe(struct platform_device *op)
 
 	dma_cap_set(DMA_SLAVE, pdev->device.cap_mask);
 	dma_cap_set(DMA_MEMCPY, pdev->device.cap_mask);
+	dma_cap_set(DMA_CYCLIC, pdev->device.cap_mask);
 	pdev->device.dev = &op->dev;
 	pdev->device.device_alloc_chan_resources = mmp_pdma_alloc_chan_resources;
 	pdev->device.device_free_chan_resources = mmp_pdma_free_chan_resources;
 	pdev->device.device_tx_status = mmp_pdma_tx_status;
 	pdev->device.device_prep_dma_memcpy = mmp_pdma_prep_memcpy;
 	pdev->device.device_prep_slave_sg = mmp_pdma_prep_slave_sg;
+	pdev->device.device_prep_dma_cyclic = mmp_pdma_prep_dma_cyclic;
 	pdev->device.device_issue_pending = mmp_pdma_issue_pending;
 	pdev->device.device_control = mmp_pdma_control;
 	pdev->device.copy_align = PDMA_ALIGNMENT;

Signature · 1 line

1.8.3.1