staging: r8188eu: SetHwReg8188EU() cleanups

PATCH Superseded Michael Straube 2 participants 15 messages Mar 28, 2022 – Mar 29, 2022 Staging

Share & tools

Michael Straube

author

2022-03-28 11:39 UTC #0 lore raw

cover staging: r8188eu: SetHwReg8188EU() cleanups

This series removes some cases from the function SetHwReg8188EU() and moves the functionality to the callers. This is part of the ongoing effort to get rid of the unwanted hal layer.

Tested on x86_64 with Inter-Tech DMG-02.

Michael Straube (9):
staging: r8188eu: remove HW_VAR_RESP_SIFS from SetHwReg8188EU()
staging: r8188eu: remove HW_VAR_MLME_DISCONNECT from SetHwReg8188EU()
staging: r8188eu: rename some macros to upper case
staging: r8188eu: remove HW_VAR_ACM_CTRL from SetHwReg8188EU()
staging: r8188eu: remove HW_VAR_RXDMA_AGG_PG_TH from SetHwReg8188EU()
staging: r8188eu: remove unnecessary if statement
staging: r8188eu: simplify if-else statement
staging: r8188eu: remove HW_VAR_TX_RPT_MAX_MACID from SetHwReg8188EU()
staging: r8188eu: remove HW_VAR_BCN_VALID from SetHwReg8188EU()

 drivers/staging/r8188eu/core/rtw_mlme.c       | 22 +++----
 drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 27 ++++++--
 drivers/staging/r8188eu/core/rtw_wlan_util.c  | 38 ++++++++---
 drivers/staging/r8188eu/hal/rtl8188e_cmd.c    | 12 ++--
 drivers/staging/r8188eu/hal/usb_halinit.c     | 63 -------------------
 drivers/staging/r8188eu/include/hal_intf.h    |  5 --
 .../staging/r8188eu/include/rtl8188e_spec.h   | 10 +--
 7 files changed, 73 insertions(+), 104 deletions(-)

Signature · 1 line

2.35.1

Michael Straube

author

2022-03-28 11:39 UTC #1 lore raw

PATCH 1/9 staging: r8188eu: remove HW_VAR_RESP_SIFS from SetHwReg8188EU()

SetHwReg8188EU() is called with HW_VAR_RESP_SIFS only in one place. Remove the HW_VAR_RESP_SIFS case from SetHwReg8188EU() and call rtw_write8() directly instead. This is part of the ongoing effort to get rid of the unwanted hal layer.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_wlan_util.c | 11 ++++++-----
 drivers/staging/r8188eu/hal/usb_halinit.c    |  8 --------
 drivers/staging/r8188eu/include/hal_intf.h   |  1 -
 3 files changed, 6 insertions(+), 14 deletions(-)

drivers/staging/r8188eu/core/rtw_wlan_util.c +6 -5

index 665b077190bc..eda4e5f9a6af 100644
@@ -1338,7 +1338,6 @@ void update_capinfo(struct adapter *Adapter, u16 updateCap)
 void update_wireless_mode(struct adapter *padapter)
 {
 	int ratelen, network_type = 0;
-	u32 SIFS_Timer;
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 	struct wlan_bssid_ex	*cur_network = &pmlmeinfo->network;

@@ -1365,10 +1364,12 @@ void update_wireless_mode(struct adapter *padapter)
 
 	pmlmeext->cur_wireless_mode = network_type & padapter->registrypriv.wireless_mode;
 
-	SIFS_Timer = 0x0a0a0808;/* 0x0808 -> for CCK, 0x0a0a -> for OFDM */
-				/* change this value if having IOT issues. */
-
-	SetHwReg8188EU(padapter, HW_VAR_RESP_SIFS, (u8 *)&SIFS_Timer);
+	/* RESP_SIFS for CCK */
+	rtw_write8(padapter, REG_R2T_SIFS, 0x08);
+	rtw_write8(padapter, REG_R2T_SIFS + 1, 0x08);
+	/* RESP_SIFS for OFDM */
+	rtw_write8(padapter, REG_T2T_SIFS, 0x0a);
+	rtw_write8(padapter, REG_T2T_SIFS + 1, 0x0a);
 
 	if (pmlmeext->cur_wireless_mode & WIRELESS_11B)
 		update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB);

drivers/staging/r8188eu/hal/usb_halinit.c +0 -8

index a92774352d2d..4ee12e260d36 100644
@@ -1119,14 +1119,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 			}
 		}
 		break;
