[PATCH] rtlwifi: cleanup 8723be ant_sel definition

[PATCH] rtlwifi: cleanup 8723be ant_sel definition

[pkshih]

From: Ping-Ke Shih <pkshih@realtek.com>

The module parameter ant_sel is used to control antenna number and path.
There is an existing enum ANT_{X2,X1} defined the antenna number, so
add a new enum ANT_{MAIN,AUX} to make it readable. After this work,
incorrect given values depend on ant_sel were exposed, so refill values
according following definition:
  ant_sel   ant_num   ant_path  print_label
     1      ANT_X1    ANT_AUX        #2
     2      ANT_X2    ANT_MAIN       #1
Then, the workaround in halbtcoutsrc.c was removed.

The experimental results with single antenna connected to specific path
are in following:
  ant_sel  ANT_MAIN(#1)  ANT_AUX(#2)
     0        -8            -62
     1        -62           -10
     2        -6            -60

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
Cc: Stable <stable@vger.kernel.org> # 4.7+
Reviewed-by: Larry Finger <Larry.Finger@lwfinger.net>
---
Hi Kalle,

This patch would send to 4.17.

Thanks
PK

---
 .../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 15 ---------------
 drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c       | 11 +++++++----
 drivers/net/wireless/realtek/rtlwifi/wifi.h               |  5 +++++
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index e0f9985582f9..738a7ea7dc4a 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -158,16 +158,6 @@ static u8 halbtc_get_wifi_central_chnl(struct btc_coexist *btcoexist)
 
 static u8 rtl_get_hwpg_single_ant_path(struct rtl_priv *rtlpriv)
 {
-	struct rtl_mod_params *mod_params = rtlpriv->cfg->mod_params;
-
-	/* override ant_num / ant_path */
-	if (mod_params->ant_sel) {
-		rtlpriv->btcoexist.btc_info.ant_num =
-			(mod_params->ant_sel == 1 ? ANT_X2 : ANT_X1);
-
-		rtlpriv->btcoexist.btc_info.single_ant_path =
-			(mod_params->ant_sel == 1 ? 0 : 1);
-	}
 	return rtlpriv->btcoexist.btc_info.single_ant_path;
 }
 
@@ -178,7 +168,6 @@ static u8 rtl_get_hwpg_bt_type(struct rtl_priv *rtlpriv)
 
 static u8 rtl_get_hwpg_ant_num(struct rtl_priv *rtlpriv)
 {
-	struct rtl_mod_params *mod_params = rtlpriv->cfg->mod_params;
 	u8 num;
 
 	if (rtlpriv->btcoexist.btc_info.ant_num == ANT_X2)
@@ -186,10 +175,6 @@ static u8 rtl_get_hwpg_ant_num(struct rtl_priv *rtlpriv)
 	else
 		num = 1;
 
-	/* override ant_num / ant_path */
-	if (mod_params->ant_sel)
-		num = (mod_params->ant_sel == 1 ? ANT_X2 : ANT_X1) + 1;
-
 	return num;
 }
 
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
index e7bbbc95cdb1..b4f3f91b590e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
@@ -848,6 +848,9 @@ static bool _rtl8723be_init_mac(struct ieee80211_hw *hw)
 		return false;
 	}
 
+	if (rtlpriv->cfg->ops->get_btc_status())
+		rtlpriv->btcoexist.btc_ops->btc_power_on_setting(rtlpriv);
+
 	bytetmp = rtl_read_byte(rtlpriv, REG_MULTI_FUNC_CTRL);
 	rtl_write_byte(rtlpriv, REG_MULTI_FUNC_CTRL, bytetmp | BIT(3));
 
@@ -2696,21 +2699,21 @@ void rtl8723be_read_bt_coexist_info_from_hwpg(struct ieee80211_hw *hw,
 		rtlpriv->btcoexist.btc_info.bt_type = BT_RTL8723B;
 		rtlpriv->btcoexist.btc_info.ant_num = (value & 0x1);
 		rtlpriv->btcoexist.btc_info.single_ant_path =
-			 (value & 0x40);	/*0xc3[6]*/
+			 (value & 0x40 ? ANT_AUX : ANT_MAIN);	/*0xc3[6]*/
 	} else {
 		rtlpriv->btcoexist.btc_info.btcoexist = 0;
 		rtlpriv->btcoexist.btc_info.bt_type = BT_RTL8723B;
 		rtlpriv->btcoexist.btc_info.ant_num = ANT_X2;
-		rtlpriv->btcoexist.btc_info.single_ant_path = 0;
+		rtlpriv->btcoexist.btc_info.single_ant_path = ANT_MAIN;
 	}
 
 	/* override ant_num / ant_path */
 	if (mod_params->ant_sel) {
 		rtlpriv->btcoexist.btc_info.ant_num =
-			(mod_params->ant_sel == 1 ? ANT_X2 : ANT_X1);
+			(mod_params->ant_sel == 1 ? ANT_X1 : ANT_X2);
 
 		rtlpriv->btcoexist.btc_info.single_ant_path =
-			(mod_params->ant_sel == 1 ? 0 : 1);
+			(mod_params->ant_sel == 1 ? ANT_AUX : ANT_MAIN);
 	}
 }
 
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index c32985cfe48d..ab32a62ead81 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -2882,6 +2882,11 @@ enum bt_ant_num {
 	ANT_X1 = 1,
 };
 
