diff mbox series

[net-next,09/11] ice: cleanup ice_find_netlist_node

Message ID 20231019173227.3175575-10-jacob.e.keller@intel.com (mailing list archive)
State Accepted
Commit 640a65f801338940ed86992179558eb070db75de
Delegated to: Netdev Maintainers
Headers show
Series Intel Wired LAN Driver Updates 2023-10-19 (ice, igb, ixgbe) | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/apply success Patch already applied to net-next

Commit Message

Jacob Keller Oct. 19, 2023, 5:32 p.m. UTC
The ice_find_netlist_node function was introduced in commit 8a3a565ff210
("ice: add admin commands to access cgu configuration"). Variations of this
function were reviewed concurrently on both intel-wired-lan[1][2], and
netdev [3][4]

[1]: https://lore.kernel.org/intel-wired-lan/20230913204943.1051233-7-vadim.fedorenko@linux.dev/
[2]: https://lore.kernel.org/intel-wired-lan/20230817000058.2433236-5-jacob.e.keller@intel.com/
[3]: https://lore.kernel.org/netdev/20230918212814.435688-1-anthony.l.nguyen@intel.com/
[4]: https://lore.kernel.org/netdev/20230913204943.1051233-7-vadim.fedorenko@linux.dev/

The variant I posted had a few changes due to review feedback which were
never incorporated into the DPLL series:

* Replace the references to ancient and long removed ICE_SUCCESS and
  ICE_ERR_DOES_NOT_EXIST status codes in the function comment.

* Return -ENOENT instead of -ENOTBLK, as a more common way to indicate that
  an entry doesn't exist.

* Avoid the use of memset() and use simple static initialization for the
  cmd variable.

* Use FIELD_PREP to assign the node_type_ctx.

* Remove an unnecessary local variable to keep track of rec_node_handle,
  just pass the node_handle pointer directly into ice_aq_get_netlist_node.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/net/ethernet/intel/ice/ice_common.c | 30 ++++++++++-----------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Przemek Kitszel Oct. 19, 2023, 8:37 p.m. UTC | #1
On 10/19/23 19:32, Jacob Keller wrote:
> The ice_find_netlist_node function was introduced in commit 8a3a565ff210
> ("ice: add admin commands to access cgu configuration"). Variations of this
> function were reviewed concurrently on both intel-wired-lan[1][2], and
> netdev [3][4]
> 
> [1]: https://lore.kernel.org/intel-wired-lan/20230913204943.1051233-7-vadim.fedorenko@linux.dev/
> [2]: https://lore.kernel.org/intel-wired-lan/20230817000058.2433236-5-jacob.e.keller@intel.com/
> [3]: https://lore.kernel.org/netdev/20230918212814.435688-1-anthony.l.nguyen@intel.com/
> [4]: https://lore.kernel.org/netdev/20230913204943.1051233-7-vadim.fedorenko@linux.dev/
> 
> The variant I posted had a few changes due to review feedback which were
> never incorporated into the DPLL series:
> 
> * Replace the references to ancient and long removed ICE_SUCCESS and
>    ICE_ERR_DOES_NOT_EXIST status codes in the function comment.
> 
> * Return -ENOENT instead of -ENOTBLK, as a more common way to indicate that
>    an entry doesn't exist.
> 
> * Avoid the use of memset() and use simple static initialization for the
>    cmd variable.
> 
> * Use FIELD_PREP to assign the node_type_ctx.
> 
> * Remove an unnecessary local variable to keep track of rec_node_handle,
>    just pass the node_handle pointer directly into ice_aq_get_netlist_node.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>   drivers/net/ethernet/intel/ice/ice_common.c | 30 ++++++++++-----------
>   1 file changed, 15 insertions(+), 15 deletions(-)
> 

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
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 8cbe63401378..377fae41bbae 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -473,41 +473,41 @@  ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo *cmd,
  * @node_part_number: node part number to look for
  * @node_handle: output parameter if node found - optional
  *
- * Find and return the node handle for a given node type and part number in the
- * netlist. When found ICE_SUCCESS is returned, ICE_ERR_DOES_NOT_EXIST
- * otherwise. If node_handle provided, it would be set to found node handle.
+ * Scan the netlist for a node handle of the given node type and part number.
+ *
+ * If node_handle is non-NULL it will be modified on function exit. It is only
+ * valid if the function returns zero, and should be ignored on any non-zero
+ * return value.
+ *
+ * Returns: 0 if the node is found, -ENOENT if no handle was found, and
+ * a negative error code on failure to access the AQ.
  */
 static int ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx,
 				 u8 node_part_number, u16 *node_handle)
 {
-	struct ice_aqc_get_link_topo cmd;
-	u8 rec_node_part_number;
-	u16 rec_node_handle;
 	u8 idx;
 
 	for (idx = 0; idx < ICE_MAX_NETLIST_SIZE; idx++) {
+		struct ice_aqc_get_link_topo cmd = {};
+		u8 rec_node_part_number;
 		int status;
 
-		memset(&cmd, 0, sizeof(cmd));
-
 		cmd.addr.topo_params.node_type_ctx =
-			(node_type_ctx << ICE_AQC_LINK_TOPO_NODE_TYPE_S);
+			FIELD_PREP(ICE_AQC_LINK_TOPO_NODE_TYPE_M,
+				   node_type_ctx);
 		cmd.addr.topo_params.index = idx;
 
 		status = ice_aq_get_netlist_node(hw, &cmd,
 						 &rec_node_part_number,
-						 &rec_node_handle);
+						 node_handle);
 		if (status)
 			return status;
 
-		if (rec_node_part_number == node_part_number) {
-			if (node_handle)
-				*node_handle = rec_node_handle;
+		if (rec_node_part_number == node_part_number)
 			return 0;
-		}
 	}
 
-	return -ENOTBLK;
+	return -ENOENT;
 }
 
 /**