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

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:

quoted

From: Yan Markman <redacted>

Hi, thanks for the patch.

quoted

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?

quoted

Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
Signed-off-by: Yan Markman <redacted>
[Antoine: commit message]
Signed-off-by: Antoine Tenart <redacted>
---
 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