[PATCH 0/7] ixgbe SFP handling fixes

[PATCH 0/7] ixgbe SFP handling fixes

[Stephen Douthit]

Hello all,

We have several platforms based on Intel's C3000 series of SoCs that
have integrated ixgbe devices (X550EM) operating in the "Native SFI"
mode (the 0x15c4 device ID).

The first five patches in the series all fix issues relating to the ID
and setup of SFPs.

Patch 6 allows slow to boot SFPs (like some XGS-PON modules) to work.

Patch 7 enables 1G Cu to run with a warning, similar to other
unofficially supported modules covered by the allow_unsupported_sfp
flag.  Currently we use this for g.Fast modules, but other modules that
enumerate as 1G Cu may also benefit.

Since all of my testing was done on a C3000 platform, and the ixgbe
driver now covers a large number of devices, any regression testing that
can be done on other ixgbe devices would be greatly appreciated.

Thanks,
Steve

Stephen Douthit (7):
  net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs
  net/ixgbe: Add ixgbe_check_sfp_cage() for testing state of PRSNT#
    signal
  net/ixgbe: Check that SFF-8472 soft rate select is supported before
    write
  net/ixgbe: Run 82599 link status workaround only on affected devices
  net/ixgbe: Fix SFP detection and linking on hotplug
  net/ixgbe: Retry SFP ID read field to handle misbehaving SFPs
  net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices

 drivers/net/ixgbe/base/ixgbe_82599.c  |  41 +++
 drivers/net/ixgbe/base/ixgbe_common.c | 106 +++++--
 drivers/net/ixgbe/base/ixgbe_common.h |   8 +
 drivers/net/ixgbe/base/ixgbe_phy.c    |  39 ++-
 drivers/net/ixgbe/base/ixgbe_phy.h    |   3 +
 drivers/net/ixgbe/base/ixgbe_type.h   |   2 +
 drivers/net/ixgbe/base/ixgbe_x550.c   |  14 +-
 drivers/net/ixgbe/ixgbe_ethdev.c      | 412 +++++++++++++++++---------
 drivers/net/ixgbe/ixgbe_ethdev.h      |  18 +-
 9 files changed, 468 insertions(+), 175 deletions(-)

-- 
2.31.1

[PATCH 1/7] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs

[Stephen Douthit]

Currently all X500EM* MAC types fallthrough to the default case and get
reported as non-SFP regardless of media type, which isn't correct.

Fixes: 0790adeb567 ("ixgbe/base: support X550em_a device")
Cc: stable@dpdk.org

Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index fe61dba81d..66f7af95de 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -781,6 +781,20 @@ ixgbe_is_sfp(struct ixgbe_hw *hw)
 	case ixgbe_phy_sfp_passive_unknown:
 		return 1;
 	default:
+		/* x550em devices may be SFP, check media type */
+		switch (hw->mac.type) {
+		case ixgbe_mac_X550EM_x:
+		case ixgbe_mac_X550EM_a:
+			switch (hw->mac.ops.get_media_type(hw)) {
+			case ixgbe_media_type_fiber:
+			case ixgbe_media_type_fiber_qsfp:
+				return 1;
+			default:
+				return 0;
+			}
+		default:
+			return 0;
+		}
 		return 0;
 	}
 }
-- 
2.31.1

[PATCH 2/7] net/ixgbe: Add ixgbe_check_sfp_cage() for testing state of PRSNT# signal

[Stephen Douthit]

Refactor the SFP check code from ixgbe_check_mac_link_generic into its own
function.

Note that the SFP present status was inverted for the X550EM family of
devices, where SDP0 represents the active low PRSNT# signal from the cage.

Call the new function in ixgbe_identify_module_generic() to short circuit
the I2C polling and greatly speed things up for devices we know are
absent.

Fixes: dd3a93cf5a2 ("net/ixgbe/base: bypass checking link for crosstalk")
Cc: stable@dpdk.org

Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/base/ixgbe_common.c | 60 +++++++++++++++++++--------
 drivers/net/ixgbe/base/ixgbe_common.h |  8 ++++
 drivers/net/ixgbe/base/ixgbe_phy.c    |  8 ++++
 3 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index aa843bd5c4..2764cf7cf1 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -4124,6 +4124,45 @@ static bool ixgbe_need_crosstalk_fix(struct ixgbe_hw *hw)
 	return true;
 }
 
+/**
+ * ixgbe_check_sfp_cage - Find present status of SFP module
+ * @hw: pointer to hardware structure
+ *
+ * Find if a SFP module is present and if this device supports SFPs
+ **/
+enum ixgbe_sfp_cage_status ixgbe_check_sfp_cage(struct ixgbe_hw *hw)
+{
+	enum ixgbe_sfp_cage_status status;
+
+	/* If we're not a fiber/fiber_qsfp, no cage to check */
+	switch (hw->mac.ops.get_media_type(hw)) {
+	case ixgbe_media_type_fiber:
+	case ixgbe_media_type_fiber_qsfp:
+		break;
+	default:
+		return IXGBE_SFP_CAGE_NOCAGE;
+	}
+
+	switch (hw->mac.type) {
+	case ixgbe_mac_82599EB:
+		status = !!(IXGBE_READ_REG(hw, IXGBE_ESDP) &
+			     IXGBE_ESDP_SDP2);
+		break;
+	case ixgbe_mac_X550EM_x:
+	case ixgbe_mac_X550EM_a:
+		/* SDP0 is the active low signal PRSNT#, so invert this */
+		status = !(IXGBE_READ_REG(hw, IXGBE_ESDP) &
+				  IXGBE_ESDP_SDP0);
+		break;
+	default:
+		/* Don't know how to check this device type yet */
+		status = IXGBE_SFP_CAGE_UNKNOWN;
+		break;
+	}
+
+	return status;
+}
+
 /**
  * ixgbe_check_mac_link_generic - Determine link and speed status
  * @hw: pointer to hardware structure
@@ -4145,25 +4184,10 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 	 * the SFP+ cage is full.
 	 */
 	if (ixgbe_need_crosstalk_fix(hw)) {
-		u32 sfp_cage_full;
-
-		switch (hw->mac.type) {
-		case ixgbe_mac_82599EB:
-			sfp_cage_full = IXGBE_READ_REG(hw, IXGBE_ESDP) &
-					IXGBE_ESDP_SDP2;
-			break;
-		case ixgbe_mac_X550EM_x:
-		case ixgbe_mac_X550EM_a:
-			sfp_cage_full = IXGBE_READ_REG(hw, IXGBE_ESDP) &
-					IXGBE_ESDP_SDP0;
-			break;
-		default:
-			/* sanity check - No SFP+ devices here */
-			sfp_cage_full = false;
-			break;
-		}
+		enum ixgbe_sfp_cage_status sfp_cage_status;
 
-		if (!sfp_cage_full) {
+		sfp_cage_status = ixgbe_check_sfp_cage(hw);
+		if (sfp_cage_status != IXGBE_SFP_CAGE_FULL) {
 			*link_up = false;
 			*speed = IXGBE_LINK_SPEED_UNKNOWN;
 			return IXGBE_SUCCESS;
diff --git a/drivers/net/ixgbe/base/ixgbe_common.h b/drivers/net/ixgbe/base/ixgbe_common.h
index 5bdb484407..30db9a08c4 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.h
+++ b/drivers/net/ixgbe/base/ixgbe_common.h
@@ -112,6 +112,14 @@ s32 ixgbe_set_vlvf_generic(struct ixgbe_hw *hw, u32 vlan, u32 vind,
 s32 ixgbe_clear_vfta_generic(struct ixgbe_hw *hw);
 s32 ixgbe_find_vlvf_slot(struct ixgbe_hw *hw, u32 vlan, bool vlvf_bypass);
 
+enum ixgbe_sfp_cage_status {
+	IXGBE_SFP_CAGE_EMPTY = 0,
+	IXGBE_SFP_CAGE_FULL,
+	IXGBE_SFP_CAGE_UNKNOWN = -1,
+	IXGBE_SFP_CAGE_NOCAGE = -2,
+};
+enum ixgbe_sfp_cage_status ixgbe_check_sfp_cage(struct ixgbe_hw *hw);
+
 s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw,
 			       ixgbe_link_speed *speed,
 			       bool *link_up, bool link_up_wait_to_complete);
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 8d4d9bbfef..d8d51d2c3f 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1228,9 +1228,17 @@ s32 ixgbe_reset_phy_nl(struct ixgbe_hw *hw)
 s32 ixgbe_identify_module_generic(struct ixgbe_hw *hw)
 {
 	s32 status = IXGBE_ERR_SFP_NOT_PRESENT;
+	enum ixgbe_sfp_cage_status sfp_cage_status;
 
 	DEBUGFUNC("ixgbe_identify_module_generic");
 
+	sfp_cage_status = ixgbe_check_sfp_cage(hw);
+	if (sfp_cage_status == IXGBE_SFP_CAGE_EMPTY ||
+	    sfp_cage_status == IXGBE_SFP_CAGE_NOCAGE) {
+		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
+		return status;
+	}
+
 	switch (hw->mac.ops.get_media_type(hw)) {
 	case ixgbe_media_type_fiber:
 		status = ixgbe_identify_sfp_module_generic(hw);
-- 
2.31.1

[PATCH 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before write

[Stephen Douthit]

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>
---
 drivers/net/ixgbe/base/ixgbe_common.c | 46 +++++++++++++++++++++++++++
 drivers/net/ixgbe/base/ixgbe_phy.h    |  3 ++
 2 files changed, 49 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index 2764cf7cf1..3be1cc7fa2 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -5371,6 +5371,7 @@ s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw,
 void ixgbe_set_soft_rate_select_speed(struct ixgbe_hw *hw,
 					ixgbe_link_speed speed)
 {
+	enum ixgbe_sfp_cage_status sfp_cage_status;
 	s32 status;
 	u8 rs, eeprom_data;
 
@@ -5387,6 +5388,51 @@ void ixgbe_set_soft_rate_select_speed(struct ixgbe_hw *hw,
 		return;
 	}
 
+	/* Can't set rate on missing devices, skip all I2C access */
+	sfp_cage_status = ixgbe_check_sfp_cage(hw);
+	if (sfp_cage_status == IXGBE_SFP_CAGE_EMPTY ||
+	    sfp_cage_status == IXGBE_SFP_CAGE_NOCAGE) {
+		DEBUGOUT("No SFP\n");
+		return;
+	}
+
+	/* This only applies to SFF-8472 devices, so check that this device has
+	 * a non-zero SFF8472 compliance code @ device 0xA0 byte 94
+	 */
+	status = hw->phy.ops.read_i2c_eeprom(hw,
+					     IXGBE_SFF_SFF_8472_COMP,
+					     &eeprom_data);
+	if (status || !eeprom_data) {
+		DEBUGOUT("Not a SFF-8472 device\n");
+		goto out;
+	}
+
+	/* (read|write)_i2c_byte() don't support the address change mechanism
+	 * outlined in section 8.9 "Addressing Modes" of SFF_8472, so if that
+	 * is a requirement give up
+	 */
+	status = hw->phy.ops.read_i2c_eeprom(hw,
+					     IXGBE_SFF_SFF_8472_SWAP,
+					     &eeprom_data);
+	if (status || (eeprom_data & IXGBE_SFF_ADDRESSING_MODE)) {
+		DEBUGOUT("Address change not supported\n");
+		goto out;
+	}
+	/* Digital diagnostic monitoring must be supported for rate select */
+	if (!(eeprom_data & IXGBE_SFF_DDM_IMPLEMENTED)) {
+		DEBUGOUT("DDM not implemented\n");
+		goto out;
+	}
+
+	/* Finally check if the optional rate select feature is implemented */
+	status = hw->phy.ops.read_i2c_eeprom(hw,
+					     IXGBE_SFF_SFF_8472_EOPT,
+					     &eeprom_data);
+	if (status || !(eeprom_data & IXGBE_SFF_HAVE_RS)) {
+		DEBUGOUT("Rate select not supported");
+		goto out;
+	}
+
 	/* 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
 #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
 #define IXGBE_SFF_SOFT_RS_SELECT_1G	0x0
 #define IXGBE_SFF_ADDRESSING_MODE	0x4
+#define IXGBE_SFF_DDM_IMPLEMENTED	0x40
+#define IXGBE_SFF_HAVE_RS		0x2
 #define IXGBE_SFF_QSFP_DA_ACTIVE_CABLE	0x1
 #define IXGBE_SFF_QSFP_DA_PASSIVE_CABLE	0x8
 #define IXGBE_SFF_QSFP_CONNECTOR_NOT_SEPARABLE	0x23
-- 
2.31.1

[PATCH 4/7] net/ixgbe: Run 82599 link status workaround only on affected devices

[Stephen Douthit]

1ca05831b9b added a check that SDP3 (used as a TX_DISABLE output to the
SFP cage on these cards) is not asserted to avoid incorrectly reporting
link up when the SFP's laser is turned off.

ff8162cb957 limited this workaround to fiber ports

Refactor this so it's:

* Not open coded in ixgbe_dev_link_update_share()
* Runs only on fiber 82599 devices, not all fiber ixgbe devs (which don't
  all use SDP3 as TX_DISABLE)

Fixes: 1ca05831b9b ("net/ixgbe: fix link status")
Fixes: ff8162cb957 ("net/ixgbe: fix link status")
Cc: stable@dpdk.org

Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/base/ixgbe_82599.c | 41 ++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.c     |  7 -----
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c b/drivers/net/ixgbe/base/ixgbe_82599.c
index 69fd4cd3fb..5786114b0a 100644
--- a/drivers/net/ixgbe/base/ixgbe_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_82599.c
@@ -28,6 +28,39 @@ STATIC s32 ixgbe_read_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
 STATIC s32 ixgbe_write_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
 					u8 dev_addr, u8 data);
 
+/**
+ * ixgbe_check_mac_link_82599_fiber - Determine link and speed status
+ *
+ * @hw: pointer to hardware structure
+ * @speed: pointer to link speed
+ * @link_up: true when link is up
+ * @link_up_wait_to_complete: bool used to wait for link up or not
+ *
+ * Call the generic MAC check_link function, but also take into account the
+ * state of SDP3, which is a GPIO configured as an output driving the TX_DISABLE
+ * pin on the SFP cage.  This prevents reporting a false positive link up in the
+ * case where the link partner is transmitting, but we are not.
+ **/
+STATIC s32 ixgbe_check_mac_link_82599_fiber(struct ixgbe_hw *hw,
+					    ixgbe_link_speed *speed,
+					    bool *link_up,
+					    bool link_up_wait_to_complete)
+{
+	u32 esdp_reg;
+	s32 err;
+
+	DEBUGFUNC("ixgbe_check_mac_link_82599_fiber");
+
+	err = ixgbe_check_mac_link_generic(hw, speed, link_up,
+					   link_up_wait_to_complete);
+
+	esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
+	if ((esdp_reg & IXGBE_ESDP_SDP3))
+		*link_up = 0;
+
+	return err;
+}
+
 void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
 {
 	struct ixgbe_mac_info *mac = &hw->mac;
@@ -52,6 +85,14 @@ void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
 		mac->ops.flap_tx_laser = NULL;
 	}
 
+	/*
+	 * For 82599 SFP+ fiber, make sure that SDP3 (TX_DISABLE to SFP cage)
+	 * isn't asserted.  Either by mac->ops.disable_tx_laser(), or possibly
+	 * management firmware
+	 */
+	if (mac->ops.get_media_type(hw) == ixgbe_media_type_fiber)
+		mac->ops.check_link = ixgbe_check_mac_link_82599_fiber;
+
 	if (hw->phy.multispeed_fiber) {
 		/* Set up dual speed SFP+ support */
 		mac->ops.setup_link = ixgbe_setup_mac_link_multispeed_fiber;
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 66f7af95de..34b7cb2d4e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4216,7 +4216,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 	bool link_up;
 	int diag;
 	int wait = 1;
-	u32 esdp_reg;
 
 	memset(&link, 0, sizeof(link));
 	link.link_status = RTE_ETH_LINK_DOWN;
@@ -4250,12 +4249,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 		return rte_eth_linkstatus_set(dev, &link);
 	}
 
-	if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
-		esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
-		if ((esdp_reg & IXGBE_ESDP_SDP3))
-			link_up = 0;
-	}
-
 	if (link_up == 0) {
 		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
 			ixgbe_dev_wait_setup_link_complete(dev, 0);
-- 
2.31.1

[PATCH 5/7] net/ixgbe: Fix SFP detection and linking on hotplug

[Stephen Douthit]

Currently the ixgbe driver does not ID any SFP except for the first one
plugged in. This can lead to no-link, or incorrect speed conditions.

For example:

* If link is initially established with a 1G SFP, and later a 1G/10G
multispeed part is later installed, then the MAC link setup functions are
never called to change from 1000BASE-X to 10GBASE-R mode, and the link
stays running at the slower rate.

* If link is initially established with a 1G SFP, and later a 10G only
module is later installed, no link is established, since we are still
trasnsmitting in 1000BASE-X mode to a 10GBASE-R only partner.

Refactor the SFP ID/setup, and link setup code, to more closely match the
flow of the mainline kernel driver which does not have these issues.  In
that driver a service task runs periodically to handle these operations
based on bit flags that have been set (usually via interrupt or userspace
request), and then get cleared once the requested subtask has been
completed.

Fixes: af75078fece ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/base/ixgbe_type.h |   2 +
 drivers/net/ixgbe/ixgbe_ethdev.c    | 391 ++++++++++++++++++----------
 drivers/net/ixgbe/ixgbe_ethdev.h    |  18 +-
 3 files changed, 266 insertions(+), 145 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index b7eec45635..c23257aa4c 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -45,6 +45,8 @@
 
 #include "ixgbe_osdep.h"
 
+#define BIT(a) (1UL << (a))
+
 /* Override this by setting IOMEM in your ixgbe_osdep.h header */
 
 /* Vendor ID */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 34b7cb2d4e..974b3427d1 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -229,9 +229,6 @@ static int ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev);
 static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev);
 static void ixgbe_dev_interrupt_handler(void *param);
 static void ixgbe_dev_interrupt_delayed_handler(void *param);
-static void *ixgbe_dev_setup_link_thread_handler(void *param);
-static int ixgbe_dev_wait_setup_link_complete(struct rte_eth_dev *dev,
-					      uint32_t timeout_ms);
 
 static int ixgbe_add_rar(struct rte_eth_dev *dev,
 			struct rte_ether_addr *mac_addr,
@@ -1032,6 +1029,205 @@ ixgbe_swfw_lock_reset(struct ixgbe_hw *hw)
 	ixgbe_release_swfw_semaphore(hw, mask);
 }
 
+static s32
+ixgbe_sfp_id_and_setup(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	enum ixgbe_sfp_cage_status sfp_cage_status;
+	s32 err;
+
+	/* Can't ID or setup SFP if it's not plugged in */
+	sfp_cage_status = ixgbe_check_sfp_cage(hw);
+	if (sfp_cage_status == IXGBE_SFP_CAGE_EMPTY ||
+	    sfp_cage_status == IXGBE_SFP_CAGE_NOCAGE)
+		return IXGBE_ERR_SFP_NOT_PRESENT;
+
+	/* Something's in the cage, ID it */
+	hw->phy.ops.identify_sfp(hw);
+
+	/* Unknown module type, give up */
+	if (hw->phy.sfp_type == ixgbe_sfp_type_unknown) {
+		PMD_DRV_LOG(ERR, "unknown SFP type, giving up");
+		return IXGBE_ERR_SFP_NOT_SUPPORTED;
+	}
+
+	/* This should be a redundant check, since we looked at the
+	 * PRSNT# signal from the cage above, but just in case this is
+	 * an SFP that's slow to respond to I2C pokes correctly, try it
+	 * again later
+	 */
+	if (hw->phy.sfp_type == ixgbe_sfp_type_not_present) {
+		PMD_DRV_LOG(ERR, "IDed SFP as absent but cage PRSNT# active!?");
+		return IXGBE_ERR_SFP_NOT_PRESENT;
+	}
+
+	/* SFP is present and identified, try to set it up */
+	err = hw->mac.ops.setup_sfp(hw);
+	if (err)
+		PMD_DRV_LOG(ERR, "setup_sfp() failed %d", err);
+
+	return err;
+}
+
+/**
+ * SFP polling task to identify and setup SFPs.
+ *
+ * If we see an SFP present, try to ID it and set it up.  If that works then let
+ * ixgbe_dev_link_update() do the remainder of the non-SFP link setup.
+ *
+ * If that fails, schedule ourself to try again.
+ *
+ * @param handle
+ *  Pointer to interrupt handle.
+ * @param param
+ *  The address of parameter (struct rte_eth_dev *) registered before.
+ *
+ * @return
+ *  void
+ */
+static void
+ixgbe_sfp_service(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	enum ixgbe_sfp_cage_status sfp_cage_status;
+	s32 err;
+
+	/* No setup requested?  Nothing to do */
+	if (!(intr->flags & IXGBE_FLAG_NEED_SFP_SETUP))
+		return;
+
+	sfp_cage_status = ixgbe_check_sfp_cage(hw);
+	if (sfp_cage_status == IXGBE_SFP_CAGE_EMPTY ||
+	    sfp_cage_status == IXGBE_SFP_CAGE_NOCAGE)
+		return;
+
+	err = ixgbe_sfp_id_and_setup(dev);
+	if (err) {
+		PMD_DRV_LOG(DEBUG, "failed to ID & setup SFP %d", err);
+		return;
+	}
+
+	/* Setup is done, clear the flag, but make sure link config runs for new SFP */
+	intr->flags &= ~IXGBE_FLAG_NEED_SFP_SETUP;
+	intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
+
+	/*
+	 * Since this is a new SFP, clear the old advertised speed mask so we don't
+	 * end up using an old slower rate
+	 */
+	hw->phy.autoneg_advertised = 0;
+}
+
+static void
+ixgbe_link_service(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	bool link_up, autoneg = false;
+	u32 speed;
+	s32 err;
+
+	/* Skip if we still need to setup an SFP, or if no link config requested
+	 */
+	if ((intr->flags & IXGBE_FLAG_NEED_SFP_SETUP) ||
+	    !(intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG))
+		return;
+
+	speed = hw->phy.autoneg_advertised;
+	if (!speed)
+		ixgbe_get_link_capabilities(hw, &speed, &autoneg);
+
+	err = ixgbe_setup_link(hw, speed, true);
+	if (err) {
+		PMD_DRV_LOG(ERR, "ixgbe_setup_link failed %d", err);
+		return;
+	}
+
+	/* Update internal link status, waiting for link */
+	err = ixgbe_check_link(hw, &speed, &link_up, 1);
+	if (!err && link_up)
+		intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
+
+	if (!ixgbe_dev_link_update(dev, 0)) {
+		ixgbe_dev_link_status_print(dev);
+		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+	}
+}
+
+/*
+ * Is time b after time a?
+ */
+static bool
+ts_time_after(const struct timespec *a, const struct timespec *b)
+{
+	if (b->tv_sec > a->tv_sec ||
+	    (b->tv_sec == a->tv_sec && b->tv_nsec > a->tv_nsec))
+		return true;
+
+	return false;
+}
+
+/*
+ * Service task thread to handle periodic tasks
+ */
+static void *
+ixgbe_dev_service_thread_handler(void *param)
+{
+#define SERVICE_THREAD_US_SLEEP (100 * 1000)
+
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct timespec ts, end_ts;
+	uint32_t speed;
+	s32 err;
+	bool link_up;
+
+	while (1) {
+		ixgbe_sfp_service(dev);
+		ixgbe_link_service(dev);
+
+		/* Run the service thread handler more frequently when link is
+		 * down to reduce link up latency (every 200ms vs 1s)
+		 *
+		 * Use a number of smaller sleeps to decrease exit latency when
+		 * ixgbe_dev_stop() wants this thread to join
+		 */
+		err = ixgbe_check_link(hw, &speed, &link_up, 0);
+		clock_gettime(CLOCK_REALTIME, &end_ts);
+		if (err == IXGBE_SUCCESS && link_up) {
+			end_ts.tv_sec += 1;
+		} else {
+			end_ts.tv_nsec += 200000000;
+			if (end_ts.tv_nsec > 1000000000) {
+				end_ts.tv_nsec -= 1000000000;
+				end_ts.tv_sec++;
+			}
+		}
+
+		while (1) {
+			/* Don't assume that rte_delay_us_sleep() provides a
+			 * cancellation point in all execution environments,
+			 * explicitly check for thread cancellation
+			 */
+			pthread_testcancel();
+			rte_delay_us_sleep(SERVICE_THREAD_US_SLEEP);
+
+			clock_gettime(CLOCK_REALTIME, &ts);
+			if (ts_time_after(&end_ts, &ts))
+				break;
+		}
+	}
+
+	/* Never return */
+	return NULL;
+}
+
 /*
  * This function is based on code in ixgbe_attach() in base/ixgbe.c.
  * It returns 0 on success.
@@ -1039,7 +1235,6 @@ ixgbe_swfw_lock_reset(struct ixgbe_hw *hw)
 static int
 eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 {
-	struct ixgbe_adapter *ad = eth_dev->data->dev_private;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct ixgbe_hw *hw =
@@ -1094,7 +1289,6 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 		return 0;
 	}
 
-	rte_atomic32_clear(&ad->link_thread_running);
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
@@ -1537,7 +1731,6 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 {
 	int diag;
 	uint32_t tc, tcs;
-	struct ixgbe_adapter *ad = eth_dev->data->dev_private;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct ixgbe_hw *hw =
@@ -1580,7 +1773,6 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
-	rte_atomic32_clear(&ad->link_thread_running);
 	ixgbevf_parse_devargs(eth_dev->data->dev_private,
 			      pci_dev->device.devargs);
 
@@ -2382,6 +2574,8 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_adapter *adapter = dev->data->dev_private;
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
 	PMD_INIT_FUNC_TRACE();
@@ -2400,6 +2594,10 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
 	/* set flag to update link status after init */
 	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
 
+	/* set flag to setup SFP after init */
+	if (ixgbe_is_sfp(hw))
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+
 	/*
 	 * Initialize to TRUE. If any of Rx queues doesn't meet the bulk
 	 * allocation or vector Rx preconditions we will reset it.
@@ -2419,13 +2617,24 @@ ixgbe_dev_phy_intr_setup(struct rte_eth_dev *dev)
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	uint32_t gpie;
 
-	/* only set up it on X550EM_X */
+	/* only set up it on X550EM_X (external PHY interrupt)
+	 * or on x550em_a_* for SFP_PRSNT# de-assertion (SFP removal event)
+	 */
 	if (hw->mac.type == ixgbe_mac_X550EM_x) {
 		gpie = IXGBE_READ_REG(hw, IXGBE_GPIE);
 		gpie |= IXGBE_SDP0_GPIEN_X550EM_x;
 		IXGBE_WRITE_REG(hw, IXGBE_GPIE, gpie);
 		if (hw->phy.type == ixgbe_phy_x550em_ext_t)
 			intr->mask |= IXGBE_EICR_GPI_SDP0_X550EM_x;
+	} else if (hw->mac.type == ixgbe_mac_X550EM_a) {
+		gpie = IXGBE_READ_REG(hw, IXGBE_GPIE);
+		gpie |= IXGBE_SDP0_GPIEN_X550EM_a;
+		IXGBE_WRITE_REG(hw, IXGBE_GPIE, gpie);
+		intr->mask |= IXGBE_EICR_GPI_SDP0_X550EM_a;
+	} else {
+		PMD_DRV_LOG(DEBUG,
+			    "No PHY/SFP interrupt for MAC %d, PHY %d\n",
+			    hw->mac.type, hw->phy.type);
 	}
 }
 
@@ -2548,8 +2757,11 @@ ixgbe_flow_ctrl_enable(struct rte_eth_dev *dev, struct ixgbe_hw *hw)
 static int
 ixgbe_dev_start(struct rte_eth_dev *dev)
 {
+	struct ixgbe_adapter *ad = dev->data->dev_private;
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_vf_info *vfinfo =
 		*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
@@ -2570,9 +2782,6 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Stop the link setup handler before resetting the HW. */
-	ixgbe_dev_wait_setup_link_complete(dev, 0);
-
 	/* disable uio/vfio intr/eventfd mapping */
 	rte_intr_disable(intr_handle);
 
@@ -2688,12 +2897,6 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) {
-		err = hw->mac.ops.setup_sfp(hw);
-		if (err)
-			goto error;
-	}
-
 	if (hw->mac.ops.get_media_type(hw) == ixgbe_media_type_copper) {
 		/* Turn on the copper */
 		ixgbe_set_phy_power(hw, true);
@@ -2805,6 +3008,20 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	ixgbe_l2_tunnel_conf(dev);
 	ixgbe_filter_restore(dev);
 
+	/* Spawn service thread */
+	if (ixgbe_is_sfp(hw)) {
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+		err = rte_ctrl_thread_create(&ad->service_thread_tid,
+					     "ixgbe-service-thread",
+					     NULL,
+					     ixgbe_dev_service_thread_handler,
+					     dev);
+		if (err) {
+			PMD_DRV_LOG(ERR, "service_thread err");
+			goto error;
+		}
+	}
+
 	if (tm_conf->root && !tm_conf->committed)
 		PMD_DRV_LOG(WARNING,
 			    "please call hierarchy_commit() "
@@ -2815,12 +3032,6 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	if (err)
 		goto error;
 
-	/*
-	 * Update link status right before return, because it may
-	 * start link configuration process in a separate thread.
-	 */
-	ixgbe_dev_link_update(dev, 0);
-
 	/* setup the macsec setting register */
 	if (macsec_setting->offload_en)
 		ixgbe_dev_macsec_register_enable(dev, macsec_setting);
@@ -2850,13 +3061,21 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 	int vf;
 	struct ixgbe_tm_conf *tm_conf =
 		IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+	void *res;
+	s32 err;
 
 	if (hw->adapter_stopped)
 		return 0;
 
 	PMD_INIT_FUNC_TRACE();
 
-	ixgbe_dev_wait_setup_link_complete(dev, 0);
+	/* Cancel the service thread, and wait for it to join */
+	err = pthread_cancel(adapter->service_thread_tid);
+	if (err)
+		PMD_DRV_LOG(ERR, "failed to cancel service thread %d", err);
+	err = pthread_join(adapter->service_thread_tid, &res);
+	if (err)
+		PMD_DRV_LOG(ERR, "failed to join service thread %d", err);
 
 	/* disable interrupts */
 	ixgbe_disable_intr(hw);
@@ -2935,7 +3154,6 @@ ixgbe_dev_set_link_up(struct rte_eth_dev *dev)
 	} else {
 		/* Turn on the laser */
 		ixgbe_enable_tx_laser(hw);
-		ixgbe_dev_link_update(dev, 0);
 	}
 
 	return 0;
@@ -2966,7 +3184,6 @@ ixgbe_dev_set_link_down(struct rte_eth_dev *dev)
 	} else {
 		/* Turn off the laser */
 		ixgbe_disable_tx_laser(hw);
-		ixgbe_dev_link_update(dev, 0);
 	}
 
 	return 0;
@@ -4118,57 +4335,6 @@ ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 	return ret_val;
 }
 
-/*
- * If @timeout_ms was 0, it means that it will not return until link complete.
- * It returns 1 on complete, return 0 on timeout.
- */
-static int
-ixgbe_dev_wait_setup_link_complete(struct rte_eth_dev *dev, uint32_t timeout_ms)
-{
-#define WARNING_TIMEOUT    9000 /* 9s  in total */
-	struct ixgbe_adapter *ad = dev->data->dev_private;
-	uint32_t timeout = timeout_ms ? timeout_ms : WARNING_TIMEOUT;
-
-	while (rte_atomic32_read(&ad->link_thread_running)) {
-		msec_delay(1);
-		timeout--;
-
-		if (timeout_ms) {
-			if (!timeout)
-				return 0;
-		} else if (!timeout) {
-			/* It will not return until link complete */
-			timeout = WARNING_TIMEOUT;
-			PMD_DRV_LOG(ERR, "IXGBE link thread not complete too long time!");
-		}
-	}
-
-	return 1;
-}
-
-static void *
-ixgbe_dev_setup_link_thread_handler(void *param)
-{
-	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
-	struct ixgbe_adapter *ad = dev->data->dev_private;
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct ixgbe_interrupt *intr =
-		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	u32 speed;
-	bool autoneg = false;
-
-	pthread_detach(pthread_self());
-	speed = hw->phy.autoneg_advertised;
-	if (!speed)
-		ixgbe_get_link_capabilities(hw, &speed, &autoneg);
-
-	ixgbe_setup_link(hw, speed, true);
-
-	intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
-	rte_atomic32_clear(&ad->link_thread_running);
-	return NULL;
-}
-
 /*
  * In freebsd environment, nic_uio drivers do not support interrupts,
  * rte_intr_callback_register() will fail to register interrupts.
@@ -4208,11 +4374,8 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 			    int wait_to_complete, int vf)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct ixgbe_adapter *ad = dev->data->dev_private;
 	struct rte_eth_link link;
 	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
-	struct ixgbe_interrupt *intr =
-		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	bool link_up;
 	int diag;
 	int wait = 1;
@@ -4226,9 +4389,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 
 	hw->mac.get_link_status = true;
 
-	if (intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG)
-		return rte_eth_linkstatus_set(dev, &link);
-
 	/* check if it needs to wait to complete, if lsc interrupt is enabled */
 	if (wait_to_complete == 0 || dev->data->dev_conf.intr_conf.lsc != 0)
 		wait = 0;
@@ -4243,38 +4403,12 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 	else
 		diag = ixgbe_check_link(hw, &link_speed, &link_up, wait);
 
-	if (diag != 0) {
+	if (diag != 0 || !link_up) {
 		link.link_speed = RTE_ETH_SPEED_NUM_100M;
 		link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
 		return rte_eth_linkstatus_set(dev, &link);
 	}
 
-	if (link_up == 0) {
-		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
-			ixgbe_dev_wait_setup_link_complete(dev, 0);
-			if (rte_atomic32_test_and_set(&ad->link_thread_running)) {
-				/* To avoid race condition between threads, set
-				 * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
-				 * when there is no link thread running.
-				 */
-				intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
-				if (rte_ctrl_thread_create(&ad->link_thread_tid,
-					"ixgbe-link-handler",
-					NULL,
-					ixgbe_dev_setup_link_thread_handler,
-					dev) < 0) {
-					PMD_DRV_LOG(ERR,
-						"Create link thread failed!");
-					rte_atomic32_clear(&ad->link_thread_running);
-				}
-			} else {
-				PMD_DRV_LOG(ERR,
-					"Other link thread is running now!");
-			}
-		}
-		return rte_eth_linkstatus_set(dev, &link);
-	}
-
 	link.link_status = RTE_ETH_LINK_UP;
 	link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
 
@@ -4480,8 +4614,6 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 	eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
 	PMD_DRV_LOG(DEBUG, "eicr %x", eicr);
 
-	intr->flags = 0;
-
 	/* set flag for async link update */
 	if (eicr & IXGBE_EICR_LSC)
 		intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
@@ -4497,6 +4629,11 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 	    (eicr & IXGBE_EICR_GPI_SDP0_X550EM_x))
 		intr->flags |= IXGBE_FLAG_PHY_INTERRUPT;
 
+	/* Check for loss of SFP */
+	if (hw->mac.type ==  ixgbe_mac_X550EM_a &&
+	    (eicr & IXGBE_EICR_GPI_SDP0_X550EM_a))
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+
 	return 0;
 }
 
@@ -4548,11 +4685,13 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
 static int
 ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 {
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	int64_t timeout;
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	int64_t timeout;
 
 	PMD_DRV_LOG(DEBUG, "intr action type %d", intr->flags);
 
@@ -4587,16 +4726,14 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 		if (rte_eal_alarm_set(timeout * 1000,
 				      ixgbe_dev_interrupt_delayed_handler, (void *)dev) < 0)
 			PMD_DRV_LOG(ERR, "Error setting alarm");
-		else {
-			/* remember original mask */
-			intr->mask_original = intr->mask;
+		else
 			/* only disable lsc interrupt */
 			intr->mask &= ~IXGBE_EIMS_LSC;
-		}
 	}
 
 	PMD_DRV_LOG(DEBUG, "enable intr immediately");
 	ixgbe_enable_intr(dev);
+	rte_intr_ack(intr_handle);
 
 	return 0;
 }
@@ -4619,8 +4756,6 @@ static void
 ixgbe_dev_interrupt_delayed_handler(void *param)
 {
 	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
-	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
@@ -4650,13 +4785,10 @@ ixgbe_dev_interrupt_delayed_handler(void *param)
 		intr->flags &= ~IXGBE_FLAG_MACSEC;
 	}
 
-	/* restore original mask */
-	intr->mask = intr->mask_original;
-	intr->mask_original = 0;
+	if (dev->data->dev_conf.intr_conf.lsc != 0)
+		intr->mask |= IXGBE_EICR_LSC;
 
-	PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
 	ixgbe_enable_intr(dev);
-	rte_intr_ack(intr_handle);
 }
 
 /**
@@ -5298,9 +5430,6 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* Stop the link setup handler before resetting the HW. */
-	ixgbe_dev_wait_setup_link_complete(dev, 0);
-
 	err = hw->mac.ops.reset_hw(hw);
 
 	/**
@@ -5380,12 +5509,6 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 	/* Re-enable interrupt for VF */
 	ixgbevf_intr_enable(dev);
 
-	/*
-	 * Update link status right before return, because it may
-	 * start link configuration process in a separate thread.
-	 */
-	ixgbevf_dev_link_update(dev, 0);
-
 	hw->adapter_stopped = false;
 
 	return 0;
