RE: [PATCH 04/24] rtw89: add debug files

[Pkshih <pkshih@realtek.com>]

> -----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:
> > 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
> > new file mode 100644
> > index 000000000000..03d2d2eb813d
> > --- /dev/null
> > +++ b/drivers/net/wireless/realtek/rtw89/debug.c

[...]

> > +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