[PATCH] staging: rtl8188eu: core: find and remove code valid only for 5 HGz.

[PATCH] staging: rtl8188eu: core: find and remove code valid only for 5 HGz.

[Sreenath Madasu]

This one of the TODO tasks for staging rtl8188eu driver. I have removed
the code referring to channel > 14 for rtw_ap.c, rtw_ieee80211.c and
rtw_mlme.c files. Please review.

Signed-off-by: Sreenath Madasu <Sreenath.Madasu@gmail.com>
---
 drivers/staging/rtl8188eu/core/rtw_ap.c        | 31 +++++++-------------------
 drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 24 ++++++--------------
 drivers/staging/rtl8188eu/core/rtw_mlme.c      |  5 +----
 3 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 293510e..1a32e7b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -497,20 +497,12 @@ void add_RATid(struct adapter *padapter, struct sta_info *psta, u8 rssi_level)
 		shortGIrate = psta_ht->sgi;
 	}
 
-	if (pcur_network->Configuration.DSConfig > 14) {
-		/*  5G band */
-		if (tx_ra_bitmap & 0xffff000)
-			sta_band |= WIRELESS_11_5N | WIRELESS_11A;
-		else
-			sta_band |= WIRELESS_11A;
-	} else {
-		if (tx_ra_bitmap & 0xffff000)
-			sta_band |= WIRELESS_11_24N | WIRELESS_11G | WIRELESS_11B;
-		else if (tx_ra_bitmap & 0xff0)
-			sta_band |= WIRELESS_11G | WIRELESS_11B;
-		else
-			sta_band |= WIRELESS_11B;
-	}
+	if (tx_ra_bitmap & 0xffff000)
+		sta_band |= WIRELESS_11_24N | WIRELESS_11G | WIRELESS_11B;
+	else if (tx_ra_bitmap & 0xff0)
+		sta_band |= WIRELESS_11G | WIRELESS_11B;
+	else
+		sta_band |= WIRELESS_11B;
 
 	psta->wireless_mode = sta_band;
 
@@ -584,15 +576,8 @@ static void update_bmc_sta(struct adapter *padapter)
 				tx_ra_bitmap |= rtw_get_bit_value_from_ieee_value(psta->bssrateset[i]&0x7f);
 		}
 
-		if (pcur_network->Configuration.DSConfig > 14) {
-			/* force to A mode. 5G doesn't support CCK rates */
-			network_type = WIRELESS_11A;
-			tx_ra_bitmap = 0x150; /*  6, 12, 24 Mbps */
-		} else {
-			/* force to b mode */
-			network_type = WIRELESS_11B;
-			tx_ra_bitmap = 0xf;
-		}
+		network_type = WIRELESS_11B;
+		tx_ra_bitmap = 0xf;
 
 		raid = networktype_to_raid(network_type);
 		init_rate = get_highest_rate_idx(tx_ra_bitmap&0x0fffffff)&0x3f;
diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
index 11b780d..f55dae1 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
@@ -113,19 +113,12 @@ uint	rtw_is_cckratesonly_included(u8 *rate)
 
 int rtw_check_network_type(unsigned char *rate, int ratelen, int channel)
 {
-	if (channel > 14) {
-		if ((rtw_is_cckrates_included(rate)) == true)
-			return WIRELESS_INVALID;
-		else
-			return WIRELESS_11A;
-	} else {  /*  could be pure B, pure G, or B/G */
-		if ((rtw_is_cckratesonly_included(rate)) == true)
-			return WIRELESS_11B;
-		else if ((rtw_is_cckrates_included(rate)) == true)
-			return	WIRELESS_11BG;
-		else
-			return WIRELESS_11G;
-	}
+	if ((rtw_is_cckratesonly_included(rate)) == true)
+		return WIRELESS_11B;
+	else if ((rtw_is_cckrates_included(rate)) == true)
+		return	WIRELESS_11BG;
+	else
+		return WIRELESS_11G;
 }
 
 u8 *rtw_set_fixed_ie(unsigned char *pbuf, unsigned int len, unsigned char *source,
@@ -404,10 +397,7 @@ int rtw_generate_ie(struct registry_priv *pregistrypriv)
 
 	/* supported rates */
 	if (pregistrypriv->wireless_mode == WIRELESS_11ABGN) {
-		if (pdev_network->Configuration.DSConfig > 14)
-			wireless_mode = WIRELESS_11A_5N;
-		else
-			wireless_mode = WIRELESS_11BG_24N;
+		wireless_mode = WIRELESS_11BG_24N;
 	} else {
 		wireless_mode = pregistrypriv->wireless_mode;
 	}
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 0558451..15d91814 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1872,10 +1872,7 @@ void rtw_update_registrypriv_dev_network(struct adapter *adapter)
 		pdev_network->NetworkTypeInUse = (Ndis802_11OFDM5);
 		break;
 	case WIRELESS_11ABGN:
-		if (pregistrypriv->channel > 14)
-			pdev_network->NetworkTypeInUse = (Ndis802_11OFDM5);
-		else
-			pdev_network->NetworkTypeInUse = (Ndis802_11OFDM24);
+		pdev_network->NetworkTypeInUse = (Ndis802_11OFDM24);
 		break;
 	default:
 		/*  TODO */
-- 
2.3.6

Re: [PATCH] staging: rtl8188eu: core: find and remove code valid only for 5 HGz.

[Sudip Mukherjee]

On Wed, Jul 15, 2015 at 10:04:08PM -0400, Sreenath Madasu wrote:
> This one of the TODO tasks for staging rtl8188eu driver. I have removed
> the code referring to channel > 14 for rtw_ap.c, rtw_ieee80211.c and
> rtw_mlme.c files. Please review.
Your patch will give a new build warning:
warning: unused variable ‘pcur_network’ [-Wunused-variable]

regards
sudip

Re: [PATCH] staging: rtl8188eu: core: find and remove code valid only for 5 HGz.

[Jakub Sitnicki]

On Thu, Jul 16, 2015 at 04:04 AM CEST, Sreenath Madasu <sreenath.madasu@gmail.com> wrote:
> This one of the TODO tasks for staging rtl8188eu driver. I have removed
> the code referring to channel > 14 for rtw_ap.c, rtw_ieee80211.c and
> rtw_mlme.c files. Please review.
>
> Signed-off-by: Sreenath Madasu <Sreenath.Madasu@gmail.com>

I would consider rewording the subject and the description to say what
the change does ("remove ...") as opposed to saying what you did ("find
and remove...").

The "Description, description, description" section from the article
below, if you haven't seen it yet, explains it better than I can:

https://github.com/gregkh/kernel-tutorial/blob/master/lxf_article/write_kernel_patch.txt

> ---
>  drivers/staging/rtl8188eu/core/rtw_ap.c        | 31 +++++++-------------------
>  drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 24 ++++++--------------
>  drivers/staging/rtl8188eu/core/rtw_mlme.c      |  5 +----
>  3 files changed, 16 insertions(+), 44 deletions(-)
>

[snip]

> @@ -584,15 +576,8 @@ static void update_bmc_sta(struct adapter *padapter)
>  				tx_ra_bitmap |= rtw_get_bit_value_from_ieee_value(psta->bssrateset[i]&0x7f);
>  		}
>  
> -		if (pcur_network->Configuration.DSConfig > 14) {
> -			/* force to A mode. 5G doesn't support CCK rates */
> -			network_type = WIRELESS_11A;
> -			tx_ra_bitmap = 0x150; /*  6, 12, 24 Mbps */
> -		} else {
> -			/* force to b mode */
> -			network_type = WIRELESS_11B;
> -			tx_ra_bitmap = 0xf;
> -		}
> +		network_type = WIRELESS_11B;
> +		tx_ra_bitmap = 0xf;
>  
>  		raid = networktype_to_raid(network_type);
>  		init_rate = get_highest_rate_idx(tx_ra_bitmap&0x0fffffff)&0x3f;

Is the dropped comment ("force to b mode") worth leaving just to draw
attention?  There is something suspicious going on in update_bmc_sta()
because just a few lines above we determine the network_type:

        network_type = rtw_check_network_type((u8 *)&pcur_network->SupportedRates, supportRateNum, 1);

... only to force to it WIRELESS_11B later on.  Unfortunately I'm not
familiar enough with the driver to know why.

> diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> index 11b780d..f55dae1 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> @@ -113,19 +113,12 @@ uint	rtw_is_cckratesonly_included(u8 *rate)
>  
>  int rtw_check_network_type(unsigned char *rate, int ratelen, int channel)
>  {
> -	if (channel > 14) {
> -		if ((rtw_is_cckrates_included(rate)) == true)
> -			return WIRELESS_INVALID;
> -		else
> -			return WIRELESS_11A;
> -	} else {  /*  could be pure B, pure G, or B/G */
> -		if ((rtw_is_cckratesonly_included(rate)) == true)
> -			return WIRELESS_11B;
> -		else if ((rtw_is_cckrates_included(rate)) == true)
> -			return	WIRELESS_11BG;
> -		else
> -			return WIRELESS_11G;
> -	}
> +	if ((rtw_is_cckratesonly_included(rate)) == true)
> +		return WIRELESS_11B;
> +	else if ((rtw_is_cckrates_included(rate)) == true)
> +		return	WIRELESS_11BG;
> +	else
> +		return WIRELESS_11G;
>  }
>

That makes the 'channel' parameter unused.  A candidate for a clean-up
together with 'ratelen'.

Cheers,
Jakub