diff mbox series

[04/21] net/ixgbe/base: x550em 10G NIC driver issue

Message ID 20200612032410.20864-5-guinanx.sun@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series update ixgbe base code |

Checks

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

Commit Message

Guinan Sun June 12, 2020, 3:23 a.m. UTC 
  With the NVM image for x550em XFI ethtool will not report
the auto-negotiation feature correctly. The auto-negotiation
should be "No" for supports and advertised items.

Signed-off-by: Piotr Skajewski <piotrx.skajewski@intel.com>
Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Ferruh Yigit June 22, 2020, 11:59 a.m. UTC | #1 
On 6/12/2020 4:23 AM, Guinan Sun wrote:
> With the NVM image for x550em XFI ethtool will not report
> the auto-negotiation feature correctly. The auto-negotiation
> should be "No" for supports and advertised items.

This is not 'ethtool' issue in this context, right? It also affects the reported
value for the DPDK?

And "driver issue" in the patch title gives only a little value, what do you
think something like:
"net/ixgbe/base: fix x550em 10G NIC auto-negotiation report"

Also does it only affects the auto-negotiation, I can see speed is updated too?

> 
> Signed-off-by: Piotr Skajewski <piotrx.skajewski@intel.com>
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> ---
>  drivers/net/ixgbe/base/ixgbe_x550.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
> index 3de406fd3..9fa999e01 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> @@ -1891,7 +1891,14 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
>  		else
>  			*speed = IXGBE_LINK_SPEED_10GB_FULL;
>  	} else {
> +		*autoneg = true;
> +
>  		switch (hw->phy.type) {
> +		case ixgbe_phy_x550em_xfi:
> +			*speed = IXGBE_LINK_SPEED_1GB_FULL |
> +				 IXGBE_LINK_SPEED_10GB_FULL;
> +			*autoneg = false;
> +			break;
>  		case ixgbe_phy_ext_1g_t:
>  #ifdef PREBOOT_SUPPORT
>  			*speed = IXGBE_LINK_SPEED_1GB_FULL;
> @@ -1925,7 +1932,6 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
>  				 IXGBE_LINK_SPEED_1GB_FULL;
>  			break;
>  		}
> -		*autoneg = true;
>  	}
>  
>  	return IXGBE_SUCCESS;
>
Guinan Sun July 1, 2020, 2:54 a.m. UTC | #2 
Hi Ferruh

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> Cc: Skajewski, PiotrX <piotrx.skajewski@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 04/21] net/ixgbe/base: x550em 10G NIC driver
> issue
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > With the NVM image for x550em XFI ethtool will not report the
> > auto-negotiation feature correctly. The auto-negotiation should be
> > "No" for supports and advertised items.
> 
> This is not 'ethtool' issue in this context, right? It also affects the reported
> value for the DPDK?

It is not only 'ethtool' issue but also affects the reported value for the DPDK.

> 
> And "driver issue" in the patch title gives only a little value, what do you think
> something like:
> "net/ixgbe/base: fix x550em 10G NIC auto-negotiation report"
> 

I agree with your opinion, patch v2 will fix it.

> Also does it only affects the auto-negotiation, I can see speed is updated too?

The speed update has an impact on the report, and patch v2 will modify the commit information to show the impact on speed.

> 
> >
> > Signed-off-by: Piotr Skajewski <piotrx.skajewski@intel.com>
> > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> > ---
> >  drivers/net/ixgbe/base/ixgbe_x550.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > index 3de406fd3..9fa999e01 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > @@ -1891,7 +1891,14 @@ s32 ixgbe_get_link_capabilities_X550em(struct
> ixgbe_hw *hw,
> >  		else
> >  			*speed = IXGBE_LINK_SPEED_10GB_FULL;
> >  	} else {
> > +		*autoneg = true;
> > +
> >  		switch (hw->phy.type) {
> > +		case ixgbe_phy_x550em_xfi:
> > +			*speed = IXGBE_LINK_SPEED_1GB_FULL |
> > +				 IXGBE_LINK_SPEED_10GB_FULL;
> > +			*autoneg = false;
> > +			break;
> >  		case ixgbe_phy_ext_1g_t:
> >  #ifdef PREBOOT_SUPPORT
> >  			*speed = IXGBE_LINK_SPEED_1GB_FULL; @@ -1925,7
> +1932,6 @@ s32
> > ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
> >  				 IXGBE_LINK_SPEED_1GB_FULL;
> >  			break;
> >  		}
> > -		*autoneg = true;
> >  	}
> >
> >  	return IXGBE_SUCCESS;
> >
diff mbox series

Patch

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 3de406fd3..9fa999e01 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1891,7 +1891,14 @@  s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 		else
 			*speed = IXGBE_LINK_SPEED_10GB_FULL;
 	} else {
+		*autoneg = true;
+
 		switch (hw->phy.type) {
+		case ixgbe_phy_x550em_xfi:
+			*speed = IXGBE_LINK_SPEED_1GB_FULL |
+				 IXGBE_LINK_SPEED_10GB_FULL;
+			*autoneg = false;
+			break;
 		case ixgbe_phy_ext_1g_t:
 #ifdef PREBOOT_SUPPORT
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
@@ -1925,7 +1932,6 @@  s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 				 IXGBE_LINK_SPEED_1GB_FULL;
 			break;
 		}
-		*autoneg = true;
 	}
 
 	return IXGBE_SUCCESS;