Re: [PATCH v8 8/8] net/hns3: support filter registers by module names

[Jie Hai]

Hi, fengchengwen,

Thanks for your review.
On 2024/9/26 9:46, fengchengwen wrote:
On 2024/9/25 14:40, Jie Hai wrote:
This patch support dumping registers which module name is the
`filter` string. The module names are in lower case and so is
the `filter`. Available module names are cmdq, common_pf,
common_vf, ring, tqp_intr, 32_bit_dfx, 64_bit_dfx, bios, igu_egu,
ssu, ppp, rpu, ncsi, rtc, rcb, etc.

Signed-off-by: Jie Hai<haij...@huawei.com>
---
  doc/guides/nics/hns3.rst     |   7 +
  drivers/net/hns3/hns3_regs.c | 257 ++++++++++++++++++++---------------
  2 files changed, 157 insertions(+), 107 deletions(-)

diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
index 97b4686bfabe..20765bd7d145 100644
--- a/doc/guides/nics/hns3.rst
+++ b/doc/guides/nics/hns3.rst
@@ -407,6 +407,13 @@ be provided to avoid scheduling the CPU core used by DPDK 
application threads fo
  other tasks. Before starting the Linux OS, add the kernel isolation boot 
parameter.
  For example, "isolcpus=1-18 nohz_full=1-18 rcu_nocbs=1-18".
  
+Dump registers
+--------------
+HNS3 supports dumping registers values with their names, and suppotrt filtering
typo of suppotrt

Will correct in V9.
+by module names. The available modules are ``bios``, ``ssu``, ``igu_egu``,
The available modules names?

+``rpu``, ``ncsi``, ``rtc``, ``ppp``, ``rcb``, ``tqp``, ``rtc``, ``cmdq``,
+``common_pf``, ``common_vf``, ``ring``, ``tqp_intr``, ``32_bit_dfx``,
+``64_bit_dfx``, etc.
etc means there are more which don't list above, please don't add it because 
the doc should contain all modules names.

Will remove 'etc' in V9.
  
  Limitations or Known issues
  ---------------------------

+static void
+hns3_get_module_names(char *names, uint32_t len)
+{
+       size_t i;
+
+       for (i = 0; i < RTE_DIM(hns3_module_name_map); i++) {
+               strlcat(names, " ", len);
+               strlcat(names, hns3_module_name_map[i].name, len);
+       }
+}
+
+static uint32_t
+hns3_parse_modules_by_filter(struct hns3_hw *hw, const char *filter)
+{
+       struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+       char names[HNS3_MAX_MODULES_LEN] = {0};
+       uint32_t modules = 0;
+       size_t i;
+
+       if (filter == NULL) {
+               modules = (1 << RTE_DIM(hns3_reg_lists)) - 1;
+       } else {
+               for (i = 0; i < RTE_DIM(hns3_module_name_map); i++) {
+                       if (strcmp(filter, hns3_module_name_map[i].name) == 0) {
+                               modules |= hns3_module_name_map[i].module;
+                               break;
+                       }
+               }
+       }
+
+       if (hns->is_vf)
+               modules &= HNS3_VF_MODULES;
+       else
+               modules &= ~HNS3_VF_ONLY_MODULES;
+       if (modules == 0) {
+               hns3_get_module_names(names, HNS3_MAX_MODULES_LEN);
the name is NULL.

I am not sure what problem it is.
You mean the parameter names of hns3_get_module_names?
It is an array and not null for sure. And it is used for the names of 
all modules.
+               hns3_err(hw, "mismatched module name! Available names 
are:\n%s.",
hns3_err already contain \n, please remove this.
This is a new line between the above expression and the name list
to prevent the overall output from being too long.
Not the last of the expression.

+                        names);
+       }
+       return modules;
  }