All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register
  2016-03-10 10:35 [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers Jean-Francois Moine
@ 2016-03-10  9:15 ` Jean-Francois Moine
  2016-03-11  6:39   ` Vinod Koul
  2016-03-11 10:56   ` Maxime Ripard
  2016-03-10  9:27 ` [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width Jean-Francois Moine
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-10  9:15 UTC (permalink / raw)
  To: linux-arm-kernel

The address of the IRQ register is computed, but its value was not
used and the register access was done by the channel index instead.
Then, only the first DMA channel was working.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 2db12e4..2ec320d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -381,9 +381,9 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 	irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
 	irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
 
-	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_offset));
+	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
 	irq_val |= DMA_IRQ_QUEUE << (irq_offset * DMA_IRQ_CHAN_WIDTH);
-	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_offset));
+	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
 
 	writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
 	writel(DMA_CHAN_ENABLE_START, pchan->base + DMA_CHAN_ENABLE);
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width
  2016-03-10 10:35 [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers Jean-Francois Moine
  2016-03-10  9:15 ` [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register Jean-Francois Moine
@ 2016-03-10  9:27 ` Jean-Francois Moine
  2016-03-11  6:42   ` Vinod Koul
  2016-03-11 10:58   ` Maxime Ripard
  2016-03-10  9:56 ` [PATCH 3/5] dmaengine: sun6i: Add 4 as a possible bust value Jean-Francois Moine
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-10  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

In the commit 1f9cd915b64bb95f "dmaengine: sun6i: Fix memcpy operation",
the signed values returned by convert_burst() and convert_buswidth()
were stored in an unsigned value.
Then, these values were considered as errors when non null.
As a result, setting burst or buswidth to values different from 1 was
making the DMA transfers to be rejected.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 2ec320d..3579ee7 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -281,25 +281,25 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
 				    dma_addr_t dst, u32 len,
 				    struct dma_slave_config *config)
 {
-	u8 src_width, dst_width, src_burst, dst_burst;
+	s8 src_width, dst_width, src_burst, dst_burst;
 
 	if (!config)
 		return -EINVAL;
 
 	src_burst = convert_burst(config->src_maxburst);
-	if (src_burst)
+	if (src_burst < 0)
 		return src_burst;
 
 	dst_burst = convert_burst(config->dst_maxburst);
-	if (dst_burst)
+	if (dst_burst < 0)
 		return dst_burst;
 
 	src_width = convert_buswidth(config->src_addr_width);
-	if (src_width)
+	if (src_width < 0)
 		return src_width;
 
 	dst_width = convert_buswidth(config->dst_addr_width);
-	if (dst_width)
+	if (dst_width < 0)
 		return dst_width;
 
 	lli->cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/5] dmaengine: sun6i: Add 4 as a possible bust value
  2016-03-10 10:35 [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers Jean-Francois Moine
  2016-03-10  9:15 ` [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register Jean-Francois Moine
  2016-03-10  9:27 ` [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width Jean-Francois Moine
@ 2016-03-10  9:56 ` Jean-Francois Moine
  2016-03-10 19:52   ` Sergei Shtylyov
  2016-03-10 10:07 ` [PATCH 4/5] dmaengine: sun6i: Set default values to burst and bus width Jean-Francois Moine
  2016-03-10 10:21 ` [PATCH 5/5] dmaengine: sun6i: Add cyclic capability Jean-Francois Moine
  4 siblings, 1 reply; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-10  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

Some DMA transfers, as for H3 audio, ask for 4 as a bust value.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 3579ee7..7c98c0d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
 	default:
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/5] dmaengine: sun6i: Set default values to burst and bus width
  2016-03-10 10:35 [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers Jean-Francois Moine
                   ` (2 preceding siblings ...)
  2016-03-10  9:56 ` [PATCH 3/5] dmaengine: sun6i: Add 4 as a possible bust value Jean-Francois Moine
@ 2016-03-10 10:07 ` Jean-Francois Moine
  2016-03-11  6:47   ` Vinod Koul
  2016-03-10 10:21 ` [PATCH 5/5] dmaengine: sun6i: Add cyclic capability Jean-Francois Moine
  4 siblings, 1 reply; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-10 10:07 UTC (permalink / raw)
  To: linux-arm-kernel

When defining the DMA transfer, the sound PCM DMA engine sets only
the burst and bus width values on the DMA side.
This was making the audio transfers to be rejected because of invalid
values on the memory side.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 7c98c0d..c27d572 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -288,19 +288,31 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
 	if (!config)
 		return -EINVAL;
 
-	src_burst = convert_burst(config->src_maxburst);
+	if (config->src_maxburst == 0)
+		src_burst = convert_burst(config->dst_maxburst);
+	else
+		src_burst = convert_burst(config->src_maxburst);
 	if (src_burst < 0)
 		return src_burst;
 
-	dst_burst = convert_burst(config->dst_maxburst);
+	if (config->dst_maxburst == 0)
+		dst_burst = convert_burst(config->src_maxburst);
+	else
+		dst_burst = convert_burst(config->dst_maxburst);
 	if (dst_burst < 0)
 		return dst_burst;
 
-	src_width = convert_buswidth(config->src_addr_width);
+	if (config->src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+		src_width = convert_buswidth(config->dst_addr_width);
+	else
+		src_width = convert_buswidth(config->src_addr_width);
 	if (src_width < 0)
 		return src_width;
 
-	dst_width = convert_buswidth(config->dst_addr_width);
+	if (config->dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+		dst_width = convert_buswidth(config->src_addr_width);
+	else
+		dst_width = convert_buswidth(config->dst_addr_width);
 	if (dst_width < 0)
 		return dst_width;
 
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/5] dmaengine: sun6i: Add cyclic capability
  2016-03-10 10:35 [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers Jean-Francois Moine
                   ` (3 preceding siblings ...)
  2016-03-10 10:07 ` [PATCH 4/5] dmaengine: sun6i: Set default values to burst and bus width Jean-Francois Moine
@ 2016-03-10 10:21 ` Jean-Francois Moine
  2016-03-11  7:32   ` Vinod Koul
  4 siblings, 1 reply; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-10 10:21 UTC (permalink / raw)
  To: linux-arm-kernel

DMA cyclic transfers are required by audio streaming.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 144 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 136 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c27d572..06d0306 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -146,6 +146,8 @@ struct sun6i_vchan {
 	struct dma_slave_config	cfg;
 	struct sun6i_pchan	*phy;
 	u8			port;
+	u8			irq_type;
+	bool			cyclic;
 };
 
 struct sun6i_dma_dev {
@@ -256,6 +258,30 @@ static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 	return addr_width >> 1;
 }
 
+static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
+{
+	struct sun6i_desc *txd = pchan->desc;
+	struct sun6i_dma_lli *lli;
+	size_t bytes;
+	dma_addr_t pos;
+
+	pos = readl(pchan->base + DMA_CHAN_LLI_ADDR);
+	bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
+
+	if (pos == LLI_LAST_ITEM)
+		return bytes;
+
+	for (lli = txd->v_lli; lli; lli = lli->v_lli_next) {
+		if (lli->p_lli_next == pos) {
+			for (lli = lli->v_lli_next; lli; lli = lli->v_lli_next)
+				bytes += lli->len;
+			break;
+		}
+	}
+
+	return bytes;
+}
+
 static void *sun6i_dma_lli_add(struct sun6i_dma_lli *prev,
 			       struct sun6i_dma_lli *next,
 			       dma_addr_t next_phy,
@@ -395,8 +421,12 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 	irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
 	irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
 
+	vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
+
 	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
-	irq_val |= DMA_IRQ_QUEUE << (irq_offset * DMA_IRQ_CHAN_WIDTH);
+	irq_val &= ~((DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
+			(irq_offset * DMA_IRQ_CHAN_WIDTH));
+	irq_val |= vchan->irq_type << (irq_offset * DMA_IRQ_CHAN_WIDTH);
 	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
 
 	writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
@@ -493,11 +523,12 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 		writel(status, sdev->base + DMA_IRQ_STAT(i));
 
 		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
-			if (status & DMA_IRQ_QUEUE) {
-				pchan = sdev->pchans + j;
-				vchan = pchan->vchan;
-
-				if (vchan) {
+			pchan = sdev->pchans + j;
+			vchan = pchan->vchan;
+			if (vchan && (status & vchan->irq_type)) {
+				if (vchan->cyclic) {
+					vchan_cyclic_callback(&pchan->desc->vd);
+				} else {
 					spin_lock(&vchan->vc.lock);
 					vchan_cookie_complete(&pchan->desc->vd);
 					pchan->done = pchan->desc;
@@ -657,6 +688,76 @@ err_lli_free:
 	return NULL;
 }
 
+static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
+					struct dma_chan *chan,
+					dma_addr_t buf_addr,
+					size_t buf_len,
+					size_t period_len,
+					enum dma_transfer_direction dir,
+					unsigned long flags)
+{
+	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
+	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
+	struct dma_slave_config *sconfig = &vchan->cfg;
+	struct sun6i_dma_lli *v_lli, *prev = NULL;
+	struct sun6i_desc *txd;
+	dma_addr_t p_lli;
+	unsigned int i, periods = buf_len / period_len;
+	int ret;
+
+	if (vchan->cyclic && vchan->phy && vchan->phy->desc)
+		return NULL;
+
+	txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
+	if (!txd)
+		return NULL;
+
+	for (i = 0; i < periods; i++){
+		v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
+		if (!v_lli) {
+			dev_err(sdev->slave.dev, "Failed to alloc lli memory\n");
+			goto err_lli_free;
+		}
+		if (dir == DMA_MEM_TO_DEV) {
+			ret = sun6i_dma_cfg_lli(v_lli,
+						buf_addr + period_len * i,
+						sconfig->dst_addr,
+						period_len, sconfig);
+			v_lli->cfg |= DMA_CHAN_CFG_DST_IO_MODE |
+					DMA_CHAN_CFG_SRC_LINEAR_MODE |
+					DMA_CHAN_CFG_SRC_DRQ(DRQ_SDRAM) |
+					DMA_CHAN_CFG_DST_DRQ(vchan->port);
+		} else {
+			ret = sun6i_dma_cfg_lli(v_lli,
+						sconfig->src_addr,
+						buf_addr + period_len * i,
+						period_len, sconfig);
+			v_lli->cfg |= DMA_CHAN_CFG_DST_LINEAR_MODE |
+					DMA_CHAN_CFG_SRC_IO_MODE |
+					DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
+					DMA_CHAN_CFG_SRC_DRQ(vchan->port);
+		}
+		if (ret < 0)
+			goto err_cur_lli_free;
+
+		prev = sun6i_dma_lli_add(prev, v_lli, p_lli, txd);
+	}
+
+	prev->p_lli_next = txd->p_lli;		/* cyclic list */
+
+	vchan->cyclic = true;
+
+	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
+
+err_cur_lli_free:
+	dma_pool_free(sdev->pool, v_lli, p_lli);
+err_lli_free:
+	for (prev = txd->v_lli; prev; prev = prev->v_lli_next)
+		dma_pool_free(sdev->pool, prev, virt_to_phys(prev));
+	kfree(txd);
+	return NULL;
+}
+
 static int sun6i_dma_config(struct dma_chan *chan,
 			    struct dma_slave_config *config)
 {
@@ -726,6 +827,16 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
 
 	spin_lock_irqsave(&vchan->vc.lock, flags);
 
+	if (vchan->cyclic) {
+		vchan->cyclic = false;
+		if (pchan && pchan->desc) {
+			struct virt_dma_desc *vd = &pchan->desc->vd;
+			struct virt_dma_chan *vc = &vchan->vc;
+
+			list_add_tail(&vd->node, &vc->desc_completed);
+		}
+	}
+
 	vchan_get_all_descriptors(&vchan->vc, &head);
 
 	if (pchan) {
@@ -773,7 +884,7 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
 	} else if (!pchan || !pchan->desc) {
 		bytes = 0;
 	} else {
-		bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
+		bytes = sun6i_get_chan_size(pchan);
 	}
 
 	spin_unlock_irqrestore(&vchan->vc.lock, flags);
@@ -792,6 +903,11 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
 	spin_lock_irqsave(&vchan->vc.lock, flags);
 
 	if (vchan_issue_pending(&vchan->vc)) {
+		if (vchan->phy && vchan->cyclic) {
+			sun6i_dma_start_desc(vchan);
+			goto out;
+		}
+
 		spin_lock(&sdev->lock);
 
 		if (!vchan->phy && list_empty(&vchan->node)) {
@@ -806,10 +922,19 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
 		dev_dbg(chan2dev(chan), "vchan %p: nothing to issue\n",
 			&vchan->vc);
 	}
-
+out:
 	spin_unlock_irqrestore(&vchan->vc.lock, flags);
 }
 
+static int sun6i_dma_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
+
+	vchan->cyclic = false;
+
+	return 0;
+}
+
 static void sun6i_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
@@ -977,13 +1102,16 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	dma_cap_set(DMA_PRIVATE, sdc->slave.cap_mask);
 	dma_cap_set(DMA_MEMCPY, sdc->slave.cap_mask);
 	dma_cap_set(DMA_SLAVE, sdc->slave.cap_mask);
+	dma_cap_set(DMA_CYCLIC, sdc->slave.cap_mask);
 
 	INIT_LIST_HEAD(&sdc->slave.channels);
+	sdc->slave.device_alloc_chan_resources	= sun6i_dma_alloc_chan_resources;
 	sdc->slave.device_free_chan_resources	= sun6i_dma_free_chan_resources;
 	sdc->slave.device_tx_status		= sun6i_dma_tx_status;
 	sdc->slave.device_issue_pending		= sun6i_dma_issue_pending;
 	sdc->slave.device_prep_slave_sg		= sun6i_dma_prep_slave_sg;
 	sdc->slave.device_prep_dma_memcpy	= sun6i_dma_prep_dma_memcpy;
+	sdc->slave.device_prep_dma_cyclic	= sun6i_dma_prep_dma_cyclic;
 	sdc->slave.copy_align			= DMAENGINE_ALIGN_4_BYTES;
 	sdc->slave.device_config		= sun6i_dma_config;
 	sdc->slave.device_pause			= sun6i_dma_pause;
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers
@ 2016-03-10 10:35 Jean-Francois Moine
  2016-03-10  9:15 ` [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register Jean-Francois Moine
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-10 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patch series contains the required fixes and changes to the sun6i
DMA driver for audio streaming.
These ones have been tested in a Allwinner H3 (Orange PI 2).

Jean-Francois Moine (5):
  dmaengine: sun6i: Fix the access of the IRQ register
  dmaengine: sun6i: Fix impossible settings of burst and bus width
  dmaengine: sun6i: Add 4 as a possible bust value
  dmaengine: sun6i: Set default values to burst and bus width
  dmaengine: sun6i: Add cyclic capability

 drivers/dma/sun6i-dma.c | 180 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 161 insertions(+), 19 deletions(-)

-- 
2.7.2

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/5] dmaengine: sun6i: Add 4 as a possible bust value
  2016-03-10  9:56 ` [PATCH 3/5] dmaengine: sun6i: Add 4 as a possible bust value Jean-Francois Moine
@ 2016-03-10 19:52   ` Sergei Shtylyov
  0 siblings, 0 replies; 15+ messages in thread
From: Sergei Shtylyov @ 2016-03-10 19:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 03/10/2016 12:56 PM, Jean-Francois Moine wrote:

> Some DMA transfers, as for H3 audio, ask for 4 as a bust value.

    Burst? :-)

> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register
  2016-03-10  9:15 ` [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register Jean-Francois Moine
@ 2016-03-11  6:39   ` Vinod Koul
  2016-03-11  7:16     ` Jean-Francois Moine
  2016-03-11 10:56   ` Maxime Ripard
  1 sibling, 1 reply; 15+ messages in thread
From: Vinod Koul @ 2016-03-11  6:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 10, 2016 at 10:15:22AM +0100, Jean-Francois Moine wrote:
> The address of the IRQ register is computed, but its value was not
						^^^^^^^
it's

> used and the register access was done by the channel index instead.
> Then, only the first DMA channel was working.
-- 
~Vinod

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width
  2016-03-10  9:27 ` [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width Jean-Francois Moine
@ 2016-03-11  6:42   ` Vinod Koul
  2016-03-11  7:20     ` Jean-Francois Moine
  2016-03-11 10:58   ` Maxime Ripard
  1 sibling, 1 reply; 15+ messages in thread
From: Vinod Koul @ 2016-03-11  6:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 10, 2016 at 10:27:37AM +0100, Jean-Francois Moine wrote:
> In the commit 1f9cd915b64bb95f "dmaengine: sun6i: Fix memcpy operation",
> the signed values returned by convert_burst() and convert_buswidth()
> were stored in an unsigned value.
> Then, these values were considered as errors when non null.
> As a result, setting burst or buswidth to values different from 1 was
> making the DMA transfers to be rejected.

A value of 0 for burst does not make sense. Frr buswidth this in undefined.
So what are we gaining from 0 value and how is it fixing something which is
passed as wrong argument?


> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  drivers/dma/sun6i-dma.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 2ec320d..3579ee7 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -281,25 +281,25 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
>  				    dma_addr_t dst, u32 len,
>  				    struct dma_slave_config *config)
>  {
> -	u8 src_width, dst_width, src_burst, dst_burst;
> +	s8 src_width, dst_width, src_burst, dst_burst;
>  
>  	if (!config)
>  		return -EINVAL;
>  
>  	src_burst = convert_burst(config->src_maxburst);
> -	if (src_burst)
> +	if (src_burst < 0)
>  		return src_burst;
>  
>  	dst_burst = convert_burst(config->dst_maxburst);
> -	if (dst_burst)
> +	if (dst_burst < 0)
>  		return dst_burst;
>  
>  	src_width = convert_buswidth(config->src_addr_width);
> -	if (src_width)
> +	if (src_width < 0)
>  		return src_width;
>  
>  	dst_width = convert_buswidth(config->dst_addr_width);
> -	if (dst_width)
> +	if (dst_width < 0)
>  		return dst_width;
>  
>  	lli->cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> -- 
> 2.7.2
> 

-- 
~Vinod

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/5] dmaengine: sun6i: Set default values to burst and bus width
  2016-03-10 10:07 ` [PATCH 4/5] dmaengine: sun6i: Set default values to burst and bus width Jean-Francois Moine
@ 2016-03-11  6:47   ` Vinod Koul
  0 siblings, 0 replies; 15+ messages in thread
From: Vinod Koul @ 2016-03-11  6:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 10, 2016 at 11:07:53AM +0100, Jean-Francois Moine wrote:
> When defining the DMA transfer, the sound PCM DMA engine sets only
> the burst and bus width values on the DMA side.
> This was making the audio transfers to be rejected because of invalid
> values on the memory side.

I think there is a misunderstanding of the API. The slave configuration set
thru dma_slave_config is only intended for peripheral and not for memory.

In memory case you need to assume values based on system. DMA to memory is
usually done for max throughput and these parameters do not make sense
there.


> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  drivers/dma/sun6i-dma.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 7c98c0d..c27d572 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -288,19 +288,31 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
>  	if (!config)
>  		return -EINVAL;
>  
> -	src_burst = convert_burst(config->src_maxburst);
> +	if (config->src_maxburst == 0)
> +		src_burst = convert_burst(config->dst_maxburst);
> +	else
> +		src_burst = convert_burst(config->src_maxburst);

No 0 is error for the transfer direction and don't assume default for
peripheral, that is just wrong

-- 
~Vinod

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register
  2016-03-11  6:39   ` Vinod Koul
@ 2016-03-11  7:16     ` Jean-Francois Moine
  0 siblings, 0 replies; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-11  7:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 11 Mar 2016 12:09:56 +0530
Vinod Koul <vinod.koul@intel.com> wrote:

> On Thu, Mar 10, 2016 at 10:15:22AM +0100, Jean-Francois Moine wrote:
> > The address of the IRQ register is computed, but its value was not
> 						^^^^^^^
> it's
> 
> > used and the register access was done by the channel index instead.
> > Then, only the first DMA channel was working.
> -- 

No. 'its value' is 'the value of the address'. I will re-phrase.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width
  2016-03-11  6:42   ` Vinod Koul
@ 2016-03-11  7:20     ` Jean-Francois Moine
  0 siblings, 0 replies; 15+ messages in thread
From: Jean-Francois Moine @ 2016-03-11  7:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 11 Mar 2016 12:12:27 +0530
Vinod Koul <vinod.koul@intel.com> wrote:

> On Thu, Mar 10, 2016 at 10:27:37AM +0100, Jean-Francois Moine wrote:
> > In the commit 1f9cd915b64bb95f "dmaengine: sun6i: Fix memcpy operation",
> > the signed values returned by convert_burst() and convert_buswidth()
> > were stored in an unsigned value.
> > Then, these values were considered as errors when non null.
> > As a result, setting burst or buswidth to values different from 1 was
> > making the DMA transfers to be rejected.
> 
> A value of 0 for burst does not make sense. Frr buswidth this in undefined.
> So what are we gaining from 0 value and how is it fixing something which is
> passed as wrong argument?

The problem is not the null value, but a value different from 1:
8 as the burst or 4 as the bus width were rejected.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5/5] dmaengine: sun6i: Add cyclic capability
  2016-03-10 10:21 ` [PATCH 5/5] dmaengine: sun6i: Add cyclic capability Jean-Francois Moine
@ 2016-03-11  7:32   ` Vinod Koul
  0 siblings, 0 replies; 15+ messages in thread
From: Vinod Koul @ 2016-03-11  7:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 10, 2016 at 11:21:34AM +0100, Jean-Francois Moine wrote:

> +
> +	if (vchan->cyclic && vchan->phy && vchan->phy->desc)
> +		return NULL;

why check for vchan->cyclic? you are setting that at the end!

> @@ -792,6 +903,11 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
>  	spin_lock_irqsave(&vchan->vc.lock, flags);
>  
>  	if (vchan_issue_pending(&vchan->vc)) {
> +		if (vchan->phy && vchan->cyclic) {
> +			sun6i_dma_start_desc(vchan);
> +			goto out;
> +		}

Why should this be cyclic specfic, Channel start should be same for all
channels?

> +static int sun6i_dma_alloc_chan_resources(struct dma_chan *chan)
> +{
> +	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
> +
> +	vchan->cyclic = false;

why do you need to set this here?

-- 
~Vinod

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register
  2016-03-10  9:15 ` [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register Jean-Francois Moine
  2016-03-11  6:39   ` Vinod Koul
@ 2016-03-11 10:56   ` Maxime Ripard
  1 sibling, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2016-03-11 10:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 10, 2016 at 10:15:22AM +0100, Jean-Francois Moine wrote:
> The address of the IRQ register is computed, but its value was not
> used and the register access was done by the channel index instead.
> Then, only the first DMA channel was working.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160311/df8a2c8b/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width
  2016-03-10  9:27 ` [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width Jean-Francois Moine
  2016-03-11  6:42   ` Vinod Koul
@ 2016-03-11 10:58   ` Maxime Ripard
  1 sibling, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2016-03-11 10:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 10, 2016 at 10:27:37AM +0100, Jean-Francois Moine wrote:
> In the commit 1f9cd915b64bb95f "dmaengine: sun6i: Fix memcpy operation",

The commit title should be enclosed in parenthesis.

> the signed values returned by convert_burst() and convert_buswidth()
> were stored in an unsigned value.
> Then, these values were considered as errors when non null.
> As a result, setting burst or buswidth to values different from 1 was
> making the DMA transfers to be rejected.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

Otherwise, Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160311/98f60af3/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-03-11 10:58 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-10 10:35 [PATCH 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers Jean-Francois Moine
2016-03-10  9:15 ` [PATCH 1/5] dmaengine: sun6i: Fix the access of the IRQ register Jean-Francois Moine
2016-03-11  6:39   ` Vinod Koul
2016-03-11  7:16     ` Jean-Francois Moine
2016-03-11 10:56   ` Maxime Ripard
2016-03-10  9:27 ` [PATCH 2/5] dmaengine: sun6i: Fix impossible settings of burst and bus width Jean-Francois Moine
2016-03-11  6:42   ` Vinod Koul
2016-03-11  7:20     ` Jean-Francois Moine
2016-03-11 10:58   ` Maxime Ripard
2016-03-10  9:56 ` [PATCH 3/5] dmaengine: sun6i: Add 4 as a possible bust value Jean-Francois Moine
2016-03-10 19:52   ` Sergei Shtylyov
2016-03-10 10:07 ` [PATCH 4/5] dmaengine: sun6i: Set default values to burst and bus width Jean-Francois Moine
2016-03-11  6:47   ` Vinod Koul
2016-03-10 10:21 ` [PATCH 5/5] dmaengine: sun6i: Add cyclic capability Jean-Francois Moine
2016-03-11  7:32   ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.