All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers
@ 2012-07-25 17:59 Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 1/6] ethtool.h: MDI setting support Jesse Brandeburg
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 17:59 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

This is an update (v2) to the patches sent previously.

This series implements MDI-X set support in the Intel gigabit ethernet
drivers.  It specifically allows a user to control the MDI state,
in order for them to work around issues with broken switches.

The only change to this series is updated descriptions and an
addition of reporting the current mdix_ctrl value in the
"get_settings" functions, which allows the new extended reporting
in the ethtool app's MDI-X: line

sent as RFC for now to get feedback while internal testing
completes. (and net-next is closed)

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

---

Bruce W Allan (1):
      e1000e: implement 82577/579 MDI setting support

Jesse Brandeburg (5):
      igb: update to allow reading/setting MDI state
      e1000e: implement MDI/MDI-X control
      e1000: configure and read MDI settings
      igb: implement 580 MDI setting support
      ethtool.h: MDI setting support


 drivers/net/ethernet/intel/e1000/e1000_ethtool.c |   39 ++++++++++++++++++++
 drivers/net/ethernet/intel/e1000/e1000_main.c    |    4 ++
 drivers/net/ethernet/intel/e1000e/ethtool.c      |   41 ++++++++++++++++++++-
 drivers/net/ethernet/intel/e1000e/phy.c          |   31 +++++++++++++++-
 drivers/net/ethernet/intel/igb/e1000_phy.c       |   29 ++++++++++++++-
 drivers/net/ethernet/intel/igb/e1000_phy.h       |    5 ++-
 drivers/net/ethernet/intel/igb/igb_ethtool.c     |   42 ++++++++++++++++++++++
 drivers/net/ethernet/intel/igb/igb_main.c        |    4 ++
 include/linux/ethtool.h                          |   17 ++++++---
 9 files changed, 198 insertions(+), 14 deletions(-)

--
Jesse Brandeburg

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC NET-NEXT PATCH 1/6] ethtool.h: MDI setting support
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
@ 2012-07-25 18:00 ` Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 2/6] e1000e: implement 82577/579 " Jesse Brandeburg
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 18:00 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

This change modifies the core ethtool struct to allow a driver to
support setting of MDI/MDI-X state for twisted pair wiring.  This
change uses a previously reserved u8 and should not change any
binary compatibility of ethtool.

Also as per Ben Hutchings' suggestion, the capabilities are
stored in a separate byte so the driver can report if it supports
changing settings.

see thread: http://kerneltrap.org/mailarchive/linux-netdev/2010/11/17/6289820/thread

see ethtool patches titled:
ethtool: allow setting MDI-X state

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
---

 include/linux/ethtool.h |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 21eff41..fcb4f8e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -45,8 +45,10 @@ struct ethtool_cmd {
 				 * bits) in Mbps. Please use
 				 * ethtool_cmd_speed()/_set() to
 				 * access it */
-	__u8	eth_tp_mdix;
-	__u8	reserved2;
+	__u8	eth_tp_mdix;	/* twisted pair MDI-X status */
+	__u8    eth_tp_mdix_ctrl; /* twisted pair MDI-X control, when set,
+				   * link should be renegotiated if necessary
+				   */
 	__u32	lp_advertising;	/* Features the link partner advertises */
 	__u32	reserved[2];
 };
@@ -1229,10 +1231,13 @@ struct ethtool_ops {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
-/* Mode MDI or MDI-X */
-#define ETH_TP_MDI_INVALID	0x00
-#define ETH_TP_MDI		0x01
-#define ETH_TP_MDI_X		0x02
+/* MDI or MDI-X status/control - if MDI/MDI_X/AUTO is set then
+ * the driver is required to renegotiate link
+ */
+#define ETH_TP_MDI_INVALID	0x00 /* status: unknown; control: unsupported */
+#define ETH_TP_MDI		0x01 /* status: MDI;     control: force MDI */
+#define ETH_TP_MDI_X		0x02 /* status: MDI-X;   control: force MDI-X */
+#define ETH_TP_MDI_AUTO		0x03 /*                  control: auto-select */
 
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC NET-NEXT PATCH 2/6] e1000e: implement 82577/579 MDI setting support
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 1/6] ethtool.h: MDI setting support Jesse Brandeburg
@ 2012-07-25 18:00 ` Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 3/6] igb: implement 580 " Jesse Brandeburg
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 18:00 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

