diff mbox

[dpdk-dev,v2] net/ixgbe: add support of 2.5G and 5G on X550

Message ID 1501061376-41938-1-git-send-email-wei.dai@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wei Dai July 26, 2017, 9:29 a.m. UTC 
  This patch adds support of 2.5G and 5G ethernet interface on X550.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

Comments

Wenzhuo Lu July 26, 2017, 1:26 p.m. UTC | #1 
Hi Wei,

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, July 26, 2017 5:30 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550
> 
> This patch adds support of 2.5G and 5G ethernet interface on X550.
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 33 ++++++++++++++++++++++++++++++--
> -
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 194058f..60e1d5a 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2647,9 +2647,28 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> 
>  	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?

> +			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,
>  		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
Wei Dai July 26, 2017, 2:10 p.m. UTC | #2 
Hi, Wenzhuo
> >  	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.

> 
> > +			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,
> >  		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
Wenzhuo Lu July 26, 2017, 2:13 p.m. UTC | #3 
Hi Wei,

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, July 26, 2017 10:10 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550
> 
> Hi, Wenzhuo
> > >  	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.

> 
> >
> > > +			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,
> > >  		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
Wei Dai July 26, 2017, 2:31 p.m. UTC | #4 
> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Wednesday, July 26, 2017 10:14 PM
> To: Dai, Wei <wei.dai@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550
> 
> Hi Wei,
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Wednesday, July 26, 2017 10:10 AM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH v2] net/ixgbe: add support of 2.5G and 5G on X550
> >
> > Hi, Wenzhuo
> > > >  	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.

> 
> >
> > >
> > > > +			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,
> > > >  		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
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 194058f..60e1d5a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2647,9 +2647,28 @@  ixgbe_dev_start(struct rte_eth_dev *dev)
 
 	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;
+			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,
 		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;