-----Original Message-----
From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
Sent: Friday, July 02, 2021 3:23 PM
To: Pkshih
Cc: kvalo@codeaurora.org; linux-wireless@vger.kernel.org
Subject: Re: [PATCH 04/24] rtw89: add debug files

On Fri, Jun 18, 2021 at 02:46:05PM +0800, Ping-Ke Shih wrote:

quoted

To recognize issues happened in field, two debug methods, debug message and
debugfs, are added.

The debug messages are written to kernel log, and four levels can be chosen
according to the cases -- debug, info, warn and err.

Debugfs is used to read and write registers and driver status.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/debug.c | 2404 ++++++++++++++++++++
 drivers/net/wireless/realtek/rtw89/debug.h |   77 +
 2 files changed, 2481 insertions(+)
 create mode 100644 drivers/net/wireless/realtek/rtw89/debug.c
 create mode 100644 drivers/net/wireless/realtek/rtw89/debug.h

diff --git a/drivers/net/wireless/realtek/rtw89/debug.c

b/drivers/net/wireless/realtek/rtw89/debug.c

quoted

new file mode 100644
index 000000000000..03d2d2eb813d

--- /dev/null
+++ b/drivers/net/wireless/realtek/rtw89/debug.c

[...]

quoted

+static int rtw89_debug_priv_rf_reg_dump_get(struct seq_file *m, void *v)
+{
+	struct rtw89_debugfs_priv *debugfs_priv = m->private;
+	struct rtw89_dev *rtwdev = debugfs_priv->rtwdev;
+	const struct rtw89_chip_info *chip = rtwdev->chip;
+	u32 addr, offset, data;
+	u8 path;
+
+	for (path = 0; path < chip->rf_path_num; path++) {
+		seq_printf(m, "RF path %d:\n\n", path);
+		for (addr = 0; addr < 0x100; addr += 4) {
+			seq_printf(m, "0x%08x: ", addr);
+			for (offset = 0; offset < 4; offset++) {
+				data = rtw89_read_rf(rtwdev, path,
+						     addr + offset, RFREG_MASK);
+				seq_printf(m, "0x%05x  ", data);
+			}
+			seq_puts(m, "\n");
+		}
+		seq_puts(m, "\n");
+	}
+
+	return 0;
+}

Based on this and other part of this driver I would recommend to use
regmap. It will provide to additional interface for the register
access. And typically for the network devices we have an ethtool
interface for that.

Could I know the 'regmap' you mentioned?

I study interfaces of ethtool. It sets/gets dump via struct ethtool_dump.
I face problems: this chip has many register domains, but there's no way to
specify the domain type. The amount of registers is large, so I need to specify
the range I want to dump, but no offset/length can be used by ETHTOOL_SET_DUMP.
Another reason I can't dump all registers is that some domains are indirect
access with two registers (one is control register, and the other is data
register), I can't access them arbitrarily because firmware can use them 
simultaneously. Instead, they are only used in fatal cases.

So, I can't use interfaces of ethtool, but I'll try to merge duplicate to
reduce code size.

--
Ping-Ke