From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH net] net: phylink: fix link state on phy-connect Date: Wed, 29 Nov 2017 19:59:12 +0000 Message-ID: <20171129195911.GG8356@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> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="gBBFr7Ir9EOA20Yy" 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" To: Yan Markman Return-path: Content-Disposition: inline In-Reply-To: <20448667430e434aad5bb8cd1b082611@IL-EXCH01.marvell.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org --gBBFr7Ir9EOA20Yy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Nov 29, 2017 at 07:33:44PM +0000, Yan Markman wrote: > Hi Russel > > On my board I have [Marvell 88E1510] phy working with STATUS-POLLING > I see some inconsistencies -- first ifconfig-up is different from furthers, no "link is down" reports. > Please refer the behavior example below. > My patch is a "simple solution" -- always reset/clear Link-state-parameters before going UP. > Possibly, more correct (but much more complicated) solution would be in the phy state machine and phylink resolve modification. > I just found that > On ifconfig-down, the phy-state-machine and phylink-resolve > are stopped before executing before passing over full graceful down/reset state. > The further ifconfig-up starts with old state parameters. > Special cases not-tested but logic 2 test-cases are: > remote side changes speed whilst link is Down or Disconnected. But local ifconfig-up starts with old speed. Hi, I think this is covered in my "phy" branch - but could probably do with further testing, specifically this patch (which I've attached): "phylink: ensure we take the link down when phylink_stop() is called" This takes the link down on the MAC side synchronously when phylink_stop() is called. However, I think your case might also benefit from this patch - please test the patch referred to without this change, and let me know if you need this change to solve your problem: diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 8f43f8779317..c90ad50204b0 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -798,6 +798,7 @@ void phylink_disconnect_phy(struct phylink *pl) mutex_lock(&pl->state_mutex); pl->netdev->phydev = NULL; pl->phydev = NULL; + pl->phy_state.link = false; mutex_unlock(&pl->state_mutex); mutex_unlock(&phy->lock); flush_work(&pl->resolve); Thanks. -- 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 --gBBFr7Ir9EOA20Yy Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=p21 From: Russell King Subject: [PATCH] phylink: ensure we take the link down when phylink_stop() is called Ensure that we tell the MAC to take the link down when phylink_stop() is called. Reported-by: Florian Fainelli Tested-by: Florian Fainelli Signed-off-by: Russell King --- drivers/net/phy/phylink.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index fb7ae3f925f8..cb446b8acac2 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -880,6 +880,7 @@ void phylink_stop(struct phylink *pl) sfp_upstream_stop(pl->sfp_bus); set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state); + queue_work(system_power_efficient_wq, &pl->resolve); flush_work(&pl->resolve); } EXPORT_SYMBOL_GPL(phylink_stop); -- 2.7.4 --gBBFr7Ir9EOA20Yy--