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

[Russell King - ARM Linux]

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?

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@xxxxxxxxxxxxxxx] 
> Sent: Wednesday, November 29, 2017 11:21 PM
> To: Yan Markman <ymarkman@xxxxxxxxxxx>
> Cc: Antoine Tenart <antoine.tenart@xxxxxxxxxxxxxxxxxx>; andrew@xxxxxxx; f.fainelli@xxxxxxxxx; davem@xxxxxxxxxxxxx; gregory.clement@xxxxxxxxxxxxxxxxxx; thomas.petazzoni@xxxxxxxxxxxxxxxxxx; miquel.raynal@xxxxxxxxxxxxxxxxxx; Nadav Haklai <nadavh@xxxxxxxxxxx>; mw@xxxxxxxxxxxx; Stefan Chulski <stefanc@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect
> 
> On Wed, Nov 29, 2017 at 09:06:56PM +0000, Yan Markman wrote:
> > The attached p21 patch doesn't change anything.
> > But another one from the mail-text is good
> > 	void phylink_disconnect_phy(struct phylink *pl)
> > 	+		pl->phy_state.link = false;
> > 
> > There still (not for my MRVL-PP2) problem:
> > 	It is expected that on  ifconfig-down the callback
> > 		pl->ops->mac_link_down(ndev, pl->link_an_mode); would be called, but 
> > it isn't
> 
> Are you calling phylink_stop() or are you just calling phylink_disconnect() ?
> 
> You must call phylink_stop() prior to phylink_disconnect().  This probably explains why the p21 patch did nothing.
> 
> --
> 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

-- 
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