| Message ID | 20191021000824.531-8-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | IXP4xx networking cleanups | expand |
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.
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?
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
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; }
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