Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
@ 2019-09-11  2:50 Chris Chiu
  2019-09-19  1:44 ` Chris Chiu
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Chris Chiu @ 2019-09-11  2:50 UTC (permalink / raw)
  To: Jes.Sorensen, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, linux

The RTL8723BU suffers the wifi disconnection problem while bluetooth
device connected. While wifi is doing tx/rx, the bluetooth will scan
without results. This is due to the wifi and bluetooth share the same
single antenna for RF communication and they need to have a mechanism
to collaborate.

BT information is provided via the packet sent from co-processor to
host (C2H). It contains the status of BT but the rtl8723bu_handle_c2h
dose not really handle it. And there's no bluetooth coexistence
mechanism to deal with it.

This commit adds a workqueue to set the tdma configurations and
coefficient table per the parsed bluetooth link status and given
wifi connection state. The tdma/coef table comes from the vendor
driver code of the RTL8192EU and RTL8723BU. However, this commit is
only for single antenna scenario which RTL8192EU is default dual
antenna. The rtl8xxxu_parse_rxdesc24 which invokes the handle_c2h
is only for 8723b and 8192e so the mechanism is expected to work
on both chips with single antenna. Note RTL8192EU dual antenna is
not supported.

Signed-off-by: Chris Chiu <chiu@endlessm.com>
---

Notes:
  v2:
   - Add helper functions to replace bunch of tdma settings
   - Reformat some lines to meet kernel coding style


 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  37 +++
 .../realtek/rtl8xxxu/rtl8xxxu_8723b.c         |   2 -
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 262 +++++++++++++++++-
 3 files changed, 294 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 582c2a346cec..22e95b11bfbb 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1220,6 +1220,37 @@ enum ratr_table_mode_new {
 	RATEID_IDX_BGN_3SS = 14,
 };
 
+#define BT_INFO_8723B_1ANT_B_FTP		BIT(7)
+#define BT_INFO_8723B_1ANT_B_A2DP		BIT(6)
+#define BT_INFO_8723B_1ANT_B_HID		BIT(5)
+#define BT_INFO_8723B_1ANT_B_SCO_BUSY		BIT(4)
+#define BT_INFO_8723B_1ANT_B_ACL_BUSY		BIT(3)
+#define BT_INFO_8723B_1ANT_B_INQ_PAGE		BIT(2)
+#define BT_INFO_8723B_1ANT_B_SCO_ESCO		BIT(1)
+#define BT_INFO_8723B_1ANT_B_CONNECTION	BIT(0)
+
+enum _BT_8723B_1ANT_STATUS {
+	BT_8723B_1ANT_STATUS_NON_CONNECTED_IDLE      = 0x0,
+	BT_8723B_1ANT_STATUS_CONNECTED_IDLE          = 0x1,
+	BT_8723B_1ANT_STATUS_INQ_PAGE                = 0x2,
+	BT_8723B_1ANT_STATUS_ACL_BUSY                = 0x3,
+	BT_8723B_1ANT_STATUS_SCO_BUSY                = 0x4,
+	BT_8723B_1ANT_STATUS_ACL_SCO_BUSY            = 0x5,
+	BT_8723B_1ANT_STATUS_MAX
+};
+
+struct rtl8xxxu_btcoex {
+	u8      bt_status;
+	bool	bt_busy;
+	bool	has_sco;
+	bool	has_a2dp;
+	bool    has_hid;
+	bool    has_pan;
+	bool	hid_only;
+	bool	a2dp_only;
+	bool    c2h_bt_inquiry;
+};
+
 #define RTL8XXXU_RATR_STA_INIT 0
 #define RTL8XXXU_RATR_STA_HIGH 1
 #define RTL8XXXU_RATR_STA_MID  2
@@ -1340,6 +1371,10 @@ struct rtl8xxxu_priv {
 	 */
 	struct ieee80211_vif *vif;
 	struct delayed_work ra_watchdog;
+	struct work_struct c2hcmd_work;
+	struct sk_buff_head c2hcmd_queue;
+	spinlock_t c2hcmd_lock;
+	struct rtl8xxxu_btcoex bt_coex;
 };
 
 struct rtl8xxxu_rx_urb {
@@ -1486,6 +1521,8 @@ void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
 			     struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
 			     bool short_preamble, bool ampdu_enable,
 			     u32 rts_rate);
+void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
+			   u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5);
 
 extern struct rtl8xxxu_fileops rtl8192cu_fops;
 extern struct rtl8xxxu_fileops rtl8192eu_fops;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index ceffe05bd65b..9ba661b3d767 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1580,9 +1580,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
 	/*
 	 * Software control, antenna at WiFi side
 	 */
-#ifdef NEED_PS_TDMA
 	rtl8723bu_set_ps_tdma(priv, 0x08, 0x00, 0x00, 0x00, 0x00);
-#endif
 
 	rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
 	rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a6f358b9e447..e4c1b08c8070 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3820,9 +3820,8 @@ void rtl8xxxu_power_off(struct rtl8xxxu_priv *priv)
 	rtl8xxxu_write8(priv, REG_RSV_CTRL, 0x0e);
 }
 
-#ifdef NEED_PS_TDMA
-static void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
-				  u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5)
+void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
+			   u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5)
 {
 	struct h2c_cmd h2c;
 
@@ -3835,7 +3834,6 @@ static void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
 	h2c.b_type_dma.data5 = arg5;
 	rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.b_type_dma));
 }
-#endif
 
 void rtl8xxxu_gen2_disable_rf(struct rtl8xxxu_priv *priv)
 {
@@ -5186,12 +5184,258 @@ static void rtl8xxxu_rx_urb_work(struct work_struct *work)
 	}
 }
 
+void rtl8723bu_set_coex_with_type(struct rtl8xxxu_priv *priv, u8 type)
+{
+	switch (type) {
+	case 0:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	case 1:
+	case 3:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x5a5a5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	case 2:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x5a5a5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x5a5a5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	case 4:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x5a5a5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaaaa5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	case 5:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x5a5a5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaa5a5a5a);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	case 6:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaaaaaaaa);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	case 7:
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0xaaaaaaaa);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaaaaaaaa);
+		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
+		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
+		break;
+	default:
+		break;
+	}
+}
+
+void rtl8723bu_update_bt_link_info(struct rtl8xxxu_priv *priv, u8 bt_info)
+{
+	struct rtl8xxxu_btcoex *btcoex = &priv->bt_coex;
+
+	if (bt_info & BT_INFO_8723B_1ANT_B_INQ_PAGE)
+		btcoex->c2h_bt_inquiry = true;
+	else
+		btcoex->c2h_bt_inquiry = false;
+
+	if (!(bt_info & BT_INFO_8723B_1ANT_B_CONNECTION)) {
+		btcoex->bt_status = BT_8723B_1ANT_STATUS_NON_CONNECTED_IDLE;
+		btcoex->has_sco = false;
+		btcoex->has_hid = false;
+		btcoex->has_pan = false;
+		btcoex->has_a2dp = false;
+	} else {
+		if ((bt_info & 0x1f) == BT_INFO_8723B_1ANT_B_CONNECTION)
+			btcoex->bt_status = BT_8723B_1ANT_STATUS_CONNECTED_IDLE;
+		else if ((bt_info & BT_INFO_8723B_1ANT_B_SCO_ESCO) ||
+			 (bt_info & BT_INFO_8723B_1ANT_B_SCO_BUSY))
+			btcoex->bt_status = BT_8723B_1ANT_STATUS_SCO_BUSY;
+		else if (bt_info & BT_INFO_8723B_1ANT_B_ACL_BUSY)
+			btcoex->bt_status = BT_8723B_1ANT_STATUS_ACL_BUSY;
+		else
+			btcoex->bt_status = BT_8723B_1ANT_STATUS_MAX;
+
+		if (bt_info & BT_INFO_8723B_1ANT_B_FTP)
+			btcoex->has_pan = true;
+		else
+			btcoex->has_pan = false;
+
+		if (bt_info & BT_INFO_8723B_1ANT_B_A2DP)
+			btcoex->has_a2dp = true;
+		else
+			btcoex->has_a2dp = false;
+
+		if (bt_info & BT_INFO_8723B_1ANT_B_HID)
+			btcoex->has_hid = true;
+		else
+			btcoex->has_hid = false;
+
+		if (bt_info & BT_INFO_8723B_1ANT_B_SCO_ESCO)
+			btcoex->has_sco = true;
+		else
+			btcoex->has_sco = false;
+	}
+
+	if (!btcoex->has_a2dp && !btcoex->has_sco &&
+	    !btcoex->has_pan && btcoex->has_hid)
+		btcoex->hid_only = true;
+	else
+		btcoex->hid_only = false;
+
+	if (!btcoex->has_sco && !btcoex->has_pan &&
+	    !btcoex->has_hid && btcoex->has_a2dp)
+		btcoex->has_a2dp = true;
+	else
+		btcoex->has_a2dp = false;
+
+	if (btcoex->bt_status == BT_8723B_1ANT_STATUS_SCO_BUSY ||
+	    btcoex->bt_status == BT_8723B_1ANT_STATUS_ACL_BUSY)
+		btcoex->bt_busy = true;
+	else
+		btcoex->bt_busy = false;
+}
+
+void rtl8723bu_handle_bt_inquiry(struct rtl8xxxu_priv *priv)
+{
+	struct ieee80211_vif *vif;
+	struct rtl8xxxu_btcoex *btcoex;
+	bool wifi_connected;
+
+	vif = priv->vif;
+	btcoex = &priv->bt_coex;
+	wifi_connected = (vif && vif->bss_conf.assoc);
+
+	if (!wifi_connected) {
+		rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
+		rtl8723bu_set_coex_with_type(priv, 0);
+	} else if (btcoex->has_sco || btcoex->has_hid || btcoex->has_a2dp) {
+		rtl8723bu_set_ps_tdma(priv, 0x61, 0x35, 0x3, 0x11, 0x11);
+		rtl8723bu_set_coex_with_type(priv, 4);
+	} else if (btcoex->has_pan) {
+		rtl8723bu_set_ps_tdma(priv, 0x61, 0x3f, 0x3, 0x11, 0x11);
+		rtl8723bu_set_coex_with_type(priv, 4);
+	} else {
+		rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
+		rtl8723bu_set_coex_with_type(priv, 7);
+	}
+}
+
+void rtl8723bu_handle_bt_info(struct rtl8xxxu_priv *priv)
+{
+	struct ieee80211_vif *vif;
+	struct rtl8xxxu_btcoex *btcoex;
+	bool wifi_connected;
+
+	vif = priv->vif;
+	btcoex = &priv->bt_coex;
+	wifi_connected = (vif && vif->bss_conf.assoc);
+
+	if (wifi_connected) {
+		u32 val32 = 0;
+		u32 high_prio_tx = 0, high_prio_rx = 0;
+
+		val32 = rtl8xxxu_read32(priv, 0x770);
+		high_prio_tx = val32 & 0x0000ffff;
+		high_prio_rx = (val32  & 0xffff0000) >> 16;
+
+		if (btcoex->bt_busy) {
+			if (btcoex->hid_only) {
+				rtl8723bu_set_ps_tdma(priv, 0x61, 0x20,
+						      0x3, 0x11, 0x11);
+				rtl8723bu_set_coex_with_type(priv, 5);
+			} else if (btcoex->a2dp_only) {
+				rtl8723bu_set_ps_tdma(priv, 0x61, 0x35,
+						      0x3, 0x11, 0x11);
+				rtl8723bu_set_coex_with_type(priv, 4);
+			} else if ((btcoex->has_a2dp && btcoex->has_pan) ||
+				   (btcoex->has_hid && btcoex->has_a2dp &&
+				    btcoex->has_pan)) {
+				rtl8723bu_set_ps_tdma(priv, 0x51, 0x21,
+						      0x3, 0x10, 0x10);
+				rtl8723bu_set_coex_with_type(priv, 4);
+			} else if (btcoex->has_hid && btcoex->has_a2dp) {
+				rtl8723bu_set_ps_tdma(priv, 0x51, 0x21,
+						      0x3, 0x10, 0x10);
+				rtl8723bu_set_coex_with_type(priv, 3);
+			} else {
+				rtl8723bu_set_ps_tdma(priv, 0x61, 0x35,
+						      0x3, 0x11, 0x11);
+				rtl8723bu_set_coex_with_type(priv, 4);
+			}
+		} else {
+			rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
+			if (high_prio_tx + high_prio_rx <= 60)
+				rtl8723bu_set_coex_with_type(priv, 2);
+			else
+				rtl8723bu_set_coex_with_type(priv, 7);
+		}
+	} else {
+		rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
+		rtl8723bu_set_coex_with_type(priv, 0);
+	}
+}
+
+static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
+{
+	struct rtl8xxxu_priv *priv;
+	struct rtl8723bu_c2h *c2h;
+	struct ieee80211_vif *vif;
+	struct device *dev;
+	struct sk_buff *skb = NULL;
+	unsigned long flags;
+	int len;
+	u8 bt_info = 0;
+	struct rtl8xxxu_btcoex *btcoex;
+
+	priv = container_of(work, struct rtl8xxxu_priv, c2hcmd_work);
+	vif = priv->vif;
+	btcoex = &priv->bt_coex;
+	dev = &priv->udev->dev;
+
+	if (priv->rf_paths > 1)
+		goto out;
+
+	while (!skb_queue_empty(&priv->c2hcmd_queue)) {
+		spin_lock_irqsave(&priv->c2hcmd_lock, flags);
+		skb = __skb_dequeue(&priv->c2hcmd_queue);
+		spin_unlock_irqrestore(&priv->c2hcmd_lock, flags);
+
+		c2h = (struct rtl8723bu_c2h *)skb->data;
+		len = skb->len - 2;
+
+		switch (c2h->id) {
+		case C2H_8723B_BT_INFO:
+			bt_info = c2h->bt_info.bt_info;
+
+			rtl8723bu_update_bt_link_info(priv, bt_info);
+			if (btcoex->c2h_bt_inquiry) {
+				rtl8723bu_handle_bt_inquiry(priv);
+				break;
+			}
+			rtl8723bu_handle_bt_info(priv);
+			break;
+		default:
+			break;
+		}
+	}
+
+out:
+	dev_kfree_skb(skb);
+}
+
 static void rtl8723bu_handle_c2h(struct rtl8xxxu_priv *priv,
 				 struct sk_buff *skb)
 {
 	struct rtl8723bu_c2h *c2h = (struct rtl8723bu_c2h *)skb->data;
 	struct device *dev = &priv->udev->dev;
 	int len;
+	unsigned long flags;
 
 	len = skb->len - 2;
 
@@ -5229,6 +5473,12 @@ static void rtl8723bu_handle_c2h(struct rtl8xxxu_priv *priv,
 			       16, 1, c2h->raw.payload, len, false);
 		break;
 	}
+
+	spin_lock_irqsave(&priv->c2hcmd_lock, flags);
+	__skb_queue_tail(&priv->c2hcmd_queue, skb);
+	spin_unlock_irqrestore(&priv->c2hcmd_lock, flags);
+
+	schedule_work(&priv->c2hcmd_work);
 }
 
 int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
@@ -5353,7 +5603,6 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
 		struct device *dev = &priv->udev->dev;
 		dev_dbg(dev, "%s: C2H packet\n", __func__);
 		rtl8723bu_handle_c2h(priv, skb);
-		dev_kfree_skb(skb);
 		return RX_TYPE_C2H;
 	}
 
@@ -6272,6 +6521,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 	spin_lock_init(&priv->rx_urb_lock);
 	INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
 	INIT_DELAYED_WORK(&priv->ra_watchdog, rtl8xxxu_watchdog_callback);
+	spin_lock_init(&priv->c2hcmd_lock);
+	INIT_WORK(&priv->c2hcmd_work, rtl8xxxu_c2hcmd_callback);
+	skb_queue_head_init(&priv->c2hcmd_queue);
 
 	usb_set_intfdata(interface, hw);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-09-11  2:50 [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna Chris Chiu
@ 2019-09-19  1:44 ` Chris Chiu
  2019-10-01  9:32   ` Chris Chiu
  2019-10-02  4:21 ` Kalle Valo
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Chris Chiu @ 2019-09-19  1:44 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, David Miller
  Cc: linux-wireless, netdev, Linux Kernel, Linux Upstreaming Team

