[PATCH v2 00/11] dma: pdma: some patches needed for PXA DT environments

[PATCH v2 00/11] dma: pdma: some patches needed for PXA DT environments

[Daniel Mack]

This is v2 of my series to add more features to the pdma driver.

Changes from v1 -> v2:

 - drop the two patches from Xiang Wang that are already applied to
   Vinod's next branch
 - add another patch to cleanup the unlocking path in lookup_phy(),
   as suggested by Ezequiel
 - provide an own xlate function rather than using
   of_dma_simple_xlate(), as suggested by Arnd


There's still an open topic with the DALGN flag, as Xiang Wang pointed
out. According to him, there's a chance we need an alternative way of
forcing the driver into byte-align mode, but that can still be done in
an additional patch later.


Thanks,
Daniel



Daniel Mack (11):
  dma: mmp_pdma: factor out DRCMR register calculation
  dma: mmp_pdma: refactor unlocking path in lookup_phy()
  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

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

-- 
1.8.3.1

[PATCH v2 01/11] dma: mmp_pdma: factor out DRCMR register calculation

[Daniel Mack]

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

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 2844eaf..8d6aae3 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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);
-- 
1.8.3.1

[PATCH v2 02/11] dma: mmp_pdma: refactor unlocking path in lookup_phy()

[Daniel Mack]

As suggested by Ezequiel Garc?a, release the spinlock at the end of the
function only, and use a goto for the control flow.

Just a minor cleanup.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 8d6aae3..f253695 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -220,7 +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;
+	struct mmp_pdma_phy *phy, *found = NULL;
 	unsigned long flags;
 
 	/*
@@ -239,14 +239,15 @@ 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;
+				found = phy;
+				goto out_unlock;
 			}
 		}
 	}
 
+out_unlock:
 	spin_unlock_irqrestore(&pdev->phy_lock, flags);
-	return NULL;
+	return found;
 }
 
 static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
-- 
1.8.3.1

[PATCH v2 03/11] dma: mmp_pdma: fix maximum transfer length

[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 <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index f253695..9d6410e 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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 */
-- 
1.8.3.1

[PATCH v2 04/11] dma: mmp_pdma: add filter function

[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 <zonque@gmail.com>
---
 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

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 9d6410e..9dda27e 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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"
 
@@ -634,8 +635,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;
@@ -884,6 +890,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");
diff --git a/include/linux/dma/mmp-pdma.h b/include/linux/dma/mmp-pdma.h
new file mode 100644
index 0000000..2dc9b2b
--- /dev/null
+++ b/include/linux/dma/mmp-pdma.h
@@ -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_ */
-- 
1.8.3.1

[PATCH v2 05/11] dma: mmp_pdma: make the controller a DMA provider

[Daniel Mack]

This patch makes the mmp_pdma controller able to provide DMA resources
in DT environments by providing an dma xlate function.

of_dma_simple_xlate() isn't used here, because if fails to handle
multiple different DMA engines or several instances of the same
controller. Instead, a private implementation is provided that makes use
of the newly introduced dma_get_slave_channel() call.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 9dda27e..0ec5c21 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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>
 
@@ -784,6 +785,39 @@ static struct of_device_id mmp_pdma_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids);
 
+static struct dma_chan *mmp_pdma_dma_xlate(struct of_phandle_args *dma_spec,
+					   struct of_dma *ofdma)
+{
+	struct mmp_pdma_device *d = ofdma->of_dma_data;
+	struct dma_chan *chan, *candidate;
+
+retry:
+	candidate = NULL;
+
+	/* walk the list of channels registered with the current instance and
+	 * find one that is currently unused */
+	list_for_each_entry(chan, &d->device.channels, device_node)
+		if (chan->client_count == 0) {
+			candidate = chan;
+			break;
+		}
+
+	if (!candidate)
+		return NULL;
+
+	/* dma_get_slave_channel will return NULL if we lost a race between
+	 * the lookup and the reservation */
+	chan = dma_get_slave_channel(candidate);
+
+	if (chan) {
+		struct mmp_pdma_chan *c = to_mmp_pdma_chan(chan);
+		c->drcmr = dma_spec->args[0];
+		return chan;
+	}
+
+	goto retry;
+}
+
 static int mmp_pdma_probe(struct platform_device *op)
 {
 	struct mmp_pdma_device *pdev;
@@ -870,6 +904,16 @@ static int mmp_pdma_probe(struct platform_device *op)
 		return ret;
 	}
 