From: Bruce W Allan <bruce.w.allan@intel.com>

in order for e1000e to support MDI setting support via
ethtool this code is needed to allow setting the MDI state
via software.

This is in regards to the related ethtool patch and
fixes bugzilla.kernel.org bug 11998

Signed-off-by: Bruce W Allan <bruce.w.allan@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---

 drivers/net/ethernet/intel/e1000e/phy.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
index b860d4f..fc62a3f 100644
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -84,8 +84,9 @@ static const u16 e1000_igp_2_cable_length_table[] = {
 #define I82577_PHY_STATUS2_SPEED_1000MBPS 0x0200
 
 /* I82577 PHY Control 2 */
-#define I82577_PHY_CTRL2_AUTO_MDIX        0x0400
-#define I82577_PHY_CTRL2_FORCE_MDI_MDIX   0x0200
+#define I82577_PHY_CTRL2_MANUAL_MDIX      0x0200
+#define I82577_PHY_CTRL2_AUTO_MDI_MDIX    0x0400
+#define I82577_PHY_CTRL2_MDIX_CFG_MASK    0x0600
 
 /* I82577 PHY Diagnostics Status */
 #define I82577_DSTATUS_CABLE_LENGTH       0x03FC
@@ -702,6 +703,32 @@ s32 e1000_copper_link_setup_82577(struct e1000_hw *hw)
 	if (ret_val)
 		return ret_val;
 
+	/* Set MDI/MDIX mode */
+	ret_val = e1e_rphy(hw, I82577_PHY_CTRL_2, &phy_data);
+	if (ret_val)
+		return ret_val;
+	phy_data &= ~I82577_PHY_CTRL2_MDIX_CFG_MASK;
+	/*
+	 * Options:
+	 *   0 - Auto (default)
+	 *   1 - MDI mode
+	 *   2 - MDI-X mode
+	 */
+	switch (hw->phy.mdix) {
+	case 1:
+		break;
+	case 2:
+		phy_data |= I82577_PHY_CTRL2_MANUAL_MDIX;
+		break;
+	case 0:
+	default:
+		phy_data |= I82577_PHY_CTRL2_AUTO_MDI_MDIX;
+		break;
+	}
+	ret_val = e1e_wphy(hw, I82577_PHY_CTRL_2, phy_data);
+	if (ret_val)
+		return ret_val;
+
 	return e1000_set_master_slave_mode(hw);
 }
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC NET-NEXT PATCH 3/6] igb: implement 580 MDI setting support
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 1/6] ethtool.h: MDI setting support Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 2/6] e1000e: implement 82577/579 " Jesse Brandeburg
@ 2012-07-25 18:00 ` Jesse Brandeburg
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings Jesse Brandeburg
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 18:00 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

in order for igb to support MDI setting support via
ethtool this code is needed to allow setting the MDI state
via software.

This is in regards to the related ethtool patch

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---

 drivers/net/ethernet/intel/igb/e1000_phy.c |   29 ++++++++++++++++++++++++++--
 drivers/net/ethernet/intel/igb/e1000_phy.h |    5 +++--
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c
index 7be98b6..3404bc7 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.c
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.c
@@ -464,6 +464,32 @@ s32 igb_copper_link_setup_82580(struct e1000_hw *hw)
 	phy_data |= I82580_CFG_ENABLE_DOWNSHIFT;
 
 	ret_val = phy->ops.write_reg(hw, I82580_CFG_REG, phy_data);
+	if (ret_val)
+		goto out;
+
+	/* Set MDI/MDIX mode */
+	ret_val = phy->ops.read_reg(hw, I82580_PHY_CTRL_2, &phy_data);
+	if (ret_val)
+		goto out;
+	phy_data &= ~I82580_PHY_CTRL2_MDIX_CFG_MASK;
+	/*
+	 * Options:
+	 *   0 - Auto (default)
+	 *   1 - MDI mode
+	 *   2 - MDI-X mode
+	 */
+	switch (hw->phy.mdix) {
+	case 1:
+		break;
+	case 2:
+		phy_data |= I82580_PHY_CTRL2_MANUAL_MDIX;
+		break;
+	case 0:
+	default:
+		phy_data |= I82580_PHY_CTRL2_AUTO_MDI_MDIX;
+		break;
+	}
+	ret_val = hw->phy.ops.write_reg(hw, I82580_PHY_CTRL_2, phy_data);
 
 out:
 	return ret_val;
@@ -2246,8 +2272,7 @@ s32 igb_phy_force_speed_duplex_82580(struct e1000_hw *hw)
 	if (ret_val)
 		goto out;
 
-	phy_data &= ~I82580_PHY_CTRL2_AUTO_MDIX;
-	phy_data &= ~I82580_PHY_CTRL2_FORCE_MDI_MDIX;
+	phy_data &= ~I82580_PHY_CTRL2_MDIX_CFG_MASK;
 
 	ret_val = phy->ops.write_reg(hw, I82580_PHY_CTRL_2, phy_data);
 	if (ret_val)
diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.h b/drivers/net/ethernet/intel/igb/e1000_phy.h
index 34e4061..6ac3299 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.h
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.h
@@ -111,8 +111,9 @@ s32  igb_check_polarity_m88(struct e1000_hw *hw);
 #define I82580_PHY_STATUS2_SPEED_100MBPS  0x0100
 
 /* I82580 PHY Control 2 */
-#define I82580_PHY_CTRL2_AUTO_MDIX        0x0400
-#define I82580_PHY_CTRL2_FORCE_MDI_MDIX   0x0200
+#define I82580_PHY_CTRL2_MANUAL_MDIX      0x0200
+#define I82580_PHY_CTRL2_AUTO_MDI_MDIX    0x0400
+#define I82580_PHY_CTRL2_MDIX_CFG_MASK    0x0600
 
 /* I82580 PHY Diagnostics Status */
 #define I82580_DSTATUS_CABLE_LENGTH       0x03FC

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
                   ` (2 preceding siblings ...)
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 3/6] igb: implement 580 " Jesse Brandeburg
@ 2012-07-25 18:00 ` Jesse Brandeburg
  2012-07-25 22:41   ` Ben Hutchings
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control Jesse Brandeburg
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 18:00 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

