All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/8] net: freescale: Convert to platform remove callback returning void
@ 2023-06-06 16:28 ` Uwe Kleine-König
  0 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Russell King, Wei Fang, Marc Kleine-Budde,
	Damien Le Moal, Michael Ellerman, Andy Shevchenko, Rob Herring,
	Pantelis Antoniou, Michal Kubiak, Claudiu Manoil, Li Yang,
	Andrew Lunn
  Cc: netdev, kernel, Shenwei Wang, Clark Wang, NXP Linux Team,
	Sean Anderson, linuxppc-dev

Hello,

(implicit) v1 of this series was sent in March and can be found at
https://lore.kernel.org/netdev/20230313103653.2753139-1-u.kleine-koenig@pengutronix.de

Changes since then:

 - Dropped "net: fec: Don't return early on error in .remove()", this is
   replaced by "net: fec: Better handle pm_runtime_get() failing in .remove()"
   which is already in v6.4-rc3 as f816b9829b19394d318e01953aa3b2721bca040d.

 - Simplified

	dev_err(&pdev->dev, ...

   to

	dev_err(dev, ...

   in patch #1; pointed out by Madalin Bucur.

 - Added some review tags received for v1.

Thanks to Madalin Bucur, Andrew Lunn, Michal Kubiak and Jakub Kicinski for
their replies to v1.

Best regards
Uwe

Uwe Kleine-König (8):
  net: dpaa: Improve error reporting
  net: dpaa: Convert to platform remove callback returning void
  net: fec: Convert to platform remove callback returning void
  net: fman: Convert to platform remove callback returning void
  net: fs_enet: Convert to platform remove callback returning void
  net: fsl_pq_mdio: Convert to platform remove callback returning void
  net: gianfar: Convert to platform remove callback returning void
  net: ucc_geth: Convert to platform remove callback returning void

 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c        | 8 ++++----
 drivers/net/ethernet/freescale/fec_main.c             | 5 ++---
 drivers/net/ethernet/freescale/fec_mpc52xx.c          | 6 ++----
 drivers/net/ethernet/freescale/fec_mpc52xx_phy.c      | 6 ++----
 drivers/net/ethernet/freescale/fman/mac.c             | 5 ++---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++---
 drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c  | 6 ++----
 drivers/net/ethernet/freescale/fs_enet/mii-fec.c      | 6 ++----
 drivers/net/ethernet/freescale/fsl_pq_mdio.c          | 6 ++----
 drivers/net/ethernet/freescale/gianfar.c              | 6 ++----
 drivers/net/ethernet/freescale/ucc_geth.c             | 6 ++----
 11 files changed, 24 insertions(+), 41 deletions(-)

base-commit: d4031ec844bc52fe7f2f844e9c476727fd6b8240
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 0/8] net: freescale: Convert to platform remove callback returning void
@ 2023-06-06 16:28 ` Uwe Kleine-König
  0 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Russell King, Wei Fang, Marc Kleine-Budde,
	Damien Le Moal, Michael Ellerman, Andy Shevchenko, Rob Herring,
	Pantelis Antoniou, Michal Kubiak, Claudiu Manoil, Li Yang,
	Andrew Lunn
  Cc: Sean Anderson, netdev, Shenwei Wang, Clark Wang, NXP Linux Team,
	kernel, linuxppc-dev

Hello,

(implicit) v1 of this series was sent in March and can be found at
https://lore.kernel.org/netdev/20230313103653.2753139-1-u.kleine-koenig@pengutronix.de

