[PATCH] Update rf_type_definition

[PATCH] Update rf_type_definition

[l4stpr0gr4m]

From: Kangmin Park <l4stpr0gr4m@gmail.com>

enum RT_RF_TYPE_DEFINITION on rtl8723bs/include/rtw_rf.h
is different from enum tag_HAL_RF_Type_Definition on
rtl8723bs/include/HalVerDef.h

So, update them to be the same reference from
enum rf_type on rtlwifi/wifi.h which recent version.

As a result, ODM_RF_TYPE_E needs to be updated as well
Therefore, it is updated reference from enum odm_rf_type
on rtlwifi/phydm/phydm_pre_define.h too.
And update some additional code to check more
chip versions that resulted from it.

Also, fixed some space required errors and
line over 80 characters warnings by checkpatch.pl.

Signed-off-by: Kangmin Park <l4stpr0gr4m@gmail.com>
---
 drivers/staging/rtl8723bs/hal/odm.h               | 18 +++---
 drivers/staging/rtl8723bs/hal/rtl8723b_dm.c       | 18 +++++-
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 12 ++++
 drivers/staging/rtl8723bs/include/HalVerDef.h     | 77 ++++++++++++++++-------
 drivers/staging/rtl8723bs/include/rtw_rf.h        | 14 +++--
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 20 +++---
 6 files changed, 111 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/odm.h b/drivers/staging/rtl8723bs/hal/odm.h
index 87a76ba..771b894 100644
--- a/drivers/staging/rtl8723bs/hal/odm.h
+++ b/drivers/staging/rtl8723bs/hal/odm.h
@@ -511,14 +511,16 @@ typedef enum tag_ODM_RF_Path_Bit_Definition {
 
 
 typedef enum tag_ODM_RF_Type_Definition {
-	ODM_1T1R	=	0,
-	ODM_1T2R	=	1,
-	ODM_2T2R	=	2,
-	ODM_2T3R	=	3,
-	ODM_2T4R	=	4,
-	ODM_3T3R	=	5,
-	ODM_3T4R	=	6,
-	ODM_4T4R	=	7,
+	ODM_1T1R		=	0,
+	ODM_1T2R		=	1,
+	ODM_2T2R		=	2,
+	ODM_2T2R_GREEN	=	3,
+	ODM_2T3R		=	4,
+	ODM_2T4R		=	5,
+	ODM_3T3R		=	6,
+	ODM_3T4R		=	7,
+	ODM_4T4R		=	8,
+	ODM_XTXR		=	9,
 } ODM_RF_TYPE_E;
 
 
diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_dm.c b/drivers/staging/rtl8723bs/hal/rtl8723b_dm.c
index b162559..6069e1c 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_dm.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_dm.c
@@ -64,10 +64,24 @@ static void Init_ODM_ComInfo_8723b(struct adapter *Adapter)
 
 	if (pHalData->rf_type == RF_1T1R) {
 		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_1T1R);
-	} else if (pHalData->rf_type == RF_2T2R) {
-		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_2T2R);
 	} else if (pHalData->rf_type == RF_1T2R) {
 		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_1T2R);
+	} else if (pHalData->rf_type == RF_2T2R) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_2T2R);
+	} else if (pHalData->rf_type == RF_2T2R_GREEN) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_2T2R_GREEN);
+	} else if (pHalData->rf_type == RF_2T3R) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_2T3R);
+	} else if (pHalData->rf_type == RF_2T4R) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_2T4R);
+	} else if (pHalData->rf_type == RF_3T3R) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_3T3R);
+	} else if (pHalData->rf_type == RF_3T4R) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_3T4R);
+	} else if (pHalData->rf_type == RF_4T4R) {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_4T4R);
+	} else {
+		ODM_CmnInfoUpdate(pDM_Odm, ODM_CMNINFO_RF_TYPE, ODM_XTXR);
 	}
 
 	pdmpriv->InitODMFlag = ODM_RF_CALIBRATION|ODM_RF_TX_PWR_TRACK;
diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index d6cef9e..a4df098 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -1926,6 +1926,18 @@ static HAL_VERSION ReadChipVersion8723B(struct adapter *padapter)
 		pHalData->rf_type = RF_1T2R;
 	else if (IS_2T2R(ChipVersion))
 		pHalData->rf_type = RF_2T2R;
+	else if (IS_2T2R_GREEN(ChipVersion))
+		pHalData->rf_type = RF_2T2R_GREEN;
+	else if (IS_2T3R(ChipVersion))
+		pHalData->rf_type = RF_2T3R;
+	else if (IS_2T4R(ChipVersion))
+		pHalData->rf_type = RF_2T4R;
+	else if (IS_3T3R(ChipVersion))
+		pHalData->rf_type = RF_3T3R;
+	else if (IS_3T4R(ChipVersion))
+		pHalData->rf_type = RF_3T4R;
+	else if (IS_4T4R(ChipVersion))
+		pHalData->rf_type = RF_4T4R;
 	else
 		pHalData->rf_type = RF_1T1R;
 
