sky2: enable rx/tx in sky2_phy_reinit()

sky2_phy_reinit is called by the ethtool helpers sky2_set_settings, sky2_nway_reset and sky2_set_pauseparam when netif_running.

However, at the end of sky2_phy_init GM_GP_CTRL has GM_GPCR_RX_ENA and GM_GPCR_TX_ENA cleared. So, doing these commands causes the device to stop working:

$ ethtool -r eth0
$ ethtool -A eth0 autoneg off

Fix this issue by enabling Rx/Tx after running sky2_phy_init in sky2_phy_reinit.

Signed-off-by Brandon Philips <[email protected]>

Tested-by Brandon Philips <[email protected]>

Cc [email protected]

---
 drivers/net/sky2.c |   19 ++++++++++++++-----

 1 files changed, 14 insertions(+), 5 deletions(-)

drivers/net/sky2.c +14 -6

index 2111c7b..7985165 100644
@@ -717,11 +717,24 @@ static void sky2_phy_power_down(struct sky2_hw *hw, unsigned port)
        sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
  }
 
+/* Enable Rx/Tx */
+static void sky2_enable_rx_tx(struct sky2_port *sky2)
+{
+       struct sky2_hw *hw = sky2->hw;
+       unsigned port = sky2->port;
+       u16 reg;
+
+       reg = gma_read16(hw, port, GM_GP_CTRL);
+       reg |= GM_GPCR_RX_ENA | GM_GPCR_TX_ENA;
+       gma_write16(hw, port, GM_GP_CTRL, reg);
+}
+
  /* Force a renegotiation */
  static void sky2_phy_reinit(struct sky2_port *sky2)
  {
        spin_lock_bh(&sky2->phy_lock);
        sky2_phy_init(sky2->hw, sky2->port);
+       sky2_enable_rx_tx(sky2);
        spin_unlock_bh(&sky2->phy_lock);
  }
 

@@ -2040,7 +2053,6 @@ static void sky2_link_up(struct sky2_port *sky2)
  {
        struct sky2_hw *hw = sky2->hw;
        unsigned port = sky2->port;
-       u16 reg;
        static const char *fc_name[] = {
                [FC_NONE]       = "none",
                [FC_TX]         = "tx",

@@ -2048,10 +2060,7 @@ static void sky2_link_up(struct sky2_port *sky2)
                [FC_BOTH]       = "both",
        };
 
-       /* enable Rx/Tx */
-       reg = gma_read16(hw, port, GM_GP_CTRL);
-       reg |= GM_GPCR_RX_ENA | GM_GPCR_TX_ENA;
-       gma_write16(hw, port, GM_GP_CTRL, reg);
+       sky2_enable_rx_tx(sky2);
 
        gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_DEF_MSK);
 
--

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Tested, and verified that it fixes the bug reported.

From 8de50faa1911933dc545f663a24f32d0caeea3b4 Mon Sep 17 00:00:00 2001

While testing this I just noticed that if I turn off speed autoneg the interface stops sending packets until I turn off pause autoneg too.

To illustrate start pinging some machine and run this:

 $ ethtool --change eth0 speed 100 duplex full autoneg off
 # At this point the ping stops

 $ ethtool -A eth0 autoneg off
 # Ping starts up again

When I tried capturing packets nothing was coming from the device.

0ea065e52eb6a0f029b5fa5ed2f142be1b66a153 implemeneted the behaviour of having seperate pause and speed ethtool controls for sky2. Reverting this (see quick ugly revert below) obviously fixes the issue.

Any ideas on how to fix this in a proper way though?

From 8de50faa1911933dc545f663a24f32d0caeea3b4 Mon Sep 17 00:00:00 2001
From: Brandon Philips <[email protected]>
Date: Wed, 23 Jun 2010 09:56:20 -0700
Subject: [PATCH] sky2: revert "fix pause negotiation"

