From: Luis R. Rodriguez <[email protected]>
Here's a slew of sparse warning fixes for all Atheros wireless drivers.
These have been itching me for a while now. I ask all patch submitters
and driver maintainers to please run their patches with these two tests:
make C=1 M=drivers/net/wireless/ath/
And then if all is happy try:
make C=2 CF="-D__CHECK_ENDIAN__" M=drivers/net/wireless/ath/
Luis R. Rodriguez (6):
ath6kl: fix sparse warning on init.c
ath9k: make ath_mci_duty_cycle static
ath9k_hw: fix sparse warnings on ar9003_rtt.c
ath9k: fix tx queue sparse complaint
ath5k: avoid sparse warnings on tracing
ath9k_hw: fix sparse complaint on ar9003_switch_com_spdt_get()
drivers/net/wireless/ath/ath5k/trace.h | 5 +++--
drivers/net/wireless/ath/ath6kl/init.c | 5 ++++-
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 4 ++--
drivers/net/wireless/ath/ath9k/ar9003_rtt.c | 1 +
drivers/net/wireless/ath/ath9k/mci.c | 2 +-
drivers/net/wireless/ath/ath9k/xmit.c | 3 +++
6 files changed, 14 insertions(+), 6 deletions(-)
--
1.7.4.15.g7811d
From: Luis R. Rodriguez <[email protected]>
This fixes this rant from sparse:
CHECK drivers/net/wireless/ath/ath9k/xmit.c
drivers/net/wireless/ath/ath9k/xmit.c:107:13: warning: context imbalance in 'ath_txq_lock' - wrong count at exit
drivers/net/wireless/ath/ath9k/xmit.c:112:13: warning: context imbalance in 'ath_txq_unlock' - unexpected unlock
drivers/net/wireless/ath/ath9k/xmit.c:123:30: warning: context imbalance in 'ath_txq_unlock_complete' - unexpected unlock
CC [M] drivers/net/wireless/ath/ath9k/xmit.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index b092523..c8fc180 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -105,16 +105,19 @@ static int ath_max_4ms_framelen[4][32] = {
/*********************/
static void ath_txq_lock(struct ath_softc *sc, struct ath_txq *txq)
+ __acquires(&txq->axq_lock)
{
spin_lock_bh(&txq->axq_lock);
}
static void ath_txq_unlock(struct ath_softc *sc, struct ath_txq *txq)
+ __releases(&txq->axq_lock)
{
spin_unlock_bh(&txq->axq_lock);
}
static void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
+ __releases(&txq->axq_lock)
{
struct sk_buff_head q;
struct sk_buff *skb;
--
1.7.4.15.g7811d
From: Luis R. Rodriguez <[email protected]>
This fixes this sparse warning:
CC [M] drivers/net/wireless/ath/ath6kl/init.o
drivers/net/wireless/ath/ath6kl/init.c: In function ‘ath6kl_init_hw_params’:
drivers/net/wireless/ath/ath6kl/init.c:1377:26: warning: ‘hw’ may be used uninitialized in this function
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath6kl/init.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 368ecbd..ef3641b 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -1374,7 +1374,7 @@ static int ath6kl_init_upload(struct ath6kl *ar)
static int ath6kl_init_hw_params(struct ath6kl *ar)
{
- const struct ath6kl_hw *hw;
+ const struct ath6kl_hw *hw = NULL;
int i;
for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
@@ -1390,6 +1390,9 @@ static int ath6kl_init_hw_params(struct ath6kl *ar)
return -EINVAL;
}
+ if (!hw)
+ return -EINVAL;
+
ar->hw = *hw;
ath6kl_dbg(ATH6KL_DBG_BOOT,
--
1.7.4.15.g7811d
On Tue, 2011-12-20 at 21:40 +0200, Kalle Valo wrote:
> On 12/20/2011 08:46 PM, Luis R. Rodriguez wrote:
> > From: Luis R. Rodriguez <[email protected]>
> > This fixes this sparse warning:
> > CC [M] drivers/net/wireless/ath/ath6kl/init.o
> > drivers/net/wireless/ath/ath6kl/init.c: In function ‘ath6kl_init_hw_params’:
> > drivers/net/wireless/ath/ath6kl/init.c:1377:26: warning: ‘hw’ may be used uninitialized in this function
> That's a compiler warning.
[]
> I can't see how hw can be uninitialised here (looking at the version in
> ath6kl.git). I copy the full code here:
> static int ath6kl_init_hw_params(struct ath6kl *ar)
> {
> const struct ath6kl_hw *hw;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> hw = &hw_list[i];
>
> if (hw->id == ar->version.target_ver)
> break;
> }
>
> if (i == ARRAY_SIZE(hw_list)) {
> ath6kl_err("Unsupported hardware version: 0x%x\n",
> ar->version.target_ver);
> return -EINVAL;
> }
>
> ar->hw = *hw;
>
> I always check for both compiler and sparse warnings and I have never
> seen this. What version of compiler do you have?
Is the intent here really to allow multiple ids
in the list and match the last one?
If not, perhaps something like this is simpler?
static int ath6kl_init_hw_params(struct ath6kl *ar)
{
int i;
const struct ath6kl_hw *hw = hw_list;
for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
if (hw->id == ar->version.target_ver) {
ar->hw = *hw;
ath6kl_dbg(ATH6KL_DBG_BOOT,
"target_ver 0x%x target_type 0x%x dataset_patch 0x%x app_load_addr 0x%x\n",
ar->version.target_ver, ar->target_type,
ar->hw.dataset_patch_addr,
ar->hw.app_load_addr);
ath6kl_dbg(ATH6KL_DBG_BOOT,
"app_start_override_addr 0x%x board_ext_data_addr 0x%x reserved_ram_size 0x%x\n",
ar->hw.app_start_override_addr,
ar->hw.board_ext_data_addr,
ar->hw.reserved_ram_size);
ath6kl_dbg(ATH6KL_DBG_BOOT,
"refclk_hz %d uarttx_pin %d\n",
ar->hw.refclk_hz, ar->hw.uarttx_pin);
return 0;
}
hw++;
}
ath6kl_err("Unsupported hardware version: 0x%x\n",
ar->version.target_ver);
return -EINVAL;
}
btw: there are missing terminating newlines in the
current ath6kl_dbg uses in this routine.
On 12/20/2011 12:46 PM, Luis R. Rodriguez wrote:
> From: Luis R. Rodriguez<[email protected]>
>
> Here's a slew of sparse warning fixes for all Atheros wireless drivers.
> These have been itching me for a while now. I ask all patch submitters
> and driver maintainers to please run their patches with these two tests:
>
> make C=1 M=drivers/net/wireless/ath/
>
> And then if all is happy try:
>
> make C=2 CF="-D__CHECK_ENDIAN__" M=drivers/net/wireless/ath/
Why not follow a suggestion that Johannes made to me? Add the line
ccflags-y += -D__CHECK_ENDIAN__
To the bottom of the make file(s). That way they only need one run. Then enforce
the rule by not ACKing any patches with too many of the endian warnings, and do
not allow any of the others.
Larry
On 12/21/2011 03:42 AM, Joe Perches wrote:
> On Tue, 2011-12-20 at 21:40 +0200, Kalle Valo wrote:
>> On 12/20/2011 08:46 PM, Luis R. Rodriguez wrote:
>>
>> for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
>> hw = &hw_list[i];
>>
>> if (hw->id == ar->version.target_ver)
>> break;
>> }
>>
>> if (i == ARRAY_SIZE(hw_list)) {
>> ath6kl_err("Unsupported hardware version: 0x%x\n",
>> ar->version.target_ver);
>> return -EINVAL;
>> }
>>
>> ar->hw = *hw;
>>
>> I always check for both compiler and sparse warnings and I have never
>> seen this. What version of compiler do you have?
>
> Is the intent here really to allow multiple ids
> in the list and match the last one?
The idea is that there should be just one hw entry per id. But if there
would be multiple entires, the code above would take the first one.
> If not, perhaps this is simpler?
>
> static int ath6kl_init_hw_params(struct ath6kl *ar)
> {
> int i;
> const struct ath6kl_hw *hw = hw_list;
>
> for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> if (hw->id == ar->version.target_ver) {
> ar->hw = *hw;
>
> ath6kl_dbg(ATH6KL_DBG_BOOT,
> "target_ver 0x%x target_type 0x%x dataset_patch 0x%x app_load_addr 0x%x\n",
> ar->version.target_ver, ar->target_type,
> ar->hw.dataset_patch_addr,
> ar->hw.app_load_addr);
> ath6kl_dbg(ATH6KL_DBG_BOOT,
> "app_start_override_addr 0x%x board_ext_data_addr 0x%x reserved_ram_size 0x%x\n",
> ar->hw.app_start_override_addr,
> ar->hw.board_ext_data_addr,
> ar->hw.reserved_ram_size);
> ath6kl_dbg(ATH6KL_DBG_BOOT,
> "refclk_hz %d uarttx_pin %d\n",
> ar->hw.refclk_hz, ar->hw.uarttx_pin);
>
> return 0;
> }
> hw++;
> }
>
> ath6kl_err("Unsupported hardware version: 0x%x\n",
> ar->version.target_ver);
> return -EINVAL;
> }
There's a lot of indentation in that form which I don't like. And I
would prefer to have the "main" (non-error) code path unindented. Easier
to read that way.
> btw: there are missing terminating newlines in the
> existing ath6kl_dbg uses in this routine.
Thanks, I didn't notice that. That needs to be fixed.
Kalle
From: Luis R. Rodriguez <[email protected]>
This fixes this sparse complaint:
make C=2 CF="-D__CHECK_ENDIAN__" M=drivers/net/wireless/ath/
CHECK drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:3544:21: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:3544:21: expected restricted __le32 [usertype] val
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:3544:21: got restricted __le16 [usertype] switchcomspdt
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:3546:21: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:3546:21: expected restricted __le32 [usertype] val
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:3546:21: got restricted __le16 [usertype] switchcomspdt
The eep->modalHeader5G.switchcomspdt is a le16 and we return u16,
so just return le16_to_cpu().
Cc: Felix Fietkau <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 391def9..9fbcbdd 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -3538,13 +3538,13 @@ static void ar9003_hw_xpa_bias_level_apply(struct ath_hw *ah, bool is2ghz)
static u16 ar9003_switch_com_spdt_get(struct ath_hw *ah, bool is_2ghz)
{
struct ar9300_eeprom *eep = &ah->eeprom.ar9300_eep;
- __le32 val;
+ __le16 val;
if (is_2ghz)
val = eep->modalHeader2G.switchcomspdt;
else
val = eep->modalHeader5G.switchcomspdt;
- return le32_to_cpu(val);
+ return le16_to_cpu(val);
}
--
1.7.4.15.g7811d
On Tue, 2011-12-20 at 21:40 +0200, Kalle Valo wrote:
> On 12/20/2011 08:46 PM, Luis R. Rodriguez wrote:
> > From: Luis R. Rodriguez <[email protected]>
> > This fixes this sparse warning:
> > CC [M] drivers/net/wireless/ath/ath6kl/init.o
> > drivers/net/wireless/ath/ath6kl/init.c: In function ‘ath6kl_init_hw_params’:
> > drivers/net/wireless/ath/ath6kl/init.c:1377:26: warning: ‘hw’ may be used uninitialized in this function
> That's a compiler warning.
[]
> I can't see how hw can be uninitialised here (looking at the version in
> ath6kl.git). I copy the full code here:
> static int ath6kl_init_hw_params(struct ath6kl *ar)
> {
> const struct ath6kl_hw *hw;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> hw = &hw_list[i];
>
> if (hw->id == ar->version.target_ver)
> break;
> }
>
> if (i == ARRAY_SIZE(hw_list)) {
> ath6kl_err("Unsupported hardware version: 0x%x\n",
> ar->version.target_ver);
> return -EINVAL;
> }
>
> ar->hw = *hw;
>
> I always check for both compiler and sparse warnings and I have never
> seen this. What version of compiler do you have?
Is the intent here really to allow multiple ids
in the list and match the last one?
If not, perhaps this is simpler?
static int ath6kl_init_hw_params(struct ath6kl *ar)
{
int i;
const struct ath6kl_hw *hw = hw_list;
for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
if (hw->id == ar->version.target_ver) {
ar->hw = *hw;
ath6kl_dbg(ATH6KL_DBG_BOOT,
"target_ver 0x%x target_type 0x%x dataset_patch 0x%x app_load_addr 0x%x\n",
ar->version.target_ver, ar->target_type,
ar->hw.dataset_patch_addr,
ar->hw.app_load_addr);
ath6kl_dbg(ATH6KL_DBG_BOOT,
"app_start_override_addr 0x%x board_ext_data_addr 0x%x reserved_ram_size 0x%x\n",
ar->hw.app_start_override_addr,
ar->hw.board_ext_data_addr,
ar->hw.reserved_ram_size);
ath6kl_dbg(ATH6KL_DBG_BOOT,
"refclk_hz %d uarttx_pin %d\n",
ar->hw.refclk_hz, ar->hw.uarttx_pin);
return 0;
}
hw++;
}
ath6kl_err("Unsupported hardware version: 0x%x\n",
ar->version.target_ver);
return -EINVAL;
}
btw: there are missing terminating newlines in the
existing ath6kl_dbg uses in this routine.
On Tue, Dec 20, 2011 at 10:46 AM, Luis R. Rodriguez
<[email protected]> wrote:
> From: Luis R. Rodriguez <[email protected]>
>
> This fixes this sparse warning:
>
> CC [M] drivers/net/wireless/ath/ath6kl/init.o
> drivers/net/wireless/ath/ath6kl/init.c: In function ‘ath6kl_init_hw_params’:
> drivers/net/wireless/ath/ath6kl/init.c:1377:26: warning: ‘hw’ may be used uninitialized in this function
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
Not sure if Kalle will suck this in or if John will.
Luis
On Tue, Dec 20, 2011 at 09:40:19PM +0200, Kalle Valo wrote:
> On 12/20/2011 08:46 PM, Luis R. Rodriguez wrote:
> > From: Luis R. Rodriguez <[email protected]>
> >
> > This fixes this sparse warning:
> >
> > CC [M] drivers/net/wireless/ath/ath6kl/init.o
> > drivers/net/wireless/ath/ath6kl/init.c: In function ‘ath6kl_init_hw_params’:
> > drivers/net/wireless/ath/ath6kl/init.c:1377:26: warning: ‘hw’ may be used uninitialized in this function
>
> That's a compiler warning.
>
> > static int ath6kl_init_hw_params(struct ath6kl *ar)
> > {
> > - const struct ath6kl_hw *hw;
> > + const struct ath6kl_hw *hw = NULL;
> > int i;
> >
> > for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> > @@ -1390,6 +1390,9 @@ static int ath6kl_init_hw_params(struct ath6kl *ar)
> > return -EINVAL;
> > }
> >
> > + if (!hw)
> > + return -EINVAL;
>
> I can't see how hw can be uninitialised here (looking at the version in
> ath6kl.git). I copy the full code here:
>
> static int ath6kl_init_hw_params(struct ath6kl *ar)
> {
> const struct ath6kl_hw *hw;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> hw = &hw_list[i];
>
> if (hw->id == ar->version.target_ver)
> break;
> }
>
> if (i == ARRAY_SIZE(hw_list)) {
> ath6kl_err("Unsupported hardware version: 0x%x\n",
> ar->version.target_ver);
> return -EINVAL;
> }
>
> ar->hw = *hw;
>
> I always check for both compiler and sparse warnings and I have never
> seen this. What version of compiler do you have?
>
> I have:
>
> gcc-4.5.real (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
Same.
mcgrof@tux ~ $ gcc --version
gcc-4.5.real (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE
Luis
From: Luis R. Rodriguez <[email protected]>
This fixes this sparse warning:
CHECK drivers/net/wireless/ath/ath9k/mci.c
drivers/net/wireless/ath/ath9k/mci.c:23:4: warning: symbol 'ath_mci_duty_cycle' was not declared. Should it be static?
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath9k/mci.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/mci.c b/drivers/net/wireless/ath/ath9k/mci.c
index fee8c6f..15629f7 100644
--- a/drivers/net/wireless/ath/ath9k/mci.c
+++ b/drivers/net/wireless/ath/ath9k/mci.c
@@ -20,7 +20,7 @@
#include "ath9k.h"
#include "mci.h"
-u8 ath_mci_duty_cycle[] = { 0, 50, 60, 70, 80, 85, 90, 95, 98 };
+static u8 ath_mci_duty_cycle[] = { 0, 50, 60, 70, 80, 85, 90, 95, 98 };
static struct ath_mci_profile_info*
ath_mci_find_profile(struct ath_mci_profile *mci,
--
1.7.4.15.g7811d
On Tue, 2011-12-20 at 10:46 -0800, Luis R. Rodriguez wrote:
> diff --git a/drivers/net/wireless/ath/ath9k/mci.c b/drivers/net/wireless/ath/ath9k/mci.c
[]
> @@ -20,7 +20,7 @@
> #include "ath9k.h"
> #include "mci.h"
>
> -u8 ath_mci_duty_cycle[] = { 0, 50, 60, 70, 80, 85, 90, 95, 98 };
> +static u8 ath_mci_duty_cycle[] = { 0, 50, 60, 70, 80, 85, 90, 95, 98 };
static const would be better.
On 12/20/2011 09:46 PM, Luis R. Rodriguez wrote:
> On Tue, Dec 20, 2011 at 09:40:19PM +0200, Kalle Valo wrote:
>> I always check for both compiler and sparse warnings and I have never
>> seen this. What version of compiler do you have?
>>
>> I have:
>>
>> gcc-4.5.real (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
>
> Same.
>
> mcgrof@tux ~ $ gcc --version
> gcc-4.5.real (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
This is odd as I don't see it, even with latest wireless-testing:
$ make M=drivers/net/wireless/ath/ath6kl
LD drivers/net/wireless/ath/ath6kl/built-in.o
CC [M] drivers/net/wireless/ath/ath6kl/debug.o
CC [M] drivers/net/wireless/ath/ath6kl/hif.o
CC [M] drivers/net/wireless/ath/ath6kl/htc.o
CC [M] drivers/net/wireless/ath/ath6kl/bmi.o
CC [M] drivers/net/wireless/ath/ath6kl/cfg80211.o
CC [M] drivers/net/wireless/ath/ath6kl/init.o
CC [M] drivers/net/wireless/ath/ath6kl/main.o
CC [M] drivers/net/wireless/ath/ath6kl/txrx.o
CC [M] drivers/net/wireless/ath/ath6kl/wmi.o
CC [M] drivers/net/wireless/ath/ath6kl/sdio.o
CC [M] drivers/net/wireless/ath/ath6kl/testmode.o
CC [M] drivers/net/wireless/ath/ath6kl/usb.o
LD [M] drivers/net/wireless/ath/ath6kl/ath6kl_sdio.o
LD [M] drivers/net/wireless/ath/ath6kl/ath6kl_usb.o
Building modules, stage 2.
MODPOST 2 modules
CC drivers/net/wireless/ath/ath6kl/ath6kl_sdio.mod.o
LD [M] drivers/net/wireless/ath/ath6kl/ath6kl_sdio.ko
CC drivers/net/wireless/ath/ath6kl/ath6kl_usb.mod.o
LD [M] drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko
I really would like to understand what's happening here...
But anyway, I think your test is not needed (unless I'm blind again).
Maybe uninitialized_var() is a better choise?
Kalle
From: Luis R. Rodriguez <[email protected]>
This fixes these sparse warnings:
CHECK drivers/net/wireless/ath/ath9k/ar9003_rtt.c
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:36:6: warning: symbol 'ar9003_hw_rtt_enable' was not declared. Should it be static?
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:41:6: warning: symbol 'ar9003_hw_rtt_disable' was not declared. Should it be static?
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:46:6: warning: symbol 'ar9003_hw_rtt_set_mask' was not declared. Should it be static?
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:52:6: warning: symbol 'ar9003_hw_rtt_force_restore' was not declared. Should it be static?
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:102:6: warning: symbol 'ar9003_hw_rtt_load_hist' was not declared. Should it be static?
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:135:6: warning: symbol 'ar9003_hw_rtt_fill_hist' was not declared. Should it be static?
drivers/net/wireless/ath/ath9k/ar9003_rtt.c:143:6: warning: symbol 'ar9003_hw_rtt_clear_hist' was not declared. Should it be stati
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath9k/ar9003_rtt.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_rtt.c b/drivers/net/wireless/ath/ath9k/ar9003_rtt.c
index 48803ee..458bedf 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_rtt.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_rtt.c
@@ -16,6 +16,7 @@
#include "hw.h"
#include "ar9003_phy.h"
+#include "ar9003_rtt.h"
#define RTT_RESTORE_TIMEOUT 1000
#define RTT_ACCESS_TIMEOUT 100
--
1.7.4.15.g7811d
On 12/20/2011 08:46 PM, Luis R. Rodriguez wrote:
> From: Luis R. Rodriguez <[email protected]>
>
> This fixes this sparse warning:
>
> CC [M] drivers/net/wireless/ath/ath6kl/init.o
> drivers/net/wireless/ath/ath6kl/init.c: In function ‘ath6kl_init_hw_params’:
> drivers/net/wireless/ath/ath6kl/init.c:1377:26: warning: ‘hw’ may be used uninitialized in this function
That's a compiler warning.
> static int ath6kl_init_hw_params(struct ath6kl *ar)
> {
> - const struct ath6kl_hw *hw;
> + const struct ath6kl_hw *hw = NULL;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> @@ -1390,6 +1390,9 @@ static int ath6kl_init_hw_params(struct ath6kl *ar)
> return -EINVAL;
> }
>
> + if (!hw)
> + return -EINVAL;
I can't see how hw can be uninitialised here (looking at the version in
ath6kl.git). I copy the full code here:
static int ath6kl_init_hw_params(struct ath6kl *ar)
{
const struct ath6kl_hw *hw;
int i;
for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
hw = &hw_list[i];
if (hw->id == ar->version.target_ver)
break;
}
if (i == ARRAY_SIZE(hw_list)) {
ath6kl_err("Unsupported hardware version: 0x%x\n",
ar->version.target_ver);
return -EINVAL;
}
ar->hw = *hw;
I always check for both compiler and sparse warnings and I have never
seen this. What version of compiler do you have?
I have:
gcc-4.5.real (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
Kalle
On Tue, 2011-12-20 at 10:46 -0800, Luis R. Rodriguez wrote:
> Just skip the sparse checks on tracing.
I seem to remember removing that elsewhere since newer sparse or newer
tracing code worked fine together.
johannes
On Tue, Dec 20, 2011 at 01:01:36PM -0600, Larry Finger wrote:
> On 12/20/2011 12:46 PM, Luis R. Rodriguez wrote:
> >From: Luis R. Rodriguez<[email protected]>
> >
> >Here's a slew of sparse warning fixes for all Atheros wireless drivers.
> >These have been itching me for a while now. I ask all patch submitters
> >and driver maintainers to please run their patches with these two tests:
> >
> >make C=1 M=drivers/net/wireless/ath/
> >
> >And then if all is happy try:
> >
> >make C=2 CF="-D__CHECK_ENDIAN__" M=drivers/net/wireless/ath/
>
> Why not follow a suggestion that Johannes made to me? Add the line
>
> ccflags-y += -D__CHECK_ENDIAN__
>
> To the bottom of the make file(s). That way they only need one run.
> Then enforce the rule by not ACKing any patches with too many of the
> endian warnings, and do not allow any of the others.
Awesome idea.
Luis
From: Luis R. Rodriguez <[email protected]>
Just skip the sparse checks on tracing.
CHECK drivers/net/wireless/ath/ath5k/base.c
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:19:1: error: incompatible types for operation (<)
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:19:1: left side has type struct ath5k_hw *<noident>
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:19:1: right side has type int
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:37:1: error: incompatible types for operation (<)
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:37:1: left side has type struct ath5k_hw *<noident>
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:37:1: right side has type int
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:63:1: error: incompatible types for operation (<)
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:63:1: left side has type struct ath5k_hw *<noident>
include/trace/../../drivers/net/wireless/ath/ath5k/trace.h:63:1: right side has type int
/home/mcgrof/wireless-testing/arch/x86/include/asm/jump_label.h:16:9: error: bad asm output
/home/mcgrof/wireless-testing/arch/x86/include/asm/jump_label.h:16:9: error: bad asm output
/home/mcgrof/wireless-testing/arch/x86/include/asm/jump_label.h:16:9: error: bad asm output
/home/mcgrof/wireless-testing/arch/x86/include/asm/jump_label.h:16:9: error: bad asm output
CC [M] drivers/net/wireless/ath/ath5k/base.o
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath5k/trace.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/trace.h b/drivers/net/wireless/ath/ath5k/trace.h
index 39f002e..00f0158 100644
--- a/drivers/net/wireless/ath/ath5k/trace.h
+++ b/drivers/net/wireless/ath/ath5k/trace.h
@@ -3,7 +3,8 @@
#include <linux/tracepoint.h>
-#ifndef CONFIG_ATH5K_TRACER
+
+#if !defined(CONFIG_ATH5K_TRACER) || defined(__CHECKER__)
#undef TRACE_EVENT
#define TRACE_EVENT(name, proto, ...) \
static inline void trace_ ## name(proto) {}
@@ -93,7 +94,7 @@ TRACE_EVENT(ath5k_tx_complete,
#endif /* __TRACE_ATH5K_H */
-#ifdef CONFIG_ATH5K_TRACER
+#if defined(CONFIG_ATH5K_TRACER) && !defined(__CHECKER__)
#undef TRACE_INCLUDE_PATH
#define TRACE_INCLUDE_PATH ../../drivers/net/wireless/ath/ath5k
--
1.7.4.15.g7811d
On Tue, Dec 20, 2011 at 8:41 PM, Joe Perches <[email protected]> wrote:
> On Tue, 2011-12-20 at 10:46 -0800, Luis R. Rodriguez wrote:
>> diff --git a/drivers/net/wireless/ath/ath9k/mci.c b/drivers/net/wireless/ath/ath9k/mci.c
> []
>> @@ -20,7 +20,7 @@
>> #include "ath9k.h"
>> #include "mci.h"
>>
>> -u8 ath_mci_duty_cycle[] = { 0, 50, 60, 70, 80, 85, 90, 95, 98 };
>> +static u8 ath_mci_duty_cycle[] = { 0, 50, 60, 70, 80, 85, 90, 95, 98 };
>
> static const would be better.
Good idea, v2 coming up.
Luis
On Wed, Dec 21, 2011 at 7:15 AM, Johannes Berg
<[email protected]> wrote:
> On Tue, 2011-12-20 at 10:46 -0800, Luis R. Rodriguez wrote:
>
>> Just skip the sparse checks on tracing.
>
> I seem to remember removing that elsewhere since newer sparse or newer
> tracing code worked fine together.
I got the complaint from sparse v0.4.3 and saw that iwlwifi, mac80211
used the same technique.
Luis