From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 682F043C27; Fri, 8 Mar 2024 10:17:48 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 44D1140298; Fri, 8 Mar 2024 10:17:48 +0100 (CET) Received: from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190]) by mails.dpdk.org (Postfix) with ESMTP id CA90440274 for ; Fri, 8 Mar 2024 10:17:45 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.163.44]) by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4TrgWk5XRSz1xqcB for ; Fri, 8 Mar 2024 17:16:02 +0800 (CST) Received: from kwepemm600004.china.huawei.com (unknown [7.193.23.242]) by mail.maildlp.com (Postfix) with ESMTPS id 923B5140135 for ; Fri, 8 Mar 2024 17:17:42 +0800 (CST) Received: from [10.67.121.59] (10.67.121.59) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Fri, 8 Mar 2024 17:17:42 +0800 Message-ID: <5b53334e-fafb-2171-e32e-e3d0424e393b@huawei.com> Date: Fri, 8 Mar 2024 17:17:41 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH v5 4/7] net/hns3: remove dump format of registers To: Jie Hai CC: , Yisen Zhuang , References: <20231214015650.3738578-1-haijie1@huawei.com> <20240307030247.599394-1-haijie1@huawei.com> <20240307030247.599394-5-haijie1@huawei.com> From: "lihuisong (C)" In-Reply-To: <20240307030247.599394-5-haijie1@huawei.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemm600004.china.huawei.com (7.193.23.242) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Jie, A few trivial comments inline, after changed, you can add Reviewed-by: Huisong Li 在 2024/3/7 11:02, Jie Hai 写道: > Since the driver is going to support reporting names of > all registers, remove the counter and insert of separators > between different register modules. > > Signed-off-by: Jie Hai > --- > drivers/net/hns3/hns3_regs.c | 67 ++++++++++-------------------------- > 1 file changed, 18 insertions(+), 49 deletions(-) > > diff --git a/drivers/net/hns3/hns3_regs.c b/drivers/net/hns3/hns3_regs.c > index d77170481a3d..b1c0d538a3c8 100644 > --- a/drivers/net/hns3/hns3_regs.c > +++ b/drivers/net/hns3/hns3_regs.c > @@ -10,12 +10,9 @@ > #include "hns3_rxtx.h" > #include "hns3_regs.h" > > -#define MAX_SEPARATE_NUM 4 > -#define SEPARATOR_VALUE 0xFFFFFFFF > -#define REG_NUM_PER_LINE 4 > -#define REG_LEN_PER_LINE (REG_NUM_PER_LINE * sizeof(uint32_t)) > +#define HNS3_64_BIT_REG_SIZE (sizeof(uint64_t) / sizeof(uint32_t)) how about HNS3_64_BIT_REG_OUTPUT_SIZE? > > -static int hns3_get_dfx_reg_line(struct hns3_hw *hw, uint32_t *lines); > +static int hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count); > > static const uint32_t cmdq_reg_addrs[] = {HNS3_CMDQ_TX_ADDR_L_REG, > HNS3_CMDQ_TX_ADDR_H_REG, > @@ -119,23 +116,22 @@ static int > hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length) > { > struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw); > - uint32_t cmdq_lines, common_lines, ring_lines, tqp_intr_lines; > + uint32_t cmdq_cnt, common_cnt, ring_cnt, tqp_intr_cnt; > uint32_t regs_num_32_bit, regs_num_64_bit; > - uint32_t dfx_reg_lines; > + uint32_t dfx_reg_cnt; > uint32_t len; > int ret; > > - cmdq_lines = sizeof(cmdq_reg_addrs) / REG_LEN_PER_LINE + 1; > + cmdq_cnt = sizeof(cmdq_reg_addrs); > if (hns->is_vf) > - common_lines = > - sizeof(common_vf_reg_addrs) / REG_LEN_PER_LINE + 1; > + common_cnt = sizeof(common_vf_reg_addrs); > else > - common_lines = sizeof(common_reg_addrs) / REG_LEN_PER_LINE + 1; > - ring_lines = sizeof(ring_reg_addrs) / REG_LEN_PER_LINE + 1; > - tqp_intr_lines = sizeof(tqp_intr_reg_addrs) / REG_LEN_PER_LINE + 1; > + common_cnt = sizeof(common_reg_addrs); > + ring_cnt = sizeof(ring_reg_addrs); > + tqp_intr_cnt = sizeof(tqp_intr_reg_addrs); > > - len = (cmdq_lines + common_lines + ring_lines * hw->tqps_num + > - tqp_intr_lines * hw->intr_tqps_num) * REG_NUM_PER_LINE; > + len = cmdq_cnt + common_cnt + ring_cnt * hw->tqps_num + > + tqp_intr_cnt * hw->intr_tqps_num; remove these local variable like cmdq_cnt and comon_cnt and directly plus all sizeof()? > > if (!hns->is_vf) { > ret = hns3_get_regs_num(hw, ®s_num_32_bit, ®s_num_64_bit); > @@ -144,18 +140,16 @@ hns3_get_regs_length(struct hns3_hw *hw, uint32_t *length) > "ret = %d.", ret); > return ret; > } > - dfx_reg_lines = regs_num_32_bit * sizeof(uint32_t) / > - REG_LEN_PER_LINE + 1; > - dfx_reg_lines += regs_num_64_bit * sizeof(uint64_t) / > - REG_LEN_PER_LINE + 1; > + dfx_reg_cnt = regs_num_32_bit + > + regs_num_64_bit * HNS3_64_BIT_REG_SIZE; > > - ret = hns3_get_dfx_reg_line(hw, &dfx_reg_lines); > + ret = hns3_get_dfx_reg_cnt(hw, &dfx_reg_cnt); > if (ret) { > hns3_err(hw, "fail to get the number of dfx registers, " > "ret = %d.", ret); > return ret; > } > - len += dfx_reg_lines * REG_NUM_PER_LINE; > + len += dfx_reg_cnt; > } > > *length = len; > @@ -276,18 +270,6 @@ hns3_get_64_bit_regs(struct hns3_hw *hw, uint32_t regs_num, void *data) > return 0; > } > > -static int > -hns3_insert_reg_separator(int reg_num, uint32_t *data) > -{ > - int separator_num; > - int i; > - > - separator_num = MAX_SEPARATE_NUM - reg_num % REG_NUM_PER_LINE; > - for (i = 0; i < separator_num; i++) > - *data++ = SEPARATOR_VALUE; > - return separator_num; > -} > - > static int > hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data) > { > @@ -302,7 +284,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data) > reg_num = sizeof(cmdq_reg_addrs) / sizeof(uint32_t); > for (i = 0; i < reg_num; i++) > *data++ = hns3_read_dev(hw, cmdq_reg_addrs[i]); > - data += hns3_insert_reg_separator(reg_num, data); > > if (hns->is_vf) > reg_num = sizeof(common_vf_reg_addrs) / sizeof(uint32_t); > @@ -313,7 +294,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data) > *data++ = hns3_read_dev(hw, common_vf_reg_addrs[i]); > else > *data++ = hns3_read_dev(hw, common_reg_addrs[i]); > - data += hns3_insert_reg_separator(reg_num, data); > > reg_num = sizeof(ring_reg_addrs) / sizeof(uint32_t); > for (j = 0; j < hw->tqps_num; j++) { > @@ -321,7 +301,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data) > for (i = 0; i < reg_num; i++) > *data++ = hns3_read_dev(hw, > ring_reg_addrs[i] + reg_offset); > - data += hns3_insert_reg_separator(reg_num, data); > } > > reg_num = sizeof(tqp_intr_reg_addrs) / sizeof(uint32_t); > @@ -330,7 +309,6 @@ hns3_direct_access_regs(struct hns3_hw *hw, uint32_t *data) > for (i = 0; i < reg_num; i++) > *data++ = hns3_read_dev(hw, tqp_intr_reg_addrs[i] + > reg_offset); > - data += hns3_insert_reg_separator(reg_num, data); > } > return data - origin_data_ptr; > } > @@ -406,17 +384,15 @@ hns3_dfx_reg_fetch_data(struct hns3_cmd_desc *desc, int bd_num, uint32_t *reg) > index = i % HNS3_CMD_DESC_DATA_NUM; > *reg++ = desc[desc_index].data[index]; > } > - reg_num += hns3_insert_reg_separator(reg_num, reg); > > return reg_num; > } > > static int > -hns3_get_dfx_reg_line(struct hns3_hw *hw, uint32_t *lines) > +hns3_get_dfx_reg_cnt(struct hns3_hw *hw, uint32_t *count) > { > int opcode_num = RTE_DIM(hns3_dfx_reg_opcode_list); > uint32_t bd_num_list[opcode_num]; > - uint32_t bd_num, data_len; > int ret; > int i; > > @@ -424,11 +400,8 @@ hns3_get_dfx_reg_line(struct hns3_hw *hw, uint32_t *lines) hns3_get_dfx_reg_line --> hns3_get_dfx_reg_num? > if (ret) > return ret; > > - for (i = 0; i < opcode_num; i++) { > - bd_num = bd_num_list[i]; > - data_len = bd_num * HNS3_CMD_DESC_DATA_NUM * sizeof(uint32_t); > - *lines += data_len / REG_LEN_PER_LINE + 1; > - } > + for (i = 0; i < opcode_num; i++) > + *count += bd_num_list[i] * HNS3_CMD_DESC_DATA_NUM; > > return 0; > } > @@ -475,7 +448,6 @@ hns3_get_dfx_regs(struct hns3_hw *hw, void **data) > int > hns3_get_regs(struct rte_eth_dev *eth_dev, struct rte_dev_reg_info *regs) > { > -#define HNS3_64_BIT_REG_SIZE (sizeof(uint64_t) / sizeof(uint32_t)) > struct hns3_adapter *hns = eth_dev->data->dev_private; > struct hns3_hw *hw = &hns->hw; > uint32_t regs_num_32_bit; > @@ -520,7 +492,6 @@ hns3_get_regs(struct rte_eth_dev *eth_dev, struct rte_dev_reg_info *regs) > return ret; > } > data += regs_num_32_bit; > - data += hns3_insert_reg_separator(regs_num_32_bit, data); > > ret = hns3_get_64_bit_regs(hw, regs_num_64_bit, data); > if (ret) { > @@ -528,8 +499,6 @@ hns3_get_regs(struct rte_eth_dev *eth_dev, struct rte_dev_reg_info *regs) > return ret; > } > data += regs_num_64_bit * HNS3_64_BIT_REG_SIZE; > - data += hns3_insert_reg_separator(regs_num_64_bit * > - HNS3_64_BIT_REG_SIZE, data); > > return hns3_get_dfx_regs(hw, (void **)&data); > }