diff --git a/drivers/staging/rtl8723bs/include/HalVerDef.h b/drivers/staging/rtl8723bs/include/HalVerDef.h
index a9e8609..08301fc 100644
--- a/drivers/staging/rtl8723bs/include/HalVerDef.h
+++ b/drivers/staging/rtl8723bs/include/HalVerDef.h
@@ -64,14 +64,15 @@ typedef enum tag_HAL_Manufacturer_Version_Definition
 
 typedef enum tag_HAL_RF_Type_Definition
 {
-	RF_TYPE_1T1R	=	0,
-	RF_TYPE_1T2R	=	1,
-	RF_TYPE_2T2R	=	2,
-	RF_TYPE_2T3R	=	3,
-	RF_TYPE_2T4R	=	4,
-	RF_TYPE_3T3R	=	5,
-	RF_TYPE_3T4R	=	6,
-	RF_TYPE_4T4R	=	7,
+	RF_TYPE_1T1R		=	0,
+	RF_TYPE_1T2R		=	1,
+	RF_TYPE_2T2R		=	2,
+	RF_TYPE_2T2R_GREEN	=	3,
+	RF_TYPE_2T3R		=	4,
+	RF_TYPE_2T4R		=	5,
+	RF_TYPE_3T3R		=	6,
+	RF_TYPE_3T4R		=	7,
+	RF_TYPE_4T4R		=	8,
 }HAL_RF_TYPE_E;
 
 typedef	struct tag_HAL_VERSION
@@ -101,27 +102,55 @@ typedef	struct tag_HAL_VERSION
 /* HAL_VERSION VersionID */
 
 /* HAL_CHIP_TYPE_E */
-#define IS_TEST_CHIP(version)			((GET_CVID_CHIP_TYPE(version) ==TEST_CHIP)? true: false)
-#define IS_NORMAL_CHIP(version)			((GET_CVID_CHIP_TYPE(version) ==NORMAL_CHIP)? true: false)
+#define IS_TEST_CHIP(version)			\
+	((GET_CVID_CHIP_TYPE(version) == TEST_CHIP) ? true : false)
+#define IS_NORMAL_CHIP(version)			\
+	((GET_CVID_CHIP_TYPE(version) == NORMAL_CHIP) ? true : false)
 
 /* HAL_CUT_VERSION_E */
-#define IS_A_CUT(version)				((GET_CVID_CUT_VERSION(version) == A_CUT_VERSION) ? true : false)
-#define IS_B_CUT(version)				((GET_CVID_CUT_VERSION(version) == B_CUT_VERSION) ? true : false)
-#define IS_C_CUT(version)				((GET_CVID_CUT_VERSION(version) == C_CUT_VERSION) ? true : false)
-#define IS_D_CUT(version)				((GET_CVID_CUT_VERSION(version) == D_CUT_VERSION) ? true : false)
-#define IS_E_CUT(version)				((GET_CVID_CUT_VERSION(version) == E_CUT_VERSION) ? true : false)
-#define IS_I_CUT(version)				((GET_CVID_CUT_VERSION(version) == I_CUT_VERSION) ? true : false)
-#define IS_J_CUT(version)				((GET_CVID_CUT_VERSION(version) == J_CUT_VERSION) ? true : false)
-#define IS_K_CUT(version)				((GET_CVID_CUT_VERSION(version) == K_CUT_VERSION) ? true : false)
+#define IS_A_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == A_CUT_VERSION) ? true : false)
+#define IS_B_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == B_CUT_VERSION) ? true : false)
+#define IS_C_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == C_CUT_VERSION) ? true : false)
+#define IS_D_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == D_CUT_VERSION) ? true : false)
+#define IS_E_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == E_CUT_VERSION) ? true : false)
+#define IS_I_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == I_CUT_VERSION) ? true : false)
+#define IS_J_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == J_CUT_VERSION) ? true : false)
+#define IS_K_CUT(version)				\
+	((GET_CVID_CUT_VERSION(version) == K_CUT_VERSION) ? true : false)
 
 /* HAL_VENDOR_E */
