[PATCH net-next v2 0/3] use more devm for ag71xx

[PATCH net-next v2 0/3] use more devm for ag71xx

[Rosen Penev]

Some of these were introduced after the driver got introduced. In any
case, using more devm allows removal of the remove function and overall
simplifies the code. All of these were tested on a TP-LINK Archer C7v2.

Rosen Penev (3):
  net: ag71xx: devm_clk_get_enabled
  net: ag71xx: use devm for of_mdiobus_register
  net: ag71xx: use devm for register_netdev

 drivers/net/ethernet/atheros/ag71xx.c | 74 ++++-----------------------
 1 file changed, 11 insertions(+), 63 deletions(-)

-- 
2.46.0

[PATCH net-next v2 1/3] net: ag71xx: devm_clk_get_enabled

[Rosen Penev]

Allows removal of clk_prepare_enable to simplify the code slightly.

Tested on a TP-LINK Archer C7v2.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/atheros/ag71xx.c | 37 +++++----------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 6fc4996c8131..e46d10a5c28c 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -691,29 +691,20 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	np = dev->of_node;
 	ag->mii_bus = NULL;
 
-	ag->clk_mdio = devm_clk_get(dev, "mdio");
+	ag->clk_mdio = devm_clk_get_enabled(dev, "mdio");
 	if (IS_ERR(ag->clk_mdio)) {
 		netif_err(ag, probe, ndev, "Failed to get mdio clk.\n");
 		return PTR_ERR(ag->clk_mdio);
 	}
 
-	err = clk_prepare_enable(ag->clk_mdio);
-	if (err) {
-		netif_err(ag, probe, ndev, "Failed to enable mdio clk.\n");
-		return err;
-	}
-
 	mii_bus = devm_mdiobus_alloc(dev);
-	if (!mii_bus) {
-		err = -ENOMEM;
-		goto mdio_err_put_clk;
-	}
+	if (!mii_bus)
+		return -ENOMEM;
 
 	ag->mdio_reset = of_reset_control_get_exclusive(np, "mdio");
 	if (IS_ERR(ag->mdio_reset)) {
 		netif_err(ag, probe, ndev, "Failed to get reset mdio.\n");
-		err = PTR_ERR(ag->mdio_reset);
-		goto mdio_err_put_clk;
+		return PTR_ERR(ag->mdio_reset);
 	}
 
 	mii_bus->name = "ag71xx_mdio";
@@ -735,22 +726,17 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	err = of_mdiobus_register(mii_bus, mnp);
 	of_node_put(mnp);
 	if (err)
-		goto mdio_err_put_clk;
+		return err;
 
 	ag->mii_bus = mii_bus;
 
 	return 0;
-
-mdio_err_put_clk:
-	clk_disable_unprepare(ag->clk_mdio);
-	return err;
 }
 
 static void ag71xx_mdio_remove(struct ag71xx *ag)
 {
 	if (ag->mii_bus)
 		mdiobus_unregister(ag->mii_bus);
-	clk_disable_unprepare(ag->clk_mdio);
 }
 
 static void ag71xx_hw_stop(struct ag71xx *ag)
@@ -1845,7 +1831,7 @@ static int ag71xx_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	ag->clk_eth = devm_clk_get(&pdev->dev, "eth");
+	ag->clk_eth = devm_clk_get_enabled(&pdev->dev, "eth");
 	if (IS_ERR(ag->clk_eth)) {
 		netif_err(ag, probe, ndev, "Failed to get eth clk.\n");
 		return PTR_ERR(ag->clk_eth);
@@ -1925,19 +1911,13 @@ static int ag71xx_probe(struct platform_device *pdev)
 	netif_napi_add_weight(ndev, &ag->napi, ag71xx_poll,
 			      AG71XX_NAPI_WEIGHT);
 
-	err = clk_prepare_enable(ag->clk_eth);
-	if (err) {
-		netif_err(ag, probe, ndev, "Failed to enable eth clk.\n");
-		return err;
-	}
-
 	ag71xx_wr(ag, AG71XX_REG_MAC_CFG1, 0);
 
 	ag71xx_hw_init(ag);
 
 	err = ag71xx_mdio_probe(ag);
 	if (err)
-		goto err_put_clk;
+		return err;
 
 	platform_set_drvdata(pdev, ndev);
 
@@ -1962,8 +1942,6 @@ static int ag71xx_probe(struct platform_device *pdev)
 
 err_mdio_remove:
 	ag71xx_mdio_remove(ag);
-err_put_clk:
-	clk_disable_unprepare(ag->clk_eth);
 	return err;
 }
 
@@ -1978,7 +1956,6 @@ static void ag71xx_remove(struct platform_device *pdev)
 	ag = netdev_priv(ndev);
 	unregister_netdev(ndev);
 	ag71xx_mdio_remove(ag);
-	clk_disable_unprepare(ag->clk_eth);
 	platform_set_drvdata(pdev, NULL);
 }
 
-- 
2.46.0

[PATCH net-next v2 2/3] net: ag71xx: use devm for of_mdiobus_register

[Rosen Penev]

Allows removing ag71xx_mdio_remove.