this is the implementation in e1000 to allow ethtool to force
MDI state, allowing users to work around some improperly
behaving switches.

forcing in this driver is for now only allowed when auto-neg is enabled.

to use must have the matching version of ethtool app that supports
this functionality.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Tushar Dave <tushar.n.dave@intel.com>
---

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c |   39 ++++++++++++++++++++++
 drivers/net/ethernet/intel/e1000/e1000_main.c    |    4 ++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 736a7d9..9089d00 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -174,6 +174,20 @@ static int e1000_get_settings(struct net_device *netdev,
 
 	ecmd->autoneg = ((hw->media_type == e1000_media_type_fiber) ||
 			 hw->autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 1; MDI => 0 */
+	if ((hw->media_type == e1000_media_type_copper) &&
+	    netif_carrier_ok(netdev))
+		ecmd->eth_tp_mdix = (!!adapter->phy_info.mdix_mode ?
+							ETH_TP_MDI_X :
+							ETH_TP_MDI);
+	else
+		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
+
+	if (hw->mdix == AUTO_ALL_MODES)
+		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	else
+		ecmd->eth_tp_mdix_ctrl = hw->mdix;
 	return 0;
 }
 
@@ -183,6 +197,22 @@ static int e1000_set_settings(struct net_device *netdev,
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 
+	/*
+	 * MDI setting is only allowed when autoneg enabled because
+	 * some hardware doesn't allow MDI setting when speed or
+	 * duplex is forced.
+	 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (hw->media_type != e1000_media_type_copper)
+			return -EOPNOTSUPP;
+
+		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+			e_err(drv, "forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
 		msleep(1);
 
@@ -199,12 +229,21 @@ static int e1000_set_settings(struct net_device *netdev,
 		ecmd->advertising = hw->autoneg_advertised;
 	} else {
 		u32 speed = ethtool_cmd_speed(ecmd);
+		/* calling this overrides forced MDI setting */
 		if (e1000_set_spd_dplx(adapter, speed, ecmd->duplex)) {
 			clear_bit(__E1000_RESETTING, &adapter->flags);
 			return -EINVAL;
 		}
 	}
 
+	/* MDI-X => 2; MDI => 1; Auto => 3 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			hw->mdix = AUTO_ALL_MODES;
+		else
+			hw->mdix = ecmd->eth_tp_mdix_ctrl;
+	}
+
 	/* reset the link */
 
 	if (netif_running(adapter->netdev)) {
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3bfbb8d..0ae2fcf 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -4939,6 +4939,10 @@ int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
 	default:
 		goto err_inval;
 	}
+
+	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
+	hw->mdix = AUTO_ALL_MODES;
+
 	return 0;
 
 err_inval:

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
                   ` (3 preceding siblings ...)
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings Jesse Brandeburg
@ 2012-07-25 18:00 ` Jesse Brandeburg
  2012-07-25 22:43   ` Ben Hutchings
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 6/6] igb: update to allow reading/setting MDI state Jesse Brandeburg
  2012-07-25 22:48 ` [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Ben Hutchings
  6 siblings, 1 reply; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 18:00 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

some users report issues with link failing when connected to certain
switches.  This gives the user the ability to control the MDI state
from the driver, allowing users to work around some improperly
behaving switches.

forcing in this driver is for now only allowed when auto-neg is
enabled.

This is in regards to the related ethtool app patch and
bugzilla.kernel.org bug 11998

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: bruce.w.allan@intel.com
CC: n.poppelier@xs4all.nl
CC: bastien@durel.org
CC: jsveiga@it.eng.br
---

 drivers/net/ethernet/intel/e1000e/ethtool.c |   41 ++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 0349e24..2e76f06 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -199,6 +199,11 @@ static int e1000_get_settings(struct net_device *netdev,
 	else
 		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
 
+	if (hw->phy.mdix == AUTO_ALL_MODES)
+		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	else
+		ecmd->eth_tp_mdix_ctrl = hw->phy.mdix;
+
 	return 0;
 }
 
@@ -241,6 +246,10 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
 	default:
 		goto err_inval;
 	}
+
+	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
+	adapter->hw.phy.mdix = AUTO_ALL_MODES;
+
 	return 0;
 
 err_inval:
@@ -264,6 +273,22 @@ static int e1000_set_settings(struct net_device *netdev,
 		return -EINVAL;
 	}
 
+	/*
+	 * MDI setting is only allowed when autoneg enabled because
+	 * some hardware doesn't allow MDI setting when speed or
+	 * duplex is forced.
+	 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (hw->phy.media_type != e1000_media_type_copper)
+			return -EOPNOTSUPP;
+
+		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+			e_err("forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__E1000_RESETTING, &adapter->state))
 		usleep_range(1000, 2000);
 
@@ -282,20 +307,32 @@ static int e1000_set_settings(struct net_device *netdev,
 			hw->fc.requested_mode = e1000_fc_default;
 	} else {
 		u32 speed = ethtool_cmd_speed(ecmd);
+		/* calling this overrides forced MDI setting */
 		if (e1000_set_spd_dplx(adapter, speed, ecmd->duplex)) {
 			clear_bit(__E1000_RESETTING, &adapter->state);
 			return -EINVAL;
 		}
 	}
 
