From: Wang, Haiyue [mailto:haiyue.wang@intel.com]
Sent: Tuesday, 21 December 2021 02.15

quoted

-----Original Message-----
From: Stephen Douthit <redacted>
Sent: Tuesday, December 21, 2021 05:33

On 12/20/21 02:53, Wang, Haiyue wrote:

quoted

quoted

-----Original Message-----
From: Stephen Douthit <redacted>
Sent: Tuesday, December 7, 2021 06:19

Make sure an SFP is really a SFF-8472 device that supports the

optional

quoted

quoted

quoted

soft rate select feature before just blindly poking those I2C

registers.

quoted

quoted

quoted

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 <redacted>
---

quoted

       /* Set RS0 */
       status = hw->phy.ops.read_i2c_byte(hw,

IXGBE_SFF_SFF_8472_OSCB,

quoted

quoted

quoted

IXGBE_I2C_EEPROM_DEV_ADDR2,

quoted

quoted

quoted

diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h

b/drivers/net/ixgbe/base/ixgbe_phy.h

quoted

quoted

quoted

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

quoted

quoted

merged. : - (

This isn't anything unique to our hardware, these values are coming

from

quoted

the SFF-8472 SFP+ I2C specification.

The ability to do a soft rate select via I2C is an optional feature,

and

quoted

modules that support it are supposed to set bit 3 in byte 93 (0x5d),

the

quoted

"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

quoted

is supposed to ignore writes to rate select control bits if the

feature

quoted

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

quoted

already happens in the driver elsewhere[1].  I'd argue that testing

that

quoted

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

quoted

our hardware, I'm not sure why.

That hotplug issue isn't seen on the same hardware when using the

Linux

quoted

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.

Working with the Linux kernel mainline drivers is good advice.

The official Intel Linux drivers seem to be ages behind the Kernel mainline, and they don't fully support the C3000 NICs, so don’t waste any time there! We recently tried using the official Intel Linux drivers for a C3338 based project (using Kernel 3.19 in 32 bit mode with x2APIC disabled), and they didn't work at all. We ended up backporting the necessary changes from the kernel mainline instead.

quoted

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

quoted

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