[PATCH] net: phy: turn carrier off on phy attach

The operstate of a networking device initially IF_OPER_UNKNOWN aka
"unknown", updated on carrier state changes (with carrier state being on
by default). This means it will stay unknown unless the carrier state
goes to off at some point, which is not the case if the phy is already
up/connected at startup.

Explicitly turn off the carrier on phy attach, leaving the phy state
machine to turn the carrier on when it has done the initial negotiation.

Signed-off-by: Sjoerd Simons <redacted>

---

 drivers/net/phy/phy_device.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0bfbaba..a30ce1a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c

@@ -668,6 +668,11 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 	phydev->state = PHY_READY;
 
+	/* Signal to the core network layer the phy supports
+	 *  carrier detection
+	 */
+	netif_carrier_off(phydev->attached_dev);
+
 	/* Do initial configuration here, now that
 	 * we have certain key parameters
 	 * (dev_flags and interface)

-- 
2.7.0.rc3

From: Sjoerd Simons <redacted>
Date: Sat,  9 Jan 2016 19:44:05 +0100

The operstate of a networking device initially IF_OPER_UNKNOWN aka
"unknown", updated on carrier state changes (with carrier state being on
by default). This means it will stay unknown unless the carrier state
goes to off at some point, which is not the case if the phy is already
up/connected at startup.

Explicitly turn off the carrier on phy attach, leaving the phy state
machine to turn the carrier on when it has done the initial negotiation.

Signed-off-by: Sjoerd Simons <redacted>

Florian or Andrew, please review this.

Thanks.

On Sat, Jan 09, 2016 at 07:44:05PM +0100, Sjoerd Simons wrote:

The operstate of a networking device initially IF_OPER_UNKNOWN aka
"unknown", updated on carrier state changes (with carrier state being on
by default). This means it will stay unknown unless the carrier state
goes to off at some point, which is not the case if the phy is already
up/connected at startup.

Explicitly turn off the carrier on phy attach, leaving the phy state
machine to turn the carrier on when it has done the initial negotiation.

RFC 2863 says:

   Whenever an interface table entry is created (usually as a result
   of system initialization), the relevant instance of ifAdminStatus
   is set to down, and ifOperStatus will be down or notPresent.

and

   The notPresent state is a refinement on the down state which
   indicates that the relevant interface is down specifically because
   some component (typically, a hardware component) is not present in
   the managed system.

So if we have a PHY, setting it to down is correct with respect to the
RFC.

I don't agree with the comment. All we are doing is getting the core
state to agree with the phy state. The next thing we do with the phy
is reset it, so the carrier is going to be off.

Please rewrite the comment, and then i can give a reviewed-by.

Thanks
	Andrew

+	netif_carrier_off(phydev->attached_dev);
+
 	/* Do initial configuration here, now that
 	 * we have certain key parameters
 	 * (dev_flags and interface)
-- 
2.7.0.rc3

On January 9, 2016 10:44:05 AM PST, Sjoerd Simons [off-list ref] wrote:

The operstate of a networking device initially IF_OPER_UNKNOWN aka
"unknown", updated on carrier state changes (with carrier state being
on
by default). This means it will stay unknown unless the carrier state
goes to off at some point, which is not the case if the phy is already
up/connected at startup.

Correct, drivers typically call netif_carrier_off prior to registering the network device to give a predictable link state, regardless of whether or not they use PHYLIB.

Explicitly turn off the carrier on phy attach, leaving the phy state
machine to turn the carrier on when it has done the initial
negotiation.

Same comment as Andrew on the comment below.

Out of curiosity, was there a particular driver you ran into issues with?

-- 
Florian

On Tue, 2016-01-12 at 17:31 -0800, Florian Fainelli wrote:

On January 9, 2016 10:44:05 AM PST, Sjoerd Simons <sjoerd.simons@coll
abora.co.uk> wrote:

quoted

The operstate of a networking device initially IF_OPER_UNKNOWN aka
"unknown", updated on carrier state changes (with carrier state
being
on
by default). This means it will stay unknown unless the carrier
state
goes to off at some point, which is not the case if the phy is
already
up/connected at startup.

Correct, drivers typically call netif_carrier_off prior to
registering the network device to give a predictable link state,
regardless of whether or not they use PHYLIB.

quoted

Explicitly turn off the carrier on phy attach, leaving the phy
state
machine to turn the carrier on when it has done the initial
negotiation.

Same comment as Andrew on the comment below.

Out of curiosity, was there a particular driver you ran into issues
with?

Prepping a v2. This came up on Rada Rock2 board, so the (Rockchip)
DWMAC driver combined with a realtek phy (RTL8211E). Thanks for the
review

-- 
Sjoerd Simons
Collabora Ltd.