* [PATCH] net: gianfar: fix reference counting for phy_node @ 2014-08-07 19:35 Uwe Kleine-König 2014-08-07 19:41 ` [PATCH v2] " Uwe Kleine-König 0 siblings, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2014-08-07 19:35 UTC (permalink / raw) To: netdev; +Cc: Claudiu Manoil, kernel, Florian Fainelli, Fabio Estevam The line before the changed if condition is: priv->phy_node = of_parse_phandle(np, "phy-handle", 0); . If this call succeeds priv->phy_node must not be overwritten in the if block; otherwise the reference to the node returned by of_parse_phandle is lost. So add a check that the if block isn't executed in this case. Furthermore in the fixed phy case no reference is aquired for phy_node resulting in an of_node_put without holding a reference. To fix that, get a reference on the MAC dt node. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/net/ethernet/freescale/gianfar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 6b0c4775cd0d..adc7c718c4bc 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -897,7 +897,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) if (err) goto err_grp_init; - priv->phy_node = np; + priv->phy_node = of_node_get(np); } /* Find the TBI PHY. If it's not there, we don't support SGMII */ -- 2.0.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] net: gianfar: fix reference counting for phy_node 2014-08-07 19:35 [PATCH] net: gianfar: fix reference counting for phy_node Uwe Kleine-König @ 2014-08-07 19:41 ` Uwe Kleine-König 2014-08-07 20:01 ` [PATCH v3] " Uwe Kleine-König 0 siblings, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2014-08-07 19:41 UTC (permalink / raw) To: netdev Cc: Claudiu Manoil, kernel, Florian Fainelli, Fabio Estevam, Thomas Petazzoni The line before the changed if condition is: priv->phy_node = of_parse_phandle(np, "phy-handle", 0); . If this call succeeds priv->phy_node must not be overwritten in the if block; otherwise the reference to the node returned by of_parse_phandle is lost. So add a check that the if block isn't executed in this case. Furthermore in the fixed phy case no reference is aquired for phy_node resulting in an of_node_put without holding a reference. To fix that, get a reference on the MAC dt node. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Changes since (implicit) v1: - really do both fixes mentioned in the commit log drivers/net/ethernet/freescale/gianfar.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 6b0c4775cd0d..fb29d049f4e1 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -892,12 +892,12 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ - if (of_phy_is_fixed_link(np)) { + if (!priv->phy_node && of_phy_is_fixed_link(np)) { err = of_phy_register_fixed_link(np); if (err) goto err_grp_init; - priv->phy_node = np; + priv->phy_node = of_node_get(np); } /* Find the TBI PHY. If it's not there, we don't support SGMII */ -- 2.0.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3] net: gianfar: fix reference counting for phy_node 2014-08-07 19:41 ` [PATCH v2] " Uwe Kleine-König @ 2014-08-07 20:01 ` Uwe Kleine-König 2014-08-07 20:17 ` [PATCH v4] " Uwe Kleine-König 0 siblings, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2014-08-07 20:01 UTC (permalink / raw) To: netdev Cc: Claudiu Manoil, kernel, Florian Fainelli, Fabio Estevam, Thomas Petazzoni The line before the changed if condition is: priv->phy_node = of_parse_phandle(np, "phy-handle", 0); . If this call succeeds priv->phy_node must not be overwritten in the if block; otherwise the reference to the node returned by of_parse_phandle is lost. So add a check that the if block isn't executed in this case. Furthermore in the fixed phy case no reference is aquired for phy_node resulting in an of_node_put without holding a reference. To fix that, get a reference on the MAC dt node. Fixes: be40364544bd ("gianfar: use the new fixed PHY helpers") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Changes since v2: - add a Fixes: footer as suggested by Thomas Petazzoni via irc drivers/net/ethernet/broadcom/genet/bcmmii.c | 6 ++++-- drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 2 +- drivers/net/ethernet/freescale/gianfar.c | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c index add8d8596084..693659f05776 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c @@ -311,12 +311,12 @@ static int bcmgenet_mii_probe(struct net_device *dev) /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ - if (of_phy_is_fixed_link(dn)) { + if (!priv->phy_dn && of_phy_is_fixed_link(dn)) { ret = of_phy_register_fixed_link(dn); if (ret) return ret; - priv->phy_dn = dn; + priv->phy_dn = of_get_node(dn); } phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup, 0, @@ -452,6 +452,7 @@ int bcmgenet_mii_init(struct net_device *dev) return 0; out: + of_node_put(priv->phy_dn); mdiobus_unregister(priv->mii_bus); out_free: kfree(priv->mii_bus->irq); @@ -463,6 +464,7 @@ void bcmgenet_mii_exit(struct net_device *dev) { struct bcmgenet_priv *priv = netdev_priv(dev); + of_node_put(priv->phy_dn); mdiobus_unregister(priv->mii_bus); kfree(priv->mii_bus->irq); mdiobus_free(priv->mii_bus); 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 cfaf17b70f3f..748fd24d3d9e 100644 --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c @@ -1033,7 +1033,7 @@ static int fs_enet_probe(struct platform_device *ofdev) /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ - fpi->phy_node = ofdev->dev.of_node; + fpi->phy_node = of_node_get(ofdev->dev.of_node); } if (of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc5125-fec")) { diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 6b0c4775cd0d..fb29d049f4e1 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -892,12 +892,12 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ - if (of_phy_is_fixed_link(np)) { + if (!priv->phy_node && of_phy_is_fixed_link(np)) { err = of_phy_register_fixed_link(np); if (err) goto err_grp_init; - priv->phy_node = np; + priv->phy_node = of_node_get(np); } /* Find the TBI PHY. If it's not there, we don't support SGMII */ -- 2.0.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v4] net: gianfar: fix reference counting for phy_node 2014-08-07 20:01 ` [PATCH v3] " Uwe Kleine-König @ 2014-08-07 20:17 ` Uwe Kleine-König 2014-08-07 23:06 ` David Miller 0 siblings, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2014-08-07 20:17 UTC (permalink / raw) To: netdev Cc: Claudiu Manoil, kernel, Florian Fainelli, Fabio Estevam, Thomas Petazzoni The line before the changed if condition is: priv->phy_node = of_parse_phandle(np, "phy-handle", 0); . If this call succeeds priv->phy_node must not be overwritten in the if block; otherwise the reference to the node returned by of_parse_phandle is lost. So add a check that the if block isn't executed in this case. Furthermore in the fixed phy case no reference is aquired for phy_node resulting in an of_node_put without holding a reference. To fix that, get a reference on the MAC dt node. Fixes: be40364544bd ("gianfar: use the new fixed PHY helpers") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, I don't know how, but somehow I managed to add unrelated stuff to v3. So: Changes since v3: - Reset patch to the v2 one. Sorry for the patch flood, I hope I got it right this time. Best regards Uwe drivers/net/ethernet/freescale/gianfar.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 6b0c4775cd0d..fb29d049f4e1 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -892,12 +892,12 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ - if (of_phy_is_fixed_link(np)) { + if (!priv->phy_node && of_phy_is_fixed_link(np)) { err = of_phy_register_fixed_link(np); if (err) goto err_grp_init; - priv->phy_node = np; + priv->phy_node = of_node_get(np); } /* Find the TBI PHY. If it's not there, we don't support SGMII */ -- 2.0.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] net: gianfar: fix reference counting for phy_node 2014-08-07 20:17 ` [PATCH v4] " Uwe Kleine-König @ 2014-08-07 23:06 ` David Miller 0 siblings, 0 replies; 5+ messages in thread From: David Miller @ 2014-08-07 23:06 UTC (permalink / raw) To: u.kleine-koenig Cc: netdev, claudiu.manoil, kernel, f.fainelli, fabio.estevam, thomas.petazzoni From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Date: Thu, 7 Aug 2014 22:17:07 +0200 > The line before the changed if condition is: > > priv->phy_node = of_parse_phandle(np, "phy-handle", 0); > > . If this call succeeds priv->phy_node must not be overwritten in the if > block; otherwise the reference to the node returned by of_parse_phandle > is lost. So add a check that the if block isn't executed in this case. > > Furthermore in the fixed phy case no reference is aquired for phy_node > resulting in an of_node_put without holding a reference. To fix that, > get a reference on the MAC dt node. > > Fixes: be40364544bd ("gianfar: use the new fixed PHY helpers") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Applied. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-08-07 23:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-07 19:35 [PATCH] net: gianfar: fix reference counting for phy_node Uwe Kleine-König 2014-08-07 19:41 ` [PATCH v2] " Uwe Kleine-König 2014-08-07 20:01 ` [PATCH v3] " Uwe Kleine-König 2014-08-07 20:17 ` [PATCH v4] " Uwe Kleine-König 2014-08-07 23:06 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).