[PATCH net v2 0/5] net: fix phydev reference leaks

[PATCH net v2 0/5] net: fix phydev reference leaks

[Johan Hovold]

This series fixes a number of phydev reference leaks (and one of_node
leak) due to failure to put the reference taken by of_phy_find_device().

Note that I did not try to fix drivers/net/phy/xilinx_gmii2rgmii.c which
still leaks a reference.

Against net but should apply just as fine to net-next.

Thanks,
Johan

v2: 
 - use put_device() instead of phy_dev_free() to put the references
   taken in net/dsa (patch 1/4).
 - add four new patches fixing similar leaks


Johan Hovold (5):
  net: dsa: fix fixed-link-phy device leaks
  net: bcmgenet: fix phydev reference leak
  net: fsl/fman: fix phydev reference leak
  net: fsl/fman: fix fixed-link-phydev reference leak
  net: qcom/emac: fix of_node and phydev leaks

 drivers/net/ethernet/broadcom/genet/bcmmii.c     | 4 +++-
 drivers/net/ethernet/freescale/fman/fman_memac.c | 3 +++
 drivers/net/ethernet/freescale/fman/mac.c        | 2 ++
 drivers/net/ethernet/qualcomm/emac/emac-phy.c    | 1 +
 drivers/net/ethernet/qualcomm/emac/emac.c        | 4 ++++
 net/dsa/dsa.c                                    | 5 ++++-
 6 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.7.3

[PATCH net v2 1/5] net: dsa: fix fixed-link-phy device leaks

[Johan Hovold]

Make sure to drop the reference taken by of_phy_find_device() when
registering and deregistering the fixed-link PHY-device.

Fixes: 39b0c705195e ("net: dsa: Allow configuration of CPU & DSA port
speeds/duplex")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 net/dsa/dsa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index a6902c1e2f28..cb0091b99592 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -233,6 +233,8 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
 		genphy_read_status(phydev);
 		if (ds->ops->adjust_link)
 			ds->ops->adjust_link(ds, port, phydev);
+
+		put_device(&phydev->mdio.dev);
 	}
 
 	return 0;
@@ -509,8 +511,9 @@ void dsa_cpu_dsa_destroy(struct device_node *port_dn)
 	if (of_phy_is_fixed_link(port_dn)) {
 		phydev = of_phy_find_device(port_dn);
 		if (phydev) {
-			phy_device_free(phydev);
 			fixed_phy_unregister(phydev);
+			put_device(&phydev->mdio.dev);
+			phy_device_free(phydev);
 		}
 	}
 }
-- 
2.7.3

[PATCH net v2 2/5] net: bcmgenet: fix phydev reference leak

[Johan Hovold]

Make sure to drop the reference taken by of_phy_find_device() when
initialising MOCA PHYs.