-	case HW_VAR_RESP_SIFS:
-		/* RESP_SIFS for CCK */
-		rtw_write8(Adapter, REG_R2T_SIFS, val[0]); /*  SIFS_T2T_CCK (0x08) */
-		rtw_write8(Adapter, REG_R2T_SIFS + 1, val[1]); /* SIFS_R2T_CCK(0x08) */
-		/* RESP_SIFS for OFDM */
-		rtw_write8(Adapter, REG_T2T_SIFS, val[2]); /* SIFS_T2T_OFDM (0x0a) */
-		rtw_write8(Adapter, REG_T2T_SIFS + 1, val[3]); /* SIFS_R2T_OFDM(0x0a) */
-		break;
 	case HW_VAR_ACK_PREAMBLE:
 		{
 			u8 regTmp;

drivers/staging/r8188eu/include/hal_intf.h +0 -1

index 3cededa4dcfc..d49e6e6505ea 100644
@@ -17,7 +17,6 @@ enum hw_variables {
 	HW_VAR_MLME_SITESURVEY,
 	HW_VAR_MLME_JOIN,
 	HW_VAR_SLOT_TIME,
-	HW_VAR_RESP_SIFS,
 	HW_VAR_ACK_PREAMBLE,
 	HW_VAR_BCN_VALID,
 	HW_VAR_DM_FLAG,

Signature · 1 line

2.35.1

Michael Straube

author

2022-03-28 11:39 UTC #2 lore raw

PATCH 2/9 staging: r8188eu: remove HW_VAR_MLME_DISCONNECT from SetHwReg8188EU()

In reply to Michael Straube

SetHwReg8188EU() is called with HW_VAR_MLME_DISCONNECT only from functions in rtw_mlme_ext.c. Move the functionality into a static function in rtw_mlme_ext.c and remove the HW_VAR_MLME_DISCONNECT case from SetHwReg8188EU(). This is part of the ongoing effort to get rid of the unwanted hal layer.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 19 ++++++++++++++++---
 drivers/staging/r8188eu/hal/usb_halinit.c   | 11 -----------
 drivers/staging/r8188eu/include/hal_intf.h  |  1 -
 3 files changed, 16 insertions(+), 15 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme_ext.c +16 -3

index 10d5f1222936..b2adb05856a0 100644
@@ -6800,13 +6800,26 @@ void mlmeext_sta_add_event_callback(struct adapter *padapter, struct sta_info *p
 	update_sta_info(padapter, psta);
 }
 
+static void mlme_disconnect(struct adapter *adapter)
+{
+	/* Set RCR to not to receive data frame when NO LINK state */
+	/* reject all data frames */
+	rtw_write16(adapter, REG_RXFLTMAP2, 0x00);
+
+	/* reset TSF */
+	rtw_write8(adapter, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
+
+	/* disable update TSF */
+	rtw_write8(adapter, REG_BCN_CTRL, rtw_read8(adapter, REG_BCN_CTRL) | BIT(4));
+}
+
 void mlmeext_sta_del_event_callback(struct adapter *padapter)
 {
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 
 	if (is_client_associated_to_ap(padapter) || is_IBSS_empty(padapter)) {
-		SetHwReg8188EU(padapter, HW_VAR_MLME_DISCONNECT, NULL);
+		mlme_disconnect(padapter);
 		SetHwReg8188EU(padapter, HW_VAR_BSSID, null_addr);
 
 		/* restore to initial setting. */

@@ -7170,7 +7183,7 @@ u8 join_cmd_hdl(struct adapter *padapter, u8 *pbuf)
 		/* set MSR to nolink -> infra. mode */
 		Set_MSR(padapter, _HW_STATE_STATION_);
 
-		SetHwReg8188EU(padapter, HW_VAR_MLME_DISCONNECT, NULL);
+		mlme_disconnect(padapter);
 	}
 
 	rtw_antenna_select_cmd(padapter, pparm->network.PhyInfo.Optimum_antenna, false);

@@ -7266,7 +7279,7 @@ u8 disconnect_hdl(struct adapter *padapter, unsigned char *pbuf)
 	if (is_client_associated_to_ap(padapter))
 		issue_deauth_ex(padapter, pnetwork->MacAddress, WLAN_REASON_DEAUTH_LEAVING, param->deauth_timeout_ms / 100, 100);
 
-	SetHwReg8188EU(padapter, HW_VAR_MLME_DISCONNECT, NULL);
+	mlme_disconnect(padapter);
 	SetHwReg8188EU(padapter, HW_VAR_BSSID, null_addr);
 
 	/* restore to initial setting. */

drivers/staging/r8188eu/hal/usb_halinit.c +0 ~~-11~~

index 4ee12e260d36..cf1e4bbfd6a9 100644
@@ -1024,17 +1024,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 				ResumeTxBeacon(Adapter);
 		}
 		break;
-	case HW_VAR_MLME_DISCONNECT:
-		/* Set RCR to not to receive data frame when NO LINK state */
-		/* reject all data frames */
-		rtw_write16(Adapter, REG_RXFLTMAP2, 0x00);
-
-		/* reset TSF */
-		rtw_write8(Adapter, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
-
-		/* disable update TSF */
-		rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) | BIT(4));
-		break;
 	case HW_VAR_MLME_SITESURVEY:
 		if (*((u8 *)val)) { /* under sitesurvey */
 			/* config RCR to receive different BSSID & not to receive data frame */

drivers/staging/r8188eu/include/hal_intf.h +0 -1

index d49e6e6505ea..a2c5724790f6 100644
@@ -13,7 +13,6 @@ enum hw_variables {
 	HW_VAR_BSSID,
 	HW_VAR_BASIC_RATE,
 	HW_VAR_CORRECT_TSF,
-	HW_VAR_MLME_DISCONNECT,
 	HW_VAR_MLME_SITESURVEY,
 	HW_VAR_MLME_JOIN,
 	HW_VAR_SLOT_TIME,

Signature · 1 line

2.35.1

Michael Straube

author

2022-03-28 11:39 UTC #3 lore raw

PATCH 3/9 staging: r8188eu: rename some macros to upper case

In reply to Michael Straube

Rename some macros to upper case to avoid camel case and improve readability. While at it, remove some unused macros.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/hal/usb_halinit.c       | 12 ++++++------
 drivers/staging/r8188eu/include/rtl8188e_spec.h | 10 +++-------
 2 files changed, 9 insertions(+), 13 deletions(-)

drivers/staging/r8188eu/hal/usb_halinit.c +6 -6

index cf1e4bbfd6a9..26a86b891aa1 100644
@@ -1152,19 +1152,19 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 				AcmCtrl = AcmCtrl | 0x1;
 
 			if (acm_ctrl & BIT(3))
-				AcmCtrl |= AcmHw_VoqEn;
+				AcmCtrl |= ACMHW_VOQEN;
 			else
-				AcmCtrl &= (~AcmHw_VoqEn);
+				AcmCtrl &= (~ACMHW_VOQEN);
 
 			if (acm_ctrl & BIT(2))
-				AcmCtrl |= AcmHw_ViqEn;
+				AcmCtrl |= ACMHW_VIQEN;
 			else
-				AcmCtrl &= (~AcmHw_ViqEn);
+				AcmCtrl &= (~ACMHW_VIQEN);
 
 			if (acm_ctrl & BIT(1))
-				AcmCtrl |= AcmHw_BeqEn;
+				AcmCtrl |= ACMHW_BEQEN;
 			else
-				AcmCtrl &= (~AcmHw_BeqEn);
+				AcmCtrl &= (~ACMHW_BEQEN);
 
 			rtw_write8(Adapter, REG_ACMHWCTRL, AcmCtrl);
 		}

drivers/staging/r8188eu/include/rtl8188e_spec.h +3 -7

index edae053e350e..ef42c4b2f20c 100644
@@ -998,13 +998,9 @@ Current IOREG MAP
 #define STOP_BCNQ		BIT(6)
 
 /* 2 ACMHWCTRL */
-#define	AcmHw_HwEn		BIT(0)
-#define	AcmHw_BeqEn		BIT(1)
-#define	AcmHw_ViqEn		BIT(2)
-#define	AcmHw_VoqEn		BIT(3)
-#define	AcmHw_BeqStatus		BIT(4)
-#define	AcmHw_ViqStatus		BIT(5)
-#define	AcmHw_VoqStatus		BIT(6)
+#define ACMHW_BEQEN		BIT(1)
+#define ACMHW_VIQEN		BIT(2)
+#define ACMHW_VOQEN		BIT(3)
 
 /* 	0x0600h ~ 0x07FFh	WMAC Configuration */
 /* 2APSD_CTRL */

Signature · 1 line

2.35.1

Michael Straube

author

2022-03-28 11:39 UTC #4 lore raw

PATCH 4/9 staging: r8188eu: remove HW_VAR_ACM_CTRL from SetHwReg8188EU()

In reply to Michael Straube

SetHwReg8188EU() is called with HW_VAR_ACM_CTRL only in one place. Remove the HW_VAR_RESP_SIFS case from SetHwReg8188EU() and replace its call with the code from that case. This is part of the ongoing effort to get rid of the unwanted hal layer.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_wlan_util.c | 27 +++++++++++++++++---
 drivers/staging/r8188eu/hal/usb_halinit.c    | 26 -------------------
 drivers/staging/r8188eu/include/hal_intf.h   |  1 -
 3 files changed, 24 insertions(+), 30 deletions(-)

drivers/staging/r8188eu/core/rtw_wlan_util.c +24 -3

index eda4e5f9a6af..cde3779ff21d 100644
@@ -571,10 +571,31 @@ void WMMOnAssocRsp(struct adapter *padapter)
 		}
 	}
 
-	if (padapter->registrypriv.acm_method == 1)
-		SetHwReg8188EU(padapter, HW_VAR_ACM_CTRL, (u8 *)(&acm_mask));
-	else
+	if (padapter->registrypriv.acm_method == 1) {
+		u8 acmctrl = rtw_read8(padapter, REG_ACMHWCTRL);
+
+		if (acm_mask > 1)
+			acmctrl = acmctrl | 0x1;
+
+		if (acm_mask & BIT(3))
+			acmctrl |= ACMHW_VOQEN;
+		else
+			acmctrl &= (~ACMHW_VOQEN);
+
+		if (acm_mask & BIT(2))
+			acmctrl |= ACMHW_VIQEN;
+		else
+			acmctrl &= (~ACMHW_VIQEN);
+
+		if (acm_mask & BIT(1))
+			acmctrl |= ACMHW_BEQEN;
+		else
+			acmctrl &= (~ACMHW_BEQEN);
+
+		rtw_write8(padapter, REG_ACMHWCTRL, acmctrl);
+	} else {
 		padapter->mlmepriv.acm_mask = acm_mask;
+	}
 
 	inx[0] = 0; inx[1] = 1; inx[2] = 2; inx[3] = 3;

drivers/staging/r8188eu/hal/usb_halinit.c +0 ~~-26~~

index 26a86b891aa1..86d8b603d4fa 100644
@@ -1143,32 +1143,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 		haldata->AcParam_BE = ((u32 *)(val))[0];
 		rtw_write32(Adapter, REG_EDCA_BE_PARAM, ((u32 *)(val))[0]);
 		break;
-	case HW_VAR_ACM_CTRL:
-		{
-			u8 acm_ctrl = *((u8 *)val);
-			u8 AcmCtrl = rtw_read8(Adapter, REG_ACMHWCTRL);
-
-			if (acm_ctrl > 1)
-				AcmCtrl = AcmCtrl | 0x1;
-
-			if (acm_ctrl & BIT(3))
-				AcmCtrl |= ACMHW_VOQEN;
-			else
-				AcmCtrl &= (~ACMHW_VOQEN);
-
-			if (acm_ctrl & BIT(2))
-				AcmCtrl |= ACMHW_VIQEN;
-			else
-				AcmCtrl &= (~ACMHW_VIQEN);
-
-			if (acm_ctrl & BIT(1))
-				AcmCtrl |= ACMHW_BEQEN;
-			else
-				AcmCtrl &= (~ACMHW_BEQEN);
-
-			rtw_write8(Adapter, REG_ACMHWCTRL, AcmCtrl);
-		}
-		break;
 	case HW_VAR_AMPDU_MIN_SPACE:
 		{
 			u8 MinSpacingToSet;

drivers/staging/r8188eu/include/hal_intf.h +0 -1

index a2c5724790f6..d0317f5b85ba 100644
@@ -23,7 +23,6 @@ enum hw_variables {
 	HW_VAR_DM_FUNC_SET,
 	HW_VAR_DM_FUNC_CLR,
 	HW_VAR_AC_PARAM_BE,
-	HW_VAR_ACM_CTRL,
 	HW_VAR_AMPDU_MIN_SPACE,
 	HW_VAR_AMPDU_FACTOR,
 	HW_VAR_RXDMA_AGG_PG_TH,

Signature · 1 line

2.35.1

Greg KH

maintainer

2022-03-29 16:02 UTC #5 lore raw

Re: [PATCH 4/9] staging: r8188eu: remove HW_VAR_ACM_CTRL from SetHwReg8188EU()

Quoted 35 lines · Michael Straube

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_wlan_util.c | 27 +++++++++++++++++---
 drivers/staging/r8188eu/hal/usb_halinit.c    | 26 -------------------
 drivers/staging/r8188eu/include/hal_intf.h   |  1 -
 3 files changed, 24 insertions(+), 30 deletions(-)

drivers/staging/r8188eu/core/rtw_wlan_util.c +12 -3

index eda4e5f9a6af..cde3779ff21d 100644
@@ -571,10 +571,31 @@ void WMMOnAssocRsp(struct adapter *padapter)
 		}
 	}
 
-	if (padapter->registrypriv.acm_method == 1)
-		SetHwReg8188EU(padapter, HW_VAR_ACM_CTRL, (u8 *)(&acm_mask));
-	else
+	if (padapter->registrypriv.acm_method == 1) {
+		u8 acmctrl = rtw_read8(padapter, REG_ACMHWCTRL);
+
+		if (acm_mask > 1)
+			acmctrl = acmctrl | 0x1;
+
+		if (acm_mask & BIT(3))
+			acmctrl |= ACMHW_VOQEN;
+		else
+			acmctrl &= (~ACMHW_VOQEN);
+
+		if (acm_mask & BIT(2))

drivers/staging/r8188eu/core/rtw_wlan_util.c void WMMOnAssocRsp(struct adapter *padapter) · line 595

Quoted 10 lines

drivers/staging/r8188eu/core/rtw_wlan_util.c +10 -0

+			acmctrl |= ACMHW_VIQEN;
+		else
+			acmctrl &= (~ACMHW_VIQEN);
+
+		if (acm_mask & BIT(1))
+			acmctrl |= ACMHW_BEQEN;
+		else
+			acmctrl &= (~ACMHW_BEQEN);
+
+		rtw_write8(padapter, REG_ACMHWCTRL, acmctrl);

Shouldn't be a function that explains what you are doing here? Like you did with previous changes in this patch series?

thanks,

greg k-h

Michael Straube

author

2022-03-29 16:25 UTC #6 lore raw

Re: [PATCH 4/9] staging: r8188eu: remove HW_VAR_ACM_CTRL from SetHwReg8188EU()

Quoted 41 lines · Greg KH

Quoted 40 lines · Michael Straube

Signed-off-by Michael Straube <[email protected]>

---
  drivers/staging/r8188eu/core/rtw_wlan_util.c | 27 +++++++++++++++++---
  drivers/staging/r8188eu/hal/usb_halinit.c    | 26 -------------------
  drivers/staging/r8188eu/include/hal_intf.h   |  1 -
  3 files changed, 24 insertions(+), 30 deletions(-)

drivers/staging/r8188eu/core/rtw_wlan_util.c +17 -3

index eda4e5f9a6af..cde3779ff21d 100644
@@ -571,10 +571,31 @@ void WMMOnAssocRsp(struct adapter *padapter)
  		}
  	}
 
