Thread (12 messages) 12 messages, 3 authors, 2018-08-27

Re: [PATCH 1/3] ARM: dts: NSP: Enable SFP on bcm958625hr

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2018-08-27 21:09:33
Also in: linux-arm-kernel, 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@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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help