-#define IS_CHIP_VENDOR_TSMC(version)	((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_TSMC)? true: false)
-#define IS_CHIP_VENDOR_UMC(version)	((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_UMC)? true: false)
-#define IS_CHIP_VENDOR_SMIC(version)	((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_SMIC)? true: false)
+#define IS_CHIP_VENDOR_TSMC(version)	\
+	((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_TSMC) ? true : false)
+#define IS_CHIP_VENDOR_UMC(version)		\
+	((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_UMC) ? true : false)
+#define IS_CHIP_VENDOR_SMIC(version)	\
+	((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_SMIC) ? true : false)
 
 /* HAL_RF_TYPE_E */
-#define IS_1T1R(version)					((GET_CVID_RF_TYPE(version) == RF_TYPE_1T1R)? true : false)
-#define IS_1T2R(version)					((GET_CVID_RF_TYPE(version) == RF_TYPE_1T2R)? true : false)
-#define IS_2T2R(version)					((GET_CVID_RF_TYPE(version) == RF_TYPE_2T2R)? true : false)
+#define IS_1T1R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_1T1R) ? true : false)
+#define IS_1T2R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_1T2R) ? true : false)
+#define IS_2T2R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_2T2R) ? true : false)
+#define IS_2T2R_GREEN(version)			\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_2T2R_GREEN) ? true : false)
+#define IS_2T3R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_2T3R) ? true : false)
+#define IS_2T4R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_2T4R) ? true : false)
+#define IS_3T3R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_3T3R) ? true : false)
+#define IS_3T4R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_3T4R) ? true : false)
+#define IS_4T4R(version)				\
+	((GET_CVID_RF_TYPE(version) == RF_TYPE_4T4R) ? true : false)
 
 #endif