-	if (padapter->registrypriv.acm_method == 1)
-		SetHwReg8188EU(padapter, HW_VAR_ACM_CTRL, (u8 *)(&acm_mask));
-	else
+	if (padapter->registrypriv.acm_method == 1) {
+		u8 acmctrl = rtw_read8(padapter, REG_ACMHWCTRL);
+
+		if (acm_mask > 1)
+			acmctrl = acmctrl | 0x1;
+
+		if (acm_mask & BIT(3))
+			acmctrl |= ACMHW_VOQEN;
+		else
+			acmctrl &= (~ACMHW_VOQEN);
+
+		if (acm_mask & BIT(2))
+			acmctrl |= ACMHW_VIQEN;
+		else
+			acmctrl &= (~ACMHW_VIQEN);
+
+		if (acm_mask & BIT(1))

Quoted 9 lines

drivers/staging/r8188eu/core/rtw_wlan_util.c void WMMOnAssocRsp(struct adapter *padapter) · line 595
Quoted 5 lines
drivers/staging/r8188eu/core/rtw_wlan_util.c +5 -0
+			acmctrl |= ACMHW_BEQEN;
+		else
+			acmctrl &= (~ACMHW_BEQEN);
+
+		rtw_write8(padapter, REG_ACMHWCTRL, acmctrl);
Shouldn't be a function that explains what you are doing here? Like you did with previous changes in this patch series?

Sound like a good idea.
I'll send v2 and address your other comments to this series too.

Thanks,
Michael

Michael Straube

author

2022-03-28 11:39 UTC #7 lore raw

PATCH 5/9 staging: r8188eu: remove HW_VAR_RXDMA_AGG_PG_TH from SetHwReg8188EU()

In reply to Michael Straube

The HW_VAR_RXDMA_AGG_PG_TH case in SetHwReg8188EU() just sets a variable conditionally and then calls rtw_write8(). Set the variable in the caller and, call rtw_write8() directly and remove the HW_VAR_RXDMA_AGG_PG_TH case from SetHwReg8188EU(). This is part of the ongoing effort to get rid of the unwanted hal layer.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c    | 7 +++++--
 drivers/staging/r8188eu/hal/usb_halinit.c  | 8 --------
 drivers/staging/r8188eu/include/hal_intf.h | 1 -
 3 files changed, 5 insertions(+), 11 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +5 -2

index 6f0bff186477..14aff6a2040a 100644
@@ -1817,10 +1817,13 @@ void rtw_joinbss_reset(struct adapter *padapter)
 			threshold = 1;
 		else
 			threshold = 0;
-		SetHwReg8188EU(padapter, HW_VAR_RXDMA_AGG_PG_TH, (u8 *)(&threshold));
+
+		if (threshold == 0)
+			threshold = USB_RXAGG_PAGE_COUNT;
+		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
 	} else {
 		threshold = 1;
-		SetHwReg8188EU(padapter, HW_VAR_RXDMA_AGG_PG_TH, (u8 *)(&threshold));
+		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
 	}
 }

