Re: mv88e6240 configuration broken for B850v3

quoted

quoted

Just out of curiosity, can you try this change? It looks like a
simple
case of mismatched conditions inside the mv88e6xxx driver between
what
is supposed to force the link down and what is supposed to force it
up:

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 20f183213cbc..d235270babf7 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c

@@ -1221,7 +1221,7 @@ int dsa_port_link_register_of(struct dsa_port

*dp)
                if (of_phy_is_fixed_link(dp->dn) || phy_np) {
                        if (ds->ops->phylink_mac_link_down)
                                ds->ops->phylink_mac_link_down(ds, port,
-                                       MLO_AN_FIXED, PHY_INTERFACE_MODE_NA);
+                                       MLO_AN_PHY, PHY_INTERFACE_MODE_NA);
                        return dsa_port_phylink_register(dp);
                }
                return 0;

Yes, that appears to also make it work.

Martyn

Well, I just pointed out what the problem is, I don't know how to solve
it, honest! :)

It's clear that the code is wrong, because it's in an "if" block that
checks for "of_phy_is_fixed_link(dp->dn) || phy_np" but then it omits
the "phy_np" part of it. On the other hand we can't just go ahead and
say "if (phy_np) mode = MLO_AN_PHY; else mode = MLO_AN_FIXED;" because
MLO_AN_INBAND is also a valid option that we may be omitting. So we'd
have to duplicate part of the logic from phylink_parse_mode(), which
does not appear ideal at all. What would be ideal is if this fabricated
phylink call would not be done at all, but I don't know enough about the
systems that need it, I expect Andrew knows more.

phylink assumes interfaces start in the down state. It will then
configure them and bring them up as needed. This is not always true
with mv88e6xxx, the interface can already by up, and then the hardware
and phylink have different state information. So this call was added
to force the link down before phylink took control of it.

So i suspect something is missing when phylink sometime later does
bring the interface up. It is not fully undoing what this down
does. Maybe enable the dev_dbg() in mv88e6xxx_port_set_link() and see
what value it has in both the good and bad case?

     Andrew