Revert 0ea065e52eb6a0f029b5fa5ed2f142be1b66a153

---
 drivers/net/sky2.c |   31 ++++++++++++-------------------
 drivers/net/sky2.h |    2 ++
 2 files changed, 14 insertions(+), 19 deletions(-)

drivers/net/sky2.c +12 -19

index 7985165..a638171 100644
@@ -333,7 +333,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 	struct sky2_port *sky2 = netdev_priv(hw->dev[port]);
 	u16 ctrl, ct1000, adv, pg, ledctrl, ledover, reg;
 
-	if ( (sky2->flags & SKY2_FLAG_AUTO_SPEED) &&
+	if (sky2->autoneg == AUTONEG_ENABLE &&
 	    !(hw->flags & SKY2_HW_NEWER_PHY)) {
 		u16 ectrl = gm_phy_read(hw, port, PHY_MARV_EXT_CTRL);
 

@@ -375,7 +375,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 			ctrl |= PHY_M_PC_MDI_XMODE(PHY_M_PC_ENA_AUTO);
 
 			/* downshift on PHY 88E1112 and 88E1149 is changed */
-			if ( (sky2->flags & SKY2_FLAG_AUTO_SPEED) &&
+			if (sky2->autoneg == AUTONEG_ENABLE &&
 			     (hw->flags & SKY2_HW_NEWER_PHY)) {
 				/* set downshift counter to 3x and enable downshift */
 				ctrl &= ~PHY_M_PC_DSC_MSK;

@@ -420,7 +420,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 	adv = PHY_AN_CSMA;
 	reg = 0;
 
-	if (sky2->flags & SKY2_FLAG_AUTO_SPEED) {
+	if (sky2->autoneg == AUTONEG_ENABLE) {
 		if (sky2_is_copper(hw)) {
 			if (sky2->advertising & ADVERTISED_1000baseT_Full)
 				ct1000 |= PHY_M_1000C_AFD;

@@ -435,11 +435,14 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 			if (sky2->advertising & ADVERTISED_10baseT_Half)
 				adv |= PHY_M_AN_10_HD;
 
+			adv |= copper_fc_adv[sky2->flow_mode];
 		} else {	/* special defines for FIBER (88E1040S only) */
 			if (sky2->advertising & ADVERTISED_1000baseT_Full)
 				adv |= PHY_M_AN_1000X_AFD;
 			if (sky2->advertising & ADVERTISED_1000baseT_Half)
 				adv |= PHY_M_AN_1000X_AHD;
+
+			adv |= fiber_fc_adv[sky2->flow_mode];
 		}
 
 		/* Restart Auto-negotiation */

@@ -448,8 +451,8 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 		/* forced speed/duplex settings */
 		ct1000 = PHY_M_1000C_MSE;
 
-		/* Disable auto update for duplex flow control and duplex */
-		reg |= GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_SPD_DIS;
+		/* Disable auto update for duplex flow control and speed */
+		reg |= GM_GPCR_AU_ALL_DIS;
 
 		switch (sky2->speed) {
 		case SPEED_1000:

@@ -467,15 +470,8 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 			ctrl |= PHY_CT_DUP_MD;
 		} else if (sky2->speed < SPEED_1000)
 			sky2->flow_mode = FC_NONE;
-	}
 
-	if (sky2->flags & SKY2_FLAG_AUTO_PAUSE) {
-		if (sky2_is_copper(hw))
-			adv |= copper_fc_adv[sky2->flow_mode];
-		else
-			adv |= fiber_fc_adv[sky2->flow_mode];
-	} else {
-		reg |= GM_GPCR_AU_FCT_DIS;
+
  		reg |= gm_fc_disable[sky2->flow_mode];
 
 		/* Forward pause packets to GMAC? */

@@ -620,8 +616,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 		/* no effect on Yukon-XL */
 		gm_phy_write(hw, port, PHY_MARV_LED_CTRL, ledctrl);
 
-		if (!(sky2->flags & SKY2_FLAG_AUTO_SPEED) ||
-		    sky2->speed == SPEED_100) {
+		if (sky2->autoneg == AUTONEG_DISABLE || sky2->speed == SPEED_100) {
 			/* turn on 100 Mbps LED (LED_LINK100) */
 			ledover |= PHY_M_LED_MO_100(MO_LED_ON);
 		}

@@ -632,7 +627,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 	}
 
 	/* Enable phy interrupt on auto-negotiation complete (or link up) */
-	if (sky2->flags & SKY2_FLAG_AUTO_SPEED)
+	if (sky2->autoneg == AUTONEG_ENABLE)
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_IS_AN_COMPL);
 	else
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_DEF_MSK);

@@ -688,9 +683,7 @@ static void sky2_phy_power_down(struct sky2_hw *hw, unsigned port)
 
 	/* setup General Purpose Control Register */
 	gma_write16(hw, port, GM_GP_CTRL,
-		    GM_GPCR_FL_PASS | GM_GPCR_SPEED_100 |
-		    GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_FCT_DIS |
-		    GM_GPCR_AU_SPD_DIS);
+		    GM_GPCR_FL_PASS | GM_GPCR_SPEED_100 | GM_GPCR_AU_ALL_DIS);
 
 	if (hw->chip_id != CHIP_ID_YUKON_EC) {
 		if (hw->chip_id == CHIP_ID_YUKON_EC_U) {

drivers/net/sky2.h +2 -0

index 084eff2..db0b2ad 100644
@@ -1755,6 +1755,7 @@ enum {
 };
 
 #define GM_GPCR_SPEED_1000	(GM_GPCR_GIGS_ENA | GM_GPCR_SPEED_100)
+#define GM_GPCR_AU_ALL_DIS	(GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_FCT_DIS|GM_GPCR_AU_SPD_DIS)
 
 /*	GM_TX_CTRL			16 bit r/w	Transmit Control Register */
 enum {

@@ -2247,6 +2248,7 @@ struct sky2_port {
 	u16		     speed;		/* SPEED_1000, SPEED_100, ... */
 	u8		     wol;		/* WAKE_ bits */
 	u8		     duplex;		/* DUPLEX_HALF, DUPLEX_FULL */
+	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */
 	u16		     flags;
 #define SKY2_FLAG_RX_CHECKSUM		0x0001
 #define SKY2_FLAG_AUTO_SPEED		0x0002

Signature · 1 line

1.7.1

Quoted 11 lines · Brandon Philips

While testing this I just noticed that if I turn off speed autoneg the interface stops sending packets until I turn off pause autoneg too.

To illustrate start pinging some machine and run this:

 $ ethtool --change eth0 speed 100 duplex full autoneg off
 # At this point the ping stops

 $ ethtool -A eth0 autoneg off
 # Ping starts up again

Quoted 7 lines

When I tried capturing packets nothing was coming from the device.

0ea065e52eb6a0f029b5fa5ed2f142be1b66a153 implemeneted the behaviour of having seperate pause and speed ethtool controls for sky2. Reverting this (see quick ugly revert below) obviously fixes the issue.

Any ideas on how to fix this in a proper way though?

sky2_phy_reinit is called by the ethtool helpers sky2_set_settings, sky2_nway_reset and sky2_set_pauseparam when netif_running.

However, at the end of sky2_phy_init GM_GP_CTRL has GM_GPCR_RX_ENA and GM_GPCR_TX_ENA cleared. So, doing these commands causes the device to stop working:

$ ethtool -r eth0
$ ethtool -A eth0 autoneg off

Fix this issue by enabling Rx/Tx after running sky2_phy_init in sky2_phy_reinit.

Signed-off-by Brandon Philips <[email protected]>

Tested-by Brandon Philips <[email protected]>

Cc [email protected]