Dear Jacob,
Thank you for your patch. A small nit for the subject: The verb *clean
up* is spelled with a space.
Am 20.09.23 um 01:34 schrieb Jacob Keller:
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.fedore...@linux.dev/
[2]:
https://lore.kernel.org/intel-wired-lan/20230817000058.2433236-5-jacob.e.kel...@intel.com/
[3]:
https://lore.kernel.org/netdev/20230918212814.435688-1-anthony.l.ngu...@intel.com/
[4]:
https://lore.kernel.org/netdev/20230913204943.1051233-7-vadim.fedore...@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.
If you need to enumerate changes, with git it’s always good to do one
commit per item. ;-) (Better to bisect and to revert.) Anyway, not
important enough.
Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 30 ++++++++++-----------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 12c09374c2be..d4e259b816b9 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;
}
/**
Reviewed-by: Paul Menzel <pmen...@molgen.mpg.de>
Kind regards,
Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
