[dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

ixgbe_identify_phy_x550em() was missing the code to handle unidentified
PHY that has been there in 82599 so it was not able to complete
initialization of ixgbe sequence if no sfp plugged in.
Port it over to return an appropriate type and complete init sequence
properly.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index f66f540761..ef51b6a9a6 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -343,13 +343,21 @@ STATIC void ixgbe_setup_mux_ctl(struct ixgbe_hw *hw)
  */
 STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)
 {
+	s32 status;
+
 	hw->mac.ops.set_lan_id(hw);
 
 	ixgbe_read_mng_if_sel_x550em(hw);
 
 	switch (hw->device_id) {
 	case IXGBE_DEV_ID_X550EM_A_SFP:
-		return ixgbe_identify_module_generic(hw);
+		status = ixgbe_identify_module_generic(hw);
+		/* Set PHY type none if no PHY detected */
+		if (hw->phy.type == ixgbe_phy_unknown) {
+			hw->phy.type = ixgbe_phy_none;
+			return IXGBE_SUCCESS;
+		}
+		return status;
 	case IXGBE_DEV_ID_X550EM_X_SFP:
 		/* set up for CS4227 usage */
 		ixgbe_setup_mux_ctl(hw);
@@ -357,8 +365,13 @@ STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)
 		/* Fallthrough */
 
 	case IXGBE_DEV_ID_X550EM_A_SFP_N:
-		return ixgbe_identify_module_generic(hw);
-		break;
+		status = ixgbe_identify_module_generic(hw);
+		/* Set PHY type none if no PHY detected */
+		if (hw->phy.type == ixgbe_phy_unknown) {
+			hw->phy.type = ixgbe_phy_none;
+			return IXGBE_SUCCESS;
+		}
+		return status;
 	case IXGBE_DEV_ID_X550EM_X_KX4:
 		hw->phy.type = ixgbe_phy_x550em_kx4;
 		break;
-- 
2.18.0

[dpdk-dev] [PATCH 2/2] net/ixgbe: add LHA ID to x550 code

[Luca Boccassi]

ixgbe is able to recognize 1G SX and LX id, but it is missing the LHA.
Add it, so that it can handle LHA sfp plugin.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
 drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
 drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
 drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
 drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
 5 files changed, 22 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c b/drivers/net/ixgbe/base/ixgbe_82599.c
index 26217212aa..0075404763 100644
--- a/drivers/net/ixgbe/base/ixgbe_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_82599.c
@@ -424,6 +424,8 @@ s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
 	/* Check if 1G SFP module. */
 	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 2df068ee30..e1e7a184c9 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1431,6 +1431,13 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 				else
 					hw->phy.sfp_type =
 						ixgbe_sfp_type_1g_lx_core1;
+			} else if (comp_codes_1g & IXGBE_SFF_1GBASELHA_CAPABLE) {
+				if (hw->bus.lan_id == 0)
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core0;
+				else
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core1;
 			} else {
 				hw->phy.sfp_type = ixgbe_sfp_type_unknown;
 			}
