On 12/20/21 02:53, Wang, Haiyue wrote:
-----Original Message----- From: Stephen Douthit <[email protected]> Sent: Tuesday, December 7, 2021 06:19 To: Wang, Haiyue <[email protected]>; Lu, Wenzhuo <[email protected]>; Changchun Ouyang <[email protected]>; Zhang, Helin <[email protected]> Cc: [email protected]; Wen Wang <[email protected]>; Stephen Douthit <[email protected]>; [email protected] Subject: [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before writeMake 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: [email protected] Signed-off-by: Stephen Douthit <[email protected]> ---/* 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 0x5DLooks 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 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. 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/ixgbe/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
