[PATCH 1/3] ARM: dts: NSP: Enable SFP on bcm958625hr
From: f.fainelli@gmail.com (Florian Fainelli)
Date: 2018-08-27 21:09:33
Also in:
linux-devicetree, lkml
On 08/27/2018 01:52 PM, Florian Fainelli wrote:
quoted hunk ↗ jump to hunk
On 08/27/2018 01:35 PM, Andrew Lunn wrote:quoted
quoted
@@ -210,6 +228,17 @@ reg = <4>; }; + port at 5 { + label = "sfp"; + phy-mode = "sgmii"; + reg = <5>; + sfp = <&sfp>; + fixed-link { + speed = <1000>; + full-duplex; + };Hi Florian You might want to add a comment about why you are using fixed-link and sgmii, which seems very odd. Is it even correct?Probably not, this is kind of left over from before adding the sfp phandle, but if I do remove it, and I can see the DSA slave network device fail to initialize, likely because we destroy the PHYLINK instance. AFAIR, when we talked about this with Russell, I did not see why we had to comment out the following:diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 962c4fd338ba..f3ae16dbf8d8 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c@@ -1227,7 +1227,7 @@ static int dsa_slave_phy_setup(struct net_device*slave_dev) netdev_err(slave_dev, "failed to connect to port %d: %d\n", dp->index, ret); - phylink_destroy(dp->pl); + //phylink_destroy(dp->pl); return ret; } } maybe you know?
Stupid question, if we have a "sfp" phandle, must one also specify a managed = "in-band-status" property? Under what circumstances are not these two things implying one another (SFF maybe)? That would explain why the code path taken from phylink_of_phy_connect() would not return 0 and we would indeed fail to connect to the built-in DSA MDIO bus. -- Florian