Changes since then:

 - Dropped "net: fec: Don't return early on error in .remove()", this is
   replaced by "net: fec: Better handle pm_runtime_get() failing in .remove()"
   which is already in v6.4-rc3 as f816b9829b19394d318e01953aa3b2721bca040d.

 - Simplified

	dev_err(&pdev->dev, ...

   to

	dev_err(dev, ...

   in patch #1; pointed out by Madalin Bucur.

 - Added some review tags received for v1.

Thanks to Madalin Bucur, Andrew Lunn, Michal Kubiak and Jakub Kicinski for
their replies to v1.

Best regards
Uwe

Uwe Kleine-König (8):
  net: dpaa: Improve error reporting
  net: dpaa: Convert to platform remove callback returning void
  net: fec: Convert to platform remove callback returning void
  net: fman: Convert to platform remove callback returning void
  net: fs_enet: Convert to platform remove callback returning void
  net: fsl_pq_mdio: Convert to platform remove callback returning void
  net: gianfar: Convert to platform remove callback returning void
  net: ucc_geth: Convert to platform remove callback returning void

 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c        | 8 ++++----
 drivers/net/ethernet/freescale/fec_main.c             | 5 ++---
 drivers/net/ethernet/freescale/fec_mpc52xx.c          | 6 ++----
 drivers/net/ethernet/freescale/fec_mpc52xx_phy.c      | 6 ++----
 drivers/net/ethernet/freescale/fman/mac.c             | 5 ++---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++---
 drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c  | 6 ++----
 drivers/net/ethernet/freescale/fs_enet/mii-fec.c      | 6 ++----
 drivers/net/ethernet/freescale/fsl_pq_mdio.c          | 6 ++----
 drivers/net/ethernet/freescale/gianfar.c              | 6 ++----
 drivers/net/ethernet/freescale/ucc_geth.c             | 6 ++----
 11 files changed, 24 insertions(+), 41 deletions(-)

base-commit: d4031ec844bc52fe7f2f844e9c476727fd6b8240
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 1/8] net: dpaa: Improve error reporting
  2023-06-06 16:28 ` Uwe Kleine-König
  (?)
@ 2023-06-06 16:28 ` Uwe Kleine-König
  2023-06-06 17:03   ` Maciej Fijalkowski
  -1 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Russell King
  Cc: netdev, kernel, Madalin Bucur, Michal Kubiak

Instead of the generic error message emitted by the driver core when a
remove callback returns an error code ("remove callback returned a
non-zero value. This will be ignored."), emit a message describing the
actual problem and return zero to suppress the generic message.

Note that apart from suppressing the generic error message there are no
side effects by changing the return value to zero. This prepares
changing the remove callback to return void.

Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 431f8917dc39..6226c03cfca0 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -3516,6 +3516,8 @@ static int dpaa_remove(struct platform_device *pdev)
 	phylink_destroy(priv->mac_dev->phylink);
 
 	err = dpaa_fq_free(dev, &priv->dpaa_fq_list);
+	if (err)
+		dev_err(dev, "Failed to free FQs on remove\n");
 
 	qman_delete_cgr_safe(&priv->ingress_cgr);
 	qman_release_cgrid(priv->ingress_cgr.cgrid);
@@ -3528,7 +3530,7 @@ static int dpaa_remove(struct platform_device *pdev)
 
 	free_netdev(net_dev);
 
-	return err;
+	return 0;
 }
 
 static const struct platform_device_id dpaa_devtype[] = {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 2/8] net: dpaa: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
  (?)
  (?)
@ 2023-06-06 16:28 ` Uwe Kleine-König
  2023-06-07 14:37   ` Simon Horman
  -1 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, kernel, Madalin Bucur, Michal Kubiak

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 6226c03cfca0..01c058cd0675 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -3497,7 +3497,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int dpaa_remove(struct platform_device *pdev)
+static void dpaa_remove(struct platform_device *pdev)
 {
 	struct net_device *net_dev;
 	struct dpaa_priv *priv;
@@ -3529,8 +3529,6 @@ static int dpaa_remove(struct platform_device *pdev)
 	dpaa_bps_free(priv);
 
 	free_netdev(net_dev);
-
-	return 0;
 }
 
 static const struct platform_device_id dpaa_devtype[] = {
@@ -3548,7 +3546,7 @@ static struct platform_driver dpaa_driver = {
 	},
 	.id_table = dpaa_devtype,
 	.probe = dpaa_eth_probe,
-	.remove = dpaa_remove
+	.remove_new = dpaa_remove
 };
 
 static int __init dpaa_load(void)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 3/8] net: fec: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
                   ` (2 preceding siblings ...)
  (?)
@ 2023-06-06 16:28 ` Uwe Kleine-König
  2023-06-07  6:48   ` Wei Fang
  2023-06-07 14:39   ` Simon Horman
  -1 siblings, 2 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Wei Fang, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Marc Kleine-Budde, Damien Le Moal, Michael Ellerman,
	Andy Shevchenko, Rob Herring
  Cc: Shenwei Wang, Clark Wang, NXP Linux Team, netdev, kernel, Michal Kubiak

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec_main.c        | 5 ++---
 drivers/net/ethernet/freescale/fec_mpc52xx.c     | 6 ++----
 drivers/net/ethernet/freescale/fec_mpc52xx_phy.c | 6 ++----
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 632bb4d589d7..f2b333b3f8c5 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -4458,7 +4458,7 @@ fec_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int
+static void
 fec_drv_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
@@ -4494,7 +4494,6 @@ fec_drv_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 
 	free_netdev(ndev);
-	return 0;
 }
 
 static int __maybe_unused fec_suspend(struct device *dev)
@@ -4650,7 +4649,7 @@ static struct platform_driver fec_driver = {
 	},
 	.id_table = fec_devtype,
 	.probe	= fec_probe,
-	.remove	= fec_drv_remove,
+	.remove_new = fec_drv_remove,
 };
 
 module_platform_driver(fec_driver);
diff --git a/drivers/net/ethernet/freescale/fec_mpc52xx.c b/drivers/net/ethernet/freescale/fec_mpc52xx.c
index b88816b71ddf..984ece5cd64f 100644
--- a/drivers/net/ethernet/freescale/fec_mpc52xx.c
+++ b/drivers/net/ethernet/freescale/fec_mpc52xx.c
@@ -974,7 +974,7 @@ static int mpc52xx_fec_probe(struct platform_device *op)
 	return rv;
 }
 
-static int
+static void
 mpc52xx_fec_remove(struct platform_device *op)
 {
 	struct net_device *ndev;
@@ -998,8 +998,6 @@ mpc52xx_fec_remove(struct platform_device *op)
 	release_mem_region(ndev->base_addr, sizeof(struct mpc52xx_fec));
 
 	free_netdev(ndev);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM
@@ -1042,7 +1040,7 @@ static struct platform_driver mpc52xx_fec_driver = {
 		.of_match_table = mpc52xx_fec_match,
 	},
 	.probe		= mpc52xx_fec_probe,
-	.remove		= mpc52xx_fec_remove,
+	.remove_new	= mpc52xx_fec_remove,
 #ifdef CONFIG_PM
 	.suspend	= mpc52xx_fec_of_suspend,
 	.resume		= mpc52xx_fec_of_resume,
diff --git a/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c b/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
index 95f778cce98c..61d3776d6750 100644
--- a/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
+++ b/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
@@ -117,7 +117,7 @@ static int mpc52xx_fec_mdio_probe(struct platform_device *of)
 	return err;
 }
 
-static int mpc52xx_fec_mdio_remove(struct platform_device *of)
+static void mpc52xx_fec_mdio_remove(struct platform_device *of)
 {
 	struct mii_bus *bus = platform_get_drvdata(of);
 	struct mpc52xx_fec_mdio_priv *priv = bus->priv;
@@ -126,8 +126,6 @@ static int mpc52xx_fec_mdio_remove(struct platform_device *of)
 	iounmap(priv->regs);
 	kfree(priv);
 	mdiobus_free(bus);
-
-	return 0;
 }
 
 static const struct of_device_id mpc52xx_fec_mdio_match[] = {
@@ -145,7 +143,7 @@ struct platform_driver mpc52xx_fec_mdio_driver = {
 		.of_match_table = mpc52xx_fec_mdio_match,
 	},
 	.probe = mpc52xx_fec_mdio_probe,
-	.remove = mpc52xx_fec_mdio_remove,
+	.remove_new = mpc52xx_fec_mdio_remove,
 };
 
 /* let fec driver call it, since this has to be registered before it */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 4/8] net: fman: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
                   ` (3 preceding siblings ...)
  (?)
@ 2023-06-06 16:28 ` Uwe Kleine-König
  2023-06-07 14:38   ` Simon Horman
  -1 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Sean Anderson, netdev, kernel, Madalin Bucur, Michal Kubiak

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/fman/mac.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 43665806c590..c5045891d694 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -331,12 +331,11 @@ static int mac_probe(struct platform_device *_of_dev)
 	return err;
 }
 