diff --git a/drivers/staging/rtl8723bs/include/rtw_rf.h b/drivers/staging/rtl8723bs/include/rtw_rf.h
index f9becab..d1fc396 100644
--- a/drivers/staging/rtl8723bs/include/rtw_rf.h
+++ b/drivers/staging/rtl8723bs/include/rtw_rf.h
@@ -146,12 +146,16 @@ enum PROTECTION_MODE {
 
 /* 2007/11/15 MH Define different RF type. */
 enum RT_RF_TYPE_DEFINITION {
-	RF_1T2R = 0,
-	RF_2T4R = 1,
+	RF_1T1R = 0,
+	RF_1T2R = 1,
 	RF_2T2R = 2,
-	RF_1T1R = 3,
-	RF_2T2R_GREEN = 4,
-	RF_MAX_TYPE = 5,
+	RF_2T2R_GREEN = 3,
+	RF_2T3R = 4,
+	RF_2T4R = 5,
+	RF_3T3R = 6,
+	RF_3T4R = 7,
+	RF_4T4R = 8,
+	RF_MAX_TYPE = 9,
 };
 
 u32 rtw_ch2freq(u32 ch);
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 51d48de..22fc6fa 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -3299,7 +3299,7 @@ static int cfg80211_rtw_sched_scan_stop(struct wiphy *wiphy,
 
 static void rtw_cfg80211_init_ht_capab(struct ieee80211_sta_ht_cap *ht_cap, enum nl80211_band band, u8 rf_type)
 {
-
+#define MAX_BIT_RATE_40MHZ_MCS23	450	/* Mbps */
 #define MAX_BIT_RATE_40MHZ_MCS15	300	/* Mbps */
 #define MAX_BIT_RATE_40MHZ_MCS7		150	/* Mbps */
 
@@ -3330,24 +3330,26 @@ static void rtw_cfg80211_init_ht_capab(struct ieee80211_sta_ht_cap *ht_cap, enum
 	 *if BW_40 rx_mask[4]= 0x01;
 	 *highest supported RX rate
 	 */
-	if (rf_type == RF_1T1R)
-	{
+	if (rf_type == RF_1T1R) {
 		ht_cap->mcs.rx_mask[0] = 0xFF;
 		ht_cap->mcs.rx_mask[1] = 0x00;
 		ht_cap->mcs.rx_mask[4] = 0x01;
 
 		ht_cap->mcs.rx_highest = cpu_to_le16(MAX_BIT_RATE_40MHZ_MCS7);
-	}
-	else if ((rf_type == RF_1T2R) || (rf_type ==RF_2T2R))
-	{
+	} else if ((rf_type == RF_1T2R) || (rf_type == RF_2T2R)
+			|| (rf_type == RF_2T2R_GREEN)) {
 		ht_cap->mcs.rx_mask[0] = 0xFF;
 		ht_cap->mcs.rx_mask[1] = 0xFF;
 		ht_cap->mcs.rx_mask[4] = 0x01;
 
 		ht_cap->mcs.rx_highest = cpu_to_le16(MAX_BIT_RATE_40MHZ_MCS15);
-	}
-	else
-	{
+	} else if ((rf_type == RF_2T3R) || (rf_type == RF_3T3R)) {
+		ht_cap->mcs.rx_mask[0] = 0xFF;
+		ht_cap->mcs.rx_mask[1] = 0xFF;
+		ht_cap->mcs.rx_mask[2] = 0x01;
+
+		ht_cap->mcs.rx_highest = cpu_to_le16(MAX_BIT_RATE_40MHZ_MCS23);
+	} else {
 		DBG_8192C("%s, error rf_type =%d\n", __func__, rf_type);
 	}
 
-- 
2.7.4

Re: [PATCH] Update rf_type_definition

[Dan Carpenter]

This is better.  But the subject should say [PATCH v2] and we need a
driver prefix.

On Wed, Feb 07, 2018 at 06:23:34AM +0900, l4stpr0gr4m@gmail.com wrote:
> From: Kangmin Park <l4stpr0gr4m@gmail.com>
> 
> enum RT_RF_TYPE_DEFINITION on rtl8723bs/include/rtw_rf.h
> is different from enum tag_HAL_RF_Type_Definition on
> rtl8723bs/include/HalVerDef.h
> 
> So, update them to be the same reference from
> enum rf_type on rtlwifi/wifi.h which recent version.
> 
> As a result, ODM_RF_TYPE_E needs to be updated as well
> Therefore, it is updated reference from enum odm_rf_type
> on rtlwifi/phydm/phydm_pre_define.h too.
> And update some additional code to check more
> chip versions that resulted from it.
> 
> Also, fixed some space required errors and
> line over 80 characters warnings by checkpatch.pl.

Don't do this.  It's considered unrelated white space change and it
makes the code hard to review.

To be honest, I don't know the code well enough to say if this patch is
a good idea generally...  It seems risky and I wish someone would test
it or a maintainer would Ack it.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Update rf_type_definition

[Kangmin Park]

[-- Attachment #1: Type: text/plain, Size: 1462 bytes --]

Thank you for your feedback. I learned a lot.
I thought I had to fix all warnings by checkpatch.pl.
If the patch is not good, I'll resubmit patch v3 which just fix space
required error by checkpatch later.

Regards.
Kangmin Park.


2018. 2. 7. 오후 8:14에 "Dan Carpenter" <dan.carpenter@oracle.com>님이 작성:

This is better.  But the subject should say [PATCH v2] and we need a
driver prefix.

On Wed, Feb 07, 2018 at 06:23:34AM +0900, l4stpr0gr4m@gmail.com wrote:
> From: Kangmin Park <l4stpr0gr4m@gmail.com>
>
> enum RT_RF_TYPE_DEFINITION on rtl8723bs/include/rtw_rf.h
> is different from enum tag_HAL_RF_Type_Definition on
> rtl8723bs/include/HalVerDef.h
>
> So, update them to be the same reference from
> enum rf_type on rtlwifi/wifi.h which recent version.
>
> As a result, ODM_RF_TYPE_E needs to be updated as well
> Therefore, it is updated reference from enum odm_rf_type
> on rtlwifi/phydm/phydm_pre_define.h too.
> And update some additional code to check more
> chip versions that resulted from it.
>
> Also, fixed some space required errors and
> line over 80 characters warnings by checkpatch.pl.

Don't do this.  It's considered unrelated white space change and it
makes the code hard to review.

To be honest, I don't know the code well enough to say if this patch is
a good idea generally...  It seems risky and I wish someone would test
it or a maintainer would Ack it.

regards,
dan carpenter

[-- Attachment #2: Type: text/html, Size: 2320 bytes --]

Re: [PATCH] Update rf_type_definition

[Greg KH]

On Wed, Feb 07, 2018 at 06:23:34AM +0900, l4stpr0gr4m@gmail.com wrote:
> From: Kangmin Park <l4stpr0gr4m@gmail.com>

Your subject line should say the subsystem and driver you are modifying.
Look at the hundreds of other patches on the mailing lists for examples
of how to properly format it.

> 
> enum RT_RF_TYPE_DEFINITION on rtl8723bs/include/rtw_rf.h
> is different from enum tag_HAL_RF_Type_Definition on
> rtl8723bs/include/HalVerDef.h
> 
> So, update them to be the same reference from
> enum rf_type on rtlwifi/wifi.h which recent version.
> 
> As a result, ODM_RF_TYPE_E needs to be updated as well
> Therefore, it is updated reference from enum odm_rf_type
> on rtlwifi/phydm/phydm_pre_define.h too.
> And update some additional code to check more
> chip versions that resulted from it.
> 
> Also, fixed some space required errors and
> line over 80 characters warnings by checkpatch.pl.

When you say "also", that means you should be splitting this patch up
into multpile patches.  Please only do one-thing-per-patch.

This should be a patch series, please fix it up and resend.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel