Subject: Re: bug: net: dsa: mv88e6xxx: unable to tx or rx with Clearfog GT 8K (with git bisect)

From: Andrew Lunn <[email-masked]>
Date: Fri, 17 Jul 2020 21:42:37 +0200

On Fri, Jul 17, 2020 at 07:51:19PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, Jul 17, 2020 at 12:50:07PM +0000, Martin Rowe wrote:
> > On Fri, 17 Jul 2020 at 09:22, Russell King - ARM Linux admin
> > <[email-masked]> wrote:
> > > The key file is /sys/kernel/debug/mv88e6xxx.0/regs - please send the
> > > contents of that file.
> > 
> > $ cat regs.broken
> >     GLOBAL GLOBAL2 SERDES     0    1    2    3    4    5
> >  0:  c800       0    ffff  9e07 9e4f 100f 100f 9e4f 170b
> >  1:     0    803e    ffff     3    3    3    3    3 201f
>                                                       ^^^^
> This is where the problem is.
> 
> >  1:     0    803e    ffff     3    3    3    3    3 203f
>                                                       ^^^^
> 
> In the broken case, the link is forced down, in the working case, the
> link is forced up.
> 
> What seems to be happening is:
> 
> dsa_port_link_register_of() gets called, and we do this:
> 
>                 phy_np = of_parse_phandle(dp->dn, "phy-handle", 0);
>                 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);
>                         return dsa_port_phylink_register(dp);
> 
> which forces the link down, and for some reason the link never comes
> back up.
>
> One of the issues here is of_phy_is_fixed_link() - it is dangerous.
> The function name leads you astray - it suggests that if it returns
> true, then you have a fixed link, but it also returns true of you
> have managed!="auto" in DT, so it's actually fixed-or-inband-link.
> 
> Andrew, any thoughts?

Hi Russell

I think that is my change, if i remember correctly. Something to do
with phylink assuming all interfaces are down to begin with. But DSA
and CPU links were defaulting to up. When phylink later finds the
fixed-link it then configures the interface up again, and because the
interface is up, nothing actually happens, or it ends up in the wrong
mode. So i think my intention was, if there is a fixed link in DT,
down the interface before registering it with phylink, so its
assumptions are true, and it will later be correctly configured up.

So in this case, do you think we are falling into the trap of
managed!="auto" ?

	Andrew