2012-10-10 18:13:22

by Franky Lin

[permalink] [raw]
Subject: [PATCH 0/7] miscellaneous fixes intended for 3.7

Here are some miscellaneous brcmfmac bug fix patches aiming for 3.7

Arend van Spriel (1):
brcmfmac: remove 'always false' condition from brcmf_c_mkiovar_bsscfg

Dan Carpenter (2):
brcmfmac: Using zero instead of NULL
brcmfmac: fix end of loop check (signedness bug)

Franky Lin (3):
brcmfmac: fix sparse warnings
brcmfmac: use control channel in roamed status reporting
brcmfmac: set dongle mode accordingly when interface up

Hante Meuleman (1):
brcmfmac: handle all exceptions as an error.

.../net/wireless/brcm80211/brcmfmac/dhd_common.c | 2 +-
drivers/net/wireless/brcm80211/brcmfmac/usb.c | 2 +-
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 77 +++++++-------------
3 files changed, 30 insertions(+), 51 deletions(-)

--
1.7.9.5




2012-10-10 18:13:22

by Franky Lin

[permalink] [raw]
Subject: [PATCH 5/7] brcmfmac: set dongle mode accordingly when interface up

The mode of WiFi dongle should be initialized in brcmf_cfg80211_up
which get called when network interface is brought up. Otherwise
brcmf_cfg80211_get_station would return error.

Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 38 ++------------------
1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index a39925a..5e51f15 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -5199,41 +5199,6 @@ brcmf_cfg80211_event(struct net_device *ndev,
schedule_work(&cfg->event_work);
}

-static s32 brcmf_dongle_mode(struct net_device *ndev, s32 iftype)
-{
- s32 infra = 0;
- s32 err = 0;
-
- switch (iftype) {
- case NL80211_IFTYPE_MONITOR:
- case NL80211_IFTYPE_WDS:
- WL_ERR("type (%d) : currently we do not support this mode\n",
- iftype);
- err = -EINVAL;
- return err;
- case NL80211_IFTYPE_ADHOC:
- infra = 0;
- break;
- case NL80211_IFTYPE_STATION:
- infra = 1;
- break;
- case NL80211_IFTYPE_AP:
- infra = 1;
- break;
- default:
- err = -EINVAL;
- WL_ERR("invalid type (%d)\n", iftype);
- return err;
- }
- err = brcmf_exec_dcmd_u32(ndev, BRCMF_C_SET_INFRA, &infra);
- if (err) {
- WL_ERR("WLC_SET_INFRA error (%d)\n", err);
- return err;
- }
-
- return 0;
-}
-
static s32 brcmf_dongle_eventmsg(struct net_device *ndev)
{
/* Room for "event_msgs" + '\0' + bitvec */
@@ -5452,7 +5417,8 @@ static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
WL_BEACON_TIMEOUT);
if (err)
goto default_conf_out;
- err = brcmf_dongle_mode(ndev, wdev->iftype);
+ err = brcmf_cfg80211_change_iface(wdev->wiphy, ndev, wdev->iftype,
+ NULL, NULL);
if (err && err != -EINPROGRESS)
goto default_conf_out;
err = brcmf_dongle_probecap(cfg);
--
1.7.9.5



2012-10-15 19:06:59

by Franky Lin

[permalink] [raw]
Subject: Re: [PATCH 0/7] miscellaneous fixes intended for 3.7

On 10/15/2012 11:27 AM, John W. Linville wrote:
> On Wed, Oct 10, 2012 at 11:13:05AM -0700, Franky Lin wrote:
>> Here are some miscellaneous brcmfmac bug fix patches aiming for 3.7
>>
>> Arend van Spriel (1):
>> brcmfmac: remove 'always false' condition from brcmf_c_mkiovar_bsscfg
>>
>> Dan Carpenter (2):
>> brcmfmac: Using zero instead of NULL
>>
>> Franky Lin (3):
>> brcmfmac: fix sparse warnings
>
> These don't strike me as meaningful bug fixes. I'll hold them for 3.8.
>
> John
>

OK, thanks John.

Franky


2012-10-10 18:13:25

by Franky Lin

[permalink] [raw]
Subject: [PATCH 1/7] brcmfmac: handle all exceptions as an error.

From: Hante Meuleman <[email protected]>

in brcmf_usb_probe_cb only return code ENOLINK was seen as an
error. This is wrong, all error codes should be returned to usb
subsystem.

Reviewed-by: Arend Van Spriel <[email protected]>
Signed-off-by: Hante Meuleman <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index a2b4b1e..7a6dfdc 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -1339,7 +1339,7 @@ static int brcmf_usb_probe_cb(struct brcmf_usbdev_info *devinfo,
}

