This patch initializes debugfs and creates root directory
for each device. The tx_ring and rx_ring debugfs files
are implemented together.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
ChangeLog:
v5 -> v6:
  - Drop debugfs_create_devm_dir() helper, suggested by Greg KH.
  v5: https://lore.kernel.org/all/20241206111629.3521865-1-shaojijie@huawei.com/
v4 -> v5:
  - Use debugfs_create_devm_dir() helper, suggested by Jakub.
  v4: https://lore.kernel.org/all/20241203150131.3139399-2-shaojijie@huawei.com/
v1 -> v2:
  - Remove debugfs file 'dev_specs' because the dump register
    does the same thing, suggested by Andrew.
  - Move 'tx timeout cnt' from debugfs to ethtool -S, suggested by Andrew.
  - Add a new patch for debugfs file 'irq_info', suggested by Andrew.
  - Ignore the error code of the debugfs initialization failure, suggested by Andrew.
  v1: https://lore.kernel.org/all/20241023134213.3359092-3-shaojijie@huawei.com/
---
 .../net/ethernet/hisilicon/hibmcge/Makefile   |  3 +-
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  | 95 +++++++++++++++++++
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.h  | 12 +++
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 29 +++++-
 4 files changed, 136 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/Makefile b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
index ae58ac38c206..1a0ec2fb8c24 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/Makefile
+++ b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
@@ -5,4 +5,5 @@
 
 obj-$(CONFIG_HIBMCGE) += hibmcge.o
 
-hibmcge-objs = hbg_main.o hbg_hw.o hbg_mdio.o hbg_irq.o hbg_txrx.o hbg_ethtool.o
+hibmcge-objs = hbg_main.o hbg_hw.o hbg_mdio.o hbg_irq.o hbg_txrx.o hbg_ethtool.o \
+		hbg_debugfs.o
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
new file mode 100644
index 000000000000..9c0b2c7231fe
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/seq_file.h>
+#include "hbg_common.h"
+#include "hbg_debugfs.h"
+#include "hbg_hw.h"
+#include "hbg_irq.h"
+#include "hbg_txrx.h"
+
+static struct dentry *hbg_dbgfs_root;
+
+struct hbg_dbg_info {
+	const char *name;
+	int (*read)(struct seq_file *seq, void *data);
+};
+
+#define hbg_get_bool_str(state) ((state) ? "true" : "false")
+
+static void hbg_dbg_ring(struct hbg_priv *priv, struct hbg_ring *ring,
+			 struct seq_file *s)
+{
+	u32 irq_mask = ring->dir == HBG_DIR_TX ? HBG_INT_MSK_TX_B :
+						 HBG_INT_MSK_RX_B;
+
+	seq_printf(s, "ring used num: %u\n",
+		   hbg_get_queue_used_num(ring));
+	seq_printf(s, "ring max num: %u\n", ring->len);
+	seq_printf(s, "ring head: %u, tail: %u\n", ring->head, ring->tail);
+	seq_printf(s, "fifo used num: %u\n",
+		   hbg_hw_get_fifo_used_num(priv, ring->dir));
+	seq_printf(s, "fifo max num: %u\n",
+		   hbg_get_spec_fifo_max_num(priv, ring->dir));
+	seq_printf(s, "irq enabled: %s\n",
+		   hbg_get_bool_str(hbg_hw_irq_is_enabled(priv, irq_mask)));
+}
+
+static int hbg_dbg_tx_ring(struct seq_file *s, void *unused)
+{
+	struct net_device *netdev = dev_get_drvdata(s->private);
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	hbg_dbg_ring(priv, &priv->tx_ring, s);
+	return 0;
+}
+
+static int hbg_dbg_rx_ring(struct seq_file *s, void *unused)
+{
+	struct net_device *netdev = dev_get_drvdata(s->private);
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	hbg_dbg_ring(priv, &priv->rx_ring, s);
+	return 0;
+}
+
+static const struct hbg_dbg_info hbg_dbg_infos[] = {
+	{ "tx_ring", hbg_dbg_tx_ring },
+	{ "rx_ring", hbg_dbg_rx_ring },
+};
+
+static void hbg_debugfs_uninit(void *data)
+{
+	debugfs_remove_recursive((struct dentry *)data);
+}
+
+void hbg_debugfs_init(struct hbg_priv *priv)
+{
+	const char *name = pci_name(priv->pdev);
+	struct device *dev = &priv->pdev->dev;
+	struct dentry *root;
+	u32 i;
+
+	root = debugfs_create_dir(name, hbg_dbgfs_root);
+
+	for (i = 0; i < ARRAY_SIZE(hbg_dbg_infos); i++)
+		debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
+					    root, hbg_dbg_infos[i].read);
+
+	/* Ignore the failure because debugfs is not a key feature. */
+	devm_add_action_or_reset(dev, hbg_debugfs_uninit, root);
+}
+
+void hbg_debugfs_register(void)
+{
+	hbg_dbgfs_root = debugfs_create_dir("hibmcge", NULL);
+}
+
+void hbg_debugfs_unregister(void)
+{
+	debugfs_remove_recursive(hbg_dbgfs_root);
+	hbg_dbgfs_root = NULL;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h
new file mode 100644
index 000000000000..80670d66bbeb
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_DEBUGFS_H
+#define __HBG_DEBUGFS_H
+
+void hbg_debugfs_register(void);
+void hbg_debugfs_unregister(void);
+
+void hbg_debugfs_init(struct hbg_priv *priv);
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 75505fb5cc4a..7a03fdfa32a7 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -11,6 +11,7 @@
 #include "hbg_irq.h"
 #include "hbg_mdio.h"
 #include "hbg_txrx.h"
+#include "hbg_debugfs.h"
 
 static void hbg_change_mtu(struct hbg_priv *priv, int new_mtu);
 
@@ -160,7 +161,12 @@ static int hbg_init(struct hbg_priv *priv)
 	if (ret)
 		return ret;
 
-	return hbg_mdio_init(priv);
+	ret = hbg_mdio_init(priv);
+	if (ret)
+		return ret;
+
+	hbg_debugfs_init(priv);
+	return 0;
 }
 
 static int hbg_pci_init(struct pci_dev *pdev)
