From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932129AbdK1P4d (ORCPT ); Tue, 28 Nov 2017 10:56:33 -0500 Received: from pandora.armlinux.org.uk ([78.32.30.218]:51724 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932087AbdK1P4a (ORCPT ); Tue, 28 Nov 2017 10:56:30 -0500 Date: Tue, 28 Nov 2017 15:56:11 +0000 From: Russell King To: Antoine Tenart Cc: andrew@lunn.ch, f.fainelli@gmail.com, davem@davemloft.net, Yan Markman , gregory.clement@free-electrons.com, thomas.petazzoni@free-electrons.com, miquel.raynal@free-electrons.com, nadavh@marvell.com, mw@semihalf.com, stefanc@marvell.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net] net: phylink: fix link state on phy-connect Message-ID: <20171128155611.GA8358@flint.armlinux.org.uk> References: <20171128132932.27196-1-antoine.tenart@free-electrons.com> <20171128155317.GA7974@flint.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171128155317.GA7974@flint.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 Oh, and lastly, please send patches to linux@armlinux.org.uk or the address I use in the sign-offs - sending them to rmk@armlinux.org.uk is for personal non-Linux mail only, and has resulted in _all_ of these messages ending up in my spam folder. Thanks. On Tue, Nov 28, 2017 at 03:53:17PM +0000, Russell King wrote: > On Tue, Nov 28, 2017 at 02:29:32PM +0100, Antoine Tenart wrote: > > From: Yan Markman > > Hi, thanks for the patch. > > > When calling successively _connect, _disconnect and _connect again, if > > the link configuration changed whilst being down from the phylink > > perspective, the last _connect would stay in an incorrect old speed. > > Fixes this by setting the link configuration parameters to an unknown > > value when calling phylink_bringup_phy. > > Under what circumstances does this occur? > > > > > Fixes: 9525ae83959b ("phylink: add phylink infrastructure") > > Signed-off-by: Yan Markman > > [Antoine: commit message] > > Signed-off-by: Antoine Tenart > > --- > > drivers/net/phy/phylink.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > index e3bbc70372d3..c2cec3eef67d 100644 > > --- a/drivers/net/phy/phylink.c > > +++ b/drivers/net/phy/phylink.c > > @@ -621,6 +621,16 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy) > > if (ret) > > return ret; > > > > + /* On _disconnect, the phy state machine and phylink resolve > > + * are stopped before executing full gracefull down/reset state. > > + * The further _connect starts with incorrect init state. Let's set > > + * init values here. > > + */ > > + pl->phy_state.link = false; > > + pl->link_config.pause = MLO_PAUSE_AN; > > + pl->link_config.speed = SPEED_UNKNOWN; > > + pl->link_config.duplex = DUPLEX_UNKNOWN; > > It would be much better to clean up the phy_state in > phylink_disconnect_phy() and trigger a resolve, rather than doing that > each time a PHY is connected - the link should be taken down when the > PHY is removed. > > However, I'd like to know under what circumstances this is happening, > since, if you're hotplugging a PHY you should be doing that via SFP > which has additional link up/down handling. What board is this with? > > Also note that there's a number of patches in my "phy" branch that > I'm intending to send as a result of working with Florian over the > last few weeks. There's several people working fairly independently > in this area and having everyone send patches independently of each > other could get painful to manage. > > I'm intending to send patches once I know that net-next is open. > > -- > Russell King > ARM architecture Linux Kernel maintainer -- Russell King ARM architecture Linux Kernel maintainer