Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753913AbcDCOXU (ORCPT ); Sun, 3 Apr 2016 10:23:20 -0400 Received: from mga04.intel.com ([192.55.52.120]:23608 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751280AbcDCOXS convert rfc822-to-8bit (ORCPT ); Sun, 3 Apr 2016 10:23:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,437,1455004800"; d="scan'208";a="937335809" From: "Ruinskiy, Dima" 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" Subject: RE: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber Thread-Topic: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber Thread-Index: AQHRirtJhEkGezfW7Uek4S4ZWUj/RZ9zQIrA Date: Sun, 3 Apr 2016 14:23:13 +0000 Message-ID: <36CDDD56DDB4D44E911123902EFC26B05B440F17@HASMSX110.ger.corp.intel.com> References: <1458943126-32258-1-git-send-email-danielwa@cisco.com> <56FC2A62.6040206@cisco.com> In-Reply-To: <56FC2A62.6040206@cisco.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.184.70.11] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4131 Lines: 85 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 > > 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 > --- > 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.