| Message ID | 20170702181239.17911-3-Larry.Finger@lwfinger.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6aad6075ccd540910438fb0eaa0264c886f36304 |
| Delegated to: | Kalle Valo |
| Headers | show |
On 2-7-2017 20:12, Larry Finger wrote: > From: Ping-Ke Shih <pkshih@realtek.com> > > We use H2C to ask BT's status, and C2H will return the status. > > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> > Cc: Yan-Hsuan Chuang <yhchuang@realtek.com> > Cc: Birming Chiu <birming@realtek.com> > Cc: Shaofu <shaofu@realtek.com> > Cc: Steven Ting <steventing@realtek.com> > --- > v2 - no changes > v3 - no changes > v4 - no changes > --- > .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 17 ++++++++++++- > .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.c | 28 ++++++++++++++++++++++ > .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.h | 1 + > .../net/wireless/realtek/rtlwifi/rtl8192ee/fw.c | 9 +++++-- > .../net/wireless/realtek/rtlwifi/rtl8723be/fw.c | 9 +++++-- > .../net/wireless/realtek/rtlwifi/rtl8821ae/fw.c | 13 +++++++--- > drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 ++ > 7 files changed, 71 insertions(+), 8 deletions(-) > [...] > > +void rtl_btc_btmpinfo_notify(struct rtl_priv *rtlpriv, u8 *tmp_buf, u8 length) > +{ > + u8 extid, seq, len; > + u16 bt_real_fw_ver; > + u8 bt_fw_ver; > + > + if ((length < 4) || (!tmp_buf)) > + return; > + > + extid = tmp_buf[0]; > + /* not response from BT FW then exit*/ > + if (extid != 1) /* C2H_TRIG_BY_BT_FW = 1 */ > + return; > + > + len = tmp_buf[1] >> 4; > + seq = tmp_buf[2] >> 4; > + > + /* BT Firmware version response */ > + if (seq == 0x0E) { > + bt_real_fw_ver = tmp_buf[3] | (tmp_buf[4] << 8); > + bt_fw_ver = tmp_buf[5]; > + > + gl_bt_coexist.bt_info.bt_real_fw_ver = bt_real_fw_ver; > + gl_bt_coexist.bt_info.bt_fw_ver = bt_fw_ver; > + } > +} Just stumbled upon this and curious. I assume gl_bt_coexist is a global variable so I guess this code will not work when running multiple realtek device with btcoexist on your system. Regards, Arend
Arend Van Spriel <arend.vanspriel@broadcom.com> writes: > On 2-7-2017 20:12, Larry Finger wrote: >> From: Ping-Ke Shih <pkshih@realtek.com> >> >> We use H2C to ask BT's status, and C2H will return the status. >> >> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com> >> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> >> Cc: Yan-Hsuan Chuang <yhchuang@realtek.com> >> Cc: Birming Chiu <birming@realtek.com> >> Cc: Shaofu <shaofu@realtek.com> >> Cc: Steven Ting <steventing@realtek.com> >> --- >> v2 - no changes >> v3 - no changes >> v4 - no changes >> --- >> .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 17 ++++++++++++- >> .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.c | 28 ++++++++++++++++++++++ >> .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.h | 1 + >> .../net/wireless/realtek/rtlwifi/rtl8192ee/fw.c | 9 +++++-- >> .../net/wireless/realtek/rtlwifi/rtl8723be/fw.c | 9 +++++-- >> .../net/wireless/realtek/rtlwifi/rtl8821ae/fw.c | 13 +++++++--- >> drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 ++ >> 7 files changed, 71 insertions(+), 8 deletions(-) >> > > [...] > >> >> +void rtl_btc_btmpinfo_notify(struct rtl_priv *rtlpriv, u8 *tmp_buf, u8 length) >> +{ >> + u8 extid, seq, len; >> + u16 bt_real_fw_ver; >> + u8 bt_fw_ver; >> + >> + if ((length < 4) || (!tmp_buf)) >> + return; >> + >> + extid = tmp_buf[0]; >> + /* not response from BT FW then exit*/ >> + if (extid != 1) /* C2H_TRIG_BY_BT_FW = 1 */ >> + return; >> + >> + len = tmp_buf[1] >> 4; >> + seq = tmp_buf[2] >> 4; >> + >> + /* BT Firmware version response */ >> + if (seq == 0x0E) { >> + bt_real_fw_ver = tmp_buf[3] | (tmp_buf[4] << 8); >> + bt_fw_ver = tmp_buf[5]; >> + >> + gl_bt_coexist.bt_info.bt_real_fw_ver = bt_real_fw_ver; >> + gl_bt_coexist.bt_info.bt_fw_ver = bt_fw_ver; >> + } >> +} > > Just stumbled upon this and curious. I assume gl_bt_coexist is a global > variable so I guess this code will not work when running multiple > realtek device with btcoexist on your system. Good catch, rtlwifi/btcoexist/halbtcoutsrc.c contains two global variables even with a big fat comment stating that: ---------------------------------------------------------------------- /*********************************************** * Global variables ***********************************************/ struct btc_coexist gl_bt_coexist; u32 btc_dbg_type[BTC_MSG_MAX]; ---------------------------------------------------------------------- Global variables do not belong to upstream drivers, instead they should be in per device structures so that multiple devices can be used at the same time. Now that seems to be horribly broken. These variables were added back in 2014: aa45a673b291 rtlwifi: btcoexist: Add new mini driver
Larry Finger <Larry.Finger@lwfinger.net> writes: > From: Ping-Ke Shih <pkshih@realtek.com> > > We use H2C to ask BT's status, and C2H will return the status. > > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> > Cc: Yan-Hsuan Chuang <yhchuang@realtek.com> > Cc: Birming Chiu <birming@realtek.com> > Cc: Shaofu <shaofu@realtek.com> > Cc: Steven Ting <steventing@realtek.com> [...] > --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c > +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c > @@ -327,7 +327,22 @@ static void halbtc_aggregation_check(struct btc_coexist *btcoexist) > > static u32 halbtc_get_bt_patch_version(struct btc_coexist *btcoexist) > { > - return 0; > + struct rtl_priv *rtlpriv = btcoexist->adapter; > + u8 cmd_buffer[4] = {0}; > + u8 oper_ver = 0; > + u8 req_num = 0x0E; > + > + if (btcoexist->bt_info.bt_real_fw_ver) > + goto label_done; > + > + cmd_buffer[0] |= (oper_ver & 0x0f); /* Set OperVer */ > + cmd_buffer[0] |= ((req_num << 4) & 0xf0); /* Set ReqNum */ > + cmd_buffer[1] = 0; /* BT_OP_GET_BT_VERSION = 0 */ > + rtlpriv->cfg->ops->fill_h2c_cmd(rtlpriv->mac80211.hw, 0x67, 4, > + &cmd_buffer[0]); > + > +label_done: > + return btcoexist->bt_info.bt_real_fw_ver; > } I don't remember if I have mentioned before but there's way too much magic numbers. There should an enum for all req_num values and cmd_buffer should be a struct so that is serves as documentation at the same time. No need to resend because of this but take this into account in the future. As an example rsi driver is having similar problems and Prameela is fixing those: https://patchwork.kernel.org/patch/9832895/
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c index e6024b013ca5..c1eacd8352a2 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c @@ -327,7 +327,22 @@ static void halbtc_aggregation_check(struct btc_coexist *btcoexist) static u32 halbtc_get_bt_patch_version(struct btc_coexist *btcoexist) { - return 0; + struct rtl_priv *rtlpriv = btcoexist->adapter; + u8 cmd_buffer[4] = {0}; + u8 oper_ver = 0; + u8 req_num = 0x0E; + + if (btcoexist->bt_info.bt_real_fw_ver) + goto label_done; + + cmd_buffer[0] |= (oper_ver & 0x0f); /* Set OperVer */ + cmd_buffer[0] |= ((req_num << 4) & 0xf0); /* Set ReqNum */ + cmd_buffer[1] = 0; /* BT_OP_GET_BT_VERSION = 0 */ + rtlpriv->cfg->ops->fill_h2c_cmd(rtlpriv->mac80211.hw, 0x67, 4, + &cmd_buffer[0]); + +label_done: + return btcoexist->bt_info.bt_real_fw_ver; } u32 halbtc_get_wifi_link_status(struct btc_coexist *btcoexist) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c index 4366c9817e1e..7d296a401b6f 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c @@ -41,6 +41,7 @@ static struct rtl_btc_ops rtl_btc_operation = { .btc_periodical = rtl_btc_periodical, .btc_halt_notify = rtl_btc_halt_notify, .btc_btinfo_notify = rtl_btc_btinfo_notify, + .btc_btmpinfo_notify = rtl_btc_btmpinfo_notify, .btc_is_limited_dig = rtl_btc_is_limited_dig, .btc_is_disable_edca_turbo = rtl_btc_is_disable_edca_turbo, .btc_is_bt_disabled = rtl_btc_is_bt_disabled, @@ -165,6 +166,33 @@ void rtl_btc_btinfo_notify(struct rtl_priv *rtlpriv, u8 *tmp_buf, u8 length) exhalbtc_bt_info_notify(&gl_bt_coexist, tmp_buf, length); } +void rtl_btc_btmpinfo_notify(struct rtl_priv *rtlpriv, u8 *tmp_buf, u8 length) +{ + u8 extid, seq, len; + u16 bt_real_fw_ver; + u8 bt_fw_ver; + + if ((length < 4) || (!tmp_buf)) + return; + + extid = tmp_buf[0]; + /* not response from BT FW then exit*/ + if (extid != 1) /* C2H_TRIG_BY_BT_FW = 1 */ + return; + + len = tmp_buf[1] >> 4; + seq = tmp_buf[2] >> 4; + + /* BT Firmware version response */ + if (seq == 0x0E) { + bt_real_fw_ver = tmp_buf[3] | (tmp_buf[4] << 8); + bt_fw_ver = tmp_buf[5]; + + gl_bt_coexist.bt_info.bt_real_fw_ver = bt_real_fw_ver; + gl_bt_coexist.bt_info.bt_fw_ver = bt_fw_ver; + } +} + bool rtl_btc_is_limited_dig(struct rtl_priv *rtlpriv) { return gl_bt_coexist.bt_info.limited_dig; diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h index 6fe521cbe7f0..ac1253c46f44 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h @@ -39,6 +39,7 @@ void rtl_btc_mediastatus_notify(struct rtl_priv *rtlpriv, void rtl_btc_periodical(struct rtl_priv *rtlpriv); void rtl_btc_halt_notify(void); void rtl_btc_btinfo_notify(struct rtl_priv *rtlpriv, u8 *tmpbuf, u8 length); +void rtl_btc_btmpinfo_notify(struct rtl_priv *rtlpriv, u8 *tmp_buf, u8 length); bool rtl_btc_is_limited_dig(struct rtl_priv *rtlpriv); bool rtl_btc_is_disable_edca_turbo(struct rtl_priv *rtlpriv); bool rtl_btc_is_bt_disabled(struct rtl_priv *rtlpriv); diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/fw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/fw.c index f5d4df985c37..7eae27f8e173 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/fw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/fw.c @@ -887,6 +887,7 @@ void rtl92ee_c2h_content_parsing(struct ieee80211_hw *hw, u8 c2h_cmd_id, u8 c2h_cmd_len, u8 *tmp_buf) { struct rtl_priv *rtlpriv = rtl_priv(hw); + struct rtl_btc_ops *btc_ops = rtlpriv->btcoexist.btc_ops; switch (c2h_cmd_id) { case C2H_8192E_DBG: @@ -905,12 +906,16 @@ void rtl92ee_c2h_content_parsing(struct ieee80211_hw *hw, u8 c2h_cmd_id, case C2H_8192E_BT_INFO: RT_TRACE(rtlpriv, COMP_FW, DBG_TRACE, "[C2H], C2H_8723BE_BT_INFO!!\n"); - rtlpriv->btcoexist.btc_ops->btc_btinfo_notify(rtlpriv, tmp_buf, - c2h_cmd_len); + if (rtlpriv->cfg->ops->get_btc_status()) + btc_ops->btc_btinfo_notify(rtlpriv, tmp_buf, + c2h_cmd_len); break; case C2H_8192E_BT_MP: RT_TRACE(rtlpriv, COMP_FW, DBG_TRACE, "[C2H], C2H_8723BE_BT_MP!!\n"); + if (rtlpriv->cfg->ops->get_btc_status()) + btc_ops->btc_btmpinfo_notify(rtlpriv, tmp_buf, + c2h_cmd_len); break; case C2H_8192E_RA_RPT: _rtl92ee_c2h_ra_report_handler(hw, tmp_buf, c2h_cmd_len); diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/fw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/fw.c index dd6f95cfaec9..4b963fd27d64 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/fw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/fw.c @@ -709,6 +709,7 @@ void rtl8723be_c2h_content_parsing(struct ieee80211_hw *hw, u8 c2h_cmd_len, u8 *tmp_buf) { struct rtl_priv *rtlpriv = rtl_priv(hw); + struct rtl_btc_ops *btc_ops = rtlpriv->btcoexist.btc_ops; switch (c2h_cmd_id) { case C2H_8723B_DBG: @@ -723,12 +724,16 @@ void rtl8723be_c2h_content_parsing(struct ieee80211_hw *hw, case C2H_8723B_BT_INFO: RT_TRACE(rtlpriv, COMP_FW, DBG_TRACE, "[C2H], C2H_8723BE_BT_INFO!!\n"); - rtlpriv->btcoexist.btc_ops->btc_btinfo_notify(rtlpriv, tmp_buf, - c2h_cmd_len); + if (rtlpriv->cfg->ops->get_btc_status()) + btc_ops->btc_btinfo_notify(rtlpriv, tmp_buf, + c2h_cmd_len); break; case C2H_8723B_BT_MP: RT_TRACE(rtlpriv, COMP_FW, DBG_TRACE, "[C2H], C2H_8723BE_BT_MP!!\n"); + if (rtlpriv->cfg->ops->get_btc_status()) + btc_ops->btc_btmpinfo_notify(rtlpriv, tmp_buf, + c2h_cmd_len); break; default: RT_TRACE(rtlpriv, COMP_FW, DBG_TRACE, diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/fw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/fw.c index 03259aa150fd..b84b4fa7b71c 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/fw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/fw.c @@ -1923,6 +1923,7 @@ void rtl8821ae_c2h_content_parsing(struct ieee80211_hw *hw, u8 *tmp_buf) { struct rtl_priv *rtlpriv = rtl_priv(hw); + struct rtl_btc_ops *btc_ops = rtlpriv->btcoexist.btc_ops; switch (c2h_cmd_id) { case C2H_8812_DBG: @@ -1938,9 +1939,15 @@ void rtl8821ae_c2h_content_parsing(struct ieee80211_hw *hw, RT_TRACE(rtlpriv, COMP_FW, DBG_LOUD, "[C2H], C2H_8812_BT_INFO!!\n"); if (rtlpriv->cfg->ops->get_btc_status()) - rtlpriv->btcoexist.btc_ops->btc_btinfo_notify(rtlpriv, - tmp_buf, - c2h_cmd_len); + btc_ops->btc_btinfo_notify(rtlpriv, tmp_buf, + c2h_cmd_len); + break; + case C2H_8812_BT_MP: + RT_TRACE(rtlpriv, COMP_FW, DBG_TRACE, + "[C2H], C2H_8812_BT_MP!!\n"); + if (rtlpriv->cfg->ops->get_btc_status()) + btc_ops->btc_btmpinfo_notify(rtlpriv, tmp_buf, + c2h_cmd_len); break; default: break; diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h index 7ec0d502a0d9..77c3b186900e 100644 --- a/drivers/net/wireless/realtek/rtlwifi/wifi.h +++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h @@ -2562,6 +2562,8 @@ struct rtl_btc_ops { void (*btc_halt_notify) (void); void (*btc_btinfo_notify) (struct rtl_priv *rtlpriv, u8 *tmp_buf, u8 length); + void (*btc_btmpinfo_notify)(struct rtl_priv *rtlpriv, + u8 *tmp_buf, u8 length); bool (*btc_is_limited_dig) (struct rtl_priv *rtlpriv); bool (*btc_is_disable_edca_turbo) (struct rtl_priv *rtlpriv); bool (*btc_is_bt_disabled) (struct rtl_priv *rtlpriv);