[PATCH 0/9] staging: r8188eu: SetHwReg8188EU() cleanups

[PATCH 0/9] staging: r8188eu: SetHwReg8188EU() cleanups

[Michael Straube]

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(-)

-- 
2.35.1

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

[Michael Straube]

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 <straube.linux@gmail.com>
---
 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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index 665b077190bc..eda4e5f9a6af 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -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);
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index a92774352d2d..4ee12e260d36 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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;
diff --git a/drivers/staging/r8188eu/include/hal_intf.h b/drivers/staging/r8188eu/include/hal_intf.h
index 3cededa4dcfc..d49e6e6505ea 100644
--- a/drivers/staging/r8188eu/include/hal_intf.h
+++ b/drivers/staging/r8188eu/include/hal_intf.h
@@ -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,
-- 
2.35.1

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

[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 <straube.linux@gmail.com>
---
 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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 10d5f1222936..b2adb05856a0 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -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. */
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 4ee12e260d36..cf1e4bbfd6a9 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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 */
diff --git a/drivers/staging/r8188eu/include/hal_intf.h b/drivers/staging/r8188eu/include/hal_intf.h
index d49e6e6505ea..a2c5724790f6 100644
--- a/drivers/staging/r8188eu/include/hal_intf.h
+++ b/drivers/staging/r8188eu/include/hal_intf.h
@@ -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,
-- 
2.35.1

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

[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 <straube.linux@gmail.com>
---
 drivers/staging/r8188eu/hal/usb_halinit.c       | 12 ++++++------
 drivers/staging/r8188eu/include/rtl8188e_spec.h | 10 +++-------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index cf1e4bbfd6a9..26a86b891aa1 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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);
 		}
diff --git a/drivers/staging/r8188eu/include/rtl8188e_spec.h b/drivers/staging/r8188eu/include/rtl8188e_spec.h
index edae053e350e..ef42c4b2f20c 100644
--- a/drivers/staging/r8188eu/include/rtl8188e_spec.h
+++ b/drivers/staging/r8188eu/include/rtl8188e_spec.h
@@ -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 */
-- 
2.35.1

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

[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 <straube.linux@gmail.com>
---
 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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index eda4e5f9a6af..cde3779ff21d 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -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;
 
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 26a86b891aa1..86d8b603d4fa 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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;
diff --git a/drivers/staging/r8188eu/include/hal_intf.h b/drivers/staging/r8188eu/include/hal_intf.h
index a2c5724790f6..d0317f5b85ba 100644
--- a/drivers/staging/r8188eu/include/hal_intf.h
+++ b/drivers/staging/r8188eu/include/hal_intf.h
@@ -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,
-- 
2.35.1

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

[Greg KH]

On Mon, Mar 28, 2022 at 01:39:35PM +0200, Michael Straube wrote:
> 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 <straube.linux@gmail.com>
> ---
>  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(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> index eda4e5f9a6af..cde3779ff21d 100644
> --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> @@ -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);

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

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

[Michael Straube]

On 3/29/22 18:02, Greg KH wrote:
> On Mon, Mar 28, 2022 at 01:39:35PM +0200, Michael Straube wrote:
>> 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 <straube.linux@gmail.com>
>> ---
>>   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(-)
>>
>> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
>> index eda4e5f9a6af..cde3779ff21d 100644
>> --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
>> +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
>> @@ -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);
> 
> 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

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

[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 <straube.linux@gmail.com>
---
 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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 6f0bff186477..14aff6a2040a 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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);
 	}
 }
 
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 86d8b603d4fa..219f0e5d36aa 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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);
diff --git a/drivers/staging/r8188eu/include/hal_intf.h b/drivers/staging/r8188eu/include/hal_intf.h
index d0317f5b85ba..9d755eb59224 100644
--- a/drivers/staging/r8188eu/include/hal_intf.h
+++ b/drivers/staging/r8188eu/include/hal_intf.h
@@ -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,
-- 
2.35.1

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

[Greg KH]

On Mon, Mar 28, 2022 at 01:39:36PM +0200, Michael Straube wrote:
> 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 <straube.linux@gmail.com>
> ---
>  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(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index 6f0bff186477..14aff6a2040a 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -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);

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

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

[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 <straube.linux@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 14aff6a2040a..83841cf85a0d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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;
-- 
2.35.1

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

[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 <straube.linux@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 83841cf85a0d..1569f719af1b 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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 */
-- 
2.35.1

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

[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 <straube.linux@gmail.com>
---
 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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 1569f719af1b..06c17a16dab9 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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);
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b2adb05856a0..b5c2e7d4cb48 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -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);
 	}
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 219f0e5d36aa..4be5c5ed8711 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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;
diff --git a/drivers/staging/r8188eu/include/hal_intf.h b/drivers/staging/r8188eu/include/hal_intf.h
index 9d755eb59224..98f00c46fe8e 100644
--- a/drivers/staging/r8188eu/include/hal_intf.h
+++ b/drivers/staging/r8188eu/include/hal_intf.h
@@ -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,
 };
-- 
2.35.1

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

[Greg KH]

On Mon, Mar 28, 2022 at 01:39:39PM +0200, Michael Straube wrote:
> 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 <straube.linux@gmail.com>
> ---
>  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(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index 1569f719af1b..06c17a16dab9 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -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

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

[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 <straube.linux@gmail.com>
---
 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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b5c2e7d4cb48..313e1a3beecb 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -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++;
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
index f1464e4ba429..f58284eba70a 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
@@ -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. */
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 4be5c5ed8711..8bff4c6c29cf 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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;
 	}
-- 
2.35.1

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

[Greg KH]

On Mon, Mar 28, 2022 at 01:39:40PM +0200, Michael Straube wrote:
> 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 <straube.linux@gmail.com>
> ---
>  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(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index b5c2e7d4cb48..313e1a3beecb 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -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