Re: [PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths
On Wed, Nov 08, 2017 at 01:22:57PM +0900, David Miller wrote: > From: Michael Grzeschik> Date: Mon, 6 Nov 2017 12:10:04 +0100 > > > We add the call of_phy_deregister_fixed_link to all associated > > error paths for memory clean up. > > > > Signed-off-by: Michael Grzeschik > > --- > > drivers/net/ethernet/cadence/macb_main.c | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/net/ethernet/cadence/macb_main.c > > b/drivers/net/ethernet/cadence/macb_main.c > > index 6df2cad61647a..2c2acd011329a 100644 > > --- a/drivers/net/ethernet/cadence/macb_main.c > > +++ b/drivers/net/ethernet/cadence/macb_main.c > > @@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp) > > err_out_unregister_bus: > > mdiobus_unregister(bp->mii_bus); > > err_out_free_mdiobus: > > + if ((np) && (of_phy_is_fixed_link(np))) > > Please don't use so many parenthesis in your conditionals: > > if (np && of_phy_is_fixed_link(np)) > > is more than sufficient. > > Please fix this in your entire set of patches. There are only two patches and not even in one series. I will resend them both with this one fixed and create a series. As the second one depends on this one. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: PGP signature
Re: [PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths
From: Michael GrzeschikDate: Mon, 6 Nov 2017 12:10:04 +0100 > We add the call of_phy_deregister_fixed_link to all associated > error paths for memory clean up. > > Signed-off-by: Michael Grzeschik > --- > drivers/net/ethernet/cadence/macb_main.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c > b/drivers/net/ethernet/cadence/macb_main.c > index 6df2cad61647a..2c2acd011329a 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp) > err_out_unregister_bus: > mdiobus_unregister(bp->mii_bus); > err_out_free_mdiobus: > + if ((np) && (of_phy_is_fixed_link(np))) Please don't use so many parenthesis in your conditionals: if (np && of_phy_is_fixed_link(np)) is more than sufficient. Please fix this in your entire set of patches. Thank you.
Re: [PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths
On 06/11/2017 at 12:10, Michael Grzeschik wrote: > We add the call of_phy_deregister_fixed_link to all associated > error paths for memory clean up. > > Signed-off-by: Michael GrzeschikAcked-by: Nicolas Ferre Thanks a lot for your quick answer! Best regards, Nicolas > --- > drivers/net/ethernet/cadence/macb_main.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c > b/drivers/net/ethernet/cadence/macb_main.c > index 6df2cad61647a..2c2acd011329a 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp) > err_out_unregister_bus: > mdiobus_unregister(bp->mii_bus); > err_out_free_mdiobus: > + if ((np) && (of_phy_is_fixed_link(np))) > + of_phy_deregister_fixed_link(np); > mdiobus_free(bp->mii_bus); > err_out: > return err; > @@ -3552,6 +3554,8 @@ static int macb_probe(struct platform_device *pdev) > err_out_unregister_mdio: > phy_disconnect(dev->phydev); > mdiobus_unregister(bp->mii_bus); > + if ((np) && (of_phy_is_fixed_link(np))) > + of_phy_deregister_fixed_link(np); > mdiobus_free(bp->mii_bus); > > /* Shutdown the PHY if there is a GPIO reset */ > @@ -3574,6 +3578,7 @@ static int macb_remove(struct platform_device *pdev) > { > struct net_device *dev; > struct macb *bp; > + struct device_node *np = pdev->dev.of_node; > > dev = platform_get_drvdata(pdev); > > @@ -3582,6 +3587,8 @@ static int macb_remove(struct platform_device *pdev) > if (dev->phydev) > phy_disconnect(dev->phydev); > mdiobus_unregister(bp->mii_bus); > + if ((np) && (of_phy_is_fixed_link(np))) > + of_phy_deregister_fixed_link(np); > dev->phydev = NULL; > mdiobus_free(bp->mii_bus); > > -- Nicolas Ferre