[PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver
@ 2014-07-30  8:30 ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan, Vinod,

Here are the patches that you requested previously, plus an additional
fix for a memory leak was spotted by Dan Carpenter.

Thanks,
Maxime

Maxime Ripard (3):
  dmaengine: sun6i: Remove switch statement from buswidth convertion
    routine
  dmaengine: sun6i: Free the interrupt before killing the tasklet
  dmaengine: sun6i: Fix memory leaks

 drivers/dma/sun6i-dma.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

-- 
2.0.2

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

* [PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver
@ 2014-07-30  8:30 ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: Dan Williams, Vinod Koul
  Cc: linux-kernel, linux-arm-kernel, dmaengine, Maxime Ripard

Hi Dan, Vinod,

Here are the patches that you requested previously, plus an additional
fix for a memory leak was spotted by Dan Carpenter.

Thanks,
Maxime

Maxime Ripard (3):
  dmaengine: sun6i: Remove switch statement from buswidth convertion
    routine
  dmaengine: sun6i: Free the interrupt before killing the tasklet
  dmaengine: sun6i: Fix memory leaks

 drivers/dma/sun6i-dma.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

-- 
2.0.2


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

* [PATCH 1/3] dmaengine: sun6i: Remove switch statement from buswidth convertion routine
  2014-07-30  8:30 ` Maxime Ripard
@ 2014-07-30  8:30   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

Since the conversion routine is quite trivial, we don't need this switch, and
we can just use a simple calculation.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/dma/sun6i-dma.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c771d90b8ded..609c5d8cb947 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -248,20 +248,11 @@ static inline int convert_burst(u32 maxburst, u8 *burst)
 
 static inline int convert_buswidth(enum dma_slave_buswidth addr_width, u8 *width)
 {
-	switch (addr_width) {
-	case DMA_SLAVE_BUSWIDTH_1_BYTE:
-		*width = 0;
-		break;
-	case DMA_SLAVE_BUSWIDTH_2_BYTES:
-		*width = 1;
-		break;
-	case DMA_SLAVE_BUSWIDTH_4_BYTES:
-		*width = 2;
-		break;
-	default:
+	if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
+	    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
 		return -EINVAL;
-	}
 
+	*width = addr_width >> 1;
 	return 0;
 }
 
-- 
2.0.2

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

* [PATCH 1/3] dmaengine: sun6i: Remove switch statement from buswidth convertion routine
@ 2014-07-30  8:30   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: Dan Williams, Vinod Koul
  Cc: linux-kernel, linux-arm-kernel, dmaengine, Maxime Ripard

Since the conversion routine is quite trivial, we don't need this switch, and
we can just use a simple calculation.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/dma/sun6i-dma.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c771d90b8ded..609c5d8cb947 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -248,20 +248,11 @@ static inline int convert_burst(u32 maxburst, u8 *burst)
 
 static inline int convert_buswidth(enum dma_slave_buswidth addr_width, u8 *width)
 {
-	switch (addr_width) {
-	case DMA_SLAVE_BUSWIDTH_1_BYTE:
-		*width = 0;
-		break;
-	case DMA_SLAVE_BUSWIDTH_2_BYTES:
-		*width = 1;
-		break;
-	case DMA_SLAVE_BUSWIDTH_4_BYTES:
-		*width = 2;
-		break;
-	default:
+	if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
+	    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
 		return -EINVAL;
-	}
 
+	*width = addr_width >> 1;
 	return 0;
 }
 
-- 
2.0.2


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

* [PATCH 2/3] dmaengine: sun6i: Free the interrupt before killing the tasklet
  2014-07-30  8:30 ` Maxime Ripard
@ 2014-07-30  8:30   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

There's still a small window between the call to sun6i_kill_tasklet and the end
of the driver remove function where a spurious interrupt might trigger, and
start using deallocated resources.

Replace the call to synchronize_irq by a free_irq, so that we're sure that we
won't get any further interrupts when we're deallocating resources.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 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 609c5d8cb947..63a1db38894e 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -836,8 +836,8 @@ static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
 	/* Prevent spurious interrupts from scheduling the tasklet */
 	atomic_inc(&sdev->tasklet_shutdown);
 
-	/* Make sure all interrupts are handled */
-	synchronize_irq(sdev->irq);
+	/* Make sure we won't have any further interrupts */
+	devm_free_irq(sdev->slave.dev, sdev->irq, sdev);
 
 	/* Actually prevent the tasklet from being scheduled */
 	tasklet_kill(&sdev->task);
-- 
2.0.2

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

* [PATCH 2/3] dmaengine: sun6i: Free the interrupt before killing the tasklet
@ 2014-07-30  8:30   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: Dan Williams, Vinod Koul
  Cc: linux-kernel, linux-arm-kernel, dmaengine, Maxime Ripard

There's still a small window between the call to sun6i_kill_tasklet and the end
of the driver remove function where a spurious interrupt might trigger, and
start using deallocated resources.

Replace the call to synchronize_irq by a free_irq, so that we're sure that we
won't get any further interrupts when we're deallocating resources.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 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 609c5d8cb947..63a1db38894e 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -836,8 +836,8 @@ static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
 	/* Prevent spurious interrupts from scheduling the tasklet */
 	atomic_inc(&sdev->tasklet_shutdown);
 
-	/* Make sure all interrupts are handled */
-	synchronize_irq(sdev->irq);
+	/* Make sure we won't have any further interrupts */
+	devm_free_irq(sdev->slave.dev, sdev->irq, sdev);
 
 	/* Actually prevent the tasklet from being scheduled */
 	tasklet_kill(&sdev->task);
-- 
2.0.2


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

* [PATCH 3/3] dmaengine: sun6i: Fix memory leaks
  2014-07-30  8:30 ` Maxime Ripard
@ 2014-07-30  8:30   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

The sun6i_dma_prep_memcpy and sun6i_dma_prep_slave_sg functions were both
leaking the descriptor they allocated if an  error was happening after a
successful dma_pool_alloc call.

It also fixes a memleak that was happening in the scatter gather list
traversal, that was allocating as much descriptor as there was scatter gather
items, but only freeing the current descriptor if an error was to arise.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/dma/sun6i-dma.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 63a1db38894e..1f92a56fd2b6 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -562,8 +562,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 	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");
-		kfree(txd);
-		return NULL;
+		goto err_txd_free;
 	}
 
 	ret = sun6i_dma_cfg_lli(v_lli, src, dest, len, sconfig);
@@ -583,6 +582,8 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 
 err_dma_free:
 	dma_pool_free(sdev->pool, v_lli, p_lli);
+err_txd_free:
+	kfree(txd);
 	return NULL;
 }
 
@@ -614,17 +615,15 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 
 	for_each_sg(sgl, sg, sg_len, i) {
 		v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
-		if (!v_lli) {
-			kfree(txd);
-			return NULL;
-		}
+		if (!v_lli)
+			goto err_lli_free;
 
 		if (dir == DMA_MEM_TO_DEV) {
 			ret = sun6i_dma_cfg_lli(v_lli, sg_dma_address(sg),
 						sconfig->dst_addr, sg_dma_len(sg),
 						sconfig);
 			if (ret)
-				goto err_dma_free;
+				goto err_cur_lli_free;
 
 			v_lli->cfg |= DMA_CHAN_CFG_DST_IO_MODE |
 				DMA_CHAN_CFG_SRC_LINEAR_MODE |
@@ -642,7 +641,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 						sg_dma_address(sg), sg_dma_len(sg),
 						sconfig);
 			if (ret)
-				goto err_dma_free;
+				goto err_cur_lli_free;
 
 			v_lli->cfg |= DMA_CHAN_CFG_DST_LINEAR_MODE |
 				DMA_CHAN_CFG_SRC_IO_MODE |
@@ -665,8 +664,12 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 
 	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
 
-err_dma_free:
+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;
 }
 
-- 
2.0.2

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

* [PATCH 3/3] dmaengine: sun6i: Fix memory leaks
@ 2014-07-30  8:30   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2014-07-30  8:30 UTC (permalink / raw)
  To: Dan Williams, Vinod Koul
  Cc: linux-kernel, linux-arm-kernel, dmaengine, Maxime Ripard

The sun6i_dma_prep_memcpy and sun6i_dma_prep_slave_sg functions were both
leaking the descriptor they allocated if an  error was happening after a
successful dma_pool_alloc call.

It also fixes a memleak that was happening in the scatter gather list
traversal, that was allocating as much descriptor as there was scatter gather
items, but only freeing the current descriptor if an error was to arise.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/dma/sun6i-dma.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 63a1db38894e..1f92a56fd2b6 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -562,8 +562,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 	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");
-		kfree(txd);
-		return NULL;
+		goto err_txd_free;
 	}
 
 	ret = sun6i_dma_cfg_lli(v_lli, src, dest, len, sconfig);
@@ -583,6 +582,8 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 
 err_dma_free:
 	dma_pool_free(sdev->pool, v_lli, p_lli);
+err_txd_free:
+	kfree(txd);
 	return NULL;
 }
 
@@ -614,17 +615,15 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 
 	for_each_sg(sgl, sg, sg_len, i) {
 		v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
-		if (!v_lli) {
-			kfree(txd);
-			return NULL;
-		}
+		if (!v_lli)
+			goto err_lli_free;
 
 		if (dir == DMA_MEM_TO_DEV) {
 			ret = sun6i_dma_cfg_lli(v_lli, sg_dma_address(sg),
 						sconfig->dst_addr, sg_dma_len(sg),
 						sconfig);
 			if (ret)
-				goto err_dma_free;
+				goto err_cur_lli_free;
 
 			v_lli->cfg |= DMA_CHAN_CFG_DST_IO_MODE |
 				DMA_CHAN_CFG_SRC_LINEAR_MODE |
@@ -642,7 +641,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 						sg_dma_address(sg), sg_dma_len(sg),
 						sconfig);
 			if (ret)
-				goto err_dma_free;
+				goto err_cur_lli_free;
 
 			v_lli->cfg |= DMA_CHAN_CFG_DST_LINEAR_MODE |
 				DMA_CHAN_CFG_SRC_IO_MODE |
@@ -665,8 +664,12 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 
 	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
 
-err_dma_free:
+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;
 }
 
-- 
2.0.2


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

* [PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver
  2014-07-30  8:30 ` Maxime Ripard
@ 2014-07-30 13:44   ` Vinod Koul
  -1 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2014-07-30 13:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 30, 2014 at 10:30:20AM +0200, Maxime Ripard wrote:
> Hi Dan, Vinod,
> 
> Here are the patches that you requested previously, plus an additional
> fix for a memory leak was spotted by Dan Carpenter.

Applied, thanks

-- 
~Vinod
> 
> Thanks,
> Maxime
> 
> Maxime Ripard (3):
>   dmaengine: sun6i: Remove switch statement from buswidth convertion
>     routine
>   dmaengine: sun6i: Free the interrupt before killing the tasklet
>   dmaengine: sun6i: Fix memory leaks
> 
>  drivers/dma/sun6i-dma.c | 40 +++++++++++++++++-----------------------
>  1 file changed, 17 insertions(+), 23 deletions(-)
> 
> -- 
> 2.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

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

* Re: [PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver
@ 2014-07-30 13:44   ` Vinod Koul
  0 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2014-07-30 13:44 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: Dan Williams, linux-kernel, linux-arm-kernel, dmaengine

On Wed, Jul 30, 2014 at 10:30:20AM +0200, Maxime Ripard wrote:
> Hi Dan, Vinod,
> 
> Here are the patches that you requested previously, plus an additional
> fix for a memory leak was spotted by Dan Carpenter.

Applied, thanks

-- 
~Vinod
> 
> Thanks,
> Maxime
> 
> Maxime Ripard (3):
>   dmaengine: sun6i: Remove switch statement from buswidth convertion
>     routine
>   dmaengine: sun6i: Free the interrupt before killing the tasklet
>   dmaengine: sun6i: Fix memory leaks
> 
>  drivers/dma/sun6i-dma.c | 40 +++++++++++++++++-----------------------
>  1 file changed, 17 insertions(+), 23 deletions(-)
> 
> -- 
> 2.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

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

end of thread, other threads:[~2014-07-30 13:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-30  8:30 [PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver Maxime Ripard
2014-07-30  8:30 ` Maxime Ripard
2014-07-30  8:30 ` [PATCH 1/3] dmaengine: sun6i: Remove switch statement from buswidth convertion routine Maxime Ripard
2014-07-30  8:30   ` Maxime Ripard
2014-07-30  8:30 ` [PATCH 2/3] dmaengine: sun6i: Free the interrupt before killing the tasklet Maxime Ripard
2014-07-30  8:30   ` Maxime Ripard
2014-07-30  8:30 ` [PATCH 3/3] dmaengine: sun6i: Fix memory leaks Maxime Ripard
2014-07-30  8:30   ` Maxime Ripard
2014-07-30 13:44 ` [PATCH 0/3] dmaengine: sun6i: Misc fixes for the dma driver Vinod Koul
2014-07-30 13:44   ` 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.