[PATCH 1/3] net: ucc_geth: drop acquired references in probe error path and remove

[PATCH 1/3] net: ucc_geth: drop acquired references in probe error path and remove

[Uwe Kleine-König]

The ucc_geth_probe function assigns to ug_info->tbi_node and
ug_info->phy_node a value returned by of_parse_phandle which returns a
new reference. Put this reference again in the error path of
ucc_geth_probe and when removing the device.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
No Fixes: footer here. The problem already exists in v2.6.31-rc1 (e.g.
commit 0b9da337dca9 (net: Rework ucc_geth driver to use of_mdio
infrastructure)). Didn't continue to research a specific commit.
---
 drivers/net/ethernet/freescale/ucc_geth.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 36fc429298e3..5f1aab9c4ee7 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3864,8 +3864,11 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 	/* Create an ethernet device instance */
 	dev = alloc_etherdev(sizeof(*ugeth));
 
-	if (dev == NULL)
+	if (dev == NULL) {
+		of_node_put(ug_info->tbi_node);
+		of_node_put(ug_info->phy_node);
 		return -ENOMEM;
+	}
 
 	ugeth = netdev_priv(dev);
 	spin_lock_init(&ugeth->lock);
@@ -3899,6 +3902,8 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 			pr_err("%s: Cannot register net device, aborting\n",
 			       dev->name);
 		free_netdev(dev);
+		of_node_put(ug_info->tbi_node);
+		of_node_put(ug_info->phy_node);
 		return err;
 	}
 
@@ -3922,6 +3927,8 @@ static int ucc_geth_remove(struct platform_device* ofdev)
 	unregister_netdev(dev);
 	free_netdev(dev);
 	ucc_geth_memclean(ugeth);
+	of_node_put(ugeth->info->tbi_node);
+	of_node_put(ugeth->info->phy_node);
 
 	return 0;
 }
-- 
2.0.1

[PATCH 2/3] net: ucc_geth: make probe consistently acquire a reference to the phy node

[Uwe Kleine-König]

When the driver attaches to a device that has a phy handle the probe
routine returns with a reference to that node. This reference is
correctly dropped in the error path and the remove function. In the
fixed phy case however no reference is acquired and so the error path
might drop a reference the driver isn't holding. Fix that by getting a
reference to the MAC.

Fixes: 87009814cdbb ("ucc_geth: use the new fixed PHY helpers")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 5f1aab9c4ee7..a22ff770f9e5 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3796,7 +3796,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 			if (err)
 				return err;
 		}
-		ug_info->phy_node = np;
+		ug_info->phy_node = of_node_get(np);
 	}
 
 	/* Find the TBI PHY node.  If it's not there, we don't support SGMII */
-- 
2.0.1

[PATCH 3/3] net: ucc_geth: Don't use the MAC as PHY without a fixed link

[Uwe Kleine-König]

This matches what the other drivers using fixed-link support do and
restores the behaviour before commit 87009814cdbb ("ucc_geth: use the
new fixed PHY helpers") for the affected device trees (i.e. no
phy-handle and no fixed-link).

Fixes: 87009814cdbb ("ucc_geth: use the new fixed PHY helpers")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index a22ff770f9e5..07415a0df88c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3787,15 +3787,14 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 	ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
 
 	ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
