| Message ID | 20210429080149.7068-1-pkshih@realtek.com |
|---|---|
| Headers | show |
| Series | rtw89: add Realtek 802.11ax driver | expand |
Hi, On Thu, Apr 29, 2021 at 04:01:43PM +0800, Ping-Ke Shih wrote: > --- /dev/null > +++ b/drivers/net/wireless/realtek/rtw89/rtw8852a.c > @@ -0,0 +1,2047 @@ ... > +static void rtw8852a_btc_init_cfg(struct rtw89_dev *rtwdev) > +{ > + struct rtw89_btc *btc = &rtwdev->btc; > + struct rtw89_btc_module *module = &btc->mdinfo; > + const struct rtw89_chip_info *chip = rtwdev->chip; > + const struct rtw89_mac_ax_coex coex_params = { > + .pta_mode = RTW89_MAC_AX_COEX_RTK_MODE, > + .direction = RTW89_MAC_AX_COEX_INNER, > + }; > + > + /* PTA init */ > + rtw89_mac_coex_init(rtwdev, &coex_params); > + > + /* set WL Tx response = Hi-Pri */ > + chip->ops->btc_set_wl_pri(rtwdev, BTC_PRI_MASK_TX_RESP, true); > + > + /* set rf gnt debug off */ > + rtw89_write_rf(rtwdev, RF_PATH_A, RR_WLSEL, 0xfffff, 0x0); I fired this up and quickly ran into this: [ 1746.061015] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:281 [ 1746.061029] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 838, name: wpa_supplicant [ 1746.061037] CPU: 2 PID: 838 Comm: wpa_supplicant Tainted: G W 5.10.27 #3 ... [ 1746.061047] Call Trace: [ 1746.061061] dump_stack+0x9e/0xe9 [ 1746.061080] ___might_sleep+0x154/0x16a [ 1746.061093] mutex_lock+0x20/0x3c [ 1746.061106] rtw8852a_btc_init_cfg+0x60/0x177 [rtw89_core] [ 1746.061127] rtw89_btc_ntfy_radio_state+0xb8/0x115 [rtw89_core] [ 1746.061153] __iterate_interfaces+0xa9/0x109 [mac80211] [ 1746.061165] ? rtw89_leave_lps+0x44/0x44 [rtw89_core] [ 1746.061175] ? rtw89_leave_lps+0x44/0x44 [rtw89_core] [ 1746.061194] ieee80211_iterate_active_interfaces_atomic+0x33/0x40 [mac80211] [ 1746.061205] rtw89_ops_sw_scan_start+0x2e/0x48 [rtw89_core] [ 1746.061234] drv_sw_scan_start+0x97/0xf0 [mac80211] [ 1746.061261] __ieee80211_start_scan+0x3c7/0x4ae [mac80211] [ 1746.061284] ieee80211_request_scan+0x33/0x4f [mac80211] [ 1746.061307] rdev_scan+0x72/0xd1 [cfg80211] [ 1746.061335] nl80211_trigger_scan+0x610/0x669 [cfg80211] [ 1746.061349] genl_rcv_msg+0x3b0/0x3e0 [ 1746.061372] ? nl80211_update_mesh_config+0x1b7/0x1b7 [cfg80211] [ 1746.061382] ? genl_rcv+0x36/0x36 [ 1746.061387] netlink_rcv_skb+0x89/0xfb [ 1746.061394] genl_rcv+0x28/0x36 [ 1746.061400] netlink_unicast+0x169/0x23b [ 1746.061408] netlink_sendmsg+0x379/0x3f1 [ 1746.061416] sock_sendmsg+0x72/0x76 [ 1746.061423] ____sys_sendmsg+0x171/0x1ea [ 1746.061429] ? copy_msghdr_from_user+0x82/0xaa [ 1746.061436] ___sys_sendmsg+0xa0/0xdc [ 1746.061445] ? _copy_from_user+0x70/0x9c [ 1746.061451] __sys_sendmsg+0x8c/0xc6 [ 1746.061460] do_syscall_64+0x43/0x55 [ 1746.061467] entry_SYSCALL_64_after_hwframe+0x44/0xa9 rtw89_write_rf() is holding a mutex (rf_mutex). Judging by its trivial usage (it's only protecting register reads/writes), it probably could be a spinlock instead -- although I do note some magic udelay()s in there. Alternatively, it looks like you'd be safe moving to the non-atomic ieee80211_iterate_active_interfaces() in rtw89_leave_lps(). Brian > + rtw89_write_rf(rtwdev, RF_PATH_B, RR_WLSEL, 0xfffff, 0x0); > + > + /* set WL Tx thru in TRX mask table if GNT_WL = 0 && BT_S1 = ss group */ > + if (module->ant.type == BTC_ANT_SHARED) { > + rtw8852a_set_trx_mask(rtwdev, > + RF_PATH_A, BTC_BT_SS_GROUP, 0x5ff); > + rtw8852a_set_trx_mask(rtwdev, > + RF_PATH_B, BTC_BT_SS_GROUP, 0x5ff); > + } else { /* set WL Tx stb if GNT_WL = 0 && BT_S1 = ss group for 3-ant */ > + rtw8852a_set_trx_mask(rtwdev, > + RF_PATH_A, BTC_BT_SS_GROUP, 0x5df); > + rtw8852a_set_trx_mask(rtwdev, > + RF_PATH_B, BTC_BT_SS_GROUP, 0x5df); > + } > + > + /* set PTA break table */ > + rtw89_write32(rtwdev, R_BTC_BREAK_TABLE, BTC_BREAK_PARAM); > + > + /* enable BT counter 0xda40[16,2] = 2b'11 */ > + rtw89_write32_set(rtwdev, > + R_AX_CSR_MODE, B_AX_BT_CNT_REST | B_AX_STATIS_BT_EN); > + rtw89_mac_cfg_ctrl_path(rtwdev, false); > + btc->cx.wl.status.map.init_ok = true; > +}
On Thu, Apr 29, 2021 at 11:43:12PM +0000, Pkshih wrote: > On Thu, 2021-04-29 at 21:10 +0000, Brian Norris wrote: > > rtw89_write_rf() is holding a mutex (rf_mutex). Judging by its trivial > > usage (it's only protecting register reads/writes), it probably could be > > a spinlock instead -- although I do note some magic udelay()s in there. > > > > The udelay() is needed to ensure the indirect-write correct. OK. Maybe deserves a comment for the future. Is this a hardware-specified timing (measured in number of cycles or similar, on the WiFi chip side), or something you're just guessing at? > > Alternatively, it looks like you'd be safe moving to the non-atomic > > ieee80211_iterate_active_interfaces() in rtw89_leave_lps(). > > > > For most cases of rtw89_leave_lps(), we can use ieee80211_iterate_active_interfaces(), > which hold iflist_mtx lock, except to > > ieee80211_recalc_ps(local); // held iflist_mtx lock > ... > ieee80211_hw_config > ... > rtw89_leave_lps() > ... > ieee80211_iterate_active_interfaces() > > That will leads deadlock. Good point. > Another variant ieee80211_iterate_active_interfaces_mtx() that doesn't > hold a lock may be a solution. The the comment says "This version can > only be used while holding the RTNL.", and the code within the function > says "lockdep_assert_wiphy(hw->wiphy);". I'm not sure if I can use it > to prevent locking iflist_mtx twice. This doesn't quite feel like the right thing. You're in the midst of many other callback layers, and I don't think this is the right place to be grabbing those locks. But I haven't researched this very closely yet. > If I can use it, I can add a argument 'mtx', like rtw89_leave_lps(rtwdev, bool mtx), > to judge using ieee80211_iterate_active_interfaces() or ieee80211_iterate_active_interfaces_mtx(). > > I'm also thinking that we can still use ieee80211_iterate_active_interfaces_atomic() > to merely collect rtwvif->mac_id list, and use a loop to do leave_lps > out of the atomic iterate. That's probably safe, because we're already holding rtwdev->mutex, so there's no chance of our mac_id going away. If that solution isn't too complex, it makes sense to me. Brian
On Fri, 2021-04-30 at 01:22 +0000, Brian Norris wrote: > On Thu, Apr 29, 2021 at 11:43:12PM +0000, Pkshih wrote: > > On Thu, 2021-04-29 at 21:10 +0000, Brian Norris wrote: > > > rtw89_write_rf() is holding a mutex (rf_mutex). Judging by its trivial > > > usage (it's only protecting register reads/writes), it probably could be > > > a spinlock instead -- although I do note some magic udelay()s in there. > > > > > > > The udelay() is needed to ensure the indirect-write correct. > > OK. Maybe deserves a comment for the future. Is this a > hardware-specified timing (measured in number of cycles or similar, on > the WiFi chip side), or something you're just guessing at? > This is estimated by designer. > > > Alternatively, it looks like you'd be safe moving to the non-atomic > > > ieee80211_iterate_active_interfaces() in rtw89_leave_lps(). > > > > > > > For most cases of rtw89_leave_lps(), we can use ieee80211_iterate_active_interfaces(), > > which hold iflist_mtx lock, except to > > > > ieee80211_recalc_ps(local); // held iflist_mtx lock > > ... > > ieee80211_hw_config > > ... > > rtw89_leave_lps() > > ... > > ieee80211_iterate_active_interfaces() > > > > That will leads deadlock. > > Good point. > > > Another variant ieee80211_iterate_active_interfaces_mtx() that doesn't > > hold a lock may be a solution. The the comment says "This version can > > only be used while holding the RTNL.", and the code within the function > > says "lockdep_assert_wiphy(hw->wiphy);". I'm not sure if I can use it > > to prevent locking iflist_mtx twice. > > This doesn't quite feel like the right thing. You're in the midst of > many other callback layers, and I don't think this is the right place to > be grabbing those locks. But I haven't researched this very closely yet. > > > If I can use it, I can add a argument 'mtx', like rtw89_leave_lps(rtwdev, bool mtx), > > to judge using ieee80211_iterate_active_interfaces() or ieee80211_iterate_active_interfaces_mtx(). > > > > I'm also thinking that we can still use ieee80211_iterate_active_interfaces_atomic() > > to merely collect rtwvif->mac_id list, and use a loop to do leave_lps > > out of the atomic iterate. > > That's probably safe, because we're already holding rtwdev->mutex, so > there's no chance of our mac_id going away. If that solution isn't too > complex, it makes sense to me. > I drop the rtw89_iterate_vifs_atomic() that cannot sleep, but it's hard to avoid calling sleep functions. To make other programmers easier to choose iterate functions, I re-design them according to running context, and listed below context iflist_mtx iterate vifs functions ------- ---------- ---------------------- BH N/A rtw89_iterate_vifs_bh() thread not held rtw89_iterate_vifs(,,,false); thread held rtw89_iterate_vifs(,,,true); It's clear that iterators of rtw89_iterate_vifs_bh() can't call sleep functions. Iterators of rtw89_iterate_vifs() can always call sleep functions, but additional forth argument must be set correctly. Next submission will include this change. -- Ping-Ke
This driver named rtw89, which is the next generation of rtw88, supports Realtek 8852AE 802.11ax 2x2 chip whose new features are OFDMA, DBCC, Spatial reuse, TWT and BSS coloring; now some of them aren't implemented though. The chip architecture is entirely different from the chips supported by rtw88 like RTL8822CE 802.11ac chip. First of all, register address ranges are totally redefined, so it's impossible to reuse register definition. To communicate with firmware, new H2C/C2H format is proposed. In order to have better utilization, TX DMA flow is changed to two stages DMA. To provide rich RX status information, additional RX PPDU packets are added. Since there are so many differences mentioned above, we decide to propose a new driver. It has many authors, they are listed in alphabetic order: Chin-Yen Lee <timlee@realtek.com> Ping-Ke Shih <pkshih@realtek.com> Po Hao Huang <phhuang@realtek.com> Tzu-En Huang <tehuang@realtek.com> Vincent Fann <vincent_fann@realtek.com> Yan-Hsuan Chuang <tony0620emma@gmail.com> Zong-Zhe Yang <kevin_yang@realtek.com> v4: - add basic BT coexistence features - add power save mode, so an new patch (two files) is added - fine tune performance - add debugfs for debugging coex, bb, ... v3: - fix "networking block comments" reported by checkpatch - Add MODULE_DEVICE_TABLE() generated by Thomas Backlund <tmb@mageia.org> - Add missed BB settings - error handle of RX BD and DESC length - reduce debug level of C2H ACKs - fix rekey failure due to wrong operator v2: - fix compiler warnings made by W=1 Reported-by: kernel test robot <lkp@intel.com> - sort header file alphabetically - fix "networking block comments" reported by checkpatch Ping-Ke Shih (19): rtw89: add CAM files rtw89: add BT coexistence files rtw89: add core and trx files rtw89: add debug files rtw89: add efuse files rtw89: add files to download and communicate with firmware rtw89: add MAC files rtw89: implement mac80211 ops rtw89: add pci files rtw89: add phy files rtw89: define register names rtw89: add regulatory support rtw89: 8852a: add 8852a specific files rtw89: 8852a: add 8852a RFK files rtw89: 8852a: add 8852a RFK tables rtw89: 8852a: add 8852a tables rtw89: add ser to recover error reported by firmware rtw89: add PS files rtw89: add Kconfig and Makefile drivers/net/wireless/realtek/Kconfig | 1 + drivers/net/wireless/realtek/Makefile | 1 + drivers/net/wireless/realtek/rtw89/Kconfig | 50 + drivers/net/wireless/realtek/rtw89/Makefile | 24 + drivers/net/wireless/realtek/rtw89/cam.c | 680 + drivers/net/wireless/realtek/rtw89/cam.h | 164 + drivers/net/wireless/realtek/rtw89/coex.c | 5455 ++ drivers/net/wireless/realtek/rtw89/coex.h | 152 + drivers/net/wireless/realtek/rtw89/core.c | 2195 + drivers/net/wireless/realtek/rtw89/core.h | 3239 + drivers/net/wireless/realtek/rtw89/debug.c | 2367 + drivers/net/wireless/realtek/rtw89/debug.h | 77 + drivers/net/wireless/realtek/rtw89/efuse.c | 188 + drivers/net/wireless/realtek/rtw89/efuse.h | 13 + drivers/net/wireless/realtek/rtw89/fw.c | 1316 + drivers/net/wireless/realtek/rtw89/fw.h | 1150 + drivers/net/wireless/realtek/rtw89/mac.c | 3639 ++ drivers/net/wireless/realtek/rtw89/mac.h | 854 + drivers/net/wireless/realtek/rtw89/mac80211.c | 568 + drivers/net/wireless/realtek/rtw89/pci.c | 2918 + drivers/net/wireless/realtek/rtw89/pci.h | 565 + drivers/net/wireless/realtek/rtw89/phy.c | 2538 + drivers/net/wireless/realtek/rtw89/phy.h | 295 + drivers/net/wireless/realtek/rtw89/ps.c | 148 + drivers/net/wireless/realtek/rtw89/ps.h | 15 + drivers/net/wireless/realtek/rtw89/reg.h | 2037 + drivers/net/wireless/realtek/rtw89/regd.c | 351 + drivers/net/wireless/realtek/rtw89/rtw8852a.c | 2047 + drivers/net/wireless/realtek/rtw89/rtw8852a.h | 109 + .../net/wireless/realtek/rtw89/rtw8852a_rfk.c | 3806 ++ .../net/wireless/realtek/rtw89/rtw8852a_rfk.h | 22 + .../realtek/rtw89/rtw8852a_rfk_table.c | 1596 + .../realtek/rtw89/rtw8852a_rfk_table.h | 132 + .../wireless/realtek/rtw89/rtw8852a_table.c | 48725 ++++++++++++++++ .../wireless/realtek/rtw89/rtw8852a_table.h | 28 + drivers/net/wireless/realtek/rtw89/ser.c | 435 + drivers/net/wireless/realtek/rtw89/ser.h | 14 + drivers/net/wireless/realtek/rtw89/txrx.h | 389 + 38 files changed, 88303 insertions(+) create mode 100644 drivers/net/wireless/realtek/rtw89/Kconfig create mode 100644 drivers/net/wireless/realtek/rtw89/Makefile create mode 100644 drivers/net/wireless/realtek/rtw89/cam.c create mode 100644 drivers/net/wireless/realtek/rtw89/cam.h create mode 100644 drivers/net/wireless/realtek/rtw89/coex.c create mode 100644 drivers/net/wireless/realtek/rtw89/coex.h create mode 100644 drivers/net/wireless/realtek/rtw89/core.c create mode 100644 drivers/net/wireless/realtek/rtw89/core.h create mode 100644 drivers/net/wireless/realtek/rtw89/debug.c create mode 100644 drivers/net/wireless/realtek/rtw89/debug.h create mode 100644 drivers/net/wireless/realtek/rtw89/efuse.c create mode 100644 drivers/net/wireless/realtek/rtw89/efuse.h create mode 100644 drivers/net/wireless/realtek/rtw89/fw.c create mode 100644 drivers/net/wireless/realtek/rtw89/fw.h create mode 100644 drivers/net/wireless/realtek/rtw89/mac.c create mode 100644 drivers/net/wireless/realtek/rtw89/mac.h create mode 100644 drivers/net/wireless/realtek/rtw89/mac80211.c create mode 100644 drivers/net/wireless/realtek/rtw89/pci.c create mode 100644 drivers/net/wireless/realtek/rtw89/pci.h create mode 100644 drivers/net/wireless/realtek/rtw89/phy.c create mode 100644 drivers/net/wireless/realtek/rtw89/phy.h create mode 100644 drivers/net/wireless/realtek/rtw89/ps.c create mode 100644 drivers/net/wireless/realtek/rtw89/ps.h create mode 100644 drivers/net/wireless/realtek/rtw89/reg.h create mode 100644 drivers/net/wireless/realtek/rtw89/regd.c create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a.c create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a.h create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.h create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a_rfk_table.c create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a_rfk_table.h create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a_table.c create mode 100644 drivers/net/wireless/realtek/rtw89/rtw8852a_table.h create mode 100644 drivers/net/wireless/realtek/rtw89/ser.c create mode 100644 drivers/net/wireless/realtek/rtw89/ser.h create mode 100644 drivers/net/wireless/realtek/rtw89/txrx.h