From: Daniel Walker <danielwa@cisco.com>
To: "Ruinskiy, Dima" <dima.ruinskiy@intel.com>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel.com>,
"Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
"Nelson, Shannon" <shannon.nelson@intel.com>,
"Wyborny, Carolyn" <carolyn.wyborny@intel.com>,
"Skidmore, Donald C" <donald.c.skidmore@intel.com>,
"Allan, Bruce W" <bruce.w.allan@intel.com>,
"Ronciak, John" <john.ronciak@intel.com>,
"Williams, Mitch A" <mitch.a.williams@intel.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"xe-kernel@external.cisco.com" <xe-kernel@external.cisco.com>,
"danielwa@fifo99.com" <danielwa@fifo99.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Steve Shih <sshih@cisco.com>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber
Date: Mon, 4 Apr 2016 16:03:50 -0700 [thread overview]
Message-ID: <5702F2D6.5010105@cisco.com> (raw)
In-Reply-To: <36CDDD56DDB4D44E911123902EFC26B05B440F17@HASMSX110.ger.corp.intel.com>
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
next prev parent reply other threads:[~2016-04-04 23:03 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-25 21:58 [PATCH-RFC] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for fiber Daniel Walker
2016-03-30 19:34 ` Daniel Walker
2016-03-30 19:44 ` Jeff Kirsher
2016-04-03 14:23 ` [Intel-wired-lan] " Ruinskiy, Dima
2016-04-04 23:03 ` Daniel Walker [this message]
2016-04-05 13:41 ` Ruinskiy, Dima
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5702F2D6.5010105@cisco.com \
--to=danielwa@cisco.com \
--cc=bruce.w.allan@intel.com \
--cc=carolyn.wyborny@intel.com \
--cc=danielwa@fifo99.com \
--cc=dima.ruinskiy@intel.com \
--cc=donald.c.skidmore@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=john.ronciak@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mitch.a.williams@intel.com \
--cc=netdev@vger.kernel.org \
--cc=shannon.nelson@intel.com \
--cc=sshih@cisco.com \
--cc=xe-kernel@external.cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).