* [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-03-25 21:58 ` Daniel Walker 0 siblings, 0 replies; 12+ messages in thread From: Daniel Walker @ 2016-03-25 21:58 UTC (permalink / raw) To: intel-wired-lan From: Steve Shih <sshih@cisco.com> This patch fixes the issues for disabling auto-negotiation and forcing speed and duplex settings for the fiber media. For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings call would not fail with -EOPNOTSUPP. e1000_set_spd_dplx should not automatically turn autoneg back on for forced 1000 Mbps full duplex settings. Cc: danielwa at fifo99.com Cc: xe-kernel at external.cisco.com Signed-off-by: Steve Shih <sshih@cisco.com> --- drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 6cab1f3..cd03dcd 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -201,6 +201,9 @@ static int e1000_get_settings(struct net_device *netdev, else ecmd->eth_tp_mdix_ctrl = hw->phy.mdix; + if (hw->phy.media_type != e1000_media_type_copper) + ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID; + return 0; } @@ -236,8 +239,7 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx) mac->forced_speed_duplex = ADVERTISE_100_FULL; break; case SPEED_1000 + DUPLEX_FULL: - mac->autoneg = 1; - adapter->hw.phy.autoneg_advertised = ADVERTISE_1000_FULL; + mac->forced_speed_duplex = ADVERTISE_1000_FULL; break; case SPEED_1000 + DUPLEX_HALF: /* not supported */ default: -- 2.5.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-03-25 21:58 ` Daniel Walker 0 siblings, 0 replies; 12+ messages in thread From: Daniel Walker @ 2016-03-25 21:58 UTC (permalink / raw) To: Jeff Kirsher, Jesse Brandeburg, Shannon Nelson, Carolyn Wyborny, Don Skidmore, Bruce Allan, John Ronciak, Mitch Williams Cc: Steve Shih, danielwa, xe-kernel, intel-wired-lan, netdev, linux-kernel From: Steve Shih <sshih@cisco.com> This patch fixes the issues for disabling auto-negotiation and forcing speed and duplex settings for the fiber media. For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings call would not fail with -EOPNOTSUPP. e1000_set_spd_dplx should not automatically turn autoneg back on for forced 1000 Mbps full duplex settings. Cc: danielwa@fifo99.com Cc: xe-kernel@external.cisco.com Signed-off-by: Steve Shih <sshih@cisco.com> --- drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 6cab1f3..cd03dcd 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -201,6 +201,9 @@ static int e1000_get_settings(struct net_device *netdev, else ecmd->eth_tp_mdix_ctrl = hw->phy.mdix; + if (hw->phy.media_type != e1000_media_type_copper) + ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID; + return 0; } @@ -236,8 +239,7 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx) mac->forced_speed_duplex = ADVERTISE_100_FULL; break; case SPEED_1000 + DUPLEX_FULL: - mac->autoneg = 1; - adapter->hw.phy.autoneg_advertised = ADVERTISE_1000_FULL; + mac->forced_speed_duplex = ADVERTISE_1000_FULL; break; case SPEED_1000 + DUPLEX_HALF: /* not supported */ default: -- 2.5.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber 2016-03-25 21:58 ` Daniel Walker @ 2016-03-30 19:34 ` Daniel Walker -1 siblings, 0 replies; 12+ messages in thread From: Daniel Walker @ 2016-03-30 19:34 UTC (permalink / raw) To: intel-wired-lan So Intel maintainers (Jeff, Jesse, Shannon, Carolyn, Don, Bruce, and John) I'm assuming no comments means this patch is acceptable , and I will resubmit it without the RFC. Is that acceptable ? On 03/25/2016 02:58 PM, Daniel Walker wrote: > From: Steve Shih <sshih@cisco.com> > > This patch fixes the issues for disabling auto-negotiation and forcing > speed and duplex settings for the fiber media. > > For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for > eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings > call would not fail with -EOPNOTSUPP. > > e1000_set_spd_dplx should not automatically turn autoneg back on for forced > 1000 Mbps full duplex settings. > > Cc: danielwa at fifo99.com > Cc: xe-kernel at external.cisco.com > Signed-off-by: Steve Shih <sshih@cisco.com> > --- > drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c > index 6cab1f3..cd03dcd 100644 > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > @@ -201,6 +201,9 @@ static int e1000_get_settings(struct net_device *netdev, > else > ecmd->eth_tp_mdix_ctrl = hw->phy.mdix; > > + if (hw->phy.media_type != e1000_media_type_copper) > + ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID; > + > return 0; > } > > @@ -236,8 +239,7 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx) > mac->forced_speed_duplex = ADVERTISE_100_FULL; > break; > case SPEED_1000 + DUPLEX_FULL: > - mac->autoneg = 1; > - adapter->hw.phy.autoneg_advertised = ADVERTISE_1000_FULL; > + mac->forced_speed_duplex = ADVERTISE_1000_FULL; > break; > case SPEED_1000 + DUPLEX_HALF: /* not supported */ > default: ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-03-30 19:34 ` Daniel Walker 0 siblings, 0 replies; 12+ messages in thread From: Daniel Walker @ 2016-03-30 19:34 UTC (permalink / raw) To: Jeff Kirsher, Jesse Brandeburg, Shannon Nelson, Carolyn Wyborny, Don Skidmore, Bruce Allan, John Ronciak, Mitch Williams Cc: Steve Shih, danielwa, xe-kernel, intel-wired-lan, netdev, linux-kernel So Intel maintainers (Jeff, Jesse, Shannon, Carolyn, Don, Bruce, and John) I'm assuming no comments means this patch is acceptable , and I will resubmit it without the RFC. Is that acceptable ? On 03/25/2016 02:58 PM, Daniel Walker wrote: > From: Steve Shih <sshih@cisco.com> > > This patch fixes the issues for disabling auto-negotiation and forcing > speed and duplex settings for the fiber media. > > For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for > eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings > call would not fail with -EOPNOTSUPP. > > e1000_set_spd_dplx should not automatically turn autoneg back on for forced > 1000 Mbps full duplex settings. > > Cc: danielwa@fifo99.com > Cc: xe-kernel@external.cisco.com > Signed-off-by: Steve Shih <sshih@cisco.com> > --- > drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c > index 6cab1f3..cd03dcd 100644 > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > @@ -201,6 +201,9 @@ static int e1000_get_settings(struct net_device *netdev, > else > ecmd->eth_tp_mdix_ctrl = hw->phy.mdix; > > + if (hw->phy.media_type != e1000_media_type_copper) > + ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID; > + > return 0; > } > > @@ -236,8 +239,7 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx) > mac->forced_speed_duplex = ADVERTISE_100_FULL; > break; > case SPEED_1000 + DUPLEX_FULL: > - mac->autoneg = 1; > - adapter->hw.phy.autoneg_advertised = ADVERTISE_1000_FULL; > + mac->forced_speed_duplex = ADVERTISE_1000_FULL; > break; > case SPEED_1000 + DUPLEX_HALF: /* not supported */ > default: ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber 2016-03-30 19:34 ` Daniel Walker @ 2016-03-30 19:44 ` Jeff Kirsher -1 siblings, 0 replies; 12+ messages in thread From: Jeff Kirsher @ 2016-03-30 19:44 UTC (permalink / raw) To: intel-wired-lan On Wed, 2016-03-30 at 12:34 -0700, Daniel Walker wrote: > > So Intel maintainers (Jeff, Jesse, Shannon, Carolyn, Don, Bruce, and > John) > > I'm assuming no comments means this patch is acceptable , and I will? > resubmit it without the RFC. Is that acceptable ? I personally do not see an issue with the patch, go ahead and submit it as a non-RFC and I will make sure that the e1000e maintainer (Dima Ruinskly) reviews it. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160330/f0335fdc/attachment.asc> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-03-30 19:44 ` Jeff Kirsher 0 siblings, 0 replies; 12+ messages in thread From: Jeff Kirsher @ 2016-03-30 19:44 UTC (permalink / raw) To: Daniel Walker, dima.ruinskiy Cc: Steve Shih, danielwa, xe-kernel, intel-wired-lan, netdev [-- Attachment #1: Type: text/plain, Size: 437 bytes --] On Wed, 2016-03-30 at 12:34 -0700, Daniel Walker wrote: > > So Intel maintainers (Jeff, Jesse, Shannon, Carolyn, Don, Bruce, and > John) > > I'm assuming no comments means this patch is acceptable , and I will > resubmit it without the RFC. Is that acceptable ? I personally do not see an issue with the patch, go ahead and submit it as a non-RFC and I will make sure that the e1000e maintainer (Dima Ruinskly) reviews it. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber 2016-03-30 19:34 ` Daniel Walker @ 2016-04-03 14:23 ` Ruinskiy, Dima -1 siblings, 0 replies; 12+ messages in thread From: Ruinskiy, Dima @ 2016-04-03 14:23 UTC (permalink / raw) To: intel-wired-lan I have a couple of comments (sorry for not getting to it a bit sooner). > For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for > eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings > call would not fail with -EOPNOTSUPP. Should this be specific to fiber? Because the code just checks media != copper. There is at least other media type (e1000_media_type_internal_serdes), which will be affected by this change as well. What is the proper behavior in this case? > e1000_set_spd_dplx should not automatically turn autoneg back on for forced > 1000 Mbps full duplex settings. I seem to remember that this code is there because the copper-based NICs only support 1000Mbps in auto-negotiation mode, not forced (this is according to the IEEE spec, as far as I know). I believe this code is there to ensure that a user can force the speed to 1000 via the API, and still have the link resolve correctly via auto-negotiation. I am concerned that changing this code will break clients that rely on it. Maybe it is best to also limit it to fiber devices only? Thanks, Dima (Intel e1000e maintainer). -----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Daniel Walker Sent: Wednesday, 30 March, 2016 22:35 To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch A Cc: netdev at vger.kernel.org; xe-kernel at external.cisco.com; danielwa at fifo99.com; linux-kernel at vger.kernel.org; Steve Shih; intel-wired-lan at lists.osuosl.org Subject: Re: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber So Intel maintainers (Jeff, Jesse, Shannon, Carolyn, Don, Bruce, and John) I'm assuming no comments means this patch is acceptable , and I will resubmit it without the RFC. Is that acceptable ? On 03/25/2016 02:58 PM, Daniel Walker wrote: > From: Steve Shih <sshih@cisco.com> > > This patch fixes the issues for disabling auto-negotiation and forcing > speed and duplex settings for the fiber media. > > For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for > eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings > call would not fail with -EOPNOTSUPP. > > e1000_set_spd_dplx should not automatically turn autoneg back on for forced > 1000 Mbps full duplex settings. > > Cc: danielwa at fifo99.com > Cc: xe-kernel at external.cisco.com > Signed-off-by: Steve Shih <sshih@cisco.com> > --- > drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c > index 6cab1f3..cd03dcd 100644 > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > @@ -201,6 +201,9 @@ static int e1000_get_settings(struct net_device *netdev, > else > ecmd->eth_tp_mdix_ctrl = hw->phy.mdix; > > + if (hw->phy.media_type != e1000_media_type_copper) > + ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID; > + > return 0; > } > > @@ -236,8 +239,7 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx) > mac->forced_speed_duplex = ADVERTISE_100_FULL; > break; > case SPEED_1000 + DUPLEX_FULL: > - mac->autoneg = 1; > - adapter->hw.phy.autoneg_advertised = ADVERTISE_1000_FULL; > + mac->forced_speed_duplex = ADVERTISE_1000_FULL; > break; > case SPEED_1000 + DUPLEX_HALF: /* not supported */ > default: _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan at lists.osuosl.org http://lists.osuosl.org/mailman/listinfo/intel-wired-lan --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-04-03 14:23 ` Ruinskiy, Dima 0 siblings, 0 replies; 12+ messages in thread From: Ruinskiy, Dima @ 2016-04-03 14:23 UTC (permalink / raw) To: Daniel Walker, Kirsher, Jeffrey T, Brandeburg, Jesse, Nelson, Shannon, Wyborny, Carolyn, Skidmore, Donald C, Allan, Bruce W, Ronciak, John, Williams, Mitch A Cc: netdev@vger.kernel.org, xe-kernel@external.cisco.com, danielwa@fifo99.com, linux-kernel@vger.kernel.org, Steve Shih, intel-wired-lan@lists.osuosl.org I have a couple of comments (sorry for not getting to it a bit sooner). > For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for > eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings > call would not fail with -EOPNOTSUPP. Should this be specific to fiber? Because the code just checks media != copper. There is at least other media type (e1000_media_type_internal_serdes), which will be affected by this change as well. What is the proper behavior in this case? > e1000_set_spd_dplx should not automatically turn autoneg back on for forced > 1000 Mbps full duplex settings. I seem to remember that this code is there because the copper-based NICs only support 1000Mbps in auto-negotiation mode, not forced (this is according to the IEEE spec, as far as I know). I believe this code is there to ensure that a user can force the speed to 1000 via the API, and still have the link resolve correctly via auto-negotiation. I am concerned that changing this code will break clients that rely on it. Maybe it is best to also limit it to fiber devices only? Thanks, Dima (Intel e1000e maintainer). -----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Daniel Walker Sent: Wednesday, 30 March, 2016 22:35 To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch A Cc: netdev@vger.kernel.org; xe-kernel@external.cisco.com; danielwa@fifo99.com; linux-kernel@vger.kernel.org; Steve Shih; intel-wired-lan@lists.osuosl.org Subject: Re: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber So Intel maintainers (Jeff, Jesse, Shannon, Carolyn, Don, Bruce, and John) I'm assuming no comments means this patch is acceptable , and I will resubmit it without the RFC. Is that acceptable ? On 03/25/2016 02:58 PM, Daniel Walker wrote: > From: Steve Shih <sshih@cisco.com> > > This patch fixes the issues for disabling auto-negotiation and forcing > speed and duplex settings for the fiber media. > > For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for > eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings > call would not fail with -EOPNOTSUPP. > > e1000_set_spd_dplx should not automatically turn autoneg back on for forced > 1000 Mbps full duplex settings. > > Cc: danielwa@fifo99.com > Cc: xe-kernel@external.cisco.com > Signed-off-by: Steve Shih <sshih@cisco.com> > --- > drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c > index 6cab1f3..cd03dcd 100644 > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > @@ -201,6 +201,9 @@ static int e1000_get_settings(struct net_device *netdev, > else > ecmd->eth_tp_mdix_ctrl = hw->phy.mdix; > > + if (hw->phy.media_type != e1000_media_type_copper) > + ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID; > + > return 0; > } > > @@ -236,8 +239,7 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx) > mac->forced_speed_duplex = ADVERTISE_100_FULL; > break; > case SPEED_1000 + DUPLEX_FULL: > - mac->autoneg = 1; > - adapter->hw.phy.autoneg_advertised = ADVERTISE_1000_FULL; > + mac->forced_speed_duplex = ADVERTISE_1000_FULL; > break; > case SPEED_1000 + DUPLEX_HALF: /* not supported */ > default: _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@lists.osuosl.org http://lists.osuosl.org/mailman/listinfo/intel-wired-lan --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber 2016-04-03 14:23 ` Ruinskiy, Dima @ 2016-04-04 23:03 ` Daniel Walker -1 siblings, 0 replies; 12+ messages in thread From: Daniel Walker @ 2016-04-04 23:03 UTC (permalink / raw) To: intel-wired-lan On 04/03/2016 07:23 AM, Ruinskiy, Dima wrote: > I have a couple of comments (sorry for not getting to it a bit sooner). > >> For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for >> eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings >> call would not fail with -EOPNOTSUPP. > Should this be specific to fiber? Because the code just checks media != copper. There is at least other media type (e1000_media_type_internal_serdes), which will be affected by this change as well. What is the proper behavior in this case? Steve is shy, so here are his comments, <Steve> Regarding the 1st comment. The issue affects all non-copper media. In fact, we are using e1000_media_type_internal_serdes and are affected due to the following check in e1000_set_settings: /* 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; } } </Steve> >> e1000_set_spd_dplx should not automatically turn autoneg back on for forced >> 1000 Mbps full duplex settings. > I seem to remember that this code is there because the copper-based NICs only support 1000Mbps in auto-negotiation mode, not forced (this is according to the IEEE spec, as far as I know). I believe this code is there to ensure that a user can force the speed to 1000 via the API, and still have the link resolve correctly via auto-negotiation. I am concerned that changing this code will break clients that rely on it. Maybe it is best to also limit it to fiber devices only? > <Steve> Regarding the 2nd comment. Yes, will limit to non-copper media. </Steve> He supplied me with a second patch which I can send.. Can we remove the RFC this time ? Daniel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-04-04 23:03 ` Daniel Walker 0 siblings, 0 replies; 12+ messages in thread From: Daniel Walker @ 2016-04-04 23:03 UTC (permalink / raw) To: Ruinskiy, Dima, Kirsher, Jeffrey T, Brandeburg, Jesse, Nelson, Shannon, Wyborny, Carolyn, Skidmore, Donald C, Allan, Bruce W, Ronciak, John, Williams, Mitch A Cc: netdev@vger.kernel.org, xe-kernel@external.cisco.com, danielwa@fifo99.com, linux-kernel@vger.kernel.org, Steve Shih, intel-wired-lan@lists.osuosl.org On 04/03/2016 07:23 AM, Ruinskiy, Dima wrote: > I have a couple of comments (sorry for not getting to it a bit sooner). > >> For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for >> eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings >> call would not fail with -EOPNOTSUPP. > Should this be specific to fiber? Because the code just checks media != copper. There is at least other media type (e1000_media_type_internal_serdes), which will be affected by this change as well. What is the proper behavior in this case? Steve is shy, so here are his comments, <Steve> Regarding the 1st comment. The issue affects all non-copper media. In fact, we are using e1000_media_type_internal_serdes and are affected due to the following check in e1000_set_settings: /* 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; } } </Steve> >> e1000_set_spd_dplx should not automatically turn autoneg back on for forced >> 1000 Mbps full duplex settings. > I seem to remember that this code is there because the copper-based NICs only support 1000Mbps in auto-negotiation mode, not forced (this is according to the IEEE spec, as far as I know). I believe this code is there to ensure that a user can force the speed to 1000 via the API, and still have the link resolve correctly via auto-negotiation. I am concerned that changing this code will break clients that rely on it. Maybe it is best to also limit it to fiber devices only? > <Steve> Regarding the 2nd comment. Yes, will limit to non-copper media. </Steve> He supplied me with a second patch which I can send.. Can we remove the RFC this time ? Daniel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber 2016-04-04 23:03 ` Daniel Walker @ 2016-04-05 13:41 ` Ruinskiy, Dima -1 siblings, 0 replies; 12+ messages in thread From: Ruinskiy, Dima @ 2016-04-05 13:41 UTC (permalink / raw) To: intel-wired-lan Sure, please send. It will be reviewed like any other patch. :) -----Original Message----- From: Daniel Walker [mailto:danielwa at cisco.com] Sent: Tuesday, April 05, 2016 02:04 To: Ruinskiy, Dima; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch A Cc: netdev at vger.kernel.org; xe-kernel at external.cisco.com; danielwa at fifo99.com; linux-kernel at vger.kernel.org; Steve Shih; intel-wired-lan at lists.osuosl.org Subject: Re: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber On 04/03/2016 07:23 AM, Ruinskiy, Dima wrote: > I have a couple of comments (sorry for not getting to it a bit sooner). > >> For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for >> eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings >> call would not fail with -EOPNOTSUPP. > Should this be specific to fiber? Because the code just checks media != copper. There is at least other media type (e1000_media_type_internal_serdes), which will be affected by this change as well. What is the proper behavior in this case? Steve is shy, so here are his comments, <Steve> Regarding the 1st comment. The issue affects all non-copper media. In fact, we are using e1000_media_type_internal_serdes and are affected due to the following check in e1000_set_settings: /* 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; } } </Steve> >> e1000_set_spd_dplx should not automatically turn autoneg back on for forced >> 1000 Mbps full duplex settings. > I seem to remember that this code is there because the copper-based NICs only support 1000Mbps in auto-negotiation mode, not forced (this is according to the IEEE spec, as far as I know). I believe this code is there to ensure that a user can force the speed to 1000 via the API, and still have the link resolve correctly via auto-negotiation. I am concerned that changing this code will break clients that rely on it. Maybe it is best to also limit it to fiber devices only? > <Steve> Regarding the 2nd comment. Yes, will limit to non-copper media. </Steve> He supplied me with a second patch which I can send.. Can we remove the RFC this time ? Daniel --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber @ 2016-04-05 13:41 ` Ruinskiy, Dima 0 siblings, 0 replies; 12+ messages in thread From: Ruinskiy, Dima @ 2016-04-05 13:41 UTC (permalink / raw) To: Daniel Walker, Kirsher, Jeffrey T, Brandeburg, Jesse, Nelson, Shannon, Wyborny, Carolyn, Skidmore, Donald C, Allan, Bruce W, Ronciak, John, Williams, Mitch A Cc: netdev@vger.kernel.org, xe-kernel@external.cisco.com, danielwa@fifo99.com, linux-kernel@vger.kernel.org, Steve Shih, intel-wired-lan@lists.osuosl.org Sure, please send. It will be reviewed like any other patch. :) -----Original Message----- From: Daniel Walker [mailto:danielwa@cisco.com] Sent: Tuesday, April 05, 2016 02:04 To: Ruinskiy, Dima; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch A Cc: netdev@vger.kernel.org; xe-kernel@external.cisco.com; danielwa@fifo99.com; linux-kernel@vger.kernel.org; Steve Shih; intel-wired-lan@lists.osuosl.org Subject: Re: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber On 04/03/2016 07:23 AM, Ruinskiy, Dima wrote: > I have a couple of comments (sorry for not getting to it a bit sooner). > >> For fiber media, e1000_get_settings should return ETH_TP_MDI_INVALID for >> eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent e1000_set_settings >> call would not fail with -EOPNOTSUPP. > Should this be specific to fiber? Because the code just checks media != copper. There is at least other media type (e1000_media_type_internal_serdes), which will be affected by this change as well. What is the proper behavior in this case? Steve is shy, so here are his comments, <Steve> Regarding the 1st comment. The issue affects all non-copper media. In fact, we are using e1000_media_type_internal_serdes and are affected due to the following check in e1000_set_settings: /* 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; } } </Steve> >> e1000_set_spd_dplx should not automatically turn autoneg back on for forced >> 1000 Mbps full duplex settings. > I seem to remember that this code is there because the copper-based NICs only support 1000Mbps in auto-negotiation mode, not forced (this is according to the IEEE spec, as far as I know). I believe this code is there to ensure that a user can force the speed to 1000 via the API, and still have the link resolve correctly via auto-negotiation. I am concerned that changing this code will break clients that rely on it. Maybe it is best to also limit it to fiber devices only? > <Steve> Regarding the 2nd comment. Yes, will limit to non-copper media. </Steve> He supplied me with a second patch which I can send.. Can we remove the RFC this time ? Daniel --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-04-05 13:41 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-25 21:58 [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber Daniel Walker 2016-03-25 21:58 ` Daniel Walker 2016-03-30 19:34 ` [Intel-wired-lan] " Daniel Walker 2016-03-30 19:34 ` Daniel Walker 2016-03-30 19:44 ` [Intel-wired-lan] " Jeff Kirsher 2016-03-30 19:44 ` Jeff Kirsher 2016-04-03 14:23 ` [Intel-wired-lan] " Ruinskiy, Dima 2016-04-03 14:23 ` Ruinskiy, Dima 2016-04-04 23:03 ` Daniel Walker 2016-04-04 23:03 ` Daniel Walker 2016-04-05 13:41 ` Ruinskiy, Dima 2016-04-05 13:41 ` Ruinskiy, Dima
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.