@@ -245,7 +251,26 @@ static struct pci_driver hbg_driver = {
 	.id_table	= hbg_pci_tbl,
 	.probe		= hbg_probe,
 };
-module_pci_driver(hbg_driver);
+
+static int __init hbg_module_init(void)
+{
+	int ret;
+
+	hbg_debugfs_register();
+	ret = pci_register_driver(&hbg_driver);
+	if (ret)
+		hbg_debugfs_unregister();
+
+	return ret;
+}
+module_init(hbg_module_init);
+
+static void __exit hbg_module_exit(void)
+{
+	pci_unregister_driver(&hbg_driver);
+	hbg_debugfs_unregister();
+}
+module_exit(hbg_module_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Huawei Tech. Co., Ltd.");
-- 
2.33.0

On Tue, 10 Dec 2024 21:48:49 +0800 Jijie Shao wrote:
> +#define hbg_get_bool_str(state) ((state) ? "true" : "false")

If you're defining a wrapper for this you're better off using
str_true_false()

on 2024/12/12 11:41, Jakub Kicinski wrote:
> On Tue, 10 Dec 2024 21:48:49 +0800 Jijie Shao wrote:
>> +#define hbg_get_bool_str(state) ((state) ? "true" : "false")
> If you're defining a wrapper for this you're better off using
> str_true_false()

Sure, use it in next version

Thanks,
Jijie Shao

On Tue, 10 Dec 2024 21:48:49 +0800 Jijie Shao wrote:
> +		debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
> +					    root, hbg_dbg_infos[i].read);

Like I said last time, if you devm_ the entire folder you don't have to
devm_ each individual file. debugfs_remove_recursive() removes all files
under specified directory.

