Re: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect

[Russell King - ARM Linux <linux@armlinux.org.uk>]

On Thu, Nov 30, 2017 at 10:10:18AM +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 30, 2017 at 08:51:21AM +0000, Yan Markman wrote:
> > The phylink_stop is called before phylink_disconnect_phy
> > You could see in mvpp2.c:
> > 
> > mvpp2_stop_dev() {
> > 	phylink_stop(port->phylink); 
> > }
> > 
> > mvpp2_stop()       { 
> > 	mvpp2_stop_dev(port);
> > 	phylink_disconnect_phy(port->phylink);
> > }
> > 
> > .ndo_stop = mvpp2_stop,
> 
> Sorry, I don't have this in mvpp2.c, so I have no visibility of what
> you're working with.
> 
> What you have above looks correct, and I see no reason why the p21
> patch would not have resolved your issue.  The p21 patch ensures
> that phylink_resolve() gets called and completes before phylink_stop()
> returns.  In that case, phylink_resolve() will call the mac_link_down()
> method if the link is not already down.  It will also print the "Link
> is Down" message.
> 
> Florian has already tested this patch after encountering a similar
> issue, and has reported that it solves the problem for him.  I've also
> tested it with mvneta, and the original mvpp2x driver on Macchiatobin.
> 
> Maybe there's something different about mvpp2, but as I have no
> visibility of that driver and the modifications therein, I can't
> comment further other than stating that it works for three different
> implementations.
> 
> Maybe you could try and work out what's going on with the p21 patch
> in your case?

I think I now realise what's probably going on.

If you call netif_carrier_off() before phylink_stop(), then phylink will
believe that the link is already down, and so it won't bother calling
mac_link_down() - it will believe that the link is already down.

I'll update the documentation for phylink_stop() to spell out this
aspect.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up