+enum bt_ant_path {
+	ANT_MAIN = 0,
+	ANT_AUX = 1,
+};
+
 enum bt_co_type {
 	BT_2WIRE = 0,
 	BT_ISSC_3WIRE = 1,
-- 
2.15.1

Re: [PATCH] rtlwifi: cleanup 8723be ant_sel definition

[Kalle Valo]

<pkshih@realtek.com> writes:

> From: Ping-Ke Shih <pkshih@realtek.com>
>
> The module parameter ant_sel is used to control antenna number and path.
> There is an existing enum ANT_{X2,X1} defined the antenna number, so
> add a new enum ANT_{MAIN,AUX} to make it readable. After this work,
> incorrect given values depend on ant_sel were exposed, so refill values
> according following definition:
>   ant_sel   ant_num   ant_path  print_label
>      1      ANT_X1    ANT_AUX        #2
>      2      ANT_X2    ANT_MAIN       #1
> Then, the workaround in halbtcoutsrc.c was removed.
>
> The experimental results with single antenna connected to specific path
> are in following:
>   ant_sel  ANT_MAIN(#1)  ANT_AUX(#2)
>      0        -8            -62
>      1        -62           -10
>      2        -6            -60
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Cc: Stable <stable@vger.kernel.org> # 4.7+
> Reviewed-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> Hi Kalle,
>
> This patch would send to 4.17.

For -rc releases I require a quite clear bug report but I do not
understand what you are fixing here, even after reading the commit log
twice. Could you try to improve it? Especially focus on describing the
bug in simple terms and how this patch changes the functionality from
user's point of view.

-- 
Kalle Valo

Re: [PATCH] rtlwifi: cleanup 8723be ant_sel definition

[Pkshih]

T24gVGh1LCAyMDE4LTA0LTE5IGF0IDEyOjUyICswMzAwLCBLYWxsZSBWYWxvIHdyb3RlOg0KPiA8
cGtzaGloQHJlYWx0ZWsuY29tPiB3cml0ZXM6DQo+IA0KPiA+IEZyb206IFBpbmctS2UgU2hpaCA8
cGtzaGloQHJlYWx0ZWsuY29tPg0KPiA+DQo+ID4gVGhlIG1vZHVsZSBwYXJhbWV0ZXIgYW50X3Nl
bCBpcyB1c2VkIHRvIGNvbnRyb2wgYW50ZW5uYSBudW1iZXIgYW5kIHBhdGguDQo+ID4gVGhlcmUg
aXMgYW4gZXhpc3RpbmcgZW51bSBBTlRfe1gyLFgxfSBkZWZpbmVkIHRoZSBhbnRlbm5hIG51bWJl
ciwgc28NCj4gPiBhZGQgYSBuZXcgZW51bSBBTlRfe01BSU4sQVVYfSB0byBtYWtlIGl0IHJlYWRh
YmxlLiBBZnRlciB0aGlzIHdvcmssDQo+ID4gaW5jb3JyZWN0IGdpdmVuIHZhbHVlcyBkZXBlbmQg
b24gYW50X3NlbCB3ZXJlIGV4cG9zZWQsIHNvIHJlZmlsbCB2YWx1ZXMNCj4gPiBhY2NvcmRpbmcg
Zm9sbG93aW5nIGRlZmluaXRpb246DQo+ID7CoMKgwqBhbnRfc2VswqDCoMKgYW50X251bcKgwqDC
oGFudF9wYXRowqDCoHByaW50X2xhYmVsDQo+ID7CoMKgwqDCoMKgwqAxwqDCoMKgwqDCoMKgQU5U
X1gxwqDCoMKgwqBBTlRfQVVYwqDCoMKgwqDCoMKgwqDCoCMyDQo+ID7CoMKgwqDCoMKgwqAywqDC
oMKgwqDCoMKgQU5UX1gywqDCoMKgwqBBTlRfTUFJTsKgwqDCoMKgwqDCoMKgIzENCj4gPiBUaGVu
LCB0aGUgd29ya2Fyb3VuZCBpbiBoYWxidGNvdXRzcmMuYyB3YXMgcmVtb3ZlZC4NCj4gPg0KPiA+
IFRoZSBleHBlcmltZW50YWwgcmVzdWx0cyB3aXRoIHNpbmdsZSBhbnRlbm5hIGNvbm5lY3RlZCB0
byBzcGVjaWZpYyBwYXRoDQo+ID4gYXJlIGluIGZvbGxvd2luZzoNCj4gPsKgwqDCoGFudF9zZWzC
oMKgQU5UX01BSU4oIzEpwqDCoEFOVF9BVVgoIzIpDQo+ID7CoMKgwqDCoMKgwqAwwqDCoMKgwqDC
oMKgwqDCoC04wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgLTYyDQo+ID7CoMKgwqDCoMKgwqAxwqDC
oMKgwqDCoMKgwqDCoC02MsKgwqDCoMKgwqDCoMKgwqDCoMKgwqAtMTANCj4gPsKgwqDCoMKgwqDC
oDLCoMKgwqDCoMKgwqDCoMKgLTbCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAtNjANCj4gPg0KPiA+
IFNpZ25lZC1vZmYtYnk6IFBpbmctS2UgU2hpaCA8cGtzaGloQHJlYWx0ZWsuY29tPg0KPiA+IENj
OiBTdGFibGUgPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ICMgNC43Kw0KPiA+IFJldmlld2VkLWJ5
OiBMYXJyeSBGaW5nZXIgPExhcnJ5LkZpbmdlckBsd2Zpbmdlci5uZXQ+DQo+ID4gLS0tDQo+ID4g
SGkgS2FsbGUsDQo+ID4NCj4gPiBUaGlzIHBhdGNoIHdvdWxkIHNlbmQgdG8gNC4xNy4NCj4gDQo+
IEZvciAtcmMgcmVsZWFzZXMgSSByZXF1aXJlIGEgcXVpdGUgY2xlYXIgYnVnIHJlcG9ydCBidXQg
SSBkbyBub3QNCj4gdW5kZXJzdGFuZCB3aGF0IHlvdSBhcmUgZml4aW5nIGhlcmUsIGV2ZW4gYWZ0
ZXIgcmVhZGluZyB0aGUgY29tbWl0IGxvZw0KPiB0d2ljZS4gQ291bGQgeW91IHRyeSB0byBpbXBy
b3ZlIGl0PyBFc3BlY2lhbGx5IGZvY3VzIG9uIGRlc2NyaWJpbmcgdGhlDQo+IGJ1ZyBpbiBzaW1w
bGUgdGVybXMgYW5kIGhvdyB0aGlzIHBhdGNoIGNoYW5nZXMgdGhlIGZ1bmN0aW9uYWxpdHkgZnJv
bQ0KPiB1c2VyJ3MgcG9pbnQgb2Ygdmlldy4NCj4gDQpUaGUgcGF0Y2ggbWFpbmx5IGZpeCB0aGUg
d3JvbmcgdmFsdWUgb2bCoGFudF9udW0gYW5kwqBzaW5nbGVfYW50X3BhdGggd2hlbg0KYW50X3Nl
bCBpcyBzZXQuDQoNCsKgIMKgIMKgIMKgIGlmIChtb2RfcGFyYW1zLT5hbnRfc2VsKSB7IMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgDQrCoMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgcnRscHJpdi0+YnRjb2V4aXN0LmJ0Y19p
bmZvLmFudF9udW0gPSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoMKgDQot
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoChtb2RfcGFyYW1z
LT5hbnRfc2VsID09IDEgPyBBTlRfWDIgOiBBTlRfWDEpOyDCoCDCoCDCoCDCoCDCoMKgDQorwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoChtb2RfcGFyYW1zLT5h
bnRfc2VsID09IDEgPyBBTlRfWDEgOiBBTlRfWDIpOyDCoCDCoCDCoCDCoCDCoMKgDQrCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoMKg
DQrCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBydGxwcml2LT5idGNvZXhpc3QuYnRjX2luZm8uc2lu
Z2xlX2FudF9wYXRoID0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqDCoA0KLcKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAobW9kX3BhcmFtcy0+YW50X3NlbCA9
PSAxID8gMCA6IDEpOyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoMKgDQorwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoChtb2RfcGFyYW1zLT5hbnRfc2Vs
ID09IDEgPyBBTlRfQVVYIDogQU5UX01BSU4pOyDCoCDCoCDCoCDCoA0KwqDCoCDCoCDCoCDCoH0g
wqAgwqANClNvLCBuZWVkIHdvcmthcm91bmQgdG8gZml4IHRoZSBwcm9ibGVtLg0KDQpGb3IgZW5k
IHVzZXIsIG1vc3QgcGVvcGxlIGRvbid0IG5lZWQgYW50X3NlbCwgYnV0IEhQIHVzZXIgbmVlZHMg
dG8gc2V0IHRvIDEuDQpJbiBjYXNlIGFudF9zZWw9MSwgdGhlIHJlc3VsdCBpcyBhbG1vc3QgdGhl
IHNhbWUsIGJ1dCBhbnRfc2VsPTIgdGhhdCBpcyByYXJlDQp1c2VkIGlzIGJyb2tlbi4gU28sIHRo
aXMgYnVnIG1heSBiZSBub3Qgc2VlbiBieSB1c2VyLg0KDQpBbm90aGVyIHRoaW5nIGlzIHdlIHdh
bnQgdG8gYXBwbHkgdGhpcyBwYXRjaCB0byBzdGFibGUgcmVsZWFzZSB0byBnaXZlIGNvcnJlY3QN
CnZhbHVlcyBhbmQgcmVtb3ZlIHRoZSB3b3JrYXJvdW5kLCBzbyBJIHRoaW5rIGl0IGNhbiBhcHBs
eSB0byAtcmMgdG9vLg0KDQpBbnl3YXksIEknbGwgYWRkIG1vcmUgZGVzY3JpcHRpb24gdG8gY29t
bWl0IGxvZywgYW5kIHlvdSBjYW4gZGVjaWRlIHRvDQphcHBseSB0aGlzIHBhdGNoIHRvIDQuMTcg
b3IgcXVldWUgdG8gNC4xOC4NCg0KVGhhbmtzDQpQSw0K

Re: [PATCH] rtlwifi: cleanup 8723be ant_sel definition

[Pkshih]

On Thu, 2018-04-19 at 12:52 +0300, Kalle Valo wrote:
> <pkshih@realtek.com> writes:
> 
> > From: Ping-Ke Shih <pkshih@realtek.com>
> >
> > The module parameter ant_sel is used to control antenna number and path.
> > There is an existing enum ANT_{X2,X1} defined the antenna number, so
> > add a new enum ANT_{MAIN,AUX} to make it readable. After this work,
> > incorrect given values depend on ant_sel were exposed, so refill values
> > according following definition:
> >   ant_sel   ant_num   ant_path  print_label
> >      1      ANT_X1    ANT_AUX        #2
> >      2      ANT_X2    ANT_MAIN       #1
> > Then, the workaround in halbtcoutsrc.c was removed.
> >
> > The experimental results with single antenna connected to specific path
> > are in following:
> >   ant_sel  ANT_MAIN(#1)  ANT_AUX(#2)
> >      0        -8            -62
> >      1        -62           -10
> >      2        -6            -60
> >
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > Cc: Stable <stable@vger.kernel.org> # 4.7+
> > Reviewed-by: Larry Finger <Larry.Finger@lwfinger.net>
> > ---
> > Hi Kalle,
> >
> > This patch would send to 4.17.
> 
> For -rc releases I require a quite clear bug report but I do not
> understand what you are fixing here, even after reading the commit log
> twice. Could you try to improve it? Especially focus on describing the
> bug in simple terms and how this patch changes the functionality from
> user's point of view.
> 
The patch mainly fix the wrong value of ant_num and single_ant_path when
ant_sel is set.

        if (mod_params->ant_sel) {                                              
                rtlpriv->btcoexist.btc_info.ant_num =                           
-                       (mod_params->ant_sel == 1 ? ANT_X2 : ANT_X1);           
+                       (mod_params->ant_sel == 1 ? ANT_X1 : ANT_X2);           
                                                                                
                rtlpriv->btcoexist.btc_info.single_ant_path =                   
-                       (mod_params->ant_sel == 1 ? 0 : 1);                     
+                       (mod_params->ant_sel == 1 ? ANT_AUX : ANT_MAIN);        
        }    
So, need workaround to fix the problem.

For end user, most people don't need ant_sel, but HP user needs to set to 1.
In case ant_sel=1, the result is almost the same, but ant_sel=2 that is rare
used is broken. So, this bug may be not seen by user.

Another thing is we want to apply this patch to stable release to give correct
values and remove the workaround, so I think it can apply to -rc too.

Anyway, I'll add more description to commit log, and you can decide to
apply this patch to 4.17 or queue to 4.18.

Thanks
PK