DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: "stephend@silicom-usa.com" <stephend@silicom-usa.com>,
	"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	Changchun Ouyang <changchun.ouyang@intel.com>,
	"Zhang, Helin" <helin.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Wang, Wen" <wenw@silicom-usa.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: RE: [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before write
Date: Tue, 21 Dec 2021 01:15:14 +0000	[thread overview]
Message-ID: <BYAPR11MB34957184FD7BED1AE8BD286FF77C9@BYAPR11MB3495.namprd11.prod.outlook.com> (raw)
In-Reply-To: <d14039a7-88c2-083a-e484-be29eeeb6dd1@silicom-usa.com>

> -----Original Message-----
> From: Stephen Douthit <stephend@silicom-usa.com>
> Sent: Tuesday, December 21, 2021 05:33
> To: Wang, Haiyue <haiyue.wang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Changchun Ouyang
> <changchun.ouyang@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Wen <wenw@silicom-usa.com>; stable@dpdk.org
> Subject: Re: [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before write
> 
> On 12/20/21 02:53, Wang, Haiyue wrote:
> >> -----Original Message-----
> >> From: Stephen Douthit <stephend@silicom-usa.com>
> >> Sent: Tuesday, December 7, 2021 06:19
> >> To: Wang, Haiyue <haiyue.wang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Changchun Ouyang
> >> <changchun.ouyang@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> >> Cc: dev@dpdk.org; Wen Wang <wenw@silicom-usa.com>; Stephen Douthit <stephend@silicom-usa.com>;
> >> stable@dpdk.org
> >> Subject: [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before write
> >>
> >> Make sure an SFP is really a SFF-8472 device that supports the optional
> >> soft rate select feature before just blindly poking those I2C registers.
> >>
> >> Skip all I2C traffic if we know there's no SFP.
> >>
> >> Fixes: f3430431aba ("ixgbe/base: add SFP+ dual-speed support")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
> >> ---
> >
> >
> >>        /* Set RS0 */
> >>        status = hw->phy.ops.read_i2c_byte(hw, IXGBE_SFF_SFF_8472_OSCB,
> >>                                           IXGBE_I2C_EEPROM_DEV_ADDR2,
> >> diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h b/drivers/net/ixgbe/base/ixgbe_phy.h
> >> index ceefbb3e68..cd57ce040f 100644
> >> --- a/drivers/net/ixgbe/base/ixgbe_phy.h
> >> +++ b/drivers/net/ixgbe/base/ixgbe_phy.h
> >> @@ -21,6 +21,7 @@
> >>   #define IXGBE_SFF_CABLE_TECHNOLOGY   0x8
> >>   #define IXGBE_SFF_CABLE_SPEC_COMP    0x3C
> >>   #define IXGBE_SFF_SFF_8472_SWAP              0x5C
> >> +#define IXGBE_SFF_SFF_8472_EOPT              0x5D
> >
> > Looks like this is YOUR platform specific, then this patchset can't be
> > merged. : - (
> 
> This isn't anything unique to our hardware, these values are coming from
> the SFF-8472 SFP+ I2C specification.
> 
> The ability to do a soft rate select via I2C is an optional feature, and
> modules that support it are supposed to set bit 3 in byte 93 (0x5d), the
> "Enhanced Options" register, to advertise the functionality.
> 
> Please see section 8.10 and Table 8-6 in the SFF-8472 spec.
> 
> Checking the RATE_SELECT bit flag may be overkill since the transceiver
> is supposed to ignore writes to rate select control bits if the feature
> isn't implemented.  I can drop that check if you like, but the other
> checks for a 8472 device (vs 8079) aren't anything different than what
> already happens in the driver elsewhere[1].  I'd argue that testing that
> a feature is supported in hardware before trying to use it is normal
> driver behavior.
> 
> If instead you mean that the entire series is somehow applicable only to
> our hardware, I'm not sure why.
> 
> That hotplug issue isn't seen on the same hardware when using the Linux
> driver; so it's a dpdk problem (at least on C3000 ixgbe devs), and not a

I can't find your related fix in two official Linux drivers:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixgbe
https://www.intel.com/content/www/us/en/download/14302/14687/intel-network-adapter-driver-for-pcie-intel-10-gigabit-ethernet-network-connections-under-linux.html?

Normally, DPDK keeps sync with this kind of release.

> hardware problem.  Fixing the hotplug/rateswap issue was my primary
> goal, the other patches fix problems I found along the way while
> debugging.
> 
> I can also reproduce the hotplug/rateswap issue on the PLCC-B, an Intel
> reference design for the C3000 family, so again, not unique to this
> platform.

I guess this is just in C3000 reference board SDK ?

I recommend you submit the fix to kernel firstly, you will get more
experts' reviews and fully test:

https://patchwork.ozlabs.org/project/intel-wired-lan/list/
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

> 
> Please let me know if that addresses your concerns, or if I've missed
> your point.
> 





> Thanks,
> Steve
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixg
> be/ixgbe_ethtool.c?h=v5.16-rc6
> 
> >>   #define IXGBE_SFF_SFF_8472_COMP              0x5E
> >>   #define IXGBE_SFF_SFF_8472_OSCB              0x6E
> >>   #define IXGBE_SFF_SFF_8472_ESCB              0x76
> >> @@ -48,6 +49,8 @@
> >>   #define IXGBE_SFF_SOFT_RS_SELECT_10G 0x8
> >> --
> >> 2.31.1
> >


  reply	other threads:[~2021-12-21  1:15 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-06 22:19 [PATCH v2 0/7] ixgbe SFP handling fixes Stephen Douthit
2021-12-06 22:19 ` [PATCH v2 1/7] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs Stephen Douthit
2021-12-20  7:45   ` Wang, Haiyue
2021-12-20 21:32     ` Stephen Douthit
2021-12-06 22:19 ` [PATCH v2 2/7] net/ixgbe: Add ixgbe_check_sfp_cage() for testing state of PRSNT# signal Stephen Douthit
2021-12-06 22:19 ` [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before write Stephen Douthit
2021-12-20  7:53   ` Wang, Haiyue
2021-12-20 21:32     ` Stephen Douthit
2021-12-21  1:15       ` Wang, Haiyue [this message]
2021-12-21  8:57         ` Morten Brørup
2021-12-22  1:24           ` Wang, Haiyue
2021-12-22 10:43             ` Morten Brørup
2021-12-22 16:03               ` Wang, Haiyue
2021-12-22 19:13                 ` Morten Brørup
2021-12-22 21:44                 ` Stephen Douthit
2021-12-23  0:55                   ` Wang, Haiyue
2022-01-18 21:06                     ` Stephen Douthit
2022-01-19  0:31                       ` Wang, Haiyue
2022-02-07 16:04                         ` Ferruh Yigit
2022-02-08 13:50                           ` Jeff Daly
2022-02-08 14:52                             ` Ferruh Yigit
2022-02-09  4:00                               ` Wang, Haiyue
2022-02-09 13:33                                 ` Ferruh Yigit
2022-02-09 13:43                                   ` Wang, Haiyue
2021-12-21 14:05         ` Stephen Douthit
2021-12-06 22:19 ` [PATCH v2 4/7] net/ixgbe: Run 82599 link status workaround only on affected devices Stephen Douthit
2021-12-06 22:19 ` [PATCH v2 5/7] net/ixgbe: Fix SFP detection and linking on hotplug Stephen Douthit
2022-02-07 16:07   ` Ferruh Yigit
2021-12-06 22:19 ` [PATCH v2 6/7] net/ixgbe: Retry SFP ID read field to handle misbehaving SFPs Stephen Douthit
2021-12-06 22:19 ` [PATCH v2 7/7] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices Stephen Douthit
2021-12-17  9:29 ` [PATCH v2 0/7] ixgbe SFP handling fixes Thomas Monjalon
2022-02-24 15:23 ` [PATCH v3 0/3] " Jeff Daly
2022-02-24 15:23   ` [PATCH v3 1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs Jeff Daly
2022-02-24 15:23   ` [PATCH v3 2/3] net/ixgbe: Limit SDP3 check of TX_DISABLE to appropriate devices Jeff Daly
2022-02-24 15:23   ` [PATCH v3 3/3] net/ixgbe: Fix SFP detection and linking on hotplug Jeff Daly
2022-02-25  1:56     ` Wang, Haiyue
2022-02-25 20:50 ` [PATCH v4 " Jeff Daly
2022-02-26 15:57   ` Ferruh Yigit
2022-02-28 15:29 ` [PATCH v4 0/3] ixgbe SFP handling fixes Jeff Daly
2022-02-28 15:29   ` [PATCH v4 1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs Jeff Daly
2022-03-01  5:56     ` Wang, Haiyue
2022-03-01 11:18       ` Zhang, Qi Z
2022-03-06 17:56         ` Thomas Monjalon
2022-03-08 15:01           ` Jeff Daly
2022-02-28 15:29   ` [PATCH v4 2/3] net/ixgbe: Limit SDP3 check of TX_DISABLE to appropriate devices Jeff Daly
2022-02-28 15:29   ` [PATCH v4 3/3] net/ixgbe: Fix SFP detection and linking on hotplug Jeff Daly
2022-03-12 13:03     ` Jeff Daly
2022-03-10 12:35   ` [PATCH v4 0/3] ixgbe SFP handling fixes Zhang, Qi Z
2022-04-12 17:34   ` [PATCH v5 0/2] " Jeff Daly
2022-04-12 17:34     ` [PATCH v5 1/2] net/ixgbe: Limit SDP3 check of TX_DISABLE to appropriate devices Jeff Daly
2022-04-12 17:34     ` [PATCH v5 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Jeff Daly
2022-04-12 17:42   ` [PATCH v6 0/2] ixgbe SFP handling fixes Jeff Daly
2022-04-12 17:42     ` [PATCH v6 1/2] net/ixgbe: Limit SDP3 check of TX_DISABLE to appropriate devices Jeff Daly
2022-04-13  1:21       ` Wang, Haiyue
2022-04-13 15:32         ` Jeff Daly
2022-04-14  1:56           ` Wang, Haiyue
2022-04-12 17:42     ` [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Jeff Daly
2022-04-13  2:46       ` Wang, Haiyue
2022-04-13  6:57         ` Morten Brørup
2022-04-13  7:01           ` Wang, Haiyue
2022-04-13  7:19             ` Morten Brørup
2022-04-13 11:49               ` Wang, Haiyue
2022-04-13 12:54                 ` Morten Brørup
2022-04-13 15:23               ` Jeff Daly
2022-04-14 10:49         ` Jeff Daly
2022-04-14 11:08           ` Jeff Daly
2022-04-14  2:49       ` Wang, Haiyue
2022-04-14  2:59         ` Wang, Haiyue
2022-04-14 10:40           ` Jeff Daly
2022-04-14 12:11             ` Wang, Haiyue
2022-04-18 21:54               ` Jeff Daly
2022-04-19  2:05                 ` Wang, Haiyue
2022-04-19 17:33                   ` Jeff Daly
2022-04-20  1:09                     ` Wang, Haiyue
2022-04-21 17:31                       ` Jeff Daly
2022-04-22  2:11                         ` Wang, Haiyue
2022-05-12  1:26       ` Zhang, Qi Z
2022-05-25 16:55         ` Jeff Daly

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=BYAPR11MB34957184FD7BED1AE8BD286FF77C9@BYAPR11MB3495.namprd11.prod.outlook.com \
    --to=haiyue.wang@intel.com \
    --cc=changchun.ouyang@intel.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    --cc=stable@dpdk.org \
    --cc=stephend@silicom-usa.com \
    --cc=wenw@silicom-usa.com \
    --cc=wenzhuo.lu@intel.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).