Re: [RFC v5 08/13] rtw88: debug files
On Mon, 2018-11-05 at 06:13 +, Tony Chuang wrote:
> > -Original Message-
> > From: Joe Perches [mailto:j...@perches.com]
> > Sent: Sunday, November 04, 2018 12:29 PM
> > To: Tony Chuang; kv...@codeaurora.org; sgrus...@redhat.com
> > Cc: larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org; Pkshih; Andy
> > Huang; johan...@sipsolutions.net
> > Subject: Re: [RFC v5 08/13] rtw88: debug files
> >
> > On Wed, 2018-10-31 at 18:12 +0800, yhchu...@realtek.com wrote
> > > From: Yan-Hsuan Chuang
> > > debug files for Realtek 802.11ac wireless network chips
> > []
> > > diff --git a/drivers/net/wireless/realtek/rtw88/debug.h
> > b/drivers/net/wireless/realtek/rtw88/debug.h
> > []
> > > +#ifdef CONFIG_RTW88_DEBUG
> > []
> > > +#else
> > > +
> > > +static inline void rtw_dbg(struct rtw_dev *rtwdev, const char *fmt, ...)
> > > {}
> > > +static inline void rtw_info(struct rtw_dev *rtwdev, const char *fmt,
> > > ...) {}
> > > +static inline void rtw_warn(struct rtw_dev *rtwdev, const char *fmt,
> > > ...) {}
> > > +static inline void rtw_err(struct rtw_dev *rtwdev, const char *fmt, ...)
> > > {}
> >
> > I still think it's _very, very bad_, and extremely
> > unusual to turn off _all_ logging, even for error
> > conditions, based on a debug config flag.
> >
[]
> So I think we should move rtw_[err/warn/info] out of the flag.
> Should we need to send it in RFC v6 or in the next patch series?
Up to you.
I think you should look at all the rtw_err/rtw_warn/rtw_info uses
as many seem to be more like they should use rtw_dbg instead.
Also, I think the rtw_err/rtw_warn/rtw_info function calls aren't
particularly useful and these should be simple macros or static
inlines and not use %pV at all.
#define rtw_err(rtw_dev, fmt, ...) \
dev_err((rtw_dev)->dev, fmt, ##__VA_ARGS__)
etc
RE: [RFC v5 08/13] rtw88: debug files
> -Original Message-
> From: Joe Perches [mailto:j...@perches.com]
> Sent: Sunday, November 04, 2018 12:29 PM
> To: Tony Chuang; kv...@codeaurora.org; sgrus...@redhat.com
> Cc: larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org; Pkshih; Andy
> Huang; johan...@sipsolutions.net
> Subject: Re: [RFC v5 08/13] rtw88: debug files
>
> On Wed, 2018-10-31 at 18:12 +0800, yhchu...@realtek.com wrote
> > From: Yan-Hsuan Chuang
> > debug files for Realtek 802.11ac wireless network chips
> []
> > diff --git a/drivers/net/wireless/realtek/rtw88/debug.h
> b/drivers/net/wireless/realtek/rtw88/debug.h
> []
> > +#ifdef CONFIG_RTW88_DEBUG
> []
> > +#else
> > +
> > +static inline void rtw_dbg(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> > +static inline void rtw_info(struct rtw_dev *rtwdev, const char *fmt, ...)
> > {}
> > +static inline void rtw_warn(struct rtw_dev *rtwdev, const char *fmt, ...)
> > {}
> > +static inline void rtw_err(struct rtw_dev *rtwdev, const char *fmt, ...) {}
>
> I still think it's _very, very bad_, and extremely
> unusual to turn off _all_ logging, even for error
> conditions, based on a debug config flag.
>
So I think we should move rtw_[err/warn/info] out of the flag.
Should we need to send it in RFC v6 or in the next patch series?
YH Chuang
Re: [RFC v5 08/13] rtw88: debug files
On Wed, 2018-10-31 at 18:12 +0800, yhchu...@realtek.com wrote
> From: Yan-Hsuan Chuang
> debug files for Realtek 802.11ac wireless network chips
[]
> diff --git a/drivers/net/wireless/realtek/rtw88/debug.h
> b/drivers/net/wireless/realtek/rtw88/debug.h
[]
> +#ifdef CONFIG_RTW88_DEBUG
[]
> +#else
> +
> +static inline void rtw_dbg(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> +static inline void rtw_info(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> +static inline void rtw_warn(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> +static inline void rtw_err(struct rtw_dev *rtwdev, const char *fmt, ...) {}
I still think it's _very, very bad_, and extremely
unusual to turn off _all_ logging, even for error
conditions, based on a debug config flag.
[RFC v5 08/13] rtw88: debug files
From: Yan-Hsuan Chuang
debug files for Realtek 802.11ac wireless network chips
Signed-off-by: Yan-Hsuan Chuang
---
drivers/net/wireless/realtek/rtw88/debug.c | 652 +
drivers/net/wireless/realtek/rtw88/debug.h | 43 ++
2 files changed, 695 insertions(+)
create mode 100644 drivers/net/wireless/realtek/rtw88/debug.c
create mode 100644 drivers/net/wireless/realtek/rtw88/debug.h
diff --git a/drivers/net/wireless/realtek/rtw88/debug.c
b/drivers/net/wireless/realtek/rtw88/debug.c
new file mode 100644
index 000..ad78e89
--- /dev/null
+++ b/drivers/net/wireless/realtek/rtw88/debug.c
@@ -0,0 +1,652 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Realtek Corporation.
+ */
+
+#include
+#include
+#include "main.h"
+#include "sec.h"
+#include "fw.h"
+#include "debug.h"
+
+#ifdef CONFIG_RTW88_DEBUGFS
+
+struct rtw_debugfs_priv {
+ struct rtw_dev *rtwdev;
+ int (*cb_read)(struct seq_file *m, void *v);
+ ssize_t (*cb_write)(struct file *filp, const char __user *buffer,
+ size_t count, loff_t *loff);
+ union {
+ u32 cb_data;
+ u8 *buf;
+ struct {
+ u32 page_offset;
+ u32 page_num;
+ } rsvd_page;
+ struct {
+ u8 rf_path;
+ u32 rf_addr;
+ u32 rf_mask;
+ };
+ struct {
+ u32 addr;
+ u32 len;
+ } read_reg;
+ };
+};
+
+static int rtw_debugfs_single_show(struct seq_file *m, void *v)
+{
+ struct rtw_debugfs_priv *debugfs_priv = m->private;
+
+ return debugfs_priv->cb_read(m, v);
+}
+
+static ssize_t rtw_debugfs_common_write(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *loff)
+{
+ struct rtw_debugfs_priv *debugfs_priv = filp->private_data;
+
+ return debugfs_priv->cb_write(filp, buffer, count, loff);
+}
+
+static ssize_t rtw_debugfs_single_write(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *loff)
+{
+ struct seq_file *seqpriv = (struct seq_file *)filp->private_data;
+ struct rtw_debugfs_priv *debugfs_priv = seqpriv->private;
+
+ return debugfs_priv->cb_write(filp, buffer, count, loff);
+}
+
+static int rtw_debugfs_single_open_rw(struct inode *inode, struct file *filp)
+{
+ return single_open(filp, rtw_debugfs_single_show, inode->i_private);
+}
+
+static int rtw_debugfs_close(struct inode *inode, struct file *filp)
+{
+ return 0;
+}
+
+static const struct file_operations file_ops_single_r = {
+ .owner = THIS_MODULE,
+ .open = rtw_debugfs_single_open_rw,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release,
+};
+
+static const struct file_operations file_ops_single_rw = {
+ .owner = THIS_MODULE,
+ .open = rtw_debugfs_single_open_rw,
+ .release = single_release,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .write = rtw_debugfs_single_write,
+};
+
+static const struct file_operations file_ops_common_write = {
+ .owner = THIS_MODULE,
+ .write = rtw_debugfs_common_write,
+ .open = simple_open,
+ .release = rtw_debugfs_close,
+};
+
+static int rtw_debugfs_get_read_reg(struct seq_file *m, void *v)
+{
+ struct rtw_debugfs_priv *debugfs_priv = m->private;
+ struct rtw_dev *rtwdev = debugfs_priv->rtwdev;
+ u32 val, len, addr;
+
+ len = debugfs_priv->read_reg.len;
+ addr = debugfs_priv->read_reg.addr;
+ switch (len) {
+ case 1:
+ val = rtw_read8(rtwdev, addr);
+ seq_printf(m, "reg 0x%03x: 0x%02x\n", addr, val);
+ break;
+ case 2:
+ val = rtw_read16(rtwdev, addr);
+ seq_printf(m, "reg 0x%03x: 0x%04x\n", addr, val);
+ break;
+ case 4:
+ val = rtw_read32(rtwdev, addr);
+ seq_printf(m, "reg 0x%03x: 0x%08x\n", addr, val);
+ break;
+ }
+ return 0;
+}
+
+static int rtw_debugfs_get_rf_read(struct seq_file *m, void *v)
+{
+ struct rtw_debugfs_priv *debugfs_priv = m->private;
+ struct rtw_dev *rtwdev = debugfs_priv->rtwdev;
+ u32 val, addr, mask;
+ u8 path;
+
+ path = debugfs_priv->rf_path;
+ addr = debugfs_priv->rf_addr;
+ mask = debugfs_priv->rf_mask;
+
+ val = rtw_read_rf(rtwdev, path, addr, mask);
+
+ seq_printf(m, "rf_read path:%d addr:0x%08x mask:0x%08x val=0x%08x\n",
+ path, addr, mask, val);
+
+ return 0;
+}
+
+static int rtw_debugfs_copy_from_user(char tmp[], int size,
+ const char __user *buffer, size_t count,
+