-static int mac_remove(struct platform_device *pdev)
+static void mac_remove(struct platform_device *pdev)
 {
 	struct mac_device *mac_dev = platform_get_drvdata(pdev);
 
 	platform_device_unregister(mac_dev->priv->eth_dev);
-	return 0;
 }
 
 static struct platform_driver mac_driver = {
@@ -345,7 +344,7 @@ static struct platform_driver mac_driver = {
 		.of_match_table	= mac_match,
 	},
 	.probe		= mac_probe,
-	.remove		= mac_remove,
+	.remove_new	= mac_remove,
 };
 
 builtin_platform_driver(mac_driver);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 5/8] net: fs_enet: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
@ 2023-06-06 16:28   ` Uwe Kleine-König
  -1 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Pantelis Antoniou, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: linuxppc-dev, netdev, kernel, Michal Kubiak

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++---
 drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c  | 6 ++----
 drivers/net/ethernet/freescale/fs_enet/mii-fec.c      | 6 ++----
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 8844a9a04fcf..f9f5b28cc72e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -1051,7 +1051,7 @@ static int fs_enet_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int fs_enet_remove(struct platform_device *ofdev)
+static void fs_enet_remove(struct platform_device *ofdev)
 {
 	struct net_device *ndev = platform_get_drvdata(ofdev);
 	struct fs_enet_private *fep = netdev_priv(ndev);
@@ -1066,7 +1066,6 @@ static int fs_enet_remove(struct platform_device *ofdev)
 	if (of_phy_is_fixed_link(ofdev->dev.of_node))
 		of_phy_deregister_fixed_link(ofdev->dev.of_node);
 	free_netdev(ndev);
-	return 0;
 }
 
 static const struct of_device_id fs_enet_match[] = {
@@ -1113,7 +1112,7 @@ static struct platform_driver fs_enet_driver = {
 		.of_match_table = fs_enet_match,
 	},
 	.probe = fs_enet_probe,
-	.remove = fs_enet_remove,
+	.remove_new = fs_enet_remove,
 };
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
index 21de56345503..91a69fc2f7c2 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
@@ -192,7 +192,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(ofdev);
 	struct bb_info *bitbang = bus->priv;
@@ -201,8 +201,6 @@ static int fs_enet_mdio_remove(struct platform_device *ofdev)
 	free_mdio_bitbang(bus);
 	iounmap(bitbang->dir);
 	kfree(bitbang);
-
-	return 0;
 }
 
 static const struct of_device_id fs_enet_mdio_bb_match[] = {
@@ -219,7 +217,7 @@ static struct platform_driver fs_enet_bb_mdio_driver = {
 		.of_match_table = fs_enet_mdio_bb_match,
 	},
 	.probe = fs_enet_mdio_probe,
-	.remove = fs_enet_mdio_remove,
+	.remove_new = fs_enet_mdio_remove,
 };
 
 module_platform_driver(fs_enet_bb_mdio_driver);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
index d37d7a19a759..94bd76c6cf9e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
@@ -187,7 +187,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(ofdev);
 	struct fec_info *fec = bus->priv;
@@ -196,8 +196,6 @@ static int fs_enet_mdio_remove(struct platform_device *ofdev)
 	iounmap(fec->fecp);
 	kfree(fec);
 	mdiobus_free(bus);
-
-	return 0;
 }
 
 static const struct of_device_id fs_enet_mdio_fec_match[] = {
@@ -220,7 +218,7 @@ static struct platform_driver fs_enet_fec_mdio_driver = {
 		.of_match_table = fs_enet_mdio_fec_match,
 	},
 	.probe = fs_enet_mdio_probe,
-	.remove = fs_enet_mdio_remove,
+	.remove_new = fs_enet_mdio_remove,
 };
 
 module_platform_driver(fs_enet_fec_mdio_driver);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 5/8] net: fs_enet: Convert to platform remove callback returning void
@ 2023-06-06 16:28   ` Uwe Kleine-König
  0 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Pantelis Antoniou, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, Michal Kubiak, linuxppc-dev, kernel

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++---
 drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c  | 6 ++----
 drivers/net/ethernet/freescale/fs_enet/mii-fec.c      | 6 ++----
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 8844a9a04fcf..f9f5b28cc72e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -1051,7 +1051,7 @@ static int fs_enet_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int fs_enet_remove(struct platform_device *ofdev)
+static void fs_enet_remove(struct platform_device *ofdev)
 {
 	struct net_device *ndev = platform_get_drvdata(ofdev);
 	struct fs_enet_private *fep = netdev_priv(ndev);
@@ -1066,7 +1066,6 @@ static int fs_enet_remove(struct platform_device *ofdev)
 	if (of_phy_is_fixed_link(ofdev->dev.of_node))
 		of_phy_deregister_fixed_link(ofdev->dev.of_node);
 	free_netdev(ndev);
-	return 0;
 }
 
 static const struct of_device_id fs_enet_match[] = {
@@ -1113,7 +1112,7 @@ static struct platform_driver fs_enet_driver = {
 		.of_match_table = fs_enet_match,
 	},
 	.probe = fs_enet_probe,
-	.remove = fs_enet_remove,
+	.remove_new = fs_enet_remove,
 };
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
index 21de56345503..91a69fc2f7c2 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
@@ -192,7 +192,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(ofdev);
 	struct bb_info *bitbang = bus->priv;
