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

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

[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: use 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 | 84 ++++++---------------------
 1 file changed, 19 insertions(+), 65 deletions(-)

-- 
2.46.0

[PATCH net-next 1/3] net: ag71xx: use 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 | 31 ++++++---------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 6fc4996c8131..c22ebd3c1f46 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -691,7 +691,7 @@ 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);
@@ -704,16 +704,13 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	}
 
 	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 +732,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 +1837,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 +1917,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 +1948,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 +1962,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 2/3] net: ag71xx: use devm for of_mdiobus_register

[Rosen Penev]

Allows removing ag71xx_mdio_remove.

Removed local mii_bus variable and assign struct members directly.
Easier to reason about.

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

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index c22ebd3c1f46..1bc882fc1388 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -684,12 +684,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 {
 	struct device *dev = &ag->pdev->dev;
 	struct net_device *ndev = ag->ndev;
-	static struct mii_bus *mii_bus;
 	struct device_node *np, *mnp;
 	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)) {
@@ -703,7 +701,7 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 		return err;
 	}
 
-	mii_bus = devm_mdiobus_alloc(dev);
+	ag->mii_bus = devm_mdiobus_alloc(dev);
 	if (!mii_bus)
 		return -ENOMEM;
 
@@ -713,13 +711,13 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 		return PTR_ERR(ag->mdio_reset);
 	}
 