on 2024/12/11 22:00, Jakub Kicinski wrote:
> On Tue, 10 Dec 2024 21:48:49 +0800 Jijie Shao wrote:
>> +		debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
>> +					    root, hbg_dbg_infos[i].read);
> Like I said last time, if you devm_ the entire folder you don't have to
> devm_ each individual file. debugfs_remove_recursive() removes all files
> under specified directory.


Sorry, there's something wrong with the format of the last reply.


I think debugfs_create_devm_seqfile() is a better choice,
if not use it, I might need to code like so：

static const struct file_operations hbg_dbg_fops = {
	.owner   = THIS_MODULE,
...
};

static int hbg_dbg_file_init(struct hbg_priv *priv, u32 cmd, const char *name)
{
	struct hbg_dbg_pri_data *data;

	data = devm_kzalloc(&priv->pdev->dev, sizeof(*data), GFP_KERNEL);
	if (!data)
		return -ENOMEM;

	data->priv = priv;
	data->cmd = cmd;
	debugfs_create_file(name, HBG_DBG_FILE_MODE,
			    priv->debugfs.root, data, &hbg_dbg_fops);

	return 0;
}

int hbg_debugfs_init(struct hbg_priv *priv)
{
...
	for (i = 0; i < ARRAY_SIZE(hbg_dbg_infos); i++) {
		ret = hbg_dbg_file_init(priv, i, hbg_dbg_infos[i].name);
...
	}


But use debugfs_create_devm_seqfile(), I only need:

void hbg_debugfs_init(struct hbg_priv *priv)
{
...
     for (i = 0; i < ARRAY_SIZE(hbg_dbg_infos); i++)
         debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
                         root, hbg_dbg_infos[i].read);


Actually I think debugfs_create_devm_seqfile() is actually similar to hbg_dbg_file_init().
And in debugfs_create_devm_seqfile(), debugfs_create_file() is also called, and the code is simplified.

Thanks,
Jijie Shao

On Thu, 12 Dec 2024 09:35:02 +0800 Jijie Shao wrote:
> I think debugfs_create_devm_seqfile() is a better choice,
> if not use it, I might need to code like so：

Oh, sorry, I thought there is a non-devm version of create_seqfile().
Let me review the rest.

on 2024/12/11 22:00, Jakub Kicinski wrote:
> On Tue, 10 Dec 2024 21:48:49 +0800 Jijie Shao wrote:
>> +		debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
>> +					    root, hbg_dbg_infos[i].read);
> Like I said last time, if you devm_ the entire folder you don't have to
> devm_ each individual file. debugfs_remove_recursive() removes all files
> under specified directory.

I think debugfs_create_devm_seqfile()is a better choice, if not use it.  and I might need to code like so： static const struct file_operations 
hbg_dbg_fops = { .owner = THIS_MODULE, ... }; static int 
hbg_dbg_file_init(struct hbg_priv *priv, u32 cmd, const char *name) { 
struct hbg_dbg_pri_data *data; data = devm_kzalloc(&priv->pdev->dev, 
sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; data->priv = 
priv; data->cmd = cmd; debugfs_create_file(name, HBG_DBG_FILE_MODE, 
priv->debugfs.root, data, &hbg_dbg_fops); return 0; } int 
hbg_debugfs_init(struct hbg_priv *priv) { ... for (i = 0; i < ARRAY_SIZE(hbg_dbg_infos); i++) { ret = hbg_dbg_file_init(priv, i, hbg_dbg_infos[i].name); ... } But use debugfs_create_devm_seqfile(), I only need：void hbg_debugfs_init(struct hbg_priv *priv)
{
...
	for (i = 0; i < ARRAY_SIZE(hbg_dbg_infos); i++)
		debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
					    root, hbg_dbg_infos[i].read);


Actually i think debugfs_create_devm_seqfile() is actually similar to hbg_dbg_file_init().
And in debugfs_create_devm_seqfile(), debugfs_create_file() is also called, and the code is simplified.

Thanks,
Jijie Shao