Discussion:
[PATCH net-next 0/6] cleanup on resource check
Varka Bhadram
2014-10-22 04:16:20 UTC
Permalink
This series removes the duplication of sanity check for
platform_get_resource() return resource. It will be checked
with devm_ioremap_resource()

Varka Bhadram (6):
ethernet: wiznet: remove unnecessary check
ethernet: wiznet: remove unnecessary check
ethernet: apm: xgene: remove unnecessary check
ethernet: marvell: remove unnecessary check
ethernet: renesas: remove unnecessary check
ethernet: samsung: sxgbe: remove unnecessary check

drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 12 ------------
drivers/net/ethernet/marvell/pxa168_eth.c | 6 ++----
drivers/net/ethernet/renesas/sh_eth.c | 4 ----
.../net/ethernet/samsung/sxgbe/sxgbe_platform.c | 3 ---
drivers/net/ethernet/wiznet/w5100.c | 3 +--
drivers/net/ethernet/wiznet/w5300.c | 3 +--
6 files changed, 4 insertions(+), 27 deletions(-)
--
1.7.9.5
Varka Bhadram
2014-10-22 04:16:22 UTC
Permalink
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.

Signed-off-by: Varka Bhadram <***@cdac.in>
---
drivers/net/ethernet/wiznet/w5300.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5300.c b/drivers/net/ethernet/wiznet/w5300.c
index f961f14..315d090 100644
--- a/drivers/net/ethernet/wiznet/w5300.c
+++ b/drivers/net/ethernet/wiznet/w5300.c
@@ -558,8 +558,7 @@ static int w5300_hw_probe(struct platform_device *pdev)
}

mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -ENXIO;
+
mem_size = resource_size(mem);

priv->base = devm_ioremap_resource(&pdev->dev, mem);
--
1.7.9.5
Sergei Shtylyov
2014-10-22 11:44:46 UTC
Permalink
Post by Varka Bhadram
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.
---
drivers/net/ethernet/wiznet/w5300.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/wiznet/w5300.c b/drivers/net/ethernet/wiznet/w5300.c
index f961f14..315d090 100644
--- a/drivers/net/ethernet/wiznet/w5300.c
+++ b/drivers/net/ethernet/wiznet/w5300.c
@@ -558,8 +558,7 @@ static int w5300_hw_probe(struct platform_device *pdev)
}
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -ENXIO;
+
mem_size = resource_size(mem);
Same problem as in the patch #1
Post by Varka Bhadram
priv->base = devm_ioremap_resource(&pdev->dev, mem);
WBR, Sergei
Varka Bhadram
2014-10-22 12:24:19 UTC
Permalink
Post by Sergei Shtylyov
Post by Varka Bhadram
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.
---
drivers/net/ethernet/wiznet/w5300.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/wiznet/w5300.c
b/drivers/net/ethernet/wiznet/w5300.c
index f961f14..315d090 100644
--- a/drivers/net/ethernet/wiznet/w5300.c
+++ b/drivers/net/ethernet/wiznet/w5300.c
@@ -558,8 +558,7 @@ static int w5300_hw_probe(struct platform_device *pdev)
}
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -ENXIO;
+
mem_size = resource_size(mem);
Same problem as in the patch #1
I will fix it . Thanks
Post by Sergei Shtylyov
Post by Varka Bhadram
priv->base = devm_ioremap_resource(&pdev->dev, mem);
WBR, Sergei
--
Regards,
Varka Bhadram.
Varka Bhadram
2014-10-22 04:16:23 UTC
Permalink
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.

Signed-off-by: Varka Bhadram <***@cdac.in>
---
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 12 ------------
1 file changed, 12 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 3c208cc..f226594 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -761,10 +761,6 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
ndev = pdata->ndev;

res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "enet_csr");
- if (!res) {
- dev_err(dev, "Resource enet_csr not defined\n");
- return -ENODEV;
- }
pdata->base_addr = devm_ioremap_resource(dev, res);
if (IS_ERR(pdata->base_addr)) {
dev_err(dev, "Unable to retrieve ENET Port CSR region\n");
@@ -772,10 +768,6 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
}

res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ring_csr");
- if (!res) {
- dev_err(dev, "Resource ring_csr not defined\n");
- return -ENODEV;
- }
pdata->ring_csr_addr = devm_ioremap_resource(dev, res);
if (IS_ERR(pdata->ring_csr_addr)) {
dev_err(dev, "Unable to retrieve ENET Ring CSR region\n");
@@ -783,10 +775,6 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
}

