[PATCH net-next 0/2] cpsw: fix resource leak for v3.8

STALE5072d

7 messages, 4 authors, 2012-11-05 · open the first message on its own page

[PATCH net-next 0/2] cpsw: fix resource leak for v3.8

From: richardcochran@gmail.com (Richard Cochran)
Date: 2012-11-03 08:25:28

While looking at the idea of removing all of the register offsets in
the CPSW's device tree, I noticed that the driver would be leaking IO
mappings. Although this is, strictly speaking, a bug fix, still it can
wait to appear in v3.8, since there is no way to use the driver in
v3.7 (or earlier) anyhow.

Thanks,
Richard


Richard Cochran (2):
  cpsw: rename register banks to match the reference manual, part 2
  cpsw: fix leaking IO mappings

 drivers/net/ethernet/ti/cpsw.c |   39 +++++++++++++++++++--------------------
 1 files changed, 19 insertions(+), 20 deletions(-)

-- 
1.7.2.5

[PATCH net-next 1/2] cpsw: rename register banks to match the reference manual, part 2

From: richardcochran@gmail.com (Richard Cochran)
Date: 2012-11-03 08:25:29

The code mixes up the CPSW_SS and the CPSW_WR register naming. This patch
changes the names to conform to the published Technical Reference Manual
from TI, in order to make working on the code less confusing.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/ti/cpsw.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 023d439..6215246 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c

@@ -286,7 +286,7 @@ struct cpsw_priv {
 	struct platform_device		*pdev;
 	struct net_device		*ndev;
 	struct resource			*cpsw_res;
-	struct resource			*cpsw_ss_res;
+	struct resource			*cpsw_wr_res;
 	struct napi_struct		napi;
 	struct device			*dev;
 	struct cpsw_platform_data	data;

@@ -1270,25 +1270,25 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
 	priv->host_port_regs = regs + data->host_port_reg_ofs;
 	priv->cpts.reg = regs + data->cpts_reg_ofs;
 
-	priv->cpsw_ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!priv->cpsw_ss_res) {
+	priv->cpsw_wr_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!priv->cpsw_wr_res) {
 		dev_err(priv->dev, "error getting i/o resource\n");
 		ret = -ENOENT;
 		goto clean_clk_ret;
 	}
 
-	if (!request_mem_region(priv->cpsw_ss_res->start,
-			resource_size(priv->cpsw_ss_res), ndev->name)) {
+	if (!request_mem_region(priv->cpsw_wr_res->start,
+			resource_size(priv->cpsw_wr_res), ndev->name)) {
 		dev_err(priv->dev, "failed request i/o region\n");
 		ret = -ENXIO;
 		goto clean_clk_ret;
 	}
 
-	regs = ioremap(priv->cpsw_ss_res->start,
-				resource_size(priv->cpsw_ss_res));
+	regs = ioremap(priv->cpsw_wr_res->start,
+				resource_size(priv->cpsw_wr_res));
 	if (!regs) {
 		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_ss_iores_ret;
+		goto clean_cpsw_wr_iores_ret;
 	}
 	priv->wr_regs = regs;

@@ -1409,9 +1409,9 @@ clean_dma_ret:
 	cpdma_ctlr_destroy(priv->dma);
 clean_iomap_ret:
 	iounmap(priv->regs);
-clean_cpsw_ss_iores_ret:
-	release_mem_region(priv->cpsw_ss_res->start,
-			   resource_size(priv->cpsw_ss_res));
+clean_cpsw_wr_iores_ret:
+	release_mem_region(priv->cpsw_wr_res->start,
+			   resource_size(priv->cpsw_wr_res));
 clean_cpsw_iores_ret:
 	release_mem_region(priv->cpsw_res->start,
 			   resource_size(priv->cpsw_res));

@@ -1442,8 +1442,8 @@ static int __devexit cpsw_remove(struct platform_device *pdev)
 	iounmap(priv->regs);
 	release_mem_region(priv->cpsw_res->start,
 			   resource_size(priv->cpsw_res));
-	release_mem_region(priv->cpsw_ss_res->start,
-			   resource_size(priv->cpsw_ss_res));
+	release_mem_region(priv->cpsw_wr_res->start,
+			   resource_size(priv->cpsw_wr_res));
 	pm_runtime_disable(&pdev->dev);
 	clk_put(priv->clk);
 	kfree(priv->slaves);

-- 
1.7.2.5

[PATCH net-next 2/2] cpsw: fix leaking IO mappings

From: richardcochran@gmail.com (Richard Cochran)
Date: 2012-11-03 08:25:30

The CPSW driver remaps two different IO regions, but fails to unmap them
both. This patch fixes the issue by calling iounmap in the appropriate
places.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/ti/cpsw.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 6215246..7654a62 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c

@@ -1252,14 +1252,12 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
 		ret = -ENOENT;
 		goto clean_clk_ret;
 	}
