[PATCH v2 0/6] dma: various fixes in different DMAC drivers

[PATCH v2 0/6] dma: various fixes in different DMAC drivers

[Andy Shevchenko]

This is a patcset of independent fixes of the DMAC drivers. They are simple and
being understandable in their context.

Since v1:
 - address all comments
 - add ACKs
 - rebase on top of recent Vinod's next branch

Andy Shevchenko (6):
  dma: of-dma: protect list write operation by spin_lock
  dmaengine.h: remove redundant else keyword
  dma: coh901318: avoid unbalanced locking
  dma: coh901318: set residue only if dma is in progress
  edma: do not waste memory for dma_mask
  dma: tegra20-apb-dma: remove unnecessary assignment

 drivers/dma/coh901318.c       |    4 +++-
 drivers/dma/coh901318_lli.c   |    2 +-
 drivers/dma/edma.c            |    6 ++++--
 drivers/dma/of-dma.c          |    2 ++
 drivers/dma/tegra20-apb-dma.c |    1 -
 include/linux/dmaengine.h     |    4 ++--
 6 files changed, 12 insertions(+), 7 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/6] dma: of-dma: protect list write operation by spin_lock

[Andy Shevchenko]

It's possible to have an inconsistency in the list due to unprotected operation
on it. The patch adds a proper locking on the list operation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/dma/of-dma.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 59631b2..1ae6120 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -117,7 +117,9 @@ int of_dma_controller_register(struct device_node *np,
 	ofdma->use_count = 0;
 
 	/* Now queue of_dma controller structure in list */
+	spin_lock(&of_dma_lock);
 	list_add_tail(&ofdma->of_dma_controllers, &of_dma_list);
+	spin_unlock(&of_dma_lock);
 
 	return 0;
 }
-- 
1.7.10.4

[PATCH v2 2/6] dmaengine.h: remove redundant else keyword

[Andy Shevchenko]

dmaengine_device_control returns -ENOSYS in case the dma driver doesn't have
such functionality.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/dmaengine.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index bfcdecb..f593999 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -610,8 +610,8 @@ static inline int dmaengine_device_control(struct dma_chan *chan,
 {
 	if (chan->device->device_control)
 		return chan->device->device_control(chan, cmd, arg);
-	else
-		return -ENOSYS;
+
+	return -ENOSYS;
 }
 
 static inline int dmaengine_slave_config(struct dma_chan *chan,
-- 
1.7.10.4

[PATCH v2 3/6] dma: coh901318: avoid unbalanced locking

[Andy Shevchenko]

In case the len is 0 we must return without trying to unlock the lock that was
not locked.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/dma/coh901318_lli.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/coh901318_lli.c b/drivers/dma/coh901318_lli.c
index 780e042..fe9cc5e 100644
--- a/drivers/dma/coh901318_lli.c
+++ b/drivers/dma/coh901318_lli.c
@@ -61,7 +61,7 @@ coh901318_lli_alloc(struct coh901318_pool *pool, unsigned int len)
 	dma_addr_t phy;
 
 	if (len == 0)
-		goto err;
+		return NULL;
 
 	spin_lock(&pool->lock);
 
-- 
1.7.10.4

[PATCH v2 4/6] dma: coh901318: set residue only if dma is in progress

[Andy Shevchenko]

When status is DMA_SUCCESS the residue should be zero. Otherwise it's a bug.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/dma/coh901318.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
index aa384e5..671e759 100644
--- a/drivers/dma/coh901318.c
+++ b/drivers/dma/coh901318.c
@@ -1147,7 +1147,9 @@ coh901318_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 	enum dma_status ret;
 
 	ret = dma_cookie_status(chan, cookie, txstate);
-	/* FIXME: should be conditional on ret != DMA_SUCCESS? */
+	if (ret == DMA_SUCCESS)
+		return ret;
+
 	dma_set_residue(txstate, coh901318_get_bytes_left(chan));
 
 	if (ret == DMA_IN_PROGRESS && cohc->stopped)
-- 
1.7.10.4

[PATCH v2 5/6] edma: do not waste memory for dma_mask

[Andy Shevchenko]

Accordingly to commentary in the platform_device_register_full the memory
allocated for dma_mask will not going to be freed. That's why is better to
assign dma_mask afterwards.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/dma/edma.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 82c8672..af44cad 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -620,13 +620,11 @@ static struct platform_device *pdev0, *pdev1;
 static const struct platform_device_info edma_dev_info0 = {
 	.name = "edma-dma-engine",
 	.id = 0,
-	.dma_mask = DMA_BIT_MASK(32),
 };
 
 static const struct platform_device_info edma_dev_info1 = {
 	.name = "edma-dma-engine",
 	.id = 1,
-	.dma_mask = DMA_BIT_MASK(32),
 };
 
 static int edma_init(void)
@@ -640,6 +638,8 @@ static int edma_init(void)
 			ret = PTR_ERR(pdev0);
 			goto out;
 		}
+		pdev0->dev.dma_mask = &pdev0->dev.coherent_dma_mask;
+		pdev0->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	}
 
 	if (EDMA_CTLRS == 2) {
@@ -649,6 +649,8 @@ static int edma_init(void)
 			platform_device_unregister(pdev0);
 			ret = PTR_ERR(pdev1);
 		}
+		pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
+		pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	}
 
 out:
-- 
1.7.10.4

[PATCH v2 6/6] dma: tegra20-apb-dma: remove unnecessary assignment

[Andy Shevchenko]

There is no need to assign 0 to residue, because dma_cookie_status() does this
for us.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/dma/tegra20-apb-dma.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 6c14481..c71812f 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -767,7 +767,6 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc,
 
 	ret = dma_cookie_status(dc, cookie, txstate);
 	if (ret == DMA_SUCCESS) {
-		dma_set_residue(txstate, 0);
 		spin_unlock_irqrestore(&tdc->lock, flags);
 		return ret;
 	}
-- 
1.7.10.4

Re: [PATCH v2 0/6] dma: various fixes in different DMAC drivers

[Vinod Koul]

On Thu, Feb 14, 2013 at 11:00:14AM +0200, Andy Shevchenko wrote:
> This is a patcset of independent fixes of the DMAC drivers. They are simple and
> being understandable in their context.
all applied, thanks

--
~Vinod
> 
> Since v1:
>  - address all comments
>  - add ACKs
>  - rebase on top of recent Vinod's next branch
> 
> Andy Shevchenko (6):
>   dma: of-dma: protect list write operation by spin_lock
>   dmaengine.h: remove redundant else keyword
>   dma: coh901318: avoid unbalanced locking
>   dma: coh901318: set residue only if dma is in progress
>   edma: do not waste memory for dma_mask
>   dma: tegra20-apb-dma: remove unnecessary assignment
> 
>  drivers/dma/coh901318.c       |    4 +++-
>  drivers/dma/coh901318_lli.c   |    2 +-
>  drivers/dma/edma.c            |    6 ++++--
>  drivers/dma/of-dma.c          |    2 ++
>  drivers/dma/tegra20-apb-dma.c |    1 -
>  include/linux/dmaengine.h     |    4 ++--
>  6 files changed, 12 insertions(+), 7 deletions(-)
> 
> -- 
> 1.7.10.4
>