[PATCHv3 net-next] net: ag71xx: get reset control using devm api

[PATCHv3 net-next] net: ag71xx: get reset control using devm api

[Rosen Penev]

Currently, the of variant is missing reset_control_put in error paths.
The devm variant does not require it.

Allows removing mdio_reset from the struct as it is not used outside the
function.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: don't call after ag71xx_mdio_probe. Already done.
 v3: use devm instead.
 drivers/net/ethernet/atheros/ag71xx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 89cd001b385f..d81aa0ccd572 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -379,7 +379,6 @@ struct ag71xx {
 	u32 fifodata[3];
 	int mac_idx;
 
-	struct reset_control *mdio_reset;
 	struct clk *clk_mdio;
 };
 
@@ -683,6 +682,7 @@ 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 reset_control *mdio_reset;
 	struct device_node *np, *mnp;
 	int err;
 
@@ -698,10 +698,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	if (!mii_bus)
 		return -ENOMEM;
 
-	ag->mdio_reset = of_reset_control_get_exclusive(np, "mdio");
-	if (IS_ERR(ag->mdio_reset)) {
+	mdio_reset = devm_reset_control_get_exclusive(dev, "mdio");
+	if (IS_ERR(mdio_reset)) {
 		netif_err(ag, probe, ndev, "Failed to get reset mdio.\n");
-		return PTR_ERR(ag->mdio_reset);
+		return PTR_ERR(mdio_reset);
 	}
 
 	mii_bus->name = "ag71xx_mdio";
@@ -712,10 +712,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag)
 	mii_bus->parent = dev;
 	snprintf(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);
+	if (!IS_ERR(mdio_reset)) {
+		reset_control_assert(mdio_reset);
 		msleep(100);
-		reset_control_deassert(ag->mdio_reset);
+		reset_control_deassert(mdio_reset);
 		msleep(200);
 	}
 
-- 
2.46.0

Re: [PATCHv3 net-next] net: ag71xx: get reset control using devm api

[Oleksij Rempel]

On Sat, Aug 24, 2024 at 11:18:56AM -0700, Rosen Penev wrote:
> Currently, the of variant is missing reset_control_put in error paths.
> The devm variant does not require it.
> 
> Allows removing mdio_reset from the struct as it is not used outside the
> function.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
 
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

Thank you!
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCHv3 net-next] net: ag71xx: get reset control using devm api

[Jakub Kicinski]

On Sat, 24 Aug 2024 11:18:56 -0700 Rosen Penev wrote:
> -	struct reset_control *mdio_reset;
>  	struct clk *clk_mdio;
>  };

If you send multiple patches which depend on each other they must be
part of one series. I'll apply the clk_eth patch shortly but you gotta
resend this one, our CI couldn't apply and test it.
-- 
pw-bot: cr

Re: [PATCHv3 net-next] net: ag71xx: get reset control using devm api

[Rosen Penev]

On Mon, Aug 26, 2024 at 9:59 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 24 Aug 2024 11:18:56 -0700 Rosen Penev wrote:
> > -     struct reset_control *mdio_reset;
> >       struct clk *clk_mdio;
> >  };
>
> If you send multiple patches which depend on each other they must be
> part of one series. I'll apply the clk_eth patch shortly but you gotta
> resend this one, our CI couldn't apply and test it.
Isn't the CI x86 only?
> --
> pw-bot: cr
>

Re: [PATCHv3 net-next] net: ag71xx: get reset control using devm api

[Andrew Lunn]

On Mon, Aug 26, 2024 at 12:26:30PM -0700, Rosen Penev wrote:
> On Mon, Aug 26, 2024 at 9:59 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sat, 24 Aug 2024 11:18:56 -0700 Rosen Penev wrote:
> > > -     struct reset_control *mdio_reset;
> > >       struct clk *clk_mdio;
> > >  };
> >
> > If you send multiple patches which depend on each other they must be
> > part of one series. I'll apply the clk_eth patch shortly but you gotta
> > resend this one, our CI couldn't apply and test it.
> Isn't the CI x86 only?

The CI also does more than X86 builds. It checks if you have Cc: all
the needed maintainer, is the patch checkpatch clean, if it is for
stable, have you Cc: stable etc. We want all these things which are
architecture independent to run, but they failed at the very first
step, applying the patch.

	Andrew