-
 	if (!request_mem_region(priv->cpsw_res->start,
 				resource_size(priv->cpsw_res), ndev->name)) {
 		dev_err(priv->dev, "failed request i/o region\n");
 		ret = -ENXIO;
 		goto clean_clk_ret;
 	}
-
 	regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
 	if (!regs) {
 		dev_err(priv->dev, "unable to map i/o region\n");

@@ -1274,16 +1272,14 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
 	if (!priv->cpsw_wr_res) {
 		dev_err(priv->dev, "error getting i/o resource\n");
 		ret = -ENOENT;
-		goto clean_clk_ret;
+		goto clean_iomap_ret;
 	}
-
 	if (!request_mem_region(priv->cpsw_wr_res->start,
 			resource_size(priv->cpsw_wr_res), ndev->name)) {
 		dev_err(priv->dev, "failed request i/o region\n");
 		ret = -ENXIO;
-		goto clean_clk_ret;
+		goto clean_iomap_ret;
 	}
-
 	regs = ioremap(priv->cpsw_wr_res->start,
 				resource_size(priv->cpsw_wr_res));
 	if (!regs) {

@@ -1326,7 +1322,7 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
 	if (!priv->dma) {
 		dev_err(priv->dev, "error initializing dma\n");
 		ret = -ENOMEM;
-		goto clean_iomap_ret;
+		goto clean_wr_iomap_ret;
 	}
 
 	priv->txch = cpdma_chan_create(priv->dma, tx_chan_num(0),

@@ -1407,11 +1403,13 @@ clean_dma_ret:
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);
 	cpdma_ctlr_destroy(priv->dma);
-clean_iomap_ret:
-	iounmap(priv->regs);
+clean_wr_iomap_ret:
+	iounmap(priv->wr_regs);
 clean_cpsw_wr_iores_ret:
 	release_mem_region(priv->cpsw_wr_res->start,
 			   resource_size(priv->cpsw_wr_res));
+clean_iomap_ret:
+	iounmap(priv->regs);
 clean_cpsw_iores_ret:
 	release_mem_region(priv->cpsw_res->start,
 			   resource_size(priv->cpsw_res));

@@ -1442,6 +1440,7 @@ static int __devexit cpsw_remove(struct platform_device *pdev)
 	iounmap(priv->regs);
 	release_mem_region(priv->cpsw_res->start,
 			   resource_size(priv->cpsw_res));
+	iounmap(priv->wr_regs);
 	release_mem_region(priv->cpsw_wr_res->start,
 			   resource_size(priv->cpsw_wr_res));
 	pm_runtime_disable(&pdev->dev);

-- 
1.7.2.5

[PATCH net-next 0/2] cpsw: fix resource leak for v3.8

From: davem@davemloft.net (David Miller)
Date: 2012-11-03 19:38:52

From: Richard Cochran <richardcochran@gmail.com>
Date: Sat,  3 Nov 2012 09:25:28 +0100

While looking at the idea of removing all of the register offsets in
the CPSW's device tree, I noticed that the driver would be leaking IO
mappings. Although this is, strictly speaking, a bug fix, still it can
wait to appear in v3.8, since there is no way to use the driver in
v3.7 (or earlier) anyhow.

All applied, thanks.

[PATCH net-next 2/2] cpsw: fix leaking IO mappings

From: Cyril Chemparathy <hidden>
Date: 2012-11-04 13:23:58

On 11/03/2012 09:25 AM, Richard Cochran wrote:

The CPSW driver remaps two different IO regions, but fails to unmap them
both. This patch fixes the issue by calling iounmap in the appropriate
places.

Any thoughts on using devres helpers to keep the bail out path simpler?

quoted hunk

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
  drivers/net/ethernet/ti/cpsw.c |   17 ++++++++---------
  1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 6215246..7654a62 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c

@@ -1252,14 +1252,12 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
  		ret = -ENOENT;
  		goto clean_clk_ret;
  	}
-
  	if (!request_mem_region(priv->cpsw_res->start,
  				resource_size(priv->cpsw_res), ndev->name)) {
  		dev_err(priv->dev, "failed request i/o region\n");
  		ret = -ENXIO;
  		goto clean_clk_ret;
  	}
-
  	regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
  	if (!regs) {
  		dev_err(priv->dev, "unable to map i/o region\n");

@@ -1274,16 +1272,14 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
  	if (!priv->cpsw_wr_res) {
  		dev_err(priv->dev, "error getting i/o resource\n");
  		ret = -ENOENT;
-		goto clean_clk_ret;
+		goto clean_iomap_ret;
  	}
-
  	if (!request_mem_region(priv->cpsw_wr_res->start,
  			resource_size(priv->cpsw_wr_res), ndev->name)) {
  		dev_err(priv->dev, "failed request i/o region\n");
  		ret = -ENXIO;
-		goto clean_clk_ret;
+		goto clean_iomap_ret;
  	}
-
  	regs = ioremap(priv->cpsw_wr_res->start,
  				resource_size(priv->cpsw_wr_res));
  	if (!regs) {

@@ -1326,7 +1322,7 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
  	if (!priv->dma) {
  		dev_err(priv->dev, "error initializing dma\n");
  		ret = -ENOMEM;
-		goto clean_iomap_ret;
+		goto clean_wr_iomap_ret;
  	}
  
  	priv->txch = cpdma_chan_create(priv->dma, tx_chan_num(0),

@@ -1407,11 +1403,13 @@ clean_dma_ret:
  	cpdma_chan_destroy(priv->txch);
  	cpdma_chan_destroy(priv->rxch);
  	cpdma_ctlr_destroy(priv->dma);
-clean_iomap_ret:
-	iounmap(priv->regs);
+clean_wr_iomap_ret:
+	iounmap(priv->wr_regs);
  clean_cpsw_wr_iores_ret:
  	release_mem_region(priv->cpsw_wr_res->start,
  			   resource_size(priv->cpsw_wr_res));
+clean_iomap_ret:
+	iounmap(priv->regs);
  clean_cpsw_iores_ret:
  	release_mem_region(priv->cpsw_res->start,
  			   resource_size(priv->cpsw_res));

@@ -1442,6 +1440,7 @@ static int __devexit cpsw_remove(struct platform_device *pdev)
  	iounmap(priv->regs);
  	release_mem_region(priv->cpsw_res->start,
  			   resource_size(priv->cpsw_res));
+	iounmap(priv->wr_regs);
  	release_mem_region(priv->cpsw_wr_res->start,
  			   resource_size(priv->cpsw_wr_res));
  	pm_runtime_disable(&pdev->dev);

[PATCH net-next 2/2] cpsw: fix leaking IO mappings

From: richardcochran@gmail.com (Richard Cochran)
Date: 2012-11-04 15:16:20

On Sun, Nov 04, 2012 at 02:23:58PM +0100, Cyril Chemparathy wrote:

On 11/03/2012 09:25 AM, Richard Cochran wrote:

quoted

The CPSW driver remaps two different IO regions, but fails to unmap them
both. This patch fixes the issue by calling iounmap in the appropriate
places.

Any thoughts on using devres helpers to keep the bail out path simpler?

I wasn't aware of devres before, but I just read through the doc. It
looks interesting, and if I were writting a new driver, then I might
use it.

But for this driver, it really isn't very complex. It only looks that
way, but I think this just accidental. I guess that the code was
adapted from the davinci emac driver.

Thanks,
Richard

[PATCH net-next 0/2] cpsw: fix resource leak for v3.8

From: N, Mugunthan V <hidden>
Date: 2012-11-05 08:40:34

-----Original Message-----
From: Richard Cochran [mailto:richardcochran@gmail.com]
Sent: Saturday, November 03, 2012 1:55 PM
To: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org; David Miller; Chemparathy,
Cyril; N, Mugunthan V; Hiremath, Vaibhav
Subject: [PATCH net-next 0/2] cpsw: fix resource leak for v3.8

While looking at the idea of removing all of the register offsets in
the CPSW's device tree, I noticed that the driver would be leaking IO
mappings. Although this is, strictly speaking, a bug fix, still it can
wait to appear in v3.8, since there is no way to use the driver in
v3.7 (or earlier) anyhow.

Thanks,
Richard

Richard Cochran (2):
  cpsw: rename register banks to match the reference manual, part 2
  cpsw: fix leaking IO mappings

 drivers/net/ethernet/ti/cpsw.c |   39 +++++++++++++++++++-------------
-------
 1 files changed, 19 insertions(+), 20 deletions(-)

--
1.7.2.5

The patches look good to me.

Acked-by: Mugunthan V N <redacted>

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help