@@ -201,8 +201,6 @@ static int fs_enet_mdio_remove(struct platform_device *ofdev)
 	free_mdio_bitbang(bus);
 	iounmap(bitbang->dir);
 	kfree(bitbang);
-
-	return 0;
 }
 
 static const struct of_device_id fs_enet_mdio_bb_match[] = {
@@ -219,7 +217,7 @@ static struct platform_driver fs_enet_bb_mdio_driver = {
 		.of_match_table = fs_enet_mdio_bb_match,
 	},
 	.probe = fs_enet_mdio_probe,
-	.remove = fs_enet_mdio_remove,
+	.remove_new = fs_enet_mdio_remove,
 };
 
 module_platform_driver(fs_enet_bb_mdio_driver);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
index d37d7a19a759..94bd76c6cf9e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
@@ -187,7 +187,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
 {
 	struct mii_bus *bus = platform_get_drvdata(ofdev);
 	struct fec_info *fec = bus->priv;
@@ -196,8 +196,6 @@ static int fs_enet_mdio_remove(struct platform_device *ofdev)
 	iounmap(fec->fecp);
 	kfree(fec);
 	mdiobus_free(bus);
-
-	return 0;
 }
 
 static const struct of_device_id fs_enet_mdio_fec_match[] = {
@@ -220,7 +218,7 @@ static struct platform_driver fs_enet_fec_mdio_driver = {
 		.of_match_table = fs_enet_mdio_fec_match,
 	},
 	.probe = fs_enet_mdio_probe,
-	.remove = fs_enet_mdio_remove,
+	.remove_new = fs_enet_mdio_remove,
 };
 
 module_platform_driver(fs_enet_fec_mdio_driver);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 6/8] net: fsl_pq_mdio: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
                   ` (5 preceding siblings ...)
  (?)
@ 2023-06-06 16:28 ` Uwe Kleine-König
  2023-06-07 14:38   ` Simon Horman
  -1 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michal Kubiak
  Cc: netdev, kernel

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 9d58d8334467..01d594886f52 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -511,7 +511,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 }
 
 
-static int fsl_pq_mdio_remove(struct platform_device *pdev)
+static void fsl_pq_mdio_remove(struct platform_device *pdev)
 {
 	struct device *device = &pdev->dev;
 	struct mii_bus *bus = dev_get_drvdata(device);
@@ -521,8 +521,6 @@ static int fsl_pq_mdio_remove(struct platform_device *pdev)
 
 	iounmap(priv->map);
 	mdiobus_free(bus);
-
-	return 0;
 }
 
 static struct platform_driver fsl_pq_mdio_driver = {
@@ -531,7 +529,7 @@ static struct platform_driver fsl_pq_mdio_driver = {
 		.of_match_table = fsl_pq_mdio_match,
 	},
 	.probe = fsl_pq_mdio_probe,
-	.remove = fsl_pq_mdio_remove,
+	.remove_new = fsl_pq_mdio_remove,
 };
 
 module_platform_driver(fsl_pq_mdio_driver);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 7/8] net: gianfar: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
                   ` (6 preceding siblings ...)
  (?)