drivers/staging/r8188eu/hal/usb_halinit.c +0 -8

index 86d8b603d4fa..219f0e5d36aa 100644
@@ -1197,14 +1197,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 			}
 		}
 		break;
-	case HW_VAR_RXDMA_AGG_PG_TH:
-		{
-			u8 threshold = *((u8 *)val);
-			if (threshold == 0)
-				threshold = USB_RXAGG_PAGE_COUNT;
-			rtw_write8(Adapter, REG_RXDMA_AGG_PG_TH, threshold);
-		}
-		break;
 	case HW_VAR_H2C_FW_PWRMODE:
 		{
 			u8 psmode = (*(u8 *)val);

drivers/staging/r8188eu/include/hal_intf.h +0 -1

index d0317f5b85ba..9d755eb59224 100644
@@ -25,7 +25,6 @@ enum hw_variables {
 	HW_VAR_AC_PARAM_BE,
 	HW_VAR_AMPDU_MIN_SPACE,
 	HW_VAR_AMPDU_FACTOR,
-	HW_VAR_RXDMA_AGG_PG_TH,
 	HW_VAR_H2C_FW_PWRMODE,
 	HW_VAR_H2C_FW_JOINBSSRPT,
 	HW_VAR_FWLPS_RF_ON,

Signature · 1 line

2.35.1

Greg KH

maintainer

2022-03-29 16:04 UTC #8 lore raw

Re: [PATCH 5/9] staging: r8188eu: remove HW_VAR_RXDMA_AGG_PG_TH from SetHwReg8188EU()

Quoted 20 lines · Michael Straube

The HW_VAR_RXDMA_AGG_PG_TH case in SetHwReg8188EU() just sets a variable conditionally and then calls rtw_write8(). Set the variable in the caller and, call rtw_write8() directly and remove the HW_VAR_RXDMA_AGG_PG_TH case from SetHwReg8188EU(). This is part of the ongoing effort to get rid of the unwanted hal layer.
Signed-off-by Michael Straube <[email protected]>
---
 drivers/staging/r8188eu/core/rtw_mlme.c    | 7 +++++--
 drivers/staging/r8188eu/hal/usb_halinit.c  | 8 --------
 drivers/staging/r8188eu/include/hal_intf.h | 1 -
 3 files changed, 5 insertions(+), 11 deletions(-)
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
index 6f0bff186477..14aff6a2040a 100644
@@ -1817,10 +1817,13 @@ void rtw_joinbss_reset(struct adapter *padapter)
 			threshold = 1;
 		else

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_joinbss_reset(struct adapter *padapter) · line 1826

Quoted 10 lines

drivers/staging/r8188eu/core/rtw_mlme.c +5 -2

 			threshold = 0;
-		SetHwReg8188EU(padapter, HW_VAR_RXDMA_AGG_PG_TH, (u8 *)(&threshold));
+
+		if (threshold == 0)
+			threshold = USB_RXAGG_PAGE_COUNT;
+		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
 	} else {
 		threshold = 1;
-		SetHwReg8188EU(padapter, HW_VAR_RXDMA_AGG_PG_TH, (u8 *)(&threshold));
+		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);

Can't you just ue 1 for the last parameter here?

And what about making this a function:
rtw_set_threshold()?

thanks,

greg k-h

Michael Straube

author

2022-03-28 11:39 UTC #9 lore raw

PATCH 6/9 staging: r8188eu: remove unnecessary if statement

In reply to Michael Straube

The local variable threshold in rtw_joinbss_reset() is set to 0 and immediately set to another value if it is 0. Remove this unnecessary if and assign the final value in the first place.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +1 -3

index 14aff6a2040a..83841cf85a0d 100644
@@ -1816,10 +1816,8 @@ void rtw_joinbss_reset(struct adapter *padapter)
 		if (padapter->registrypriv.wifi_spec == 1)
 			threshold = 1;
 		else
-			threshold = 0;
-
-		if (threshold == 0)
 			threshold = USB_RXAGG_PAGE_COUNT;
+
 		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
 	} else {
 		threshold = 1;

Signature · 1 line

2.35.1

Michael Straube

author

2022-03-28 11:39 UTC #10 lore raw

PATCH 7/9 staging: r8188eu: simplify if-else statement

In reply to Michael Straube

Simplify a nested if-else statement to a single if statement in rtw_joinbss_reset(). This improves readability and shortens the code.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +7 ~~-11~~

index 83841cf85a0d..1569f719af1b 100644
@@ -1810,19 +1810,15 @@ void rtw_joinbss_reset(struct adapter *padapter)
 
 	phtpriv->ampdu_enable = false;/* reset to disabled */
 
-	/*  TH = 1 => means that invalidate usb rx aggregation */
-	/*  TH = 0 => means that validate usb rx aggregation, use init value. */
-	if (phtpriv->ht_option) {
-		if (padapter->registrypriv.wifi_spec == 1)
-			threshold = 1;
-		else
-			threshold = USB_RXAGG_PAGE_COUNT;
+	 /* invalidate usb rx aggregation */
+	threshold = 1;
 
-		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
-	} else {
-		threshold = 1;
-		rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
+	if (phtpriv->ht_option && padapter->registrypriv.wifi_spec != 1) {
+		/* validate usb rx aggregation, use init value. */
+		threshold = USB_RXAGG_PAGE_COUNT;
 	}
+
+	rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, threshold);
 }
 
 /* the function is >= passive_level */

