| Message ID | 20200309075852.11454-3-yhchuang@realtek.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | rtw88: add coex related debugfs | expand |
<yhchuang@realtek.com> writes: > From: Yan-Hsuan Chuang <yhchuang@realtek.com> > > Sometimes we need to stop the coex mechanism to debug, so that we > can manually control the device through various outer commands. > Hence, add a new debugfs coex_enable to allow us to enable/disable > the coex mechanism when driver is running. > > To disable coex > echo 0 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > > To enable coex > echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > > To check coex dm is enabled or not > cat /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable I forgot, did we add a command to nl80211 for managing btcoex? At least we have talking about that for years. Please check that first before adding a debugfs interface for this.
Kalle Valo <kvalo@codeaurora.org> : > <yhchuang@realtek.com> writes: > > > From: Yan-Hsuan Chuang <yhchuang@realtek.com> > > > > Sometimes we need to stop the coex mechanism to debug, so that we > > can manually control the device through various outer commands. > > Hence, add a new debugfs coex_enable to allow us to enable/disable > > the coex mechanism when driver is running. > > > > To disable coex > > echo 0 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > > > > To enable coex > > echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > > > > To check coex dm is enabled or not > > cat /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > > I forgot, did we add a command to nl80211 for managing btcoex? At least > we have talking about that for years. Please check that first before > adding a debugfs interface for this. > Yes, I found there was a thread [1] talking about adding a callback to enable/disable btcoex, but it seems not get applied eventually. And there's another thread [2] talking about add a btcoex subsystem. But seems like nobody can implement it cleanly in the host. I think adding btcoex subsystem could have a lot of pain since each vendor is using different mechanism controlling the btcoex, and it usually comes with RF related design which is difficult to write a common function to deal with all kinds of them. Yen-Hsuan [1] https://patchwork.kernel.org/patch/10252135/ [2] https://www.spinics.net/lists/linux-wireless/msg133333.html
Tony Chuang <yhchuang@realtek.com> writes: > Kalle Valo <kvalo@codeaurora.org> : > >> <yhchuang@realtek.com> writes: >> >> > From: Yan-Hsuan Chuang <yhchuang@realtek.com> >> > >> > Sometimes we need to stop the coex mechanism to debug, so that we >> > can manually control the device through various outer commands. >> > Hence, add a new debugfs coex_enable to allow us to enable/disable >> > the coex mechanism when driver is running. >> > >> > To disable coex >> > echo 0 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable >> > >> > To enable coex >> > echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable >> > >> > To check coex dm is enabled or not >> > cat /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable >> >> I forgot, did we add a command to nl80211 for managing btcoex? At least >> we have talking about that for years. Please check that first before >> adding a debugfs interface for this. >> > > Yes, I found there was a thread [1] talking about adding a callback to > enable/disable btcoex, but it seems not get applied eventually. Too bad, I really think we should have at least enable/disable functionality in nl80211. But if it's not there, I guess it's ok to have yet another driver custom debugfs file :/ > And there's another thread [2] talking about add a btcoex subsystem. > But seems like nobody can implement it cleanly in the host. > > I think adding btcoex subsystem could have a lot of pain since each > vendor is using different mechanism controlling the btcoex, and it > usually comes with RF related design which is difficult to write a common > function to deal with all kinds of them. Yeah, btcoex subsystem is a big challenge.
Kalle Valo <kvalo@codeaurora.org> writes: > Tony Chuang <yhchuang@realtek.com> writes: > > > Kalle Valo <kvalo@codeaurora.org> : > > > >> <yhchuang@realtek.com> writes: > >> > >> > From: Yan-Hsuan Chuang <yhchuang@realtek.com> > >> > > >> > Sometimes we need to stop the coex mechanism to debug, so that we > >> > can manually control the device through various outer commands. > >> > Hence, add a new debugfs coex_enable to allow us to enable/disable > >> > the coex mechanism when driver is running. > >> > > >> > To disable coex > >> > echo 0 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > >> > > >> > To enable coex > >> > echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > >> > > >> > To check coex dm is enabled or not > >> > cat /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > >> > >> I forgot, did we add a command to nl80211 for managing btcoex? At least > >> we have talking about that for years. Please check that first before > >> adding a debugfs interface for this. > >> > > > > Yes, I found there was a thread [1] talking about adding a callback to > > enable/disable btcoex, but it seems not get applied eventually. > > Too bad, I really think we should have at least enable/disable > functionality in nl80211. But if it's not there, I guess it's ok to have > yet another driver custom debugfs file :/ > Yes, so please take this ;) Thanks Yen-Hsuan
> Tony Chuang <yhchuang@realtek.com> writes: > > > Kalle Valo <kvalo@codeaurora.org> : > > > >> <yhchuang@realtek.com> writes: > >> > >> > From: Yan-Hsuan Chuang <yhchuang@realtek.com> > >> > > >> > Sometimes we need to stop the coex mechanism to debug, so that we > >> > can manually control the device through various outer commands. > >> > Hence, add a new debugfs coex_enable to allow us to enable/disable > >> > the coex mechanism when driver is running. > >> > > >> > To disable coex > >> > echo 0 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > >> > > >> > To enable coex > >> > echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > >> > > >> > To check coex dm is enabled or not > >> > cat /sys/kernel/debug/ieee80211/phyX/rtw88/coex_enable > >> > >> I forgot, did we add a command to nl80211 for managing btcoex? At least > >> we have talking about that for years. Please check that first before > >> adding a debugfs interface for this. > >> > > > > Yes, I found there was a thread [1] talking about adding a callback to > > enable/disable btcoex, but it seems not get applied eventually. > > Too bad, I really think we should have at least enable/disable > functionality in nl80211. But if it's not there, I guess it's ok to have > yet another driver custom debugfs file :/ > > > And there's another thread [2] talking about add a btcoex subsystem. > > But seems like nobody can implement it cleanly in the host. > > > > I think adding btcoex subsystem could have a lot of pain since each > > vendor is using different mechanism controlling the btcoex, and it > > usually comes with RF related design which is difficult to write a common > > function to deal with all kinds of them. > > Yeah, btcoex subsystem is a big challenge. > So I think we can take this patch set. It is really useful to debug on coex's issues. Yen-Hsuan
diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c index b2d264270752..b00eee68b3fb 100644 --- a/drivers/net/wireless/realtek/rtw88/debug.c +++ b/drivers/net/wireless/realtek/rtw88/debug.c @@ -706,6 +706,46 @@ static int rtw_debugfs_get_coex_info(struct seq_file *m, void *v) return 0; } +static ssize_t rtw_debugfs_set_coex_enable(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; + struct rtw_dev *rtwdev = debugfs_priv->rtwdev; + struct rtw_coex *coex = &rtwdev->coex; + char tmp[32 + 1]; + u32 enable; + int num; + + rtw_debugfs_copy_from_user(tmp, sizeof(tmp), buffer, count, 2); + + num = sscanf(tmp, "%d", &enable); + + if (num != 1) { + rtw_warn(rtwdev, "invalid arguments\n"); + return num; + } + + mutex_lock(&rtwdev->mutex); + coex->stop_dm = enable == 0; + mutex_unlock(&rtwdev->mutex); + + return count; +} + +static int rtw_debugfs_get_coex_enable(struct seq_file *m, void *v) +{ + struct rtw_debugfs_priv *debugfs_priv = m->private; + struct rtw_dev *rtwdev = debugfs_priv->rtwdev; + struct rtw_coex *coex = &rtwdev->coex; + + seq_printf(m, "coex mechanism %s\n", + coex->stop_dm ? "disabled" : "enabled"); + + return 0; +} + #define rtw_debug_impl_mac(page, addr) \ static struct rtw_debugfs_priv rtw_debug_priv_mac_ ##page = { \ .cb_read = rtw_debug_get_mac_page, \ @@ -796,6 +836,11 @@ static struct rtw_debugfs_priv rtw_debug_priv_phy_info = { .cb_read = rtw_debugfs_get_phy_info, }; +static struct rtw_debugfs_priv rtw_debug_priv_coex_enable = { + .cb_write = rtw_debugfs_set_coex_enable, + .cb_read = rtw_debugfs_get_coex_enable, +}; + static struct rtw_debugfs_priv rtw_debug_priv_coex_info = { .cb_read = rtw_debugfs_get_coex_info, }; @@ -831,6 +876,7 @@ void rtw_debugfs_init(struct rtw_dev *rtwdev) rtw_debugfs_add_rw(rsvd_page); rtw_debugfs_add_r(phy_info); rtw_debugfs_add_r(coex_info); + rtw_debugfs_add_rw(coex_enable); rtw_debugfs_add_r(mac_0); rtw_debugfs_add_r(mac_1); rtw_debugfs_add_r(mac_2);