@ 2023-06-06 16:28 ` Uwe Kleine-König
  -1 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Claudiu Manoil, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, kernel, Michal Kubiak

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/gianfar.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 38d5013c6fed..0c898f9ee358 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -3364,7 +3364,7 @@ static int gfar_probe(struct platform_device *ofdev)
 	return err;
 }
 
-static int gfar_remove(struct platform_device *ofdev)
+static void gfar_remove(struct platform_device *ofdev)
 {
 	struct gfar_private *priv = platform_get_drvdata(ofdev);
 	struct device_node *np = ofdev->dev.of_node;
@@ -3381,8 +3381,6 @@ static int gfar_remove(struct platform_device *ofdev)
 	gfar_free_rx_queues(priv);
 	gfar_free_tx_queues(priv);
 	free_gfar_dev(priv);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM
@@ -3642,7 +3640,7 @@ static struct platform_driver gfar_driver = {
 		.of_match_table = gfar_match,
 	},
 	.probe = gfar_probe,
-	.remove = gfar_remove,
+	.remove_new = gfar_remove,
 };
 
 module_platform_driver(gfar_driver);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 8/8] net: ucc_geth: Convert to platform remove callback returning void
  2023-06-06 16:28 ` Uwe Kleine-König
@ 2023-06-06 16:28   ` Uwe Kleine-König
  -1 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Li Yang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: netdev, linuxppc-dev, kernel, Michal Kubiak

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 7a4cb4f07c32..2b3a15f24e7c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3753,7 +3753,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 	return err;
 }
 
-static int ucc_geth_remove(struct platform_device* ofdev)
+static void ucc_geth_remove(struct platform_device* ofdev)
 {
 	struct net_device *dev = platform_get_drvdata(ofdev);
 	struct ucc_geth_private *ugeth = netdev_priv(dev);
@@ -3767,8 +3767,6 @@ static int ucc_geth_remove(struct platform_device* ofdev)
 	of_node_put(ugeth->ug_info->phy_node);
 	kfree(ugeth->ug_info);
 	free_netdev(dev);
-
-	return 0;
 }
 
 static const struct of_device_id ucc_geth_match[] = {
@@ -3787,7 +3785,7 @@ static struct platform_driver ucc_geth_driver = {
 		.of_match_table = ucc_geth_match,
 	},
 	.probe		= ucc_geth_probe,
-	.remove		= ucc_geth_remove,
+	.remove_new	= ucc_geth_remove,
 	.suspend	= ucc_geth_suspend,
 	.resume		= ucc_geth_resume,
 };
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH net-next v2 8/8] net: ucc_geth: Convert to platform remove callback returning void
@ 2023-06-06 16:28   ` Uwe Kleine-König
  0 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-06 16:28 UTC (permalink / raw)
  To: Li Yang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Michal Kubiak, linuxppc-dev, kernel

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 7a4cb4f07c32..2b3a15f24e7c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3753,7 +3753,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 	return err;
 }
 