@@ -5404,8 +5527,6 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	ixgbe_dev_wait_setup_link_complete(dev, 0);
-
 	ixgbevf_intr_disable(dev);
 
 	dev->data->dev_started = 0;
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 83e8b5e56a..14658a2a82 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -24,11 +24,12 @@
 #include <rte_tm_driver.h>
 
 /* need update link, bit flag */
-#define IXGBE_FLAG_NEED_LINK_UPDATE (uint32_t)(1 << 0)
-#define IXGBE_FLAG_MAILBOX          (uint32_t)(1 << 1)
-#define IXGBE_FLAG_PHY_INTERRUPT    (uint32_t)(1 << 2)
-#define IXGBE_FLAG_MACSEC           (uint32_t)(1 << 3)
-#define IXGBE_FLAG_NEED_LINK_CONFIG (uint32_t)(1 << 4)
+#define IXGBE_FLAG_NEED_LINK_UPDATE BIT(0)
+#define IXGBE_FLAG_MAILBOX          BIT(1)
+#define IXGBE_FLAG_PHY_INTERRUPT    BIT(2)
+#define IXGBE_FLAG_MACSEC           BIT(3)
+#define IXGBE_FLAG_NEED_LINK_CONFIG BIT(4)
+#define IXGBE_FLAG_NEED_SFP_SETUP   BIT(5)
 
 /*
  * Defines that were not part of ixgbe_type.h as they are not used by the
@@ -41,7 +42,7 @@
 #define IXGBE_RXDADV_ERR_CKSUM_MSK  3
 #define IXGBE_ADVTXD_MACLEN_SHIFT   9          /* Bit shift for l2_len */
 #define IXGBE_NB_STAT_MAPPING_REGS  32