-	if (!ug_info->phy_node) {
-		/* In the case of a fixed PHY, the DT node associated
+	if (!ug_info->phy_node && of_phy_is_fixed_link(np)) {
+		/*
+		 * 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)) {
-			err = of_phy_register_fixed_link(np);
-			if (err)
-				return err;
-		}
+		err = of_phy_register_fixed_link(np);
+		if (err)
+			return err;
 		ug_info->phy_node = of_node_get(np);
 	}
 
-- 
2.0.1

Re: [PATCH 1/3] net: ucc_geth: drop acquired references in probe error path and remove

[David Miller]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Thu,  7 Aug 2014 23:48:24 +0200

> The ucc_geth_probe function assigns to ug_info->tbi_node and
> ug_info->phy_node a value returned by of_parse_phandle which returns a
> new reference. Put this reference again in the error path of
> ucc_geth_probe and when removing the device.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied.

Re: [PATCH 2/3] net: ucc_geth: make probe consistently acquire a reference to the phy node

[David Miller]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Thu,  7 Aug 2014 23:48:25 +0200

> When the driver attaches to a device that has a phy handle the probe
> routine returns with a reference to that node. This reference is
> correctly dropped in the error path and the remove function. In the
> fixed phy case however no reference is acquired and so the error path
> might drop a reference the driver isn't holding. Fix that by getting a
> reference to the MAC.
> 
> Fixes: 87009814cdbb ("ucc_geth: use the new fixed PHY helpers")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied.

Re: [PATCH 3/3] net: ucc_geth: Don't use the MAC as PHY without a fixed link

[David Miller]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Thu,  7 Aug 2014 23:48:26 +0200

> This matches what the other drivers using fixed-link support do and
> restores the behaviour before commit 87009814cdbb ("ucc_geth: use the
> new fixed PHY helpers") for the affected device trees (i.e. no
> phy-handle and no fixed-link).
> 
> Fixes: 87009814cdbb ("ucc_geth: use the new fixed PHY helpers")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied.

Re: [PATCH 1/3] net: ucc_geth: drop acquired references in probe error path and remove

[Uwe Kleine-König]

On Thu, Aug 07, 2014 at 11:48:24PM +0200, Uwe Kleine-König wrote:
> The ucc_geth_probe function assigns to ug_info->tbi_node and
> ug_info->phy_node a value returned by of_parse_phandle which returns a
> new reference. Put this reference again in the error path of
> ucc_geth_probe and when removing the device.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> No Fixes: footer here. The problem already exists in v2.6.31-rc1 (e.g.
> commit 0b9da337dca9 (net: Rework ucc_geth driver to use of_mdio
> infrastructure)). Didn't continue to research a specific commit.
> ---
>  drivers/net/ethernet/freescale/ucc_geth.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index 36fc429298e3..5f1aab9c4ee7 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -3864,8 +3864,11 @@ static int ucc_geth_probe(struct platform_device* ofdev)
>  	/* Create an ethernet device instance */
>  	dev = alloc_etherdev(sizeof(*ugeth));
>  
> -	if (dev == NULL)
> +	if (dev == NULL) {
> +		of_node_put(ug_info->tbi_node);
> +		of_node_put(ug_info->phy_node);
>  		return -ENOMEM;
> +	}
>  
>  	ugeth = netdev_priv(dev);
>  	spin_lock_init(&ugeth->lock);
> @@ -3899,6 +3902,8 @@ static int ucc_geth_probe(struct platform_device* ofdev)
>  			pr_err("%s: Cannot register net device, aborting\n",
>  			       dev->name);
>  		free_netdev(dev);
> +		of_node_put(ug_info->tbi_node);
> +		of_node_put(ug_info->phy_node);
>  		return err;
>  	}
>  
> @@ -3922,6 +3927,8 @@ static int ucc_geth_remove(struct platform_device* ofdev)
>  	unregister_netdev(dev);
>  	free_netdev(dev);
>  	ucc_geth_memclean(ugeth);
> +	of_node_put(ugeth->info->tbi_node);
> +	of_node_put(ugeth->info->phy_node);
this must read:

	of_node_put(ugeth->ug_info->tbi_node);
	of_node_put(ugeth->ug_info->phy_node);

otherwise it fails to build.

@davem: Should I send a fixup or a new version?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/3] net: ucc_geth: drop acquired references in probe error path and remove

[David Miller]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Fri, 8 Aug 2014 22:34:56 +0200

> @davem: Should I send a fixup or a new version?

Fixup, please.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] net: ucc_geth: fix build failure

[Uwe Kleine-König]

My series to fix the reference counting of dt nodes introduced a build
failure. Fix it.

Fixes: fa310789a488 ("net: ucc_geth: drop acquired references in probe error path and remove")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

I found a powerpc toolchain now, so this really builds now. Sorry for the
inconvenience.

Uwe

 drivers/net/ethernet/freescale/ucc_geth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index d6b64e316527..3cf0478b3728 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3924,8 +3924,8 @@ static int ucc_geth_remove(struct platform_device* ofdev)
 	unregister_netdev(dev);
 	free_netdev(dev);
 	ucc_geth_memclean(ugeth);
-	of_node_put(ugeth->info->tbi_node);
-	of_node_put(ugeth->info->phy_node);
+	of_node_put(ugeth->ug_info->tbi_node);
+	of_node_put(ugeth->ug_info->phy_node);
 
 	return 0;
 }
-- 
2.0.1

Re: [PATCH] net: ucc_geth: fix build failure

[David Miller]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Sun, 10 Aug 2014 20:32:05 +0200

> My series to fix the reference counting of dt nodes introduced a build
> failure. Fix it.
> 
> Fixes: fa310789a488 ("net: ucc_geth: drop acquired references in probe error path and remove")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied, thanks.