-	mii_bus->name = "ag71xx_mdio";
-	mii_bus->read = ag71xx_mdio_mii_read;
-	mii_bus->write = ag71xx_mdio_mii_write;
-	mii_bus->reset = ag71xx_mdio_reset;
-	mii_bus->priv = ag;
-	mii_bus->parent = dev;
-	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s.%d", np->name, ag->mac_idx);
+	ag->mii_bus->name = "ag71xx_mdio";
+	ag->mii_bus->read = ag71xx_mdio_mii_read;
+	ag->mii_bus->write = ag71xx_mdio_mii_write;
+	ag->mii_bus->reset = ag71xx_mdio_reset;
+	ag->mii_bus->priv = ag;
+	ag->mii_bus->parent = dev;
+	snprintf(ag->mii_bus->id, MII_BUS_ID_SIZE, "%s.%d", np->name, ag->mac_idx);
 
 	if (!IS_ERR(ag->mdio_reset)) {
 		reset_control_assert(ag->mdio_reset);
@@ -729,22 +727,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(ag->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 */
@@ -1930,14 +1920,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",
@@ -1945,23 +1935,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 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 1bc882fc1388..43f0e2cf987f 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -1923,7 +1923,7 @@ static int ag71xx_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = register_netdev(ndev);
+	err = devm_register_netdev(ndev);
 	if (err) {
 		netif_err(ag, probe, ndev, "unable to register net device\n");
 		platform_set_drvdata(pdev, NULL);
@@ -1937,17 +1937,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,
 };
@@ -2032,7 +2021,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 1/3] net: ag71xx: use devm_clk_get_enabled

[Daniel Golle]

On Mon, Aug 12, 2024 at 12:06:51PM -0700, Rosen Penev wrote:
> 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>

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

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

[Daniel Golle]

On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> Allows removing ag71xx_mdio_remove.
> 
> Removed local mii_bus variable and assign struct members directly.
> Easier to reason about.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

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

[Daniel Golle]

On Mon, Aug 12, 2024 at 12:06:53PM -0700, Rosen Penev wrote:
> 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>

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

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

[Rosen Penev]

On Mon, Aug 12, 2024 at 12:07 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> 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 | 31 ++++++---------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index 6fc4996c8131..c22ebd3c1f46 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -691,7 +691,7 @@ 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");
I forgot to remove the following section. Will do this in v2. I'll
send tomorrow as rules seem to be no new patches for 24 hours.
>         if (IS_ERR(ag->clk_mdio)) {
>                 netif_err(ag, probe, ndev, "Failed to get mdio clk.\n");
>                 return PTR_ERR(ag->clk_mdio);
> @@ -704,16 +704,13 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
>         }
>
>         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 +732,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 +1837,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 +1917,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 +1948,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 +1962,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
>

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

[Andrew Lunn]

On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> Allows removing ag71xx_mdio_remove.
> 
> Removed local mii_bus variable and assign struct members directly.
> Easier to reason about.

This mixes up two different things, making the patch harder to
review. Ideally you want lots of little patches, each doing one thing,
and being obviously correct. 

Is ag->mii_bus actually used anywhere, outside of ag71xx_mdio_probe()?
Often swapping to devm_ means the driver does not need to keep hold of
the resources. So i actually think you can remove ag->mii_bus. This
might of been more obvious if you had first swapped to
devm_of_mdiobus_register() without the other changes mixed in.

    Andrew

---
pw-bot: cr

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

[Rosen Penev]

On Mon, Aug 12, 2024 at 2:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> > Allows removing ag71xx_mdio_remove.
> >
> > Removed local mii_bus variable and assign struct members directly.
> > Easier to reason about.
>
> This mixes up two different things, making the patch harder to
> review. Ideally you want lots of little patches, each doing one thing,
> and being obviously correct.
>
> Is ag->mii_bus actually used anywhere, outside of ag71xx_mdio_probe()?
> Often swapping to devm_ means the driver does not need to keep hold of
> the resources. So i actually think you can remove ag->mii_bus. This
> might of been more obvious if you had first swapped to
> devm_of_mdiobus_register() without the other changes mixed in.
not sure I follow. mdiobus_unregister would need to be called in
remove without devm. That would need a private mii_bus of some kind.
So with devm this is unneeded?
>
>     Andrew
>
> ---
> pw-bot: cr

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

[Andrew Lunn]

On Mon, Aug 12, 2024 at 02:35:45PM -0700, Rosen Penev wrote:
> On Mon, Aug 12, 2024 at 2:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> > > Allows removing ag71xx_mdio_remove.
> > >
> > > Removed local mii_bus variable and assign struct members directly.
> > > Easier to reason about.
> >
> > This mixes up two different things, making the patch harder to
> > review. Ideally you want lots of little patches, each doing one thing,
> > and being obviously correct.
> >
> > Is ag->mii_bus actually used anywhere, outside of ag71xx_mdio_probe()?
> > Often swapping to devm_ means the driver does not need to keep hold of
> > the resources. So i actually think you can remove ag->mii_bus. This
> > might of been more obvious if you had first swapped to
> > devm_of_mdiobus_register() without the other changes mixed in.
> not sure I follow. mdiobus_unregister would need to be called in
> remove without devm. That would need a private mii_bus of some kind.
> So with devm this is unneeded?

If you use devm_of_mdiobus_register(), the device core will call
devm_mdiobus_unregister() on remove. Your patch removed
mdiobus_unregister() in remove....

Is there any user of ag->mii_bus left after converting to
devm_of_mdiobus_register()?

	Andrew

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

[Rosen Penev]

On Mon, Aug 12, 2024 at 2:35 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> On Mon, Aug 12, 2024 at 2:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> > > Allows removing ag71xx_mdio_remove.
> > >
> > > Removed local mii_bus variable and assign struct members directly.
> > > Easier to reason about.
> >
> > This mixes up two different things, making the patch harder to
> > review. Ideally you want lots of little patches, each doing one thing,
> > and being obviously correct.
> >
> > Is ag->mii_bus actually used anywhere, outside of ag71xx_mdio_probe()?
> > Often swapping to devm_ means the driver does not need to keep hold of
> > the resources. So i actually think you can remove ag->mii_bus. This
> > might of been more obvious if you had first swapped to
> > devm_of_mdiobus_register() without the other changes mixed in.
> not sure I follow. mdiobus_unregister would need to be called in
> remove without devm. That would need a private mii_bus of some kind.
> So with devm this is unneeded?
Just checked drivers/net/dsa/lantiq_gswip.c

This seems correct. Will make the change for v2.
> >
> >     Andrew
> >
> > ---
> > pw-bot: cr

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

[Rosen Penev]

On Mon, Aug 12, 2024 at 7:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Aug 12, 2024 at 02:35:45PM -0700, Rosen Penev wrote:
> > On Mon, Aug 12, 2024 at 2:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> > > > Allows removing ag71xx_mdio_remove.
> > > >
> > > > Removed local mii_bus variable and assign struct members directly.
> > > > Easier to reason about.
> > >
> > > This mixes up two different things, making the patch harder to
> > > review. Ideally you want lots of little patches, each doing one thing,
> > > and being obviously correct.
> > >
> > > Is ag->mii_bus actually used anywhere, outside of ag71xx_mdio_probe()?
> > > Often swapping to devm_ means the driver does not need to keep hold of
> > > the resources. So i actually think you can remove ag->mii_bus. This
> > > might of been more obvious if you had first swapped to
> > > devm_of_mdiobus_register() without the other changes mixed in.
> > not sure I follow. mdiobus_unregister would need to be called in
> > remove without devm. That would need a private mii_bus of some kind.
> > So with devm this is unneeded?
>
> If you use devm_of_mdiobus_register(), the device core will call
> devm_mdiobus_unregister() on remove. Your patch removed
> mdiobus_unregister() in remove....
>
> Is there any user of ag->mii_bus left after converting to
> devm_of_mdiobus_register()?
There is not. I've applied the change removing mii_bus from ag locally.

From testing, nothing has blown up. Although there's still a problem
with switched ports (except for lan1) dying after a while. I think
that bug's in qca8k though.

calling restart results in no surprises, unlike with some of my other
questionable patches.
>
>         Andrew

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

[Simon Horman]

On Mon, Aug 12, 2024 at 12:06:52PM -0700, Rosen Penev wrote:
> Allows removing ag71xx_mdio_remove.
> 
> Removed local mii_bus variable and assign struct members directly.
> Easier to reason about.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/ethernet/atheros/ag71xx.c | 39 ++++++++-------------------
>  1 file changed, 11 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index c22ebd3c1f46..1bc882fc1388 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -684,12 +684,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
>  {
>  	struct device *dev = &ag->pdev->dev;
>  	struct net_device *ndev = ag->ndev;
> -	static struct mii_bus *mii_bus;
>  	struct device_node *np, *mnp;
>  	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)) {
> @@ -703,7 +701,7 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
>  		return err;
>  	}
>  
> -	mii_bus = devm_mdiobus_alloc(dev);
> +	ag->mii_bus = devm_mdiobus_alloc(dev);
>  	if (!mii_bus)

Above mii_bus is removed, but here it is used.

>  		return -ENOMEM;
>  

...