diff mbox series

[iwl-next,v1,2/3] ice: Add helper function ice_is_generic_mac

Message ID 20231206192919.3826128-3-grzegorz.nitka@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series ice: add E825C device family support | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers warning 5 maintainers not CCed: kuba@kernel.org pabeni@redhat.com anthony.l.nguyen@intel.com jesse.brandeburg@intel.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1142 this patch: 1142
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 52 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Grzegorz Nitka Dec. 6, 2023, 7:29 p.m. UTC
E800 series devices have a couple of quirks:
1. Sideband control queues are not supported
2. The registers that the driver needs to program for the "Precision
   Time Protocol (PTP)" feature are different for E800 series devices
   compared to other devices supported by this driver.

Both these require conditional logic based on the underlying device we
are dealing with.

The function ice_is_sbq_supported added by commit 8f5ee3c477a8
("ice: add support for sideband messages") addresses (1).
The same function can be used to address (2) as well
but this just looks weird readability wise in cases that have nothing
to do with sideband control queues:

	if (ice_is_sbq_supported(hw))
		/* program register A */
	else
		/* program register B */

For these cases, the function ice_is_generic_mac introduced by this
patch communicates the idea/intention better. Also rework
ice_is_sbq_supported to use this new function.
As side-band queue is supported for E825C devices, it's mac_type is
considered as generic mac_type.

Co-developed-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_common.c   | 12 ++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.h   |  1 +
 drivers/net/ethernet/intel/ice/ice_controlq.c |  2 +-
 drivers/net/ethernet/intel/ice/ice_main.c     |  6 ++++--
 drivers/net/ethernet/intel/ice/ice_type.h     |  1 +
 5 files changed, 19 insertions(+), 3 deletions(-)

Comments

Simon Horman Dec. 12, 2023, 10:01 a.m. UTC | #1
On Wed, Dec 06, 2023 at 08:29:18PM +0100, Grzegorz Nitka wrote:
> E800 series devices have a couple of quirks:
> 1. Sideband control queues are not supported
> 2. The registers that the driver needs to program for the "Precision
>    Time Protocol (PTP)" feature are different for E800 series devices
>    compared to other devices supported by this driver.
> 
> Both these require conditional logic based on the underlying device we
> are dealing with.
> 
> The function ice_is_sbq_supported added by commit 8f5ee3c477a8
> ("ice: add support for sideband messages") addresses (1).
> The same function can be used to address (2) as well
> but this just looks weird readability wise in cases that have nothing
> to do with sideband control queues:
> 
> 	if (ice_is_sbq_supported(hw))
> 		/* program register A */
> 	else
> 		/* program register B */
> 
> For these cases, the function ice_is_generic_mac introduced by this
> patch communicates the idea/intention better. Also rework
> ice_is_sbq_supported to use this new function.
> As side-band queue is supported for E825C devices, it's mac_type is
> considered as generic mac_type.
> 
> Co-developed-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>
Pucha, HimasekharX Reddy Jan. 8, 2024, 4:53 a.m. UTC | #2
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Grzegorz Nitka
> Sent: Thursday, December 7, 2023 12:59 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v1 2/3] ice: Add helper function ice_is_generic_mac
>
> E800 series devices have a couple of quirks:
> 1. Sideband control queues are not supported
> 2. The registers that the driver needs to program for the "Precision
>    Time Protocol (PTP)" feature are different for E800 series devices
>    compared to other devices supported by this driver.
>
> Both these require conditional logic based on the underlying device we
> are dealing with.
>
> The function ice_is_sbq_supported added by commit 8f5ee3c477a8
> ("ice: add support for sideband messages") addresses (1).
> The same function can be used to address (2) as well
> but this just looks weird readability wise in cases that have nothing
> to do with sideband control queues:
>
> 	if (ice_is_sbq_supported(hw))
> 		/* program register A */
> 	else
> 		/* program register B */
> 
> For these cases, the function ice_is_generic_mac introduced by this
> patch communicates the idea/intention better. Also rework
> ice_is_sbq_supported to use this new function.
> As side-band queue is supported for E825C devices, it's mac_type is
> considered as generic mac_type.
>
> Co-developed-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_common.c   | 12 ++++++++++++
>  drivers/net/ethernet/intel/ice/ice_common.h   |  1 +
>  drivers/net/ethernet/intel/ice/ice_controlq.c |  2 +-
>  drivers/net/ethernet/intel/ice/ice_main.c     |  6 ++++--
>  drivers/net/ethernet/intel/ice/ice_type.h     |  1 +
>  5 files changed, 19 insertions(+), 3 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 0a519310472b..2a973fac8e54 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -169,6 +169,18 @@  static int ice_set_mac_type(struct ice_hw *hw)
 	return 0;
 }
 
+/**
+ * ice_is_generic_mac - check if device's mac_type is generic
+ * @hw: pointer to the hardware structure
+ *
+ * Return: true if mac_type is generic (with SBQ support), false if not
+ */
+bool ice_is_generic_mac(struct ice_hw *hw)
+{
+	return (hw->mac_type == ICE_MAC_GENERIC ||
+		hw->mac_type == ICE_MAC_GENERIC_3K_E825);
+}
+
 /**
  * ice_is_e810
  * @hw: pointer to the hardware structure
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 39be42ae3711..d0ef1fdd6493 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -111,6 +111,7 @@  ice_update_phy_type(u64 *phy_type_low, u64 *phy_type_high,
 int
 ice_aq_manage_mac_write(struct ice_hw *hw, const u8 *mac_addr, u8 flags,
 			struct ice_sq_cd *cd);
+bool ice_is_generic_mac(struct ice_hw *hw);
 bool ice_is_e810(struct ice_hw *hw);
 int ice_clear_pf_cfg(struct ice_hw *hw);
 int
diff --git a/drivers/net/ethernet/intel/ice/ice_controlq.c b/drivers/net/ethernet/intel/ice/ice_controlq.c
index e7d2474c431c..ffe660f34992 100644
--- a/drivers/net/ethernet/intel/ice/ice_controlq.c
+++ b/drivers/net/ethernet/intel/ice/ice_controlq.c
@@ -666,7 +666,7 @@  bool ice_is_sbq_supported(struct ice_hw *hw)
 	/* The device sideband queue is only supported on devices with the
 	 * generic MAC type.
 	 */
-	return hw->mac_type == ICE_MAC_GENERIC;
+	return ice_is_generic_mac(hw);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 0c3b64f65a0d..71e6d1252df5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1649,8 +1649,10 @@  static void ice_clean_sbq_subtask(struct ice_pf *pf)
 {
 	struct ice_hw *hw = &pf->hw;
 
-	/* Nothing to do here if sideband queue is not supported */
-	if (!ice_is_sbq_supported(hw)) {
+	/* if mac_type is not generic, sideband is not supported
+	 * and there's nothing to do here
+	 */
+	if (!ice_is_generic_mac(hw)) {
 		clear_bit(ICE_SIDEBANDQ_EVENT_PENDING, pf->state);
 		return;
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 1fff865d0661..935b1a3394de 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -132,6 +132,7 @@  enum ice_mac_type {
 	ICE_MAC_E810,
 	ICE_MAC_E830,
 	ICE_MAC_GENERIC,
+	ICE_MAC_GENERIC_3K_E825,
 };
 
 /* Media Types */