-static int ucc_geth_remove(struct platform_device* ofdev)
+static void ucc_geth_remove(struct platform_device* ofdev)
 {
 	struct net_device *dev = platform_get_drvdata(ofdev);
 	struct ucc_geth_private *ugeth = netdev_priv(dev);
@@ -3767,8 +3767,6 @@ static int ucc_geth_remove(struct platform_device* ofdev)
 	of_node_put(ugeth->ug_info->phy_node);
 	kfree(ugeth->ug_info);
 	free_netdev(dev);
-
-	return 0;
 }
 
 static const struct of_device_id ucc_geth_match[] = {
@@ -3787,7 +3785,7 @@ static struct platform_driver ucc_geth_driver = {
 		.of_match_table = ucc_geth_match,
 	},
 	.probe		= ucc_geth_probe,
-	.remove		= ucc_geth_remove,
+	.remove_new	= ucc_geth_remove,
 	.suspend	= ucc_geth_suspend,
 	.resume		= ucc_geth_resume,
 };
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 1/8] net: dpaa: Improve error reporting
  2023-06-06 16:28 ` [PATCH net-next v2 1/8] net: dpaa: Improve error reporting Uwe Kleine-König
@ 2023-06-06 17:03   ` Maciej Fijalkowski
  2023-06-07  6:42     ` Uwe Kleine-König
  0 siblings, 1 reply; 24+ messages in thread
From: Maciej Fijalkowski @ 2023-06-06 17:03 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Russell King, netdev, kernel, Madalin Bucur,
	Michal Kubiak

On Tue, Jun 06, 2023 at 06:28:22PM +0200, Uwe Kleine-König wrote:
> Instead of the generic error message emitted by the driver core when a
> remove callback returns an error code ("remove callback returned a
> non-zero value. This will be ignored."), emit a message describing the
> actual problem and return zero to suppress the generic message.
> 
> Note that apart from suppressing the generic error message there are no
> side effects by changing the return value to zero. This prepares
> changing the remove callback to return void.
> 
> Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index 431f8917dc39..6226c03cfca0 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -3516,6 +3516,8 @@ static int dpaa_remove(struct platform_device *pdev)
>  	phylink_destroy(priv->mac_dev->phylink);
>  
>  	err = dpaa_fq_free(dev, &priv->dpaa_fq_list);
> +	if (err)
> +		dev_err(dev, "Failed to free FQs on remove\n");

so 'err' is now redundant - if you don't have any value in printing this
out then you could remove this variable altogether.

>  
>  	qman_delete_cgr_safe(&priv->ingress_cgr);
>  	qman_release_cgrid(priv->ingress_cgr.cgrid);
> @@ -3528,7 +3530,7 @@ static int dpaa_remove(struct platform_device *pdev)
>  
>  	free_netdev(net_dev);
>  
> -	return err;
> +	return 0;
>  }
>  
>  static const struct platform_device_id dpaa_devtype[] = {
> -- 
> 2.39.2
> 
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 1/8] net: dpaa: Improve error reporting
  2023-06-06 17:03   ` Maciej Fijalkowski
@ 2023-06-07  6:42     ` Uwe Kleine-König
  2023-06-07  7:58       ` Russell King (Oracle)
  0 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2023-06-07  6:42 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Madalin Bucur, Madalin Bucur, netdev, Russell King, Eric Dumazet,
	Michal Kubiak, kernel, Jakub Kicinski, Paolo Abeni,
	David S. Miller

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