On Wed, Sep 11, 2019 at 10:50 AM Chris Chiu <chiu@endlessm.com> wrote:
>
>
> Notes:
>   v2:
>    - Add helper functions to replace bunch of tdma settings
>    - Reformat some lines to meet kernel coding style
>
>

Gentle ping. Any suggestions would be appreciated. Thanks.

Chris

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-09-19  1:44 ` Chris Chiu
@ 2019-10-01  9:32   ` Chris Chiu
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Chiu @ 2019-10-01  9:32 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, David Miller
  Cc: linux-wireless, netdev, Linux Kernel, Linux Upstreaming Team

On Thu, Sep 19, 2019 at 9:44 AM Chris Chiu <chiu@endlessm.com> wrote:
>
> On Wed, Sep 11, 2019 at 10:50 AM Chris Chiu <chiu@endlessm.com> wrote:
> >
> >
> > Notes:
> >   v2:
> >    - Add helper functions to replace bunch of tdma settings
> >    - Reformat some lines to meet kernel coding style
> >
> >
>
Hi Jes,
    I've refactored the code per your suggestion. Any comment for further
improvement? Thanks.

Chris

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-09-11  2:50 [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna Chris Chiu
  2019-09-19  1:44 ` Chris Chiu
@ 2019-10-02  4:21 ` Kalle Valo
  2019-10-02  4:29 ` Kalle Valo
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2019-10-02  4:21 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes.Sorensen, davem, linux-wireless, netdev, linux-kernel, linux

Chris Chiu <chiu@endlessm.com> wrote:

> The RTL8723BU suffers the wifi disconnection problem while bluetooth
> device connected. While wifi is doing tx/rx, the bluetooth will scan
> without results. This is due to the wifi and bluetooth share the same
> single antenna for RF communication and they need to have a mechanism
> to collaborate.
> 
> BT information is provided via the packet sent from co-processor to
> host (C2H). It contains the status of BT but the rtl8723bu_handle_c2h
> dose not really handle it. And there's no bluetooth coexistence
> mechanism to deal with it.
> 
> This commit adds a workqueue to set the tdma configurations and
> coefficient table per the parsed bluetooth link status and given
> wifi connection state. The tdma/coef table comes from the vendor
> driver code of the RTL8192EU and RTL8723BU. However, this commit is
> only for single antenna scenario which RTL8192EU is default dual
> antenna. The rtl8xxxu_parse_rxdesc24 which invokes the handle_c2h
> is only for 8723b and 8192e so the mechanism is expected to work
> on both chips with single antenna. Note RTL8192EU dual antenna is
> not supported.
> 
> Signed-off-by: Chris Chiu <chiu@endlessm.com>

As Jes was positive about this in v1 and had only cosmetic comments, I'm
planning to apply this. If there are any changes needed, those can be fixed in
a followup patch.

-- 
https://patchwork.kernel.org/patch/11140223/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-09-11  2:50 [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna Chris Chiu
  2019-09-19  1:44 ` Chris Chiu
  2019-10-02  4:21 ` Kalle Valo
@ 2019-10-02  4:29 ` Kalle Valo
       [not found] ` <20191002042911.2E755611BF@smtp.codeaurora.org>
  2019-10-02 15:03 ` Jes Sorensen
  4 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2019-10-02  4:29 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes.Sorensen, davem, linux-wireless, netdev, linux-kernel, linux

Chris Chiu <chiu@endlessm.com> wrote:

> The RTL8723BU suffers the wifi disconnection problem while bluetooth
> device connected. While wifi is doing tx/rx, the bluetooth will scan
> without results. This is due to the wifi and bluetooth share the same
> single antenna for RF communication and they need to have a mechanism
> to collaborate.
> 
> BT information is provided via the packet sent from co-processor to
> host (C2H). It contains the status of BT but the rtl8723bu_handle_c2h
> dose not really handle it. And there's no bluetooth coexistence
> mechanism to deal with it.
> 
> This commit adds a workqueue to set the tdma configurations and
> coefficient table per the parsed bluetooth link status and given
> wifi connection state. The tdma/coef table comes from the vendor
> driver code of the RTL8192EU and RTL8723BU. However, this commit is
> only for single antenna scenario which RTL8192EU is default dual
> antenna. The rtl8xxxu_parse_rxdesc24 which invokes the handle_c2h
> is only for 8723b and 8192e so the mechanism is expected to work
> on both chips with single antenna. Note RTL8192EU dual antenna is
> not supported.
> 
> Signed-off-by: Chris Chiu <chiu@endlessm.com>

Failed to apply, please rebase on top of wireless-drivers-next.

fatal: sha1 information is lacking or useless (drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h).
error: could not build fake ancestor
Applying: rtl8xxxu: add bluetooth co-existence support for single antenna
Patch failed at 0001 rtl8xxxu: add bluetooth co-existence support for single antenna
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/11140223/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
       [not found] ` <20191002042911.2E755611BF@smtp.codeaurora.org>
@ 2019-10-02 12:38   ` Chris Chiu
  2019-10-02 16:37     ` Kalle Valo
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Chiu @ 2019-10-02 12:38 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Jes Sorensen, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team

On Wed, Oct 2, 2019 at 12:29 PM Kalle Valo <kvalo@codeaurora.org> wrote:

> Failed to apply, please rebase on top of wireless-drivers-next.
>
> fatal: sha1 information is lacking or useless (drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h).
> error: could not build fake ancestor
> Applying: rtl8xxxu: add bluetooth co-existence support for single antenna
> Patch failed at 0001 rtl8xxxu: add bluetooth co-existence support for single antenna
> The copy of the patch that failed is found in: .git/rebase-apply/patch
>
> Patch set to Changes Requested.
>
> --
> https://patchwork.kernel.org/patch/11140223/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>

The failure is because this patch needs the 'enum wireless_mode' from another
patch https://patchwork.kernel.org/patch/11148163/ which I already submit the
new v8 version. I didn't put them in the same series due to it really
took me a long
time to come out after tx performance improvement patch upstream. Please apply
this one after https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2117331.html.
Thanks.

Chris

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-09-11  2:50 [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna Chris Chiu
                   ` (3 preceding siblings ...)
       [not found] ` <20191002042911.2E755611BF@smtp.codeaurora.org>
@ 2019-10-02 15:03 ` Jes Sorensen
  2019-10-03  1:19   ` Chris Chiu
  4 siblings, 1 reply; 10+ messages in thread
From: Jes Sorensen @ 2019-10-02 15:03 UTC (permalink / raw)
  To: Chris Chiu, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, linux

On 9/10/19 10:50 PM, Chris Chiu wrote:
> The RTL8723BU suffers the wifi disconnection problem while bluetooth
> device connected. While wifi is doing tx/rx, the bluetooth will scan
> without results. This is due to the wifi and bluetooth share the same
> single antenna for RF communication and they need to have a mechanism
> to collaborate.
> 
> BT information is provided via the packet sent from co-processor to
> host (C2H). It contains the status of BT but the rtl8723bu_handle_c2h
> dose not really handle it. And there's no bluetooth coexistence
> mechanism to deal with it.
> 
> This commit adds a workqueue to set the tdma configurations and
> coefficient table per the parsed bluetooth link status and given
> wifi connection state. The tdma/coef table comes from the vendor
> driver code of the RTL8192EU and RTL8723BU. However, this commit is
> only for single antenna scenario which RTL8192EU is default dual
> antenna. The rtl8xxxu_parse_rxdesc24 which invokes the handle_c2h
> is only for 8723b and 8192e so the mechanism is expected to work
> on both chips with single antenna. Note RTL8192EU dual antenna is
> not supported.
> 
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
> 
> Notes:
>    v2:
>     - Add helper functions to replace bunch of tdma settings
>     - Reformat some lines to meet kernel coding style
> 
> 
>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  37 +++
>   .../realtek/rtl8xxxu/rtl8xxxu_8723b.c         |   2 -
>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 262 +++++++++++++++++-
>   3 files changed, 294 insertions(+), 7 deletions(-)

In general I think it looks good! One nit below:

Sorry I have been traveling for the last three weeks, so just catching up.


> +void rtl8723bu_set_coex_with_type(struct rtl8xxxu_priv *priv, u8 type)
> +{
> +	switch (type) {
> +	case 0:
> +		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
> +		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
> +		rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +		rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +		break;
> +	case 1:
> +	case 3:

The one item here, I would prefer introducing some defined types to 
avoid the hard coded type numbers. It's much easier to read and debug 
when named.

If you shortened the name of the function to rtl8723bu_set_coex() you 
won't have problems with line lengths at the calling point.

Cheers,
Jes

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-10-02 12:38   ` Chris Chiu
@ 2019-10-02 16:37     ` Kalle Valo
  0 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2019-10-02 16:37 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes Sorensen, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team

Chris Chiu <chiu@endlessm.com> writes:

> On Wed, Oct 2, 2019 at 12:29 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
>> Failed to apply, please rebase on top of wireless-drivers-next.
>>
>> fatal: sha1 information is lacking or useless
>> (drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h).
>> error: could not build fake ancestor
>> Applying: rtl8xxxu: add bluetooth co-existence support for single antenna
>> Patch failed at 0001 rtl8xxxu: add bluetooth co-existence support for single antenna
>> The copy of the patch that failed is found in: .git/rebase-apply/patch
>>
>> Patch set to Changes Requested.
>>
>> --
>> https://patchwork.kernel.org/patch/11140223/
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>>
>
> The failure is because this patch needs the 'enum wireless_mode' from another
> patch https://patchwork.kernel.org/patch/11148163/ which I already submit the
> new v8 version. I didn't put them in the same series due to it really
> took me a long
> time to come out after tx performance improvement patch upstream. Please apply
> this one after
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2117331.html.

Ok, but please always clearly document if there are any dependencies. I
don't have time to start testing in which order I'm supposed to apply
them. And the best is if you submit the patches in same patchset, that
way I don't need to do anything extra.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-10-02 15:03 ` Jes Sorensen
@ 2019-10-03  1:19   ` Chris Chiu
  2019-10-04 15:11     ` Jes Sorensen
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Chiu @ 2019-10-03  1:19 UTC (permalink / raw)
  To: Jes Sorensen
  Cc: Kalle Valo, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team

On Wed, Oct 2, 2019 at 11:04 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>
>
> In general I think it looks good! One nit below:
>
> Sorry I have been traveling for the last three weeks, so just catching up.
>
>
> > +void rtl8723bu_set_coex_with_type(struct rtl8xxxu_priv *priv, u8 type)
> > +{
> > +     switch (type) {
> > +     case 0:
> > +             rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
> > +             rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
> > +             rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> > +             rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> > +             break;
> > +     case 1:
> > +     case 3:
>
> The one item here, I would prefer introducing some defined types to
> avoid the hard coded type numbers. It's much easier to read and debug
> when named.
>
Honestly, I also thought of that but there's no meaningful description for these
numbers in the vendor driver. Even based on where they're invoked, I can merely
give a rough definition on 0. So I left it as it is for the covenience
if I have to do
cross-comparison with vendor driver in the future for some possible
unknown bugs.

> If you shortened the name of the function to rtl8723bu_set_coex() you
> won't have problems with line lengths at the calling point.
>
I think the rtl8723bu_set_ps_tdma() function would cause the line length problem
more than rtl8723bu_set_coex_with_type() at the calling point. But as the same
debug reason as mentioned, I may like to keep it because I don't know how to
categorize the 5 magic parameters. I also reference the latest rtw88
driver code,
it seems no better solution so far. I'll keep watching if there's any
better idea.

Chris

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
  2019-10-03  1:19   ` Chris Chiu
@ 2019-10-04 15:11     ` Jes Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2019-10-04 15:11 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Kalle Valo, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team

On 10/2/19 9:19 PM, Chris Chiu wrote:
> On Wed, Oct 2, 2019 at 11:04 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>>
>>
>> In general I think it looks good! One nit below:
>>
>> Sorry I have been traveling for the last three weeks, so just catching up.
>>
>>
>>> +void rtl8723bu_set_coex_with_type(struct rtl8xxxu_priv *priv, u8 type)
>>> +{
>>> +     switch (type) {
>>> +     case 0:
>>> +             rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
>>> +             rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
>>> +             rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
>>> +             rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
>>> +             break;
>>> +     case 1:
>>> +     case 3:
>>
>> The one item here, I would prefer introducing some defined types to
>> avoid the hard coded type numbers. It's much easier to read and debug
>> when named.
>>
> Honestly, I also thought of that but there's no meaningful description for these
> numbers in the vendor driver. Even based on where they're invoked, I can merely
> give a rough definition on 0. So I left it as it is for the covenience
> if I have to do
> cross-comparison with vendor driver in the future for some possible
> unknown bugs.
> 
>> If you shortened the name of the function to rtl8723bu_set_coex() you
>> won't have problems with line lengths at the calling point.
>>
> I think the rtl8723bu_set_ps_tdma() function would cause the line length problem
> more than rtl8723bu_set_coex_with_type() at the calling point. But as the same
> debug reason as mentioned, I may like to keep it because I don't know how to
> categorize the 5 magic parameters. I also reference the latest rtw88
> driver code,
> it seems no better solution so far. I'll keep watching if there's any
> better idea.

Personally I would still prefer to name it COEX_TYPE_1 etc. but I can 
live with this. Would you mind at least adding some comments in the code 
about it?

Cheers,
Jes



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11  2:50 [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna Chris Chiu
2019-09-19  1:44 ` Chris Chiu
2019-10-01  9:32   ` Chris Chiu
2019-10-02  4:21 ` Kalle Valo
2019-10-02  4:29 ` Kalle Valo
     [not found] ` <20191002042911.2E755611BF@smtp.codeaurora.org>
2019-10-02 12:38   ` Chris Chiu
2019-10-02 16:37     ` Kalle Valo
2019-10-02 15:03 ` Jes Sorensen
2019-10-03  1:19   ` Chris Chiu
2019-10-04 15:11     ` Jes Sorensen

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox