2015-07-16 02:04:31

by Sreenath Madasu

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

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


2015-07-16 05:32:18

by Sudip Mukherjee

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

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

2015-07-16 15:42:41

by Jakub Sitnicki

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

On Thu, Jul 16, 2015 at 04:04 AM CEST, Sreenath Madasu <[email protected]> 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 <[email protected]>

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