On Tue, Jun 06, 2023 at 07:03:31PM +0200, Maciej Fijalkowski wrote:
> On Tue, Jun 06, 2023 at 06:28:22PM +0200, Uwe Kleine-König wrote:
> > Instead of the generic error message emitted by the driver core when a
> > remove callback returns an error code ("remove callback returned a
> > non-zero value. This will be ignored."), emit a message describing the
> > actual problem and return zero to suppress the generic message.
> > 
> > Note that apart from suppressing the generic error message there are no
> > side effects by changing the return value to zero. This prepares
> > changing the remove callback to return void.
> > 
> > Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 431f8917dc39..6226c03cfca0 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -3516,6 +3516,8 @@ static int dpaa_remove(struct platform_device *pdev)
> >  	phylink_destroy(priv->mac_dev->phylink);
> >  
> >  	err = dpaa_fq_free(dev, &priv->dpaa_fq_list);
> > +	if (err)
> > +		dev_err(dev, "Failed to free FQs on remove\n");
> 
> so 'err' is now redundant - if you don't have any value in printing this
> out then you could remove this variable altogether.

Good hint, err can have different values here (at least -EINVAL and
-EBUSY), so printing it has some values. I'll wait a bit for more
feedback to this series and then prepare a v3 with

	dev_err(dev, "Failed to free FQs on remove (%pE)\n", err);

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH net-next v2 3/8] net: fec: Convert to platform remove callback returning void
  2023-06-06 16:28 ` [PATCH net-next v2 3/8] net: fec: " Uwe Kleine-König
@ 2023-06-07  6:48   ` Wei Fang
  2023-06-07 14:39   ` Simon Horman
  1 sibling, 0 replies; 24+ messages in thread
From: Wei Fang @ 2023-06-07  6:48 UTC (permalink / raw)
  To: Uwe Kleine-König, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Marc Kleine-Budde, Damien Le Moal,
	Michael Ellerman, Andy Shevchenko, Rob Herring
  Cc: Shenwei Wang, Clark Wang, dl-linux-imx, netdev, kernel, Michal Kubiak

> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Sent: 2023年6月7日 0:28
> To: Wei Fang <wei.fang@nxp.com>; David S. Miller <davem@davemloft.net>;
> Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Marc Kleine-Budde
> <mkl@pengutronix.de>; Damien Le Moal
> <damien.lemoal@opensource.wdc.com>; Michael Ellerman
> <mpe@ellerman.id.au>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Rob Herring <robh@kernel.org>
> Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> netdev@vger.kernel.org; kernel@pengutronix.de; Michal Kubiak
> <michal.kubiak@intel.com>
> Subject: [PATCH net-next v2 3/8] net: fec: Convert to platform remove callback
> returning void
> 
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/net/ethernet/freescale/fec_main.c        | 5 ++---
>  drivers/net/ethernet/freescale/fec_mpc52xx.c     | 6 ++----
>  drivers/net/ethernet/freescale/fec_mpc52xx_phy.c | 6 ++----
>  3 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 632bb4d589d7..f2b333b3f8c5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -4458,7 +4458,7 @@ fec_probe(struct platform_device *pdev)
>  	return ret;
>  }
> 
> -static int
> +static void
>  fec_drv_remove(struct platform_device *pdev)
>  {
>  	struct net_device *ndev = platform_get_drvdata(pdev);
> @@ -4494,7 +4494,6 @@ fec_drv_remove(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
> 
>  	free_netdev(ndev);
> -	return 0;
>  }
> 
>  static int __maybe_unused fec_suspend(struct device *dev)
> @@ -4650,7 +4649,7 @@ static struct platform_driver fec_driver = {
>  	},
>  	.id_table = fec_devtype,
>  	.probe	= fec_probe,
> -	.remove	= fec_drv_remove,
> +	.remove_new = fec_drv_remove,
>  };
> 
>  module_platform_driver(fec_driver);

Thank you!
Reviewed-by: Wei Fang <wei.fang@nxp.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 1/8] net: dpaa: Improve error reporting
  2023-06-07  6:42     ` Uwe Kleine-König
@ 2023-06-07  7:58       ` Russell King (Oracle)
  0 siblings, 0 replies; 24+ messages in thread
From: Russell King (Oracle) @ 2023-06-07  7:58 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Maciej Fijalkowski, Madalin Bucur, Madalin Bucur, netdev,
	Eric Dumazet, Michal Kubiak, kernel, Jakub Kicinski, Paolo Abeni,
	David S. Miller

On Wed, Jun 07, 2023 at 08:42:35AM +0200, Uwe Kleine-König wrote:
> On Tue, Jun 06, 2023 at 07:03:31PM +0200, Maciej Fijalkowski wrote:
> > On Tue, Jun 06, 2023 at 06:28:22PM +0200, Uwe Kleine-König wrote:
> > > Instead of the generic error message emitted by the driver core when a
> > > remove callback returns an error code ("remove callback returned a
> > > non-zero value. This will be ignored."), emit a message describing the
> > > actual problem and return zero to suppress the generic message.
> > > 
> > > Note that apart from suppressing the generic error message there are no
> > > side effects by changing the return value to zero. This prepares
> > > changing the remove callback to return void.
> > > 
> > > Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > > Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > index 431f8917dc39..6226c03cfca0 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > @@ -3516,6 +3516,8 @@ static int dpaa_remove(struct platform_device *pdev)
> > >  	phylink_destroy(priv->mac_dev->phylink);
> > >  
> > >  	err = dpaa_fq_free(dev, &priv->dpaa_fq_list);
> > > +	if (err)
> > > +		dev_err(dev, "Failed to free FQs on remove\n");
> > 
> > so 'err' is now redundant - if you don't have any value in printing this
> > out then you could remove this variable altogether.
> 
> Good hint, err can have different values here (at least -EINVAL and
> -EBUSY), so printing it has some values. I'll wait a bit for more
> feedback to this series and then prepare a v3 with
> 
> 	dev_err(dev, "Failed to free FQs on remove (%pE)\n", err);

							     ERR_PTR(err)

%p formats always take a pointer.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 8/8] net: ucc_geth: Convert to platform remove callback returning void
  2023-06-06 16:28   ` Uwe Kleine-König