Fixes: 6ac9de5f6563 ("net: bcmgenet: Register link_update callback for
all MoCA PHYs")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 457c3bc8cfff..2e745bd51df4 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -542,8 +542,10 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 	/* Make sure we initialize MoCA PHYs with a link down */
 	if (phy_mode == PHY_INTERFACE_MODE_MOCA) {
 		phydev = of_phy_find_device(dn);
-		if (phydev)
+		if (phydev) {
 			phydev->link = 0;
+			put_device(&phydev->mdio.dev);
+		}
 	}
 
 	return 0;
-- 
2.7.3

[PATCH net v2 3/5] net: fsl/fman: fix phydev reference leak

[Johan Hovold]

Make sure to drop the reference taken by of_phy_find_device() during
initialisation when later freeing the struct fman_mac.

Fixes: 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 53ef51e3bd9e..71a5ded9d1de 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -1107,6 +1107,9 @@ int memac_free(struct fman_mac *memac)
 {
 	free_init_resources(memac);
 
+	if (memac->pcsphy)
+		put_device(&memac->pcsphy->mdio.dev);
+
 	kfree(memac->memac_drv_param);
 	kfree(memac);
 
-- 
2.7.3

[PATCH net v2 4/5] net: fsl/fman: fix fixed-link-phydev reference leak

[Johan Hovold]

Make sure to drop the reference taken by of_phy_find_device() when
looking up a fixed-link phydev during probe.

Fixes: 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/freescale/fman/mac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 8fe6b3e253fa..736db9d9b0ad 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -892,6 +892,8 @@ static int mac_probe(struct platform_device *_of_dev)
 		priv->fixed_link->duplex = phy->duplex;
 		priv->fixed_link->pause = phy->pause;
 		priv->fixed_link->asym_pause = phy->asym_pause;
+
+		put_device(&phy->mdio.dev);
 	}
 
 	err = mac_dev->init(mac_dev);
-- 
2.7.3

[PATCH net v2 5/5] net: qcom/emac: fix of_node and phydev leaks

[Johan Hovold]

Make sure to drop the reference taken by of_phy_find_device() during
probe on probe errors and on driver unbind.

Also drop the of_node reference taken by of_parse_phandle() in the same
path.

Fixes: b9b17debc69d ("net: emac: emac gigabit ethernet controller driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 1 +
 drivers/net/ethernet/qualcomm/emac/emac.c     | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index da4e90db4d98..99a14df28b96 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
@@ -212,6 +212,7 @@ int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
 
 		phy_np = of_parse_phandle(np, "phy-handle", 0);
 		adpt->phydev = of_phy_find_device(phy_np);
+		of_node_put(phy_np);
 	}
 
 	if (!adpt->phydev) {
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 4fede4b86538..57b35aeac51a 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -711,6 +711,8 @@ static int emac_probe(struct platform_device *pdev)
 err_undo_napi:
 	netif_napi_del(&adpt->rx_q.napi);
 err_undo_mdiobus:
+	if (!has_acpi_companion(&pdev->dev))
+		put_device(&adpt->phydev->mdio.dev);
 	mdiobus_unregister(adpt->mii_bus);
 err_undo_clocks:
 	emac_clks_teardown(adpt);
@@ -730,6 +732,8 @@ static int emac_remove(struct platform_device *pdev)
 
 	emac_clks_teardown(adpt);
 
+	if (!has_acpi_companion(&pdev->dev))
+		put_device(&adpt->phydev->mdio.dev);
 	mdiobus_unregister(adpt->mii_bus);
 	free_netdev(netdev);
 
-- 
2.7.3

Re: [PATCH net v2 5/5] net: qcom/emac: fix of_node and phydev leaks

[Timur Tabi]

Johan Hovold wrote:
> Make sure to drop the reference taken by of_phy_find_device() during
> probe on probe errors and on driver unbind.
>
> Also drop the of_node reference taken by of_parse_phandle() in the same
> path.
>
> Fixes: b9b17debc69d ("net: emac: emac gigabit ethernet controller driver")
> Signed-off-by: Johan Hovold<johan@kernel.org>

When I first wrote this code, I had a lot of trouble getting the PHY to 
re-probe when the driver was unloaded and reloaded, and it was because 
of some odd problems with get/put of of_nodes.

Since I know that you didn't test this driver on real hardware, I have 
to test these patches myself before I can ACK them.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

RE: [PATCH net v2 4/5] net: fsl/fman: fix fixed-link-phydev reference leak

[Madalin-Cristian Bucur]

> -----Original Message-----
> From: Johan Hovold [mailto:jhovold@gmail.com] On Behalf Of Johan Hovold
> Sent: Thursday, November 24, 2016 8:22 PM
> 
> Make sure to drop the reference taken by of_phy_find_device() when
> looking up a fixed-link phydev during probe.
> 
> Fixes: 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/net/ethernet/freescale/fman/mac.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> b/drivers/net/ethernet/freescale/fman/mac.c
> index 8fe6b3e253fa..736db9d9b0ad 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -892,6 +892,8 @@ static int mac_probe(struct platform_device *_of_dev)
>  		priv->fixed_link->duplex = phy->duplex;
>  		priv->fixed_link->pause = phy->pause;
>  		priv->fixed_link->asym_pause = phy->asym_pause;
> +
> +		put_device(&phy->mdio.dev);
>  	}
> 
>  	err = mac_dev->init(mac_dev);
> --
> 2.7.3

Thank you,
Madalin

Re: [PATCH net v2 0/5] net: fix phydev reference leaks

[David Miller]

From: Johan Hovold <johan@kernel.org>
Date: Thu, 24 Nov 2016 19:21:26 +0100

> This series fixes a number of phydev reference leaks (and one of_node
> leak) due to failure to put the reference taken by of_phy_find_device().
> 
> Note that I did not try to fix drivers/net/phy/xilinx_gmii2rgmii.c which
> still leaks a reference.
> 
> Against net but should apply just as fine to net-next.
 ...
> v2: 
>  - use put_device() instead of phy_dev_free() to put the references
>    taken in net/dsa (patch 1/4).
>  - add four new patches fixing similar leaks

Series applied, thanks.

Re: [PATCH net v2 0/5] net: fix phydev reference leaks

[Timur Tabi]

David Miller wrote:
> Series applied, thanks.

I was really hoping you'd give me the chance to test the patches before 
applying them.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH net v2 0/5] net: fix phydev reference leaks

[David Miller]

From: Timur Tabi <timur@codeaurora.org>
Date: Sun, 27 Nov 2016 20:11:17 -0600

> David Miller wrote:
>> Series applied, thanks.
> 
> I was really hoping you'd give me the chance to test the patches
> before applying them.

Sorry, if anything is broken I will happily revert if it isn't
fixed promptly.

Re: [PATCH net v2 5/5] net: qcom/emac: fix of_node and phydev leaks

[Timur Tabi]

On 11/24/2016 12:21 PM, Johan Hovold wrote:
> +	if (!has_acpi_companion(&pdev->dev))
> +		put_device(&adpt->phydev->mdio.dev);

I was wondering if, instead of calling put_device() only on non-ACPI systems, 
would it be better if on an ACPI system I called get_device() manually?  That 
is, some thing like this:

int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
{
...
	if (has_acpi_companion(&pdev->dev)) {
...
		get_device(&mii_bus->dev);
	} else {
...

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH net v2 5/5] net: qcom/emac: fix of_node and phydev leaks

[Johan Hovold]

On Mon, Jan 09, 2017 at 01:49:53PM -0600, Timur Tabi wrote:
> On 11/24/2016 12:21 PM, Johan Hovold wrote:
> > +	if (!has_acpi_companion(&pdev->dev))
> > +		put_device(&adpt->phydev->mdio.dev);
> 
> I was wondering if, instead of calling put_device() only on non-ACPI systems, 
> would it be better if on an ACPI system I called get_device() manually?  That 
> is, some thing like this:
> 
> int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
> {
> ...
> 	if (has_acpi_companion(&pdev->dev)) {
> ...
> 		get_device(&mii_bus->dev);
> 	} else {
> ...

Yeah, that's better.

Thanks,
Johan