res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ring_cmd");
- if (!res) {
- dev_err(dev, "Resource ring_cmd not defined\n");
- return -ENODEV;
- }
pdata->ring_cmd_addr = devm_ioremap_resource(dev, res);
if (IS_ERR(pdata->ring_cmd_addr)) {
dev_err(dev, "Unable to retrieve ENET Ring command region\n");
--
1.7.9.5
Varka Bhadram
2014-10-22 04:16:24 UTC
Permalink
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.

Signed-off-by: Varka Bhadram <***@cdac.in>
---
drivers/net/ethernet/marvell/pxa168_eth.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index c3b209c..a378c92 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -1505,16 +1505,14 @@ static int pxa168_eth_probe(struct platform_device *pdev)
pep = netdev_priv(dev);
pep->dev = dev;
pep->clk = clk;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res == NULL) {
- err = -ENODEV;
- goto err_netdev;
- }
pep->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(pep->base)) {
err = -ENOMEM;
goto err_netdev;
}
+
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
BUG_ON(!res);
dev->irq = res->start;
--
1.7.9.5
Varka Bhadram
2014-10-22 04:16:26 UTC
Permalink
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.

Signed-off-by: Varka Bhadram <***@cdac.in>
---
.../net/ethernet/samsung/sxgbe/sxgbe_platform.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
index b147d46..7fd6e27 100644
--- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
+++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
@@ -90,9 +90,6 @@ static int sxgbe_platform_probe(struct platform_device *pdev)

/* Get memory resource */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- goto err_out;
-
addr = devm_ioremap_resource(dev, res);
if (IS_ERR(addr))
return PTR_ERR(addr);
--
1.7.9.5
Varka Bhadram
2014-10-22 04:16:25 UTC
Permalink
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.

Signed-off-by: Varka Bhadram <***@cdac.in>
---
drivers/net/ethernet/renesas/sh_eth.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 60e9c2c..d824ba5 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2769,10 +2769,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)

/* get base addr */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (unlikely(res == NULL)) {
- dev_err(&pdev->dev, "invalid resource\n");
- return -EINVAL;
- }

ndev = alloc_etherdev(sizeof(struct sh_eth_private));
if (!ndev)
--
1.7.9.5
Sergei Shtylyov
2014-10-22 11:47:43 UTC
Permalink
Post by Varka Bhadram
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.
---
drivers/net/ethernet/renesas/sh_eth.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 60e9c2c..d824ba5 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2769,10 +2769,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
/* get base addr */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (unlikely(res == NULL)) {
- dev_err(&pdev->dev, "invalid resource\n");
- return -EINVAL;
- }
The driver dereferences 'res' further on, so you can't remove this check.

WBR, Sergei
Varka Bhadram
2014-10-22 12:22:28 UTC
Permalink
Post by Sergei Shtylyov
Post by Varka Bhadram
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.
---
drivers/net/ethernet/renesas/sh_eth.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c
b/drivers/net/ethernet/renesas/sh_eth.c
index 60e9c2c..d824ba5 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2769,10 +2769,6 @@ static int sh_eth_drv_probe(struct
platform_device *pdev)
/* get base addr */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (unlikely(res == NULL)) {
- dev_err(&pdev->dev, "invalid resource\n");
- return -EINVAL;
- }
The driver dereferences 'res' further on, so you can't remove this check.
Yes its my mistake . I will fix it . Thankx
Post by Sergei Shtylyov
WBR, Sergei
--
Regards,
Varka Bhadram.
Varka Bhadram
2014-10-22 04:16:21 UTC
Permalink
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.

Signed-off-by: Varka Bhadram <***@cdac.in>
---
drivers/net/ethernet/wiznet/w5100.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 0f56b1c..bf195e3 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -638,8 +638,7 @@ static int w5100_hw_probe(struct platform_device *pdev)
}

mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -ENXIO;
+
mem_size = resource_size(mem);

priv->base = devm_ioremap_resource(&pdev->dev, mem);
--
1.7.9.5
Sergei Shtylyov
2014-10-22 11:43:50 UTC
Permalink
Hello.
Post by Varka Bhadram
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.
---
drivers/net/ethernet/wiznet/w5100.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 0f56b1c..bf195e3 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -638,8 +638,7 @@ static int w5100_hw_probe(struct platform_device *pdev)
}
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -ENXIO;
+
mem_size = resource_size(mem);
This would cause a NULL dereference on resource_size() call if 'mem' is
NULL. You can't remove the NULL check because of that.
Post by Varka Bhadram
priv->base = devm_ioremap_resource(&pdev->dev, mem);
WBR, Sergei
Varka Bhadram
2014-10-22 12:23:46 UTC
Permalink
Post by Sergei Shtylyov
Hello.
Post by Varka Bhadram
devm_ioremap_resource checks platform_get_resource() return value.
We can remove the duplicate check here.
---
drivers/net/ethernet/wiznet/w5100.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/wiznet/w5100.c
b/drivers/net/ethernet/wiznet/w5100.c
index 0f56b1c..bf195e3 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -638,8 +638,7 @@ static int w5100_hw_probe(struct platform_device *pdev)
}
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -ENXIO;
+
mem_size = resource_size(mem);
This would cause a NULL dereference on resource_size() call if
'mem' is NULL. You can't remove the NULL check because of that.
Ok i will fix it . Thankx for the review.
Post by Sergei Shtylyov
Post by Varka Bhadram
priv->base = devm_ioremap_resource(&pdev->dev, mem);
WBR, Sergei
--
Regards,
Varka Bhadram.
Loading...