Re: [PATCH v4 02/10] rtlwifi: Add BT_MP_INFO to c2h handler.

[Kalle Valo]

Larry Finger  writes:

> From: Ping-Ke Shih 
>
> We use H2C to ask BT's status, and C2H will return the status.
>
> Signed-off-by: Ping-Ke Shih 
> Signed-off-by: Larry Finger 
> Cc: Yan-Hsuan Chuang 
> Cc: Birming Chiu 
> Cc: Shaofu 
> Cc: Steven Ting 

[...]

> --- 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,
> + _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/

-- 
Kalle Valo

Re: [PATCH v4 02/10] rtlwifi: Add BT_MP_INFO to c2h handler.

[Kalle Valo]

Arend Van Spriel  writes:

> On 2-7-2017 20:12, Larry Finger wrote:
>> From: Ping-Ke Shih 
>> 
>> We use H2C to ask BT's status, and C2H will return the status.
>> 
>> Signed-off-by: Ping-Ke Shih 
>> Signed-off-by: Larry Finger 
>> Cc: Yan-Hsuan Chuang 
>> Cc: Birming Chiu 
>> Cc: Shaofu 
>> Cc: Steven Ting 
>> ---
>> 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

-- 
Kalle Valo

Re: [PATCH v4 02/10] rtlwifi: Add BT_MP_INFO to c2h handler.

[Arend Van Spriel]

On 2-7-2017 20:12, Larry Finger wrote:
> From: Ping-Ke Shih 
> 
> We use H2C to ask BT's status, and C2H will return the status.
> 
> Signed-off-by: Ping-Ke Shih 
> Signed-off-by: Larry Finger 
> Cc: Yan-Hsuan Chuang 
> Cc: Birming Chiu 
> Cc: Shaofu 
> Cc: Steven Ting 
> ---
> 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

[PATCH v4 02/10] rtlwifi: Add BT_MP_INFO to c2h handler.

[Larry Finger]

From: Ping-Ke Shih 

We use H2C to ask BT's status, and C2H will return the status.

Signed-off-by: Ping-Ke Shih 
Signed-off-by: Larry Finger 
Cc: Yan-Hsuan Chuang 
Cc: Birming Chiu 
Cc: Shaofu 
Cc: Steven Ting 
---
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(-)

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,
+   _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(_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