Skip to content

Commit

Permalink
Merge branch 'net-ethernet-convert-to-platform-remove-callback-return…
Browse files Browse the repository at this point in the history
…ing-void'

Uwe Kleine-König says:

====================
net: ethernet: Convert to platform remove callback returning void

in (implicit) v1 of this series
(https://lore.kernel.org/netdev/20231117091655.872426-1-u.kleine-koenig@pengutronix.de)
I tried to address the resource leaks in the three cpsw drivers. However
this is hard to get right without being able to test the changes. So
here comes a series that just converts all drivers below
drivers/net/ethernet to use .remove_new() and adds a comment about the
potential leaks for someone else to fix the problem.

See commit 5c5a768 ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
The TL;DR; is to prevent bugs like the three noticed here.

Note this series results in no change of behaviour apart from improving
the error message for the three cpsw drivers from

	remove callback returned a non-zero value. This will be ignored.

to

	Failed to resume device (-ESOMETHING)
====================

Link: https://lore.kernel.org/r/20231128173823.867512-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Paolo Abeni committed Nov 30, 2023
2 parents 0444718 + 7ec1bb2 commit 7e02226
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 19 deletions.
6 changes: 2 additions & 4 deletions drivers/net/ethernet/ezchip/nps_enet.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,16 +633,14 @@ static s32 nps_enet_probe(struct platform_device *pdev)
return err;
}

static s32 nps_enet_remove(struct platform_device *pdev)
static void nps_enet_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct nps_enet_priv *priv = netdev_priv(ndev);

unregister_netdev(ndev);
netif_napi_del(&priv->napi);
free_netdev(ndev);

return 0;
}

static const struct of_device_id nps_enet_dt_ids[] = {
Expand All @@ -653,7 +651,7 @@ MODULE_DEVICE_TABLE(of, nps_enet_dt_ids);

static struct platform_driver nps_enet_driver = {
.probe = nps_enet_probe,
.remove = nps_enet_remove,
.remove_new = nps_enet_remove,
.driver = {
.name = DRV_NAME,
.of_match_table = nps_enet_dt_ids,
Expand Down
15 changes: 10 additions & 5 deletions drivers/net/ethernet/ti/am65-cpsw-nuss.c
Original file line number Diff line number Diff line change
Expand Up @@ -3028,7 +3028,7 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev)
return ret;
}

static int am65_cpsw_nuss_remove(struct platform_device *pdev)
static void am65_cpsw_nuss_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct am65_cpsw_common *common;
Expand All @@ -3037,8 +3037,14 @@ static int am65_cpsw_nuss_remove(struct platform_device *pdev)
common = dev_get_drvdata(dev);

ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret < 0)
return ret;
if (ret < 0) {
/* Note, if this error path is taken, we're leaking some
* resources.
*/
dev_err(&pdev->dev, "Failed to resume device (%pe)\n",
ERR_PTR(ret));
return;
}

am65_cpsw_unregister_devlink(common);
am65_cpsw_unregister_notifiers(common);
Expand All @@ -3056,7 +3062,6 @@ static int am65_cpsw_nuss_remove(struct platform_device *pdev)

pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
return 0;
}

static int am65_cpsw_nuss_suspend(struct device *dev)
Expand Down Expand Up @@ -3156,7 +3161,7 @@ static struct platform_driver am65_cpsw_nuss_driver = {
.pm = &am65_cpsw_nuss_dev_pm_ops,
},
.probe = am65_cpsw_nuss_probe,
.remove = am65_cpsw_nuss_remove,
.remove_new = am65_cpsw_nuss_remove,
};

module_platform_driver(am65_cpsw_nuss_driver);
Expand Down
15 changes: 10 additions & 5 deletions drivers/net/ethernet/ti/cpsw.c
Original file line number Diff line number Diff line change
Expand Up @@ -1722,14 +1722,20 @@ static int cpsw_probe(struct platform_device *pdev)
return ret;
}

static int cpsw_remove(struct platform_device *pdev)
static void cpsw_remove(struct platform_device *pdev)
{
struct cpsw_common *cpsw = platform_get_drvdata(pdev);
int i, ret;

ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret < 0)
return ret;
if (ret < 0) {
/* Note, if this error path is taken, we're leaking some
* resources.
*/
dev_err(&pdev->dev, "Failed to resume device (%pe)\n",
ERR_PTR(ret));
return;
}

for (i = 0; i < cpsw->data.slaves; i++)
if (cpsw->slaves[i].ndev)
Expand All @@ -1740,7 +1746,6 @@ static int cpsw_remove(struct platform_device *pdev)
cpsw_remove_dt(pdev);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
return 0;
}

#ifdef CONFIG_PM_SLEEP
Expand Down Expand Up @@ -1795,7 +1800,7 @@ static struct platform_driver cpsw_driver = {
.of_match_table = cpsw_of_mtable,
},
.probe = cpsw_probe,
.remove = cpsw_remove,
.remove_new = cpsw_remove,
};

module_platform_driver(cpsw_driver);
Expand Down
15 changes: 10 additions & 5 deletions drivers/net/ethernet/ti/cpsw_new.c
Original file line number Diff line number Diff line change
Expand Up @@ -2037,14 +2037,20 @@ static int cpsw_probe(struct platform_device *pdev)
return ret;
}

static int cpsw_remove(struct platform_device *pdev)
static void cpsw_remove(struct platform_device *pdev)
{
struct cpsw_common *cpsw = platform_get_drvdata(pdev);
int ret;

ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret < 0)
return ret;
if (ret < 0) {
/* Note, if this error path is taken, we're leaking some
* resources.
*/
dev_err(&pdev->dev, "Failed to resume device (%pe)\n",
ERR_PTR(ret));
return;
}

cpsw_unregister_notifiers(cpsw);
cpsw_unregister_devlink(cpsw);
Expand All @@ -2055,7 +2061,6 @@ static int cpsw_remove(struct platform_device *pdev)
cpsw_remove_dt(cpsw);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
return 0;
}

static int __maybe_unused cpsw_suspend(struct device *dev)
Expand Down Expand Up @@ -2116,7 +2121,7 @@ static struct platform_driver cpsw_driver = {
.of_match_table = cpsw_of_mtable,
},
.probe = cpsw_probe,
.remove = cpsw_remove,
.remove_new = cpsw_remove,
};

module_platform_driver(cpsw_driver);
Expand Down

0 comments on commit 7e02226

Please sign in to comment.