2016-07-20 15:18:48

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/3] staging/rtl8192u: use s8 instead of char

Compiling the rtlwifi drivers for ARM with gcc -Wextra warns about lots of
incorrect code that results from 'char' being unsigned here, e.g.

staging/rtl8192u/r8192U_core.c:4150:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
staging/rtl8192u/r8192U_dm.c:646:50: error: comparison is always false due to limited range of data type [-Werror=type-limits]

This patch changes all uses of 'char' in this driver that refer to
8-bit integers to use 's8' instead, which is signed on all architectures.

Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Jes Sorensen <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/ieee80211.h | 4 ++--
drivers/staging/rtl8192u/r8192U.h | 4 ++--
drivers/staging/rtl8192u/r8192U_core.c | 14 +++++++-------
3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 09e9499b7f9d..077ea13eb1e7 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -746,7 +746,7 @@ struct ieee80211_rx_stats {
bool bisrxaggrsubframe;
bool bPacketBeacon; //cosa add for rssi
bool bToSelfBA; //cosa add for rssi
- char cck_adc_pwdb[4]; //cosa add for rx path selection
+ s8 cck_adc_pwdb[4]; //cosa add for rx path selection
u16 Seq_Num;

};
@@ -1814,7 +1814,7 @@ struct ieee80211_device {
u32 wpax_type_notify; //{added by David, 2006.9.26}

/* QoS related flag */
- char init_wmmparam_flag;
+ s8 init_wmmparam_flag;
/* set on initialization */
u8 qos_support;

diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 5dba6a28dd9b..b28bc7812caa 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -533,7 +533,7 @@ typedef struct _rt_9x_tx_rate_history {
u32 ht_mcs[4][16];
} rt_tx_rahis_t, *prt_tx_rahis_t;
typedef struct _RT_SMOOTH_DATA_4RF {
- char elements[4][100]; /* array to store values */
+ s8 elements[4][100]; /* array to store values */
u32 index; /* index to current array to store */
u32 TotalNum; /* num of valid elements */
u32 TotalVal[4]; /* sum of valid elements */
@@ -1031,7 +1031,7 @@ typedef struct r8192_priv {
s8 cck_present_attentuation;
u8 cck_present_attentuation_20Mdefault;
u8 cck_present_attentuation_40Mdefault;
- char cck_present_attentuation_difference;
+ s8 cck_present_attentuation_difference;
bool btxpower_tracking;
bool bcck_in_ch14;
bool btxpowerdata_readfromEEPORM;
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index dd0970facdf5..f36b2d3b1ee9 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4209,7 +4209,7 @@ static void rtl8192_process_phyinfo(struct r8192_priv *priv, u8 *buffer,
*
* Return: 0-100 percentage
*---------------------------------------------------------------------------*/
-static u8 rtl819x_query_rxpwrpercentage(char antpower)
+static u8 rtl819x_query_rxpwrpercentage(s8 antpower)
{
if ((antpower <= -100) || (antpower >= 20))
return 0;
@@ -4220,9 +4220,9 @@ static u8 rtl819x_query_rxpwrpercentage(char antpower)

} /* QueryRxPwrPercentage */

-static u8 rtl819x_evm_dbtopercentage(char value)
+static u8 rtl819x_evm_dbtopercentage(s8 value)
{
- char ret_val;
+ s8 ret_val;

ret_val = value;

@@ -4297,8 +4297,8 @@ static void rtl8192_query_rxphystatus(struct r8192_priv *priv,
phy_ofdm_rx_status_rxsc_sgien_exintfflag *prxsc;
u8 *prxpkt;
u8 i, max_spatial_stream, tmp_rxsnr, tmp_rxevm, rxsc_sgien_exflg;
- char rx_pwr[4], rx_pwr_all = 0;
- char rx_snrX, rx_evmX;
+ s8 rx_pwr[4], rx_pwr_all = 0;
+ s8 rx_snrX, rx_evmX;
u8 evm, pwdb_all;
u32 RSSI, total_rssi = 0;
u8 is_cck_rate = 0;
@@ -4423,7 +4423,7 @@ static void rtl8192_query_rxphystatus(struct r8192_priv *priv,

/* Get Rx snr value in DB */
tmp_rxsnr = pofdm_buf->rxsnr_X[i];
- rx_snrX = (char)(tmp_rxsnr);
+ rx_snrX = (s8)(tmp_rxsnr);
rx_snrX /= 2;
priv->stats.rxSNRdB[i] = (long)rx_snrX;

@@ -4457,7 +4457,7 @@ static void rtl8192_query_rxphystatus(struct r8192_priv *priv,

for (i = 0; i < max_spatial_stream; i++) {
tmp_rxevm = pofdm_buf->rxevm_X[i];
- rx_evmX = (char)(tmp_rxevm);
+ rx_evmX = (s8)(tmp_rxevm);

/* Do not use shift operation like "rx_evmX >>= 1"
* because the compiler of free build environment will
--
2.9.0



2016-07-20 15:26:34

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/3] staging/rtl8192e: use s8 instead of char

Compiling the rtlwifi drivers for ARM with gcc -Wextra warns about lots of
incorrect code that results from 'char' being unsigned here, e.g.

staging/rtl8192e/rtl8192e/r8192E_phy.c:1072:36: error: comparison is always false due to limited range of data type [-Werror=type-limits]
staging/rtl8192e/rtl8192e/r8192E_phy.c:1104:36: error: comparison is always false due to limited range of data type [-Werror=type-limits]
staging/rtl8192e/rtl8192e/rtl_core.c:1987:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
staging/rtl8192e/rtl8192e/rtl_dm.c:782:37: error: comparison is always false due to limited range of data type [-Werror=type-limits]
staging/rtl8192e/rtllib_softmac_wx.c:465:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]

This patch changes all uses of 'char' in this driver that refer to
8-bit integers to use 's8' instead, which is signed on all architectures.

Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Jes Sorensen <[email protected]>
---
drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c | 8 ++++----
drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c | 2 +-
drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 6 +++---
drivers/staging/rtl8192e/rtl8192e/rtl_core.h | 8 ++++----
4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
index ba64a4f1b3a8..8d6bca61e7aa 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
@@ -1487,8 +1487,8 @@ static void _rtl92e_query_rxphystatus(
struct phy_ofdm_rx_status_rxsc_sgien_exintfflag *prxsc;
u8 *prxpkt;
u8 i, max_spatial_stream, tmp_rxsnr, tmp_rxevm, rxsc_sgien_exflg;
- char rx_pwr[4], rx_pwr_all = 0;
- char rx_snrX, rx_evmX;
+ s8 rx_pwr[4], rx_pwr_all = 0;
+ s8 rx_snrX, rx_evmX;
u8 evm, pwdb_all;
u32 RSSI, total_rssi = 0;
u8 is_cck_rate = 0;
@@ -1613,7 +1613,7 @@ static void _rtl92e_query_rxphystatus(
2) - 110;

tmp_rxsnr = pofdm_buf->rxsnr_X[i];
- rx_snrX = (char)(tmp_rxsnr);
+ rx_snrX = (s8)(tmp_rxsnr);
rx_snrX /= 2;
priv->stats.rxSNRdB[i] = (long)rx_snrX;

@@ -1643,7 +1643,7 @@ static void _rtl92e_query_rxphystatus(

for (i = 0; i < max_spatial_stream; i++) {
tmp_rxevm = pofdm_buf->rxevm_X[i];
- rx_evmX = (char)(tmp_rxevm);
+ rx_evmX = (s8)(tmp_rxevm);

rx_evmX /= 2;

diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
index 5e3bbe5c3ca4..0698131e4300 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
@@ -630,7 +630,7 @@ void rtl92e_set_tx_power(struct net_device *dev, u8 channel)
{
struct r8192_priv *priv = rtllib_priv(dev);
u8 powerlevel = 0, powerlevelOFDM24G = 0;
- char ant_pwr_diff;
+ s8 ant_pwr_diff;
u32 u4RegValue;

if (priv->epromtype == EEPROM_93C46) {
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 13a5ddc2bea5..41e05f206300 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -1982,7 +1982,7 @@ void rtl92e_update_rx_statistics(struct r8192_priv *priv,
weighting) / 6;
}

-u8 rtl92e_rx_db_to_percent(char antpower)
+u8 rtl92e_rx_db_to_percent(s8 antpower)
{
if ((antpower <= -100) || (antpower >= 20))
return 0;
@@ -1993,9 +1993,9 @@ u8 rtl92e_rx_db_to_percent(char antpower)

} /* QueryRxPwrPercentage */

-u8 rtl92e_evm_db_to_percent(char value)
+u8 rtl92e_evm_db_to_percent(s8 value)
{
- char ret_val;
+ s8 ret_val;

ret_val = value;

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.h b/drivers/staging/rtl8192e/rtl8192e/rtl_core.h
index f627fdc15a58..6921125c9d35 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.h
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.h
@@ -503,8 +503,8 @@ struct r8192_priv {
u32 Pwr_Track;
u8 CCKPresentAttentuation_20Mdefault;
u8 CCKPresentAttentuation_40Mdefault;
- char CCKPresentAttentuation_difference;
- char CCKPresentAttentuation;
+ s8 CCKPresentAttentuation_difference;
+ s8 CCKPresentAttentuation;
long undecorated_smoothed_pwdb;

u32 MCSTxPowerLevelOriginalOffset[6];
@@ -604,8 +604,8 @@ void rtl92e_update_rx_pkt_timestamp(struct net_device *dev,
long rtl92e_translate_to_dbm(struct r8192_priv *priv, u8 signal_strength_index);
void rtl92e_update_rx_statistics(struct r8192_priv *priv,
struct rtllib_rx_stats *pprevious_stats);
-u8 rtl92e_evm_db_to_percent(char value);
-u8 rtl92e_rx_db_to_percent(char antpower);
+u8 rtl92e_evm_db_to_percent(s8 value);
+u8 rtl92e_rx_db_to_percent(s8 antpower);
void rtl92e_copy_mpdu_stats(struct rtllib_rx_stats *psrc_stats,
struct rtllib_rx_stats *ptarget_stats);
bool rtl92e_enable_nic(struct net_device *dev);
--
2.9.0


2016-07-20 15:48:30

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char

On Wednesday, July 20, 2016 5:06:27 PM CEST Xose Vazquez Perez wrote:
> Arnd Bergmann <[email protected]> wrote:
>
> > rtlwifi, but I found the older r8712u device to work fine with
> > the staging/rtl8712 driver.
>
> A replacement for "staging/rtl8712", with MAC80211 support, is
> available at: https://github.com/chunkeey/rtl8192su
>
> Also a fullmac/cfg80211 driver(r92su) is available at the
> same repository.
>
Yes, it has its problems. The rtl8712/r92su isn't really
a fullmac device. While the MLME (scan, probe, auth, assoc) is
done by the firmware. The 802.11 frame creation (from 802.3)
frames needs to be done by the driver. The rtl8712 firmware
however has its fair share of issues. Like: no support for
(tx) fragmentation and ERP is a mystery too. monitor
mode is barely working and limited to 20Mhz wide channels.
There's also limited tx injection support.... and of course
stability issues (just like with the staging/rtl8712 driver
or FreeBSD's rsu driver).

The rtl8192su driver (based on rtlwifi) in the same repository
has proper fragmentation support. But it uses the firmware from
the windows/mac os x driver, which is similar to rtl8192se softmac
firmware in design. Getting it to work properly in station mode
however either needs "some magic" or help from Realtek's USB group...



2016-07-20 15:41:37

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging/rtl8192e: avoid comparing unsigned type >= 0

Arnd Bergmann <[email protected]> writes:
> There is one remaining warning about a type limit check in rtl8192e:
>
> staging/rtl8192e/rtl819x_TSProc.c:326:14: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>
> This changes a macro into a local function to clarify the types and simplify
> the check while removing the warning.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/staging/rtl8192e/rtl819x_Qos.h | 3 ---
> drivers/staging/rtl8192e/rtl819x_TSProc.c | 5 +++++
> 2 files changed, 5 insertions(+), 3 deletions(-)

Looks good to me!

Acked-by: Jes Sorensen <[email protected]>

> diff --git a/drivers/staging/rtl8192e/rtl819x_Qos.h b/drivers/staging/rtl8192e/rtl819x_Qos.h
> index 463122db6d29..61da8f7475bb 100644
> --- a/drivers/staging/rtl8192e/rtl819x_Qos.h
> +++ b/drivers/staging/rtl8192e/rtl819x_Qos.h
> @@ -169,9 +169,6 @@ union qos_tclas {
> } TYPE2_8021Q;
> };
>
> -#define IsACValid(ac) ((ac >= 0 && ac <= 7) ? true : false)
> -
> -
> union aci_aifsn {
> u8 charData;
>
> diff --git a/drivers/staging/rtl8192e/rtl819x_TSProc.c b/drivers/staging/rtl8192e/rtl819x_TSProc.c
> index 2c8a526773ed..a966a8e490ab 100644
> --- a/drivers/staging/rtl8192e/rtl819x_TSProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_TSProc.c
> @@ -306,6 +306,11 @@ static void MakeTSEntry(struct ts_common_info *pTsCommonInfo, u8 *Addr,
> pTsCommonInfo->TClasNum = TCLAS_Num;
> }
>
> +static bool IsACValid(unsigned int tid)
> +{
> + return tid < 7;
> +}
> +
> bool GetTs(struct rtllib_device *ieee, struct ts_common_info **ppTS,
> u8 *Addr, u8 TID, enum tr_select TxRxSelect, bool bAddNewTs)
> {

2016-07-20 15:26:34

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 3/3] staging/rtl8192e: avoid comparing unsigned type >= 0

There is one remaining warning about a type limit check in rtl8192e:

staging/rtl8192e/rtl819x_TSProc.c:326:14: error: comparison is always true due to limited range of data type [-Werror=type-limits]

This changes a macro into a local function to clarify the types and simplify
the check while removing the warning.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/staging/rtl8192e/rtl819x_Qos.h | 3 ---
drivers/staging/rtl8192e/rtl819x_TSProc.c | 5 +++++
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl819x_Qos.h b/drivers/staging/rtl8192e/rtl819x_Qos.h
index 463122db6d29..61da8f7475bb 100644
--- a/drivers/staging/rtl8192e/rtl819x_Qos.h
+++ b/drivers/staging/rtl8192e/rtl819x_Qos.h
@@ -169,9 +169,6 @@ union qos_tclas {
} TYPE2_8021Q;
};

-#define IsACValid(ac) ((ac >= 0 && ac <= 7) ? true : false)
-
-
union aci_aifsn {
u8 charData;

diff --git a/drivers/staging/rtl8192e/rtl819x_TSProc.c b/drivers/staging/rtl8192e/rtl819x_TSProc.c
index 2c8a526773ed..a966a8e490ab 100644
--- a/drivers/staging/rtl8192e/rtl819x_TSProc.c
+++ b/drivers/staging/rtl8192e/rtl819x_TSProc.c
@@ -306,6 +306,11 @@ static void MakeTSEntry(struct ts_common_info *pTsCommonInfo, u8 *Addr,
pTsCommonInfo->TClasNum = TCLAS_Num;
}

+static bool IsACValid(unsigned int tid)
+{
+ return tid < 7;
+}
+
bool GetTs(struct rtllib_device *ieee, struct ts_common_info **ppTS,
u8 *Addr, u8 TID, enum tr_select TxRxSelect, bool bAddNewTs)
{
--
2.9.0


2016-07-22 20:52:00

by Stefan Lippers-Hollmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char

Hi

On 2016-07-22, Arnd Bergmann wrote:
> On Friday, July 22, 2016 7:55:36 AM CEST Jes Sorensen wrote:
> > Stefan Lippers-Hollmann <[email protected]> writes:
> > > On 2016-07-20, Arnd Bergmann wrote:
> > >> On Wednesday, July 20, 2016 11:33:43 AM CEST Jes Sorensen wrote:
> > >> > Arnd Bergmann <[email protected]> writes:
> > >> > > On Wednesday, July 20, 2016 7:25:19 AM CEST Jes Sorensen wrote:
> > >> > >> Arnd Bergmann <[email protected]> writes:
[...]
> > > While probably not overly common, it was/ is (hardware-wise) a pretty
> > > interesting device due to its support for 5 GHz[1] - actually I hoped
> > > it to be a (supported-) RTL8192CU variant when I bought it.
> > > Unfortunately no driver[2] made it to staging or the proper kernel.
> >
> > I actually have one of those in my USB dongle box, but as you say, not
> > overly common so not sure if/when I'll get to it.
> >
> > Adding 8192du support for 2.4GHz to rtl8xxxu probably wouldn't be too
> > complicated.
>
> My guess is that these devices have largely been replaced by
> 802.11ac devices on the market, and whoever has one of the old ones
> probably bought it because of the 5GHz support, so adding 2.4GHz-only
> support for it may not help all that much either.

Talking purely for myself, I've certainly bought the rtl8192du device
because of its 5 GHz support, as I've made it a fairly hard policy for
me not to buy 2.4 GHz only devices anymore. But this doesn't mean that
a mainline driver only supporting 2.4 GHz for the time being would not be
appreciated dearly, given that the current state of the device is pretty
much being a doorstop[1] - especially considering that 5 GHz support
might even become a possibility at a later time, when 802.11ac devices
start demanding most of the functionality for potentially more common
newer chipset generations.

So even with my current personal policy of only buying 5 GHz capable
devices, in practice you probably won't find 5 GHz only AP installations
(aside from long range/ outdoor point-to-point connections), be it
because of the plethora of existing 2.4 GHz only devices or just because
of the longer indoor (walls) range of the 2.4 GHz band. In practice, by
far most of my existing wireless devices don't support 5 GHz (the router
does, of course) because of the reasons mentioned above, but replacing
older devices takes its time.

Regards
Stefan Lippers-Hollmann

[1] yes, I know about https://github.com/lwfinger/rtl8192du/ and
even have a couple of clean-up patches pending[2] for the
kernel-version branch, but those need some further testing.
[2] http://aptosid.com/slh/rtl8192du/kernel-version/

2016-07-20 15:06:31

by Xose Vazquez Perez

[permalink] [raw]
Subject: Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char

Arnd Bergmann <[email protected]> wrote:

> rtlwifi, but I found the older r8712u device to work fine with
> the staging/rtl8712 driver.

A replacement for "staging/rtl8712", with MAC80211 support, is
available at: https://github.com/chunkeey/rtl8192su

Also a fullmac/cfg80211 driver(r92su) is available at the
same repository.