From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752361AbdK3N24 (ORCPT ); Thu, 30 Nov 2017 08:28:56 -0500 Received: from pandora.armlinux.org.uk ([78.32.30.218]:55828 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751810AbdK3N2z (ORCPT ); Thu, 30 Nov 2017 08:28:55 -0500 Date: Thu, 30 Nov 2017 13:28:30 +0000 From: Russell King - ARM Linux To: Yan Markman Cc: Antoine Tenart , "andrew@lunn.ch" , "f.fainelli@gmail.com" , "davem@davemloft.net" , "gregory.clement@free-electrons.com" , "thomas.petazzoni@free-electrons.com" , "miquel.raynal@free-electrons.com" , Nadav Haklai , "mw@semihalf.com" , Stefan Chulski , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect Message-ID: <20171130132830.GA5529@n2100.armlinux.org.uk> References: <20171128132932.27196-1-antoine.tenart@free-electrons.com> <20171128155317.GA7974@flint.armlinux.org.uk> <20171128155611.GA8358@flint.armlinux.org.uk> <20448667430e434aad5bb8cd1b082611@IL-EXCH01.marvell.com> <20171129195911.GG8356@n2100.armlinux.org.uk> <21ec97be76d54a6c8a80fd5b56d35678@IL-EXCH01.marvell.com> <20171129212032.GI8356@n2100.armlinux.org.uk> <20171130101018.GA10595@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171130101018.GA10595@n2100.armlinux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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