Signature · 1 line

2.35.1

Michael Straube

author

2022-03-28 11:40 UTC #11 lore raw

PATCH 8/9 staging: r8188eu: remove HW_VAR_TX_RPT_MAX_MACID from SetHwReg8188EU()

In reply to Michael Straube

The HW_VAR_TX_RPT_MAX_MACID case in SetHwReg8188EU() just calls rtw_write8(). Remove HW_VAR_TX_RPT_MAX_MACID from SetHwReg8188EU() and call rtw_write8() directly. This is part of the ongoing effort to get rid of the unwanted hal layer.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c     | 3 ++-
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
 drivers/staging/r8188eu/hal/usb_halinit.c   | 6 ------
 drivers/staging/r8188eu/include/hal_intf.h  | 1 -
 4 files changed, 4 insertions(+), 9 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +2 -1

index 1569f719af1b..06c17a16dab9 100644
@@ -1141,7 +1141,8 @@ void rtw_sta_media_status_rpt(struct adapter *adapter, struct sta_info *psta,
 		return;
 
 	macid = search_max_mac_id(adapter);
-	SetHwReg8188EU(adapter, HW_VAR_TX_RPT_MAX_MACID, (u8 *)&macid);
+	rtw_write8(adapter, REG_TX_RPT_CTRL + 1, macid + 1);
+
 	/* MACID|OPMODE:1 connect */
 	media_status_rpt = (u16)((psta->mac_id << 8) | mstatus);
 	SetHwReg8188EU(adapter, HW_VAR_H2C_MEDIA_STATUS_RPT, (u8 *)&media_status_rpt);

drivers/staging/r8188eu/core/rtw_mlme_ext.c +2 -1

index b2adb05856a0..b5c2e7d4cb48 100644
@@ -6750,7 +6750,8 @@ void mlmeext_joinbss_event_callback(struct adapter *padapter, int join_res)
 
 		/* set per sta rate after updating HT cap. */
 		set_sta_rate(padapter, psta);
-		SetHwReg8188EU(padapter, HW_VAR_TX_RPT_MAX_MACID, (u8 *)&psta->mac_id);
+		rtw_write8(padapter, REG_TX_RPT_CTRL + 1, psta->mac_id + 1);
+
 		media_status = (psta->mac_id << 8) | 1; /*   MACID|OPMODE: 1 means connect */
 		SetHwReg8188EU(padapter, HW_VAR_H2C_MEDIA_STATUS_RPT, (u8 *)&media_status);
 	}

