Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
On 10/2/19 9:19 PM, Chris Chiu wrote:
On Wed, Oct 2, 2019 at 11:04 PM Jes Sorensen 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, 0x);
+ rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x);
+ rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ff);
+ 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
On Wed, Oct 2, 2019 at 11:04 PM Jes Sorensen 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, 0x);
> > + rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x);
> > + rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ff);
> > + 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
Chris Chiu writes: > On Wed, Oct 2, 2019 at 12:29 PM Kalle Valo 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
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
---
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, 0x);
+ rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x);
+ rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ff);
+ 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
On Wed, Oct 2, 2019 at 12:29 PM Kalle Valo 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
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 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
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 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
On Thu, Sep 19, 2019 at 9:44 AM Chris Chiu wrote: > > On Wed, Sep 11, 2019 at 10:50 AM Chris Chiu 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
Re: [PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
On Wed, Sep 11, 2019 at 10:50 AM Chris Chiu 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
[PATCH v2] rtl8xxxu: add bluetooth co-existence support for single antenna
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
---
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_CONNECTIONBIT(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;
+ boolbt_busy;
+ boolhas_sco;
+ boolhas_a2dp;
+ boolhas_hid;
+ boolhas_pan;
+ boolhid_only;
+ boola2dp_only;
+ boolc2h_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, 0x);
rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x);
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