@@ -1518,6 +1525,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (comp_codes_10g == 0 &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1537,6 +1546,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (!(enforce_sfp & IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1864,11 +1875,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct ixgbe_hw *hw,
 	 */
 	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
+	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
 		sfp_type = ixgbe_sfp_type_srlr_core0;
 	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
+		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
 		sfp_type = ixgbe_sfp_type_srlr_core1;
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h b/drivers/net/ixgbe/base/ixgbe_phy.h
index cf8cadd96a..5a5f995003 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.h
+++ b/drivers/net/ixgbe/base/ixgbe_phy.h
@@ -70,6 +70,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
 #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
 #define IXGBE_SFF_1GBASET_CAPABLE	0x8
+#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
 #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
 #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
 #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index 6e03089eb9..2a384b71c5 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3748,6 +3748,8 @@ enum ixgbe_sfp_type {
 	ixgbe_sfp_type_1g_sx_core1 = 12,
 	ixgbe_sfp_type_1g_lx_core0 = 13,
 	ixgbe_sfp_type_1g_lx_core1 = 14,
+	ixgbe_sfp_type_1g_lha_core0 = 15,
+	ixgbe_sfp_type_1g_lha_core1 = 16,
 	ixgbe_sfp_type_not_present = 0xFFFE,
 	ixgbe_sfp_type_unknown = 0xFFFF
 };
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index ef51b6a9a6..f24c3c266c 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1575,6 +1575,8 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
 	case ixgbe_sfp_type_1g_sx_core1:
 	case ixgbe_sfp_type_1g_lx_core0:
 	case ixgbe_sfp_type_1g_lx_core1:
+	case ixgbe_sfp_type_1g_lha_core0:
+	case ixgbe_sfp_type_1g_lha_core1:
 		*linear = false;
 		break;
 	case ixgbe_sfp_type_unknown:
@@ -1915,6 +1917,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 		/* Check if 1G SFP module. */
 		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
+		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1
 		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1) {
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
-- 
2.18.0

Re: [dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Wed, 2018-08-15 at 18:09 +0100, Luca Boccassi wrote:
> ixgbe_identify_phy_x550em() was missing the code to handle
> unidentified
> PHY that has been there in 82599 so it was not able to complete
> initialization of ixgbe sequence if no sfp plugged in.
> Port it over to return an appropriate type and complete init sequence
> properly.
> 
> Fixes: d2e72774e58c ("ixgbe/base: support X550")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
>  drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)

Ping. Any chance this series could get a review? Thanks!

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

ixgbe_identify_phy_x550em() was missing the code to handle unidentified
PHY that has been there in 82599 so it was not able to complete
initialization of ixgbe sequence if no sfp plugged in.
Port it over to return an appropriate type and complete init sequence
properly.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
v2: refresh to remove merge conflict with master

 drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index f7b98af52..83b394861 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -315,13 +315,21 @@ STATIC void ixgbe_setup_mux_ctl(struct ixgbe_hw *hw)
  */
 STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)
 {
+	s32 status;
+
 	hw->mac.ops.set_lan_id(hw);
 
 	ixgbe_read_mng_if_sel_x550em(hw);
 
 	switch (hw->device_id) {
 	case IXGBE_DEV_ID_X550EM_A_SFP:
-		return ixgbe_identify_sfp_module_X550em(hw);
+		status = ixgbe_identify_sfp_module_X550em(hw);
+		/* Set PHY type none if no PHY detected */
+		if (hw->phy.type == ixgbe_phy_unknown) {
+			hw->phy.type = ixgbe_phy_none;
+			return IXGBE_SUCCESS;
+		}
+		return status;
 	case IXGBE_DEV_ID_X550EM_X_SFP:
 		/* set up for CS4227 usage */
 		ixgbe_setup_mux_ctl(hw);
@@ -329,8 +337,13 @@ STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)
 		/* Fallthrough */
 
 	case IXGBE_DEV_ID_X550EM_A_SFP_N:
-		return ixgbe_identify_sfp_module_X550em(hw);
-		break;
+		status = ixgbe_identify_sfp_module_X550em(hw);
+		/* Set PHY type none if no PHY detected */
+		if (hw->phy.type == ixgbe_phy_unknown) {
+			hw->phy.type = ixgbe_phy_none;
+			return IXGBE_SUCCESS;
+		}
+		return status;
 	case IXGBE_DEV_ID_X550EM_X_KX4:
 		hw->phy.type = ixgbe_phy_x550em_kx4;
 		break;
-- 
2.19.1

[dpdk-dev] [PATCH 2/2] net/ixgbe: add LHA ID to x550 code

[Luca Boccassi]

ixgbe is able to recognize 1G SX and LX id, but it is missing the LHA.
Add it, so that it can handle LHA sfp plugin.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
 drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
 drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
 drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
 drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
 5 files changed, 22 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c b/drivers/net/ixgbe/base/ixgbe_82599.c
index 7de753fd3..96bdde62c 100644
--- a/drivers/net/ixgbe/base/ixgbe_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_82599.c
@@ -392,6 +392,8 @@ s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
 	/* Check if 1G SFP module. */
 	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 6cdd8fbab..dd118f917 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1402,6 +1402,13 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 				else
 					hw->phy.sfp_type =
 						ixgbe_sfp_type_1g_lx_core1;
+			} else if (comp_codes_1g & IXGBE_SFF_1GBASELHA_CAPABLE) {
+				if (hw->bus.lan_id == 0)
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core0;
+				else
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core1;
 			} else {
 				hw->phy.sfp_type = ixgbe_sfp_type_unknown;
 			}
@@ -1489,6 +1496,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (comp_codes_10g == 0 &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1508,6 +1517,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (!(enforce_sfp & IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1835,11 +1846,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct ixgbe_hw *hw,
 	 */
 	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
+	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
 		sfp_type = ixgbe_sfp_type_srlr_core0;
 	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
+		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
 		sfp_type = ixgbe_sfp_type_srlr_core1;
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h b/drivers/net/ixgbe/base/ixgbe_phy.h
index 132fa542b..f1605f2cc 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.h
+++ b/drivers/net/ixgbe/base/ixgbe_phy.h
@@ -41,6 +41,7 @@
 #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
 #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
 #define IXGBE_SFF_1GBASET_CAPABLE	0x8
+#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
 #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
 #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
 #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index cee6ba2e0..077b8f01c 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3724,6 +3724,8 @@ enum ixgbe_sfp_type {
 	ixgbe_sfp_type_1g_sx_core1 = 12,
 	ixgbe_sfp_type_1g_lx_core0 = 13,
 	ixgbe_sfp_type_1g_lx_core1 = 14,
+	ixgbe_sfp_type_1g_lha_core0 = 15,
+	ixgbe_sfp_type_1g_lha_core1 = 16,
 	ixgbe_sfp_type_not_present = 0xFFFE,
 	ixgbe_sfp_type_unknown = 0xFFFF
 };
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 83b394861..d2b1000e9 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1547,6 +1547,8 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
 	case ixgbe_sfp_type_1g_sx_core1:
 	case ixgbe_sfp_type_1g_lx_core0:
 	case ixgbe_sfp_type_1g_lx_core1:
+	case ixgbe_sfp_type_1g_lha_core0:
+	case ixgbe_sfp_type_1g_lha_core1:
 		*linear = false;
 		break;
 	case ixgbe_sfp_type_unknown:
@@ -1887,6 +1889,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 		/* Check if 1G SFP module. */
 		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
+		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1
 		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1) {
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
-- 
2.19.1

Re: [dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Thu, 2018-11-01 at 14:04 +0000, Luca Boccassi wrote:
> ixgbe_identify_phy_x550em() was missing the code to handle
> unidentified
> PHY that has been there in 82599 so it was not able to complete
> initialization of ixgbe sequence if no sfp plugged in.
> Port it over to return an appropriate type and complete init sequence
> properly.
> 
> Fixes: d2e72774e58c ("ixgbe/base: support X550")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: refresh to remove merge conflict with master
> 
>  drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> b/drivers/net/ixgbe/base/ixgbe_x550.c
> index f7b98af52..83b394861 100644

Sorry, finger slipped and missed the -v2 from the command so it's not
in the subject - it's the same, just rebased to fix a conflict in the
first patch.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Zhang, Qi Z]

> -----Original Message-----
> From: Luca Boccassi [mailto:bluca@debian.org]
> Sent: Thursday, November 1, 2018 9:04 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Luca
> Boccassi <bluca@debian.org>; stable@dpdk.org
> Subject: [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY
> 
> ixgbe_identify_phy_x550em() was missing the code to handle unidentified
> PHY that has been there in 82599 so it was not able to complete initialization
> of ixgbe sequence if no sfp plugged in.
> Port it over to return an appropriate type and complete init sequence
> properly.
> 
> Fixes: d2e72774e58c ("ixgbe/base: support X550")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: refresh to remove merge conflict with master
> 
>  drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> b/drivers/net/ixgbe/base/ixgbe_x550.c
> index f7b98af52..83b394861 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> @@ -315,13 +315,21 @@ STATIC void ixgbe_setup_mux_ctl(struct ixgbe_hw
> *hw)
>   */
>  STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)  {
> +	s32 status;
> +
>  	hw->mac.ops.set_lan_id(hw);
> 
>  	ixgbe_read_mng_if_sel_x550em(hw);
> 
>  	switch (hw->device_id) {
>  	case IXGBE_DEV_ID_X550EM_A_SFP:
> -		return ixgbe_identify_sfp_module_X550em(hw);
> +		status = ixgbe_identify_sfp_module_X550em(hw);
> +		/* Set PHY type none if no PHY detected */
> +		if (hw->phy.type == ixgbe_phy_unknown) {
> +			hw->phy.type = ixgbe_phy_none;
> +			return IXGBE_SUCCESS;
> +		}

Why this can't be handled at caller, why we replace phy_unknown by phy_none only for x550?

> +		return status;
>  	case IXGBE_DEV_ID_X550EM_X_SFP:
>  		/* set up for CS4227 usage */
>  		ixgbe_setup_mux_ctl(hw);
> @@ -329,8 +337,13 @@ STATIC s32 ixgbe_identify_phy_x550em(struct
> ixgbe_hw *hw)
>  		/* Fallthrough */
> 
>  	case IXGBE_DEV_ID_X550EM_A_SFP_N:
> -		return ixgbe_identify_sfp_module_X550em(hw);
> -		break;
> +		status = ixgbe_identify_sfp_module_X550em(hw);
> +		/* Set PHY type none if no PHY detected */
> +		if (hw->phy.type == ixgbe_phy_unknown) {
> +			hw->phy.type = ixgbe_phy_none;
> +			return IXGBE_SUCCESS;
> +		}
> +		return status;
>  	case IXGBE_DEV_ID_X550EM_X_KX4:
>  		hw->phy.type = ixgbe_phy_x550em_kx4;
>  		break;
> --
> 2.19.1

[dpdk-dev] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

ixgbe_identify_sfp_module_X550em() was missing the code to handle
unidentified PHY that has been there in 82599 so it was not able to
complete initialization of ixgbe sequence if no sfp plugged in.
Port it over to return an appropriate type and complete init sequence
properly.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
v2: refresh to remove merge conflict with master
v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
    code duplication, improve comment

 drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index f7b98af52..a88d5c86a 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1561,6 +1561,12 @@ s32 ixgbe_identify_sfp_module_X550em(struct ixgbe_hw *hw)
 
 	status = ixgbe_identify_module_generic(hw);
 
+	/* Set PHY type none if no PHY detected to allow init without SFP */
+	if (hw->phy.type == ixgbe_phy_unknown) {
+		hw->phy.type = ixgbe_phy_none;
+		return IXGBE_SUCCESS;
+	}
+
 	if (status != IXGBE_SUCCESS)
 		return status;
 
-- 
2.19.1

[dpdk-dev] [PATCH v3 2/2] net/ixgbe: add LHA ID to x550 code

[Luca Boccassi]

ixgbe is able to recognize 1G SX and LX id, but it is missing the LHA.
Add it, so that it can handle LHA sfp plugin.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
 drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
 drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
 drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
 drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
 5 files changed, 22 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c b/drivers/net/ixgbe/base/ixgbe_82599.c
index 7de753fd3..96bdde62c 100644
--- a/drivers/net/ixgbe/base/ixgbe_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_82599.c
@@ -392,6 +392,8 @@ s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
 	/* Check if 1G SFP module. */
 	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 6cdd8fbab..dd118f917 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1402,6 +1402,13 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 				else
 					hw->phy.sfp_type =
 						ixgbe_sfp_type_1g_lx_core1;
+			} else if (comp_codes_1g & IXGBE_SFF_1GBASELHA_CAPABLE) {
+				if (hw->bus.lan_id == 0)
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core0;
+				else
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core1;
 			} else {
 				hw->phy.sfp_type = ixgbe_sfp_type_unknown;
 			}
@@ -1489,6 +1496,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (comp_codes_10g == 0 &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1508,6 +1517,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (!(enforce_sfp & IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1835,11 +1846,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct ixgbe_hw *hw,
 	 */
 	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
+	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
 		sfp_type = ixgbe_sfp_type_srlr_core0;
 	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
+		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
 		sfp_type = ixgbe_sfp_type_srlr_core1;
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h b/drivers/net/ixgbe/base/ixgbe_phy.h
index 132fa542b..f1605f2cc 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.h
+++ b/drivers/net/ixgbe/base/ixgbe_phy.h
@@ -41,6 +41,7 @@
 #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
 #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
 #define IXGBE_SFF_1GBASET_CAPABLE	0x8
+#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
 #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
 #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
 #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index cee6ba2e0..077b8f01c 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3724,6 +3724,8 @@ enum ixgbe_sfp_type {
 	ixgbe_sfp_type_1g_sx_core1 = 12,
 	ixgbe_sfp_type_1g_lx_core0 = 13,
 	ixgbe_sfp_type_1g_lx_core1 = 14,
+	ixgbe_sfp_type_1g_lha_core0 = 15,
+	ixgbe_sfp_type_1g_lha_core1 = 16,
 	ixgbe_sfp_type_not_present = 0xFFFE,
 	ixgbe_sfp_type_unknown = 0xFFFF
 };
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index a88d5c86a..1114d3faf 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1534,6 +1534,8 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
 	case ixgbe_sfp_type_1g_sx_core1:
 	case ixgbe_sfp_type_1g_lx_core0:
 	case ixgbe_sfp_type_1g_lx_core1:
+	case ixgbe_sfp_type_1g_lha_core0:
+	case ixgbe_sfp_type_1g_lha_core1:
 		*linear = false;
 		break;
 	case ixgbe_sfp_type_unknown:
@@ -1880,6 +1882,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 		/* Check if 1G SFP module. */
 		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
+		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1
 		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1) {
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
-- 
2.19.1

Re: [dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Fri, 2018-11-02 at 14:11 +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Luca Boccassi [mailto:bluca@debian.org]
> > Sent: Thursday, November 1, 2018 9:04 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Luca
> > Boccassi <bluca@debian.org>; stable@dpdk.org
> > Subject: [PATCH 1/2] net/ixgbe: fix x550 code to handle
> > unidentified PHY
> > 
> > ixgbe_identify_phy_x550em() was missing the code to handle
> > unidentified
> > PHY that has been there in 82599 so it was not able to complete
> > initialization
> > of ixgbe sequence if no sfp plugged in.
> > Port it over to return an appropriate type and complete init
> > sequence
> > properly.
> > 
> > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > ---
> > v2: refresh to remove merge conflict with master
> > 
> >  drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > index f7b98af52..83b394861 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > @@ -315,13 +315,21 @@ STATIC void ixgbe_setup_mux_ctl(struct
> > ixgbe_hw
> > *hw)
> >   */
> >  STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)  {
> > +	s32 status;
> > +
> >  	hw->mac.ops.set_lan_id(hw);
> > 
> >  	ixgbe_read_mng_if_sel_x550em(hw);
> > 
> >  	switch (hw->device_id) {
> >  	case IXGBE_DEV_ID_X550EM_A_SFP:
> > -		return ixgbe_identify_sfp_module_X550em(hw);
> > +		status = ixgbe_identify_sfp_module_X550em(hw);
> > +		/* Set PHY type none if no PHY detected */
> > +		if (hw->phy.type == ixgbe_phy_unknown) {
> > +			hw->phy.type = ixgbe_phy_none;
> > +			return IXGBE_SUCCESS;
> > +		}
> 
> Why this can't be handled at caller, why we replace phy_unknown by
> phy_none only for x550?

Hi, thanks for the review.

I've moved the code into the caller in v3.

It's done for x550 simply because that's the hardware that we have and
that this was tested with, I didn't want to take extra risks. It's also
the hardware that our customer reported the issue with.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Zhang, Qi Z]

> -----Original Message-----
> From: Luca Boccassi [mailto:bluca@debian.org]
> Sent: Friday, November 2, 2018 10:21 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY
> 
> On Fri, 2018-11-02 at 14:11 +0000, Zhang, Qi Z wrote:
> > > -----Original Message-----
> > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > Sent: Thursday, November 1, 2018 9:04 AM
> > > To: dev@dpdk.org
> > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > > Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> > > Subject: [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified
> > > PHY
> > >
> > > ixgbe_identify_phy_x550em() was missing the code to handle
> > > unidentified PHY that has been there in 82599 so it was not able to
> > > complete initialization of ixgbe sequence if no sfp plugged in.
> > > Port it over to return an appropriate type and complete init
> > > sequence properly.
> > >
> > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > ---
> > > v2: refresh to remove merge conflict with master
> > >
> > >  drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > index f7b98af52..83b394861 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > @@ -315,13 +315,21 @@ STATIC void ixgbe_setup_mux_ctl(struct
> > > ixgbe_hw
> > > *hw)
> > >   */
> > >  STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)  {
> > > +	s32 status;
> > > +
> > >  	hw->mac.ops.set_lan_id(hw);
> > >
> > >  	ixgbe_read_mng_if_sel_x550em(hw);
> > >
> > >  	switch (hw->device_id) {
> > >  	case IXGBE_DEV_ID_X550EM_A_SFP:
> > > -		return ixgbe_identify_sfp_module_X550em(hw);
> > > +		status = ixgbe_identify_sfp_module_X550em(hw);
> > > +		/* Set PHY type none if no PHY detected */
> > > +		if (hw->phy.type == ixgbe_phy_unknown) {
> > > +			hw->phy.type = ixgbe_phy_none;
> > > +			return IXGBE_SUCCESS;
> > > +		}
> >
> > Why this can't be handled at caller, why we replace phy_unknown by
> > phy_none only for x550?
> 
> Hi, thanks for the review.
> 
> I've moved the code into the caller in v3.

Sorry, this is not what I suggested. 
I'm not sure for X550EM_A_SFP, it is the case that the device does not have PHY so we should correct it (by replace it with no_phy) 
or it is the case that we can't identify PHY so we just replace it by no_phy to bypass the check for workaround?
If the second case, is it possible not to do replacement and handle it at upper layer (caller or caller's caller ... ) and keep the information more accurate?

> 
> It's done for x550 simply because that's the hardware that we have and that
> this was tested with, I didn't want to take extra risks. It's also the hardware
> that our customer reported the issue with.


> 
> --
> Kind regards,
> Luca Boccassi

Re: [dpdk-dev] [dpdk-stable] [PATCH 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Fri, 2018-11-02 at 16:49 +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Luca Boccassi [mailto:bluca@debian.org]
> > Sent: Friday, November 2, 2018 10:21 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > Subject: Re: [PATCH 1/2] net/ixgbe: fix x550 code to handle
> > unidentified PHY
> > 
> > On Fri, 2018-11-02 at 14:11 +0000, Zhang, Qi Z wrote:
> > > > -----Original Message-----
> > > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > > Sent: Thursday, November 1, 2018 9:04 AM
> > > > To: dev@dpdk.org
> > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.c
> > > > om>;
> > > > Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> > > > Subject: [PATCH 1/2] net/ixgbe: fix x550 code to handle
> > > > unidentified
> > > > PHY
> > > > 
> > > > ixgbe_identify_phy_x550em() was missing the code to handle
> > > > unidentified PHY that has been there in 82599 so it was not
> > > > able to
> > > > complete initialization of ixgbe sequence if no sfp plugged in.
> > > > Port it over to return an appropriate type and complete init
> > > > sequence properly.
> > > > 
> > > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > > Cc: stable@dpdk.org
> > > > 
> > > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > > ---
> > > > v2: refresh to remove merge conflict with master
> > > > 
> > > >  drivers/net/ixgbe/base/ixgbe_x550.c | 19 ++++++++++++++++---
> > > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > index f7b98af52..83b394861 100644
> > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > @@ -315,13 +315,21 @@ STATIC void ixgbe_setup_mux_ctl(struct
> > > > ixgbe_hw
> > > > *hw)
> > > >   */
> > > >  STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)  {
> > > > +	s32 status;
> > > > +
> > > >  	hw->mac.ops.set_lan_id(hw);
> > > > 
> > > >  	ixgbe_read_mng_if_sel_x550em(hw);
> > > > 
> > > >  	switch (hw->device_id) {
> > > >  	case IXGBE_DEV_ID_X550EM_A_SFP:
> > > > -		return ixgbe_identify_sfp_module_X550em(hw);
> > > > +		status = ixgbe_identify_sfp_module_X550em(hw);
> > > > +		/* Set PHY type none if no PHY detected */
> > > > +		if (hw->phy.type == ixgbe_phy_unknown) {
> > > > +			hw->phy.type = ixgbe_phy_none;
> > > > +			return IXGBE_SUCCESS;
> > > > +		}
> > > 
> > > Why this can't be handled at caller, why we replace phy_unknown
> > > by
> > > phy_none only for x550?
> > 
> > Hi, thanks for the review.
> > 
> > I've moved the code into the caller in v3.
> 
> Sorry, this is not what I suggested. 
> I'm not sure for X550EM_A_SFP, it is the case that the device does
> not have PHY so we should correct it (by replace it with no_phy) 
> or it is the case that we can't identify PHY so we just replace it by
> no_phy to bypass the check for workaround?
> If the second case, is it possible not to do replacement and handle
> it at upper layer (caller or caller's caller ... ) and keep the
> information more accurate?

It is the second case - I've amended the comment in v3, the issue is
that the driver will fail to initialise if there is no SFP present at
boot (not very hotplug-friendly!).
The issue with calling it one layer above is that this function is
saved in a table, and there are at least 3-4 places where it is called
from. So it would have to be repeated many times, and also I'm not 100%
sure of all the places where to do this, as the function table is
updated several times across the base driver.

So, when you say caller or caller's caller, do you have identified
precisely where? I'm happy to change it if you can point me exactly to
the right places.

Thanks!

> > 
> > It's done for x550 simply because that's the hardware that we have
> > and that
> > this was tested with, I didn't want to take extra risks. It's also
> > the hardware
> > that our customer reported the issue with.
> 
> 
> > 
> > --
> > Kind regards,
> > Luca Boccassi

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Zhang, Qi Z]

> -----Original Message-----
> From: Luca Boccassi [mailto:bluca@debian.org]
> Sent: Friday, November 2, 2018 8:19 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY
> 
> ixgbe_identify_sfp_module_X550em() was missing the code to handle
> unidentified PHY that has been there in 82599 so it was not able to complete
> initialization of ixgbe sequence if no sfp plugged in.
> Port it over to return an appropriate type and complete init sequence
> properly.
> 
> Fixes: d2e72774e58c ("ixgbe/base: support X550")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: refresh to remove merge conflict with master
> v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
>     code duplication, improve comment
> 
>  drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> b/drivers/net/ixgbe/base/ixgbe_x550.c
> index f7b98af52..a88d5c86a 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> @@ -1561,6 +1561,12 @@ s32 ixgbe_identify_sfp_module_X550em(struct
> ixgbe_hw *hw)
> 
>  	status = ixgbe_identify_module_generic(hw);
> 
> +	/* Set PHY type none if no PHY detected to allow init without SFP */
> +	if (hw->phy.type == ixgbe_phy_unknown) {
> +		hw->phy.type = ixgbe_phy_none;

Set PHY type to none for a device that does have PHY looks weird.  
does ixgeb_phy_generic works here?

Where is failure you met with ixgbe_phy_unknown?

> +		return IXGBE_SUCCESS;
> +	}
> +
>  	if (status != IXGBE_SUCCESS)
>  		return status;
> 
> --
> 2.19.1

Re: [dpdk-dev] [dpdk-stable] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Mon, 2018-11-05 at 17:41 +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Luca Boccassi [mailto:bluca@debian.org]
> > Sent: Friday, November 2, 2018 8:19 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> > Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > unidentified PHY
> > 
> > ixgbe_identify_sfp_module_X550em() was missing the code to handle
> > unidentified PHY that has been there in 82599 so it was not able to
> > complete
> > initialization of ixgbe sequence if no sfp plugged in.
> > Port it over to return an appropriate type and complete init
> > sequence
> > properly.
> > 
> > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > ---
> > v2: refresh to remove merge conflict with master
> > v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
> >     code duplication, improve comment
> > 
> >  drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > index f7b98af52..a88d5c86a 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > @@ -1561,6 +1561,12 @@ s32 ixgbe_identify_sfp_module_X550em(struct
> > ixgbe_hw *hw)
> > 
> >  	status = ixgbe_identify_module_generic(hw);
> > 
> > +	/* Set PHY type none if no PHY detected to allow init
> > without SFP */
> > +	if (hw->phy.type == ixgbe_phy_unknown) {
> > +		hw->phy.type = ixgbe_phy_none;
> 
> Set PHY type to none for a device that does have PHY looks weird.  
> does ixgeb_phy_generic works here?
> 
> Where is failure you met with ixgbe_phy_unknown?

Yes it is a bit weird, but it works :-)

The issue is that the PMD fails to initialise when there is no SFP
plugged, and then it will always stay in that failed state when an SFP
is later plugged in, and it won't work until the machine is rebooted
with the SFP plugged in.

The logs (with an older DPDK version):

PMD: eth_ixgbe_dev_init(): Hardware Initialization Failure: -20
EAL: Requested device 0000:04:00.0 cannot be used

The kernel driver didn't like it either:

[    7.579782] ixgbe 0000:04:00.0: Multiqueue Enabled: Rx Queue count = 8, Tx Queue count = 8
[    7.649766] ixgbe 0000:04:00.0: MAC: 5, PHY: 0, PBA No: 020A00-000
[    7.649774] ixgbe 0000:04:00.0: 00:25:90:5e:05:20
[    8.763790] ixgbe 0000:04:00.0 0000:04:00.0 (uninitialized): CS4227 reset did not complete
[    8.772051] ixgbe 0000:04:00.0 0000:04:00.0 (uninitialized): CS4227 reset failed: -3
[    9.059374] ixgbe 0000:04:00.0: Intel(R) 10 Gigabit Network Connection

But with the kernel driver, if an SFP is plugged in later then the
interface works correctly.

With this series, an SFP can be plugged in after booting and
initialising the DPDK application.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Chas Williams]

On 11/05/2018 12:41 PM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Luca Boccassi [mailto:bluca@debian.org]
>> Sent: Friday, November 2, 2018 8:19 AM
>> To: dev@dpdk.org
>> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
>> Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY
>>
>> ixgbe_identify_sfp_module_X550em() was missing the code to handle
>> unidentified PHY that has been there in 82599 so it was not able to complete
>> initialization of ixgbe sequence if no sfp plugged in.
>> Port it over to return an appropriate type and complete init sequence
>> properly.
>>
>> Fixes: d2e72774e58c ("ixgbe/base: support X550")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Luca Boccassi <bluca@debian.org>
>> ---
>> v2: refresh to remove merge conflict with master
>> v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
>>      code duplication, improve comment
>>
>>   drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
>> b/drivers/net/ixgbe/base/ixgbe_x550.c
>> index f7b98af52..a88d5c86a 100644
>> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
>> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
>> @@ -1561,6 +1561,12 @@ s32 ixgbe_identify_sfp_module_X550em(struct
>> ixgbe_hw *hw)
>>
>>   	status = ixgbe_identify_module_generic(hw);
>>
>> +	/* Set PHY type none if no PHY detected to allow init without SFP */
>> +	if (hw->phy.type == ixgbe_phy_unknown) {
>> +		hw->phy.type = ixgbe_phy_none;
> 
> Set PHY type to none for a device that does have PHY looks weird.
> does ixgeb_phy_generic works here?

Yes, it does seem strange but that's what ixgbe_identify_sfp_module_generic
seems to do:

	err_read_i2c_eeprom:
		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
		if (hw->phy.type != ixgbe_phy_nl) {
			hw->phy.id = 0;
			hw->phy.type = ixgbe_phy_unknown;
		}

The QSFP version a little more forceful:

	err_read_i2c_eeprom:
		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
		hw->phy.id = 0;
		hw->phy.type = ixgbe_phy_unknown;

If we go forward without setting the phy_type to none, we will eventually
run into issues calling other phy routines.

So should a lack of SFP, reset the PHY type? It's hazy because the 
difference
between PHY and SFP isn't that clear to me here.

> Where is failure you met with ixgbe_phy_unknown?
> 
>> +		return IXGBE_SUCCESS;
>> +	}
>> +
>>   	if (status != IXGBE_SUCCESS)
>>   		return status;
>>
>> --
>> 2.19.1
> 

Re: [dpdk-dev] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Zhang, Qi Z]

> -----Original Message-----
> From: Chas Williams [mailto:3chas3@gmail.com]
> Sent: Monday, November 5, 2018 11:19 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Luca Boccassi <bluca@debian.org>;
> dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified
> PHY
> 
> 
> 
> On 11/05/2018 12:41 PM, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: Luca Boccassi [mailto:bluca@debian.org]
> >> Sent: Friday, November 2, 2018 8:19 AM
> >> To: dev@dpdk.org
> >> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> >> <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >> 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> >> Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> >> unidentified PHY
> >>
> >> ixgbe_identify_sfp_module_X550em() was missing the code to handle
> >> unidentified PHY that has been there in 82599 so it was not able to
> >> complete initialization of ixgbe sequence if no sfp plugged in.
> >> Port it over to return an appropriate type and complete init sequence
> >> properly.
> >>
> >> Fixes: d2e72774e58c ("ixgbe/base: support X550")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Luca Boccassi <bluca@debian.org>
> >> ---
> >> v2: refresh to remove merge conflict with master
> >> v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
> >>      code duplication, improve comment
> >>
> >>   drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> >> b/drivers/net/ixgbe/base/ixgbe_x550.c
> >> index f7b98af52..a88d5c86a 100644
> >> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> >> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> >> @@ -1561,6 +1561,12 @@ s32
> ixgbe_identify_sfp_module_X550em(struct
> >> ixgbe_hw *hw)
> >>
> >>   	status = ixgbe_identify_module_generic(hw);
> >>
> >> +	/* Set PHY type none if no PHY detected to allow init without SFP */
> >> +	if (hw->phy.type == ixgbe_phy_unknown) {
> >> +		hw->phy.type = ixgbe_phy_none;
> >
> > Set PHY type to none for a device that does have PHY looks weird.
> > does ixgeb_phy_generic works here?
> 
> Yes, it does seem strange but that's what ixgbe_identify_sfp_module_generic
> seems to do:
> 
> 	err_read_i2c_eeprom:
> 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> 		if (hw->phy.type != ixgbe_phy_nl) {
> 			hw->phy.id = 0;
> 			hw->phy.type = ixgbe_phy_unknown;
> 		}
> 
> The QSFP version a little more forceful:
> 
> 	err_read_i2c_eeprom:
> 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> 		hw->phy.id = 0;
> 		hw->phy.type = ixgbe_phy_unknown;
> 
> If we go forward without setting the phy_type to none, we will eventually run
> into issues calling other phy routines.
> 
> So should a lack of SFP, reset the PHY type? It's hazy because the difference
> between PHY and SFP isn't that clear to me here.

I'm not sure that's the same case:).
Just feel that it's better to handle ixgbe_phy_unknown directly for some device id as a special case than just replace it to ixgbe_phy_none to cheat the check path, since that rely on we never change the way to handle ixgbe_phy_none.

So still have the question?
What is the failure if you go with ixgbe_phy_unknown?
Is that possible to work around this like
if (phy_type == ixgbe_phy_unknown && dev_id == xxxx)
	...

> 
> > Where is failure you met with ixgbe_phy_unknown?
> >
> >> +		return IXGBE_SUCCESS;
> >> +	}
> >> +
> >>   	if (status != IXGBE_SUCCESS)
> >>   		return status;
> >>
> >> --
> >> 2.19.1
> >

Re: [dpdk-dev] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Tue, 2018-11-06 at 23:31 +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Chas Williams [mailto:3chas3@gmail.com]
> > Sent: Monday, November 5, 2018 11:19 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Luca Boccassi <bluca@debian
> > .org>;
> > dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > unidentified
> > PHY
> > 
> > 
> > 
> > On 11/05/2018 12:41 PM, Zhang, Qi Z wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > > Sent: Friday, November 2, 2018 8:19 AM
> > > > To: dev@dpdk.org
> > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.c
> > > > om>;
> > > > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk
> > > > .org
> > > > Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > > > unidentified PHY
> > > > 
> > > > ixgbe_identify_sfp_module_X550em() was missing the code to
> > > > handle
> > > > unidentified PHY that has been there in 82599 so it was not
> > > > able to
> > > > complete initialization of ixgbe sequence if no sfp plugged in.
> > > > Port it over to return an appropriate type and complete init
> > > > sequence
> > > > properly.
> > > > 
> > > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > > Cc: stable@dpdk.org
> > > > 
> > > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > > ---
> > > > v2: refresh to remove merge conflict with master
> > > > v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
> > > >      code duplication, improve comment
> > > > 
> > > >   drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
> > > >   1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > index f7b98af52..a88d5c86a 100644
> > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > @@ -1561,6 +1561,12 @@ s32
> > 
> > ixgbe_identify_sfp_module_X550em(struct
> > > > ixgbe_hw *hw)
> > > > 
> > > >   	status = ixgbe_identify_module_generic(hw);
> > > > 
> > > > +	/* Set PHY type none if no PHY detected to allow init
> > > > without SFP */
> > > > +	if (hw->phy.type == ixgbe_phy_unknown) {
> > > > +		hw->phy.type = ixgbe_phy_none;
> > > 
> > > Set PHY type to none for a device that does have PHY looks weird.
> > > does ixgeb_phy_generic works here?
> > 
> > Yes, it does seem strange but that's what
> > ixgbe_identify_sfp_module_generic
> > seems to do:
> > 
> > 	err_read_i2c_eeprom:
> > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > 		if (hw->phy.type != ixgbe_phy_nl) {
> > 			hw->phy.id = 0;
> > 			hw->phy.type = ixgbe_phy_unknown;
> > 		}
> > 
> > The QSFP version a little more forceful:
> > 
> > 	err_read_i2c_eeprom:
> > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > 		hw->phy.id = 0;
> > 		hw->phy.type = ixgbe_phy_unknown;
> > 
> > If we go forward without setting the phy_type to none, we will
> > eventually run
> > into issues calling other phy routines.
> > 
> > So should a lack of SFP, reset the PHY type? It's hazy because the
> > difference
> > between PHY and SFP isn't that clear to me here.
> 
> I'm not sure that's the same case:).
> Just feel that it's better to handle ixgbe_phy_unknown directly for
> some device id as a special case than just replace it to
> ixgbe_phy_none to cheat the check path, since that rely on we never
> change the way to handle ixgbe_phy_none.
> 
> So still have the question?
> What is the failure if you go with ixgbe_phy_unknown?
> Is that possible to work around this like
> if (phy_type == ixgbe_phy_unknown && dev_id == xxxx)
> 	...

Hi,

Thanks for having a look at this again. If you could please see the
other answer, from myself, I've quoted the exact error we see and the
issue it causes.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Zhang, Qi Z]

> -----Original Message-----
> From: Luca Boccassi [mailto:bluca@debian.org]
> Sent: Wednesday, November 7, 2018 5:55 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Chas Williams <3chas3@gmail.com>;
> dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified
> PHY
> 
> On Tue, 2018-11-06 at 23:31 +0000, Zhang, Qi Z wrote:
> > > -----Original Message-----
> > > From: Chas Williams [mailto:3chas3@gmail.com]
> > > Sent: Monday, November 5, 2018 11:19 AM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Luca Boccassi <bluca@debian
> > > .org>; dev@dpdk.org
> > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > > Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > > unidentified PHY
> > >
> > >
> > >
> > > On 11/05/2018 12:41 PM, Zhang, Qi Z wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > > > Sent: Friday, November 2, 2018 8:19 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.c
> > > > > om>;
> > > > > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk
> > > > > .org
> > > > > Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > > > > unidentified PHY
> > > > >
> > > > > ixgbe_identify_sfp_module_X550em() was missing the code to
> > > > > handle
> > > > > unidentified PHY that has been there in 82599 so it was not
> > > > > able to
> > > > > complete initialization of ixgbe sequence if no sfp plugged in.
> > > > > Port it over to return an appropriate type and complete init
> > > > > sequence
> > > > > properly.
> > > > >
> > > > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > > > ---
> > > > > v2: refresh to remove merge conflict with master
> > > > > v3: coalesce fix into ixgbe_identify_sfp_module_X550em to avoid
> > > > >      code duplication, improve comment
> > > > >
> > > > >   drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
> > > > >   1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > index f7b98af52..a88d5c86a 100644
> > > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > @@ -1561,6 +1561,12 @@ s32
> > >
> > > ixgbe_identify_sfp_module_X550em(struct
> > > > > ixgbe_hw *hw)
> > > > >
> > > > >   	status = ixgbe_identify_module_generic(hw);
> > > > >
> > > > > +	/* Set PHY type none if no PHY detected to allow init
> > > > > without SFP */
> > > > > +	if (hw->phy.type == ixgbe_phy_unknown) {
> > > > > +		hw->phy.type = ixgbe_phy_none;
> > > >
> > > > Set PHY type to none for a device that does have PHY looks weird.
> > > > does ixgeb_phy_generic works here?
> > >
> > > Yes, it does seem strange but that's what
> > > ixgbe_identify_sfp_module_generic
> > > seems to do:
> > >
> > > 	err_read_i2c_eeprom:
> > > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > 		if (hw->phy.type != ixgbe_phy_nl) {
> > > 			hw->phy.id = 0;
> > > 			hw->phy.type = ixgbe_phy_unknown;
> > > 		}
> > >
> > > The QSFP version a little more forceful:
> > >
> > > 	err_read_i2c_eeprom:
> > > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > 		hw->phy.id = 0;
> > > 		hw->phy.type = ixgbe_phy_unknown;
> > >
> > > If we go forward without setting the phy_type to none, we will
> > > eventually run
> > > into issues calling other phy routines.
> > >
> > > So should a lack of SFP, reset the PHY type? It's hazy because the
> > > difference
> > > between PHY and SFP isn't that clear to me here.
> >
> > I'm not sure that's the same case:).
> > Just feel that it's better to handle ixgbe_phy_unknown directly for
> > some device id as a special case than just replace it to
> > ixgbe_phy_none to cheat the check path, since that rely on we never
> > change the way to handle ixgbe_phy_none.
> >
> > So still have the question?
> > What is the failure if you go with ixgbe_phy_unknown?
> > Is that possible to work around this like
> > if (phy_type == ixgbe_phy_unknown && dev_id == xxxx)
> > 	...
> 
> Hi,
> 
> Thanks for having a look at this again. If you could please see the
> other answer, from myself, I've quoted the exact error we see and the
> issue it causes.

Yes, I see it failed at eth_ixgbe_dev_init, it will be better if you can provide more detail for the call stack, so we can figure out if we can work around this by handle ixgbe_phy_unknown directly with some special case.

> 
> --
> Kind regards,
> Luca Boccassi

Re: [dpdk-dev] [dpdk-stable] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Wed, 2018-11-07 at 18:27 +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Luca Boccassi [mailto:bluca@debian.org]
> > Sent: Wednesday, November 7, 2018 5:55 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Chas Williams <3chas3@gmail
> > .com>;
> > dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > unidentified
> > PHY
> > 
> > On Tue, 2018-11-06 at 23:31 +0000, Zhang, Qi Z wrote:
> > > > -----Original Message-----
> > > > From: Chas Williams [mailto:3chas3@gmail.com]
> > > > Sent: Monday, November 5, 2018 11:19 AM
> > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Luca Boccassi <bluca@de
> > > > bian
> > > > .org>; dev@dpdk.org
> > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > > > Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > > > unidentified PHY
> > > > 
> > > > 
> > > > 
> > > > On 11/05/2018 12:41 PM, Zhang, Qi Z wrote:
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > > > > Sent: Friday, November 2, 2018 8:19 AM
> > > > > > To: dev@dpdk.org
> > > > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@int
> > > > > > el.c
> > > > > > om>;
> > > > > > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@
> > > > > > dpdk
> > > > > > .org
> > > > > > Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > > > > > unidentified PHY
> > > > > > 
> > > > > > ixgbe_identify_sfp_module_X550em() was missing the code to
> > > > > > handle
> > > > > > unidentified PHY that has been there in 82599 so it was not
> > > > > > able to
> > > > > > complete initialization of ixgbe sequence if no sfp plugged
> > > > > > in.
> > > > > > Port it over to return an appropriate type and complete
> > > > > > init
> > > > > > sequence
> > > > > > properly.
> > > > > > 
> > > > > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > > > > Cc: stable@dpdk.org
> > > > > > 
> > > > > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > > > > ---
> > > > > > v2: refresh to remove merge conflict with master
> > > > > > v3: coalesce fix into ixgbe_identify_sfp_module_X550em to
> > > > > > avoid
> > > > > >      code duplication, improve comment
> > > > > > 
> > > > > >   drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
> > > > > >   1 file changed, 6 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > index f7b98af52..a88d5c86a 100644
> > > > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > @@ -1561,6 +1561,12 @@ s32
> > > > 
> > > > ixgbe_identify_sfp_module_X550em(struct
> > > > > > ixgbe_hw *hw)
> > > > > > 
> > > > > >   	status = ixgbe_identify_module_generic(hw);
> > > > > > 
> > > > > > +	/* Set PHY type none if no PHY detected to allow
> > > > > > init
> > > > > > without SFP */
> > > > > > +	if (hw->phy.type == ixgbe_phy_unknown) {
> > > > > > +		hw->phy.type = ixgbe_phy_none;
> > > > > 
> > > > > Set PHY type to none for a device that does have PHY looks
> > > > > weird.
> > > > > does ixgeb_phy_generic works here?
> > > > 
> > > > Yes, it does seem strange but that's what
> > > > ixgbe_identify_sfp_module_generic
> > > > seems to do:
> > > > 
> > > > 	err_read_i2c_eeprom:
> > > > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > > 		if (hw->phy.type != ixgbe_phy_nl) {
> > > > 			hw->phy.id = 0;
> > > > 			hw->phy.type = ixgbe_phy_unknown;
> > > > 		}
> > > > 
> > > > The QSFP version a little more forceful:
> > > > 
> > > > 	err_read_i2c_eeprom:
> > > > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > > 		hw->phy.id = 0;
> > > > 		hw->phy.type = ixgbe_phy_unknown;
> > > > 
> > > > If we go forward without setting the phy_type to none, we will
> > > > eventually run
> > > > into issues calling other phy routines.
> > > > 
> > > > So should a lack of SFP, reset the PHY type? It's hazy because
> > > > the
> > > > difference
> > > > between PHY and SFP isn't that clear to me here.
> > > 
> > > I'm not sure that's the same case:).
> > > Just feel that it's better to handle ixgbe_phy_unknown directly
> > > for
> > > some device id as a special case than just replace it to
> > > ixgbe_phy_none to cheat the check path, since that rely on we
> > > never
> > > change the way to handle ixgbe_phy_none.
> > > 
> > > So still have the question?
> > > What is the failure if you go with ixgbe_phy_unknown?
> > > Is that possible to work around this like
> > > if (phy_type == ixgbe_phy_unknown && dev_id == xxxx)
> > > 	...
> > 
> > Hi,
> > 
> > Thanks for having a look at this again. If you could please see the
> > other answer, from myself, I've quoted the exact error we see and
> > the
> > issue it causes.
> 
> Yes, I see it failed at eth_ixgbe_dev_init, it will be better if you
> can provide more detail for the call stack, so we can figure out if
> we can work around this by handle ixgbe_phy_unknown directly with
> some special case.

Hi,

The original problem was found and fixed internally a while ago, so
it's taking some time to get hold of the same hardware again. I hope to
get back with more details next week, sorry for the delay.

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH v4] net/ixgbe: add LHA ID to x550 code

[Luca Boccassi]

ixgbe is able to recognize 1G SX and LX id, but it is missing the LHA.
Add it, so that it can handle LHA sfp plugin.

Fixes: d2e72774e58c ("ixgbe/base: support X550")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
v4: dropped first patch, no longer necessary in 18.11

 drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
 drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
 drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
 drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
 drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
 5 files changed, 22 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c b/drivers/net/ixgbe/base/ixgbe_82599.c
index 7de753fd3..96bdde62c 100644
--- a/drivers/net/ixgbe/base/ixgbe_82599.c
+++ b/drivers/net/ixgbe/base/ixgbe_82599.c
@@ -392,6 +392,8 @@ s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
 	/* Check if 1G SFP module. */
 	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 6cdd8fbab..dd118f917 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1402,6 +1402,13 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 				else
 					hw->phy.sfp_type =
 						ixgbe_sfp_type_1g_lx_core1;
+			} else if (comp_codes_1g & IXGBE_SFF_1GBASELHA_CAPABLE) {
+				if (hw->bus.lan_id == 0)
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core0;
+				else
+					hw->phy.sfp_type =
+						ixgbe_sfp_type_1g_lha_core1;
 			} else {
 				hw->phy.sfp_type = ixgbe_sfp_type_unknown;
 			}
@@ -1489,6 +1496,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (comp_codes_10g == 0 &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1508,6 +1517,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 		if (!(enforce_sfp & IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
 		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
 		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
@@ -1835,11 +1846,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct ixgbe_hw *hw,
 	 */
 	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
+	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
 	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
 		sfp_type = ixgbe_sfp_type_srlr_core0;
 	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
+		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
 		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
 		sfp_type = ixgbe_sfp_type_srlr_core1;
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h b/drivers/net/ixgbe/base/ixgbe_phy.h
index 132fa542b..f1605f2cc 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.h
+++ b/drivers/net/ixgbe/base/ixgbe_phy.h
@@ -41,6 +41,7 @@
 #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
 #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
 #define IXGBE_SFF_1GBASET_CAPABLE	0x8
+#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
 #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
 #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
 #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index cee6ba2e0..077b8f01c 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3724,6 +3724,8 @@ enum ixgbe_sfp_type {
 	ixgbe_sfp_type_1g_sx_core1 = 12,
 	ixgbe_sfp_type_1g_lx_core0 = 13,
 	ixgbe_sfp_type_1g_lx_core1 = 14,
+	ixgbe_sfp_type_1g_lha_core0 = 15,
+	ixgbe_sfp_type_1g_lha_core1 = 16,
 	ixgbe_sfp_type_not_present = 0xFFFE,
 	ixgbe_sfp_type_unknown = 0xFFFF
 };
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index f7b98af52..a920a146e 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1534,6 +1534,8 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
 	case ixgbe_sfp_type_1g_sx_core1:
 	case ixgbe_sfp_type_1g_lx_core0:
 	case ixgbe_sfp_type_1g_lx_core1:
+	case ixgbe_sfp_type_1g_lha_core0:
+	case ixgbe_sfp_type_1g_lha_core1:
 		*linear = false;
 		break;
 	case ixgbe_sfp_type_unknown:
@@ -1874,6 +1876,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 		/* Check if 1G SFP module. */
 		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
+		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
+		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1
 		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
 		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1) {
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
-- 
2.19.1

Re: [dpdk-dev] [dpdk-stable] [PATCH v3 1/2] net/ixgbe: fix x550 code to handle unidentified PHY

[Luca Boccassi]

On Fri, 2018-11-09 at 13:18 +0000, Luca Boccassi wrote:
> On Wed, 2018-11-07 at 18:27 +0000, Zhang, Qi Z wrote:
> > > -----Original Message-----
> > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > Sent: Wednesday, November 7, 2018 5:55 AM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Chas Williams <3chas3@gma
> > > il
> > > .com>;
> > > dev@dpdk.org
> > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > > Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to handle
> > > unidentified
> > > PHY
> > > 
> > > On Tue, 2018-11-06 at 23:31 +0000, Zhang, Qi Z wrote:
> > > > > -----Original Message-----
> > > > > From: Chas Williams [mailto:3chas3@gmail.com]
> > > > > Sent: Monday, November 5, 2018 11:19 AM
> > > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Luca Boccassi <bluca@
> > > > > de
> > > > > bian
> > > > > .org>; dev@dpdk.org
> > > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > > <konstantin.ananyev@intel.com>; stable@dpdk.org
> > > > > Subject: Re: [PATCH v3 1/2] net/ixgbe: fix x550 code to
> > > > > handle
> > > > > unidentified PHY
> > > > > 
> > > > > 
> > > > > 
> > > > > On 11/05/2018 12:41 PM, Zhang, Qi Z wrote:
> > > > > > 
> > > > > > 
> > > > > > > -----Original Message-----
> > > > > > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > > > > > Sent: Friday, November 2, 2018 8:19 AM
> > > > > > > To: dev@dpdk.org
> > > > > > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
> > > > > > > Konstantin
> > > > > > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@i
> > > > > > > nt
> > > > > > > el.c
> > > > > > > om>;
> > > > > > > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>;
> > > > > > > stable@
> > > > > > > dpdk
> > > > > > > .org
> > > > > > > Subject: [PATCH v3 1/2] net/ixgbe: fix x550 code to
> > > > > > > handle
> > > > > > > unidentified PHY
> > > > > > > 
> > > > > > > ixgbe_identify_sfp_module_X550em() was missing the code
> > > > > > > to
> > > > > > > handle
> > > > > > > unidentified PHY that has been there in 82599 so it was
> > > > > > > not
> > > > > > > able to
> > > > > > > complete initialization of ixgbe sequence if no sfp
> > > > > > > plugged
> > > > > > > in.
> > > > > > > Port it over to return an appropriate type and complete
> > > > > > > init
> > > > > > > sequence
> > > > > > > properly.
> > > > > > > 
> > > > > > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > > > > > Cc: stable@dpdk.org
> > > > > > > 
> > > > > > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > > > > > ---
> > > > > > > v2: refresh to remove merge conflict with master
> > > > > > > v3: coalesce fix into ixgbe_identify_sfp_module_X550em to
> > > > > > > avoid
> > > > > > >      code duplication, improve comment
> > > > > > > 
> > > > > > >   drivers/net/ixgbe/base/ixgbe_x550.c | 6 ++++++
> > > > > > >   1 file changed, 6 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > > index f7b98af52..a88d5c86a 100644
> > > > > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > > @@ -1561,6 +1561,12 @@ s32
> > > > > 
> > > > > ixgbe_identify_sfp_module_X550em(struct
> > > > > > > ixgbe_hw *hw)
> > > > > > > 
> > > > > > >   	status = ixgbe_identify_module_generic(hw);
> > > > > > > 
> > > > > > > +	/* Set PHY type none if no PHY detected to allow
> > > > > > > init
> > > > > > > without SFP */
> > > > > > > +	if (hw->phy.type == ixgbe_phy_unknown) {
> > > > > > > +		hw->phy.type = ixgbe_phy_none;
> > > > > > 
> > > > > > Set PHY type to none for a device that does have PHY looks
> > > > > > weird.
> > > > > > does ixgeb_phy_generic works here?
> > > > > 
> > > > > Yes, it does seem strange but that's what
> > > > > ixgbe_identify_sfp_module_generic
> > > > > seems to do:
> > > > > 
> > > > > 	err_read_i2c_eeprom:
> > > > > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > > > 		if (hw->phy.type != ixgbe_phy_nl) {
> > > > > 			hw->phy.id = 0;
> > > > > 			hw->phy.type = ixgbe_phy_unknown;
> > > > > 		}
> > > > > 
> > > > > The QSFP version a little more forceful:
> > > > > 
> > > > > 	err_read_i2c_eeprom:
> > > > > 		hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > > > 		hw->phy.id = 0;
> > > > > 		hw->phy.type = ixgbe_phy_unknown;
> > > > > 
> > > > > If we go forward without setting the phy_type to none, we
> > > > > will
> > > > > eventually run
> > > > > into issues calling other phy routines.
> > > > > 
> > > > > So should a lack of SFP, reset the PHY type? It's hazy
> > > > > because
> > > > > the
> > > > > difference
> > > > > between PHY and SFP isn't that clear to me here.
> > > > 
> > > > I'm not sure that's the same case:).
> > > > Just feel that it's better to handle ixgbe_phy_unknown directly
> > > > for
> > > > some device id as a special case than just replace it to
> > > > ixgbe_phy_none to cheat the check path, since that rely on we
> > > > never
> > > > change the way to handle ixgbe_phy_none.
> > > > 
> > > > So still have the question?
> > > > What is the failure if you go with ixgbe_phy_unknown?
> > > > Is that possible to work around this like
> > > > if (phy_type == ixgbe_phy_unknown && dev_id == xxxx)
> > > > 	...
> > > 
> > > Hi,
> > > 
> > > Thanks for having a look at this again. If you could please see
> > > the
> > > other answer, from myself, I've quoted the exact error we see and
> > > the
> > > issue it causes.
> > 
> > Yes, I see it failed at eth_ixgbe_dev_init, it will be better if
> > you
> > can provide more detail for the call stack, so we can figure out if
> > we can work around this by handle ixgbe_phy_unknown directly with
> > some special case.
> 
> Hi,
> 
> The original problem was found and fixed internally a while ago, so
> it's taking some time to get hold of the same hardware again. I hope
> to
> get back with more details next week, sorry for the delay.

Hi,

I had the chance to rebase the application on 18.11 and test again, and
it seems like the first patch is no longer necessary. Which is good!
Unfortunately I don't have time to bisect and find exactly when it was
fixed.

I have sent a v4 dropping the first patch.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH v4] net/ixgbe: add LHA ID to x550 code

[Zhang, Qi Z]

> -----Original Message-----
> From: Luca Boccassi [mailto:bluca@debian.org]
> Sent: Tuesday, November 20, 2018 3:27 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> Subject: [PATCH v4] net/ixgbe: add LHA ID to x550 code
> 
> ixgbe is able to recognize 1G SX and LX id, but it is missing the LHA.
> Add it, so that it can handle LHA sfp plugin.
> 
> Fixes: d2e72774e58c ("ixgbe/base: support X550")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>

The patch is OK for me, but here is some coding style need to fix.

Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>


> ---
> v4: dropped first patch, no longer necessary in 18.11
> 
>  drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
>  drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
>  drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
>  drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
> drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
>  5 files changed, 22 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c
> b/drivers/net/ixgbe/base/ixgbe_82599.c
> index 7de753fd3..96bdde62c 100644
> --- a/drivers/net/ixgbe/base/ixgbe_82599.c
> +++ b/drivers/net/ixgbe/base/ixgbe_82599.c
> @@ -392,6 +392,8 @@ s32 ixgbe_get_link_capabilities_82599(struct
> ixgbe_hw *hw,
>  	/* Check if 1G SFP module. */
>  	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
>  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
> +	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> +	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
>  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
>  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
>  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 || diff --git
> a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
> index 6cdd8fbab..dd118f917 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> @@ -1402,6 +1402,13 @@ s32 ixgbe_identify_sfp_module_generic(struct
> ixgbe_hw *hw)
>  				else
>  					hw->phy.sfp_type =
>  						ixgbe_sfp_type_1g_lx_core1;
> +			} else if (comp_codes_1g & IXGBE_SFF_1GBASELHA_CAPABLE) {
> +				if (hw->bus.lan_id == 0)
> +					hw->phy.sfp_type =
> +						ixgbe_sfp_type_1g_lha_core0;
> +				else
> +					hw->phy.sfp_type =
> +						ixgbe_sfp_type_1g_lha_core1;
>  			} else {
>  				hw->phy.sfp_type = ixgbe_sfp_type_unknown;
>  			}
> @@ -1489,6 +1496,8 @@ s32 ixgbe_identify_sfp_module_generic(struct
> ixgbe_hw *hw)
>  		if (comp_codes_10g == 0 &&
>  		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
> +		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> +		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 || @@
> -1508,6 +1517,8 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw
> *hw)
>  		if (!(enforce_sfp & IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
>  		    !(hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
> +		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> +		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
>  		      hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 || @@
> -1835,11 +1846,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct
> ixgbe_hw *hw,
>  	 */
>  	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
>  	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
> +	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
>  	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
>  	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
>  		sfp_type = ixgbe_sfp_type_srlr_core0;
>  	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
>  		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
> +		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
>  		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
>  		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
>  		sfp_type = ixgbe_sfp_type_srlr_core1; diff --git
> a/drivers/net/ixgbe/base/ixgbe_phy.h b/drivers/net/ixgbe/base/ixgbe_phy.h
> index 132fa542b..f1605f2cc 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.h
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.h
> @@ -41,6 +41,7 @@
>  #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
>  #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
>  #define IXGBE_SFF_1GBASET_CAPABLE	0x8
> +#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
>  #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
>  #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
>  #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
> diff --git a/drivers/net/ixgbe/base/ixgbe_type.h
> b/drivers/net/ixgbe/base/ixgbe_type.h
> index cee6ba2e0..077b8f01c 100644
> --- a/drivers/net/ixgbe/base/ixgbe_type.h
> +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> @@ -3724,6 +3724,8 @@ enum ixgbe_sfp_type {
>  	ixgbe_sfp_type_1g_sx_core1 = 12,
>  	ixgbe_sfp_type_1g_lx_core0 = 13,
>  	ixgbe_sfp_type_1g_lx_core1 = 14,
> +	ixgbe_sfp_type_1g_lha_core0 = 15,
> +	ixgbe_sfp_type_1g_lha_core1 = 16,
>  	ixgbe_sfp_type_not_present = 0xFFFE,
>  	ixgbe_sfp_type_unknown = 0xFFFF
>  };
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> b/drivers/net/ixgbe/base/ixgbe_x550.c
> index f7b98af52..a920a146e 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> @@ -1534,6 +1534,8 @@ STATIC s32
> ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
>  	case ixgbe_sfp_type_1g_sx_core1:
>  	case ixgbe_sfp_type_1g_lx_core0:
>  	case ixgbe_sfp_type_1g_lx_core1:
> +	case ixgbe_sfp_type_1g_lha_core0:
> +	case ixgbe_sfp_type_1g_lha_core1:
>  		*linear = false;
>  		break;
>  	case ixgbe_sfp_type_unknown:
> @@ -1874,6 +1876,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct
> ixgbe_hw *hw,
>  		/* Check if 1G SFP module. */
>  		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 ||
>  		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
> +		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> +		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1
>  		    || hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
>  		    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1) {
>  			*speed = IXGBE_LINK_SPEED_1GB_FULL;
> --
> 2.19.1

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] net/ixgbe: add LHA ID to x550 code

[Luca Boccassi]

On Tue, 2018-11-20 at 19:52 +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Luca Boccassi [mailto:bluca@debian.org]
> > Sent: Tuesday, November 20, 2018 3:27 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> > Subject: [PATCH v4] net/ixgbe: add LHA ID to x550 code
> > 
> > ixgbe is able to recognize 1G SX and LX id, but it is missing the
> > LHA.
> > Add it, so that it can handle LHA sfp plugin.
> > 
> > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Luca Boccassi <bluca@debian.org>
> 
> The patch is OK for me, but here is some coding style need to fix.
> 
> Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>

Thanks for the review!

I intentionally left the checkpatch complaint unfixed as it applies
just the same to the other existing lines above and below the addition
of this patch, so it makes more sense for it to be fixed (if desired)
separately and for all affected lines.

> > ---
> > v4: dropped first patch, no longer necessary in 18.11
> > 
> >  drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
> >  drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
> >  drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
> >  drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
> > drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
> >  5 files changed, 22 insertions(+)
> > 
> > diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c
> > b/drivers/net/ixgbe/base/ixgbe_82599.c
> > index 7de753fd3..96bdde62c 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_82599.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_82599.c
> > @@ -392,6 +392,8 @@ s32 ixgbe_get_link_capabilities_82599(struct
> > ixgbe_hw *hw,
> >  	/* Check if 1G SFP module. */
> >  	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
> >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
> > +	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> > +	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
> >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
> >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
> >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 || diff
> > --git
> > a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index 6cdd8fbab..dd118f917 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -1402,6 +1402,13 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw *hw)
> >  				else
> >  					hw->phy.sfp_type =
> >  						ixgbe_sfp_type_1g_
> > lx_core1;
> > +			} else if (comp_codes_1g &
> > IXGBE_SFF_1GBASELHA_CAPABLE) {
> > +				if (hw->bus.lan_id == 0)
> > +					hw->phy.sfp_type =
> > +						ixgbe_sfp_type_1g_
> > lha_core0;
> > +				else
> > +					hw->phy.sfp_type =
> > +						ixgbe_sfp_type_1g_
> > lha_core1;
> >  			} else {
> >  				hw->phy.sfp_type =
> > ixgbe_sfp_type_unknown;
> >  			}
> > @@ -1489,6 +1496,8 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw *hw)
> >  		if (comp_codes_10g == 0 &&
> >  		    !(hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_cu_core1 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_cu_core0 ||
> > +		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lha_core0 ||
> > +		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lha_core1 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lx_core0 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lx_core1 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_sx_core0 || @@
> > -1508,6 +1517,8 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw
> > *hw)
> >  		if (!(enforce_sfp &
> > IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
> >  		    !(hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_cu_core0 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_cu_core1 ||
> > +		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lha_core0 ||
> > +		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lha_core1 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lx_core0 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lx_core1 ||
> >  		      hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_sx_core0 || @@
> > -1835,11 +1846,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct
> > ixgbe_hw *hw,
> >  	 */
> >  	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
> >  	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
> > +	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> >  	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
> >  	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
> >  		sfp_type = ixgbe_sfp_type_srlr_core0;
> >  	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
> >  		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
> > +		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
> >  		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
> >  		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
> >  		sfp_type = ixgbe_sfp_type_srlr_core1; diff --git
> > a/drivers/net/ixgbe/base/ixgbe_phy.h
> > b/drivers/net/ixgbe/base/ixgbe_phy.h
> > index 132fa542b..f1605f2cc 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.h
> > @@ -41,6 +41,7 @@
> >  #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
> >  #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
> >  #define IXGBE_SFF_1GBASET_CAPABLE	0x8
> > +#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
> >  #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
> >  #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
> >  #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
> > diff --git a/drivers/net/ixgbe/base/ixgbe_type.h
> > b/drivers/net/ixgbe/base/ixgbe_type.h
> > index cee6ba2e0..077b8f01c 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_type.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> > @@ -3724,6 +3724,8 @@ enum ixgbe_sfp_type {
> >  	ixgbe_sfp_type_1g_sx_core1 = 12,
> >  	ixgbe_sfp_type_1g_lx_core0 = 13,
> >  	ixgbe_sfp_type_1g_lx_core1 = 14,
> > +	ixgbe_sfp_type_1g_lha_core0 = 15,
> > +	ixgbe_sfp_type_1g_lha_core1 = 16,
> >  	ixgbe_sfp_type_not_present = 0xFFFE,
> >  	ixgbe_sfp_type_unknown = 0xFFFF
> >  };
> > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > index f7b98af52..a920a146e 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > @@ -1534,6 +1534,8 @@ STATIC s32
> > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool
> > *linear)
> >  	case ixgbe_sfp_type_1g_sx_core1:
> >  	case ixgbe_sfp_type_1g_lx_core0:
> >  	case ixgbe_sfp_type_1g_lx_core1:
> > +	case ixgbe_sfp_type_1g_lha_core0:
> > +	case ixgbe_sfp_type_1g_lha_core1:
> >  		*linear = false;
> >  		break;
> >  	case ixgbe_sfp_type_unknown:
> > @@ -1874,6 +1876,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct
> > ixgbe_hw *hw,
> >  		/* Check if 1G SFP module. */
> >  		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0
> > ||
> >  		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
> > +		    || hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lha_core0 ||
> > +		    hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lha_core1
> >  		    || hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lx_core0 ||
> >  		    hw->phy.sfp_type ==
> > ixgbe_sfp_type_1g_lx_core1) {
> >  			*speed = IXGBE_LINK_SPEED_1GB_FULL;
> > --
> > 2.19.1
> 
> 
-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] net/ixgbe: add LHA ID to x550 code

[Zhang, Qi Z]

> -----Original Message-----
> From: Luca Boccassi [mailto:bluca@debian.org]
> Sent: Tuesday, November 20, 2018 1:08 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; 3chas3@gmail.com; stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH v4] net/ixgbe: add LHA ID to x550 code
> 
> On Tue, 2018-11-20 at 19:52 +0000, Zhang, Qi Z wrote:
> > > -----Original Message-----
> > > From: Luca Boccassi [mailto:bluca@debian.org]
> > > Sent: Tuesday, November 20, 2018 3:27 AM
> > > To: dev@dpdk.org
> > > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > > 3chas3@gmail.com; Luca Boccassi <bluca@debian.org>; stable@dpdk.org
> > > Subject: [PATCH v4] net/ixgbe: add LHA ID to x550 code
> > >
> > > ixgbe is able to recognize 1G SX and LX id, but it is missing the
> > > LHA.
> > > Add it, so that it can handle LHA sfp plugin.
> > >
> > > Fixes: d2e72774e58c ("ixgbe/base: support X550")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> >
> > The patch is OK for me, but here is some coding style need to fix.
> >
> > Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Thanks for the review!
> 
> I intentionally left the checkpatch complaint unfixed as it applies just the same to
> the other existing lines above and below the addition of this patch, so it makes
> more sense for it to be fixed (if desired) separately and for all affected lines.

Applied to dpdk-next-net-intel.

Thanks
Qi

> 
> > > ---
> > > v4: dropped first patch, no longer necessary in 18.11
> > >
> > >  drivers/net/ixgbe/base/ixgbe_82599.c |  2 ++
> > >  drivers/net/ixgbe/base/ixgbe_phy.c   | 13 +++++++++++++
> > >  drivers/net/ixgbe/base/ixgbe_phy.h   |  1 +
> > >  drivers/net/ixgbe/base/ixgbe_type.h  |  2 ++
> > > drivers/net/ixgbe/base/ixgbe_x550.c  |  4 ++++
> > >  5 files changed, 22 insertions(+)
> > >
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c
> > > b/drivers/net/ixgbe/base/ixgbe_82599.c
> > > index 7de753fd3..96bdde62c 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_82599.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_82599.c
> > > @@ -392,6 +392,8 @@ s32 ixgbe_get_link_capabilities_82599(struct
> > > ixgbe_hw *hw,
> > >  	/* Check if 1G SFP module. */
> > >  	if (hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
> > >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
> > > +	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> > > +	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
> > >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
> > >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
> > >  	    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0 || diff --git
> > > a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > index 6cdd8fbab..dd118f917 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > @@ -1402,6 +1402,13 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > > ixgbe_hw *hw)
> > >  				else
> > >  					hw->phy.sfp_type =
> > >  						ixgbe_sfp_type_1g_
> > > lx_core1;
> > > +			} else if (comp_codes_1g &
> > > IXGBE_SFF_1GBASELHA_CAPABLE) {
> > > +				if (hw->bus.lan_id == 0)
> > > +					hw->phy.sfp_type =
> > > +						ixgbe_sfp_type_1g_
> > > lha_core0;
> > > +				else
> > > +					hw->phy.sfp_type =
> > > +						ixgbe_sfp_type_1g_
> > > lha_core1;
> > >  			} else {
> > >  				hw->phy.sfp_type =
> > > ixgbe_sfp_type_unknown;
> > >  			}
> > > @@ -1489,6 +1496,8 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > > ixgbe_hw *hw)
> > >  		if (comp_codes_10g == 0 &&
> > >  		    !(hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_cu_core1 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_cu_core0 ||
> > > +		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lha_core0 ||
> > > +		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lha_core1 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lx_core0 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lx_core1 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_sx_core0 || @@
> > > -1508,6 +1517,8 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > > ixgbe_hw
> > > *hw)
> > >  		if (!(enforce_sfp &
> > > IXGBE_DEVICE_CAPS_ALLOW_ANY_SFP) &&
> > >  		    !(hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_cu_core0 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_cu_core1 ||
> > > +		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lha_core0 ||
> > > +		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lha_core1 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lx_core0 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lx_core1 ||
> > >  		      hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_sx_core0 || @@
> > > -1835,11 +1846,13 @@ s32 ixgbe_get_sfp_init_sequence_offsets(struct
> > > ixgbe_hw *hw,
> > >  	 */
> > >  	if (sfp_type == ixgbe_sfp_type_da_act_lmt_core0 ||
> > >  	    sfp_type == ixgbe_sfp_type_1g_lx_core0 ||
> > > +	    sfp_type == ixgbe_sfp_type_1g_lha_core0 ||
> > >  	    sfp_type == ixgbe_sfp_type_1g_cu_core0 ||
> > >  	    sfp_type == ixgbe_sfp_type_1g_sx_core0)
> > >  		sfp_type = ixgbe_sfp_type_srlr_core0;
> > >  	else if (sfp_type == ixgbe_sfp_type_da_act_lmt_core1 ||
> > >  		 sfp_type == ixgbe_sfp_type_1g_lx_core1 ||
> > > +		 sfp_type == ixgbe_sfp_type_1g_lha_core1 ||
> > >  		 sfp_type == ixgbe_sfp_type_1g_cu_core1 ||
> > >  		 sfp_type == ixgbe_sfp_type_1g_sx_core1)
> > >  		sfp_type = ixgbe_sfp_type_srlr_core1; diff --git
> > > a/drivers/net/ixgbe/base/ixgbe_phy.h
> > > b/drivers/net/ixgbe/base/ixgbe_phy.h
> > > index 132fa542b..f1605f2cc 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_phy.h
> > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.h
> > > @@ -41,6 +41,7 @@
> > >  #define IXGBE_SFF_1GBASESX_CAPABLE	0x1
> > >  #define IXGBE_SFF_1GBASELX_CAPABLE	0x2
> > >  #define IXGBE_SFF_1GBASET_CAPABLE	0x8
> > > +#define IXGBE_SFF_1GBASELHA_CAPABLE	0x10
> > >  #define IXGBE_SFF_10GBASESR_CAPABLE	0x10
> > >  #define IXGBE_SFF_10GBASELR_CAPABLE	0x20
> > >  #define IXGBE_SFF_SOFT_RS_SELECT_MASK	0x8
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_type.h
> > > b/drivers/net/ixgbe/base/ixgbe_type.h
> > > index cee6ba2e0..077b8f01c 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_type.h
> > > +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> > > @@ -3724,6 +3724,8 @@ enum ixgbe_sfp_type {
> > >  	ixgbe_sfp_type_1g_sx_core1 = 12,
> > >  	ixgbe_sfp_type_1g_lx_core0 = 13,
> > >  	ixgbe_sfp_type_1g_lx_core1 = 14,
> > > +	ixgbe_sfp_type_1g_lha_core0 = 15,
> > > +	ixgbe_sfp_type_1g_lha_core1 = 16,
> > >  	ixgbe_sfp_type_not_present = 0xFFFE,
> > >  	ixgbe_sfp_type_unknown = 0xFFFF
> > >  };
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > index f7b98af52..a920a146e 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > @@ -1534,6 +1534,8 @@ STATIC s32
> > > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool
> > > *linear)
> > >  	case ixgbe_sfp_type_1g_sx_core1:
> > >  	case ixgbe_sfp_type_1g_lx_core0:
> > >  	case ixgbe_sfp_type_1g_lx_core1:
> > > +	case ixgbe_sfp_type_1g_lha_core0:
> > > +	case ixgbe_sfp_type_1g_lha_core1:
> > >  		*linear = false;
> > >  		break;
> > >  	case ixgbe_sfp_type_unknown:
> > > @@ -1874,6 +1876,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct
> > > ixgbe_hw *hw,
> > >  		/* Check if 1G SFP module. */
> > >  		if (hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core0
> > > ||
> > >  		    hw->phy.sfp_type == ixgbe_sfp_type_1g_sx_core1
> > > +		    || hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lha_core0 ||
> > > +		    hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lha_core1
> > >  		    || hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lx_core0 ||
> > >  		    hw->phy.sfp_type ==
> > > ixgbe_sfp_type_1g_lx_core1) {
> > >  			*speed = IXGBE_LINK_SPEED_1GB_FULL;
> > > --
> > > 2.19.1
> >
> >
> --
> Kind regards,
> Luca Boccassi