DPDK patches and discussions
 help / color / mirror / Atom feed
From: "lihuisong (C)" <lihuisong@huawei.com>
To: Jie Hai <haijie1@huawei.com>
Cc: <fengchengwen@huawei.com>, Yisen Zhuang <yisen.zhuang@huawei.com>,
	<dev@dpdk.org>
Subject: Re: [PATCH v5 4/7] net/hns3: remove dump format of registers
Date: Fri, 8 Mar 2024 17:17:41 +0800	[thread overview]
Message-ID: <5b53334e-fafb-2171-e32e-e3d0424e393b@huawei.com> (raw)
In-Reply-To: <20240307030247.599394-5-haijie1@huawei.com>

Hi Jie,

A few trivial comments inline,
after changed, you can add
Reviewed-by: Huisong Li <lihuisong@huawei.com>


在 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 <haijie1@huawei.com>
> ---
>   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, &regs_num_32_bit, &regs_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);
>   }

  reply	other threads:[~2024-03-08  9:17 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14  1:56 [PATCH] ethdev: add dump regs for telemetry Jie Hai
2023-12-14 12:49 ` Ferruh Yigit
2024-01-09  2:19   ` Jie Hai
2024-01-09  2:41     ` Jie Hai
2024-01-09 18:06     ` Ferruh Yigit
2024-01-10  1:38       ` fengchengwen
2024-01-10 12:15         ` Ferruh Yigit
2024-01-10 14:09           ` Thomas Monjalon
2024-01-10 15:48             ` Ferruh Yigit
2024-01-11  1:55           ` fengchengwen
2024-01-11 11:11             ` Ferruh Yigit
2024-01-11 12:43               ` fengchengwen
2024-02-05 10:51 ` [PATCH v2 0/7] support dump reigser names and filter them Jie Hai
2024-02-05 10:51   ` [PATCH v2 1/7] ethdev: support report register names and filter Jie Hai
2024-02-07 17:00     ` Ferruh Yigit
2024-02-20  8:43       ` Jie Hai
2024-02-05 10:51   ` [PATCH v2 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-02-07 17:03     ` Ferruh Yigit
2024-02-22  9:01       ` Jie Hai
2024-02-05 10:51   ` [PATCH v2 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-02-05 10:51   ` [PATCH v2 4/7] net/hns3: remove dump format " Jie Hai
2024-02-05 10:51   ` [PATCH v2 5/7] net/hns3: add names for registers Jie Hai
2024-02-05 10:51   ` [PATCH v2 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-02-05 10:51   ` [PATCH v2 7/7] net/hns3: support filter dump of registers Jie Hai
2024-02-20 10:58 ` [PATCH v3 0/7] support dump reigser names and filter them Jie Hai
2024-02-20 10:58   ` [PATCH v3 1/7] ethdev: support report register names and filter Jie Hai
2024-02-20 15:09     ` Stephen Hemminger
2024-02-26  2:33       ` Jie Hai
2024-02-20 15:13     ` Stephen Hemminger
2024-02-26  2:41       ` Jie Hai
2024-02-20 15:14     ` Stephen Hemminger
2024-02-26  2:57       ` Jie Hai
2024-02-20 15:14     ` Stephen Hemminger
2024-02-26  2:33       ` Jie Hai
2024-02-20 10:58   ` [PATCH v3 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-02-20 10:58   ` [PATCH v3 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-02-20 10:58   ` [PATCH v3 4/7] net/hns3: remove dump format " Jie Hai
2024-02-20 10:58   ` [PATCH v3 5/7] net/hns3: add names for registers Jie Hai
2024-02-20 10:58   ` [PATCH v3 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-02-20 10:58   ` [PATCH v3 7/7] net/hns3: support filter dump of registers Jie Hai
2024-02-26  3:07 ` [PATCH v4 0/7] support dump reigser names and filter them Jie Hai
2024-02-26  3:07   ` [PATCH v4 1/7] ethdev: support report register names and filter Jie Hai
2024-02-26  8:01     ` fengchengwen
2024-03-06  7:22       ` Jie Hai
2024-02-29  9:52     ` Thomas Monjalon
2024-03-05  7:45       ` Jie Hai
2024-02-26  3:07   ` [PATCH v4 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-02-26  9:09     ` fengchengwen
2024-03-06  7:18       ` Jie Hai
2024-02-26  3:07   ` [PATCH v4 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-02-26  3:07   ` [PATCH v4 4/7] net/hns3: remove dump format " Jie Hai
2024-02-26  3:07   ` [PATCH v4 5/7] net/hns3: add names for registers Jie Hai
2024-02-26  3:07   ` [PATCH v4 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-02-26  3:07   ` [PATCH v4 7/7] net/hns3: support filter dump of registers Jie Hai
2024-03-07  3:02 ` [PATCH v5 0/7] support dump reigser names and filter them Jie Hai
2024-03-07  3:02   ` [PATCH v5 1/7] ethdev: support report register names and filter Jie Hai
2024-03-08  8:09     ` lihuisong (C)
2024-03-07  3:02   ` [PATCH v5 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-03-08  8:48     ` lihuisong (C)
2024-03-07  3:02   ` [PATCH v5 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-03-08  8:49     ` lihuisong (C)
2024-03-07  3:02   ` [PATCH v5 4/7] net/hns3: remove dump format " Jie Hai
2024-03-08  9:17     ` lihuisong (C) [this message]
2024-03-07  3:02   ` [PATCH v5 5/7] net/hns3: add names for registers Jie Hai
2024-03-08  9:41     ` lihuisong (C)
2024-03-08 10:24     ` lihuisong (C)
2024-03-07  3:02   ` [PATCH v5 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-03-08  9:41     ` lihuisong (C)
2024-03-07  3:02   ` [PATCH v5 7/7] net/hns3: support filter dump of registers Jie Hai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b53334e-fafb-2171-e32e-e3d0424e393b@huawei.com \
    --to=lihuisong@huawei.com \
    --cc=dev@dpdk.org \
    --cc=fengchengwen@huawei.com \
    --cc=haijie1@huawei.com \
    --cc=yisen.zhuang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).