drivers/staging/r8188eu/hal/usb_halinit.c +0 -6

index 219f0e5d36aa..4be5c5ed8711 100644
@@ -1278,12 +1278,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 			}
 		}
 		break;
-	case HW_VAR_TX_RPT_MAX_MACID:
-		{
-			u8 maxMacid = *val;
-			rtw_write8(Adapter, REG_TX_RPT_CTRL + 1, maxMacid + 1);
-		}
-		break;
 	case HW_VAR_H2C_MEDIA_STATUS_RPT:
 		rtl8188e_set_FwMediaStatus_cmd(Adapter, (*(__le16 *)val));
 		break;

drivers/staging/r8188eu/include/hal_intf.h +0 -1

index 9d755eb59224..98f00c46fe8e 100644
@@ -33,7 +33,6 @@ enum hw_variables {
 	HW_VAR_ANTENNA_DIVERSITY_SELECT,
 	HW_VAR_FIFO_CLEARN_UP,
 	HW_VAR_RPT_TIMER_SETTING,
-	HW_VAR_TX_RPT_MAX_MACID,
 	HW_VAR_H2C_MEDIA_STATUS_RPT,
 	HW_VAR_CHK_HI_QUEUE_EMPTY,
 };

Signature · 1 line

2.35.1

Greg KH

