* [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
@ 2011-12-20 18:46 ` Luis R. Rodriguez
2011-12-20 18:49 ` Luis R. Rodriguez
2011-12-20 19:40 ` Kalle Valo
2011-12-20 18:46 ` [PATCH 2/6] ath9k: make ath_mci_duty_cycle static Luis R. Rodriguez
` (5 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
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 <mcgrof@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 18:46 ` [PATCH 1/6] ath6kl: fix sparse warning on init.c Luis R. Rodriguez
@ 2011-12-20 18:49 ` Luis R. Rodriguez
2011-12-20 19:40 ` Kalle Valo
1 sibling, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:49 UTC (permalink / raw)
To: linville, Kalle Valo; +Cc: linux-wireless, Luis R. Rodriguez
On Tue, Dec 20, 2011 at 10:46 AM, Luis R. Rodriguez
<rodrigue@qca.qualcomm.com> wrote:
> From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
>
> 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 <mcgrof@qca.qualcomm.com>
Not sure if Kalle will suck this in or if John will.
Luis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 18:46 ` [PATCH 1/6] ath6kl: fix sparse warning on init.c Luis R. Rodriguez
2011-12-20 18:49 ` Luis R. Rodriguez
@ 2011-12-20 19:40 ` Kalle Valo
2011-12-20 19:46 ` Luis R. Rodriguez
` (2 more replies)
1 sibling, 3 replies; 20+ messages in thread
From: Kalle Valo @ 2011-12-20 19:40 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
On 12/20/2011 08:46 PM, Luis R. Rodriguez wrote:
> From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
>
> 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
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 19:40 ` Kalle Valo
@ 2011-12-20 19:46 ` Luis R. Rodriguez
2011-12-20 20:02 ` Kalle Valo
2011-12-21 1:41 ` Joe Perches
2011-12-21 1:42 ` Joe Perches
2 siblings, 1 reply; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 19:46 UTC (permalink / raw)
To: Kalle Valo; +Cc: linville, linux-wireless
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 <mcgrof@qca.qualcomm.com>
> >
> > 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
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 19:46 ` Luis R. Rodriguez
@ 2011-12-20 20:02 ` Kalle Valo
0 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2011-12-20 20:02 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 19:40 ` Kalle Valo
2011-12-20 19:46 ` Luis R. Rodriguez
@ 2011-12-21 1:41 ` Joe Perches
2011-12-21 1:42 ` Joe Perches
2 siblings, 0 replies; 20+ messages in thread
From: Joe Perches @ 2011-12-21 1:41 UTC (permalink / raw)
To: Kalle Valo; +Cc: Luis R. Rodriguez, linville, linux-wireless
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 <mcgrof@qca.qualcomm.com>
> > 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.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-20 19:40 ` Kalle Valo
2011-12-20 19:46 ` Luis R. Rodriguez
2011-12-21 1:41 ` Joe Perches
@ 2011-12-21 1:42 ` Joe Perches
2011-12-23 8:27 ` Kalle Valo
2 siblings, 1 reply; 20+ messages in thread
From: Joe Perches @ 2011-12-21 1:42 UTC (permalink / raw)
To: Kalle Valo; +Cc: Luis R. Rodriguez, linville, linux-wireless
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 <mcgrof@qca.qualcomm.com>
> > 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.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] ath6kl: fix sparse warning on init.c
2011-12-21 1:42 ` Joe Perches
@ 2011-12-23 8:27 ` Kalle Valo
0 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2011-12-23 8:27 UTC (permalink / raw)
To: Joe Perches; +Cc: Luis R. Rodriguez, linville, linux-wireless
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/6] ath9k: make ath_mci_duty_cycle static
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
2011-12-20 18:46 ` [PATCH 1/6] ath6kl: fix sparse warning on init.c Luis R. Rodriguez
@ 2011-12-20 18:46 ` Luis R. Rodriguez
2011-12-21 4:41 ` Joe Perches
2011-12-20 18:46 ` [PATCH 3/6] ath9k_hw: fix sparse warnings on ar9003_rtt.c Luis R. Rodriguez
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
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 <mcgrof@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/6] ath9k: make ath_mci_duty_cycle static
2011-12-20 18:46 ` [PATCH 2/6] ath9k: make ath_mci_duty_cycle static Luis R. Rodriguez
@ 2011-12-21 4:41 ` Joe Perches
2011-12-21 17:26 ` Luis R. Rodriguez
0 siblings, 1 reply; 20+ messages in thread
From: Joe Perches @ 2011-12-21 4:41 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
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.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/6] ath9k: make ath_mci_duty_cycle static
2011-12-21 4:41 ` Joe Perches
@ 2011-12-21 17:26 ` Luis R. Rodriguez
0 siblings, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-21 17:26 UTC (permalink / raw)
To: Joe Perches; +Cc: linville, linux-wireless
On Tue, Dec 20, 2011 at 8:41 PM, Joe Perches <joe@perches.com> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/6] ath9k_hw: fix sparse warnings on ar9003_rtt.c
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
2011-12-20 18:46 ` [PATCH 1/6] ath6kl: fix sparse warning on init.c Luis R. Rodriguez
2011-12-20 18:46 ` [PATCH 2/6] ath9k: make ath_mci_duty_cycle static Luis R. Rodriguez
@ 2011-12-20 18:46 ` Luis R. Rodriguez
2011-12-20 18:46 ` [PATCH 4/6] ath9k: fix tx queue sparse complaint Luis R. Rodriguez
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
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 <mcgrof@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 4/6] ath9k: fix tx queue sparse complaint
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
` (2 preceding siblings ...)
2011-12-20 18:46 ` [PATCH 3/6] ath9k_hw: fix sparse warnings on ar9003_rtt.c Luis R. Rodriguez
@ 2011-12-20 18:46 ` Luis R. Rodriguez
2011-12-20 18:46 ` [PATCH 5/6] ath5k: avoid sparse warnings on tracing Luis R. Rodriguez
` (2 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
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 <mcgrof@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 5/6] ath5k: avoid sparse warnings on tracing
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
` (3 preceding siblings ...)
2011-12-20 18:46 ` [PATCH 4/6] ath9k: fix tx queue sparse complaint Luis R. Rodriguez
@ 2011-12-20 18:46 ` Luis R. Rodriguez
2011-12-21 15:15 ` Johannes Berg
2011-12-20 18:46 ` [PATCH 6/6] ath9k_hw: fix sparse complaint on ar9003_switch_com_spdt_get() Luis R. Rodriguez
2011-12-20 19:01 ` [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Larry Finger
6 siblings, 1 reply; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
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 <mcgrof@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/6] ath5k: avoid sparse warnings on tracing
2011-12-20 18:46 ` [PATCH 5/6] ath5k: avoid sparse warnings on tracing Luis R. Rodriguez
@ 2011-12-21 15:15 ` Johannes Berg
2011-12-21 16:04 ` Luis R. Rodriguez
0 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2011-12-21 15:15 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/6] ath5k: avoid sparse warnings on tracing
2011-12-21 15:15 ` Johannes Berg
@ 2011-12-21 16:04 ` Luis R. Rodriguez
0 siblings, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-21 16:04 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
On Wed, Dec 21, 2011 at 7:15 AM, Johannes Berg
<johannes@sipsolutions.net> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/6] ath9k_hw: fix sparse complaint on ar9003_switch_com_spdt_get()
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
` (4 preceding siblings ...)
2011-12-20 18:46 ` [PATCH 5/6] ath5k: avoid sparse warnings on tracing Luis R. Rodriguez
@ 2011-12-20 18:46 ` Luis R. Rodriguez
2011-12-20 19:01 ` [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Larry Finger
6 siblings, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 18:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez, Felix Fietkau
From: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
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 <nbd@openwrt.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho
2011-12-20 18:46 [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Luis R. Rodriguez
` (5 preceding siblings ...)
2011-12-20 18:46 ` [PATCH 6/6] ath9k_hw: fix sparse complaint on ar9003_switch_com_spdt_get() Luis R. Rodriguez
@ 2011-12-20 19:01 ` Larry Finger
2011-12-20 19:12 ` Luis R. Rodriguez
6 siblings, 1 reply; 20+ messages in thread
From: Larry Finger @ 2011-12-20 19:01 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
On 12/20/2011 12:46 PM, Luis R. Rodriguez wrote:
> From: Luis R. Rodriguez<mcgrof@qca.qualcomm.com>
>
> 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
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho
2011-12-20 19:01 ` [PATCH 0/6] atheros-wireless: fix sparse warnings ho ho ho Larry Finger
@ 2011-12-20 19:12 ` Luis R. Rodriguez
0 siblings, 0 replies; 20+ messages in thread
From: Luis R. Rodriguez @ 2011-12-20 19:12 UTC (permalink / raw)
To: Larry Finger; +Cc: linville, linux-wireless
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<mcgrof@qca.qualcomm.com>
> >
> >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
^ permalink raw reply [flat|nested] 20+ messages in thread