Re: [v4,net-next,06/11] net/nebula-matrix: add common resource implementation

This is an AI-generated review of your patch. The human sending this
email says "This is an automated AI code reviewer, please treat what
it says with a grain of salt."

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/nebula-matrix: add common resource implementation

This patch adds the common resource layer for the nebula-matrix network
driver, implementing conversion relationships among vsi_id, func_id,
eth_id, and pf_id.

[ ... ]

+static void nbl_res_get_eth_id(struct nbl_resource_mgt *res_mgt, u16 vsi_id,
+			       u8 *eth_mode, u8 *eth_id, u8 *logic_eth_id)
+{
+	struct nbl_eth_info *eth_info = NBL_RES_MGT_TO_ETH_INFO(res_mgt);
+	u16 pf_id = nbl_res_vsi_id_to_pf_id(res_mgt, vsi_id);
+
+	*eth_mode = eth_info->eth_num;
+	if (pf_id < eth_info->eth_num) {
+		*eth_id = eth_info->eth_id[pf_id];
+		*logic_eth_id = pf_id;
+		/* if pf_id > eth_num, use eth_id 0 */
+	} else {

This isn't a bug, but the comment appears to be misplaced. It describes
what happens in the else branch but is located inside the if branch. Should
it be moved above the else?

[ ... ]

+static int nbl_res_ctrl_dev_setup_eth_info(struct nbl_resource_mgt *res_mgt)
+{

[ ... ]

+	for (i = 0; i < NBL_RES_MGT_TO_PF_NUM(res_mgt); i++) {
+		/* if pf_id <= eth_num, the pf relate corresponding eth_id*/
+		if (i < eth_num) {

This isn't a bug, but the comment says "pf_id <= eth_num" while the code
checks "i < eth_num". Should the comment say "<" instead of "<="?

+			eth_id = eth_info->eth_id[i];
+			eth_info->pf_bitmap[eth_id] |= BIT(i);
+		}
+		/* if pf_id > eth_num, the pf relate eth 0*/
+		else

Similarly, the comment says "pf_id > eth_num" but the else handles the case
where "i >= eth_num". Should the comment say ">=" instead of ">"?

+			eth_info->pf_bitmap[0] |= BIT(i);
+	}

[ ... ]

[ ... ]

+static int vsi_id_to_pf_id(struct nbl_resource_mgt *res_mgt, u16 vsi_id)
+{
+	struct nbl_vsi_info *vsi_info = NBL_RES_MGT_TO_VSI_INFO(res_mgt);
+	struct nbl_common_info *common = res_mgt->common;
+	bool vsi_find = false;
+	u32 pf_id = U32_MAX;
+	int i, j;
+
+	for (i = 0; i < vsi_info->num; i++) {
+		for (j = 0; j < NBL_VSI_SERV_MAX_TYPE; j++)
+			if (vsi_id >= vsi_info->serv_info[i][j].base_id &&
+			    (vsi_id < vsi_info->serv_info[i][j].base_id +
+					      vsi_info->serv_info[i][j].num)) {
+				vsi_find = true;
+				break;
+			}
+
+		if (vsi_find)
+			break;
+	}
+
+	if (vsi_find) {
+		/* if pf_id < eth_num */
+		if (j >= NBL_VSI_SERV_PF_DATA_TYPE &&
+		    j <= NBL_VSI_SERV_PF_DATA_TYPE)

This isn't a bug, but the comment says "if pf_id < eth_num" while the code
checks if j equals NBL_VSI_SERV_PF_DATA_TYPE (which is 0). The comment does
not seem to describe the actual condition being tested. Could it be updated
to reflect what the code is checking?

+			pf_id = i + common->mgt_pf;
+	}
+
+	return pf_id;
+}