+	if (op->dev.of_node) {
+		/* Device-tree DMA controller registration */
+		ret = of_dma_controller_register(op->dev.of_node,
+						 mmp_pdma_dma_xlate, pdev);
+		if (ret < 0) {
+			dev_err(&op->dev, "of_dma_controller_register failed\n");
+			return ret;
+		}
+	}
+
 	dev_info(pdev->device.dev, "initialized\n");
 	return 0;
 }
-- 
1.8.3.1

[PATCH v2 06/11] dma: mmp_pdma: print the number of channels at probe time

[Daniel Mack]

That helps check the provided runtime information.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 0ec5c21..493f59c 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -914,7 +914,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;
 }
 
-- 
1.8.3.1

[PATCH v2 07/11] dma: mmp_pdma: remove duplicate assignment

[Daniel Mack]

The DMA_SLAVE is currently set twice.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 493f59c..d7ccc90 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -882,7 +882,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;
-- 
1.8.3.1

[PATCH v2 08/11] dma: mmp_pdma: add support for byte-aligned transfers

[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 <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index d7ccc90..579f79a 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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);
@@ -454,6 +462,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;
@@ -470,6 +479,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;
@@ -529,12 +540,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);
-- 
1.8.3.1

[PATCH v2 09/11] dma: mmp_pdma: implement DMA_PAUSE and DMA_RESUME

[Daniel Mack]

This is needed at least for audio operation.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 579f79a..7a0956b 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -659,6 +659,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;
 	}
-- 
1.8.3.1

[PATCH v2 10/11] dma: mmp_pdma: add support for residue reporting

[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 <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 7a0956b..ab0303b 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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;
@@ -470,6 +477,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);
@@ -541,11 +555,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)
@@ -672,6 +692,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)
 {
@@ -681,6 +715,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;
-- 
1.8.3.1

[PATCH v2 11/11] dma: mmp_pdma: add support for cyclic DMA descriptors

[Daniel Mack]

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

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 115 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index ab0303b..eba9d2d 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -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
@@ -531,6 +533,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:
@@ -612,6 +616,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:
@@ -747,8 +841,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 */
@@ -781,6 +892,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);
@@ -938,12 +1050,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;
-- 
1.8.3.1

[PATCH v2 00/11] dma: pdma: some patches needed for PXA DT environments

[Daniel Mack]

On 10.08.2013 18:52, Daniel Mack wrote:
> This is v2 of my series to add more features to the pdma driver.

I forgot to mention that this series depends on a patch from Zhangfei
Gao which provides dma_get_slave_channel(). It was posted here:

  http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077

[PATCH v2 00/11] dma: pdma: some patches needed for PXA DT environments

[Arnd Bergmann]

On Saturday 10 August 2013, Daniel Mack wrote:
> This is v2 of my series to add more features to the pdma driver.
> 
> Changes from v1 -> v2:
> 
>  - drop the two patches from Xiang Wang that are already applied to
>    Vinod's next branch
>  - add another patch to cleanup the unlocking path in lookup_phy(),
>    as suggested by Ezequiel
>  - provide an own xlate function rather than using
>    of_dma_simple_xlate(), as suggested by Arnd
> 
> 
> There's still an open topic with the DALGN flag, as Xiang Wang pointed
> out. According to him, there's a chance we need an alternative way of
> forcing the driver into byte-align mode, but that can still be done in
> an additional patch later.
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 00/11] dma: pdma: some patches needed for PXA DT environments

[Daniel Mack]

On 10.08.2013 23:25, Arnd Bergmann wrote:
> On Saturday 10 August 2013, Daniel Mack wrote:
>> This is v2 of my series to add more features to the pdma driver.
>>
>> Changes from v1 -> v2:
>>
>>  - drop the two patches from Xiang Wang that are already applied to
>>    Vinod's next branch
>>  - add another patch to cleanup the unlocking path in lookup_phy(),
>>    as suggested by Ezequiel
>>  - provide an own xlate function rather than using
>>    of_dma_simple_xlate(), as suggested by Arnd
>>
>>
>> There's still an open topic with the DALGN flag, as Xiang Wang pointed
>> out. According to him, there's a chance we need an alternative way of
>> forcing the driver into byte-align mode, but that can still be done in
>> an additional patch later.
>>
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 

