diff mbox series

[net-next,v1,2/2] ice: Implement ETHTOOL_FEC_ALL support

Message ID 20221128170354.2537171-3-mikael.barsehyan@intel.com
State Changes Requested
Headers show
Series add ethtool FEC-all option | expand

Commit Message

Mikael Barsehyan Nov. 28, 2022, 5:03 p.m. UTC 
From: Wojciech Drewek <wojciech.drewek@intel.com>

In case of ETHTOOL_FEC_ALL we enable same flags in fw
as in case of ETHTOOL_FEC_AUTO. One additional flag that
we anable is ICE_AQC_PHY_FEC_DIS which allow to detect
No-Fec mode.

Introduce ice_fw_supports_fec_all function which
checks if this feature is supported by the firmware.

Co-developed-by: Paul Greenwalt <paul.greenwalt@intel.com>
Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
 drivers/net/ethernet/intel/ice/ice_common.c   | 53 ++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_common.h   |  1 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c  | 13 ++++-
 drivers/net/ethernet/intel/ice/ice_main.c     |  3 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |  9 +++-
 6 files changed, 76 insertions(+), 4 deletions(-)

Comments

Tony Nguyen Nov. 30, 2022, 6:18 p.m. UTC | #1 
On 11/28/2022 9:03 AM, Mikael Barsehyan wrote:
> From: Wojciech Drewek <wojciech.drewek@intel.com>
> 
> In case of ETHTOOL_FEC_ALL we enable same flags in fw
> as in case of ETHTOOL_FEC_AUTO. One additional flag that
> we anable is ICE_AQC_PHY_FEC_DIS which allow to detect

s/anable/enable

