[PATCH v2 0/6] staging: r8188eu: some rx cleanups

[PATCH v2 0/6] staging: r8188eu: some rx cleanups

[Martin Kaiser]

Here's another set with simple cleanups in the code for receiving frames.

v2 fixes a build error on 64-bit systems in the last patch.

Martin Kaiser (6):
  staging: r8188eu: remove unnecessary initializations
  staging: r8188eu: remove three unused receive defines
  staging: r8188eu: remove unused function prototype
  staging: r8188eu: make rtl8188e_process_phy_info static
  staging: r8188eu: remove some unused local ieee80211 macros
  staging: r8188eu: remove local BIT macro

 drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c |  6 +---
 .../staging/r8188eu/include/rtl8188e_recv.h   |  6 ----
 .../staging/r8188eu/include/rtw_mlme_ext.h    |  2 +-
 drivers/staging/r8188eu/include/wifi.h        | 34 +------------------
 4 files changed, 3 insertions(+), 45 deletions(-)

-- 
2.30.2

[PATCH v2 1/6] staging: r8188eu: remove unnecessary initializations

[Martin Kaiser]

Do not initialise bPacketMatchBSSID, bPacketToSelf and bPacketBeacon to
false at the start of update_recvframe_phyinfo_88e. All of them are set to
their actual values before they're used.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c b/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c
index b4c9738ed868..387af789797d 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c
@@ -118,10 +118,6 @@ void update_recvframe_phyinfo_88e(struct recv_frame *precvframe, struct phy_stat
 	struct sta_priv *pstapriv;
 	struct sta_info *psta;
 
-	pkt_info.bPacketMatchBSSID = false;
-	pkt_info.bPacketToSelf = false;
-	pkt_info.bPacketBeacon = false;
-
 	pkt_info.bPacketMatchBSSID = ((!IsFrameTypeCtrl(wlanhdr)) &&
 		!pattrib->icv_err && !pattrib->crc_err &&
 		!memcmp(get_hdr_bssid(wlanhdr),
-- 
2.30.2

[PATCH v2 2/6] staging: r8188eu: remove three unused receive defines

[Martin Kaiser]

Remove three unused defines from rtl8188e_recv.h.

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

diff --git a/drivers/staging/r8188eu/include/rtl8188e_recv.h b/drivers/staging/r8188eu/include/rtl8188e_recv.h
index a2320af11216..09adffd756de 100644
--- a/drivers/staging/r8188eu/include/rtl8188e_recv.h
+++ b/drivers/staging/r8188eu/include/rtl8188e_recv.h
@@ -6,10 +6,6 @@
 
 #define TX_RPT1_PKT_LEN 8
 
-#define RECV_BLK_SZ 512
-#define RECV_BLK_CNT 16
-#define RECV_BLK_TH RECV_BLK_CNT
-
 #define NR_PREALLOC_RECV_SKB (8)
 
 #define NR_RECVBUFF (4)
-- 
2.30.2

[PATCH v2 3/6] staging: r8188eu: remove unused function prototype

[Martin Kaiser]

Remove the prototype for rtl8188eu_recv_hdl. This function does not exist
in the r8188eu driver.

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

diff --git a/drivers/staging/r8188eu/include/rtl8188e_recv.h b/drivers/staging/r8188eu/include/rtl8188e_recv.h
index 09adffd756de..8d06815d0374 100644
--- a/drivers/staging/r8188eu/include/rtl8188e_recv.h
+++ b/drivers/staging/r8188eu/include/rtl8188e_recv.h
@@ -35,7 +35,6 @@ enum rx_packet_type {
 
 s32 rtl8188eu_init_recv_priv(struct adapter *padapter);
 void rtl8188eu_free_recv_priv(struct adapter * padapter);
-void rtl8188eu_recv_hdl(struct adapter * padapter, struct recv_buf *precvbuf);
 void rtl8188eu_recv_tasklet(unsigned long priv);
 void rtl8188e_process_phy_info(struct adapter * padapter, void *prframe);
 void update_recvframe_phyinfo_88e(struct recv_frame *fra, struct phy_stat *phy);
-- 
2.30.2

[PATCH v2 4/6] staging: r8188eu: make rtl8188e_process_phy_info static

[Martin Kaiser]

rtl8188e_process_phy_info is used only in rtl8188e_rxdesc.c. Make the
function static and remove its prototype from rtl8188e_recv.h.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c   | 2 +-
 drivers/staging/r8188eu/include/rtl8188e_recv.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c b/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c
index 387af789797d..9bf7a9248026 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c
@@ -45,7 +45,7 @@ static void process_link_qual(struct adapter *padapter, struct recv_frame *prfra
 	signal_stat->avg_val = signal_stat->total_val / signal_stat->total_num;
 }
 
-void rtl8188e_process_phy_info(struct adapter *padapter, void *prframe)
+static void rtl8188e_process_phy_info(struct adapter *padapter, void *prframe)
 {
 	struct recv_frame *precvframe = (struct recv_frame *)prframe;
 
diff --git a/drivers/staging/r8188eu/include/rtl8188e_recv.h b/drivers/staging/r8188eu/include/rtl8188e_recv.h
index 8d06815d0374..b752c5c06309 100644
--- a/drivers/staging/r8188eu/include/rtl8188e_recv.h
+++ b/drivers/staging/r8188eu/include/rtl8188e_recv.h
@@ -36,7 +36,6 @@ enum rx_packet_type {
 s32 rtl8188eu_init_recv_priv(struct adapter *padapter);
 void rtl8188eu_free_recv_priv(struct adapter * padapter);
 void rtl8188eu_recv_tasklet(unsigned long priv);
-void rtl8188e_process_phy_info(struct adapter * padapter, void *prframe);
 void update_recvframe_phyinfo_88e(struct recv_frame *fra, struct phy_stat *phy);
 void update_recvframe_attrib_88e(struct recv_frame *fra, struct recv_stat *stat);
 
-- 
2.30.2

[PATCH v2 5/6] staging: r8188eu: remove some unused local ieee80211 macros

[Martin Kaiser]

Remove some macros from wifi.h which are not used by this driver.

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

diff --git a/drivers/staging/r8188eu/include/wifi.h b/drivers/staging/r8188eu/include/wifi.h
index c1c9bae58d6d..c331be19ff83 100644
--- a/drivers/staging/r8188eu/include/wifi.h
+++ b/drivers/staging/r8188eu/include/wifi.h
@@ -167,17 +167,11 @@ enum WIFI_REG_DOMAIN {
 
 #define GetToDs(pbuf)	(((*(__le16 *)(pbuf)) & cpu_to_le16(_TO_DS_)) != 0)
 
-#define ClearToDs(pbuf)	\
-	*(__le16 *)(pbuf) &= (~cpu_to_le16(_TO_DS_))
-
 #define SetFrDs(pbuf)	\
 	*(__le16 *)(pbuf) |= cpu_to_le16(_FROM_DS_)
 
 #define GetFrDs(pbuf)	(((*(__le16 *)(pbuf)) & cpu_to_le16(_FROM_DS_)) != 0)
 
-#define ClearFrDs(pbuf)	\
-	*(__le16 *)(pbuf) &= (~cpu_to_le16(_FROM_DS_))
-
 #define get_tofr_ds(pframe)	((GetToDs(pframe) << 1) | GetFrDs(pframe))
 
 #define SetMFrag(pbuf)	\
@@ -186,46 +180,25 @@ enum WIFI_REG_DOMAIN {
 #define ClearMFrag(pbuf)	\
 	*(__le16 *)(pbuf) &= (~cpu_to_le16(_MORE_FRAG_))
 
-#define SetRetry(pbuf)	\
-	*(__le16 *)(pbuf) |= cpu_to_le16(_RETRY_)
-
 #define GetRetry(pbuf)	(((*(__le16 *)(pbuf)) & cpu_to_le16(_RETRY_)) != 0)
 
-#define ClearRetry(pbuf)	\
-	*(__le16 *)(pbuf) &= (~cpu_to_le16(_RETRY_))
-
 #define SetPwrMgt(pbuf)	\
 	*(__le16 *)(pbuf) |= cpu_to_le16(_PWRMGT_)
 
 #define GetPwrMgt(pbuf)	(((*(__le16 *)(pbuf)) & cpu_to_le16(_PWRMGT_)) != 0)
 
-#define ClearPwrMgt(pbuf)	\
-	*(__le16 *)(pbuf) &= (~cpu_to_le16(_PWRMGT_))
-
 #define SetMData(pbuf)	\
 	*(__le16 *)(pbuf) |= cpu_to_le16(_MORE_DATA_)
 
-#define ClearMData(pbuf)	\
-	*(__le16 *)(pbuf) &= (~cpu_to_le16(_MORE_DATA_))
-
 #define SetPrivacy(pbuf)	\
 	*(__le16 *)(pbuf) |= cpu_to_le16(_PRIVACY_)
 
 #define GetPrivacy(pbuf)					\
 	(((*(__le16 *)(pbuf)) & cpu_to_le16(_PRIVACY_)) != 0)
 
-#define ClearPrivacy(pbuf)	\
-	*(__le16 *)(pbuf) &= (~cpu_to_le16(_PRIVACY_))
-
 #define GetFrameType(pbuf)				\
 	(le16_to_cpu(*(__le16 *)(pbuf)) & (BIT(3) | BIT(2)))
 
-#define SetFrameType(pbuf, type)	\
-	do {	\
-		*(unsigned short *)(pbuf) &= __constant_cpu_to_le16(~(BIT(3) | BIT(2))); \
-		*(unsigned short *)(pbuf) |= __constant_cpu_to_le16(type); \
-	} while (0)
-
 #define GetFrameSubType(pbuf)	(le16_to_cpu(*(__le16 *)(pbuf)) & (BIT(7) |\
 	 BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2)))
 
-- 
2.30.2

[PATCH v2 6/6] staging: r8188eu: remove local BIT macro

[Martin Kaiser]

The r8188eu driver defines a local BIT(x) macro. Remove this local macro
and use the one from include/linux/bits.h.

The global BIT macro returns an unsigned long value. Therefore, we have to
cast DYNAMIC_BB_DYNAMIC_TXPWR to u32 explicitly. This define is used with
the bitwise not operator.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - cast DYNAMIC_BB_DYNAMIC_TXPWR to u32, fix build error on 64-bit systems

 drivers/staging/r8188eu/include/rtw_mlme_ext.h | 2 +-
 drivers/staging/r8188eu/include/wifi.h         | 7 +------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index 0c555ea6719b..2b5089be5156 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -34,7 +34,7 @@
 /*  BB ODM section BIT 0-15 */
 #define	DYNAMIC_BB_DIG				BIT(0)
 #define	DYNAMIC_BB_RA_MASK			BIT(1)
-#define	DYNAMIC_BB_DYNAMIC_TXPWR	BIT(2)
+#define	DYNAMIC_BB_DYNAMIC_TXPWR	((u32)BIT(2))
 #define	DYNAMIC_BB_BB_FA_CNT			BIT(3)
 
 #define		DYNAMIC_BB_RSSI_MONITOR		BIT(4)
diff --git a/drivers/staging/r8188eu/include/wifi.h b/drivers/staging/r8188eu/include/wifi.h
index c331be19ff83..299553351246 100644
--- a/drivers/staging/r8188eu/include/wifi.h
+++ b/drivers/staging/r8188eu/include/wifi.h
@@ -4,14 +4,9 @@
 #ifndef _WIFI_H_
 #define _WIFI_H_
 
+#include <linux/bits.h>
 #include <linux/ieee80211.h>
 
-#ifdef BIT
-/* error	"BIT define occurred earlier elsewhere!\n" */
-#undef BIT
-#endif
-#define BIT(x)	(1 << (x))
-
 #define WLAN_ETHHDR_LEN		14
 #define WLAN_HDR_A3_LEN		24
 #define WLAN_HDR_A3_QOS_LEN	26
-- 
2.30.2

Re: [PATCH v2 6/6] staging: r8188eu: remove local BIT macro

[Greg Kroah-Hartman]

On Tue, Mar 15, 2022 at 09:50:41PM +0100, Martin Kaiser wrote:
> The r8188eu driver defines a local BIT(x) macro. Remove this local macro
> and use the one from include/linux/bits.h.
> 
> The global BIT macro returns an unsigned long value. Therefore, we have to
> cast DYNAMIC_BB_DYNAMIC_TXPWR to u32 explicitly. This define is used with
> the bitwise not operator.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> v2:
>  - cast DYNAMIC_BB_DYNAMIC_TXPWR to u32, fix build error on 64-bit systems
> 
>  drivers/staging/r8188eu/include/rtw_mlme_ext.h | 2 +-
>  drivers/staging/r8188eu/include/wifi.h         | 7 +------
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> index 0c555ea6719b..2b5089be5156 100644
> --- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> +++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> @@ -34,7 +34,7 @@
>  /*  BB ODM section BIT 0-15 */
>  #define	DYNAMIC_BB_DIG				BIT(0)
>  #define	DYNAMIC_BB_RA_MASK			BIT(1)
> -#define	DYNAMIC_BB_DYNAMIC_TXPWR	BIT(2)
> +#define	DYNAMIC_BB_DYNAMIC_TXPWR	((u32)BIT(2))

That's really odd, why the cast for just one define?  Try fixing up
where this is used instead.

thanks,

greg k-h

Re: [PATCH v2 6/6] staging: r8188eu: remove local BIT macro

[Dan Carpenter]

On Tue, Mar 15, 2022 at 09:50:41PM +0100, Martin Kaiser wrote:
> The r8188eu driver defines a local BIT(x) macro. Remove this local macro
> and use the one from include/linux/bits.h.
> 
> The global BIT macro returns an unsigned long value. Therefore, we have to
> cast DYNAMIC_BB_DYNAMIC_TXPWR to u32 explicitly. This define is used with
> the bitwise not operator.

It doesn't change run time at all if you leave it as unsigned long.
I don't know if there are static checkers which care, but if there are
then those checkers are wrong.  It's nicer to not have the unnecesary
cast.

Also in the original code the type was int so the negative bit would
have been sign extended if we cast it to unsigned long.

regards,
dan carpenter

Re: [PATCH v2 6/6] staging: r8188eu: remove local BIT macro

[Dan Carpenter]

On Wed, Mar 16, 2022 at 12:01:28PM +0300, Dan Carpenter wrote:
> On Tue, Mar 15, 2022 at 09:50:41PM +0100, Martin Kaiser wrote:
> > The r8188eu driver defines a local BIT(x) macro. Remove this local macro
> > and use the one from include/linux/bits.h.
> > 
> > The global BIT macro returns an unsigned long value. Therefore, we have to
> > cast DYNAMIC_BB_DYNAMIC_TXPWR to u32 explicitly. This define is used with
> > the bitwise not operator.
> 
> It doesn't change run time at all if you leave it as unsigned long.
> I don't know if there are static checkers which care, but if there are
> then those checkers are wrong.  It's nicer to not have the unnecesary
> cast.

Oop.  It's a GCC warning...

People accept such absolute garbage from GCC.  It's like in Africa when
you get bitten by a snake they rub cow dung into the wound.  If it hurts
it must be good.

regards,
dan carpenter

Re: [PATCH v2 0/6] staging: r8188eu: some rx cleanups

[Greg Kroah-Hartman]

On Tue, Mar 15, 2022 at 09:50:35PM +0100, Martin Kaiser wrote:
> Here's another set with simple cleanups in the code for receiving frames.
> 
> v2 fixes a build error on 64-bit systems in the last patch.
> 
> Martin Kaiser (6):
>   staging: r8188eu: remove unnecessary initializations
>   staging: r8188eu: remove three unused receive defines
>   staging: r8188eu: remove unused function prototype
>   staging: r8188eu: make rtl8188e_process_phy_info static
>   staging: r8188eu: remove some unused local ieee80211 macros
>   staging: r8188eu: remove local BIT macro
> 
>  drivers/staging/r8188eu/hal/rtl8188e_rxdesc.c |  6 +---
>  .../staging/r8188eu/include/rtl8188e_recv.h   |  6 ----
>  .../staging/r8188eu/include/rtw_mlme_ext.h    |  2 +-
>  drivers/staging/r8188eu/include/wifi.h        | 34 +------------------
>  4 files changed, 3 insertions(+), 45 deletions(-)
> 
> -- 
> 2.30.2
> 
> 

I took the first 5 patches here, thanks.

greg k-h