* [net-next 1/6] ethtool.h: MDI setting support
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
@ 2012-07-20 21:43 ` Jeff Kirsher
2012-07-20 21:43 ` [net-next 2/6] e1000e: implement 82577/579 " Jeff Kirsher
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-20 21:43 UTC (permalink / raw)
To: davem
Cc: Jesse Brandeburg, netdev, gospo, sassmann, Ben Hutchings,
Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
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>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
include/linux/ethtool.h | 17 +++++++++++------
1 file 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)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [net-next 2/6] e1000e: implement 82577/579 MDI setting support
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
2012-07-20 21:43 ` [net-next 1/6] ethtool.h: MDI setting support Jeff Kirsher
@ 2012-07-20 21:43 ` Jeff Kirsher
2012-07-20 21:43 ` [net-next 3/6] igb: implement 580 " Jeff Kirsher
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-20 21:43 UTC (permalink / raw)
To: davem
Cc: Bruce W Allan, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
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>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/phy.c | 31 +++++++++++++++++++++++++++++--
1 file 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);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [net-next 3/6] igb: implement 580 MDI setting support
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
2012-07-20 21:43 ` [net-next 1/6] ethtool.h: MDI setting support Jeff Kirsher
2012-07-20 21:43 ` [net-next 2/6] e1000e: implement 82577/579 " Jeff Kirsher
@ 2012-07-20 21:43 ` Jeff Kirsher
2012-07-20 21:43 ` [net-next 4/6] e1000: configure and read MDI settings Jeff Kirsher
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-20 21:43 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
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>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@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
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [net-next 4/6] e1000: configure and read MDI settings
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (2 preceding siblings ...)
2012-07-20 21:43 ` [net-next 3/6] igb: implement 580 " Jeff Kirsher
@ 2012-07-20 21:43 ` Jeff Kirsher
2012-07-20 23:27 ` Ben Hutchings
2012-07-20 21:43 ` [net-next 5/6] e1000e: implement MDI/MDI-X control Jeff Kirsher
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-20 21:43 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Tushar Dave,
Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
This is the implementation in e1000 to allow ethtool to force
MDI state, allowing users to work around some improperly
behaving switches.
Current get_settings behavior slightly changes in that now when link is down
get_settings will return the MDI state of the last link because get_settings
needs to succeed to allow the set to work even when link is down.
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>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 34 ++++++++++++++++++++++
drivers/net/ethernet/intel/e1000/e1000_main.c | 4 +++
2 files changed, 38 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 3103f0b..1d96bda 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -174,6 +174,15 @@ 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)
+ 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;
+
return 0;
}
@@ -183,6 +192,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 +224,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:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [net-next 4/6] e1000: configure and read MDI settings
2012-07-20 21:43 ` [net-next 4/6] e1000: configure and read MDI settings Jeff Kirsher
@ 2012-07-20 23:27 ` Ben Hutchings
2012-07-21 1:17 ` Brandeburg, Jesse
0 siblings, 1 reply; 13+ messages in thread
From: Ben Hutchings @ 2012-07-20 23:27 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann, Tushar Dave
On Fri, 2012-07-20 at 14:43 -0700, Jeff Kirsher wrote:
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> This is the implementation in e1000 to allow ethtool to force
> MDI state, allowing users to work around some improperly
> behaving switches.
>
> Current get_settings behavior slightly changes in that now when link is down
> get_settings will return the MDI state of the last link because get_settings
> needs to succeed to allow the set to work even when link is down.
>
> 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>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 34 ++++++++++++++++++++++
> drivers/net/ethernet/intel/e1000/e1000_main.c | 4 +++
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> index 3103f0b..1d96bda 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> @@ -174,6 +174,15 @@ 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)
> + 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;
[...]
Why don't you set ecmd->eth_tp_mdix_ctrl here?
If you also leave it as 0, it's impossible for userland to tell whether
the current mode was forced or automatically selected.
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] 13+ messages in thread
* Re: [net-next 4/6] e1000: configure and read MDI settings
2012-07-20 23:27 ` Ben Hutchings
@ 2012-07-21 1:17 ` Brandeburg, Jesse
2012-07-21 15:37 ` Ben Hutchings
0 siblings, 1 reply; 13+ messages in thread
From: Brandeburg, Jesse @ 2012-07-21 1:17 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Jeff Kirsher, davem, netdev, gospo, sassmann, Tushar Dave
On Fri, 20 Jul 2012, Ben Hutchings wrote:
> Why don't you set ecmd->eth_tp_mdix_ctrl here?
>
> If you also leave it as 0, it's impossible for userland to tell whether
> the current mode was forced or automatically selected.
Thanks for the review, right now the get interface (and ethtool display)
doesn't support any way to report if the setting was forced or not. I
didn't think about changing the get because I didn't want to modify the
userland reporting (I also figured it was a simple interface right now,
and didn't need changing, and was focused on the _set_ which is the part
fixing the users' reported bugs.)
I think the patches as they currently stand are okay, do you agree? I
would be glad to submit a followon to implement the new "get" interface if
we can hash out the interface changes, but I see no reason to gate these
patches.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net-next 4/6] e1000: configure and read MDI settings
2012-07-21 1:17 ` Brandeburg, Jesse
@ 2012-07-21 15:37 ` Ben Hutchings
2012-07-21 17:37 ` Jeff Kirsher
0 siblings, 1 reply; 13+ messages in thread
From: Ben Hutchings @ 2012-07-21 15:37 UTC (permalink / raw)
To: Brandeburg, Jesse
Cc: Jeff Kirsher, davem, netdev, gospo, sassmann, Tushar Dave
On Fri, 2012-07-20 at 18:17 -0700, Brandeburg, Jesse wrote:
>
> On Fri, 20 Jul 2012, Ben Hutchings wrote:
> > Why don't you set ecmd->eth_tp_mdix_ctrl here?
> >
> > If you also leave it as 0, it's impossible for userland to tell whether
> > the current mode was forced or automatically selected.
>
> Thanks for the review, right now the get interface (and ethtool display)
> doesn't support any way to report if the setting was forced or not. I
> didn't think about changing the get because I didn't want to modify the
> userland reporting (I also figured it was a simple interface right now,
> and didn't need changing, and was focused on the _set_ which is the part
> fixing the users' reported bugs.)
Everything else you can change with ETHTOOL_SSET is also reported by
ETHTOOL_GSET; why would this be any different?
> I think the patches as they currently stand are okay, do you agree? I
> would be glad to submit a followon to implement the new "get" interface if
> we can hash out the interface changes, but I see no reason to gate these
> patches.
You left this for 20 months, what's the hurry now?
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] 13+ messages in thread
* Re: [net-next 4/6] e1000: configure and read MDI settings
2012-07-21 15:37 ` Ben Hutchings
@ 2012-07-21 17:37 ` Jeff Kirsher
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-21 17:37 UTC (permalink / raw)
To: Ben Hutchings
Cc: Brandeburg, Jesse, davem, netdev, gospo, sassmann, Tushar Dave
[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]
On Sat, 2012-07-21 at 16:37 +0100, Ben Hutchings wrote:
>
> On Fri, 2012-07-20 at 18:17 -0700, Brandeburg, Jesse wrote:
> >
> > On Fri, 20 Jul 2012, Ben Hutchings wrote:
> > > Why don't you set ecmd->eth_tp_mdix_ctrl here?
> > >
> > > If you also leave it as 0, it's impossible for userland to tell
> whether
> > > the current mode was forced or automatically selected.
> >
> > Thanks for the review, right now the get interface (and ethtool
> display)
> > doesn't support any way to report if the setting was forced or not.
> I
> > didn't think about changing the get because I didn't want to modify
> the
> > userland reporting (I also figured it was a simple interface right
> now,
> > and didn't need changing, and was focused on the _set_ which is the
> part
> > fixing the users' reported bugs.)
>
> Everything else you can change with ETHTOOL_SSET is also reported by
> ETHTOOL_GSET; why would this be any different?
>
> > I think the patches as they currently stand are okay, do you agree?
> I
> > would be glad to submit a followon to implement the new "get"
> interface if
> > we can hash out the interface changes, but I see no reason to gate
> these
> > patches.
>
> You left this for 20 months, what's the hurry now?
Since there some changes that are needed in this patch set, I will drop
this series from my tree so that I can continue pushing additional
ixgbe/ixgbevf patches.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [net-next 5/6] e1000e: implement MDI/MDI-X control
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (3 preceding siblings ...)
2012-07-20 21:43 ` [net-next 4/6] e1000: configure and read MDI settings Jeff Kirsher
@ 2012-07-20 21:43 ` Jeff Kirsher
2012-07-20 21:43 ` [net-next 6/6] igb: update to allow reading/setting MDI state Jeff Kirsher
2012-07-21 17:37 ` [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-20 21:43 UTC (permalink / raw)
To: davem
Cc: Jesse Brandeburg, netdev, gospo, sassmann, bruce.w.allan,
n.poppelier, bastien, jsveiga, Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
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.
Current get_settings behavior slightly changes in that now when link
is down get_settings will return the MDI state of the last link
because get_settings needs to succeed to allow the set to work even
when link is down.
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
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ethtool.c | 39 ++++++++++++++++++++++++---
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 105d554..9b1e082 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -192,8 +192,7 @@ static int e1000_get_settings(struct net_device *netdev,
hw->mac.autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
/* MDI-X => 2; MDI =>1; Invalid =>0 */
- if ((hw->phy.media_type == e1000_media_type_copper) &&
- netif_carrier_ok(netdev))
+ 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
@@ -241,6 +240,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 +267,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 +301,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;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [net-next 6/6] igb: update to allow reading/setting MDI state
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (4 preceding siblings ...)
2012-07-20 21:43 ` [net-next 5/6] e1000e: implement MDI/MDI-X control Jeff Kirsher
@ 2012-07-20 21:43 ` Jeff Kirsher
2012-07-21 17:37 ` [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-20 21:43 UTC (permalink / raw)
To: davem
Cc: Jesse Brandeburg, netdev, gospo, sassmann, Carolyn Wyborny,
Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
This is the implementation for igb to allow forcing MDI state
via ethtool, allowing users to work around some improperly
behaving switches.
get_settings will now return the MDI state of the last link
because get_settings needs to succeed to allow the set to work even
when link is down.
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>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 37 ++++++++++++++++++++++++++
drivers/net/ethernet/intel/igb/igb_main.c | 4 +++
2 files changed, 41 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index a19c84c..bc3c5b4 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -198,6 +198,14 @@ 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;
+
return 0;
}
@@ -214,6 +222,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 +251,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 8adeca9..4df7848 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6675,6 +6675,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:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates
2012-07-20 21:43 [net-next 0/6][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (5 preceding siblings ...)
2012-07-20 21:43 ` [net-next 6/6] igb: update to allow reading/setting MDI state Jeff Kirsher
@ 2012-07-21 17:37 ` Jeff Kirsher
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2012-07-21 17:37 UTC (permalink / raw)
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]
On Fri, 2012-07-20 at 14:43 -0700, Kirsher, Jeffrey T wrote:
> This series contains updates to ethtool, e1000, e1000e and igb with
> regards to the new MDI ethtool support patches submitted earlier.
>
> The following are changes since commit fa0afcd10951afad2022dda09777d2bf70cdab3d:
> atl1c: fix issue of io access mode for AR8152 v2.1
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
>
> Bruce W Allan (1):
> e1000e: implement 82577/579 MDI setting support
>
> Jesse Brandeburg (5):
> ethtool.h: MDI setting support
> igb: implement 580 MDI setting support
> e1000: configure and read MDI settings
> e1000e: implement MDI/MDI-X control
> igb: update to allow reading/setting MDI state
>
> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 34 +++++++++++++++++++
> drivers/net/ethernet/intel/e1000/e1000_main.c | 4 +++
> drivers/net/ethernet/intel/e1000e/ethtool.c | 39 +++++++++++++++++++---
> 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 | 37 ++++++++++++++++++++
> drivers/net/ethernet/intel/igb/igb_main.c | 4 +++
> include/linux/ethtool.h | 17 ++++++----
> 9 files changed, 184 insertions(+), 16 deletions(-)
>
> --
> 1.7.10.4
>
Since there some changes that are needed in this patch set, I will drop
this series from my tree so that I can continue pushing additional
ixgbe/ixgbevf patches.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread