[07/10] net: ehernet: ixp4xx: Use devm_alloc_etherdev()

Message ID 20191021000824.531-8-linus.walleij@linaro.org
State New
Headers show
Series
  • IXP4xx networking cleanups
Related show

Commit Message

Linus Walleij Oct. 21, 2019, 12:08 a.m.
Using the devm_alloc_etherdev() function simplifies the error
path. I also patch the message to use dev_info().

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

-- 
2.21.0

Comments

Jakub Kicinski Oct. 22, 2019, 1:21 a.m. | #1
On Mon, 21 Oct 2019 02:08:21 +0200, Linus Walleij wrote:
> Using the devm_alloc_etherdev() function simplifies the error

> path. I also patch the message to use dev_info().

> 

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  drivers/net/ethernet/xscale/ixp4xx_eth.c | 18 ++++++------------

>  1 file changed, 6 insertions(+), 12 deletions(-)

> 

> diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c

> index fbe328693de5..df18d8ebb170 100644

> --- a/drivers/net/ethernet/xscale/ixp4xx_eth.c

> +++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c

> @@ -1378,7 +1378,7 @@ static int ixp4xx_eth_probe(struct platform_device *pdev)

>  

>  	plat = dev_get_platdata(dev);

>  

> -	if (!(ndev = alloc_etherdev(sizeof(struct port))))

> +	if (!(ndev = devm_alloc_etherdev(dev, sizeof(struct port))))

>  		return -ENOMEM;

>  

>  	SET_NETDEV_DEV(ndev, dev);


Okay, I see you do devm_ here.. please reorder the patches, then.
Joe Perches Oct. 26, 2019, 10:24 p.m. | #2
On Mon, 2019-10-21 at 02:08 +0200, Linus Walleij wrote:
> Using the devm_alloc_etherdev() function simplifies the error

> path. I also patch the message to use dev_info().


slight typo in subject

> diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c


Maybe it's better to avoid changing this driver.
Is this device still sold?  It's 15+ years old.

> @@ -1378,7 +1378,7 @@ static int ixp4xx_eth_probe(struct platform_device *pdev)

>  

>  	plat = dev_get_platdata(dev);

>  

> -	if (!(ndev = alloc_etherdev(sizeof(struct port))))

> +	if (!(ndev = devm_alloc_etherdev(dev, sizeof(struct port))))


Probably nicer to split the assignment and test too.

> @@ -1479,8 +1476,8 @@ static int ixp4xx_eth_probe(struct platform_device *pdev)

>  	if ((err = register_netdev(ndev)))

>  		goto err_phy_dis;

>  

> -	printk(KERN_INFO "%s: MII PHY %i on %s\n", ndev->name, plat->phy,

> -	       npe_name(port->npe));

> +	dev_info(dev, "%s: MII PHY %i on %s\n", ndev->name, plat->phy,

> +		 npe_name(port->npe));


and this should probably be

	netdev_info(ndev, "MII PHY %d on %s\n", plat->phy, npe_name(port->npe));

But there are 30+ printks in this file, so why just this one?
Linus Walleij Oct. 31, 2019, 11:13 p.m. | #3
On Sun, Oct 27, 2019 at 12:24 AM Joe Perches <joe@perches.com> wrote:
> On Mon, 2019-10-21 at 02:08 +0200, Linus Walleij wrote:


> > diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c

>

> Maybe it's better to avoid changing this driver.

> Is this device still sold?  It's 15+ years old.


I am converting the whole platform to device tree so I need to
change this and many other drivers.

The rationale has been explained elsewhere but here it is for your
convenience:

A major reason why IXP4xx silicon is still produced and deployed is
the operating conditions. If you look at for example the Gateworks
Cambria GW2358-4 network processor you notice the strictly
military operating conditions:

Temperature: -40°C to +85°C
Humidity (non-condensing): 20% to 90%
MTBF (mean time before failure): 60 Years at 55°C

We have good reasons to believe that these are used in critical
systems that are not consumer products and do not adhere to
consumer product life cycle expectations. Think more like this:
https://www.c4isrnet.com/air/2019/10/17/the-us-nuclear-forces-dr-strangelove-era-messaging-system-finally-got-rid-of-its-floppy-disks/

Yours,
Linus Walleij

Patch

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index fbe328693de5..df18d8ebb170 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -1378,7 +1378,7 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 
 	plat = dev_get_platdata(dev);
 
-	if (!(ndev = alloc_etherdev(sizeof(struct port))))
+	if (!(ndev = devm_alloc_etherdev(dev, sizeof(struct port))))
 		return -ENOMEM;
 
 	SET_NETDEV_DEV(ndev, dev);
@@ -1432,8 +1432,7 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 		regs_phys  = IXP4XX_EthC_BASE_PHYS;
 		break;
 	default:
-		err = -ENODEV;
-		goto err_free;
+		return -ENODEV;
 	}
 
 	ndev->netdev_ops = &ixp4xx_netdev_ops;
@@ -1442,10 +1441,8 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 
 	netif_napi_add(ndev, &port->napi, eth_poll, NAPI_WEIGHT);
 
-	if (!(port->npe = npe_request(NPE_ID(port->id)))) {
-		err = -EIO;
-		goto err_free;
-	}
+	if (!(port->npe = npe_request(NPE_ID(port->id))))
+		return -EIO;
 
 	port->mem_res = request_mem_region(regs_phys, REGS_SIZE, ndev->name);
 	if (!port->mem_res) {
@@ -1479,8 +1476,8 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 	if ((err = register_netdev(ndev)))
 		goto err_phy_dis;
 
-	printk(KERN_INFO "%s: MII PHY %i on %s\n", ndev->name, plat->phy,
-	       npe_name(port->npe));
+	dev_info(dev, "%s: MII PHY %i on %s\n", ndev->name, plat->phy,
+		 npe_name(port->npe));
 
 	return 0;
 
@@ -1491,8 +1488,6 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 	release_resource(port->mem_res);
 err_npe_rel:
 	npe_release(port->npe);
-err_free:
-	free_netdev(ndev);
 	return err;
 }
 
@@ -1508,7 +1503,6 @@  static int ixp4xx_eth_remove(struct platform_device *pdev)
 	npe_port_tab[NPE_ID(port->id)] = NULL;
 	npe_release(port->npe);
 	release_resource(port->mem_res);
-	free_netdev(ndev);
 	return 0;
 }