Re: [PATCH net-next v01 1/9] hinic3: Add PF framework
From: Pavan Chebbi <pavan.chebbi@broadcom.com>
Date: 2025-10-15 10:29:52
Also in:
linux-doc, lkml
On Wed, Oct 15, 2025 at 12:48 PM Fan Gong [off-list ref] wrote:
quoted hunk ↗ jump to hunk
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c@@ -314,6 +314,9 @@ int hinic3_func_rx_tx_flush(struct hinic3_hwdev *hwdev) ret = -EFAULT; } + if (HINIC3_FUNC_TYPE(hwdev) != HINIC3_FUNC_TYPE_VF)
Is this check needed? Other places hinic3_set_pf_status() is called without checking. I see the function has an internal check for PF.
quoted hunk ↗ jump to hunk
+ hinic3_set_pf_status(hwif, HINIC3_PF_STATUS_FLR_START_FLAG); + clr_res.func_id = hwif->attr.func_global_idx; msg_params.buf_in = &clr_res; msg_params.in_size = sizeof(clr_res);@@ -337,6 +340,54 @@ int hinic3_func_rx_tx_flush(struct hinic3_hwdev *hwdev) return ret; } +int hinic3_set_bdf_ctxt(struct hinic3_hwdev *hwdev, u8 bus, u8 device, u8 function)
You could just send *bdf_info as a parameter with values pre-set, makes it look better
+{
+ struct comm_cmd_bdf_info bdf_info = {};
+ struct mgmt_msg_params msg_params = {};
+ int err;
+
+ bdf_info.function_idx = hinic3_global_func_id(hwdev);
+ bdf_info.bus = bus;
+ bdf_info.device = device;
+ bdf_info.function = function;
+
+ mgmt_msg_params_init_default(&msg_params, &bdf_info, sizeof(bdf_info));
+
+ err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_COMM,
+ COMM_CMD_SEND_BDF_INFO, &msg_params);
+ if (err || bdf_info.head.status) {
+ dev_err(hwdev->dev,
+ "Failed to set bdf info to fw, err: %d, status: 0x%x\n",
+ err, bdf_info.head.status);
+ return -EIO;
+ }
+
+ return 0;
+}<-->
+static void set_ppf(struct hinic3_hwdev *hwdev)
Any particular reason why this function does not have hinic3 prefix?
+{
+ struct hinic3_hwif *hwif = hwdev->hwif;
+ struct hinic3_func_attr *attr;
+ u32 addr, val;
+
+ if (HINIC3_IS_VF(hwdev))
+ return;
+
+ /* Read Modify Write */
+ attr = &hwif->attr;
+ addr = HINIC3_CSR_PPF_ELECTION_ADDR;
+ val = hinic3_hwif_read_reg(hwif, addr);
+ val &= ~HINIC3_PPF_ELECTION_IDX_MASK;
+ val |= HINIC3_PPF_ELECTION_SET(attr->func_global_idx, IDX);
+ hinic3_hwif_write_reg(hwif, addr, val);
+
+ /* Check PPF index */
+ val = hinic3_hwif_read_reg(hwif, addr);
+ attr->ppf_idx = HINIC3_PPF_ELECTION_GET(val, IDX);
+}
+<-->
+int hinic3_get_default_mac(struct hinic3_hwdev *hwdev, u8 *mac_addr)
+{
+ struct l2nic_cmd_set_mac mac_info = {};
+ struct mgmt_msg_params msg_params = {};
+ int err;
+
+ mac_info.func_id = hinic3_global_func_id(hwdev);
+
+ mgmt_msg_params_init_default(&msg_params, &mac_info, sizeof(mac_info));
+
+ err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_L2NIC,
+ L2NIC_CMD_GET_MAC,
+ &msg_params);
+
+ if (err || mac_info.msg_head.status) {
+ dev_err(hwdev->dev,
+ "Failed to get mac, err: %d, status: 0x%x\n",
+ err, mac_info.msg_head.status);
+ return -EINVAL;EINVAL looks odd. I see other places it is -EIO.
quoted hunk ↗ jump to hunk
+ } + + ether_addr_copy(mac_addr, mac_info.mac); + + return 0; +} + int hinic3_set_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id, u16 func_id) {@@ -157,9 +189,9 @@ int hinic3_set_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id, return -EIO; } - if (mac_info.msg_head.status == MGMT_STATUS_PF_SET_VF_ALREADY) { + if (PF_SET_VF_MAC(hwdev, mac_info.msg_head.status)) { dev_warn(hwdev->dev, "PF has already set VF mac, Ignore set operation\n"); - return 0; + return HINIC3_PF_SET_VF_ALREADY; } if (mac_info.msg_head.status == MGMT_STATUS_EXIST) {@@ -191,11 +223,17 @@ int hinic3_del_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id, err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_L2NIC, L2NIC_CMD_DEL_MAC, &msg_params); - if (err) { + if (err || (mac_info.msg_head.status && + !PF_SET_VF_MAC(hwdev, mac_info.msg_head.status))) { dev_err(hwdev->dev, "Failed to delete MAC, err: %d, status: 0x%x\n", err, mac_info.msg_head.status); - return err; + return -EIO; + } + + if (PF_SET_VF_MAC(hwdev, mac_info.msg_head.status)) { + dev_warn(hwdev->dev, "PF has already set VF mac, Ignore delete operation.\n"); + return HINIC3_PF_SET_VF_ALREADY; } return 0;@@ -231,6 +269,17 @@ int hinic3_update_mac(struct hinic3_hwdev *hwdev, const u8 *old_mac, return -EIO; } + if (PF_SET_VF_MAC(hwdev, mac_info.msg_head.status)) { + dev_warn(hwdev->dev, "PF has already set VF MAC. Ignore update operation\n"); + return HINIC3_PF_SET_VF_ALREADY; + } + + if (mac_info.msg_head.status == HINIC3_MGMT_STATUS_EXIST) { + dev_warn(hwdev->dev, + "MAC is repeated. Ignore update operation\n"); + return 0; + } + return 0; }diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h index b83b567fa542..08bf14679bf8 100644 --- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h@@ -16,6 +16,8 @@ struct hinic3_nic_dev; #define HINIC3_MAX_JUMBO_FRAME_SIZE 9600 #define HINIC3_VLAN_ID_MASK 0x7FFF +#define HINIC3_PF_SET_VF_ALREADY 0x4 +#define HINIC3_MGMT_STATUS_EXIST 0x6 enum hinic3_nic_event_type { HINIC3_NIC_EVENT_LINK_DOWN = 0,@@ -41,6 +43,7 @@ void hinic3_update_nic_feature(struct hinic3_nic_dev *nic_dev, u64 feature_cap); int hinic3_init_function_table(struct hinic3_nic_dev *nic_dev); int hinic3_set_port_mtu(struct net_device *netdev, u16 new_mtu); +int hinic3_get_default_mac(struct hinic3_hwdev *hwdev, u8 *mac_addr); int hinic3_set_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id, u16 func_id); int hinic3_del_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id, --2.43.0
Attachments
- smime.p7s [application/pkcs7-signature] 5469 bytes