Re: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550

-----Original Message-----
From: Lu, Wenzhuo
Sent: Wednesday, July 26, 2017 10:14 PM
To: Dai, Wei <redacted>; Ananyev, Konstantin
[off-list ref]
Cc: dev@dpdk.org
Subject: RE: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550

Hi Wei,

quoted

-----Original Message-----
From: Dai, Wei
Sent: Wednesday, July 26, 2017 10:10 AM
To: Lu, Wenzhuo <redacted>; Ananyev, Konstantin
[off-list ref]
Cc: dev@dpdk.org
Subject: RE: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550

Hi, Wenzhuo

quoted

quoted

 	speed = 0x0;
 	if (*link_speeds == ETH_LINK_SPEED_AUTONEG) {
-		speed = (hw->mac.type != ixgbe_mac_82598EB) ?
-				IXGBE_LINK_SPEED_82599_AUTONEG :
-				IXGBE_LINK_SPEED_82598_AUTONEG;
+		switch (hw->mac.type) {
+		case ixgbe_mac_82598EB:
+			speed = IXGBE_LINK_SPEED_82598_AUTONEG;
+			break;
+
+		case ixgbe_mac_82599EB:
+		case ixgbe_mac_X540:
+			speed = IXGBE_LINK_SPEED_82599_AUTONEG;
+			break;
+
+		case ixgbe_mac_X550:
+		case ixgbe_mac_X550EM_x:
+		case ixgbe_mac_X550EM_a:
+			speed = IXGBE_LINK_SPEED_100_FULL |
+				IXGBE_LINK_SPEED_1GB_FULL |
+				IXGBE_LINK_SPEED_10GB_FULL |
+				IXGBE_LINK_SPEED_2_5GB_FULL |
+				IXGBE_LINK_SPEED_5GB_FULL;

Thanks for the patch. It looks good to me. A suggestion, why not
define a macro like IXGBE_LINK_SPEED_82599_AUTONEG for x550?

IXGBE_LINK_SPEED_82599_AUTONEG is defined in
drivers/net/ixgbe/base/ixgbe_type.h .
Normally, the base driver code in this file is provided and maintained
by another team.
I will ask them to add similar macro in it.
Currently, I'd like do this way. And I will replace it till a similar
macro is introduced.

No. I'm talking about changing the base code. You can add a macro in any .h
or even .c file you like.
Then when there's a macro in base code, it's easier to replace a macro by
another macro.

I'll follow your guidance in my v3 patch.

quoted

quoted

quoted

+			break;
+		default:
+			speed = IXGBE_LINK_SPEED_82599_AUTONEG;
+		}
 	} else {
 		if (*link_speeds & ETH_LINK_SPEED_10G)
 			speed |= IXGBE_LINK_SPEED_10GB_FULL; @@ -
3972,6 +3991,14 @@ ixgbe_dev_link_update_share(struct rte_eth_dev

*dev,

quoted

 		link.link_speed = ETH_SPEED_NUM_1G;
 		break;

+	case IXGBE_LINK_SPEED_2_5GB_FULL:
+		link.link_speed = ETH_SPEED_NUM_2_5G;
+		break;
+
+	case IXGBE_LINK_SPEED_5GB_FULL:
+		link.link_speed = ETH_SPEED_NUM_5G;
+		break;
+
 	case IXGBE_LINK_SPEED_10GB_FULL:
 		link.link_speed = ETH_SPEED_NUM_10G;
 		break;
--
2.7.5