-#define IXGBE_EXTENDED_VLAN	  (uint32_t)(1 << 26) /* EXTENDED VLAN ENABLE */
+#define IXGBE_EXTENDED_VLAN	    BIT(26)    /* EXTENDED VLAN ENABLE */
 #define IXGBE_VFTA_SIZE 128
 #define IXGBE_HKEY_MAX_INDEX 10
 #define IXGBE_MAX_RX_QUEUE_NUM	128
@@ -223,8 +224,6 @@ struct ixgbe_rte_flow_rss_conf {
 struct ixgbe_interrupt {
 	uint32_t flags;
 	uint32_t mask;
-	/*to save original mask during delayed handler */
-	uint32_t mask_original;
 };
 
 struct ixgbe_stat_mapping_registers {
@@ -506,8 +505,7 @@ struct ixgbe_adapter {
 	 */
 	uint8_t pflink_fullchk;
 	uint8_t mac_ctrl_frame_fwd;
-	rte_atomic32_t link_thread_running;
-	pthread_t link_thread_tid;
+	pthread_t service_thread_tid;
 };
 
 struct ixgbe_vf_representor {
-- 
2.31.1

[PATCH 6/7] net/ixgbe: Retry SFP ID read field to handle misbehaving SFPs

[Stephen Douthit]

Some XGS-PON SFPs have been observed ACKing I2C reads and return
uninitialized garbage while their uC boots.  This can lead to the SFP ID
code marking an otherwise working SFP module as unsupported if a bogus
ID value is read while it's internal PHY/microcontroller is still
booting.

Retry the ID read several times looking not just for NAK, but also for a
valid ID field.

Since the device isn't NAKing the trasaction the existing longer retry code
in ixgbe_read_i2c_byte_generic_int() doesn't apply here.

Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/base/ixgbe_phy.c | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index d8d51d2c3f..429b2b4142 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1275,6 +1275,7 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 	u8 cable_tech = 0;
 	u8 cable_spec = 0;
 	u16 enforce_sfp = 0;
+	u8 id_reads;
 
 	DEBUGFUNC("ixgbe_identify_sfp_module_generic");
 
@@ -1287,12 +1288,34 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 	/* LAN ID is needed for I2C access */
 	hw->mac.ops.set_lan_id(hw);
 
-	status = hw->phy.ops.read_i2c_eeprom(hw,
-					     IXGBE_SFF_IDENTIFIER,
-					     &identifier);
+	/* Need to check this a couple of times for a sane value.
+	 *
+	 * SFPs that have a uC slaved to the I2C bus (vs. a dumb EEPROM) can be
+	 * poorly designed such that they will ACK I2C reads and return
+	 * whatever bogus data is in the SRAM (or whatever is backing the slave
+	 * device) before things are truely initialized.
+	 *
+	 * In a perfect world devices would NAK I2C requests until they were
+	 * sane, but here we are.
+	 *
+	 * Give such devices a couple tries to get their act together before
+	 * marking the device as unsupported.
+	 */
+	for (id_reads = 0; id_reads < 5; id_reads++) {
+		status = hw->phy.ops.read_i2c_eeprom(hw,
+						     IXGBE_SFF_IDENTIFIER,
+						     &identifier);
 
-	if (status != IXGBE_SUCCESS)
+		DEBUGOUT("status %d, id %d\n", status, identifier);
+		if (!status &&
+		    identifier == IXGBE_SFF_IDENTIFIER_SFP)
+			break;
+	}
+
+	if (status != IXGBE_SUCCESS) {
+		DEBUGOUT("Failed SFF ID read (%d attempts)\n", id_reads);
 		goto err_read_i2c_eeprom;
+	}
 
 	if (identifier != IXGBE_SFF_IDENTIFIER_SFP) {
 		hw->phy.type = ixgbe_phy_sfp_unsupported;
-- 
2.31.1

[PATCH 7/7] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices

[Stephen Douthit]

1G Cu SFPs are not officially supported on the X552/X553 family of devices
but treat them as 1G SX modules since they usually work.  Print a warning
though since support isn't validated, similar to what already happens for
other unofficially supported SFPs enabled via the allow_unsupported_sfps
parameter inherited from the mainline Linux driver.

Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 8810d1658e..8d1bc6c80d 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1538,9 +1538,21 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
 	case ixgbe_sfp_type_1g_lha_core1:
 		*linear = false;
 		break;
-	case ixgbe_sfp_type_unknown:
+	/* Copper SFPs are not officially supported for x550em devices, but can
+	 * often be made to work at fixed 1G speeds.  Pretend they're 1g_sx
+	 * modules here to allow g.Fast DSL SFPs to work.
+	 */
 	case ixgbe_sfp_type_1g_cu_core0:
+		EWARN(hw, "Pretending that unsupported 1g_cu SFP is 1g_sx\n");
+		*linear = false;
+		hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
+		break;
 	case ixgbe_sfp_type_1g_cu_core1:
+		EWARN(hw, "Pretending that unsupported 1g_cu SFP is 1g_sx\n");
+		*linear = false;
+		hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
+		break;
+	case ixgbe_sfp_type_unknown:
 	default:
 		return IXGBE_ERR_SFP_NOT_SUPPORTED;
 	}
-- 
2.31.1