Removed ag.mii_bus variable. Local one can be used with devm. Easier to
reason about as mii_bus is only used here now. Also shrinks the struct
slightly.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/atheros/ag71xx.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index e46d10a5c28c..3c16f6c5e75c 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -380,7 +380,6 @@ struct ag71xx {
 	int mac_idx;
 
 	struct reset_control *mdio_reset;
-	struct mii_bus *mii_bus;
 	struct clk *clk_mdio;
 	struct clk *clk_eth;
 };
@@ -689,7 +688,6 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	int err;
 
 	np = dev->of_node;
-	ag->mii_bus = NULL;
 
 	ag->clk_mdio = devm_clk_get_enabled(dev, "mdio");
 	if (IS_ERR(ag->clk_mdio)) {
@@ -723,22 +721,14 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	}
 
 	mnp = of_get_child_by_name(np, "mdio");
-	err = of_mdiobus_register(mii_bus, mnp);
+	err = devm_of_mdiobus_register(dev, mii_bus, mnp);
 	of_node_put(mnp);
 	if (err)
 		return err;
 
-	ag->mii_bus = mii_bus;
-
 	return 0;
 }
 
-static void ag71xx_mdio_remove(struct ag71xx *ag)
-{
-	if (ag->mii_bus)
-		mdiobus_unregister(ag->mii_bus);
-}
-
 static void ag71xx_hw_stop(struct ag71xx *ag)
 {
 	/* disable all interrupts and stop the rx/tx engine */
@@ -1924,14 +1914,14 @@ static int ag71xx_probe(struct platform_device *pdev)
 	err = ag71xx_phylink_setup(ag);
 	if (err) {
 		netif_err(ag, probe, ndev, "failed to setup phylink (%d)\n", err);
-		goto err_mdio_remove;
+		return err;
 	}
 
 	err = register_netdev(ndev);
 	if (err) {
 		netif_err(ag, probe, ndev, "unable to register net device\n");
 		platform_set_drvdata(pdev, NULL);
-		goto err_mdio_remove;
+		return err;
 	}
 
 	netif_info(ag, probe, ndev, "Atheros AG71xx at 0x%08lx, irq %d, mode:%s\n",
@@ -1939,23 +1929,16 @@ static int ag71xx_probe(struct platform_device *pdev)
 		   phy_modes(ag->phy_if_mode));
 
 	return 0;
-
-err_mdio_remove:
-	ag71xx_mdio_remove(ag);
-	return err;
 }
 
 static void ag71xx_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct ag71xx *ag;
 
 	if (!ndev)
 		return;
 
-	ag = netdev_priv(ndev);
 	unregister_netdev(ndev);
-	ag71xx_mdio_remove(ag);
 	platform_set_drvdata(pdev, NULL);
 }
 
-- 
2.46.0

[PATCH net-next v2 3/3] net: ag71xx: use devm for register_netdev

[Rosen Penev]

Allows completely removing the remove function. Nothing is being done
manually now.

Tested on TP-LINK Archer C7v2.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/atheros/ag71xx.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 3c16f6c5e75c..b74856760be3 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -1917,7 +1917,7 @@ static int ag71xx_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = register_netdev(ndev);
+	err = devm_register_netdev(&pdev->dev, ndev);
 	if (err) {
 		netif_err(ag, probe, ndev, "unable to register net device\n");
 		platform_set_drvdata(pdev, NULL);
@@ -1931,17 +1931,6 @@ static int ag71xx_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static void ag71xx_remove(struct platform_device *pdev)
-{
-	struct net_device *ndev = platform_get_drvdata(pdev);
-
-	if (!ndev)
-		return;
-
-	unregister_netdev(ndev);
-	platform_set_drvdata(pdev, NULL);
-}
-
 static const u32 ar71xx_fifo_ar7100[] = {
 	0x0fff0000, 0x00001fff, 0x00780fff,
 };
@@ -2026,7 +2015,6 @@ static const struct of_device_id ag71xx_match[] = {
 
 static struct platform_driver ag71xx_driver = {
 	.probe		= ag71xx_probe,
-	.remove_new	= ag71xx_remove,
 	.driver = {
 		.name	= "ag71xx",
 		.of_match_table = ag71xx_match,
-- 
2.46.0

Re: [PATCH net-next v2 0/3] use more devm for ag71xx

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 13 Aug 2024 10:04:56 -0700 you wrote:
> Some of these were introduced after the driver got introduced. In any
> case, using more devm allows removal of the remove function and overall
> simplifies the code. All of these were tested on a TP-LINK Archer C7v2.
> 
> Rosen Penev (3):
>   net: ag71xx: devm_clk_get_enabled
>   net: ag71xx: use devm for of_mdiobus_register
>   net: ag71xx: use devm for register_netdev
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] net: ag71xx: devm_clk_get_enabled
    https://git.kernel.org/netdev/net-next/c/df37fcf58f2a
  - [net-next,v2,2/3] net: ag71xx: use devm for of_mdiobus_register
    https://git.kernel.org/netdev/net-next/c/8ef34bea8cad
  - [net-next,v2,3/3] net: ag71xx: use devm for register_netdev
    https://git.kernel.org/netdev/net-next/c/cc20a4791641

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html