ret = brcmf_bus_start(dev);
- if (ret == -ENOLINK) {
+ if (ret) {
brcmf_dbg(ERROR, "dongle is not responding\n");
brcmf_detach(dev);
goto fail;
--
1.7.9.5



2012-10-10 18:13:22

by Franky Lin

[permalink] [raw]
Subject: [PATCH 4/7] brcmfmac: use control channel in roamed status reporting

Channel reported in scan results passed to cfg80211 is control
channel. But chanspec is reported while notifying cfg80211 about
roamed update. Cfg80211 complains because it could not find the
bss in the list. Report control channel while calling
cfg80211_roamed.

Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 26 +++++++++++++++-----
1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index 9c3fe1a..a39925a 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -4605,12 +4605,13 @@ brcmf_bss_roaming_done(struct brcmf_cfg80211_info *cfg,
struct brcmf_cfg80211_profile *profile = cfg->profile;
struct brcmf_cfg80211_connect_info *conn_info = cfg_to_conn(cfg);
struct wiphy *wiphy = cfg_to_wiphy(cfg);
- struct brcmf_channel_info_le channel_le;
- struct ieee80211_channel *notify_channel;
+ struct ieee80211_channel *notify_channel = NULL;
struct ieee80211_supported_band *band;
+ struct brcmf_bss_info_le *bi;
u32 freq;
s32 err = 0;
u32 target_channel;
+ u8 *buf;

WL_TRACE("Enter\n");

@@ -4618,11 +4619,22 @@ brcmf_bss_roaming_done(struct brcmf_cfg80211_info *cfg,
memcpy(profile->bssid, e->addr, ETH_ALEN);
brcmf_update_bss_info(cfg);

- brcmf_exec_dcmd(ndev, BRCMF_C_GET_CHANNEL, &channel_le,
- sizeof(channel_le));
+ buf = kzalloc(WL_BSS_INFO_MAX, GFP_KERNEL);
+ if (buf == NULL) {
+ err = -ENOMEM;
+ goto done;
+ }
+
+ /* data sent to dongle has to be little endian */
+ *(__le32 *)buf = cpu_to_le32(WL_BSS_INFO_MAX);
+ err = brcmf_exec_dcmd(ndev, BRCMF_C_GET_BSS_INFO, buf, WL_BSS_INFO_MAX);
+
+ if (err)
+ goto done;

- target_channel = le32_to_cpu(channel_le.target_channel);
- WL_CONN("Roamed to channel %d\n", target_channel);
+ bi = (struct brcmf_bss_info_le *)(buf + 4);
+ target_channel = bi->ctl_ch ? bi->ctl_ch :
+ CHSPEC_CHANNEL(le16_to_cpu(bi->chanspec));

if (target_channel <= CH_MAX_2G_CHANNEL)
band = wiphy->bands[IEEE80211_BAND_2GHZ];
@@ -4632,6 +4644,8 @@ brcmf_bss_roaming_done(struct brcmf_cfg80211_info *cfg,
freq = ieee80211_channel_to_frequency(target_channel, band->band);
notify_channel = ieee80211_get_channel(wiphy, freq);

+done:
+ kfree(buf);
cfg80211_roamed(ndev, notify_channel, (u8 *)profile->bssid,
conn_info->req_ie, conn_info->req_ie_len,
conn_info->resp_ie, conn_info->resp_ie_len, GFP_KERNEL);
--
1.7.9.5



2012-10-10 18:13:25

by Franky Lin

[permalink] [raw]
Subject: [PATCH 3/7] brcmfmac: fix sparse warnings

Following sparse warning is fixed:
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c:2518:21: warning: symbol 'brcmf_find_wpaie' was not declared. Should it be static?
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c:3768:1: warning: symbol 'brcmf_set_management_ie' was not declared. Should it be static?

Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index b1c9be7..9c3fe1a 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -2674,7 +2674,7 @@ brcmf_tlv_has_ie(u8 *ie, u8 **tlvs, u32 *tlvs_len,
return false;
}

-struct brcmf_vs_tlv *
+static struct brcmf_vs_tlv *
brcmf_find_wpaie(u8 *parse, u32 len)
{
struct brcmf_tlv *ie;
@@ -3963,7 +3963,7 @@ brcmf_vndr_ie(u8 *iebuf, s32 pktflag, u8 *ie_ptr, u32 ie_len, s8 *add_del_cmd)
return ie_len + VNDR_IE_HDR_SIZE;
}

-s32
+static s32
brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
struct net_device *ndev, s32 bssidx, s32 pktflag,
u8 *vndr_ie_buf, u32 vndr_ie_len)
--
1.7.9.5



2012-10-10 18:13:25

by Franky Lin

[permalink] [raw]
Subject: [PATCH 2/7] brcmfmac: Using zero instead of NULL

From: Dan Carpenter <[email protected]>

Sparse complains that we use zero instead of NULL here. In fact, the
initialization is wrong and should be removed. Doing these kinds of
bogus initializations means that GCC can't detect unitialized variables
and leads to bugs.

Reported-by: Fengguang Wu <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Signed-off-by: Dan Carpenter <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index c1abaa6..b1c9be7 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -3972,8 +3972,8 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
u8 *iovar_ie_buf;
u8 *curr_ie_buf;
u8 *mgmt_ie_buf = NULL;
- u32 mgmt_ie_buf_len = 0;
- u32 *mgmt_ie_len = 0;
+ u32 mgmt_ie_buf_len;
+ u32 *mgmt_ie_len;
u32 del_add_ie_buf_len = 0;
u32 total_ie_buf_len = 0;
u32 parsed_ie_buf_len = 0;
@@ -3995,8 +3995,7 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
case VNDR_IE_PRBRSP_FLAG:
mgmt_ie_buf = cfg->ap_info->probe_res_ie;
mgmt_ie_len = &cfg->ap_info->probe_res_ie_len;
- mgmt_ie_buf_len =
- sizeof(cfg->ap_info->probe_res_ie);
+ mgmt_ie_buf_len = sizeof(cfg->ap_info->probe_res_ie);
break;
case VNDR_IE_BEACON_FLAG:
mgmt_ie_buf = cfg->ap_info->beacon_ie;
--
1.7.9.5



2012-10-10 18:13:25

by Franky Lin

[permalink] [raw]
Subject: [PATCH 7/7] brcmfmac: fix end of loop check (signedness bug)

From: Dan Carpenter <[email protected]>

The problem here is that we loop until "remained_buf_len" is less than
zero, but since it is unsigned, it never is.

"remained_buf_len" has to be large enough to hold the value from
"mgmt_ie_buf_len". That variable is type u32, but it only holds small
values so I have changed to both variables to int.

Also I removed the bogus initialization from "mgmt_ie_buf_len" so that
GCC can detect if it is used unitialized. I moved the declaration of
"remained_buf_len" closer to where it is used so it's easier to read.

Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Signed-off-by: Dan Carpenter <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index 5e51f15..1c83612 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -3972,7 +3972,7 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
u8 *iovar_ie_buf;
u8 *curr_ie_buf;
u8 *mgmt_ie_buf = NULL;
- u32 mgmt_ie_buf_len;
+ int mgmt_ie_buf_len;
u32 *mgmt_ie_len;
u32 del_add_ie_buf_len = 0;
u32 total_ie_buf_len = 0;
@@ -3982,7 +3982,7 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
struct parsed_vndr_ie_info *vndrie_info;
s32 i;
u8 *ptr;
- u32 remained_buf_len;
+ int remained_buf_len;

WL_TRACE("bssidx %d, pktflag : 0x%02X\n", bssidx, pktflag);
iovar_ie_buf = kzalloc(WL_EXTRA_BUF_MAX, GFP_KERNEL);
--
1.7.9.5



2012-10-15 18:30:49

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 0/7] miscellaneous fixes intended for 3.7

On Wed, Oct 10, 2012 at 11:13:05AM -0700, Franky Lin wrote:
> Here are some miscellaneous brcmfmac bug fix patches aiming for 3.7
>
> Arend van Spriel (1):
> brcmfmac: remove 'always false' condition from brcmf_c_mkiovar_bsscfg
>
> Dan Carpenter (2):
> brcmfmac: Using zero instead of NULL
>
> Franky Lin (3):
> brcmfmac: fix sparse warnings

These don't strike me as meaningful bug fixes. I'll hold them for 3.8.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2012-10-10 18:13:25

by Franky Lin

[permalink] [raw]
Subject: [PATCH 6/7] brcmfmac: remove 'always false' condition from brcmf_c_mkiovar_bsscfg

From: Arend van Spriel <[email protected]>

The parameter buflen is unsigned so the condition buflen < 0 is
always false. The patch fixes the if statement checking the buffer
length.

Reported-by: Dan Carpenter <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/dhd_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c
index 15c5db5..a081e68 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c
@@ -106,7 +106,7 @@ brcmf_c_mkiovar_bsscfg(char *name, char *data, uint datalen,
namelen = (u32) strlen(name) + 1; /* lengh of iovar name + null */
iolen = prefixlen + namelen + sizeof(bssidx_le) + datalen;

- if (buflen < 0 || iolen > (u32)buflen) {
+ if ((u32)buflen < iolen) {
brcmf_dbg(ERROR, "buffer is too short\n");
return 0;
}
--
1.7.9.5