> -----Original Message-----
> From: Stephen Douthit <steph...@silicom-usa.com>
> Sent: Tuesday, December 21, 2021 05:33
> To: Wang, Haiyue <haiyue.w...@intel.com>; Lu, Wenzhuo <wenzhuo...@intel.com>;
> Changchun Ouyang
> <changchun.ouy...@intel.com>; Zhang, Helin <helin.zh...@intel.com>
> Cc: dev@dpdk.org; Wang, Wen <w...@silicom-usa.com>; sta...@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 <steph...@silicom-usa.com>
> >> Sent: Tuesday, December 7, 2021 06:19
> >> To: Wang, Haiyue <haiyue.w...@intel.com>; Lu, Wenzhuo
> >> <wenzhuo...@intel.com>; Changchun Ouyang
> >> <changchun.ouy...@intel.com>; Zhang, Helin <helin.zh...@intel.com>
> >> Cc: dev@dpdk.org; Wen Wang <w...@silicom-usa.com>; Stephen Douthit
> >> <steph...@silicom-usa.com>;
> >> sta...@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: sta...@dpdk.org
> >>
> >> Signed-off-by: Stephen Douthit <steph...@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
> >
