2021-10-20 19:56:45

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 1/5] staging: r8188eu: remove unused dm_priv components

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 <[email protected]>
---
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


2021-10-20 19:56:45

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant

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 <[email protected]>
---
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

2021-10-20 19:57:04

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address

Use the is_broadcast_ether_addr function to check for a
broadcast address.

Signed-off-by: Martin Kaiser <[email protected]>
---
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

2021-10-20 19:57:29

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 4/5] staging: r8188eu: use helper to set broadcast address

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 <[email protected]>
---
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

2021-10-20 19:58:48

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 5/5] staging: r8188eu: remove unused defines and enums

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

Signed-off-by: Martin Kaiser <[email protected]>
---
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

2021-10-20 21:08:15

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: r8188eu: remove unused dm_priv components

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards,
Phil

2021-10-20 21:10:33

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards,
Phil

2021-10-20 21:11:17

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards,
Phil

2021-10-20 21:13:03

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH 5/5] staging: r8188eu: remove unused defines and enums

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards,
Phil

2021-10-20 21:14:30

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH 4/5] staging: r8188eu: use helper to set broadcast address

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards,
Phil

2021-10-21 10:17:19

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address

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 <[email protected]>
>> ---
>> 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

2021-10-21 10:19:38

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: r8188eu: remove unused dm_priv components

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-10-21 10:21:40

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 2/5] staging: r8188eu: odm_rate_adapt Type is constant

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-10-21 10:23:04

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 4/5] staging: r8188eu: use helper to set broadcast address

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-10-21 10:24:02

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 5/5] staging: r8188eu: remove unused defines and enums

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-10-22 09:22:22

by Martin Kaiser

[permalink] [raw]
Subject: Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address

Thus wrote Michael Straube ([email protected]):

> > > + !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

2021-11-02 15:24:16

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 3/5] staging: r8188eu: use helper to check for broadcast address

On Fri, Oct 22, 2021 at 11:21:10AM +0200, Martin Kaiser wrote:
> Thus wrote Michael Straube ([email protected]):
>
> > > > + !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