linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] staging: r8188eu: remove unused dm_priv components
@ 2021-10-20 19:53 Martin Kaiser
  2021-10-20 19:53 ` [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant Martin Kaiser
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Martin Kaiser @ 2021-10-20 19:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Remove unused components from struct dm_priv.

DMFlag is only written to, but never read.
InitDMFlag is assigned to DMFlag and not used elsewhere.
DM_Type is also write-only.
UndecoratedSmoothedPWDB and UndecoratedSmoothedCCK are not used at all.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/rtl8188e_dm.c     | 3 ---
 drivers/staging/r8188eu/hal/usb_halinit.c     | 1 -
 drivers/staging/r8188eu/include/rtl8188e_dm.h | 5 -----
 3 files changed, 9 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/rtl8188e_dm.c b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
index 4ce2c3749665..5d76f6ea91c4 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
@@ -87,12 +87,9 @@ static void Update_ODM_ComInfo_88E(struct adapter *Adapter)
 void rtl8188e_InitHalDm(struct adapter *Adapter)
 {
 	struct hal_data_8188e *hal_data = GET_HAL_DATA(Adapter);
-	struct dm_priv	*pdmpriv = &hal_data->dmpriv;
 	struct odm_dm_struct *dm_odm = &hal_data->odmpriv;
 
 	dm_InitGPIOSetting(Adapter);
-	pdmpriv->DM_Type = DM_Type_ByDriver;
-	pdmpriv->DMFlag = DYNAMIC_FUNC_DISABLE;
 	Update_ODM_ComInfo_88E(Adapter);
 	ODM_DMInit(dm_odm);
 	Adapter->fix_rate = 0xFF;
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index cdc602fa9af8..ef1ae95d7db0 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -1469,7 +1469,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
 		break;
 	case HW_VAR_DM_FUNC_SET:
 		if (*((u32 *)val) == DYNAMIC_ALL_FUNC_ENABLE) {
-			pdmpriv->DMFlag = pdmpriv->InitDMFlag;
 			podmpriv->SupportAbility =	pdmpriv->InitODMFlag;
 		} else {
 			podmpriv->SupportAbility |= *((u32 *)val);
diff --git a/drivers/staging/r8188eu/include/rtl8188e_dm.h b/drivers/staging/r8188eu/include/rtl8188e_dm.h
index 4a0608313f7a..208bea050f6f 100644
--- a/drivers/staging/r8188eu/include/rtl8188e_dm.h
+++ b/drivers/staging/r8188eu/include/rtl8188e_dm.h
@@ -15,14 +15,9 @@ enum{
 #define HP_THERMAL_NUM		8
 /*  duplicate code,will move to ODM ######### */
 struct	dm_priv {
-	u8	DM_Type;
-	u8	DMFlag;
-	u8	InitDMFlag;
 	u32	InitODMFlag;
 
 	/*  Upper and Lower Signal threshold for Rate Adaptive*/
-	int	UndecoratedSmoothedPWDB;
-	int	UndecoratedSmoothedCCK;
 	int	EntryMinUndecoratedSmoothedPWDB;
 	int	EntryMaxUndecoratedSmoothedPWDB;
 	int	MinUndecoratedPWDBForDM;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant
  2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
@ 2021-10-20 19:53 ` Martin Kaiser
  2021-10-20 21:06   ` Phillip Potter
  2021-10-21 10:18   ` Michael Straube
  2021-10-20 19:53 ` [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address Martin Kaiser
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Martin Kaiser @ 2021-10-20 19:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Type in struct odm_rate_adapt is always DM_Type_ByDriver.
Therefore, bUseRAMask is always true.

Remove the constant components, unused defines and dead code.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/odm.c     | 9 ---------
 drivers/staging/r8188eu/include/odm.h | 6 ------
 2 files changed, 15 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
index 47d8cdf1c3e8..67cf8f7baba5 100644
--- a/drivers/staging/r8188eu/hal/odm.c
+++ b/drivers/staging/r8188eu/hal/odm.c
@@ -669,12 +669,6 @@ void odm_RateAdaptiveMaskInit(struct odm_dm_struct *pDM_Odm)
 {
 	struct odm_rate_adapt *pOdmRA = &pDM_Odm->RateAdaptive;
 
-	pOdmRA->Type = DM_Type_ByDriver;
-	if (pOdmRA->Type == DM_Type_ByDriver)
-		pDM_Odm->bUseRAMask = true;
-	else
-		pDM_Odm->bUseRAMask = false;
-
 	pOdmRA->RATRState = DM_RATR_STA_INIT;
 	pOdmRA->HighRSSIThresh = 50;
 	pOdmRA->LowRSSIThresh = 20;
@@ -755,9 +749,6 @@ void odm_RefreshRateAdaptiveMask(struct odm_dm_struct *pDM_Odm)
 	if (pAdapter->bDriverStopped)
 		return;
 
-	if (!pDM_Odm->bUseRAMask)
-		return;
-
 	for (i = 0; i < ODM_ASSOCIATE_ENTRY_NUM; i++) {
 		struct sta_info *pstat = pDM_Odm->pODM_StaInfo[i];
 		if (IS_STA_VALID(pstat)) {
diff --git a/drivers/staging/r8188eu/include/odm.h b/drivers/staging/r8188eu/include/odm.h
index 1fc90a8dc063..f08655208b32 100644
--- a/drivers/staging/r8188eu/include/odm.h
+++ b/drivers/staging/r8188eu/include/odm.h
@@ -150,7 +150,6 @@ struct edca_turbo {
 };
 
 struct odm_rate_adapt {
-	u8	Type;		/*  DM_Type_ByFW/DM_Type_ByDriver */
 	u8	HighRSSIThresh;	/*  if RSSI > HighRSSIThresh	=> RATRState is DM_RATR_STA_HIGH */
 	u8	LowRSSIThresh;	/*  if RSSI <= LowRSSIThresh	=> RATRState is DM_RATR_STA_LOW */
 	u8	RATRState;	/*  Current RSSI level, DM_RATR_STA_HIGH/DM_RATR_STA_MIDDLE/DM_RATR_STA_LOW */
@@ -165,8 +164,6 @@ struct odm_rate_adapt {
 #define AVG_THERMAL_NUM		8
 #define IQK_Matrix_REG_NUM	8
 
-#define	DM_Type_ByDriver	1
-
 struct odm_phy_dbg_info {
 	/* ODM Write,debug info */
 	s8	RxSNRdB[MAX_PATH_NUM_92CS];
@@ -586,9 +583,6 @@ struct odm_dm_struct {
 	bool	bPSDinProcess;
 	bool	bDMInitialGainEnable;
 
-	/* for rate adaptive, in fact,  88c/92c fw will handle this */
-	u8	bUseRAMask;
-
 	struct odm_rate_adapt RateAdaptive;
 
 	struct odm_rf_cal RFCalibrateInfo;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address
  2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
  2021-10-20 19:53 ` [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant Martin Kaiser
@ 2021-10-20 19:53 ` Martin Kaiser
  2021-10-20 21:07   ` Phillip Potter
  2021-10-20 19:54 ` [PATCH 4/5] staging: r8188eu: use helper to set " Martin Kaiser
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Martin Kaiser @ 2021-10-20 19:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Use the is_broadcast_ether_addr function to check for a
broadcast address.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 ++----
 drivers/staging/r8188eu/hal/odm.c           | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 995a0248c26f..b0dfafe526f7 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -392,13 +392,12 @@ void free_mlme_ext_priv(struct mlme_ext_priv *pmlmeext)
 
 static void _mgt_dispatcher(struct adapter *padapter, struct mlme_handler *ptable, struct recv_frame *precv_frame)
 {
-	u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 	u8 *pframe = precv_frame->rx_data;
 
 	if (ptable->func) {
 	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
 		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
-		    memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
+		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
 			return;
 		ptable->func(padapter, precv_frame);
 	}
@@ -409,7 +408,6 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 	int index;
 	struct mlme_handler *ptable;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
-	u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 	u8 *pframe = precv_frame->rx_data;
 	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, GetAddr2Ptr(pframe));
 
@@ -418,7 +416,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 
 	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
 	if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
-	    memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
+	    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
 		return;
 
 	ptable = mlme_sta_tbl;
diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
index 67cf8f7baba5..21f115194df8 100644
--- a/drivers/staging/r8188eu/hal/odm.c
+++ b/drivers/staging/r8188eu/hal/odm.c
@@ -829,7 +829,6 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
 	u8	sta_cnt = 0;
 	u32 PWDB_rssi[NUM_STA] = {0};/* 0~15]:MACID, [16~31]:PWDB_rssi */
 	struct sta_info *psta;
-	u8 bcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 
 	if (!(pDM_Odm->SupportAbility & ODM_BB_RSSI_MONITOR))
 		return;
@@ -841,7 +840,7 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
 		psta = pDM_Odm->pODM_StaInfo[i];
 		if (IS_STA_VALID(psta) &&
 		    (psta->state & WIFI_ASOC_STATE) &&
-		    memcmp(psta->hwaddr, bcast_addr, ETH_ALEN) &&
+		    !is_broadcast_ether_addr(psta->hwaddr) &&
 		    memcmp(psta->hwaddr, myid(&Adapter->eeprompriv), ETH_ALEN)) {
 			if (psta->rssi_stat.UndecoratedSmoothedPWDB < tmpEntryMinPWDB)
 				tmpEntryMinPWDB = psta->rssi_stat.UndecoratedSmoothedPWDB;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/5] staging: r8188eu: use helper to set broadcast address
  2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
  2021-10-20 19:53 ` [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant Martin Kaiser
  2021-10-20 19:53 ` [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address Martin Kaiser
@ 2021-10-20 19:54 ` Martin Kaiser
  2021-10-20 21:08   ` Phillip Potter
  2021-10-21 10:20   ` Michael Straube
  2021-10-20 19:54 ` [PATCH 5/5] staging: r8188eu: remove unused defines and enums Martin Kaiser
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Martin Kaiser @ 2021-10-20 19:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

The eth_broadcast_addr helper assigns the broadcast address to an address
array. Call this function instead of copying the address bytes manually.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 13 +++++--------
 drivers/staging/r8188eu/hal/rtl8188e_cmd.c  |  3 +--
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b0dfafe526f7..7b372374e638 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3409,7 +3409,6 @@ static int _issue_probereq_p2p(struct adapter *padapter, u8 *da, int wait_ack)
 	unsigned char			*mac;
 	struct xmit_priv		*pxmitpriv = &padapter->xmitpriv;
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
-	u8 bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 	struct wifidirect_info	*pwdinfo = &padapter->wdinfo;
 	u8 wpsie[255] = { 0x00 }, p2pie[255] = { 0x00 };
 	u16 wpsielen = 0, p2pielen = 0;
@@ -3443,8 +3442,8 @@ static int _issue_probereq_p2p(struct adapter *padapter, u8 *da, int wait_ack)
 			memcpy(pwlanhdr->addr3, pwdinfo->p2p_peer_interface_addr, ETH_ALEN);
 		} else {
 			/*	broadcast probe request frame */
-			memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
-			memcpy(pwlanhdr->addr3, bc_addr, ETH_ALEN);
+			eth_broadcast_addr(pwlanhdr->addr1);
+			eth_broadcast_addr(pwlanhdr->addr3);
 		}
 	}
 	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
@@ -4311,7 +4310,6 @@ void issue_beacon(struct adapter *padapter, int timeout_ms)
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 	struct wlan_bssid_ex		*cur_network = &pmlmeinfo->network;
-	u8	bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 	struct wifidirect_info	*pwdinfo = &padapter->wdinfo;
 
 	pmgntframe = alloc_mgtxmitframe(pxmitpriv);
@@ -4334,7 +4332,7 @@ void issue_beacon(struct adapter *padapter, int timeout_ms)
 	fctrl = &pwlanhdr->frame_ctl;
 	*(fctrl) = 0;
 
-	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
+	eth_broadcast_addr(pwlanhdr->addr1);
 	memcpy(pwlanhdr->addr2, myid(&padapter->eeprompriv), ETH_ALEN);
 	memcpy(pwlanhdr->addr3, get_my_bssid(cur_network), ETH_ALEN);
 
@@ -4676,7 +4674,6 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
 	int	bssrate_len = 0;
-	u8	bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 
 	pmgntframe = alloc_mgtxmitframe(pxmitpriv);
 	if (!pmgntframe)
@@ -4702,8 +4699,8 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps
 		memcpy(pwlanhdr->addr3, da, ETH_ALEN);
 	} else {
 		/*	broadcast probe request frame */
-		memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
-		memcpy(pwlanhdr->addr3, bc_addr, ETH_ALEN);
+		eth_broadcast_addr(pwlanhdr->addr1);
+		eth_broadcast_addr(pwlanhdr->addr3);
 	}
 
 	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
index c5f9353fe3e6..14eed14a4c6a 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
@@ -226,14 +226,13 @@ static void ConstructBeacon(struct adapter *adapt, u8 *pframe, u32 *pLength)
 	struct mlme_ext_priv *pmlmeext = &adapt->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 	struct wlan_bssid_ex		*cur_network = &pmlmeinfo->network;
-	u8 bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 
 	pwlanhdr = (struct rtw_ieee80211_hdr *)pframe;
 
 	fctrl = &pwlanhdr->frame_ctl;
 	*(fctrl) = 0;
 
-	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
+	eth_broadcast_addr(pwlanhdr->addr1);
 	memcpy(pwlanhdr->addr2, myid(&adapt->eeprompriv), ETH_ALEN);
 	memcpy(pwlanhdr->addr3, get_my_bssid(cur_network), ETH_ALEN);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/5] staging: r8188eu: remove unused defines and enums
  2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
                   ` (2 preceding siblings ...)
  2021-10-20 19:54 ` [PATCH 4/5] staging: r8188eu: use helper to set " Martin Kaiser
@ 2021-10-20 19:54 ` Martin Kaiser
  2021-10-20 21:10   ` Phillip Potter
  2021-10-21 10:21   ` Michael Straube
  2021-10-20 21:05 ` [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Phillip Potter
  2021-10-21 10:17 ` Michael Straube
  5 siblings, 2 replies; 17+ messages in thread
From: Martin Kaiser @ 2021-10-20 19:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Remove a couple of unused defines and an unused enum
from rtl8188e_cmd.h.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/rtl8188e_cmd.h | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/staging/r8188eu/include/rtl8188e_cmd.h b/drivers/staging/r8188eu/include/rtl8188e_cmd.h
index 01404b774ebd..1e01c1662f9a 100644
--- a/drivers/staging/r8188eu/include/rtl8188e_cmd.h
+++ b/drivers/staging/r8188eu/include/rtl8188e_cmd.h
@@ -27,15 +27,6 @@ enum RTL8188E_H2C_CMD_ID {
 	/* Class DM */
 	H2C_DM_MACID_CFG		= 0x40,
 	H2C_DM_TXBF			= 0x41,
-
-	/* Class BT */
-	H2C_BT_COEX_MASK		= 0x60,
-	H2C_BT_COEX_GPIO_MODE		= 0x61,
-	H2C_BT_DAC_SWING_VAL		= 0x62,
-	H2C_BT_PSD_RST			= 0x63,
-
-	/* Class */
-	 H2C_RESET_TSF			= 0xc0,
 };
 
 struct cmd_msg_parm {
@@ -44,10 +35,6 @@ struct cmd_msg_parm {
 	u8 buf[6];
 };
 
-enum {
-	PWRS
-};
-
 struct setpwrmode_parm {
 	u8 Mode;/* 0:Active,1:LPS,2:WMMPS */
 	u8 SmartPS_RLBM;/* LPS= 0:PS_Poll,1:PS_Poll,2:NullData,WMM= 0:PS_Poll,1:NullData */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/5] staging: r8188eu: remove unused dm_priv components
  2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
                   ` (3 preceding siblings ...)
  2021-10-20 19:54 ` [PATCH 5/5] staging: r8188eu: remove unused defines and enums Martin Kaiser
@ 2021-10-20 21:05 ` Phillip Potter
  2021-10-21 10:17 ` Michael Straube
  5 siblings, 0 replies; 17+ messages in thread
From: Phillip Potter @ 2021-10-20 21:05 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Michael Straube, linux-staging,
	linux-kernel

On Wed, Oct 20, 2021 at 09:53:57PM +0200, Martin Kaiser wrote:
> Remove unused components from struct dm_priv.
> 
> DMFlag is only written to, but never read.
> InitDMFlag is assigned to DMFlag and not used elsewhere.
> DM_Type is also write-only.
> UndecoratedSmoothedPWDB and UndecoratedSmoothedCCK are not used at all.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/rtl8188e_dm.c     | 3 ---
>  drivers/staging/r8188eu/hal/usb_halinit.c     | 1 -
>  drivers/staging/r8188eu/include/rtl8188e_dm.h | 5 -----
>  3 files changed, 9 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_dm.c b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
> index 4ce2c3749665..5d76f6ea91c4 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_dm.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
> @@ -87,12 +87,9 @@ static void Update_ODM_ComInfo_88E(struct adapter *Adapter)
>  void rtl8188e_InitHalDm(struct adapter *Adapter)
>  {
>  	struct hal_data_8188e *hal_data = GET_HAL_DATA(Adapter);
> -	struct dm_priv	*pdmpriv = &hal_data->dmpriv;
>  	struct odm_dm_struct *dm_odm = &hal_data->odmpriv;
>  
>  	dm_InitGPIOSetting(Adapter);
> -	pdmpriv->DM_Type = DM_Type_ByDriver;
> -	pdmpriv->DMFlag = DYNAMIC_FUNC_DISABLE;
>  	Update_ODM_ComInfo_88E(Adapter);
>  	ODM_DMInit(dm_odm);
>  	Adapter->fix_rate = 0xFF;
> diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
> index cdc602fa9af8..ef1ae95d7db0 100644
> --- a/drivers/staging/r8188eu/hal/usb_halinit.c
> +++ b/drivers/staging/r8188eu/hal/usb_halinit.c
> @@ -1469,7 +1469,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
>  		break;
>  	case HW_VAR_DM_FUNC_SET:
>  		if (*((u32 *)val) == DYNAMIC_ALL_FUNC_ENABLE) {
> -			pdmpriv->DMFlag = pdmpriv->InitDMFlag;
>  			podmpriv->SupportAbility =	pdmpriv->InitODMFlag;
>  		} else {
>  			podmpriv->SupportAbility |= *((u32 *)val);
> diff --git a/drivers/staging/r8188eu/include/rtl8188e_dm.h b/drivers/staging/r8188eu/include/rtl8188e_dm.h
> index 4a0608313f7a..208bea050f6f 100644
> --- a/drivers/staging/r8188eu/include/rtl8188e_dm.h
> +++ b/drivers/staging/r8188eu/include/rtl8188e_dm.h
> @@ -15,14 +15,9 @@ enum{
>  #define HP_THERMAL_NUM		8
>  /*  duplicate code,will move to ODM ######### */
>  struct	dm_priv {
> -	u8	DM_Type;
> -	u8	DMFlag;
> -	u8	InitDMFlag;
>  	u32	InitODMFlag;
>  
>  	/*  Upper and Lower Signal threshold for Rate Adaptive*/
> -	int	UndecoratedSmoothedPWDB;
> -	int	UndecoratedSmoothedCCK;
>  	int	EntryMinUndecoratedSmoothedPWDB;
>  	int	EntryMaxUndecoratedSmoothedPWDB;
>  	int	MinUndecoratedPWDBForDM;
> -- 
> 2.20.1
> 

Looks good.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant
  2021-10-20 19:53 ` [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant Martin Kaiser
@ 2021-10-20 21:06   ` Phillip Potter
  2021-10-21 10:18   ` Michael Straube
  1 sibling, 0 replies; 17+ messages in thread
From: Phillip Potter @ 2021-10-20 21:06 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Michael Straube, linux-staging,
	linux-kernel

On Wed, Oct 20, 2021 at 09:53:58PM +0200, Martin Kaiser wrote:
> Type in struct odm_rate_adapt is always DM_Type_ByDriver.
> Therefore, bUseRAMask is always true.
> 
> Remove the constant components, unused defines and dead code.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/odm.c     | 9 ---------
>  drivers/staging/r8188eu/include/odm.h | 6 ------
>  2 files changed, 15 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
> index 47d8cdf1c3e8..67cf8f7baba5 100644
> --- a/drivers/staging/r8188eu/hal/odm.c
> +++ b/drivers/staging/r8188eu/hal/odm.c
> @@ -669,12 +669,6 @@ void odm_RateAdaptiveMaskInit(struct odm_dm_struct *pDM_Odm)
>  {
>  	struct odm_rate_adapt *pOdmRA = &pDM_Odm->RateAdaptive;
>  
> -	pOdmRA->Type = DM_Type_ByDriver;
> -	if (pOdmRA->Type == DM_Type_ByDriver)
> -		pDM_Odm->bUseRAMask = true;
> -	else
> -		pDM_Odm->bUseRAMask = false;
> -
>  	pOdmRA->RATRState = DM_RATR_STA_INIT;
>  	pOdmRA->HighRSSIThresh = 50;
>  	pOdmRA->LowRSSIThresh = 20;
> @@ -755,9 +749,6 @@ void odm_RefreshRateAdaptiveMask(struct odm_dm_struct *pDM_Odm)
>  	if (pAdapter->bDriverStopped)
>  		return;
>  
> -	if (!pDM_Odm->bUseRAMask)
> -		return;
> -
>  	for (i = 0; i < ODM_ASSOCIATE_ENTRY_NUM; i++) {
>  		struct sta_info *pstat = pDM_Odm->pODM_StaInfo[i];
>  		if (IS_STA_VALID(pstat)) {
> diff --git a/drivers/staging/r8188eu/include/odm.h b/drivers/staging/r8188eu/include/odm.h
> index 1fc90a8dc063..f08655208b32 100644
> --- a/drivers/staging/r8188eu/include/odm.h
> +++ b/drivers/staging/r8188eu/include/odm.h
> @@ -150,7 +150,6 @@ struct edca_turbo {
>  };
>  
>  struct odm_rate_adapt {
> -	u8	Type;		/*  DM_Type_ByFW/DM_Type_ByDriver */
>  	u8	HighRSSIThresh;	/*  if RSSI > HighRSSIThresh	=> RATRState is DM_RATR_STA_HIGH */
>  	u8	LowRSSIThresh;	/*  if RSSI <= LowRSSIThresh	=> RATRState is DM_RATR_STA_LOW */
>  	u8	RATRState;	/*  Current RSSI level, DM_RATR_STA_HIGH/DM_RATR_STA_MIDDLE/DM_RATR_STA_LOW */
> @@ -165,8 +164,6 @@ struct odm_rate_adapt {
>  #define AVG_THERMAL_NUM		8
>  #define IQK_Matrix_REG_NUM	8
>  
> -#define	DM_Type_ByDriver	1
> -
>  struct odm_phy_dbg_info {
>  	/* ODM Write,debug info */
>  	s8	RxSNRdB[MAX_PATH_NUM_92CS];
> @@ -586,9 +583,6 @@ struct odm_dm_struct {
>  	bool	bPSDinProcess;
>  	bool	bDMInitialGainEnable;
>  
> -	/* for rate adaptive, in fact,  88c/92c fw will handle this */
> -	u8	bUseRAMask;
> -
>  	struct odm_rate_adapt RateAdaptive;
>  
>  	struct odm_rf_cal RFCalibrateInfo;
> -- 
> 2.20.1
> 

Looks good.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address
  2021-10-20 19:53 ` [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address Martin Kaiser
@ 2021-10-20 21:07   ` Phillip Potter
  2021-10-21 10:12     ` Michael Straube
  0 siblings, 1 reply; 17+ messages in thread
From: Phillip Potter @ 2021-10-20 21:07 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Michael Straube, linux-staging,
	linux-kernel

On Wed, Oct 20, 2021 at 09:53:59PM +0200, Martin Kaiser wrote:
> Use the is_broadcast_ether_addr function to check for a
> broadcast address.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 ++----
>  drivers/staging/r8188eu/hal/odm.c           | 3 +--
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index 995a0248c26f..b0dfafe526f7 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -392,13 +392,12 @@ void free_mlme_ext_priv(struct mlme_ext_priv *pmlmeext)
>  
>  static void _mgt_dispatcher(struct adapter *padapter, struct mlme_handler *ptable, struct recv_frame *precv_frame)
>  {
> -	u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  	u8 *pframe = precv_frame->rx_data;
>  
>  	if (ptable->func) {
>  	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
>  		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
> -		    memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
> +		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
>  			return;
>  		ptable->func(padapter, precv_frame);
>  	}
> @@ -409,7 +408,6 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
>  	int index;
>  	struct mlme_handler *ptable;
>  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> -	u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  	u8 *pframe = precv_frame->rx_data;
>  	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, GetAddr2Ptr(pframe));
>  
> @@ -418,7 +416,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
>  
>  	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
>  	if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
> -	    memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
> +	    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
>  		return;
>  
>  	ptable = mlme_sta_tbl;
> diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
> index 67cf8f7baba5..21f115194df8 100644
> --- a/drivers/staging/r8188eu/hal/odm.c
> +++ b/drivers/staging/r8188eu/hal/odm.c
> @@ -829,7 +829,6 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
>  	u8	sta_cnt = 0;
>  	u32 PWDB_rssi[NUM_STA] = {0};/* 0~15]:MACID, [16~31]:PWDB_rssi */
>  	struct sta_info *psta;
> -	u8 bcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  
>  	if (!(pDM_Odm->SupportAbility & ODM_BB_RSSI_MONITOR))
>  		return;
> @@ -841,7 +840,7 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
>  		psta = pDM_Odm->pODM_StaInfo[i];
>  		if (IS_STA_VALID(psta) &&
>  		    (psta->state & WIFI_ASOC_STATE) &&
> -		    memcmp(psta->hwaddr, bcast_addr, ETH_ALEN) &&
> +		    !is_broadcast_ether_addr(psta->hwaddr) &&
>  		    memcmp(psta->hwaddr, myid(&Adapter->eeprompriv), ETH_ALEN)) {
>  			if (psta->rssi_stat.UndecoratedSmoothedPWDB < tmpEntryMinPWDB)
>  				tmpEntryMinPWDB = psta->rssi_stat.UndecoratedSmoothedPWDB;
> -- 
> 2.20.1
>

Looks good.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/5] staging: r8188eu: use helper to set broadcast address
  2021-10-20 19:54 ` [PATCH 4/5] staging: r8188eu: use helper to set " Martin Kaiser
@ 2021-10-20 21:08   ` Phillip Potter
  2021-10-21 10:20   ` Michael Straube
  1 sibling, 0 replies; 17+ messages in thread
From: Phillip Potter @ 2021-10-20 21:08 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Michael Straube, linux-staging,
	linux-kernel

On Wed, Oct 20, 2021 at 09:54:00PM +0200, Martin Kaiser wrote:
> The eth_broadcast_addr helper assigns the broadcast address to an address
> array. Call this function instead of copying the address bytes manually.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/core/rtw_mlme_ext.c | 13 +++++--------
>  drivers/staging/r8188eu/hal/rtl8188e_cmd.c  |  3 +--
>  2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index b0dfafe526f7..7b372374e638 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -3409,7 +3409,6 @@ static int _issue_probereq_p2p(struct adapter *padapter, u8 *da, int wait_ack)
>  	unsigned char			*mac;
>  	struct xmit_priv		*pxmitpriv = &padapter->xmitpriv;
>  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
> -	u8 bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  	struct wifidirect_info	*pwdinfo = &padapter->wdinfo;
>  	u8 wpsie[255] = { 0x00 }, p2pie[255] = { 0x00 };
>  	u16 wpsielen = 0, p2pielen = 0;
> @@ -3443,8 +3442,8 @@ static int _issue_probereq_p2p(struct adapter *padapter, u8 *da, int wait_ack)
>  			memcpy(pwlanhdr->addr3, pwdinfo->p2p_peer_interface_addr, ETH_ALEN);
>  		} else {
>  			/*	broadcast probe request frame */
> -			memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> -			memcpy(pwlanhdr->addr3, bc_addr, ETH_ALEN);
> +			eth_broadcast_addr(pwlanhdr->addr1);
> +			eth_broadcast_addr(pwlanhdr->addr3);
>  		}
>  	}
>  	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
> @@ -4311,7 +4310,6 @@ void issue_beacon(struct adapter *padapter, int timeout_ms)
>  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>  	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>  	struct wlan_bssid_ex		*cur_network = &pmlmeinfo->network;
> -	u8	bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  	struct wifidirect_info	*pwdinfo = &padapter->wdinfo;
>  
>  	pmgntframe = alloc_mgtxmitframe(pxmitpriv);
> @@ -4334,7 +4332,7 @@ void issue_beacon(struct adapter *padapter, int timeout_ms)
>  	fctrl = &pwlanhdr->frame_ctl;
>  	*(fctrl) = 0;
>  
> -	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> +	eth_broadcast_addr(pwlanhdr->addr1);
>  	memcpy(pwlanhdr->addr2, myid(&padapter->eeprompriv), ETH_ALEN);
>  	memcpy(pwlanhdr->addr3, get_my_bssid(cur_network), ETH_ALEN);
>  
> @@ -4676,7 +4674,6 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps
>  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>  	int	bssrate_len = 0;
> -	u8	bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  
>  	pmgntframe = alloc_mgtxmitframe(pxmitpriv);
>  	if (!pmgntframe)
> @@ -4702,8 +4699,8 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps
>  		memcpy(pwlanhdr->addr3, da, ETH_ALEN);
>  	} else {
>  		/*	broadcast probe request frame */
> -		memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> -		memcpy(pwlanhdr->addr3, bc_addr, ETH_ALEN);
> +		eth_broadcast_addr(pwlanhdr->addr1);
> +		eth_broadcast_addr(pwlanhdr->addr3);
>  	}
>  
>  	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
> index c5f9353fe3e6..14eed14a4c6a 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
> @@ -226,14 +226,13 @@ static void ConstructBeacon(struct adapter *adapt, u8 *pframe, u32 *pLength)
>  	struct mlme_ext_priv *pmlmeext = &adapt->mlmeextpriv;
>  	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>  	struct wlan_bssid_ex		*cur_network = &pmlmeinfo->network;
> -	u8 bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>  
>  	pwlanhdr = (struct rtw_ieee80211_hdr *)pframe;
>  
>  	fctrl = &pwlanhdr->frame_ctl;
>  	*(fctrl) = 0;
>  
> -	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> +	eth_broadcast_addr(pwlanhdr->addr1);
>  	memcpy(pwlanhdr->addr2, myid(&adapt->eeprompriv), ETH_ALEN);
>  	memcpy(pwlanhdr->addr3, get_my_bssid(cur_network), ETH_ALEN);
>  
> -- 
> 2.20.1
> 

Looks good.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] staging: r8188eu: remove unused defines and enums
  2021-10-20 19:54 ` [PATCH 5/5] staging: r8188eu: remove unused defines and enums Martin Kaiser
@ 2021-10-20 21:10   ` Phillip Potter
  2021-10-21 10:21   ` Michael Straube
  1 sibling, 0 replies; 17+ messages in thread
From: Phillip Potter @ 2021-10-20 21:10 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Michael Straube, linux-staging,
	linux-kernel

On Wed, Oct 20, 2021 at 09:54:01PM +0200, Martin Kaiser wrote:
> Remove a couple of unused defines and an unused enum
> from rtl8188e_cmd.h.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/include/rtl8188e_cmd.h | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/rtl8188e_cmd.h b/drivers/staging/r8188eu/include/rtl8188e_cmd.h
> index 01404b774ebd..1e01c1662f9a 100644
> --- a/drivers/staging/r8188eu/include/rtl8188e_cmd.h
> +++ b/drivers/staging/r8188eu/include/rtl8188e_cmd.h
> @@ -27,15 +27,6 @@ enum RTL8188E_H2C_CMD_ID {
>  	/* Class DM */
>  	H2C_DM_MACID_CFG		= 0x40,
>  	H2C_DM_TXBF			= 0x41,
> -
> -	/* Class BT */
> -	H2C_BT_COEX_MASK		= 0x60,
> -	H2C_BT_COEX_GPIO_MODE		= 0x61,
> -	H2C_BT_DAC_SWING_VAL		= 0x62,
> -	H2C_BT_PSD_RST			= 0x63,
> -
> -	/* Class */
> -	 H2C_RESET_TSF			= 0xc0,
>  };
>  
>  struct cmd_msg_parm {
> @@ -44,10 +35,6 @@ struct cmd_msg_parm {
>  	u8 buf[6];
>  };
>  
> -enum {
> -	PWRS
> -};
> -
>  struct setpwrmode_parm {
>  	u8 Mode;/* 0:Active,1:LPS,2:WMMPS */
>  	u8 SmartPS_RLBM;/* LPS= 0:PS_Poll,1:PS_Poll,2:NullData,WMM= 0:PS_Poll,1:NullData */
> -- 
> 2.20.1
> 

Looks good. Built and tested whole series btw :-)

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address
  2021-10-20 21:07   ` Phillip Potter
@ 2021-10-21 10:12     ` Michael Straube
  2021-10-22  9:21       ` Martin Kaiser
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Straube @ 2021-10-21 10:12 UTC (permalink / raw)
  To: Phillip Potter, Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, linux-staging, linux-kernel

On 10/20/21 23:07, Phillip Potter wrote:
> On Wed, Oct 20, 2021 at 09:53:59PM +0200, Martin Kaiser wrote:
>> Use the is_broadcast_ether_addr function to check for a
>> broadcast address.
>>
>> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
>> ---
>>   drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 ++----
>>   drivers/staging/r8188eu/hal/odm.c           | 3 +--
>>   2 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
>> index 995a0248c26f..b0dfafe526f7 100644
>> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
>> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
>> @@ -392,13 +392,12 @@ void free_mlme_ext_priv(struct mlme_ext_priv *pmlmeext)
>>   
>>   static void _mgt_dispatcher(struct adapter *padapter, struct mlme_handler *ptable, struct recv_frame *precv_frame)
>>   {
>> -	u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>>   	u8 *pframe = precv_frame->rx_data;
>>   
>>   	if (ptable->func) {
>>   	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
>>   		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
>> -		    memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
>> +		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))

Hi Martin,

I'm not an expert regarding alignment. Is GetAddr1Ptr(pframe) always 
__aligned(2) as required by is_broadcast_ether_addr?

>>   			return;
>>   		ptable->func(padapter, precv_frame);
>>   	}
>> @@ -409,7 +408,6 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
>>   	int index;
>>   	struct mlme_handler *ptable;
>>   	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>> -	u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>>   	u8 *pframe = precv_frame->rx_data;
>>   	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, GetAddr2Ptr(pframe));
>>   
>> @@ -418,7 +416,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
>>   
>>   	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
>>   	if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
>> -	    memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
>> +	    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
>>   		return;

Same here.

>>   
>>   	ptable = mlme_sta_tbl;
>> diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
>> index 67cf8f7baba5..21f115194df8 100644
>> --- a/drivers/staging/r8188eu/hal/odm.c
>> +++ b/drivers/staging/r8188eu/hal/odm.c
>> @@ -829,7 +829,6 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
>>   	u8	sta_cnt = 0;
>>   	u32 PWDB_rssi[NUM_STA] = {0};/* 0~15]:MACID, [16~31]:PWDB_rssi */
>>   	struct sta_info *psta;
>> -	u8 bcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>>   
>>   	if (!(pDM_Odm->SupportAbility & ODM_BB_RSSI_MONITOR))
>>   		return;
>> @@ -841,7 +840,7 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
>>   		psta = pDM_Odm->pODM_StaInfo[i];
>>   		if (IS_STA_VALID(psta) &&
>>   		    (psta->state & WIFI_ASOC_STATE) &&
>> -		    memcmp(psta->hwaddr, bcast_addr, ETH_ALEN) &&
>> +		    !is_broadcast_ether_addr(psta->hwaddr) &&

Perhaps we should add __aligned(2) to the hwaddr variable in struct
sta_info to be safe?

u8	hwaddr[ETH_ALEN] __aligned(2);


>>   		    memcmp(psta->hwaddr, myid(&Adapter->eeprompriv), ETH_ALEN)) {
>>   			if (psta->rssi_stat.UndecoratedSmoothedPWDB < tmpEntryMinPWDB)
>>   				tmpEntryMinPWDB = psta->rssi_stat.UndecoratedSmoothedPWDB;
>> -- 
>> 2.20.1
>>

Other than that the patch looks good, thanks.

Michael


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/5] staging: r8188eu: remove unused dm_priv components
  2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
                   ` (4 preceding siblings ...)
  2021-10-20 21:05 ` [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Phillip Potter
@ 2021-10-21 10:17 ` Michael Straube
  5 siblings, 0 replies; 17+ messages in thread
From: Michael Straube @ 2021-10-21 10:17 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

Am 20.10.21 um 21:53 schrieb Martin Kaiser:
> Remove unused components from struct dm_priv.
> 
> DMFlag is only written to, but never read.
> InitDMFlag is assigned to DMFlag and not used elsewhere.
> DM_Type is also write-only.
> UndecoratedSmoothedPWDB and UndecoratedSmoothedCCK are not used at all.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/rtl8188e_dm.c     | 3 ---
>   drivers/staging/r8188eu/hal/usb_halinit.c     | 1 -
>   drivers/staging/r8188eu/include/rtl8188e_dm.h | 5 -----
>   3 files changed, 9 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_dm.c b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
> index 4ce2c3749665..5d76f6ea91c4 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_dm.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
> @@ -87,12 +87,9 @@ static void Update_ODM_ComInfo_88E(struct adapter *Adapter)
>   void rtl8188e_InitHalDm(struct adapter *Adapter)
>   {
>   	struct hal_data_8188e *hal_data = GET_HAL_DATA(Adapter);
> -	struct dm_priv	*pdmpriv = &hal_data->dmpriv;
>   	struct odm_dm_struct *dm_odm = &hal_data->odmpriv;
>   
>   	dm_InitGPIOSetting(Adapter);
> -	pdmpriv->DM_Type = DM_Type_ByDriver;
> -	pdmpriv->DMFlag = DYNAMIC_FUNC_DISABLE;
>   	Update_ODM_ComInfo_88E(Adapter);
>   	ODM_DMInit(dm_odm);
>   	Adapter->fix_rate = 0xFF;
> diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
> index cdc602fa9af8..ef1ae95d7db0 100644
> --- a/drivers/staging/r8188eu/hal/usb_halinit.c
> +++ b/drivers/staging/r8188eu/hal/usb_halinit.c
> @@ -1469,7 +1469,6 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
>   		break;
>   	case HW_VAR_DM_FUNC_SET:
>   		if (*((u32 *)val) == DYNAMIC_ALL_FUNC_ENABLE) {
> -			pdmpriv->DMFlag = pdmpriv->InitDMFlag;
>   			podmpriv->SupportAbility =	pdmpriv->InitODMFlag;
>   		} else {
>   			podmpriv->SupportAbility |= *((u32 *)val);
> diff --git a/drivers/staging/r8188eu/include/rtl8188e_dm.h b/drivers/staging/r8188eu/include/rtl8188e_dm.h
> index 4a0608313f7a..208bea050f6f 100644
> --- a/drivers/staging/r8188eu/include/rtl8188e_dm.h
> +++ b/drivers/staging/r8188eu/include/rtl8188e_dm.h
> @@ -15,14 +15,9 @@ enum{
>   #define HP_THERMAL_NUM		8
>   /*  duplicate code,will move to ODM ######### */
>   struct	dm_priv {
> -	u8	DM_Type;
> -	u8	DMFlag;
> -	u8	InitDMFlag;
>   	u32	InitODMFlag;
>   
>   	/*  Upper and Lower Signal threshold for Rate Adaptive*/
> -	int	UndecoratedSmoothedPWDB;
> -	int	UndecoratedSmoothedCCK;
>   	int	EntryMinUndecoratedSmoothedPWDB;
>   	int	EntryMaxUndecoratedSmoothedPWDB;
>   	int	MinUndecoratedPWDBForDM;
> 

Looks good, thanks.

Acked-by: Michael Straube <straube.linux@gmail.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant
  2021-10-20 19:53 ` [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant Martin Kaiser
  2021-10-20 21:06   ` Phillip Potter
@ 2021-10-21 10:18   ` Michael Straube
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Straube @ 2021-10-21 10:18 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On 10/20/21 21:53, Martin Kaiser wrote:
> Type in struct odm_rate_adapt is always DM_Type_ByDriver.
> Therefore, bUseRAMask is always true.
> 
> Remove the constant components, unused defines and dead code.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/odm.c     | 9 ---------
>   drivers/staging/r8188eu/include/odm.h | 6 ------
>   2 files changed, 15 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
> index 47d8cdf1c3e8..67cf8f7baba5 100644
> --- a/drivers/staging/r8188eu/hal/odm.c
> +++ b/drivers/staging/r8188eu/hal/odm.c
> @@ -669,12 +669,6 @@ void odm_RateAdaptiveMaskInit(struct odm_dm_struct *pDM_Odm)
>   {
>   	struct odm_rate_adapt *pOdmRA = &pDM_Odm->RateAdaptive;
>   
> -	pOdmRA->Type = DM_Type_ByDriver;
> -	if (pOdmRA->Type == DM_Type_ByDriver)
> -		pDM_Odm->bUseRAMask = true;
> -	else
> -		pDM_Odm->bUseRAMask = false;
> -
>   	pOdmRA->RATRState = DM_RATR_STA_INIT;
>   	pOdmRA->HighRSSIThresh = 50;
>   	pOdmRA->LowRSSIThresh = 20;
> @@ -755,9 +749,6 @@ void odm_RefreshRateAdaptiveMask(struct odm_dm_struct *pDM_Odm)
>   	if (pAdapter->bDriverStopped)
>   		return;
>   
> -	if (!pDM_Odm->bUseRAMask)
> -		return;
> -
>   	for (i = 0; i < ODM_ASSOCIATE_ENTRY_NUM; i++) {
>   		struct sta_info *pstat = pDM_Odm->pODM_StaInfo[i];
>   		if (IS_STA_VALID(pstat)) {
> diff --git a/drivers/staging/r8188eu/include/odm.h b/drivers/staging/r8188eu/include/odm.h
> index 1fc90a8dc063..f08655208b32 100644
> --- a/drivers/staging/r8188eu/include/odm.h
> +++ b/drivers/staging/r8188eu/include/odm.h
> @@ -150,7 +150,6 @@ struct edca_turbo {
>   };
>   
>   struct odm_rate_adapt {
> -	u8	Type;		/*  DM_Type_ByFW/DM_Type_ByDriver */
>   	u8	HighRSSIThresh;	/*  if RSSI > HighRSSIThresh	=> RATRState is DM_RATR_STA_HIGH */
>   	u8	LowRSSIThresh;	/*  if RSSI <= LowRSSIThresh	=> RATRState is DM_RATR_STA_LOW */
>   	u8	RATRState;	/*  Current RSSI level, DM_RATR_STA_HIGH/DM_RATR_STA_MIDDLE/DM_RATR_STA_LOW */
> @@ -165,8 +164,6 @@ struct odm_rate_adapt {
>   #define AVG_THERMAL_NUM		8
>   #define IQK_Matrix_REG_NUM	8
>   
> -#define	DM_Type_ByDriver	1
> -
>   struct odm_phy_dbg_info {
>   	/* ODM Write,debug info */
>   	s8	RxSNRdB[MAX_PATH_NUM_92CS];
> @@ -586,9 +583,6 @@ struct odm_dm_struct {
>   	bool	bPSDinProcess;
>   	bool	bDMInitialGainEnable;
>   
> -	/* for rate adaptive, in fact,  88c/92c fw will handle this */
> -	u8	bUseRAMask;
> -
>   	struct odm_rate_adapt RateAdaptive;
>   
>   	struct odm_rf_cal RFCalibrateInfo;
> 


Looks good, thanks.

Acked-by: Michael Straube <straube.linux@gmail.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/5] staging: r8188eu: use helper to set broadcast address
  2021-10-20 19:54 ` [PATCH 4/5] staging: r8188eu: use helper to set " Martin Kaiser
  2021-10-20 21:08   ` Phillip Potter
@ 2021-10-21 10:20   ` Michael Straube
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Straube @ 2021-10-21 10:20 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On 10/20/21 21:54, Martin Kaiser wrote:
> The eth_broadcast_addr helper assigns the broadcast address to an address
> array. Call this function instead of copying the address bytes manually.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/core/rtw_mlme_ext.c | 13 +++++--------
>   drivers/staging/r8188eu/hal/rtl8188e_cmd.c  |  3 +--
>   2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index b0dfafe526f7..7b372374e638 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -3409,7 +3409,6 @@ static int _issue_probereq_p2p(struct adapter *padapter, u8 *da, int wait_ack)
>   	unsigned char			*mac;
>   	struct xmit_priv		*pxmitpriv = &padapter->xmitpriv;
>   	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
> -	u8 bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>   	struct wifidirect_info	*pwdinfo = &padapter->wdinfo;
>   	u8 wpsie[255] = { 0x00 }, p2pie[255] = { 0x00 };
>   	u16 wpsielen = 0, p2pielen = 0;
> @@ -3443,8 +3442,8 @@ static int _issue_probereq_p2p(struct adapter *padapter, u8 *da, int wait_ack)
>   			memcpy(pwlanhdr->addr3, pwdinfo->p2p_peer_interface_addr, ETH_ALEN);
>   		} else {
>   			/*	broadcast probe request frame */
> -			memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> -			memcpy(pwlanhdr->addr3, bc_addr, ETH_ALEN);
> +			eth_broadcast_addr(pwlanhdr->addr1);
> +			eth_broadcast_addr(pwlanhdr->addr3);
>   		}
>   	}
>   	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
> @@ -4311,7 +4310,6 @@ void issue_beacon(struct adapter *padapter, int timeout_ms)
>   	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>   	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>   	struct wlan_bssid_ex		*cur_network = &pmlmeinfo->network;
> -	u8	bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>   	struct wifidirect_info	*pwdinfo = &padapter->wdinfo;
>   
>   	pmgntframe = alloc_mgtxmitframe(pxmitpriv);
> @@ -4334,7 +4332,7 @@ void issue_beacon(struct adapter *padapter, int timeout_ms)
>   	fctrl = &pwlanhdr->frame_ctl;
>   	*(fctrl) = 0;
>   
> -	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> +	eth_broadcast_addr(pwlanhdr->addr1);
>   	memcpy(pwlanhdr->addr2, myid(&padapter->eeprompriv), ETH_ALEN);
>   	memcpy(pwlanhdr->addr3, get_my_bssid(cur_network), ETH_ALEN);
>   
> @@ -4676,7 +4674,6 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps
>   	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>   	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>   	int	bssrate_len = 0;
> -	u8	bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>   
>   	pmgntframe = alloc_mgtxmitframe(pxmitpriv);
>   	if (!pmgntframe)
> @@ -4702,8 +4699,8 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps
>   		memcpy(pwlanhdr->addr3, da, ETH_ALEN);
>   	} else {
>   		/*	broadcast probe request frame */
> -		memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> -		memcpy(pwlanhdr->addr3, bc_addr, ETH_ALEN);
> +		eth_broadcast_addr(pwlanhdr->addr1);
> +		eth_broadcast_addr(pwlanhdr->addr3);
>   	}
>   
>   	memcpy(pwlanhdr->addr2, mac, ETH_ALEN);
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
> index c5f9353fe3e6..14eed14a4c6a 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
> @@ -226,14 +226,13 @@ static void ConstructBeacon(struct adapter *adapt, u8 *pframe, u32 *pLength)
>   	struct mlme_ext_priv *pmlmeext = &adapt->mlmeextpriv;
>   	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>   	struct wlan_bssid_ex		*cur_network = &pmlmeinfo->network;
> -	u8 bc_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>   
>   	pwlanhdr = (struct rtw_ieee80211_hdr *)pframe;
>   
>   	fctrl = &pwlanhdr->frame_ctl;
>   	*(fctrl) = 0;
>   
> -	memcpy(pwlanhdr->addr1, bc_addr, ETH_ALEN);
> +	eth_broadcast_addr(pwlanhdr->addr1);
>   	memcpy(pwlanhdr->addr2, myid(&adapt->eeprompriv), ETH_ALEN);
>   	memcpy(pwlanhdr->addr3, get_my_bssid(cur_network), ETH_ALEN);
>   
> 


Looks good, thanks.

Acked-by: Michael Straube <straube.linux@gmail.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] staging: r8188eu: remove unused defines and enums
  2021-10-20 19:54 ` [PATCH 5/5] staging: r8188eu: remove unused defines and enums Martin Kaiser
  2021-10-20 21:10   ` Phillip Potter
@ 2021-10-21 10:21   ` Michael Straube
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Straube @ 2021-10-21 10:21 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On 10/20/21 21:54, Martin Kaiser wrote:
> Remove a couple of unused defines and an unused enum
> from rtl8188e_cmd.h.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/include/rtl8188e_cmd.h | 13 -------------
>   1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/rtl8188e_cmd.h b/drivers/staging/r8188eu/include/rtl8188e_cmd.h
> index 01404b774ebd..1e01c1662f9a 100644
> --- a/drivers/staging/r8188eu/include/rtl8188e_cmd.h
> +++ b/drivers/staging/r8188eu/include/rtl8188e_cmd.h
> @@ -27,15 +27,6 @@ enum RTL8188E_H2C_CMD_ID {
>   	/* Class DM */
>   	H2C_DM_MACID_CFG		= 0x40,
>   	H2C_DM_TXBF			= 0x41,
> -
> -	/* Class BT */
> -	H2C_BT_COEX_MASK		= 0x60,
> -	H2C_BT_COEX_GPIO_MODE		= 0x61,
> -	H2C_BT_DAC_SWING_VAL		= 0x62,
> -	H2C_BT_PSD_RST			= 0x63,
> -
> -	/* Class */
> -	 H2C_RESET_TSF			= 0xc0,
>   };
>   
>   struct cmd_msg_parm {
> @@ -44,10 +35,6 @@ struct cmd_msg_parm {
>   	u8 buf[6];
>   };
>   
> -enum {
> -	PWRS
> -};
> -
>   struct setpwrmode_parm {
>   	u8 Mode;/* 0:Active,1:LPS,2:WMMPS */
>   	u8 SmartPS_RLBM;/* LPS= 0:PS_Poll,1:PS_Poll,2:NullData,WMM= 0:PS_Poll,1:NullData */
> 

Looks good, thanks.

Acked-by: Michael Straube <straube.linux@gmail.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address
  2021-10-21 10:12     ` Michael Straube
@ 2021-10-22  9:21       ` Martin Kaiser
  2021-11-02 14:59         ` Dan Carpenter
  0 siblings, 1 reply; 17+ messages in thread
From: Martin Kaiser @ 2021-10-22  9:21 UTC (permalink / raw)
  To: Michael Straube
  Cc: Phillip Potter, Greg Kroah-Hartman, Larry Finger, linux-staging,
	linux-kernel

Thus wrote Michael Straube (straube.linux@gmail.com):

> > > +		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))

> Hi Martin,

> I'm not an expert regarding alignment. Is GetAddr1Ptr(pframe) always
> __aligned(2) as required by is_broadcast_ether_addr?

Hi Michael,

thanks for spotting this. To be honest, I didn't look at this in much
detail when I wrote the patch.

I suppose the pframe comes from recvbuf2recvframe().
precvframe = rtw_alloc_recvframe(pfree_recv_queue);
with
struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue; 

This should be initialised in _rtw_init_recv_priv().
rtw_init_queue(&precvpriv->free_recv_queue);
...
precvpriv->precv_frame_buf = (u8 *)N_BYTE_ALIGMENT((size_t)(precvpriv->pallocated_frame_buf), RXFRAME_ALIGN_SZ);
precvframe = (struct recv_frame *)precvpriv->precv_frame_buf;
and the frames are added to the free_recv_queue.
RXFRAME_ALIGN_SZ is 1<<8.

So pframe should be 256-byte aligned.
GetAddr1Ptr adds 4 to the start of pframe.

I guess we're safe here.

> > > @@ -841,7 +840,7 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
> > >   		psta = pDM_Odm->pODM_StaInfo[i];
> > >   		if (IS_STA_VALID(psta) &&
> > >   		    (psta->state & WIFI_ASOC_STATE) &&
> > > -		    memcmp(psta->hwaddr, bcast_addr, ETH_ALEN) &&
> > > +		    !is_broadcast_ether_addr(psta->hwaddr) &&

> Perhaps we should add __aligned(2) to the hwaddr variable in struct
> sta_info to be safe?

> u8	hwaddr[ETH_ALEN] __aligned(2);

Hmm, some of those arrays in other parts of the kernel have
__aligned(2), others don't...

Can anyone else give some guidance?

Thanks,
Martin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address
  2021-10-22  9:21       ` Martin Kaiser
@ 2021-11-02 14:59         ` Dan Carpenter
  0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2021-11-02 14:59 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Michael Straube, Phillip Potter, Greg Kroah-Hartman,
	Larry Finger, linux-staging, linux-kernel

On Fri, Oct 22, 2021 at 11:21:10AM +0200, Martin Kaiser wrote:
> Thus wrote Michael Straube (straube.linux@gmail.com):
> 
> > > > +		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
> 
> > Hi Martin,
> 
> > I'm not an expert regarding alignment. Is GetAddr1Ptr(pframe) always
> > __aligned(2) as required by is_broadcast_ether_addr?
> 
> Hi Michael,
> 
> thanks for spotting this. To be honest, I didn't look at this in much
> detail when I wrote the patch.
> 
> I suppose the pframe comes from recvbuf2recvframe().
> precvframe = rtw_alloc_recvframe(pfree_recv_queue);
> with
> struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue; 
> 
> This should be initialised in _rtw_init_recv_priv().
> rtw_init_queue(&precvpriv->free_recv_queue);
> ...
> precvpriv->precv_frame_buf = (u8 *)N_BYTE_ALIGMENT((size_t)(precvpriv->pallocated_frame_buf), RXFRAME_ALIGN_SZ);
> precvframe = (struct recv_frame *)precvpriv->precv_frame_buf;
> and the frames are added to the free_recv_queue.
> RXFRAME_ALIGN_SZ is 1<<8.
> 
> So pframe should be 256-byte aligned.
> GetAddr1Ptr adds 4 to the start of pframe.
> 
> I guess we're safe here.

Greg already applied this patch.

I'm not sure why you're looking at precvpriv->precv_frame_buf instead of
"precv_frame->rx_data"?  Am I missing something?  This is actually a bit
tricky for me to analyze because it gets set in two places:

drivers/staging/r8188eu/core/rtw_recv.c | recvframe_pull | (struct recv_frame)->rx_data | 0-u64max
drivers/staging/r8188eu/hal/usb_ops_linux.c | recvbuf2recvframe | (struct recv_frame)->rx_data | 0-u64max

But the fact that we're using GetAddr1Ptr() on it at all suggests that
it must be aligned and hopefully we've verified that the ->rx_data has
enough data etc...  ?  This code is much trickier than I like it to be.
Anyway, this patch doesn't introduce bugs that weren't present in the
original.

> 
> > > > @@ -841,7 +840,7 @@ void odm_RSSIMonitorCheck(struct odm_dm_struct *pDM_Odm)
> > > >   		psta = pDM_Odm->pODM_StaInfo[i];
> > > >   		if (IS_STA_VALID(psta) &&
> > > >   		    (psta->state & WIFI_ASOC_STATE) &&
> > > > -		    memcmp(psta->hwaddr, bcast_addr, ETH_ALEN) &&
> > > > +		    !is_broadcast_ether_addr(psta->hwaddr) &&
> 
> > Perhaps we should add __aligned(2) to the hwaddr variable in struct
> > sta_info to be safe?
> 
> > u8	hwaddr[ETH_ALEN] __aligned(2);
> 
> Hmm, some of those arrays in other parts of the kernel have
> __aligned(2), others don't...
> 
> Can anyone else give some guidance?

This array comes after a u32 and it's not packed so it's going to
__aligned(4) already.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-11-02 15:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 19:53 [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Martin Kaiser
2021-10-20 19:53 ` [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant Martin Kaiser
2021-10-20 21:06   ` Phillip Potter
2021-10-21 10:18   ` Michael Straube
2021-10-20 19:53 ` [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address Martin Kaiser
2021-10-20 21:07   ` Phillip Potter
2021-10-21 10:12     ` Michael Straube
2021-10-22  9:21       ` Martin Kaiser
2021-11-02 14:59         ` Dan Carpenter
2021-10-20 19:54 ` [PATCH 4/5] staging: r8188eu: use helper to set " Martin Kaiser
2021-10-20 21:08   ` Phillip Potter
2021-10-21 10:20   ` Michael Straube
2021-10-20 19:54 ` [PATCH 5/5] staging: r8188eu: remove unused defines and enums Martin Kaiser
2021-10-20 21:10   ` Phillip Potter
2021-10-21 10:21   ` Michael Straube
2021-10-20 21:05 ` [PATCH 1/5] staging: r8188eu: remove unused dm_priv components Phillip Potter
2021-10-21 10:17 ` Michael Straube

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).