diff mbox

net: phy: turn carrier off on phy attach

Message ID 1452365045-5364-1-git-send-email-sjoerd.simons@collabora.co.uk
State Superseded
Headers show

Commit Message

Sjoerd Simons Jan. 9, 2016, 6:44 p.m. UTC
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 <sjoerd.simons@collabora.co.uk>


---

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

-- 
2.7.0.rc3

Comments

David Miller Jan. 11, 2016, 10:17 p.m. UTC | #1
From: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

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 <sjoerd.simons@collabora.co.uk>


Florian or Andrew, please review this.

Thanks.

> 

> ---

> 

>  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

>
Sjoerd Simons Jan. 14, 2016, 8:22 p.m. UTC | #2
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:

> > 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?


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.
diff mbox

Patch

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)