[v4,1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs

Message ID 20220228152937.21247-2-jeffd@silicom-usa.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series ixgbe SFP handling fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jeff Daly Feb. 28, 2022, 3:29 p.m. UTC
  From: Stephen Douthit <stephend@silicom-usa.com>

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

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

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

Comments

Wang, Haiyue March 1, 2022, 5:56 a.m. UTC | #1
> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Monday, February 28, 2022 23:30
> To: dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; stable@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [PATCH v4 1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs
> 
> From: Stephen Douthit <stephend@silicom-usa.com>
> 
> Currently all X500EM* MAC types fallthrough to the default case and get
> reported as non-SFP regardless of media type, which isn't correct.
> 
> Fixes: 0790adeb567 ("ixgbe/base: support X550em_a device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
> Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Acked-by: Haiyue Wang <haiyue.wang@intel.com>

> --
> 2.25.1
  
Qi Zhang March 1, 2022, 11:18 a.m. UTC | #2
> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Tuesday, March 1, 2022 1:57 PM
> To: Daly, Jeff <jeffd@silicom-usa.com>; dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; stable@dpdk.org; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Subject: RE: [PATCH v4 1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid result
> for X550EM_a devs

> 
> > -----Original Message-----
> > From: Jeff Daly <jeffd@silicom-usa.com>
> > Sent: Monday, February 28, 2022 23:30
> > To: dev@dpdk.org
> > Cc: Stephen Douthit <stephend@silicom-usa.com>; stable@dpdk.org;
> Wang,
> > Haiyue <haiyue.wang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > Subject: [PATCH v4 1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid
> > result for X550EM_a devs
> >
> > From: Stephen Douthit <stephend@silicom-usa.com>
> >
> > Currently all X500EM* MAC types fallthrough to the default case and
> > get reported as non-SFP regardless of media type, which isn't correct.
> >
> > Fixes: 0790adeb567 ("ixgbe/base: support X550em_a device")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
> > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> 
> Acked-by: Haiyue Wang <haiyue.wang@intel.com>

Applied to dpdk-next-net-intel after renaming the title to "fix FSP check for X550EM devices" to fix check-git-log error.

Thanks
Qi
> 
> > --
> > 2.25.1
  
Thomas Monjalon March 6, 2022, 5:56 p.m. UTC | #3
01/03/2022 12:18, Zhang, Qi Z:
> From: Wang, Haiyue <haiyue.wang@intel.com>
> > From: Jeff Daly <jeffd@silicom-usa.com>
> > > From: Stephen Douthit <stephend@silicom-usa.com>
> > >
> > > Currently all X500EM* MAC types fallthrough to the default case and
> > > get reported as non-SFP regardless of media type, which isn't correct.
> > >
> > > Fixes: 0790adeb567 ("ixgbe/base: support X550em_a device")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
> > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > 
> > Acked-by: Haiyue Wang <haiyue.wang@intel.com>
> 
> Applied to dpdk-next-net-intel after renaming the title to "fix FSP check for X550EM devices" to fix check-git-log error.

It seems you have applied only the first patch of the series. Why?
Is there a good reason to split a series without any justification?
What about the two other patches?
  
Jeff Daly March 8, 2022, 3:01 p.m. UTC | #4
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Sunday, March 6, 2022 12:56 PM
> To: Wang, Haiyue <haiyue.wang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; ferruh.yigit@intel.com
> Cc: Jeff Daly <jeffd@silicom-usa.com>; dev@dpdk.org; stable@dpdk.org;
> Stephen Douthit <stephend@silicom-usa.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [PATCH v4 1/3] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for
> X550EM_a devs
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments.
> 
> 
> 01/03/2022 12:18, Zhang, Qi Z:
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > > From: Jeff Daly <jeffd@silicom-usa.com>
> > > > From: Stephen Douthit <stephend@silicom-usa.com>
> > > >
> > > > Currently all X500EM* MAC types fallthrough to the default case
> > > > and get reported as non-SFP regardless of media type, which isn't correct.
> > > >
> > > > Fixes: 0790adeb567 ("ixgbe/base: support X550em_a device")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Stephen Douthit <stephend@silicom-usa.com>
> > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > >
> > > Acked-by: Haiyue Wang <haiyue.wang@intel.com>
> >
> > Applied to dpdk-next-net-intel after renaming the title to "fix FSP check for
> X550EM devices" to fix check-git-log error.
> 
> It seems you have applied only the first patch of the series. Why?
> Is there a good reason to split a series without any justification?
> What about the two other patches?
> 

I should explain the [PATCH v4 x/y] and why it changed from 7 patches to 3 initially.....     In Stephen's initial v1 of the patch series there were 7 patches and during the discussion of the v2 version before Stephen left he intended to break it into 3 separate submissions.  After speaking with Ekinops/Swisscom regarding the best way to move these forward it was decided to move some functionality back to ethdev.c.  So currently:  [PATCH v4 1/3] is basically [PATCH v2 1/7], [PATCH v2 2/7] and [PATCH v2 5/7] were reworked and included in [PATCH v4 3/3] .  [PATCH v2 4/7] became [PATCH v4 2/3].   I submitted 2 separate patches yesterday for what was [PATCH v2 6/7] and [PATCH v2 7/7] since they were small patches to base/ rather than ethdev.   Finally, [PATCH v2 3/7] i'm working out whether it can be reworked into something that only touches ethdev (could be more kludgey) or really kinda just fits into base as originally submitted, like the last 2 patches I submitted for special handling of some SFPs.

I discovered the ixgbe_api functionality the other day while looking into this, and was thinking perhaps it could be somehow done there.  Still under base, but not touching the main driver files.   Is there any documentation that describes the ixgbe_api usage or process to get a wrapper call added to it?
  

Patch

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