Re: [PATCH] staging: rtlwifi: Improve debugging by using debugfs
Fri, Aug 25, 2017 at 7:16 AM, Andrew Lunn wrote: > On Fri, Aug 25, 2017 at 08:47:00AM -0500, Larry Finger wrote: >> On 08/24/2017 08:54 PM, Andrew Lunn wrote: >> >netdev frowns upon debugfs. You should try to keep this altogether, >> >making it easy to throw away before the driver is moved out of >> >staging. >> > >> >You might want to look at ethtool -d. That will be accepted. >> >> Andrew, >> >> What is the problem with debugfs? > > You should probably look back in the discussions on the netdev > mailling list. But basically, anything you want to export should > follow generic well defined interface, which can be used by other > drivers. debugfs tends to be a mess, a wild west, each driver doing > its own thing, not standardisation. It is O.K. for your own > development work, you can have your own out of tree patches adding in > debugfs, but such patches are unlikely to be accepted into mainline. > David has threatened to simply rip out all debugfs code from all > network drivers. There is push back on adding any new debugfs code, > and some driver writers have taken out debugfs code in their own > drivers, often replacing it with something generic all drivers can > use. I think the bigger issue is that many people end up using debugfs to try and configure things, or they create redundant functionality with existing interfaces which is generally frowned upon. So generally it is okay for things like peeking into driver state machines, or as a means to dump descriptor rings, but not okay for using to program filters, write registers, change the device state, or collect generic statistics. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: Improve debugging by using debugfs
On Fri, Aug 25, 2017 at 08:47:00AM -0500, Larry Finger wrote: > On 08/24/2017 08:54 PM, Andrew Lunn wrote: > >netdev frowns upon debugfs. You should try to keep this altogether, > >making it easy to throw away before the driver is moved out of > >staging. > > > >You might want to look at ethtool -d. That will be accepted. > > Andrew, > > What is the problem with debugfs? You should probably look back in the discussions on the netdev mailling list. But basically, anything you want to export should follow generic well defined interface, which can be used by other drivers. debugfs tends to be a mess, a wild west, each driver doing its own thing, not standardisation. It is O.K. for your own development work, you can have your own out of tree patches adding in debugfs, but such patches are unlikely to be accepted into mainline. David has threatened to simply rip out all debugfs code from all network drivers. There is push back on adding any new debugfs code, and some driver writers have taken out debugfs code in their own drivers, often replacing it with something generic all drivers can use. > Please suggest which driver has the best example of an ethtool -d > implementation that we might study. The API is very simple. In your ethtool_ops you need to function: .get_regs_len, returns an int, the number of registers you can dump. .get_regs returns the contents. There are plenty of examples, e.g.: http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/amd/pcnet32.c#L1436 If you need something more structured, look around, see if other drivers have similar needs, and propose something generic. > The first version of the debugfs changes were sent to wireless-drivers on > July 2. Why are we first hearing of this objection nearly 2 months later? Different reviewers tend to review different things. I personally don't care so much who the vendor is, but look out for some specific things i feel qualified to review. Ethernet PHYs and MDIO drivers, drivers which make use of Ethernet PHYs, APIs which allow userspace to write to registers, debugfs, changing MTUs, etc. There are plenty of patches on netdev for me to review, so i don't cast a wider net to other lists, like for example wireless-drivers. You would of got this comment sooner or later, since you should be posting to netdev at some point anyway. I can also imaging there is also some reluctance to review staging code, until it is getting close to moving out of staging. Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: Improve debugging by using debugfs
On 08/24/2017 08:54 PM, Andrew Lunn wrote: netdev frowns upon debugfs. You should try to keep this altogether, making it easy to throw away before the driver is moved out of staging. You might want to look at ethtool -d. That will be accepted. Andrew, What is the problem with debugfs? Please suggest which driver has the best example of an ethtool -d implementation that we might study. The first version of the debugfs changes were sent to wireless-drivers on July 2. Why are we first hearing of this objection nearly 2 months later? Larry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: Improve debugging by using debugfs
On Thu, Aug 24, 2017 at 04:28:08PM -0500, Larry Finger wrote: > The changes in this commit are also being sent to the main rtlwifi > drivers in wireless-next; however, these changes will also be useful for > any debugging of r8822be before it gets moved into the main tree. > > Use debugfs to dump register and btcoex status, and also write registers > and h2c. > > We create topdir in /sys/kernel/debug/rtlwifi/, and use the MAC address > as subdirectory with several entries to dump mac_reg, bb_reg, rf_reg etc. > An example is > /sys/kernel/debug/rtlwifi/00-11-22-33-44-55-66/mac_0 > > This change permits examination of device registers in a dynamic manner, > a feature not available with the current debug mechanism. Hi Larry netdev frowns upon debugfs. You should try to keep this altogether, making it easy to throw away before the driver is moved out of staging. You might want to look at ethtool -d. That will be accepted. Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtlwifi: Improve debugging by using debugfs
The changes in this commit are also being sent to the main rtlwifi
drivers in wireless-next; however, these changes will also be useful for
any debugging of r8822be before it gets moved into the main tree.
Use debugfs to dump register and btcoex status, and also write registers
and h2c.
We create topdir in /sys/kernel/debug/rtlwifi/, and use the MAC address
as subdirectory with several entries to dump mac_reg, bb_reg, rf_reg etc.
An example is
/sys/kernel/debug/rtlwifi/00-11-22-33-44-55-66/mac_0
This change permits examination of device registers in a dynamic manner,
a feature not available with the current debug mechanism.
We use seq_file to replace RT_TRACE to dump status, then we can use 'cat'
to access btcoex's status through debugfs.
(i.e. /sys/kernel/debug/rtlwifi/00-11-22-33-44-55-66/btcoex)
Other related changes are
1. implement btc_disp_dbg_msg() to access btcoex's common status.
2. remove obsolete field bt_exist
Signed-off-by: Ping-Ke Shih
Signed-off-by: Larry Finger
Cc: Yan-Hsuan Chuang
Cc: Birming Chiu
Cc: Shaofu
Cc: Steven Ting
---
drivers/staging/rtlwifi/debug.c | 226
1 file changed, 135 insertions(+), 91 deletions(-)
diff --git a/drivers/staging/rtlwifi/debug.c b/drivers/staging/rtlwifi/debug.c
index b9fd47aeaa9b..7446d71c41d1 100644
--- a/drivers/staging/rtlwifi/debug.c
+++ b/drivers/staging/rtlwifi/debug.c
@@ -80,9 +80,11 @@ void _rtl_dbg_print_data(struct rtl_priv *rtlpriv, u64 comp,
int level,
}
}
-struct rtl_debgufs_priv {
+struct rtl_debugfs_priv {
struct rtl_priv *rtlpriv;
- int (*cb)(struct seq_file *m, void *v);
+ 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);
u32 cb_data;
};
@@ -90,9 +92,9 @@ static struct dentry *debugfs_topdir;
static int rtl_debug_get_common(struct seq_file *m, void *v)
{
- struct rtl_debgufs_priv *debugfs_priv = m->private;
+ struct rtl_debugfs_priv *debugfs_priv = m->private;
- return debugfs_priv->cb(m, v);
+ return debugfs_priv->cb_read(m, v);
}
static int dl_debug_open_common(struct inode *inode, struct file *file)
@@ -109,7 +111,7 @@ static const struct file_operations file_ops_common = {
static int rtl_debug_get_mac_page(struct seq_file *m, void *v)
{
- struct rtl_debgufs_priv *debugfs_priv = m->private;
+ struct rtl_debugfs_priv *debugfs_priv = m->private;
struct rtl_priv *rtlpriv = debugfs_priv->rtlpriv;
u32 page = debugfs_priv->cb_data;
int i, n;
@@ -126,8 +128,8 @@ static int rtl_debug_get_mac_page(struct seq_file *m, void
*v)
}
#define RTL_DEBUG_IMPL_MAC_SERIES(page, addr) \
-struct rtl_debgufs_priv rtl_debug_priv_mac_ ##page = { \
- .cb = rtl_debug_get_mac_page, \
+struct rtl_debugfs_priv rtl_debug_priv_mac_ ##page = { \
+ .cb_read = rtl_debug_get_mac_page, \
.cb_data = addr,\
}
@@ -150,7 +152,7 @@ RTL_DEBUG_IMPL_MAC_SERIES(17, 0x1700);
static int rtl_debug_get_bb_page(struct seq_file *m, void *v)
{
- struct rtl_debgufs_priv *debugfs_priv = m->private;
+ struct rtl_debugfs_priv *debugfs_priv = m->private;
struct rtl_priv *rtlpriv = debugfs_priv->rtlpriv;
struct ieee80211_hw *hw = rtlpriv->hw;
u32 page = debugfs_priv->cb_data;
@@ -168,8 +170,8 @@ static int rtl_debug_get_bb_page(struct seq_file *m, void
*v)
}
#define RTL_DEBUG_IMPL_BB_SERIES(page, addr) \
-struct rtl_debgufs_priv rtl_debug_priv_bb_ ##page = { \
- .cb = rtl_debug_get_bb_page,\
+struct rtl_debugfs_priv rtl_debug_priv_bb_ ##page = { \
+ .cb_read = rtl_debug_get_bb_page, \
.cb_data = addr,\
}
@@ -192,7 +194,7 @@ RTL_DEBUG_IMPL_BB_SERIES(1f, 0x1f00);
static int rtl_debug_get_reg_rf(struct seq_file *m, void *v)
{
- struct rtl_debgufs_priv *debugfs_priv = m->private;
+ struct rtl_debugfs_priv *debugfs_priv = m->private;
struct rtl_priv *rtlpriv = debugfs_priv->rtlpriv;
struct ieee80211_hw *hw = rtlpriv->hw;
enum radio_path rfpath = debugfs_priv->cb_data;
@@ -215,8 +217,8 @@ static int rtl_debug_get_reg_rf(struct seq_file *m, void *v)
}
#define RTL_DEBUG_IMPL_RF_SERIES(page, addr) \
-struct rtl_debgufs_priv rtl_debug_priv_rf_ ##page = { \
- .cb = rtl_debug_get_reg_rf, \
+struct rtl_debugfs_priv rtl_debug_priv_rf_ ##page = { \
+ .cb_read = rtl_debug_get_reg_rf,\
.cb_data = addr,\
}
@@ -225,7 +227,7 @@ RTL_DEBUG_IMPL_RF_SERIES(b, RF90_PATH_B);
static int rtl_debug_get_cam_register(struct seq_file *m, void *v)
{
- struct rtl_debgufs_priv *debugfs_priv = m->private;
+ struct rtl_debugfs_priv *de
