[PATCH 1/2] net: macb: Add phy-handle DT support

[PATCH 1/2] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..cc5b9e6e3526 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -567,6 +567,9 @@ static int macb_mii_init(struct macb *bp)
 
 			err = mdiobus_register(bp->mii_bus);
 		} else {
+			/* attempt to find a phy-handle */
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+
 			/* try dt phy registration */
 			err = of_mdiobus_register(bp->mii_bus, np);
 
-- 
2.16.2

[PATCH 1/2] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..cc5b9e6e3526 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -567,6 +567,9 @@ static int macb_mii_init(struct macb *bp)
 
 			err = mdiobus_register(bp->mii_bus);
 		} else {
+			/* attempt to find a phy-handle */
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+
 			/* try dt phy registration */
 			err = of_mdiobus_register(bp->mii_bus, np);
 
-- 
2.16.2

[PATCH 2/2] Documentation: macb: Document phy-handle optional binding

[Brad Mouring]

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

[PATCH 2/2] Documentation: macb: Document phy-handle optional binding

[Brad Mouring]

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

Re: [1/2] net: macb: Add phy-handle DT support

[Andrew Lunn]

On Wed, Mar 07, 2018 at 04:42:56PM -0600, Brad Mouring wrote:
> This optional binding (as described in the ethernet DT bindings doc)
> directs the netdev to the phydev to use. This is useful for a phy
> chip that has >1 phy in it, and two netdevs are using the same phy
> chip (i.e. the second mac's phy lives on the first mac's MDIO bus)
> 
> The devicetree snippet would look something like this:
> 
> ethernet@feedf00d {
> 	...
> 	phy-handle = <&phy0> // the first netdev is physically wired to phy0
> 	...
> 	phy0: phy@0 {
> 		...
> 		reg = <0x0> // MDIO address 0
> 		...
> 	}
> 	phy1: phy@1 {
> 		...
> 		reg = <0x1> // MDIO address 1
> 		...
> 	}
> ...
> }
> 
> ethernet@deadbeef {
> 	...
> 	phy-handle = <&phy1> // tells the driver to use phy1 on the
> 						 // first mac's mdio bus (it's wired thusly)
> 	...
> }
> 
> The work done to add the phy_node in the first place (dacdbb4dfc1a1:
> "net: macb: add fixed-link node support") will consume the
> device_node (if found).
> 
> Signed-off-by: Brad Mouring <brad.mouring@ni.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index e84afcf1ecb5..cc5b9e6e3526 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -567,6 +567,9 @@ static int macb_mii_init(struct macb *bp)
>  
>  			err = mdiobus_register(bp->mii_bus);
>  		} else {
> +			/* attempt to find a phy-handle */
> +			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
> +
>  			/* try dt phy registration */
>  			err = of_mdiobus_register(bp->mii_bus, np);
>  

Hi Brad

I think it is more logical to do this in macb_mii_probe().

I would probably also move the fixed_link code from macb_mii_init() to
macb_mii_probe(). I would probably also move the fallback to standard
phy registration. Make macb_mii_init() about registering the MDIO bus,
and macb_mii_probe() about probing the MDIO bus to find the PHY to
use. At the moment, it is all rather mixed up.

     Andrew

Re: [1/2] net: macb: Add phy-handle DT support

[Brad Mouring]

On Thu, Mar 08, 2018 at 06:32:47PM +0100, Andrew Lunn wrote:
> On Wed, Mar 07, 2018 at 04:42:56PM -0600, Brad Mouring wrote:
> > This optional binding (as described in the ethernet DT bindings doc)
> > directs the netdev to the phydev to use. This is useful for a phy
> > chip that has >1 phy in it, and two netdevs are using the same phy
> > chip (i.e. the second mac's phy lives on the first mac's MDIO bus)
> > ...
> Hi Brad
> 
> I think it is more logical to do this in macb_mii_probe().
> 
> I would probably also move the fixed_link code from macb_mii_init() to
> macb_mii_probe(). I would probably also move the fallback to standard
> phy registration. Make macb_mii_init() about registering the MDIO bus,
> and macb_mii_probe() about probing the MDIO bus to find the PHY to
> use. At the moment, it is all rather mixed up.
> 
>      Andrew

Hi Andrew

That makes sense, I'll rework and resend. Thanks for the suggestion.

Brad

Re: [1/2] net: macb: Add phy-handle DT support

[Brad Mouring]

On Thu, Mar 08, 2018 at 06:32:47PM +0100, Andrew Lunn wrote:
> On Wed, Mar 07, 2018 at 04:42:56PM -0600, Brad Mouring wrote:
> > This optional binding (as described in the ethernet DT bindings doc)
> > directs the netdev to the phydev to use. This is useful for a phy
> > chip that has >1 phy in it, and two netdevs are using the same phy
> > chip (i.e. the second mac's phy lives on the first mac's MDIO bus)
> > ...
> Hi Brad
> 
> I think it is more logical to do this in macb_mii_probe().
> 
> I would probably also move the fixed_link code from macb_mii_init() to
> macb_mii_probe(). I would probably also move the fallback to standard
> phy registration. Make macb_mii_init() about registering the MDIO bus,
> and macb_mii_probe() about probing the MDIO bus to find the PHY to
> use. At the moment, it is all rather mixed up.
> 
>      Andrew

Hi Andrew

That makes sense, I'll rework and resend. Thanks for the suggestion.

Brad

[PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cadence/macb_main.c | 89 ++++++++++++++++----------------
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..7b69a291ab7a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -470,10 +470,48 @@ static void macb_handle_link_change(struct net_device *dev)
 static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
-	struct macb_platform_data *pdata;
+	struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
 	struct phy_device *phydev;
+	struct device_node *np = bp->pdev->dev.of_node;
 	int phy_irq;
-	int ret;
+	int ret, i;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	} else {
+		for (i = 0; i < PHY_MAX_ADDR; i++)
+			bp->mii_bus->irq[i] = PHY_POLL;
+
+		if (pdata)
+			bp->mii_bus->phy_mask = pdata->phy_mask;
+
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +526,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +570,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,46 +593,10 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
 
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
-
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
-		if (pdata)
-			bp->mii_bus->phy_mask = pdata->phy_mask;
-
 		err = mdiobus_register(bp->mii_bus);
 	}
 
@@ -610,10 +611,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cadence/macb_main.c | 89 ++++++++++++++++----------------
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..7b69a291ab7a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -470,10 +470,48 @@ static void macb_handle_link_change(struct net_device *dev)
 static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
-	struct macb_platform_data *pdata;
+	struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
 	struct phy_device *phydev;
+	struct device_node *np = bp->pdev->dev.of_node;
 	int phy_irq;
-	int ret;
+	int ret, i;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	} else {
+		for (i = 0; i < PHY_MAX_ADDR; i++)
+			bp->mii_bus->irq[i] = PHY_POLL;
+
+		if (pdata)
+			bp->mii_bus->phy_mask = pdata->phy_mask;
+
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +526,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +570,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,46 +593,10 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
 
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
-
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
-		if (pdata)
-			bp->mii_bus->phy_mask = pdata->phy_mask;
-
 		err = mdiobus_register(bp->mii_bus);
 	}
 
@@ -610,10 +611,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v2 net-next 2/3] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7b69a291ab7a..af458ab62601 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -485,6 +485,9 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
+			/* attempt to find a phy-handle */
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+
 			/* fallback to standard phy registration if no phy were
 			 * found during dt phy registration
 			 */
-- 
2.16.2

[PATCH v2 net-next 2/3] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7b69a291ab7a..af458ab62601 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -485,6 +485,9 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
+			/* attempt to find a phy-handle */
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+
 			/* fallback to standard phy registration if no phy were
 			 * found during dt phy registration
 			 */
-- 
2.16.2

[PATCH v2 net-next 3/3] Documentation: macb: Document phy-handle optional binding

[Brad Mouring]

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

[PATCH v2 net-next 3/3] Documentation: macb: Document phy-handle optional binding

[Brad Mouring]

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

Re: [PATCH v2 net-next 3/3] Documentation: macb: Document phy-handle optional binding

