of: mdio: Support fixed links in of_phy_get_and_connect()

Message ID 20180711174511.15308-1-linus.walleij@linaro.org
State Accepted
Delegated to: David Miller
Headers show
Series
  • of: mdio: Support fixed links in of_phy_get_and_connect()
Related show

Commit Message

Linus Walleij July 11, 2018, 5:45 p.m.
By a simple extension of of_phy_get_and_connect() drivers
that have a fixed link on e.g. RGMII can support also
fixed links, so in addition to:

ethernet-port {
	phy-mode = "rgmii";
	phy-handle = <&foo>;
};

This setup with a fixed-link node and no phy-handle will
now also work just fine:

ethernet-port {
	phy-mode = "rgmii";
	fixed-link {
		speed = <1000>;
		full-duplex;
		pause;
	};
};

This is very helpful for connecting random ethernet ports
to e.g. DSA switches that typically reside on fixed links.

The phy-mode is still there as the fixes link in this case
is still an RGMII link.

Tested on the Cortina Gemini driver with the Vitesse DSA
router chip on a fixed 1Gbit link.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/of/of_mdio.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Andrew Lunn July 11, 2018, 8:04 p.m. | #1
On Wed, Jul 11, 2018 at 07:45:11PM +0200, Linus Walleij wrote:
> By a simple extension of of_phy_get_and_connect() drivers
> that have a fixed link on e.g. RGMII can support also
> fixed links, so in addition to:
> 
> ethernet-port {
> 	phy-mode = "rgmii";
> 	phy-handle = <&foo>;
> };
> 
> This setup with a fixed-link node and no phy-handle will
> now also work just fine:
> 
> ethernet-port {
> 	phy-mode = "rgmii";
> 	fixed-link {
> 		speed = <1000>;
> 		full-duplex;
> 		pause;
> 	};
> };
> 
> This is very helpful for connecting random ethernet ports
> to e.g. DSA switches that typically reside on fixed links.
> 
> The phy-mode is still there as the fixes link in this case
> is still an RGMII link.
> 
> Tested on the Cortina Gemini driver with the Vitesse DSA
> router chip on a fixed 1Gbit link.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

What probably make sense as a followup is add a
of_phy_disconnect_and_put(). When the module is unloaded, you leak a
fixed link, because of_phy_deregister_fixed_link() is not being
called. You also hold a reference to np which does not appear to be
released.

    Andrew
David Miller July 14, 2018, 1:25 a.m. | #2
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 19:45:11 +0200

> By a simple extension of of_phy_get_and_connect() drivers
> that have a fixed link on e.g. RGMII can support also
> fixed links, so in addition to:
> 
> ethernet-port {
> 	phy-mode = "rgmii";
> 	phy-handle = <&foo>;
> };
> 
> This setup with a fixed-link node and no phy-handle will
> now also work just fine:
> 
> ethernet-port {
> 	phy-mode = "rgmii";
> 	fixed-link {
> 		speed = <1000>;
> 		full-duplex;
> 		pause;
> 	};
> };
> 
> This is very helpful for connecting random ethernet ports
> to e.g. DSA switches that typically reside on fixed links.
> 
> The phy-mode is still there as the fixes link in this case
> is still an RGMII link.
> 
> Tested on the Cortina Gemini driver with the Vitesse DSA
> router chip on a fixed 1Gbit link.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied.

Patch

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index d963baf8e53a..e92391d6d1bd 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -367,14 +367,23 @@  struct phy_device *of_phy_get_and_connect(struct net_device *dev,
 	phy_interface_t iface;
 	struct device_node *phy_np;
 	struct phy_device *phy;
+	int ret;
 
 	iface = of_get_phy_mode(np);
 	if (iface < 0)
 		return NULL;
-
-	phy_np = of_parse_phandle(np, "phy-handle", 0);
-	if (!phy_np)
-		return NULL;
+	if (of_phy_is_fixed_link(np)) {
+		ret = of_phy_register_fixed_link(np);
+		if (ret < 0) {
+			netdev_err(dev, "broken fixed-link specification\n");
+			return NULL;
+		}
+		phy_np = of_node_get(np);
+	} else {
+		phy_np = of_parse_phandle(np, "phy-handle", 0);
+		if (!phy_np)
+			return NULL;
+	}
 
 	phy = of_phy_connect(dev, phy_np, hndlr, 0, iface);