Vinod, Dan, are you fine with this series as well?


Thanks,
Daniel

[PATCH v2 09/11] dma: mmp_pdma: implement DMA_PAUSE and DMA_RESUME

[Vinod Koul]

On Sat, Aug 10, 2013 at 06:52:23PM +0200, Daniel Mack wrote:
> This is needed at least for audio operation.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  drivers/dma/mmp_pdma.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 579f79a..7a0956b 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
> @@ -659,6 +659,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);
This is your usual start_pending ops. The meaning of pause and resume is NOT to
drop current descriptor and start from next one. You need to start from where
you had left off.

The way I see in this driver you are going to pick next descriptor and since
usage is audio, imply loss of audio in puase-resume case.

You need to resume here, not start next one

~Vinod

[PATCH v2 10/11] dma: mmp_pdma: add support for residue reporting

[Vinod Koul]

On Sat, Aug 10, 2013 at 06:52:24PM +0200, Daniel Mack wrote:
> 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 <zonque@gmail.com>
> ---
>  drivers/dma/mmp_pdma.c | 39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 7a0956b..ab0303b 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
> @@ -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;
well instead of rembering two values and doing two computations runtime, you can store
last_addr = start_addr + total_len;

then calculate
	residue = last_addr - dma_poistion;

Neverthless, this is okay too, so am going to apply this one..

~Vinod
>  
>  	/* channel's basic info */
>  	struct tasklet_struct tasklet;
> @@ -470,6 +477,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);
> @@ -541,11 +555,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)
> @@ -672,6 +692,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;
simpler logic:
store the 
> +}
> +
>  static enum dma_status mmp_pdma_tx_status(struct dma_chan *dchan,
>  			dma_cookie_t cookie, struct dma_tx_state *txstate)
>  {
> @@ -681,6 +715,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;
> -- 
> 1.8.3.1
> 

-- 

[PATCH v2 00/11] dma: pdma: some patches needed for PXA DT environments

[Vinod Koul]

On Sat, Aug 10, 2013 at 07:18:58PM +0200, Daniel Mack wrote:
> On 10.08.2013 18:52, Daniel Mack wrote:
> > This is v2 of my series to add more features to the pdma driver.
> 
> I forgot to mention that this series depends on a patch from Zhangfei
> Gao which provides dma_get_slave_channel(). It was posted here:
> 
>   http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077
Okay i saw this later, since I had kept that in branch had to merge it first
and then redo yours.

I have applied all till Pause, resume patch for which i have commented. The
patch after that failed to apply, perhpas due to dependency on this one. So you
can rebase those two and resend

~Vinod

[PATCH v2 09/11] dma: mmp_pdma: implement DMA_PAUSE and DMA_RESUME

[Vinod Koul]

On Wed, Aug 14, 2013 at 11:02:06AM +0200, Daniel Mack wrote:
> On 14.08.2013 10:15, Vinod Koul wrote:
> > On Sat, Aug 10, 2013 at 06:52:23PM +0200, Daniel Mack wrote:
> 
> Ah, ok. Thanks for the review. Can you just omit that one and take the
> rest of the patches? Pause and resume is something I can take care of in
> a separate patch then, and it should hold off the other changes IMO.
It shouldnt & it hasnt!

~Vinod

[PATCH v2 09/11] dma: mmp_pdma: implement DMA_PAUSE and DMA_RESUME

[Daniel Mack]

On 14.08.2013 10:15, Vinod Koul wrote:
> On Sat, Aug 10, 2013 at 06:52:23PM +0200, Daniel Mack wrote:
>> This is needed at least for audio operation.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>>  drivers/dma/mmp_pdma.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
>> index 579f79a..7a0956b 100644
>> --- a/drivers/dma/mmp_pdma.c
>> +++ b/drivers/dma/mmp_pdma.c
>> @@ -659,6 +659,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);
> This is your usual start_pending ops. The meaning of pause and resume is NOT to
> drop current descriptor and start from next one. You need to start from where
> you had left off.
> 
> The way I see in this driver you are going to pick next descriptor and since
> usage is audio, imply loss of audio in puase-resume case.
> 
> You need to resume here, not start next one

Ah, ok. Thanks for the review. Can you just omit that one and take the
rest of the patches? Pause and resume is something I can take care of in
a separate patch then, and it should hold off the other changes IMO.


Thanks,
Daniel