[Andrew Lunn]

On Fri, Mar 09, 2018 at 04:12:33PM -0600, Brad Mouring wrote:

You should have at least one line here, even if it is the subject line
differently worded.

> Signed-off-by: Brad Mouring <brad.mouring@ni.com>

Re: [PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[Andrew Lunn]

On Fri, Mar 09, 2018 at 04:12:31PM -0600, Brad Mouring wrote:
> The macb mii setup (mii_probe() and mii_init()) previously was
> somewhat interspersed, likely a result of organic growth and hacking.

Hi Brad

For netdev it is normal to include a cover note for patch series,
which explains the big picture of the series. The cover note is then
used as the merge commit message.

> 
> This change moves mii bus registration into mii_init and probing the
> bus for devices into mii_probe.
> 
> Signed-off-by: Brad Mouring <brad.mouring@ni.com>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 89 ++++++++++++++++----------------
>  1 file changed, 45 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index e84afcf1ecb5..7b69a291ab7a 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -470,10 +470,48 @@ static void macb_handle_link_change(struct net_device *dev)
>  static int macb_mii_probe(struct net_device *dev)
>  {
>  	struct macb *bp = netdev_priv(dev);
> -	struct macb_platform_data *pdata;
> +	struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
>  	struct phy_device *phydev;
> +	struct device_node *np = bp->pdev->dev.of_node;
>  	int phy_irq;
> -	int ret;
> +	int ret, i;

These should be in 'Reverse Christmas Tree' order. i.e. longest
first. That makes pdata tricky, so do the assignment in the body of
the function.

> +
> +	if (np) {
> +		if (of_phy_is_fixed_link(np)) {
> +			if (of_phy_register_fixed_link(np) < 0) {
> +				dev_err(&bp->pdev->dev,
> +					"broken fixed-link specification\n");
> +				return -ENODEV;
> +			}
> +			bp->phy_node = of_node_get(np);
> +		} else {
> +			/* fallback to standard phy registration if no phy were
> +			 * found during dt phy registration
> +			 */
> +			if (!phy_find_first(bp->mii_bus)) {
> +				for (i = 0; i < PHY_MAX_ADDR; i++) {
> +					struct phy_device *phydev;
> +
> +					phydev = mdiobus_scan(bp->mii_bus, i);
> +					if (IS_ERR(phydev) &&
> +					    PTR_ERR(phydev) != -ENODEV) {
> +						ret = PTR_ERR(phydev);
> +						break;
> +					}
> +				}
> +
> +				if (ret)
> +					return -ENODEV;
> +			}
> +		}
> +	} else {
> +		for (i = 0; i < PHY_MAX_ADDR; i++)
> +			bp->mii_bus->irq[i] = PHY_POLL;
> +
> +		if (pdata)
> +			bp->mii_bus->phy_mask = pdata->phy_mask;

I would say these two are to do with the mdio bus setup. Setting irq
to POLL is not required, the core will do that. So you could add one
patch removing that.

      Andrew

Re: [PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

Hi Andrew,

On Sat, Mar 10, 2018 at 05:17:18PM +0100, Andrew Lunn wrote:
> On Fri, Mar 09, 2018 at 04:12:31PM -0600, Brad Mouring wrote:
> > The macb mii setup (mii_probe() and mii_init()) previously was
> > somewhat interspersed, likely a result of organic growth and hacking.
> 
> Hi Brad
> 
> For netdev it is normal to include a cover note for patch series,
> which explains the big picture of the series. The cover note is then
> used as the merge commit message.
> 
> > 
> > This change moves mii bus registration into mii_init and probing the
> > bus for devices into mii_probe.
> > 
> > Signed-off-by: Brad Mouring <brad.mouring@ni.com>
> > Suggested-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/net/ethernet/cadence/macb_main.c | 89 ++++++++++++++++----------------
> >  1 file changed, 45 insertions(+), 44 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index e84afcf1ecb5..7b69a291ab7a 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -470,10 +470,48 @@ static void macb_handle_link_change(struct net_device *dev)
> >  static int macb_mii_probe(struct net_device *dev)
> >  {
> >  	struct macb *bp = netdev_priv(dev);
> > -	struct macb_platform_data *pdata;
> > +	struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
> >  	struct phy_device *phydev;
> > +	struct device_node *np = bp->pdev->dev.of_node;
> >  	int phy_irq;
> > -	int ret;
> > +	int ret, i;
> 
> These should be in 'Reverse Christmas Tree' order. i.e. longest
> first. That makes pdata tricky, so do the assignment in the body of
> the function.
> 
> > +
> > +	if (np) {
> > +		if (of_phy_is_fixed_link(np)) {
> > +			if (of_phy_register_fixed_link(np) < 0) {
> > +				dev_err(&bp->pdev->dev,
> > +					"broken fixed-link specification\n");
> > +				return -ENODEV;
> > +			}
> > +			bp->phy_node = of_node_get(np);
> > +		} else {
> > +			/* fallback to standard phy registration if no phy were
> > +			 * found during dt phy registration
> > +			 */
> > +			if (!phy_find_first(bp->mii_bus)) {
> > +				for (i = 0; i < PHY_MAX_ADDR; i++) {
> > +					struct phy_device *phydev;
> > +
> > +					phydev = mdiobus_scan(bp->mii_bus, i);
> > +					if (IS_ERR(phydev) &&
> > +					    PTR_ERR(phydev) != -ENODEV) {
> > +						ret = PTR_ERR(phydev);
> > +						break;
> > +					}
> > +				}
> > +
> > +				if (ret)
> > +					return -ENODEV;
> > +			}
> > +		}
> > +	} else {
> > +		for (i = 0; i < PHY_MAX_ADDR; i++)
> > +			bp->mii_bus->irq[i] = PHY_POLL;
> > +
> > +		if (pdata)
> > +			bp->mii_bus->phy_mask = pdata->phy_mask;
> 
> I would say these two are to do with the mdio bus setup. Setting irq
> to POLL is not required, the core will do that. So you could add one
> patch removing that.
> 
>       Andrew

Thank you for the excellent feedback and patience. I've prepped a set
of patches addressing your points, but I'd like to test the changes
on hardware first (both with a DT that calls out the phys and one
that depends on default discovery) prior to a v3 (with appropriate
cover letter).

Brad

Re: [PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

Hi Andrew,

On Sat, Mar 10, 2018 at 05:17:18PM +0100, Andrew Lunn wrote:
> On Fri, Mar 09, 2018 at 04:12:31PM -0600, Brad Mouring wrote:
> > The macb mii setup (mii_probe() and mii_init()) previously was
> > somewhat interspersed, likely a result of organic growth and hacking.
> 
> Hi Brad
> 
> For netdev it is normal to include a cover note for patch series,
> which explains the big picture of the series. The cover note is then
> used as the merge commit message.
> 
> > 
> > This change moves mii bus registration into mii_init and probing the
> > bus for devices into mii_probe.
> > 
> > Signed-off-by: Brad Mouring <brad.mouring@ni.com>
> > Suggested-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/net/ethernet/cadence/macb_main.c | 89 ++++++++++++++++----------------
> >  1 file changed, 45 insertions(+), 44 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index e84afcf1ecb5..7b69a291ab7a 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -470,10 +470,48 @@ static void macb_handle_link_change(struct net_device *dev)
> >  static int macb_mii_probe(struct net_device *dev)
> >  {
> >  	struct macb *bp = netdev_priv(dev);
> > -	struct macb_platform_data *pdata;
> > +	struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
> >  	struct phy_device *phydev;
> > +	struct device_node *np = bp->pdev->dev.of_node;
> >  	int phy_irq;
> > -	int ret;
> > +	int ret, i;
> 
> These should be in 'Reverse Christmas Tree' order. i.e. longest
> first. That makes pdata tricky, so do the assignment in the body of
> the function.
> 
> > +
> > +	if (np) {
> > +		if (of_phy_is_fixed_link(np)) {
> > +			if (of_phy_register_fixed_link(np) < 0) {
> > +				dev_err(&bp->pdev->dev,
> > +					"broken fixed-link specification\n");
> > +				return -ENODEV;
> > +			}
> > +			bp->phy_node = of_node_get(np);
> > +		} else {
> > +			/* fallback to standard phy registration if no phy were
> > +			 * found during dt phy registration
> > +			 */
> > +			if (!phy_find_first(bp->mii_bus)) {
> > +				for (i = 0; i < PHY_MAX_ADDR; i++) {
> > +					struct phy_device *phydev;
> > +
> > +					phydev = mdiobus_scan(bp->mii_bus, i);
> > +					if (IS_ERR(phydev) &&
> > +					    PTR_ERR(phydev) != -ENODEV) {
> > +						ret = PTR_ERR(phydev);
> > +						break;
> > +					}
> > +				}
> > +
> > +				if (ret)
> > +					return -ENODEV;
> > +			}
> > +		}
> > +	} else {
> > +		for (i = 0; i < PHY_MAX_ADDR; i++)
> > +			bp->mii_bus->irq[i] = PHY_POLL;
> > +
> > +		if (pdata)
> > +			bp->mii_bus->phy_mask = pdata->phy_mask;
> 
> I would say these two are to do with the mdio bus setup. Setting irq
> to POLL is not required, the core will do that. So you could add one
> patch removing that.
> 
>       Andrew

Thank you for the excellent feedback and patience. I've prepped a set
of patches addressing your points, but I'd like to test the changes
on hardware first (both with a DT that calls out the phys and one
that depends on default discovery) prior to a v3 (with appropriate
cover letter).

Brad

[PATCH v3 net-next 0/4] macb: Introduce phy-handle DT functionality

[Brad Mouring]

Consider the situation where a macb netdev is connected through
a phydev that sits on a mii bus other than the one provided to
this particular netdev. This situation is what this patchset aims
to accomplish through the existing phy-handle optional binding.

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is precisely the
situation this patchset aims to solve, so it makes sense to introduce
the functionality to this driver (where the physical layout discussed
was encountered).

The devicetree snippet would look something like this:

...
   ethernet@feedf00d {
           ...
           phy-handle = <&phy0> // the first netdev is physically wired to phy0
           ...
           phy0: phy@0 {
                   ...
                   reg = <0x0> // MDIO address 0
                   ...
           }
           phy1: phy@1 {
                   ...
                   reg = <0x1> // MDIO address 1
                   ...
           }
   ...
   }
   
   ethernet@deadbeef {
           ...
           phy-handle = <&phy1> // tells the driver to use phy1 on the
                                // first mac's mdio bus (it's wired thusly)
           ...
   }
...

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

v2: Reorganization of mii probe/init functions, suggested by Andrew Lunn
v3: Moved some of the bus init code back into init (erroneously moved to probe)
    some style issues, and an unintialized variable warning addressed.

[PATCH v3 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..9b6195fbbf8e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
-	int ret;
+	struct device_node *np;
+	int phy_irq, ret, i;
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	np = bp->pdev->dev.of_node;
+	ret = 0;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +522,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,39 +589,9 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
-
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
 
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			bp->mii_bus->irq[i] = PHY_POLL;
@@ -610,10 +613,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v3 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..9b6195fbbf8e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
-	int ret;
+	struct device_node *np;
+	int phy_irq, ret, i;
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	np = bp->pdev->dev.of_node;
+	ret = 0;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +522,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,39 +589,9 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
-
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
 
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			bp->mii_bus->irq[i] = PHY_POLL;
@@ -610,10 +613,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

Re: [PATCH v3 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Florian Fainelli]

On 03/12/2018 10:09 AM, Brad Mouring wrote:
> The macb mii setup (mii_probe() and mii_init()) previously was
> somewhat interspersed, likely a result of organic growth and hacking.
> 
> This change moves mii bus registration into mii_init and probing the
> bus for devices into mii_probe.
> 
> Signed-off-by: Brad Mouring <brad.mouring@ni.com>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
>  1 file changed, 41 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index e84afcf1ecb5..9b6195fbbf8e 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
>  	struct macb *bp = netdev_priv(dev);
>  	struct macb_platform_data *pdata;
>  	struct phy_device *phydev;
> -	int phy_irq;
> -	int ret;
> +	struct device_node *np;
> +	int phy_irq, ret, i;
> +
> +	pdata = dev_get_platdata(&bp->pdev->dev);
> +	np = bp->pdev->dev.of_node;
> +	ret = 0;
> +
> +	if (np) {

Nit: a future cleanup (not this patch series) should consider doing an
early check on np to reduce the indentation.
-- 
Florian

Re: [PATCH v3 net-next 1/4] net: macb: Reorganize macb_mii bringup

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8078 bytes --]

Hi Brad,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brad-Mouring/net-macb-Reorganize-macb_mii-bringup/20180313-113743
config: x86_64-randconfig-x003-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Brad-Mouring/net-macb-Reorganize-macb_mii-bringup/20180313-113743 HEAD dbe45eb4d14a7cd3d0b9c7ee8e7ca034c1ef3852 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/net/ethernet/cadence/macb_main.c: In function 'macb_mii_init':
>> drivers/net/ethernet/cadence/macb_main.c:596:8: error: 'i' undeclared (first use in this function)
      for (i = 0; i < PHY_MAX_ADDR; i++)
           ^
   drivers/net/ethernet/cadence/macb_main.c:596:8: note: each undeclared identifier is reported only once for each function it appears in

vim +/i +596 drivers/net/ethernet/cadence/macb_main.c

6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  564  
421d9df0 drivers/net/ethernet/cadence/macb.c      Cyrille Pitchen    2015-03-07  565  static int macb_mii_init(struct macb *bp)
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  566  {
84e0cdb0 drivers/net/ethernet/cadence/macb.c      Jamie Iles         2011-03-08  567  	struct macb_platform_data *pdata;
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  568  	struct device_node *np;
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  569  	int err;
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  570  
3dbda77e drivers/net/macb.c                       Uwe Kleine-Koenig  2009-07-23  571  	/* Enable management port */
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  572  	macb_writel(bp, NCR, MACB_BIT(MPE));
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  573  
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  574  	bp->mii_bus = mdiobus_alloc();
aa50b552 drivers/net/ethernet/cadence/macb.c      Moritz Fischer     2016-03-29  575  	if (!bp->mii_bus) {
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  576  		err = -ENOMEM;
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  577  		goto err_out;
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  578  	}
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  579  
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  580  	bp->mii_bus->name = "MACB_mii_bus";
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  581  	bp->mii_bus->read = &macb_mdio_read;
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  582  	bp->mii_bus->write = &macb_mdio_write;
98d5e57e drivers/net/ethernet/cadence/macb.c      Florian Fainelli   2012-01-09  583  	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
98d5e57e drivers/net/ethernet/cadence/macb.c      Florian Fainelli   2012-01-09  584  		 bp->pdev->name, bp->pdev->id);
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  585  	bp->mii_bus->priv = bp;
cf669660 drivers/net/ethernet/cadence/macb.c      Florian Fainelli   2016-05-02  586  	bp->mii_bus->parent = &bp->pdev->dev;
c607a0d9 drivers/net/ethernet/cadence/macb.c      Jingoo Han         2013-08-30  587  	pdata = dev_get_platdata(&bp->pdev->dev);
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  588  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  589  	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  590  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  591  	np = bp->pdev->dev.of_node;
dacdbb4d drivers/net/ethernet/cadence/macb.c      Michael Grzeschik  2017-06-23  592  
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  593  	if (np) {
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  594  		err = of_mdiobus_register(bp->mii_bus, np);
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  595  	} else {
83a77e9e drivers/net/ethernet/cadence/macb.c      Bartosz Folta      2016-12-14 @596  		for (i = 0; i < PHY_MAX_ADDR; i++)
83a77e9e drivers/net/ethernet/cadence/macb.c      Bartosz Folta      2016-12-14  597  			bp->mii_bus->irq[i] = PHY_POLL;
83a77e9e drivers/net/ethernet/cadence/macb.c      Bartosz Folta      2016-12-14  598  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  599  		if (pdata)
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  600  			bp->mii_bus->phy_mask = pdata->phy_mask;
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  601  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  602  		err = mdiobus_register(bp->mii_bus);
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  603  	}
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  604  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  605  	if (err)
e7f4dc35 drivers/net/ethernet/cadence/macb.c      Andrew Lunn        2016-01-06  606  		goto err_out_free_mdiobus;
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  607  
7daa78e3 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-27  608  	err = macb_mii_probe(bp->dev);
7daa78e3 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-27  609  	if (err)
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  610  		goto err_out_unregister_bus;
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  611  
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  612  	return 0;
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  613  
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  614  err_out_unregister_bus:
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  615  	mdiobus_unregister(bp->mii_bus);
9ce98140 drivers/net/ethernet/cadence/macb_main.c Michael Grzeschik  2017-11-08  616  	if (np && of_phy_is_fixed_link(np))
9ce98140 drivers/net/ethernet/cadence/macb_main.c Michael Grzeschik  2017-11-08  617  		of_phy_deregister_fixed_link(np);
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  618  err_out_free_mdiobus:
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  619  	of_node_put(bp->phy_node);
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  620  	mdiobus_free(bp->mii_bus);
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  621  err_out:
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  622  	return err;
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  623  }
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  624  

:::::: The code at line 596 was first introduced by commit
:::::: 83a77e9ec4150ee4acc635638f7dedd9da523a26 net: macb: Added PCI wrapper for Platform Driver.

:::::: TO: Bartosz Folta <bfolta@cadence.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29486 bytes --]

Re: [PATCH v3 net-next 1/4] net: macb: Reorganize macb_mii bringup

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8078 bytes --]

Hi Brad,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brad-Mouring/net-macb-Reorganize-macb_mii-bringup/20180313-113743
config: x86_64-randconfig-x003-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Brad-Mouring/net-macb-Reorganize-macb_mii-bringup/20180313-113743 HEAD dbe45eb4d14a7cd3d0b9c7ee8e7ca034c1ef3852 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/net/ethernet/cadence/macb_main.c: In function 'macb_mii_init':
>> drivers/net/ethernet/cadence/macb_main.c:596:8: error: 'i' undeclared (first use in this function)
      for (i = 0; i < PHY_MAX_ADDR; i++)
           ^
   drivers/net/ethernet/cadence/macb_main.c:596:8: note: each undeclared identifier is reported only once for each function it appears in

vim +/i +596 drivers/net/ethernet/cadence/macb_main.c

6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  564  
421d9df0 drivers/net/ethernet/cadence/macb.c      Cyrille Pitchen    2015-03-07  565  static int macb_mii_init(struct macb *bp)
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  566  {
84e0cdb0 drivers/net/ethernet/cadence/macb.c      Jamie Iles         2011-03-08  567  	struct macb_platform_data *pdata;
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  568  	struct device_node *np;
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  569  	int err;
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  570  
3dbda77e drivers/net/macb.c                       Uwe Kleine-Koenig  2009-07-23  571  	/* Enable management port */
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  572  	macb_writel(bp, NCR, MACB_BIT(MPE));
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  573  
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  574  	bp->mii_bus = mdiobus_alloc();
aa50b552 drivers/net/ethernet/cadence/macb.c      Moritz Fischer     2016-03-29  575  	if (!bp->mii_bus) {
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  576  		err = -ENOMEM;
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  577  		goto err_out;
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  578  	}
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  579  
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  580  	bp->mii_bus->name = "MACB_mii_bus";
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  581  	bp->mii_bus->read = &macb_mdio_read;
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  582  	bp->mii_bus->write = &macb_mdio_write;
98d5e57e drivers/net/ethernet/cadence/macb.c      Florian Fainelli   2012-01-09  583  	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
98d5e57e drivers/net/ethernet/cadence/macb.c      Florian Fainelli   2012-01-09  584  		 bp->pdev->name, bp->pdev->id);
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  585  	bp->mii_bus->priv = bp;
cf669660 drivers/net/ethernet/cadence/macb.c      Florian Fainelli   2016-05-02  586  	bp->mii_bus->parent = &bp->pdev->dev;
c607a0d9 drivers/net/ethernet/cadence/macb.c      Jingoo Han         2013-08-30  587  	pdata = dev_get_platdata(&bp->pdev->dev);
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  588  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  589  	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  590  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  591  	np = bp->pdev->dev.of_node;
dacdbb4d drivers/net/ethernet/cadence/macb.c      Michael Grzeschik  2017-06-23  592  
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  593  	if (np) {
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  594  		err = of_mdiobus_register(bp->mii_bus, np);
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  595  	} else {
83a77e9e drivers/net/ethernet/cadence/macb.c      Bartosz Folta      2016-12-14 @596  		for (i = 0; i < PHY_MAX_ADDR; i++)
83a77e9e drivers/net/ethernet/cadence/macb.c      Bartosz Folta      2016-12-14  597  			bp->mii_bus->irq[i] = PHY_POLL;
83a77e9e drivers/net/ethernet/cadence/macb.c      Bartosz Folta      2016-12-14  598  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  599  		if (pdata)
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  600  			bp->mii_bus->phy_mask = pdata->phy_mask;
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  601  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  602  		err = mdiobus_register(bp->mii_bus);
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  603  	}
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  604  
148cbb53 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-22  605  	if (err)
e7f4dc35 drivers/net/ethernet/cadence/macb.c      Andrew Lunn        2016-01-06  606  		goto err_out_free_mdiobus;
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  607  
7daa78e3 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-27  608  	err = macb_mii_probe(bp->dev);
7daa78e3 drivers/net/ethernet/cadence/macb.c      Boris Brezillon    2013-08-27  609  	if (err)
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  610  		goto err_out_unregister_bus;
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  611  
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  612  	return 0;
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  613  
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  614  err_out_unregister_bus:
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  615  	mdiobus_unregister(bp->mii_bus);
9ce98140 drivers/net/ethernet/cadence/macb_main.c Michael Grzeschik  2017-11-08  616  	if (np && of_phy_is_fixed_link(np))
9ce98140 drivers/net/ethernet/cadence/macb_main.c Michael Grzeschik  2017-11-08  617  		of_phy_deregister_fixed_link(np);
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  618  err_out_free_mdiobus:
6576bbb8 drivers/net/ethernet/cadence/macb_main.c Brad Mouring       2018-03-12  619  	of_node_put(bp->phy_node);
298cf9be drivers/net/macb.c                       Lennert Buytenhek  2008-10-08  620  	mdiobus_free(bp->mii_bus);
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  621  err_out:
6c36a707 drivers/net/macb.c                       frederic RODO      2007-07-12  622  	return err;
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  623  }
89e5785f drivers/net/macb.c                       Haavard Skinnemoen 2006-11-09  624  

:::::: The code at line 596 was first introduced by commit
:::::: 83a77e9ec4150ee4acc635638f7dedd9da523a26 net: macb: Added PCI wrapper for Platform Driver.

:::::: TO: Bartosz Folta <bfolta@cadence.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29486 bytes --]

[PATCH v3 net-next 2/4] net: macb: Remove redundant poll irq assignment

In phy_device's general probe, this device will already be set for
phy register polling, rendering this code redundant.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 9b6195fbbf8e..db1dc301bed3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -593,9 +593,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
 
-- 
2.16.2

[PATCH v3 net-next 2/4] net: macb: Remove redundant poll irq assignment

[Brad Mouring]

In phy_device's general probe, this device will already be set for
phy register polling, rendering this code redundant.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 9b6195fbbf8e..db1dc301bed3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -593,9 +593,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
 
-- 
2.16.2

Re: [PATCH v3 net-next 2/4] net: macb: Remove redundant poll irq assignment

[Florian Fainelli]

On 03/12/2018 10:09 AM, Brad Mouring wrote:
> In phy_device's general probe, this device will already be set for
> phy register polling, rendering this code redundant.
> 
> Signed-off-by: Brad Mouring <brad.mouring@ni.com>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v3 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db1dc301bed3..b0700a531f3e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -488,6 +488,9 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
+			/* attempt to find a phy-handle */
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+
 			/* fallback to standard phy registration if no phy were
 			 * found during dt phy registration
 			 */
-- 
2.16.2

[PATCH v3 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db1dc301bed3..b0700a531f3e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -488,6 +488,9 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
+			/* attempt to find a phy-handle */
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+
 			/* fallback to standard phy registration if no phy were
 			 * found during dt phy registration
 			 */
-- 
2.16.2

Re: [PATCH v3 net-next 3/4] net: macb: Add phy-handle DT support

[Andrew Lunn]

> @@ -488,6 +488,9 @@ static int macb_mii_probe(struct net_device *dev)
>  			}
>  			bp->phy_node = of_node_get(np);
>  		} else {
> +			/* attempt to find a phy-handle */
> +			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
> +

Hi Brad

It seems like you should check the return value here, any only
continue with the fallback if there was not a phy-handle.

>  			/* fallback to standard phy registration if no phy were
>  			 * found during dt phy registration
>  			 */

	 Andrew

[PATCH v4 net-next 0/4]

[Brad Mouring]

Consider the situation where a macb netdev is connected through
a phydev that sits on a mii bus other than the one provided to
this particular netdev. This situation is what this patchset aims
to accomplish through the existing phy-handle optional binding.

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is precisely the
situation this patchset aims to solve, so it makes sense to introduce
the functionality to this driver (where the physical layout discussed
was encountered).

The devicetree snippet would look something like this:

...
   ethernet@feedf00d {
           ...
           phy-handle = <&phy0> // the first netdev is physically wired to phy0
           ...
           phy0: phy@0 {
                   ...
                   reg = <0x0> // MDIO address 0
                   ...
           }
           phy1: phy@1 {
                   ...
                   reg = <0x1> // MDIO address 1
                   ...
           }
   ...
   }

   ethernet@deadbeef {
           ...
           phy-handle = <&phy1> // tells the driver to use phy1 on the
                                // first mac's mdio bus (it's wired thusly)
           ...
   }
...

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

v2: Reorganization of mii probe/init functions, suggested by Andrew Lunn
v3: Moved some of the bus init code back into init (erroneously moved to probe)
    some style issues, and an unintialized variable warning addressed.
v4: Add Reviewed-by: tags
    Skip fallback code if phy-handle phandle is found

[PATCH v4 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..9b6195fbbf8e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
-	int ret;
+	struct device_node *np;
+	int phy_irq, ret, i;
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	np = bp->pdev->dev.of_node;
+	ret = 0;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +522,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,39 +589,9 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
-
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
 
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			bp->mii_bus->irq[i] = PHY_POLL;
@@ -610,10 +613,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v4 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..9b6195fbbf8e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
-	int ret;
+	struct device_node *np;
+	int phy_irq, ret, i;
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	np = bp->pdev->dev.of_node;
+	ret = 0;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +522,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,39 +589,9 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
-
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
 
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			bp->mii_bus->irq[i] = PHY_POLL;
@@ -610,10 +613,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v4 net-next 2/4] net: macb: Remove redundant poll irq assignment

In phy_device's general probe, this device will already be set for
phy register polling, rendering this code redundant.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 9b6195fbbf8e..db1dc301bed3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -593,9 +593,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
 
-- 
2.16.2

[PATCH v4 net-next 2/4] net: macb: Remove redundant poll irq assignment

[Brad Mouring]

In phy_device's general probe, this device will already be set for
phy register polling, rendering this code redundant.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 9b6195fbbf8e..db1dc301bed3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -593,9 +593,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
 
-- 
2.16.2

[PATCH v4 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 34 ++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db1dc301bed3..4a2ad35d41aa 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -488,23 +488,27 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						ret = PTR_ERR(phydev);
-						break;
+			/* attempt to find a phy-handle */
+			if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
+
+				/* fallback to standard phy registration if no phy were
+				 * found during dt phy registration
+				 */
+				if (!phy_find_first(bp->mii_bus)) {
+					for (i = 0; i < PHY_MAX_ADDR; i++) {
+						struct phy_device *phydev;
+	
+						phydev = mdiobus_scan(bp->mii_bus, i);
+						if (IS_ERR(phydev) &&
+						    PTR_ERR(phydev) != -ENODEV) {
+							ret = PTR_ERR(phydev);
+							break;
+						}
 					}
+	
+					if (ret)
+						return -ENODEV;
 				}
-
-				if (ret)
-					return -ENODEV;
 			}
 		}
 	}
-- 
2.16.2

[PATCH v4 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 34 ++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db1dc301bed3..4a2ad35d41aa 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -488,23 +488,27 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
-
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						ret = PTR_ERR(phydev);
-						break;
+			/* attempt to find a phy-handle */
+			if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
+
+				/* fallback to standard phy registration if no phy were
+				 * found during dt phy registration
+				 */
+				if (!phy_find_first(bp->mii_bus)) {
+					for (i = 0; i < PHY_MAX_ADDR; i++) {
+						struct phy_device *phydev;
+	
+						phydev = mdiobus_scan(bp->mii_bus, i);
+						if (IS_ERR(phydev) &&
+						    PTR_ERR(phydev) != -ENODEV) {
+							ret = PTR_ERR(phydev);
+							break;
+						}
 					}
+	
+					if (ret)
+						return -ENODEV;
 				}
-
-				if (ret)
-					return -ENODEV;
 			}
 		}
 	}
-- 
2.16.2

Re: [PATCH v4 net-next 3/4] net: macb: Add phy-handle DT support

[Andrew Lunn]

> +			/* attempt to find a phy-handle */
> +			if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
> +
> +				/* fallback to standard phy registration if no phy were
> +				 * found during dt phy registration
> +				 */
> +				if (!phy_find_first(bp->mii_bus)) {
> +					for (i = 0; i < PHY_MAX_ADDR; i++) {
> +						struct phy_device *phydev;
> +	
> +						phydev = mdiobus_scan(bp->mii_bus, i);
> +						if (IS_ERR(phydev) &&
> +						    PTR_ERR(phydev) != -ENODEV) {
> +							ret = PTR_ERR(phydev);
> +							break;
> +						}

Hi Brad

./scipts/checkpatch.pl ~/brad.mouring 
WARNING: line over 80 characters
#122: FILE: drivers/net/ethernet/cadence/macb_main.c:492:
+		if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {

ERROR: do not use assignment in if condition
#122: FILE: drivers/net/ethernet/cadence/macb_main.c:492:
+		if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {

CHECK: Blank lines aren't necessary after an open brace '{'
#123: FILE: drivers/net/ethernet/cadence/macb_main.c:493:
+		if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
+

WARNING: line over 80 characters
#124: FILE: drivers/net/ethernet/cadence/macb_main.c:494:
+			/* fallback to standard phy registration if no phy were

ERROR: trailing whitespace
#130: FILE: drivers/net/ethernet/cadence/macb_main.c:500:
+^I$

WARNING: line over 80 characters
#131: FILE: drivers/net/ethernet/cadence/macb_main.c:501:
+					phydev = mdiobus_scan(bp->mii_bus, i);

WARNING: Too many leading tabs - consider code refactoring
#132: FILE: drivers/net/ethernet/cadence/macb_main.c:502:
+					if (IS_ERR(phydev) &&

etc

	Andrew

Re: [PATCH v4 net-next 3/4] net: macb: Add phy-handle DT support

[Florian Fainelli]

On 03/12/2018 02:57 PM, Andrew Lunn wrote:
>> +			/* attempt to find a phy-handle */
>> +			if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
>> +
>> +				/* fallback to standard phy registration if no phy were
>> +				 * found during dt phy registration
>> +				 */
>> +				if (!phy_find_first(bp->mii_bus)) {
>> +					for (i = 0; i < PHY_MAX_ADDR; i++) {
>> +						struct phy_device *phydev;
>> +	
>> +						phydev = mdiobus_scan(bp->mii_bus, i);
>> +						if (IS_ERR(phydev) &&
>> +						    PTR_ERR(phydev) != -ENODEV) {
>> +							ret = PTR_ERR(phydev);
>> +							break;
>> +						}
> 
> Hi Brad

I would be a bit relaxed on these warnings because the existing function
indentation really makes it easy to be over 80 columns. Unless you have
a preliminary patch which, as I was suggesting earlier, re-arranges the
branches such that if (!np) is the first thing you test, I don't see how
this can look any better...

> 
> ./scipts/checkpatch.pl ~/brad.mouring 
> WARNING: line over 80 characters
> #122: FILE: drivers/net/ethernet/cadence/macb_main.c:492:
> +		if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
> 
> ERROR: do not use assignment in if condition
> #122: FILE: drivers/net/ethernet/cadence/macb_main.c:492:
> +		if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
> 
> CHECK: Blank lines aren't necessary after an open brace '{'
> #123: FILE: drivers/net/ethernet/cadence/macb_main.c:493:
> +		if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
> +
> 
> WARNING: line over 80 characters
> #124: FILE: drivers/net/ethernet/cadence/macb_main.c:494:
> +			/* fallback to standard phy registration if no phy were
> 
> ERROR: trailing whitespace
> #130: FILE: drivers/net/ethernet/cadence/macb_main.c:500:
> +^I$
> 
> WARNING: line over 80 characters
> #131: FILE: drivers/net/ethernet/cadence/macb_main.c:501:
> +					phydev = mdiobus_scan(bp->mii_bus, i);
> 
> WARNING: Too many leading tabs - consider code refactoring
> #132: FILE: drivers/net/ethernet/cadence/macb_main.c:502:
> +					if (IS_ERR(phydev) &&
> 
> etc
> 
> 	Andrew
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Florian

Re: [PATCH v4 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

On Mon, Mar 12, 2018 at 03:30:53PM -0700, Florian Fainelli wrote:
> On 03/12/2018 02:57 PM, Andrew Lunn wrote:
> >> +			/* attempt to find a phy-handle */
> >> +			if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
> >> +
> >> +				/* fallback to standard phy registration if no phy were
> >> +				 * found during dt phy registration
> >> +				 */
> >> +				if (!phy_find_first(bp->mii_bus)) {
> >> +					for (i = 0; i < PHY_MAX_ADDR; i++) {
> >> +						struct phy_device *phydev;
> >> +	
> >> +						phydev = mdiobus_scan(bp->mii_bus, i);
> >> +						if (IS_ERR(phydev) &&
> >> +						    PTR_ERR(phydev) != -ENODEV) {
> >> +							ret = PTR_ERR(phydev);
> >> +							break;
> >> +						}
> > 
> > Hi Brad
> 
> I would be a bit relaxed on these warnings because the existing function
> indentation really makes it easy to be over 80 columns. Unless you have
> a preliminary patch which, as I was suggesting earlier, re-arranges the
> branches such that if (!np) is the first thing you test, I don't see how
> this can look any better...

I'll try to work this a bit more, thanks for the feedback and the
patience.

-- 
Brad

Re: [PATCH v4 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

On Mon, Mar 12, 2018 at 03:30:53PM -0700, Florian Fainelli wrote:
> On 03/12/2018 02:57 PM, Andrew Lunn wrote:
> >> +			/* attempt to find a phy-handle */
> >> +			if (!(bp->phy_node = of_parse_phandle(np, "phy-handle", 0))) {
> >> +
> >> +				/* fallback to standard phy registration if no phy were
> >> +				 * found during dt phy registration
> >> +				 */
> >> +				if (!phy_find_first(bp->mii_bus)) {
> >> +					for (i = 0; i < PHY_MAX_ADDR; i++) {
> >> +						struct phy_device *phydev;
> >> +	
> >> +						phydev = mdiobus_scan(bp->mii_bus, i);
> >> +						if (IS_ERR(phydev) &&
> >> +						    PTR_ERR(phydev) != -ENODEV) {
> >> +							ret = PTR_ERR(phydev);
> >> +							break;
> >> +						}
> > 
> > Hi Brad
> 
> I would be a bit relaxed on these warnings because the existing function
> indentation really makes it easy to be over 80 columns. Unless you have
> a preliminary patch which, as I was suggesting earlier, re-arranges the
> branches such that if (!np) is the first thing you test, I don't see how
> this can look any better...

I'll try to work this a bit more, thanks for the feedback and the
patience.

-- 
Brad

[PATCH v5 net-next 0/4] net: macb: Introduce phy-handle DT functionality

[Brad Mouring]

Consider the situation where a macb netdev is connected through
a phydev that sits on a mii bus other than the one provided to
this particular netdev. This situation is what this patchset aims
to accomplish through the existing phy-handle optional binding.

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is precisely the
situation this patchset aims to solve, so it makes sense to introduce
the functionality to this driver (where the physical layout discussed
was encountered).

The devicetree snippet would look something like this:

...
   ethernet@feedf00d {
           ...
           phy-handle = <&phy0> // the first netdev is physically wired to phy0
           ...
           phy0: phy@0 {
                   ...
                   reg = <0x0> // MDIO address 0
                   ...
           }
           phy1: phy@1 {
                   ...
                   reg = <0x1> // MDIO address 1
                   ...
           }
           ...
   }

   ethernet@deadbeef {
           ...
           phy-handle = <&phy1> // tells the driver to use phy1 on the
                                // first mac's mdio bus (it's wired thusly)
           ...
   }
...

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

v2: Reorganization of mii probe/init functions, suggested by Andrew Lunn
v3: Moved some of the bus init code back into init (erroneously moved to probe)
    some style issues, and an unintialized variable warning addressed.
v4: Add Reviewed-by: tags
    Skip fallback code if phy-handle phandle is found
v5: Cleanup formatting issues
    Fix compile failure introduced in 1/4 "net: macb: Reorganize macb_mii
        bringup"
    Fix typo in "Documentation: macb: Document phy-handle binding"

[PATCH v5 net-next 0/4] net: macb: Introduce phy-handle DT functionality

[Brad Mouring]

Consider the situation where a macb netdev is connected through
a phydev that sits on a mii bus other than the one provided to
this particular netdev. This situation is what this patchset aims
to accomplish through the existing phy-handle optional binding.

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is precisely the
situation this patchset aims to solve, so it makes sense to introduce
the functionality to this driver (where the physical layout discussed
was encountered).

The devicetree snippet would look something like this:

...
   ethernet@feedf00d {
           ...
           phy-handle = <&phy0> // the first netdev is physically wired to phy0
           ...
           phy0: phy@0 {
                   ...
                   reg = <0x0> // MDIO address 0
                   ...
           }
           phy1: phy@1 {
                   ...
                   reg = <0x1> // MDIO address 1
                   ...
           }
           ...
   }

   ethernet@deadbeef {
           ...
           phy-handle = <&phy1> // tells the driver to use phy1 on the
                                // first mac's mdio bus (it's wired thusly)
           ...
   }
...

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

v2: Reorganization of mii probe/init functions, suggested by Andrew Lunn
v3: Moved some of the bus init code back into init (erroneously moved to probe)
    some style issues, and an unintialized variable warning addressed.
v4: Add Reviewed-by: tags
    Skip fallback code if phy-handle phandle is found
v5: Cleanup formatting issues
    Fix compile failure introduced in 1/4 "net: macb: Reorganize macb_mii
        bringup"
    Fix typo in "Documentation: macb: Document phy-handle binding"

[PATCH v5 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..4b514c705e57 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
-	int ret;
+	struct device_node *np;
+	int phy_irq, ret, i;
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	np = bp->pdev->dev.of_node;
+	ret = 0;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +522,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err, i;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,39 +589,9 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
-
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
 
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
-
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			bp->mii_bus->irq[i] = PHY_POLL;
@@ -610,10 +613,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v5 net-next 1/4] net: macb: Reorganize macb_mii bringup

[Brad Mouring]

The macb mii setup (mii_probe() and mii_init()) previously was
somewhat interspersed, likely a result of organic growth and hacking.

This change moves mii bus registration into mii_init and probing the
bus for devices into mii_probe.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 79 +++++++++++++++++---------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf1ecb5..4b514c705e57 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -472,8 +472,42 @@ static int macb_mii_probe(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_platform_data *pdata;
 	struct phy_device *phydev;
-	int phy_irq;
-	int ret;
+	struct device_node *np;
+	int phy_irq, ret, i;
+
+	pdata = dev_get_platdata(&bp->pdev->dev);
+	np = bp->pdev->dev.of_node;
+	ret = 0;
+
+	if (np) {
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				return -ENODEV;
+			}
+			bp->phy_node = of_node_get(np);
+		} else {
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						ret = PTR_ERR(phydev);
+						break;
+					}
+				}
+
+				if (ret)
+					return -ENODEV;
+			}
+		}
+	}
 
 	if (bp->phy_node) {
 		phydev = of_phy_connect(dev, bp->phy_node,
@@ -488,7 +522,6 @@ static int macb_mii_probe(struct net_device *dev)
 			return -ENXIO;
 		}
 
-		pdata = dev_get_platdata(&bp->pdev->dev);
 		if (pdata) {
 			if (gpio_is_valid(pdata->phy_irq_pin)) {
 				ret = devm_gpio_request(&bp->pdev->dev,
@@ -533,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err = -ENXIO, i;
+	int err, i;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -556,39 +589,9 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				goto err_out_unregister_bus;
-			}
-			bp->phy_node = of_node_get(np);
-
-			err = mdiobus_register(bp->mii_bus);
-		} else {
-			/* try dt phy registration */
-			err = of_mdiobus_register(bp->mii_bus, np);
-
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
-			 */
-			if (!err && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					struct phy_device *phydev;
 
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						err = PTR_ERR(phydev);
-						break;
-					}
-				}
-
-				if (err)
-					goto err_out_unregister_bus;
-			}
-		}
+	if (np) {
+		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			bp->mii_bus->irq[i] = PHY_POLL;
@@ -610,10 +613,10 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
-	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
-- 
2.16.2

[PATCH v5 net-next 2/4] net: macb: Remove redundant poll irq assignment

[Brad Mouring]

In phy_device's general probe, this device will already be set for
phy register polling, rendering this code redundant.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4b514c705e57..db1dc301bed3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -566,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -593,9 +593,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
 
-- 
2.16.2

[PATCH v5 net-next 2/4] net: macb: Remove redundant poll irq assignment

[Brad Mouring]

In phy_device's general probe, this device will already be set for
phy register polling, rendering this code redundant.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4b514c705e57..db1dc301bed3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -566,7 +566,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err, i;
+	int err;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -593,9 +593,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		err = of_mdiobus_register(bp->mii_bus, np);
 	} else {
-		for (i = 0; i < PHY_MAX_ADDR; i++)
-			bp->mii_bus->irq[i] = PHY_POLL;
-
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
 
-- 
2.16.2

[PATCH v5 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db1dc301bed3..d09bd43680b3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -488,10 +488,12 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+			/* fallback to standard phy registration if no
+			 * phy-handle was found nor any phy found during
+			 * dt phy registration
 			 */
-			if (!phy_find_first(bp->mii_bus)) {
+			if (!bp->phy_node && !phy_find_first(bp->mii_bus)) {
 				for (i = 0; i < PHY_MAX_ADDR; i++) {
 					struct phy_device *phydev;
 
-- 
2.16.2

[PATCH v5 net-next 3/4] net: macb: Add phy-handle DT support

[Brad Mouring]

This optional binding (as described in the ethernet DT bindings doc)
directs the netdev to the phydev to use. This is useful for a phy
chip that has >1 phy in it, and two netdevs are using the same phy
chip (i.e. the second mac's phy lives on the first mac's MDIO bus)

The devicetree snippet would look something like this:

ethernet@feedf00d {
	...
	phy-handle = <&phy0> // the first netdev is physically wired to phy0
	...
	phy0: phy@0 {
		...
		reg = <0x0> // MDIO address 0
		...
	}
	phy1: phy@1 {
		...
		reg = <0x1> // MDIO address 1
		...
	}
...
}

ethernet@deadbeef {
	...
	phy-handle = <&phy1> // tells the driver to use phy1 on the
						 // first mac's mdio bus (it's wired thusly)
	...
}

The work done to add the phy_node in the first place (dacdbb4dfc1a1:
"net: macb: add fixed-link node support") will consume the
device_node (if found).

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db1dc301bed3..d09bd43680b3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -488,10 +488,12 @@ static int macb_mii_probe(struct net_device *dev)
 			}
 			bp->phy_node = of_node_get(np);
 		} else {
-			/* fallback to standard phy registration if no phy were
-			 * found during dt phy registration
+			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
+			/* fallback to standard phy registration if no
+			 * phy-handle was found nor any phy found during
+			 * dt phy registration
 			 */
-			if (!phy_find_first(bp->mii_bus)) {
+			if (!bp->phy_node && !phy_find_first(bp->mii_bus)) {
 				for (i = 0; i < PHY_MAX_ADDR; i++) {
 					struct phy_device *phydev;
 
-- 
2.16.2

[PATCH v5 net-next 4/4] Documentation: macb: Document phy-handle binding

[Brad Mouring]

Document the existence of the optional binding, directing to the
general ethernet document that describes this binding.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

[PATCH v5 net-next 4/4] Documentation: macb: Document phy-handle binding

[Brad Mouring]

Document the existence of the optional binding, directing to the
general ethernet document that describes this binding.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

Re: [PATCH v5 net-next 0/4] net: macb: Introduce phy-handle DT functionality

[David Miller]

From: Brad Mouring <brad.mouring@ni.com>
Date: Tue, 13 Mar 2018 16:32:12 -0500

> Consider the situation where a macb netdev is connected through
> a phydev that sits on a mii bus other than the one provided to
> this particular netdev. This situation is what this patchset aims
> to accomplish through the existing phy-handle optional binding.
> 
> This optional binding (as described in the ethernet DT bindings doc)
> directs the netdev to the phydev to use. This is precisely the
> situation this patchset aims to solve, so it makes sense to introduce
> the functionality to this driver (where the physical layout discussed
> was encountered).
> 
> The devicetree snippet would look something like this:
 ...
> The work done to add the phy_node in the first place (dacdbb4dfc1a1:
> "net: macb: add fixed-link node support") will consume the
> device_node (if found).
> 
> v2: Reorganization of mii probe/init functions, suggested by Andrew Lunn
> v3: Moved some of the bus init code back into init (erroneously moved to probe)
>     some style issues, and an unintialized variable warning addressed.
> v4: Add Reviewed-by: tags
>     Skip fallback code if phy-handle phandle is found
> v5: Cleanup formatting issues
>     Fix compile failure introduced in 1/4 "net: macb: Reorganize macb_mii
>         bringup"
>     Fix typo in "Documentation: macb: Document phy-handle binding"

Series applied, thank you.

[PATCH v4 net-next 4/4] Documentation: macb: Document phy-handle binding

[Brad Mouring]

Document the existance of the optional binding, directing to the
general ethernet document that describes this binding.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

[PATCH v4 net-next 4/4] Documentation: macb: Document phy-handle binding

[Brad Mouring]

Document the existance of the optional binding, directing to the
general ethernet document that describes this binding.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

[PATCH v3 net-next 4/4] Documentation: macb: Document phy-handle binding

[Brad Mouring]

Document the existance of the optional binding, directing to the
general ethernet document that describes this binding.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

[PATCH v3 net-next 4/4] Documentation: macb: Document phy-handle binding

[Brad Mouring]

Document the existance of the optional binding, directing to the
general ethernet document that describes this binding.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 27966ae741e0..457d5ae16f23 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -29,6 +29,7 @@ Optional properties for PHY child node:
 - reset-gpios : Should specify the gpio for phy reset
 - magic-packet : If present, indicates that the hardware supports waking
   up via magic packet.
+- phy-handle : see ethernet.txt file in the same directory
 
 Examples:
 
-- 
2.16.2

Re: [PATCH v3 net-next 4/4] Documentation: macb: Document phy-handle binding

[Florian Fainelli]

On 03/12/2018 10:10 AM, Brad Mouring wrote:
> Document the existance of the optional binding, directing to the
> general ethernet document that describes this binding.
> 
> Signed-off-by: Brad Mouring <brad.mouring@ni.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4174 bytes --]

Hi Brad,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Brad-Mouring/net-macb-Reorganize-macb_mii-bringup/20180311-133616
config: i386-randconfig-x012-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/cadence/macb_main.c: In function 'macb_probe':
>> drivers/net/ethernet/cadence/macb_main.c:503:8: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
        if (ret)
           ^
   drivers/net/ethernet/cadence/macb_main.c:477:6: note: 'ret' was declared here
     int ret, i;
         ^~~

vim +/ret +503 drivers/net/ethernet/cadence/macb_main.c

   468	
   469	/* based on au1000_eth. c*/
   470	static int macb_mii_probe(struct net_device *dev)
   471	{
   472		struct macb *bp = netdev_priv(dev);
   473		struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
   474		struct phy_device *phydev;
   475		struct device_node *np = bp->pdev->dev.of_node;
   476		int phy_irq;
   477		int ret, i;
   478	
   479		if (np) {
   480			if (of_phy_is_fixed_link(np)) {
   481				if (of_phy_register_fixed_link(np) < 0) {
   482					dev_err(&bp->pdev->dev,
   483						"broken fixed-link specification\n");
   484					return -ENODEV;
   485				}
   486				bp->phy_node = of_node_get(np);
   487			} else {
   488				/* fallback to standard phy registration if no phy were
   489				 * found during dt phy registration
   490				 */
   491				if (!phy_find_first(bp->mii_bus)) {
   492					for (i = 0; i < PHY_MAX_ADDR; i++) {
   493						struct phy_device *phydev;
   494	
   495						phydev = mdiobus_scan(bp->mii_bus, i);
   496						if (IS_ERR(phydev) &&
   497						    PTR_ERR(phydev) != -ENODEV) {
   498							ret = PTR_ERR(phydev);
   499							break;
   500						}
   501					}
   502	
 > 503					if (ret)
   504						return -ENODEV;
   505				}
   506			}
   507		} else {
   508			for (i = 0; i < PHY_MAX_ADDR; i++)
   509				bp->mii_bus->irq[i] = PHY_POLL;
   510	
   511			if (pdata)
   512				bp->mii_bus->phy_mask = pdata->phy_mask;
   513	
   514		}
   515	
   516		if (bp->phy_node) {
   517			phydev = of_phy_connect(dev, bp->phy_node,
   518						&macb_handle_link_change, 0,
   519						bp->phy_interface);
   520			if (!phydev)
   521				return -ENODEV;
   522		} else {
   523			phydev = phy_find_first(bp->mii_bus);
   524			if (!phydev) {
   525				netdev_err(dev, "no PHY found\n");
   526				return -ENXIO;
   527			}
   528	
   529			if (pdata) {
   530				if (gpio_is_valid(pdata->phy_irq_pin)) {
   531					ret = devm_gpio_request(&bp->pdev->dev,
   532								pdata->phy_irq_pin, "phy int");
   533					if (!ret) {
   534						phy_irq = gpio_to_irq(pdata->phy_irq_pin);
   535						phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
   536					}
   537				} else {
   538					phydev->irq = PHY_POLL;
   539				}
   540			}
   541	
   542			/* attach the mac to the phy */
   543			ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
   544						 bp->phy_interface);
   545			if (ret) {
   546				netdev_err(dev, "Could not attach to PHY\n");
   547				return ret;
   548			}
   549		}
   550	
   551		/* mask with MAC supported features */
   552		if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
   553			phydev->supported &= PHY_GBIT_FEATURES;
   554		else
   555			phydev->supported &= PHY_BASIC_FEATURES;
   556	
   557		if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
   558			phydev->supported &= ~SUPPORTED_1000baseT_Half;
   559	
   560		phydev->advertising = phydev->supported;
   561	
   562		bp->link = 0;
   563		bp->speed = 0;
   564		bp->duplex = -1;
   565	
   566		return 0;
   567	}
   568	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34070 bytes --]

Re: [PATCH v2 net-next 1/3] net: macb: Reorganize macb_mii bringup

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4174 bytes --]

Hi Brad,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Brad-Mouring/net-macb-Reorganize-macb_mii-bringup/20180311-133616
config: i386-randconfig-x012-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/cadence/macb_main.c: In function 'macb_probe':
>> drivers/net/ethernet/cadence/macb_main.c:503:8: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
        if (ret)
           ^
   drivers/net/ethernet/cadence/macb_main.c:477:6: note: 'ret' was declared here
     int ret, i;
         ^~~

vim +/ret +503 drivers/net/ethernet/cadence/macb_main.c

   468	
   469	/* based on au1000_eth. c*/
   470	static int macb_mii_probe(struct net_device *dev)
   471	{
   472		struct macb *bp = netdev_priv(dev);
   473		struct macb_platform_data *pdata = dev_get_platdata(&bp->pdev->dev);
   474		struct phy_device *phydev;
   475		struct device_node *np = bp->pdev->dev.of_node;
   476		int phy_irq;
   477		int ret, i;
   478	
   479		if (np) {
   480			if (of_phy_is_fixed_link(np)) {
   481				if (of_phy_register_fixed_link(np) < 0) {
   482					dev_err(&bp->pdev->dev,
   483						"broken fixed-link specification\n");
   484					return -ENODEV;
   485				}
   486				bp->phy_node = of_node_get(np);
   487			} else {
   488				/* fallback to standard phy registration if no phy were
   489				 * found during dt phy registration
   490				 */
   491				if (!phy_find_first(bp->mii_bus)) {
   492					for (i = 0; i < PHY_MAX_ADDR; i++) {
   493						struct phy_device *phydev;
   494	
   495						phydev = mdiobus_scan(bp->mii_bus, i);
   496						if (IS_ERR(phydev) &&
   497						    PTR_ERR(phydev) != -ENODEV) {
   498							ret = PTR_ERR(phydev);
   499							break;
   500						}
   501					}
   502	
 > 503					if (ret)
   504						return -ENODEV;
   505				}
   506			}
   507		} else {
   508			for (i = 0; i < PHY_MAX_ADDR; i++)
   509				bp->mii_bus->irq[i] = PHY_POLL;
   510	
   511			if (pdata)
   512				bp->mii_bus->phy_mask = pdata->phy_mask;
   513	
   514		}
   515	
   516		if (bp->phy_node) {
   517			phydev = of_phy_connect(dev, bp->phy_node,
   518						&macb_handle_link_change, 0,
   519						bp->phy_interface);
   520			if (!phydev)
   521				return -ENODEV;
   522		} else {
   523			phydev = phy_find_first(bp->mii_bus);
   524			if (!phydev) {
   525				netdev_err(dev, "no PHY found\n");
   526				return -ENXIO;
   527			}
   528	
   529			if (pdata) {
   530				if (gpio_is_valid(pdata->phy_irq_pin)) {
   531					ret = devm_gpio_request(&bp->pdev->dev,
   532								pdata->phy_irq_pin, "phy int");
   533					if (!ret) {
   534						phy_irq = gpio_to_irq(pdata->phy_irq_pin);
   535						phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
   536					}
   537				} else {
   538					phydev->irq = PHY_POLL;
   539				}
   540			}
   541	
   542			/* attach the mac to the phy */
   543			ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
   544						 bp->phy_interface);
   545			if (ret) {
   546				netdev_err(dev, "Could not attach to PHY\n");
   547				return ret;
   548			}
   549		}
   550	
   551		/* mask with MAC supported features */
   552		if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
   553			phydev->supported &= PHY_GBIT_FEATURES;
   554		else
   555			phydev->supported &= PHY_BASIC_FEATURES;
   556	
   557		if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
   558			phydev->supported &= ~SUPPORTED_1000baseT_Half;
   559	
   560		phydev->advertising = phydev->supported;
   561	
   562		bp->link = 0;
   563		bp->speed = 0;
   564		bp->duplex = -1;
   565	
   566		return 0;
   567	}
   568	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34070 bytes --]