maintainer

2022-03-29 16:04 UTC #12 lore raw

Re: [PATCH 8/9] staging: r8188eu: remove HW_VAR_TX_RPT_MAX_MACID from SetHwReg8188EU()

Quoted 18 lines · Michael Straube

The HW_VAR_TX_RPT_MAX_MACID case in SetHwReg8188EU() just calls rtw_write8(). Remove HW_VAR_TX_RPT_MAX_MACID from SetHwReg8188EU() and call rtw_write8() directly. This is part of the ongoing effort to get rid of the unwanted hal layer.
Signed-off-by Michael Straube <[email protected]>
---
 drivers/staging/r8188eu/core/rtw_mlme.c     | 3 ++-
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
 drivers/staging/r8188eu/hal/usb_halinit.c   | 6 ------
 drivers/staging/r8188eu/include/hal_intf.h  | 1 -
 4 files changed, 4 insertions(+), 9 deletions(-)
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
index 1569f719af1b..06c17a16dab9 100644

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_sta_media_status_rpt(struct adapter *adapter, struct sta_info *psta, · line 1144

Quoted 5 lines

drivers/staging/r8188eu/core/rtw_mlme.c +1 -1

@@ -1141,7 +1141,8 @@ void rtw_sta_media_status_rpt(struct adapter *adapter, struct sta_info *psta,
 		return;
 
 	macid = search_max_mac_id(adapter);
-	SetHwReg8188EU(adapter, HW_VAR_TX_RPT_MAX_MACID, (u8 *)&macid);
+	rtw_write8(adapter, REG_TX_RPT_CTRL + 1, macid + 1);

This too should be a function to help explain what you are doing.

thanks,

greg k-h

Michael Straube

author

2022-03-28 11:40 UTC #13 lore raw

PATCH 9/9 staging: r8188eu: remove HW_VAR_BCN_VALID from SetHwReg8188EU()

In reply to Michael Straube

The HW_VAR_BCN_VALID case in SetHwReg8188EU() just calls rtw_write8(). Remove HW_VAR_BCN_VALID from SetHwReg8188EU() and call rtw_write8() directly. This is part of the ongoing effort to getrid of the unwanted hal layer.

Signed-off-by Michael Straube <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c |  5 ++++-
 drivers/staging/r8188eu/hal/rtl8188e_cmd.c  | 12 ++++++++----
 drivers/staging/r8188eu/hal/usb_halinit.c   |  4 ----
 3 files changed, 12 insertions(+), 9 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme_ext.c +4 -1

index b5c2e7d4cb48..313e1a3beecb 100644
@@ -5765,7 +5765,10 @@ unsigned int send_beacon(struct adapter *padapter)
 
 	u32 start = jiffies;
 
-	SetHwReg8188EU(padapter, HW_VAR_BCN_VALID, NULL);
+	/* Clear beacon valid check bit. */
+	/* BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2, write 1 to clear, Clear by sw */
+	rtw_write8(padapter, REG_TDECTRL + 2, rtw_read8(padapter, REG_TDECTRL + 2) | BIT(0));
+
 	do {
 		issue_beacon(padapter, 100);
 		issue++;

drivers/staging/r8188eu/hal/rtl8188e_cmd.c +8 -4

index f1464e4ba429..f58284eba70a 100644
@@ -557,8 +557,9 @@ void rtl8188e_set_FwJoinBssReport_cmd(struct adapter *adapt, u8 mstatus)
 		rtw_write8(adapt, REG_FWHW_TXQ_CTRL + 2, (haldata->RegFwHwTxQCtrl & (~BIT(6))));
 		haldata->RegFwHwTxQCtrl &= (~BIT(6));
 
-		/*  Clear beacon valid check bit. */
-		SetHwReg8188EU(adapt, HW_VAR_BCN_VALID, NULL);
+		/* Clear beacon valid check bit. */
+		/* BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2, write 1 to clear, Clear by sw */
+		rtw_write8(adapt, REG_TDECTRL + 2, rtw_read8(adapt, REG_TDECTRL + 2) | BIT(0));
 		DLBcnCount = 0;
 		poll = 0;
 		do {

@@ -596,8 +597,11 @@ void rtl8188e_set_FwJoinBssReport_cmd(struct adapter *adapt, u8 mstatus)
 		}
 
 		/*  Update RSVD page location H2C to Fw. */
-		if (bcn_valid)
-			SetHwReg8188EU(adapt, HW_VAR_BCN_VALID, NULL);
+		if (bcn_valid) {
+			/* Clear beacon valid check bit. */
+			/* BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2, write 1 to clear, Clear by sw */
+			rtw_write8(adapt, REG_TDECTRL + 2, rtw_read8(adapt, REG_TDECTRL + 2) | BIT(0));
+		}
 
 		/*  Do not enable HW DMA BCN or it will cause Pcie interface hang by timing issue. 2011.11.24. by tynli. */
 		/*  Clear CR[8] or beacon packet will not be send to TxBuf anymore. */

drivers/staging/r8188eu/hal/usb_halinit.c +0 -4

index 4be5c5ed8711..8bff4c6c29cf 100644
@@ -1281,10 +1281,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 	case HW_VAR_H2C_MEDIA_STATUS_RPT:
 		rtl8188e_set_FwMediaStatus_cmd(Adapter, (*(__le16 *)val));
 		break;
-	case HW_VAR_BCN_VALID:
-		/* BCN_VALID, BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2, write 1 to clear, Clear by sw */
-		rtw_write8(Adapter, REG_TDECTRL + 2, rtw_read8(Adapter, REG_TDECTRL + 2) | BIT(0));
-		break;
 	default:
 		break;
 	}

Signature · 1 line

2.35.1

Greg KH

maintainer

2022-03-29 16:05 UTC #14 lore raw

Re: [PATCH 9/9] staging: r8188eu: remove HW_VAR_BCN_VALID from SetHwReg8188EU()

Quoted 17 lines · Michael Straube

The HW_VAR_BCN_VALID case in SetHwReg8188EU() just calls rtw_write8(). Remove HW_VAR_BCN_VALID from SetHwReg8188EU() and call rtw_write8() directly. This is part of the ongoing effort to getrid of the unwanted hal layer.
Signed-off-by Michael Straube <[email protected]>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c |  5 ++++-
 drivers/staging/r8188eu/hal/rtl8188e_cmd.c  | 12 ++++++++----
 drivers/staging/r8188eu/hal/usb_halinit.c   |  4 ----
 3 files changed, 12 insertions(+), 9 deletions(-)
drivers/staging/r8188eu/core/rtw_mlme_ext.c +0 -0
index b5c2e7d4cb48..313e1a3beecb 100644

drivers/staging/r8188eu/core/rtw_mlme_ext.c unsigned int send_beacon(struct adapter *padapter) · line 5770

Quoted 7 lines

drivers/staging/r8188eu/core/rtw_mlme_ext.c +3 -1

@@ -5765,7 +5765,10 @@ unsigned int send_beacon(struct adapter *padapter)
 
 	u32 start = jiffies;
 
-	SetHwReg8188EU(padapter, HW_VAR_BCN_VALID, NULL);
+	/* Clear beacon valid check bit. */
+	/* BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2, write 1 to clear, Clear by sw */
+	rtw_write8(padapter, REG_TDECTRL + 2, rtw_read8(padapter, REG_TDECTRL + 2) | BIT(0));

You commented this twice, it should be a single function :)

thanks,

greg k-h