+	/* MDI-X => 2; MDI => 1; Auto => 3 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		/*
+		 * fix up the value for auto (3 => 0) as zero is mapped
+		 * internally to auto
+		 */
+		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			hw->phy.mdix = AUTO_ALL_MODES;
+		else
+			hw->phy.mdix = ecmd->eth_tp_mdix_ctrl;
+	}
+
 	/* reset the link */
 
 	if (netif_running(adapter->netdev)) {
 		e1000e_down(adapter);
 		e1000e_up(adapter);
-	} else {
+	} else
 		e1000e_reset(adapter);
-	}
 
 	clear_bit(__E1000_RESETTING, &adapter->state);
 	return 0;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC NET-NEXT PATCH 6/6] igb: update to allow reading/setting MDI state
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
                   ` (4 preceding siblings ...)
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control Jesse Brandeburg
@ 2012-07-25 18:00 ` Jesse Brandeburg
  2012-07-25 22:48 ` [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Ben Hutchings
  6 siblings, 0 replies; 10+ messages in thread
From: Jesse Brandeburg @ 2012-07-25 18:00 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, jesse.brandeburg

This is the implementation for igb to allow forcing MDI state
via ethtool, allowing users to work around some improperly
behaving switches.

forcing in this driver is for now only allowed when auto-neg is
enabled.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
---

 drivers/net/ethernet/intel/igb/igb_ethtool.c |   42 ++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igb/igb_main.c    |    4 ++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index a19c84c..cf0e9fb 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -198,6 +198,19 @@ static int igb_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 	}
 
 	ecmd->autoneg = hw->mac.autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 2; MDI =>1; Invalid =>0 */
+	if (hw->phy.media_type == e1000_media_type_copper)
+		ecmd->eth_tp_mdix = hw->phy.is_mdix ? ETH_TP_MDI_X :
+						      ETH_TP_MDI;
+	else
+		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
+
+	if (hw->phy.mdix == AUTO_ALL_MODES)
+		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	else
+		ecmd->eth_tp_mdix_ctrl = hw->phy.mdix;
+
 	return 0;
 }
 
@@ -214,6 +227,22 @@ static int igb_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 		return -EINVAL;
 	}
 
+	/*
+	 * MDI setting is only allowed when autoneg enabled because
+	 * some hardware doesn't allow MDI setting when speed or
+	 * duplex is forced.
+	 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (hw->phy.media_type != e1000_media_type_copper)
+			return -EOPNOTSUPP;
+
+		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+			dev_err(&adapter->pdev->dev, "forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__IGB_RESETTING, &adapter->state))
 		msleep(1);
 
@@ -227,12 +256,25 @@ static int igb_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 			hw->fc.requested_mode = e1000_fc_default;
 	} else {
 		u32 speed = ethtool_cmd_speed(ecmd);
+		/* calling this overrides forced MDI setting */
 		if (igb_set_spd_dplx(adapter, speed, ecmd->duplex)) {
 			clear_bit(__IGB_RESETTING, &adapter->state);
 			return -EINVAL;
 		}
 	}
 
+	/* MDI-X => 2; MDI => 1; Auto => 3 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		/*
+		 * fix up the value for auto (3 => 0) as zero is mapped
+		 * internally to auto
+		 */
+		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			hw->phy.mdix = AUTO_ALL_MODES;
+		else
+			hw->phy.mdix = ecmd->eth_tp_mdix_ctrl;
+	}
+
 	/* reset the link */
 	if (netif_running(adapter->netdev)) {
 		igb_down(adapter);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 1050411..447e131 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6676,6 +6676,10 @@ int igb_set_spd_dplx(struct igb_adapter *adapter, u32 spd, u8 dplx)
 	default:
 		goto err_inval;
 	}
+
+	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
+	adapter->hw.phy.mdix = AUTO_ALL_MODES;
+
 	return 0;
 
 err_inval:

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings Jesse Brandeburg
@ 2012-07-25 22:41   ` Ben Hutchings
  0 siblings, 0 replies; 10+ messages in thread
From: Ben Hutchings @ 2012-07-25 22:41 UTC (permalink / raw)
  To: Jesse Brandeburg; +Cc: netdev

On Wed, 2012-07-25 at 11:00 -0700, Jesse Brandeburg wrote:
> this is the implementation in e1000 to allow ethtool to force
> MDI state, allowing users to work around some improperly
> behaving switches.
> 
> forcing in this driver is for now only allowed when auto-neg is enabled.
> 
> to use must have the matching version of ethtool app that supports
> this functionality.
[...]
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -4939,6 +4939,10 @@ int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
>  	default:
>  		goto err_inval;
>  	}
> +
> +	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
> +	hw->mdix = AUTO_ALL_MODES;

You will translate AUTO_ALL_MODES to ETH_TP_MDI_AUTO in
e1000_get_settings, and then treat that as an error in
e1000_set_settings if hw->media_type != e1000_media_type_copper.  So,
does this assignment need to be conditional on hw->media_type ==
e1000_media_type_copper?

Ben.

>  	return 0;
>  
>  err_inval:
> 

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control Jesse Brandeburg
@ 2012-07-25 22:43   ` Ben Hutchings
  0 siblings, 0 replies; 10+ messages in thread
From: Ben Hutchings @ 2012-07-25 22:43 UTC (permalink / raw)
  To: Jesse Brandeburg; +Cc: netdev

On Wed, 2012-07-25 at 11:00 -0700, Jesse Brandeburg wrote:
> some users report issues with link failing when connected to certain
> switches.  This gives the user the ability to control the MDI state
> from the driver, allowing users to work around some improperly
> behaving switches.
> 
> forcing in this driver is for now only allowed when auto-neg is
> enabled.
[...]
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
[...] 
> @@ -241,6 +246,10 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
>  	default:
>  		goto err_inval;
>  	}
> +
> +	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
> +	adapter->hw.phy.mdix = AUTO_ALL_MODES;
> +
[...]

I have the same question about this assignment as in e1000.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers
  2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
                   ` (5 preceding siblings ...)
  2012-07-25 18:00 ` [RFC NET-NEXT PATCH 6/6] igb: update to allow reading/setting MDI state Jesse Brandeburg
@ 2012-07-25 22:48 ` Ben Hutchings
  6 siblings, 0 replies; 10+ messages in thread
From: Ben Hutchings @ 2012-07-25 22:48 UTC (permalink / raw)
  To: Jesse Brandeburg; +Cc: netdev

On Wed, 2012-07-25 at 10:59 -0700, Jesse Brandeburg wrote:
> This is an update (v2) to the patches sent previously.
> 
> This series implements MDI-X set support in the Intel gigabit ethernet
> drivers.  It specifically allows a user to control the MDI state,
> in order for them to work around issues with broken switches.
> 
> The only change to this series is updated descriptions and an
> addition of reporting the current mdix_ctrl value in the
> "get_settings" functions, which allows the new extended reporting
> in the ethtool app's MDI-X: line
> 
> sent as RFC for now to get feedback while internal testing
> completes. (and net-next is closed)
[...]

This all looks sane.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-07-25 22:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-25 17:59 [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Jesse Brandeburg
2012-07-25 18:00 ` [RFC NET-NEXT PATCH 1/6] ethtool.h: MDI setting support Jesse Brandeburg
2012-07-25 18:00 ` [RFC NET-NEXT PATCH 2/6] e1000e: implement 82577/579 " Jesse Brandeburg
2012-07-25 18:00 ` [RFC NET-NEXT PATCH 3/6] igb: implement 580 " Jesse Brandeburg
2012-07-25 18:00 ` [RFC NET-NEXT PATCH 4/6] e1000: configure and read MDI settings Jesse Brandeburg
2012-07-25 22:41   ` Ben Hutchings
2012-07-25 18:00 ` [RFC NET-NEXT PATCH 5/6] e1000e: implement MDI/MDI-X control Jesse Brandeburg
2012-07-25 22:43   ` Ben Hutchings
2012-07-25 18:00 ` [RFC NET-NEXT PATCH 6/6] igb: update to allow reading/setting MDI state Jesse Brandeburg
2012-07-25 22:48 ` [RFC NET-NEXT PATCH 0/6] implement MDI-X set support in Intel drivers Ben Hutchings

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.