@ 2023-06-07 14:36     ` Simon Horman
  -1 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:36 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Li Yang, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linuxppc-dev, kernel, Michal Kubiak

On Tue, Jun 06, 2023 at 06:28:29PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 8/8] net: ucc_geth: Convert to platform remove callback returning void
@ 2023-06-07 14:36     ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:36 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: netdev, Li Yang, Eric Dumazet, Michal Kubiak, kernel,
	Jakub Kicinski, Paolo Abeni, linuxppc-dev, David S. Miller

On Tue, Jun 06, 2023 at 06:28:29PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 2/8] net: dpaa: Convert to platform remove callback returning void
  2023-06-06 16:28 ` [PATCH net-next v2 2/8] net: dpaa: Convert to platform remove callback returning void Uwe Kleine-König
@ 2023-06-07 14:37   ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:37 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, kernel, Madalin Bucur, Michal Kubiak

On Tue, Jun 06, 2023 at 06:28:23PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 4/8] net: fman: Convert to platform remove callback returning void
  2023-06-06 16:28 ` [PATCH net-next v2 4/8] net: fman: " Uwe Kleine-König
@ 2023-06-07 14:38   ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:38 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Madalin Bucur, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Sean Anderson, netdev, kernel, Madalin Bucur,
	Michal Kubiak

On Tue, Jun 06, 2023 at 06:28:25PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 6/8] net: fsl_pq_mdio: Convert to platform remove callback returning void
  2023-06-06 16:28 ` [PATCH net-next v2 6/8] net: fsl_pq_mdio: " Uwe Kleine-König
@ 2023-06-07 14:38   ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:38 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michal Kubiak, netdev, kernel

On Tue, Jun 06, 2023 at 06:28:27PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 5/8] net: fs_enet: Convert to platform remove callback returning void
  2023-06-06 16:28   ` Uwe Kleine-König
@ 2023-06-07 14:38     ` Simon Horman
  -1 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:38 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Pantelis Antoniou, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linuxppc-dev, netdev, kernel, Michal Kubiak

On Tue, Jun 06, 2023 at 06:28:26PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 5/8] net: fs_enet: Convert to platform remove callback returning void
@ 2023-06-07 14:38     ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:38 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: netdev, Eric Dumazet, Michal Kubiak, kernel, Jakub Kicinski,
	Paolo Abeni, linuxppc-dev, David S. Miller

On Tue, Jun 06, 2023 at 06:28:26PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH net-next v2 3/8] net: fec: Convert to platform remove callback returning void
  2023-06-06 16:28 ` [PATCH net-next v2 3/8] net: fec: " Uwe Kleine-König
  2023-06-07  6:48   ` Wei Fang
@ 2023-06-07 14:39   ` Simon Horman
  1 sibling, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-06-07 14:39 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wei Fang, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Marc Kleine-Budde, Damien Le Moal, Michael Ellerman,
	Andy Shevchenko, Rob Herring, Shenwei Wang, Clark Wang,
	NXP Linux Team, netdev, kernel, Michal Kubiak

On Tue, Jun 06, 2023 at 06:28:24PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-06-07 14:40 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-06 16:28 [PATCH net-next v2 0/8] net: freescale: Convert to platform remove callback returning void Uwe Kleine-König
2023-06-06 16:28 ` Uwe Kleine-König
2023-06-06 16:28 ` [PATCH net-next v2 1/8] net: dpaa: Improve error reporting Uwe Kleine-König
2023-06-06 17:03   ` Maciej Fijalkowski
2023-06-07  6:42     ` Uwe Kleine-König
2023-06-07  7:58       ` Russell King (Oracle)
2023-06-06 16:28 ` [PATCH net-next v2 2/8] net: dpaa: Convert to platform remove callback returning void Uwe Kleine-König
2023-06-07 14:37   ` Simon Horman
2023-06-06 16:28 ` [PATCH net-next v2 3/8] net: fec: " Uwe Kleine-König
2023-06-07  6:48   ` Wei Fang
2023-06-07 14:39   ` Simon Horman
2023-06-06 16:28 ` [PATCH net-next v2 4/8] net: fman: " Uwe Kleine-König
2023-06-07 14:38   ` Simon Horman
2023-06-06 16:28 ` [PATCH net-next v2 5/8] net: fs_enet: " Uwe Kleine-König
2023-06-06 16:28   ` Uwe Kleine-König
2023-06-07 14:38   ` Simon Horman
2023-06-07 14:38     ` Simon Horman
2023-06-06 16:28 ` [PATCH net-next v2 6/8] net: fsl_pq_mdio: " Uwe Kleine-König
2023-06-07 14:38   ` Simon Horman
2023-06-06 16:28 ` [PATCH net-next v2 7/8] net: gianfar: " Uwe Kleine-König
2023-06-06 16:28 ` [PATCH net-next v2 8/8] net: ucc_geth: " Uwe Kleine-König
2023-06-06 16:28   ` Uwe Kleine-König
2023-06-07 14:36   ` Simon Horman
2023-06-07 14:36     ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.