> No-Fec mode.
> 
> Introduce ice_fw_supports_fec_all function which
> checks if this feature is supported by the firmware.
> 
> Co-developed-by: Paul Greenwalt <paul.greenwalt@intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
>   .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
>   drivers/net/ethernet/intel/ice/ice_common.c   | 53 ++++++++++++++++++-
>   drivers/net/ethernet/intel/ice/ice_common.h   |  1 +
>   drivers/net/ethernet/intel/ice/ice_ethtool.c  | 13 ++++-
>   drivers/net/ethernet/intel/ice/ice_main.c     |  3 +-
>   drivers/net/ethernet/intel/ice/ice_type.h     |  9 +++-
>   6 files changed, 76 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 54c87ced4fad..4e3934c4e9b7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -1141,6 +1141,7 @@ struct ice_aqc_get_phy_caps_data {
>   #define ICE_AQC_PHY_FEC_25G_RS_528_REQ			BIT(2)
>   #define ICE_AQC_PHY_FEC_25G_KR_REQ			BIT(3)
>   #define ICE_AQC_PHY_FEC_25G_RS_544_REQ			BIT(4)
> +#define ICE_AQC_PHY_FEC_DIS				BIT(5)

Perhaps ICE_AQC_PHY_NO_FEC_EN would match the existing naming better

>   #define ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN		BIT(6)
>   #define ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN		BIT(7)
>   #define ICE_AQC_PHY_FEC_MASK				ICE_M(0xdf, 0)
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 0e9584e50d82..f7db43ac41e8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -3300,8 +3300,11 @@ enum ice_fc_mode ice_caps_to_fc_mode(u8 caps)
>    */
>   enum ice_fec_mode ice_caps_to_fec_mode(u8 caps, u8 fec_options)
>   {
> -	if (caps & ICE_AQC_PHY_EN_AUTO_FEC)
> +	if (caps & ICE_AQC_PHY_EN_AUTO_FEC) {
> +		if (fec_options & ICE_AQC_PHY_FEC_DIS)

I believe this is a capability bit which would mean that Auto FEC and a 
module that supports No FEC would report FEC All?

> +			return ICE_FEC_ALL;
>   		return ICE_FEC_AUTO;
> +	}
>   
>   	if (fec_options & (ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
>   			   ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |

<snip>

> @@ -1257,8 +1265,11 @@ ice_get_fecparam(struct net_device *netdev, struct ethtool_fecparam *fecparam)
>   		goto done;
>   
>   	/* Set supported/configured FEC modes based on PHY capability */
> -	if (caps->caps & ICE_AQC_PHY_EN_AUTO_FEC)
> +	if (caps->caps & ICE_AQC_PHY_EN_AUTO_FEC) {
> +		if (caps->link_fec_options & ICE_AQC_PHY_FEC_DIS)

If it's a capability bit, this should probably check 
ice_fw_supports_fec_all() instead.

> +			fecparam->fec |= ETHTOOL_FEC_ALL;
>   		fecparam->fec |= ETHTOOL_FEC_AUTO;
> +	}
>   	if (caps->link_fec_options & ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN ||
>   	    caps->link_fec_options & ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ ||
>   	    caps->link_fec_options & ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN ||
Wojciech Drewek Dec. 1, 2022, 12:16 p.m. UTC | #2 
Hi Tony,
Thx for the review

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Tony Nguyen
> Sent: środa, 30 listopada 2022 19:18
> To: Barsehyan, Mikael <mikael.barsehyan@intel.com>; intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH net-next v1 2/2] ice: Implement ETHTOOL_FEC_ALL support
> 
> On 11/28/2022 9:03 AM, Mikael Barsehyan wrote:
> > From: Wojciech Drewek <wojciech.drewek@intel.com>
> >
> > In case of ETHTOOL_FEC_ALL we enable same flags in fw
> > as in case of ETHTOOL_FEC_AUTO. One additional flag that
> > we anable is ICE_AQC_PHY_FEC_DIS which allow to detect
> 
> s/anable/enable

Will be fixed

> 
> > No-Fec mode.
> >
> > Introduce ice_fw_supports_fec_all function which
> > checks if this feature is supported by the firmware.
> >
> > Co-developed-by: Paul Greenwalt <paul.greenwalt@intel.com>
> > Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> > ---
> >   .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
> >   drivers/net/ethernet/intel/ice/ice_common.c   | 53 ++++++++++++++++++-
> >   drivers/net/ethernet/intel/ice/ice_common.h   |  1 +
> >   drivers/net/ethernet/intel/ice/ice_ethtool.c  | 13 ++++-
> >   drivers/net/ethernet/intel/ice/ice_main.c     |  3 +-
> >   drivers/net/ethernet/intel/ice/ice_type.h     |  9 +++-
> >   6 files changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > index 54c87ced4fad..4e3934c4e9b7 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > @@ -1141,6 +1141,7 @@ struct ice_aqc_get_phy_caps_data {
> >   #define ICE_AQC_PHY_FEC_25G_RS_528_REQ			BIT(2)
> >   #define ICE_AQC_PHY_FEC_25G_KR_REQ			BIT(3)
> >   #define ICE_AQC_PHY_FEC_25G_RS_544_REQ			BIT(4)
> > +#define ICE_AQC_PHY_FEC_DIS				BIT(5)
> 
> Perhaps ICE_AQC_PHY_NO_FEC_EN would match the existing naming better

Agree

> 
> >   #define ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN		BIT(6)
> >   #define ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN		BIT(7)
> >   #define ICE_AQC_PHY_FEC_MASK				ICE_M(0xdf, 0)
> > diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> > index 0e9584e50d82..f7db43ac41e8 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_common.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> > @@ -3300,8 +3300,11 @@ enum ice_fc_mode ice_caps_to_fc_mode(u8 caps)
> >    */
> >   enum ice_fec_mode ice_caps_to_fec_mode(u8 caps, u8 fec_options)
> >   {
> > -	if (caps & ICE_AQC_PHY_EN_AUTO_FEC)
> > +	if (caps & ICE_AQC_PHY_EN_AUTO_FEC) {
> > +		if (fec_options & ICE_AQC_PHY_FEC_DIS)
> 
> I believe this is a capability bit which would mean that Auto FEC and a
> module that supports No FEC would report FEC All?

I think so.

> 
> > +			return ICE_FEC_ALL;
> >   		return ICE_FEC_AUTO;
> > +	}
> >
> >   	if (fec_options & (ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
> >   			   ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |
> 
> <snip>
> 
> > @@ -1257,8 +1265,11 @@ ice_get_fecparam(struct net_device *netdev, struct ethtool_fecparam *fecparam)
> >   		goto done;
> >
> >   	/* Set supported/configured FEC modes based on PHY capability */
> > -	if (caps->caps & ICE_AQC_PHY_EN_AUTO_FEC)
> > +	if (caps->caps & ICE_AQC_PHY_EN_AUTO_FEC) {
> > +		if (caps->link_fec_options & ICE_AQC_PHY_FEC_DIS)
> 
> If it's a capability bit, this should probably check
> ice_fw_supports_fec_all() instead.

Agree

> 
> > +			fecparam->fec |= ETHTOOL_FEC_ALL;
> >   		fecparam->fec |= ETHTOOL_FEC_AUTO;
> > +	}
> >   	if (caps->link_fec_options & ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN ||
> >   	    caps->link_fec_options & ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ ||
> >   	    caps->link_fec_options & ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN ||
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 54c87ced4fad..4e3934c4e9b7 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1141,6 +1141,7 @@  struct ice_aqc_get_phy_caps_data {
 #define ICE_AQC_PHY_FEC_25G_RS_528_REQ			BIT(2)
 #define ICE_AQC_PHY_FEC_25G_KR_REQ			BIT(3)
 #define ICE_AQC_PHY_FEC_25G_RS_544_REQ			BIT(4)
+#define ICE_AQC_PHY_FEC_DIS				BIT(5)
 #define ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN		BIT(6)
 #define ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN		BIT(7)
 #define ICE_AQC_PHY_FEC_MASK				ICE_M(0xdf, 0)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 0e9584e50d82..f7db43ac41e8 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -3300,8 +3300,11 @@  enum ice_fc_mode ice_caps_to_fc_mode(u8 caps)
  */
 enum ice_fec_mode ice_caps_to_fec_mode(u8 caps, u8 fec_options)
 {
-	if (caps & ICE_AQC_PHY_EN_AUTO_FEC)
+	if (caps & ICE_AQC_PHY_EN_AUTO_FEC) {
+		if (fec_options & ICE_AQC_PHY_FEC_DIS)
+			return ICE_FEC_ALL;
 		return ICE_FEC_AUTO;
+	}
 
 	if (fec_options & (ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
 			   ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |
@@ -3560,6 +3563,11 @@  ice_cfg_phy_fec(struct ice_port_info *pi, struct ice_aqc_set_phy_cfg_data *cfg,
 		/* Clear all FEC option bits. */
 		cfg->link_fec_opt &= ~ICE_AQC_PHY_FEC_MASK;
 		break;
+	case ICE_FEC_ALL:
+		if (!ice_fw_supports_fec_all(hw))
+			return -EOPNOTSUPP;
+		cfg->link_fec_opt |= ICE_AQC_PHY_FEC_DIS;
+		fallthrough;
 	case ICE_FEC_AUTO:
 		/* AND auto FEC bit, and all caps bits. */
 		cfg->caps &= ICE_AQC_PHY_CAPS_MASK;
@@ -5359,6 +5367,35 @@  static bool ice_is_fw_api_min_ver(struct ice_hw *hw, u8 maj, u8 min, u8 patch)
 	return false;
 }
 
+/**
+ * ice_is_fw_min_ver
+ * @hw: pointer to the hardware structure
+ * @branch: branch version
+ * @maj: major version
+ * @min: minor version
+ * @patch: patch version
+ *
+ * Checks if the firmware is minimum version
+ */
+static bool ice_is_fw_min_ver(struct ice_hw *hw, u8 branch, u8 maj, u8 min,
+			      u8 patch)
+{
+	if (hw->fw_branch == branch) {
+		if (hw->fw_maj_ver > maj)
+			return true;
+		if (hw->fw_maj_ver == maj) {
+			if (hw->fw_min_ver > min)
+				return true;
+			if (hw->fw_min_ver == min && hw->fw_patch >= patch)
+				return true;
+		}
+	} else if (hw->fw_branch > branch) {
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * ice_fw_supports_link_override
  * @hw: pointer to the hardware structure
@@ -5559,6 +5596,20 @@  bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw)
 				     ICE_FW_API_REPORT_DFLT_CFG_PATCH);
 }
 
+/**
+ * ice_fw_supports_fec_all
+ * @hw: pointer to the hardware structure
+ *
+ * Checks if the firmware supports FEC disable in Auto FEC mode
+ */
+bool ice_fw_supports_fec_all(struct ice_hw *hw)
+{
+	return ice_is_fw_min_ver(hw, ICE_FW_FEC_DIS_AUTO_BRANCH,
+				 ICE_FW_FEC_DIS_AUTO_MAJ,
+				 ICE_FW_FEC_DIS_AUTO_MIN,
+				 ICE_FW_FEC_DIS_AUTO_PATCH);
+}
+
 /* each of the indexes into the following array match the speed of a return
  * value from the list of AQ returned speeds like the range:
  * ICE_AQ_LINK_SPEED_10MB .. ICE_AQ_LINK_SPEED_100GB excluding
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 6a7898565072..ef313ea680c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -110,6 +110,7 @@  int
 ice_aq_set_phy_cfg(struct ice_hw *hw, struct ice_port_info *pi,
 		   struct ice_aqc_set_phy_cfg_data *cfg, struct ice_sq_cd *cd);
 bool ice_fw_supports_link_override(struct ice_hw *hw);
+bool ice_fw_supports_fec_all(struct ice_hw *hw);
 int
 ice_get_link_default_override(struct ice_link_default_override_tlv *ldo,
 			      struct ice_port_info *pi);
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index ba4ccc5f7d60..d8f8e1d214c0 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -1187,12 +1187,20 @@  ice_set_fecparam(struct net_device *netdev, struct ethtool_fecparam *fecparam)
 {
 	struct ice_netdev_priv *np = netdev_priv(netdev);
 	struct ice_vsi *vsi = np->vsi;
+	struct ice_pf *pf = vsi->back;
 	enum ice_fec_mode fec;
 
 	switch (fecparam->fec) {
 	case ETHTOOL_FEC_AUTO:
 		fec = ICE_FEC_AUTO;
 		break;
+	case ETHTOOL_FEC_ALL:
+		if (!ice_fw_supports_fec_all(&pf->hw)) {
+			dev_warn(ice_pf_to_dev(vsi->back), "FEC ALL is not supported by current firmware\n");
+			return -EOPNOTSUPP;
+		}
+		fec = ICE_FEC_ALL;
+		break;
 	case ETHTOOL_FEC_RS:
 		fec = ICE_FEC_RS;
 		break;
@@ -1257,8 +1265,11 @@  ice_get_fecparam(struct net_device *netdev, struct ethtool_fecparam *fecparam)
 		goto done;
 
 	/* Set supported/configured FEC modes based on PHY capability */
-	if (caps->caps & ICE_AQC_PHY_EN_AUTO_FEC)
+	if (caps->caps & ICE_AQC_PHY_EN_AUTO_FEC) {
+		if (caps->link_fec_options & ICE_AQC_PHY_FEC_DIS)
+			fecparam->fec |= ETHTOOL_FEC_ALL;
 		fecparam->fec |= ETHTOOL_FEC_AUTO;
+	}
 	if (caps->link_fec_options & ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN ||
 	    caps->link_fec_options & ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ ||
 	    caps->link_fec_options & ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN ||
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 69984fea7fce..2bf27b40d09f 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2180,7 +2180,8 @@  static int ice_configure_phy(struct ice_vsi *vsi)
 	ice_cfg_phy_fec(pi, cfg, phy->curr_user_fec_req);
 
 	/* Can't provide what was requested; use PHY capabilities */
-	if (cfg->link_fec_opt !=
+	if (phy->curr_user_fec_req != ICE_FEC_ALL &&
+	    cfg->link_fec_opt !=
 	    (cfg->link_fec_opt & pcaps->link_fec_options)) {
 		cfg->caps |= pcaps->caps & ICE_AQC_PHY_EN_AUTO_FEC;
 		cfg->link_fec_opt = pcaps->link_fec_options;
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index f77992cbfa41..b60ac101db31 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -107,7 +107,8 @@  enum ice_fec_mode {
 	ICE_FEC_NONE = 0,
 	ICE_FEC_RS,
 	ICE_FEC_BASER,
-	ICE_FEC_AUTO
+	ICE_FEC_AUTO,
+	ICE_FEC_ALL
 };
 
 struct ice_phy_cache_mode_data {
@@ -1155,4 +1156,10 @@  struct ice_aq_get_set_rss_lut_params {
 #define ICE_FW_API_REPORT_DFLT_CFG_MIN		7
 #define ICE_FW_API_REPORT_DFLT_CFG_PATCH	3
 
+/* FW version for FEC disable in Auto FEC mode */
+#define ICE_FW_FEC_DIS_AUTO_BRANCH		1
+#define ICE_FW_FEC_DIS_AUTO_MAJ			7
+#define ICE_FW_FEC_DIS_AUTO_MIN			0
+#define ICE_FW_FEC_DIS_AUTO_PATCH		5
+
 #endif /* _ICE_TYPE_H_ */